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

[celery] patch TaskRegistry to support old-style task with ddtrace-run #484

Merged
merged 1 commit into from
Jun 11, 2018

Conversation

palazzem
Copy link

Overview

After solving #465 #469 to fix #423, we discovered that ddtrace-run wasn't instrumenting old-style tasks. The issue was related to the fact that celery.task.Task (Task base class) wasn't instrumented by our system; unfortunately, it wasn't possible to instrument it directly because of how the base class is used by Celery. Indeed, a direct instrumentation of celery.task.Task ended with the following exception:

[2018-06-11 12:37:50,595: ERROR/MainProcess] Received unregistered task of type u'_get_exec_options'.
The message has been ignored and discarded.

Did you remember to import the module containing this task?
Or maybe you're using relative imports?

Please see
http://docs.celeryq.org/en/latest/internals/protocol.html
for more information.

The full contents of the message body was:
'[[], {"stop": true}, {"chord": null, "callbacks": null, "errbacks": null, "chain": null}]' (89b)
Traceback (most recent call last):
  File "[...]/celery/worker/consumer/consumer.py", line 561, in on_task_received
    strategy = strategies[type_]
KeyError: u'_get_exec_options'

With this patch, we properly patch a Task when it's registered instead of doing it at import time.

@palazzem palazzem added this to the 0.12.1 milestone Jun 11, 2018
@alxch- alxch- self-requested a review June 11, 2018 14:24
Copy link
Contributor

@alxch- alxch- left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Very good idea to directly patch register 👍

@palazzem palazzem merged commit 3023caa into master Jun 11, 2018
@palazzem palazzem deleted the palazzem/celery-old-style-task branch June 11, 2018 14:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

ddtrace-run breaks Celery support, have to patch manually
2 participants