Page MenuHomeSoftware Heritage
Differential D1357

Fix logging configuration and harmonize loggers use
ClosedPublic
Actions

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

Details

Reviewers
olasd
Group Reviewers
Reviewers
Commits
rDJNL03c1f19a5130: Harmonize loggers and remove unused one in journal.client
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

anlambert created this revision.Apr 5 2019, 4:47 PM
Herald added a reviewer: Reviewers. · View Herald TranscriptApr 5 2019, 4:47 PM
swh-public-ci added a comment.Apr 5 2019, 4:54 PM

Build has FAILED

Link to build: https://jenkins.softwareheritage.org/job/DJNL/job/tox/105/
See console output for more information: https://jenkins.softwareheritage.org/job/DJNL/job/tox/105/console

Harbormaster failed remote builds in B5139: Diff 4336!Apr 5 2019, 4:54 PM
anlambert updated this revision to Diff 4345.Apr 5 2019, 5:30 PM

Update: add log format

Harbormaster failed remote builds in B5148: Diff 4345!Apr 5 2019, 5:31 PM
swh-public-ci added a comment.Apr 5 2019, 5:31 PM

Build has FAILED

Link to build: https://jenkins.softwareheritage.org/job/DJNL/job/tox/113/
See console output for more information: https://jenkins.softwareheritage.org/job/DJNL/job/tox/113/console

ardumont added a subscriber: ardumont.Apr 5 2019, 5:44 PM
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',
anlambert updated this revision to Diff 4350.Apr 5 2019, 5:46 PM

Update log format

Harbormaster failed remote builds in B5153: Diff 4350!Apr 5 2019, 5:47 PM
swh-public-ci added a comment.Apr 5 2019, 5:47 PM

Build has FAILED

Link to build: https://jenkins.softwareheritage.org/job/DJNL/job/tox/114/
See console output for more information: https://jenkins.softwareheritage.org/job/DJNL/job/tox/114/console

anlambert updated this revision to Diff 4351.Apr 5 2019, 5:53 PM

Rebase

swh-public-ci added a comment.Apr 5 2019, 5:55 PM

Build has FAILED

Link to build: https://jenkins.softwareheritage.org/job/DJNL/job/tox/115/
See console output for more information: https://jenkins.softwareheritage.org/job/DJNL/job/tox/115/console

Harbormaster failed remote builds in B5154: Diff 4351!Apr 5 2019, 5:55 PM
anlambert added a child revision: D1358: Add extra configuration to kafka consumer from journal.client.Apr 5 2019, 6:00 PM
anlambert edited the summary of this revision. (Show Details)Apr 6 2019, 1:30 AM
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
anlambert updated this revision to Diff 4458.Apr 9 2019, 5:33 PM

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

olasd accepted this revision.Apr 9 2019, 5:34 PM
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
Closed by commit rDJNL03c1f19a5130: Harmonize loggers and remove unused one in journal.client (authored by anlambert). · Explain Why
This revision was automatically updated to reflect the committed changes.
swh-public-ci added a comment.Apr 9 2019, 5:37 PM

Build has FAILED

Link to build: https://jenkins.softwareheritage.org/job/DJNL/job/tox/134/
See console output for more information: https://jenkins.softwareheritage.org/job/DJNL/job/tox/134/console

Harbormaster failed remote builds in B5266: Diff 4458!Apr 9 2019, 5:37 PM

Revision Contents
Changeset List

PathSize
swh/
journal/
10 lines
2 lines
5 lines
32 lines
4 lines

Diff 4350

View Options

swh/journal/cli.py

Loading...
View Options

swh/journal/client.py

Loading...
View Options

swh/journal/direct_writer.py

Loading...
View Options

swh/journal/publisher.py

Loading...
View Options

swh/journal/replay.py

Loading...
About · Developer information · Legal