open-telemetry / opentelemetry-demo

This repository contains the OpenTelemetry Astronomy Shop, a microservice-based distributed system intended to illustrate the implementation of OpenTelemetry in a near real-world environment.
https://opentelemetry.io/docs/demo/
Apache License 2.0
1.7k stars 1.07k forks source link

Rename root span name to concisely identifies the work represented by the Span #1676

Open rogercoll opened 1 month ago

rogercoll commented 1 month ago

Feature Request

Is your feature request related to a problem?

HTTP generated spans contain a too general name which does not help to identify and monitor the whole lifecycle. For example, the frontend-web service only generate two spans named HTTP GET and HTTP POST for all the queries it performs. As the API defines, we should provide a more human-readable name for root spans, which might include additional information like the URL path.

Sample frontend-proxy span:

Span #0
    Trace ID       : 3004cbd39f480fde9fc69de395d472e9
    Parent ID      : 
    ID             : a602a066ac8af08b
    Name           : HTTP GET
    Trace ID       : 3004cbd39f480fde9fc69de395d472e9
    Parent ID      : 
    ID             : a602a066ac8af08b
    Name           : HTTP GET
    Kind           : Client
    Start time     : 2024-07-18 14:52:29.949 +0000 UTC
    End time       : 2024-07-18 14:52:29.966 +0000 UTC
    Kind           : Client
    Start time     : 2024-07-18 14:52:29.949 +0000 UTC
    End time       : 2024-07-18 14:52:29.966 +0000 UTC
    Status code    : Unset
    Status message : 
Attributes:
     -> component: Str(fetch)
    Status code    : Unset
    Status message : 
Attributes:
     -> component: Str(fetch)
     -> http.method: Str(GET)
     -> http.url: Str(http://frontend-proxy:8080/images/products/RoofBinoculars.jpg)

Describe the solution you'd like:

Provide a more concisely name for root spans. For example, the previous span could be named images instead.

cc @davidgeorgehope

julianocosta89 commented 1 month ago

@rogercoll I think this is a great point and we should definitely raise it to the envoy repo.

julianocosta89 commented 1 month ago

@rogercoll do you think that would be the way to solve this?

https://github.com/envoyproxy/envoy/issues/30821

rogercoll commented 1 month ago

@rogercoll do you think that would be the way to solve this?

I think it is related but not the same, in our case we would like to have a better name for the root spans. The actual attributes are fine, although as more aligned to SemConv the better :smile:

I will try to find some time to play around the envoy project and see how spans would look like if the name is based on the HTTP path instead of the current method.

julianocosta89 commented 1 month ago

Ah, true, that's for the attributes and not span name 🙃

julianocosta89 commented 1 week ago

@joaopgrassi as you were the last one from the OTel community to change envoy, do you think this would be something you could help us? Or maybe point us to someone on the envoy community to give us a hand?

We have discussed in the SIG meeting and we would love to have that solved upstream instead of using a transform processor on the Collector.

julianocosta89 commented 1 week ago

@rogercoll I got it all wrong 🤦🏽 The frontend-web is not envoy, but js-frontend.

I have just discussed that case with @joaopgrassi and even though what you mentioned before is valid: https://github.com/open-telemetry/opentelemetry-specification/blob/v1.33.0/specification/trace/api.md#span

Usually the client doesn't know about all the endpoints available on the server and having GET /endpoint can cause a cardinality explosion.

This is what we have in the SemConv for HTTP: https://opentelemetry.io/docs/specs/semconv/http/http-spans/#name.

@joaopgrassi also shared this PR with me: https://github.com/open-telemetry/semantic-conventions/pull/675, which would allow us to define a low-cardinality route/path.

This seems to be available as an experimental semconv: https://github.com/open-telemetry/opentelemetry-js/blob/b78fec34060f15499c1756fd966f745262e148d9/semantic-conventions/src/experimental_attributes.ts#L6872

But I can't see a way of implementing it without adding a lot of boilerplate code to the frontend.

Any ideas?

joaopgrassi commented 1 week ago

Do you guys know which http instrumentation the frontend service uses? Most likely the HTTP client instrumentation is the one that has to implement such thing. Maybe we can ask the JS folks to see if they have anything planned.

rogercoll commented 4 days ago

The frontend-web service uses the auto-instrumentations-web package: https://github.com/open-telemetry/opentelemetry-demo/blob/main/src/frontend/utils/telemetry/FrontendTracer.ts#L53

The auto-instrumentation package uses the auto-fetch package to retrieve and generate the corresponding http attributes: https://github.com/open-telemetry/opentelemetry-js/tree/main/experimental/packages/opentelemetry-instrumentation-fetch

Maybe we can ask the JS folks to see if they have anything planned.

That would be great, I reckon this issue is not only specific to the Demo but a broader one. If the url.template can be constructed from any given URL, we could use that value to construct the span name using a simple transform processor (or even in the frontend service)