Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Use client/server span name as x-envoy-decorator-operation request/response header #11376

Closed
danielgblanco opened this issue May 16, 2024 · 5 comments
Labels
enhancement New feature or request needs triage New issue that requires triage

Comments

@danielgblanco
Copy link

Is your feature request related to a problem? Please describe.

Envoy tracing currently does not follow OTel semantic conventions when naming HTTP/RPC client/server spans, see docs, specifically:

The span also includes a name (or operation) which by default is defined as the host of the invoked service. However this can be customized using a config.route.v3.Decorator on the route. The name can also be overridden using the x-envoy-decorator-operation header.

Although there's no requirement for Envoy to follow OTel semantic conventions, and it does not have the necessary info to extract low-cardinality http.route from server spans, as seen above, it does allow for the name to be overridden by passing the x-envoy-decorator-operation request/response header.

Having no http.route or rpc.method in span names can affect the ability of end-users using Envoy proxies to define boundaries to their services in a way that includes the operation name.

Describe the solution you'd like

I would like to consider the possibility of instrumenting x-envoy-decorator-operation in HTTP/gRPC instrumentation libraries. These libraries already have access to request/response headers and span names within the context of an operation.

If there was option (disabled by default) to include x-envoy-decorator-operation, this would result in consistent naming between OTel spans and Envoy spans.

Describe alternatives you've considered

Other alternatives considered would include configuring config.route.v3.Decorator or adding the x-envoy-decorator-operation in custom filters/interceptors, which would need to be maintained by end-users.

Additional context

No response

@danielgblanco danielgblanco added enhancement New feature or request needs triage New issue that requires triage labels May 16, 2024
@trask
Copy link
Member

trask commented May 20, 2024

it does allow for the name to be overridden by passing the x-envoy-decorator-operation request/response header.

would https://opentelemetry.io/docs/languages/java/automatic/configuration/#capturing-http-request-and-response-headers work to capture that header as a span attribute, and then you could use the collector to update the span name using that attribute?

@danielgblanco
Copy link
Author

Thanks @trask, I think it's related, however I don't think it'd solve the problem. Putting aside client spans, for which Envoy could follow semconv (i.e. use {method} as the span name), when we consider server spans, the application sitting behind an Envoy proxy could "do better" to inform Envoy what to name a span for an HTTP request that was just served (this is where the header would be set). Envoy lacks the information necessary to extract low-cardinality names from the the http.route passed to it (e.g. GET /api/v1/user/dan becoming a span with name GET /api/{version: (v1|v2)}/user/{userId}). However, if server instrumentation could inform Envoy what to name a server span, there would be a more uniform way of defining server spans, between proxies and the applications behind them.

I'm a bit conflicted if this should be something that's handled by OTel instrumentation, or if it should be up to users to define filters/interceptors to handle this. However, it does feel like doing this at the OTel instrumentation layer could reduce toil for end-users.

@trask
Copy link
Member

trask commented Jun 11, 2024

thanks, makes sense

I would like to consider the possibility of instrumenting x-envoy-decorator-operation in HTTP/gRPC instrumentation libraries. These libraries already have access to request/response headers and span names within the context of an operation.

If there was option (disabled by default) to include x-envoy-decorator-operation, this would result in consistent naming between OTel spans and Envoy spans.

I don't love the idea of adding envoy-specific code to the HTTP and gRPC instrumentation libraries.

Maybe we could define an "OpenTelemetry" response header that could be opted in to return the server's http.route?

This could potentially be an interesting option for HTTP client spans which mostly lack a good span name for similar reasons that they don't know the route. (cc @lmolkova)

@lmolkova
Copy link
Contributor

lmolkova commented Jun 11, 2024

I like the idea of returning route in response. It would only work if you can modify the other side which is not always the case.

Another cheap option could be to add envoy-specific propagator that adds x-envoy-decorator-operation header with the value matching current span name (or something else in the current context).

@danielgblanco
Copy link
Author

danielgblanco commented Jun 17, 2024

I fully agree that adding Envoy-specific code to OTel instro packages is not a great way forward, but the only I could (naively) propose to fix this issue. I would not have proposed it if Envoy was not part of CNCF, and as I mentioned above I was a bit conflicted myself. This is why I raised it here in Java instro rather than something more general for everyone to adopt.

However, I think returning a standard header would be a very neat solution. It could be used by proxies to name server spans appropriately, or even by clients to name client spans in a more informative way. I wonder if tracestate could be a good use case for this, maybe as part of open-telemetry/opentelemetry-specification#3811 ?

@lmolkova I did consider an extra propagator, but this would have to be a response propagator? In any case, I will add a comment related to this in the spec issue above and get this issue closed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request needs triage New issue that requires triage
Projects
None yet
Development

No branches or pull requests

3 participants