1419989 - rework logging

Status: RESOLVED FIXED

(Socorro :: General, task, P2)

Description

[Will Kahn-Greene [:willkg] ET needinfo? me]

Python has a logging module that allows you to declare loggers at Python module import time and configure the whole logging system later when it's convenient for your application's bootstrapping. Socorro apps use this module for logging, but they set up a Socorro logger for the whole app and then pass it along in config.

There are some nice things about having this, namely it's easier to mock logging in the system, however the tangling makes architecture a little trickier since you have to make sure the logger gets passed around correctly and all that has to happen after the application has bootstrapped.

This bug covers removing the socorro logger and changing all the code that expects it to be in config to instead pull it from a module-level logger. In that way, we de-tangle the yarn of logging and simplify the app a bit.

Comment 1

[Will Kahn-Greene [:willkg] ET needinfo? me]

One consequence of this is that we'll probably need a logging mock and have to do some test rewriting. There are some libraries that do this. I wrote one for Antenna:

https://github.com/mozilla-services/antenna/blob/master/testlib/loggingmock.py

We could use that. If we don't, then we might want to update Antenna with what we do use so that both projects use the same system.

Comment 2

[Will Kahn-Greene [:willkg] ET needinfo? me]

I want to do a few stages of fixing logging.

First, I want to change setup_logging so it uses logging.config.dictConfig(). That makes it easier to read and manipulate.

Second, I want to remove all the syslog setup--we don't need that anymore.

Third, I want to go through and remove logging as a thing in config. That's a bit more involved.

Fourth, I want to go through the tests and redo things to use pytest's caplog:

https://docs.pytest.org/en/latest/logging.html

That'll simplify logging bits in the tests and we can drop a bunch of mocking.

Why do all this? Socorro doesn't have the same needs it did when this code was written. Redoing this now simplifies how Socorro uses logging, switches to  best practices that other projects use, makes it easier to switch to Dockerflow's mozlog format later, and lets us remove a bunch of code.

Throwing it in my queue to start working through.

Comment 3

[Will Kahn-Greene [:willkg] ET needinfo? me]

Attachment: pr 4398: bug 1419989 - drop syslog and switch to logging.config.dictConfig

Comment 4

[[github robot]]

Commits pushed to master at https://github.com/mozilla-services/socorro

https://github.com/mozilla-services/socorro/commit/cbe8040d6c65e06c6602be5e587f46b7ea17c30a
bug 1419989 - drop syslog and switch to logging.config.dictConfig

https://github.com/mozilla-services/socorro/commit/a9f141804b9f20ad7d40eedb21b448fec56fb469
Merge pull request #4398 from willkg/1419989-dict-config

bug 1419989, 1412590 - fix logging and metrics

Comment 5

[Will Kahn-Greene [:willkg] ET needinfo? me]

PR 4398 did the following:

1. Change setup_logging() to use logging.config.dictConfig()

2. Nix all the syslog setup.


Things that are not done:

3. Remove the logger from config.

4. Redo the tests using pytest's caplog fixture. This is hard to do currently since our tests derive from unittest's TestCase.


Neither of those are critical. I think they clean up the code a bit, but behaviorally it's effectively the same. Given that, I'm going to close this out and if we want to do those things, we can put them in new bugs.