Sentry uses logging's levelname instead of levelno when mapping to levels.

[rrauenza]

How do you use Sentry?

Self-hosted/on-premise

Version

1.5.12

Steps to Reproduce

Where I work, it has been a convention to do the following in order to get python logging to modify its output to be a little cleaner:

logging.addLevelName(logging.DEBUG, "debug: ")
logging.addLevelName(logging.INFO, "")
logging.addLevelName(logging.WARNING, "warning: ")
logging.addLevelName(logging.ERROR, "error: ")
logging.addLevelName(logging.CRITICAL, "critical: ")
logging.addLevelName(logging.FATAL, "fatal: ")`

This along with:

 _FMT = "%(asctime)s.%(msecs)03d " + SCRIPT_NAME + ": %(levelname)s%(message)s"
LOGGING_FORMATTER = logging.Formatter(_FMT, _DATEFMT)
handler.setFormatter(LOGGING_FORMATTER)

This causes a sentry error that the level name of warning: doesn't exist. Probably because sentry sdk is mapping to sentry levels using levelname of the log message and not levelno.

Expected Result

Sentry should map from the logging level correctly.

Actual Result

I get an invalid level name of 'warning:' in the sentry record in the GUI.

I'll include my work around below in further comment.

[rrauenza]

This might just need to have a warning in the docs that sentry expects you not to override your level names, and that doing so can break sentry level reporting (although changing the names to lower case is handled ok.)

My work around was to no longer modify the levelname in my logging setup, but rather to make a custom formatter that dynamically changes the formatting based on the level, per https://stackoverflow.com/questions/1343227/can-pythons-logging-format-be-modified-depending-on-the-message-log-level

But I also wonder if the project wants to consider using record.levelno instead of record.levelname. That could be a breaking change, however....

[rrauenza]

The code in question is:

def _logging_to_event_level(levelname):
    # type: (str) -> str
    return {"critical": "fatal"}.get(levelname.lower(), levelname.lower())

I would like to suggest that a more correct implementation would be something like:

levelno_to_sentry_level = {
  logging.INFO: 'info',
  logging.WARNING: 'warning',
  logging.CRITICAL: 'critical',
  logging.DEBUG: 'debug',
  logging.ERROR: 'error',
  logging.FATAL: 'fatal', # CRITICAL is same as FATAL
} 

def _logging_to_event_level(record):
   return levelno_to_sentry_level.get(record.levelno, 'error')

[antonpirker]

Hey @rrauenza !

Thanks for bringing this up! I agree matching the event name with the log level instead number instead of the log level name is way more robust.

If you provide a PR than I am happy to review and merge it!

[rrauenza]

Is 'error' the default we want to use if the levelno is unknown?

def _logging_to_event_level(record):
   return levelno_to_sentry_level.get(record.levelno, 'error')

...before, if the levelname was 'foo' we would just post to sentry with 'foo' and complain.

[antonpirker]

Good question.

I think we should not change the behavior in case the log level name is unknown. (Because we would probably break users existing code)

So if the level can not be matched the lower case level name should be used (as it is now)

[antonpirker]

Hey @rrauenza, I have reviewed your PR: #1449

[rrauenza]

@antonpirker PR has been updated.

[antonpirker]

This issue is fixed in #1449 and will be released soon.

Thanks again @rrauenza for your contribution! (See the PR on how to get you some stickers as a thank you!)