Add messaging.system for celery

[shivanshuraj1333]

Description

Please include a summary of the change and which issue is fixed. Please also include relevant motivation and context. List any dependencies that are required for this change.

Fixes # #2913

Type of change

Please delete options that are not relevant.

• Bug fix (non-breaking change which fixes an issue)
• New feature (non-breaking change which adds functionality)
• Breaking change (fix or feature that would cause existing functionality to not work as expected)
• This change requires a documentation update

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

• Test locally

Does This PR Require a Core Repo Change?

• Yes. - Link to PR:
• No.

Checklist:

See contributing.md for styleguide, changelog guidelines, and more.

• Followed the style guidelines of this project
• Changelogs have been updated
• [] Unit tests have been added
• [] Documentation has been updated

[xrmx]

Please also update tests

[shalevr]

I’m not sure if “celery” is the right value here.
According to https://opentelemetry.io/docs/specs/semconv/messaging/messaging-spans/#messaging-attributes
the expected values are Kafka, ActiveMQ, RabbitMQ, etc.
What do you think?

[xrmx]

The semconv states: "If one of them applies, then the respective value MUST be used; otherwise, a custom value MAY be used."
We can add celery to the semconv but I don't see it as blocker since the intent to describe the system is clear.

[xrmx]

Here's a pr for adding celery to semantic conventions open-telemetry/semantic-conventions#1954

[xrmx]

Discussed this a bit in the semconv pr above, would it be possible to use the name of broker we are using?

[shivanshuraj1333]

that requires extracting broker info from the context. The context that we instrument doesn't have that info.

[shivanshuraj1333]

code references:
-> these are the context attributes that are available in celery https://github.com/celery/celery/blob/main/celery/app/task.py#L60

-> these we instrument https://github.com/open-telemetry/opentelemetry-python-contrib/blob/main/instrumentation/opentelemetry-instrumentation-celery/src/opentelemetry/instrumentation/celery/utils.py#L35

IMO, there's no easy way to figure out the backend info (when I was looking into it some time back), and hence having celery is backend makes most sense to me. Ideally I agree having more fine grained info about makes most sense (from sem conv perspective). But do we have a way to get backend info?

[lmolkova]

it seems celery has a bunch of celery-specific attributes and only a minor intersection with messaging - in particular it only sets these two mesaging attributes

opentelemetry-python-contrib/instrumentation/opentelemetry-instrumentation-celery/src/opentelemetry/instrumentation/celery/__init__.py

Lines 246 to 247 in 437648e

	span.set_attribute(SpanAttributes.MESSAGING_MESSAGE_ID, task_id)
	span.set_attribute(SpanAttributes.MESSAGING_SYSTEM, _QUEUE_NAME)

Messaging semconv and not designed for background task processing and have very limited usefullness for in-memory queues. I think it's misleading to try to use messaging semconv for celery and the way they are used today (or proposed in this PR) is very far from semconv.

I suggest the following mental model:

• celery instrumentation may create spans that describe celery only and might provide a bit of context into the underlying stack (messaging/db/in-memory). These spans don't follow messaging semconv. There could be celery-specific semconv documented in this repo.
• underlying messaging/db library is instrumented too and provide the real messaging/db observability
• together they are two different correlated layers and users may install both instrumentation or just one of them depending on the level of details they want

[lmolkova]

Ah, I missed a few more attributes here

opentelemetry-python-contrib/instrumentation/opentelemetry-instrumentation-celery/src/opentelemetry/instrumentation/celery/utils.py

Lines 95 to 116 in 437648e

	SpanAttributes.MESSAGING_DESTINATION, routing_key
	)
	value = str(value)
	elif key == "id":
	attribute_name = SpanAttributes.MESSAGING_MESSAGE_ID
	elif key == "correlation_id":
	attribute_name = SpanAttributes.MESSAGING_CONVERSATION_ID
	elif key == "routing_key":
	attribute_name = SpanAttributes.MESSAGING_DESTINATION
	# according to https://docs.celeryproject.org/en/stable/userguide/routing.html#exchange-types
	elif key == "declare":
	attribute_name = SpanAttributes.MESSAGING_DESTINATION_KIND
	for declare in value:
	if declare.exchange.type == "direct":
	value = "queue"
	break
	if declare.exchange.type == "topic":

which also add conversation id and destination name (routing key).

It does not change a general impression: celery is not a messaging system and does not (potentially cannot) provide messaging-level instrumentation