getsentry/sentry-python

Add an `ignore_loggers` kwarg to `LoggingIntegration`

Opened this issue · 3 comments

ddabble commented

Problem Statement

It's simply more convenient to have a kwarg for this, instead of having to call ignore_logger() multiple times 🙂

Solution Brainstorm

class LoggingIntegration(Integration):
    def __init__(
        self,
        ...,
        ignore_loggers=None,
    ):
        ...
        # --- Either: ---
        for logger in ignore_loggers or ():
            ignore_logger(logger)
        # --- Or: ---
        _IGNORED_LOGGERS.update(ignore_loggers or ())
sentrivana commented

Hey @ddabble, thanks for the suggestion. Just out of curiosity, how many loggers do you need to ignore in your setup?

In principle we can add a new way to configure multiple loggers at once. Some random thoughts for posterity:

  • In SDK 3.0 (some weeks/months away), we'll be changing the logging integration so that error logs by default don't create events, so there will likely be less of a need to ignore random loggers
  • Ideally the new way of ignoring loggers would be more like the existing way of doing it. I.e., maybe we can have an ignore_loggers([logger1, logger2, ...]) top-level function to complement (or even replace) ignore_logger. Or we get rid of ignore_logger (needs a new major, since it's a breaking change) and exclusively use LoggingIntegration(ignore_loggers=[]). Otherwise, if we had both an integration-level option as well as top-level function, I can imagine this being confusing as to if there is any difference between the two.
ddabble commented

Just out of curiosity, how many loggers do you need to ignore in your setup?

Currently, we've only felt the need to ignore one logger, but we've not been using the logging integration for very long, so I can imagine that we'll find a couple more in the coming months 😅 But it's absolutely not an unwieldy amount :)

For extra context, I can add that we do in practice ignore many more loggers (including the previously mentioned one), using Django's LOGGING setting in our "plain" Django containers. However, we're also using Celery, which has its own logging setup that more or less entirely overrides our Django LOGGING setup - but only in the Celery containers. That's a behavior we'd like to keep when developing locally, but not in prod, and it seems like the simplest solution for us is to make Sentry ignore the logger I mentioned in the beginning.

  • In SDK 3.0 (some weeks/months away), we'll be changing the logging integration so that error logs by default don't create events, so there will likely be less of a need to ignore random loggers

Oh that's neat! ..though not very relevant for our particular case, as the initially mentioned logger mainly produces INFO logs 🙂

  • Ideally the new way of ignoring loggers would be more like the existing way of doing it. I.e., maybe we can have an ignore_loggers([logger1, logger2, ...]) top-level function to complement (or even replace) ignore_logger. Or we get rid of ignore_logger (needs a new major, since it's a breaking change) and exclusively use LoggingIntegration(ignore_loggers=[]).

All our current Sentry settings are set through sentry_sdk.init(), so the ignore_logger() function does stand out in that regard - unless it's intended to be used to dynamically ignore loggers at various points after the program has been initialized, which I find hard to imagine a use case for 🤔 So I'd personally vote for having it as a kwarg to LoggingIntegration(), to be more in line with the way most other settings are set :)

Otherwise, if we had both an integration-level option as well as top-level function, I can imagine this being confusing as to if there is any difference between the two.

I agree, there should preferably be just one option - unless dynamically ignoring loggers is a use case, as mentioned above 👍

  • 👍1