-
Notifications
You must be signed in to change notification settings - Fork 534
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
Socket.io instrumentation span name not following semantic convention #1937
Comments
Hi @makeavish Is it possible to have a little more context?
Thanks! :) |
I've just tried with the chat example but could not reproduce. This is the repo if you want to add some code that may reproduce the issue What I can see in Kibana is a trace with the name |
@david-luna In our case, the Socket.io span name includes the room ID making every span name different as the room IDs are auto-generated random IDs. This restricts us from using the span name for filtering/aggregations. It clutters up the list of operations like this as every span is having a unique name Example Span in a trace:
Span Tags:
According to the semantic conventions for span names, the destination (room ID in this context) can be part of the span name only when it is neither temporary nor anonymous.
As these room IDs are dynamically generated and temporary, i'd suggest not using them in the span name. Instead, only have |
fixed in #2495 |
What version of OpenTelemetry are you using?
@opentelemetry/auto-instrumentations-node@^0.39.4
What version of Node are you using?
18
What did you do?
Instrumented with latest socket.io otel instrumentation: https://www.npmjs.com/package/@opentelemetry/instrumentation-socket.io
What did you expect to see?
According to otel spec guideline the span name should not contain a unique identifier and should be general: https://github.com/open-telemetry/opentelemetry-specification/blob/v1.22.0/specification/trace/api.md#span
What did you see instead?
I saw unique span names for spans: eg:
/[--3s4wJNxrT5DwjTABGg] send
Additional context
The text was updated successfully, but these errors were encountered: