TykTechnologies / opentelemetry

Internal Tyk Opentelemetry library
Mozilla Public License 2.0
4 stars 1 forks source link

Prevent high cardinality in span names #38

Open chazapp opened 1 month ago

chazapp commented 1 month ago

Describe the problem

The Tyk Gateway OpenTelemetry implementation generates high cardinality span names, from this line of code: https://github.com/TykTechnologies/opentelemetry/blob/545b95987720821156bf3f0547fb02b9ee063a57/trace/handler.go#L55-L57

This code can create span names containing URI data with high cardinality, such as:

 GET /service/v1/foo/E97AF021-B82C-4EF3-9A2C-BF49EF04CCD8/bar

OpenTelemetry's Semantic Conventions for HTTP Spans clearly states:

HTTP span names SHOULD be {method} {target} if there is a (low-cardinality) target available. If there is no (low-cardinality) {target} available, HTTP span names SHOULD be {method}.

Instrumentation MUST NOT default to using URI path as a {target}.

Describe the solution you'd like

The issue can be solved in different ways. Here are a few ideas:

Change the span name to `{method}` Per OpenTelemetry convention, if finding a low-cardinality target is not possible, set the span name to the HTTP method only.
Change the span name to `{method} {apiName}` Either via context or parsing, use only the downstream API name.
Allow a pattern to be passed via config The span name could be formed using a pattern that would be user provided from API configuration.

Describe alternatives you've considered

I can use OTEL-Collector transform processor to edit spans before they reach my storage backend, but I'd rather see the issue fixed upstream, in the span creator, respecting Open Telemetry conventions.

tbuchaillot commented 5 days ago

The span name could be formed using a pattern that would be user provided from API configuration.

I think we should definitely go with this options since any of the others will mean a breaking change for existing users.