search

open-telemetry / opentelemetry-python-contrib

OpenTelemetry instrumentation for Python modules
https://opentelemetry.io
Apache License 2.0
739 stars 612 forks source link

Should celery add a span link instead? #3002

Open Kludex opened 1 week ago

Kludex commented 1 week ago

Describe your environment

Currently, the celery task that is executing gets the context from the headers, and attaches it to the new span:

https://github.com/open-telemetry/opentelemetry-python-contrib/blob/e3ba54b95c68a0b56daccb80cd57f1987714ddf4/instrumentation/opentelemetry-instrumentation-celery/src/opentelemetry/instrumentation/celery/__init__.py#L173-L180

I don't think this is the correct behavior. It should actually be a span link - according to the docs:

Links exist so that you can associate one span with one or more spans, implying a causal relationship. For example, let’s say we have a distributed system where some operations are tracked by a trace.

Which is exactly this case.

When a web framework triggers a celery task, it shouldn't be on the same trace. It's already a new one... But they are linked through the span link.

Would you like to implement a fix?

Yes

samuelcolvin commented 1 week ago

agreed, I think this is exactly what span links are for.

unflxw commented 1 week ago

To add to the agreement, note that this is the behaviour speciified in the OpenTelemetry Semantic Conventions for Messaging Spans (see "Trace structure" and "Examples")