-
-
Notifications
You must be signed in to change notification settings - Fork 653
Add handler to celery task logger #778
Conversation
muravitskiy
commented
Jun 2, 2016
+1 for this, but maybe you should separate registering global logger handlers and task logger handlers
register_logger_signal(client, loglevel=logging.ERROR) register_task_logger_signal(client, loglevel=logging.ERROR)
This can be done by passing celery signal as argument and creating some proxy-functions
... from celery.signals import after_setup_logger, after_setup_task_logger ... def _register_logger_signal(signal, client, logger=None, loglevel=logging.ERROR): filter_ = CeleryFilter() handler = SentryHandler(client) handler.setLevel(loglevel) handler.addFilter(filter_) def process_logger_event(sender, logger, loglevel, logfile, format, colorize, **kw): # Attempt to find an existing SentryHandler, and if it exists ensure # that the CeleryFilter is installed. # If one is found, we do not attempt to install another one. for h in logger.handlers: if type(h) == SentryHandler: h.addFilter(filter_) return False logger.addHandler(handler) signal.connect(process_logger_event, weak=False) def register_logger_signal(client, logger=None, loglevel=logging.ERROR): _register_logger_signal(after_setup_logger, client, logger, loglevel) def register_task_logger_signal(client, logger=None, loglevel=logging.ERROR): _register_logger_signal(after_setup_task_logger, client, logger, loglevel)
omarkhan
commented
Jun 3, 2016
@muravitskiy I'm not sure I understand the benefit of your approach. What's wrong with using the same handler for the global logger and the task loggers?
d0cd326 to
4f0d5f2
Compare
muravitskiy
commented
Jun 3, 2016
@omarkhan Celery separates this two loggers and you should be able to configure them separately
+1 for this as well. I was debugging why logger.error() messages inside celery workers aren't propagated to sentry and reached the same conclusion.
Agreed, this change allowed me to properly log to sentry.io from within a celery task using logger.error(). Trying to log error messages (not exceptions) to sentry from a celery task seems to fail otherwise with the usual setup:
https://docs.sentry.io/clients/python/integrations/celery/
Using
django==1.9.9
django-celery==3.1.17
raven==5.27.1
joshma
commented
Jul 31, 2018
Thank you @omarkhan for this - it's a few years later, but we've taken this PR and attached a similar handler from the outside. Would be great to merge!
Uh oh!
There was an error while loading. Please reload this page.
Celery sets up 2 kinds of loggers: a global logger and task loggers. The celery docs include an example on how to use the task loggers, which makes task loggers look like the recommended way to log from celery tasks.
The
register_logger_signalhook in the sentry celery integration adds a handler to the global celery logger using theafter_setup_loggersignal. This does not affect the task loggers, meaning that any exception logged by a task logger will not be captured by sentry.This pull request modifies
register_logger_signalto connect to theafter_setup_task_loggersignal as well, allowing exceptions logged by task loggers to be captured.This change is Reviewable