getsentry/sentry-python

Make `ignore_logger()` accept a log level

Opened this issue · 4 comments

Problem Statement

We've set LoggingIntegration(level=logging.INFO, event_level=logging.ERROR, sentry_logs_level=logging.INFO), and we have a somewhat noisy dependency that does a lot of logging with pretty much the whole range of levels.

We want to avoid sending the DEBUG and INFO logs of that dependency to Sentry, but we'd still like logs with WARNING and above to be sent, which there doesn't seem to be any simple way of doing (other than e.g. providing both before_breadcrumb and before_send_log with a callback that does that filtering manually).

Solution Brainstorm

Change the ignore_logger() implementation to something like:

from logging import _checkLevel

_IGNORED_LOGGERS = {
    "sentry_sdk.errors": logging.CRITICAL,
    "urllib3.connectionpool": logging.CRITICAL,
    "urllib3.connection": logging.CRITICAL,
}


def ignore_logger(
    name, *, level_or_below=logging.CRITICAL
):
    _IGNORED_LOGGERS[name] = _checkLevel(level_or_below)

and then change _BaseHandler._can_record() to:

class _BaseHandler(logging.Handler):
    def _can_record(self, record):
        for logger, level_or_below in _IGNORED_LOGGERS.items():
            if (
                fnmatch(record.name.strip(), logger)
                and record.levelno <= level_or_below
            ):
                return False
        return True

Hello team, i'm going through the codebase of sentry as a hobby, would be interested in picking this up if its approved

Hey @ddabble, the proposal sounds good to me. As long as we don't break backwards compatibility (i.e., the level in ignore_logger is optional and if you don't specify it, everything works as before) and the logger-specific levels interact with the integration-level options in a way that makes sense, this is ok to add from my POV.

@YashKumarVerma Up for grabs if you're still interested.

As long as we don't break backwards compatibility (i.e., the level in ignore_logger is optional and if you don't specify it, everything works as before) and the logger-specific levels interact with the integration-level options in a way that makes sense, this is ok to add from my POV.

Nice, that sounds good to me! 😊 And yes, if something similar to my suggestion with adding a level_or_below argument is implemented, the default value of logging.CRITICAL should make it backward-compatible, as CRITICAL is the highest publicly defined value of the logging library. It can be mentioned that it's of course possible to create logs whose level is any numerical value, so if we want to be extra sure, we could set the default value to e.g. float("inf") - though that feels maybe slightly overly cautious 😅