open-telemetry / opentelemetry-js-contrib

OpenTelemetry instrumentation for JavaScript modules
https://opentelemetry.io
Apache License 2.0
703 stars 518 forks source link

Socket.io instrumentation span name not following semantic convention #1937

Open makeavish opened 9 months ago

makeavish commented 9 months ago

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

david-luna commented 6 months ago

Hi @makeavish

Is it possible to have a little more context?

Thanks! :)

david-luna commented 6 months ago

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 https://github.com/david-luna/socket-io-chat-example/tree/dluna/add-otel-instrumentation

What I can see in Kibana is a trace with the name chat message receive Screenshot 2024-04-26 at 13 46 29

prashanth-nani commented 2 months ago

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

Example Span in a trace:

name: /[d31f44fd-11f3-43ba-a378-7b09df3bf900-cc762f38-2a17-4c51-887a-1a6f2ba1a53f] send
kind: producer

Span Tags:

messaging.destination: /
messaging.destination_kind: topic
messaging.socket.io.event_name: notification
messaging.socket.io.namespace: /
messaging.socket.io.rooms: ["d31f44fd-11f3-43ba-a378-7b09df3bf900-cc762f38-2a17-4c51-887a-1a6f2ba1a53f"]
messaging.system: socket.io
process.command: /app/app.js
process.command_args: ["/usr/local/bin/node","-r","./tracing.js","/app/app.js"]
process.executable.name: node
process.executable.path: /usr/local/bin/node
process.owner: root
process.pid: 1
process.runtime.description: Node.js
process.runtime.name: nodejs
process.runtime.version: 20.14.0
telemetry.sdk.language: nodejs
telemetry.sdk.name: opentelemetrymessaging.socket.io.rooms
telemetry.sdk.version: 1.21.0

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.

messaging.destination.name SHOULD be used when the destination is known to be 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 send as the span name and have any other additional data as part of the tags. In this case, the room details are already present in messaging.socket.io.rooms.