fix(instrumentation-asgi): remove high cardinal path from span name #2650

srikanthccv · 2024-07-02T08:21:45Z

Description

The path value of the ASGI scope is a non-generalized path. This is not an issue in other frameworks such as fastapi or falsk where the templatized path is accessible. This PR updates the span name to not contain the high cardinal value for ASGI instrumentation span names. The highly cardinal span names will become a problem when used with the span metrics connection from the collector.

The spec guidelines say https://opentelemetry.io/docs/specs/otel/trace/api/#span

The span name concisely identifies the work represented by the Span, for example, an RPC method name, a function name, or the name of a subtask or stage within a larger computation. The span name SHOULD be the most general string that identifies a (statistically) interesting class of Spans, rather than individual Span instances while still being human-readable. That is, “get_user” is a reasonable name, while “get_user/314159”, where “314159” is a user ID, is not a good name due to its high cardinality. Generality SHOULD be prioritized over human-readability.

…python-contrib into fix-asgi-name

emdneto · 2025-11-10T12:17:15Z

...tation/opentelemetry-instrumentation-asgi/src/opentelemetry/instrumentation/asgi/__init__.py


 def get_default_span_details(scope: dict) -> Tuple[str, dict]:
    """
    Default span name is the HTTP method and URL path, or just the method.


Can we update the comment and also reference the one you mentioned in the PR description?

emdneto · 2025-11-10T12:18:13Z

CHANGELOG.md

  ([#3882](https://github.com/open-telemetry/opentelemetry-python-contrib/pull/3882))
 - `opentelemetry-instrumentation-aiohttp-server`: delay initialization of tracer, meter and excluded urls to instrumentation for testability
  ([#3836](https://github.com/open-telemetry/opentelemetry-python-contrib/pull/3836))
+- `opentelemetry-instrumentation-asgi` remove high cardinal path from span name


plus remove the protocol type from the span name if http?

aabmass · 2025-11-10T16:58:16Z

...tation/opentelemetry-instrumentation-asgi/src/opentelemetry/instrumentation/asgi/__init__.py

Good to see you @srikanthccv 👋

Is the span name being modified here supposed to follow the OTel HTTP server semconv or is it a more internal span? This change would be a breakage for anyone relying on this particular name, but if it's not the main {method} {target} span it may be OK.

Is the span name being modified here supposed to follow the OTel HTTP server semconv or is it a more internal span?

Correct, specifically this part of the spec: If there is no (low-cardinality) {target} available, HTTP span names SHOULD be {method}. The path value of the ASGI scope for both HTTP and WebSocket contains the non-templatized path. The change in this PR removes the path as the target and just sets the method as recommended.

Some examples

This change would be a breakage for anyone relying on this particular name

Agreed, but at the same time, it doesn't follow the semantic convention and spec guidelines for span names. I would say it's a necessary breaking change. Even from the end-user perspective, I believe this is a useful change; otherwise, IDs (from messaging-adjacent apps, in my experience) become part of the name, leading to no meaningful aggregation for server spans. In many deployments, the spanmetrics connector is used to derive metrics from these spans, which creates a problem.

Agreed, but at the same time, it doesn't follow the semantic convention and spec guidelines for span names. I would say it's a necessary breaking change. Even from the end-user perspective, I believe this is a useful change

Ya I agree, maybe we can just gate this behind the new semconv stability opt in like we do here?

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

Lines 741 to 743 in bd3c1f2

attributes = collect_request_attributes(

scope, self._sem_conv_opt_in_mode

)

How hard would that be to do?

Let me take a look at it.

I went through the code. It should be possible to keep the old and new name following the opt in mode. Let me know if we should make it opt-in.

Hi! 👋

There is a similar PR open for the aiohttp server instrumentation that will also introduce a breaking change to span name in order to follow semconv and reduce cardinality: #3896 (comment) . @aabmass should that PR also include an opt-in? cc @krnr

IMO it would be better since the purpose of the opt-in was to prevent breaking people

Any objections to that though? Does it work for you @srikanthccv ?

Sounds good to me 👍

aabmass · 2025-11-11T02:56:29Z

...tation/opentelemetry-instrumentation-asgi/src/opentelemetry/instrumentation/asgi/__init__.py

Agreed, but at the same time, it doesn't follow the semantic convention and spec guidelines for span names. I would say it's a necessary breaking change. Even from the end-user perspective, I believe this is a useful change

Ya I agree, maybe we can just gate this behind the new semconv stability opt in like we do here?

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

Lines 741 to 743 in bd3c1f2

attributes = collect_request_attributes(

scope, self._sem_conv_opt_in_mode

)

How hard would that be to do?

aabmass · 2025-11-11T02:57:20Z

instrumentation/opentelemetry-instrumentation-asgi/tests/test_asgi_middleware.py

        expected_old = [
            {
-                "name": "GET / http receive",
+                "name": "GET receive",


Don't have a ton of context on these tests, but this still doesn't seem to be following the HTTP semconv. What is receive here?

These are the internal spans created in the receive and send callable of the ASGI application https://asgi.readthedocs.io/en/latest/specs/main.html#applications. The name will be {method} receive or {method} send depending on the event. Does that clarify?

Ah so these are in addition to the standard HTTP semconv, generally showing as child spans? SGTM

Ah so these are in addition to the standard HTTP semconv, generally showing as child spans?

Yes

srikanthccv added 8 commits July 2, 2024 13:46

fix(instrumentation-asgi): remove high cardinal path from span name

076144c

resolve conflicts

251b3c1

fix asgi instrumentation

5de29db

Merge branch 'main' into fix-asgi-name

2da1dac

Fix merge conflicts

3ed3922

Fix name and update tests

1c1b748

Merge branch 'fix-asgi-name' of github.com:srikanthccv/opentelemetry-…

3f5758e

…python-contrib into fix-asgi-name

Format and CHANGELOG

99923c2

srikanthccv requested a review from a team as a code owner March 9, 2025 03:36

shalevr approved these changes Mar 9, 2025

View reviewed changes

srikanthccv added 4 commits November 8, 2025 18:24

Merge branch 'main' into fix-asgi-name

498527f

chore: fix test

95b0c48

chore: update CHANGELOG.md

f98022e

chore: fix format

e2ff03e

emdneto reviewed Nov 10, 2025

View reviewed changes

aabmass reviewed Nov 10, 2025

View reviewed changes

aabmass reviewed Nov 11, 2025

View reviewed changes

tammy-baylis-swi mentioned this pull request Nov 12, 2025

Use canonical name for aiohttp request span name #3896

Open

8 tasks

	attributes = collect_request_attributes(
	scope, self._sem_conv_opt_in_mode
	)

fix(instrumentation-asgi): remove high cardinal path from span name #2650

Are you sure you want to change the base?

fix(instrumentation-asgi): remove high cardinal path from span name #2650

Conversation

srikanthccv commented Jul 2, 2024

Description

Uh oh!

Choose a reason for hiding this comment

Uh oh!

emdneto Nov 10, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

emdneto Nov 10, 2025 •

edited

Loading