open-telemetry / opentelemetry-specification

Specifications for OpenTelemetry
https://opentelemetry.io
Apache License 2.0
3.69k stars 884 forks source link

Clarify cardinality requirements of a span name #3534

Open lmolkova opened 1 year ago

lmolkova commented 1 year ago

Came up in: https://github.com/open-telemetry/semantic-conventions/pull/35#issuecomment-1574020499

In #3470 we realized that http.request.method can have high cardinality when custom HTTP methods are used. While it clearly represents a problem for metrics, it's not clear what the consequences are for the span name.

The spec does not clearly require span name to have low cardinality, instead, it talks about names being general, identifying a class of operations rather than individual ones:

https://github.com/open-telemetry/opentelemetry-specification/blob/6941cada4adc5cca06abd6f5d41ac71e6b8b7477/specification/trace/api.md?plain=1#L303-L311

Based on this and the history of the changes (#557, #270), the idea is to identify a class of operations and work as a grouping key. In this sense, the cardinality restrictions are soft and the cardinality term doesn't quite apply.

In the context of http method, grouping operations should work well even when invalid methods are recorded in the span name - there just will be a long tail of rare custom methods.

Question: Do we use/envision using the span name as a dimension on metrics?

Oberon00 commented 1 year ago

Generally, I feel it is not good for a span name to have high cardinality.

The question is if we want to live with the "defect" we currently have in the HTTP conventions, or find another solution like normalization (nontrivial) or using a rather unhelpful span name (like "HTTP client" instead of "HTTP <method>")

lmolkova commented 1 year ago

Generally, I feel it is not good for a span name to have high cardinality.

I agree. And yes, the questions are:

trask commented 1 year ago

The question is if we want to live with the "defect" we currently have in the HTTP conventions, or find another solution like normalization (nontrivial)

isn't this defect solved if we go with Limit HTTP request method cardinality: original_method (option A)?

this is the one reason I'm currently leaning towards "Option A"

cc @reyang @jsuereth @arminru

Oberon00 commented 1 year ago

I think normally one would not use high-cardinality attributes as input for the span name, instead of normalizing. But that would leave nothing at all for (client-side) HTTP except for "HTTP client"... (Server-side, one could consider reverting #2998, or clarifying that it only applies to accepted requests)

pyohannes commented 1 year ago

What's exactly the scope of this issue? Just span names recommended by semantic conventions, or all span names in general?

If it's the latter, I'd be reluctant to add this as a SHOULD into the API specification, as there is no way to enforce this (as it's given by the end user of the API).

Oberon00 commented 1 year ago

IMHO the wording should/could address the consumers here: SHOULD they be able to handle high cardinality in a reasonable manner or MAY they assume span names to be low-cardinality (and exhibit bad behavior like dropping spans entirely in an unsystematic way, or creating UIs pages that are so large that they freeze the browser on load, etc.)

A (at least as strict) recommendation for the semantic conventions can be made additionally.

lmolkova commented 1 year ago

I would rather be strict about assumptions and say "Consumers SHOULD NOT assume span names to have low cardinality".

Even when every instrumentation does its best and creates names from low-cardinality attributes, in a big system number of distinct operations can reach hundreds (thousands?). We don't define what constitutes low or high cardinality (#2996), but it feels like at least middle-ish.

lmolkova commented 1 year ago

What's exactly the scope of this issue? Just span names recommended by semantic conventions, or all span names in general?

great question. Agree we can't control span names in general. The question is how much effort we need to do for semconv and otel-authored instrumentations.

austinlparker commented 3 months ago

Discussed by GC - We concur that clarifying language to consumers of OTel Spans should indicate that span names MAY be high cardinality, but that the interpretation of 'statistically interesting' in the linked spec document should be clarified to read as 'low cardinality'.