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

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

Open
rogercoll opened this issue Jul 18, 2024 · 8 comments
Labels
enhancement New feature or request help wanted Extra attention is needed

Comments

@rogercoll
Copy link
Contributor

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
Copy link
Member

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

@julianocosta89
Copy link
Member

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

envoyproxy/envoy#30821

@rogercoll
Copy link
Contributor Author

@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 😄

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
Copy link
Member

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

@julianocosta89
Copy link
Member

@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
Copy link
Member

@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: open-telemetry/semantic-conventions#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
Copy link
Member

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
Copy link
Contributor Author

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)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request help wanted Extra attention is needed
Projects
None yet
Development

No branches or pull requests

3 participants