Bandit: Add the request_path to the span name#401
Bandit: Add the request_path to the span name#401joshk wants to merge 1 commit intoopen-telemetry:mainfrom
request_path to the span name#401Conversation
The Phoenix instrumentation updates the span name to include the `request_path`, but some requests don't go through the router (like LiveView websocket connections), which means the root span is missing context for what the span is related to.
|
The reason it is only set when going through the router is that it can't have a high cardinality value for the span name: https://github.com/open-telemetry/semantic-conventions/blob/main/docs/http/http-spans.md#name If the user wants the path, no matter what, in the name then it can be set with the Or if there is something about being routed to a LiveView that means it is a low cardinality value then it could be updated by the instrumentation there? |
|
Hi @tsloughter , thanks for the comment and information. I agree with you, this PR would be going against the high cardinality rule for span names. While socket("/live", Phoenix.LiveView.Socket, websocket: [connect_info: [session: @session_options]])
socket("/socket", NervesHubWeb.UserSocket, websocket: [connect_info: [session: @session_options]])Is there a recommended approach to updating a parent spans name? This would work for the case of the second socket, which is in our code base, but not for the socket used for Live View communications. |
|
A slightly different approach would be to update the span name only if the status code is 101. eg. This would be done in |
|
For the time being I've done this in our Phoenix app: https://github.com/nerves-hub/nerves_hub_web/pull/1612/files#diff-a90641c8be89f7c4fa30427d348f740fc6a884d91c446fdc5987b78a92dcb8e5 |
|
@joshk what about checking if the path is |
|
@tsloughter great advice! I checked the URL patterns for Phoenix websocket connections, and they all end with I think having this in the Phoenix instrumentation would be useful, but there is no telemetry event to hook into, which is why I ended up using the Bandit |
|
@tsloughter , thank you for your reviews and advice. |
aah, wonder why Phoenix doesn't send a telemetry event for these... |
|
Yeah, its a little odd and annoying, and I'm sure they would be open to a PR, but I'll delay looking into that until another day. |
|
I'm trying to figure out if we can do something upstream to be able to attach to sockets: phoenixframework/phoenix#6019 |
Bandit created HTTP server spans with atom names (e.g. :GET) from semconv
values. The OTel spec requires span names to be strings. This changes
the initial span name to use conn.method (a string like "GET").
Additionally, at request completion, the span name is updated to include
the route template ("{method} {route}") when conn.private[:plug_route]
is available. This is set by Phoenix and other Plug-based routers during
dispatch, enabling proper span names without requiring opentelemetry_phoenix.
Follows up on open-telemetry#401 and the discussion about OTel HTTP semantic conventions
requiring span names to be "{method} {http.route}" when the route is known.
3 participants
The Phoenix instrumentation updates the span name to include the
request_path, but some requests don't go through the router (like LiveView websocket connections), which means the root span is missing context for what the span is related to.This PR changes the default Bandit span name to include the
conn.request_path.