Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Integration with log libraries to be able to tag log messages with trace/transaction #520

Closed
alvarolobato opened this issue Jul 5, 2019 · 2 comments · Fixed by #586
Closed
Assignees

Comments

@alvarolobato
Copy link

alvarolobato commented Jul 5, 2019

No description provided.

@alvarolobato alvarolobato changed the title ntegration with log libraries to be able to tag log messages with trace/transaction Integration with log libraries to be able to tag log messages with trace/transaction Jul 5, 2019
@beniwohli
Copy link
Contributor

beniwohli commented Sep 4, 2019

I think the most reasonable approach here is to provide hooks for the different logging libraries and document how to use them. Alternatively, we could monkeypatch into the logging libraries, but IMO we should avoid that if there are perfectly fine extension points already provided by the libraries.

For the stdlib logging library, I see several approaches:

  • we could provide a logging.Logger subclass with an overriden makeRecord method that attaches transaction/trace IDs to the log record. This logger class can then be used by calling logging.setLoggerClass(OurCustomLogger)
    • Benefits: it's global, so the user doesn't have to set it up for every logger
    • Drawbacks:
      • not configurable via file config or dictConfig AFAICT
      • this isn't really what the logging API intends
  • we could provide a logging.Filter subclass that attaches the IDs to the record.
    • Benefits:
      • intended use of the API, makes us a good citizen
      • configurable through all usual channels (code, file, dictConfig)
    • Drawbacks:
      • configuration needs to be done for every logger that the user configured AFAICT
  • we could provide a logging.LoggerAdapter subclass that attaches the IDs to the record.
    • Benefits: same as Filter
    • Drawbacks: AFAICT, this can't be configured via file or dictConfig

IMO, a custom Filter seems to be the best match for our use case.

For structlog, providing a custom processor seems the way to go: https://www.structlog.org/en/stable/api.html#module-structlog.processors

@basepi
Copy link
Contributor

basepi commented Sep 4, 2019

Filters are perfect for this task. I also assumed that I could probably insert the Filter automagically into the root logger and have all the descendent loggers pick it up, unfortunately this does not appear to be the case

Note that filters attached to handlers are consulted before an event is emitted by the handler, whereas filters attached to loggers are consulted whenever an event is logged (using debug(), info(), etc.), before sending an event to handlers. This means that events which have been generated by descendant loggers will not be filtered by a logger’s filter setting, unless the filter has also been applied to those descendant loggers.

So, I'm planning to provide a Filter class that users can add to their logging handlers (as handlers do propagate to descendants). Not as automagical as I might wish, but it will still be pretty trivial to consume.

Same with the structlog processor. Can't insert it myself, but I can provide it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants