fetchHandler span does not follow OTEL conventions?

[lboynton]

This came up in a review when I showed someone who is more knowledgeable than me on this subject a project using this library. Wondered what your thoughts were.

The Open Telemetry convention for HTTP spans says the following:

HTTP spans MUST follow the overall guidelines for span names. HTTP server span names SHOULD be {http.request.method} {http.route} if there is a (low-cardinality) http.route available. HTTP server span names SHOULD be {http.method} if there is no (low-cardinality) http.route available. HTTP client spans have no http.route attribute since client-side instrumentation is not generally aware of the "route", and therefore HTTP client spans SHOULD use {http.method}. Instrumentation MUST NOT default to using URI path as span name, but MAY provide hooks to allow custom logic to override the default span name.

Currently the library sets the span name to fetchHandler, whereas the convention suggests it should be the HTTP method as it's an HTTP client. It's easy to change this with a post processor function but do you think the library should follow these conventions?

[evanderkoogh]

I was aware there were semantic attribute names, but not span names. I agree that it is good to follow specs as much as possible, but as we do not have any route information, the span name would just be the method? We could have an optional config parameter to get route information I guess?

Love to hear your thoughts (and your colleague/friend/other person who reviewed it)

[evanderkoogh]

Also..I forgot to mention there is the updateName method on a span that would update the name, so it wouldn't require an entire postProcessor setup just to change the name.

[evanderkoogh]

Going to close this for now, as it is easy to update the name at the moment. Let me know if you have strong feelings about doing this in the library, but having just a bunch of 'GET' as the name of all top level spans might be a bit too confusing?

[rtbenfield]

The span naming is something I noticed as well. span.updateName is a great suggestion for this 👍🏻 I've been using Hono and will probably be updating the span name to incorporate the route pattern rather than the incoming request path. I haven't put all that together yet though, so we'll see if it works out 😄

I don't fully agree with the OpenTelemetry conventions on this (I also find it confusing and non-descriptive), but I'd prefer following it anyway for consistency when adopting the package. I'm not strongly opinionated on it though and will likely be modifying the span names regardless; just sharing my thoughts 🙂