ext/celery: propagate the span and link the publisher to the runner#957
ext/celery: propagate the span and link the publisher to the runner#957nightmared wants to merge 3 commits intoopen-telemetry:masterfrom
Conversation
nightmared
force-pushed
the
celery-propagate-span
branch
from
2e02a47 to
bd70461
Compare
nightmared
force-pushed
the
celery-propagate-span
branch
from
bd70461 to
3e3b159
Compare
codeboten
added
the
instrumentation
| parent=parent, | ||
| links=links, | ||
| kind=trace.SpanKind.CONSUMER, | ||
| ) |
There was a problem hiding this comment.
Can the else condition be removed and parent be passed to the function always. If it is None, it shouldn't have any effect on the span right? Rest of the arguments seem to be exactly the same.
| headers[CTX_PUBLISHER_SPAN_ID] = str(ctx.span_id) | ||
| headers[CTX_PARENT_TRACE_FLAGS_SAMPLED] = getattr( | ||
| parent_ctx.trace_flags, "sampled", False, | ||
| ) |
There was a problem hiding this comment.
We should just use the configured propagator here to inject trace context and use propagator again to extract the context on the other side. No reason celery instrumentation should ignore globally configured propagators.
|
Thanks for the review, alas I no longer work for the company where I worked on this (I only did a short internship there). I will probably take a look at this, but maybe not very soon (I no longer have access to the project upon which I tested this, and this is not my uttermost priority currently). I will keep you posted. |
|
@nightmared Thanks for replying. Do you mind if I submit another PR for this? I kinda need it urgently and can submit a PR within the next day or two. |
|
No worries, sure thing ! To be honest, I am even happier if I don't have to do it myself ;) Thanks ! |
|
The issue this PR was addressing was fixed in #1135 |
Co-authored-by: Daniel Dyla <dyladan@users.noreply.github.com>
3 participants
Description
This change propagates the {
trace_id,span_id} of the parent span and thespan_idof the span of the publisher task.Fixes #876
Type of change
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
django-email-celeryexample from Celery Instrumentation with apply_async function #876. I uploaded the modified version here, along with a screenshot of the result in jaeger.Checklist:
django-email-celery.zip