Page MenuHomeSoftware Heritage

Fix logging configuration and harmonize loggers use
ClosedPublic

Authored by anlambert on Apr 5 2019, 4:47 PM.

Details

Summary

While testing kafka components inside the docker-compose environement, I noticed some
logger outputs were missing (notably the journal publisher ones).

That diff fixes the logger configurations by using logging.basicConfig in the
cli entrypoint. All loggers from swh-journal will inherit from this default
configuration.

Also, harmonize the way a logger is created: prefer to do that in class constructor
instead of doing it at module level.

Diff Detail

Repository
rDJNL Journal infrastructure
Branch
fix-logging
Lint
No Linters Available
Unit
No Unit Test Coverage
Build Status
Buildable 5153
Build 6935: tox-on-jenkinsJenkins
Build 6934: arc lint + arc unit

Event Timeline

ardumont added inline comments.
swh/journal/cli.py
48

Can you please add the %(name)s which gives the logger name.

As in:

format='%(asctime)s %(process)d %(levelname)s %(name)s %(message)s',
olasd requested changes to this revision.Apr 8 2019, 11:40 AM
olasd added a subscriber: olasd.

I'm fine with harmonizing all loggers to use logging.getLogger(__name__), but I'm not convinced by the "object-level" logger pattern. In this specific instance it looks like it's causing issues because we're mocking a class.

If a diff is touching all logging statements, we should also make sure to use logger.debug("format string", argument1, argument2) instead of logger.debug("format string" % (argument1, argument2)). This way the arguments end up in structured logging, and we don't pay the interpolation cost when the log line isn't output.

This revision now requires changes to proceed.Apr 8 2019, 11:40 AM

Update: rebase to master, address olasd comment and remove unused logger in journal.client

This revision is now accepted and ready to land.Apr 9 2019, 5:34 PM
This revision was landed with ongoing or failed builds.Apr 9 2019, 5:35 PM
This revision was automatically updated to reflect the committed changes.