Standardize logging, and add loggers for tools.
Review Request #11568 — Created April 1, 2021 and submitted
Review Bot's logging was all over the place. Some code was using the
baselogging
module, some was callinglogging.getLogger()
, and some
was (rightfully) using Celery'sget_logger()
orget_task_logger()
functions. The Celery versions are pretty important, given how much
Celery controls the environment that our code runs in, so we actually
had some logging that didn't really work before.This change aims to standardize logging, future-proofing it at the same
time. We now have our ownreviewbot.utils.log.get_logger()
function
that wraps Celery'sget_logger()
/get_task_logger()
. It defaults to
creating task loggers, since most operations will run within the context
of a task.
BaseTool
provides a newlogger
property for logging. It will
dynamically construct/retrieve a logger as needed, determining whether
that logger will be a task logger or a standard logger. This simplifies
tools even further, taking the guess work out of each implementation.The log message formats have been altered to be a little bit nicer and
more consistent with typical log messages, and to include the logger
name (which Celery's default message format omits).Some log levels have also been adjusted here and there, to cut down on
unwanted noise.
Unit tests pass for Python 2.7 and 3.x.
Verified that the logs on startup (for dependency checks and configuration)
were using standard loggers without task information, but that all the log
messages going through during a task run were showing task information.
Summary | ID |
---|---|
8bea4a0440ff763d25d1d1e700ff6e3a6f086397 |
Description | From | Last Updated |
---|---|---|
I think it's more useful to have this in the method definition rather than the module. Or perhaps both. |
david |
- Change Summary:
-
Added a
Version Added
toget_logger()
. - Description:
-
Review Bot's logging was all over the place. Some code was using the
base logging
module, some was callinglogging.getLogger()
, and somewas (rightfully) using Celery's get_logger()
orget_task_logger()
functions. The Celery versions are pretty important, given how much Celery controls the environment that our code runs in, so we actually had some logging that didn't really work before. This change aims to standardize logging, future-proofing it at the same
time. We now have our own reviewbot.utils.log.get_logger()
functionthat wraps Celery's get_logger()
/get_task_logger()
. It defaults tocreating task loggers, since most operations will run within the context of a task. ~ BaseTool
provides a enwlogger
property for logging. It will~ BaseTool
provides a newlogger
property for logging. It willdynamically construct/retrieve a logger as needed, determining whether that logger will be a task logger or a standard logger. This simplifies tools even further, taking the guess work out of each implementation. The log message formats have been altered to be a little bit nicer and
more consistent with typical log messages, and to include the logger name (which Celery's default message format omits). Some log levels have also been adjusted here and there, to cut down on
unwanted noise. - Commits:
-
Summary ID 6121586985943d112640f1cf0d749b6432e6a84b 8bea4a0440ff763d25d1d1e700ff6e3a6f086397