Celery add support for anon tasks #1052
Conversation
There was a problem hiding this comment.
@goatsthatcode Did you run the reproducible example provided in #609 and can confirm it is fixed?
| @@ -132,6 +132,11 @@ def attach_span(task, task_id, span, is_publish=False): | |||
| NOTE: We cannot test for this well yet, because we do not run a celery worker, | |||
| and cannot run `task.apply_async()` | |||
| """ | |||
| if task is None: | |||
There was a problem hiding this comment.
I'll copy the comment I wrote on the original PR in your fork for the wide discussion:
I think the effect of this return statement is that the span (along with activation) will not be recorded into the CTX_KEY, which means that a later invocation of the _trace_after_publish signal will not be able to retrieve_span thus will not end the span.
Am I missing something?
Didn't actually try to run it, so I might just not fully understand the details of what causes the "publish" span to be ended.
|
Hi guys, sorry for the radio silence here. I got really sick this last week so was offline for a bit. I am picking up the remaining pieces today to make sure this at least ready to be merged (unless any additional comments come up). Sorry for the delay :D |
|
Please resolves build errors. |
|
@goatsthatcode can you please resolve the build errors? |
| def test_hidden_task(self): | ||
| # no-op since already instrumented | ||
| CeleryInstrumentor().instrument() | ||
| import ipdb; ipdb.set_trace() |
There was a problem hiding this comment.
| import ipdb; ipdb.set_trace() | |
| import ipdb | |
| ipdb.set_trace() |
There was a problem hiding this comment.
pytest-ipdb is unfortunately not supported anymore.
You can remove these lines.
The solution is to add "--pdb --pdbcls=IPython.terminal.debugger:Pdb" with pytest command
From the help command:
pytest -h
--pdb start the interactive Python debugger on errors.
--pdbcls=modulename:classname
start a custom interactive Python debugger on errors.
For example:
--pdbcls=IPython.terminal.debugger:TerminalPdb
| self.assertEqual( | ||
| producer.name, "apply_async/app.hidden_task" | ||
| ) |
There was a problem hiding this comment.
| self.assertEqual( | |
| producer.name, "apply_async/app.hidden_task" | |
| ) | |
| self.assertEqual(producer.name, "apply_async/app.hidden_task") |
There was a problem hiding this comment.
Are we still working on this issue ?
|
There are still some lint and generate build issues. Please do resolve them |
|
@goatsthatcode i'm very interested in this fix, are you still working on it? if not, could i take it from here? |
|
succeeded by #1407 |
Description
This changes the way spans are initialized for the celery instrumentation hooks and handles the case where Celery tasks are not available in the registry and called anonymously using the
signature()orsend_task()Fixes #1002, #609
possibly addresses #784
Type of change
Please delete options that are not relevant.
How Has This Been Tested?
Please describe the tests that you ran to verify your changes. Provide instructions so we can reproduce. Please also list any relevant details for your test configuration
Does This PR Require a Core Repo Change?
Checklist:
See contributing.md for styleguide, changelog guidelines, and more.