HTTP span name: Recommend handler over route name

[Oberon00]

Recommend handler name over route name, improve formatting (split into paragraphs).

Depends-on: #557

[arminru]

Sounds reasonable!

[carlosalberto]

Overall LGTM

[yurishkuro]

I disagree with this. When an Ops person is looking at a trace, seeing the route (service API related concept) is likely going to be more informative than the handler method name (an implementation detail).

[Oberon00]

On the other hand, if sampling is done based on the span name, then this new name is an improvement. I think that is more a problem of the span name concept which seems to play a double-role as a human-readable name and a grouping key.

Still, a reasonable argument.

[Oberon00]

However, the route is also in a semantic attribute http.route, and it should be possible for a backend to replace the name with that (in the UI). This could even be configured differently for ops teams and dev teams.

[bogdandrutu]

Same argument can be made against this to have the handler name as convention.

[Oberon00]

I closed this PR and opened #557 to track the span name double bind issue.

[Oberon00]

@bogdandrutu The handler name will always have a lower or the same cardinality as the route, so it is strictly better as grouping key if it's known.

[Oberon00]

@yurishkuro In #730 (comment) you argue against the display.name convention by saying that backends will compute a better display name anyway based on span attributes. Here you argue against using a particular span name because it is bad as display name. It seems to me like these comments contradict each other. Did you change your opinion or am I misunderstanding you?

[yurishkuro]

I didn't say the backends necessarily would do that, but they might do it if they wanted to. My argument against display.name was that in most cases it is redundant, especially in the examples provided in #730. But there may be cases when it is useful. However, it has another big issue when it comes to displaying aggregations, which I just posted in #730 (comment).

I think my position remains the same - span name is both a grouping key and the display name. The additional display.name attribute is possible, but not well thought-through yet, it raises more questions for me now than it answers.

[Oberon00]

Closing this, as @yurishkuro made a good argument as to why the current suggestion of the route name is at least not worse than the handler name and there seems to be no traction in another direction.

[Oberon00]

The current discussion in #557 seems to lean towards low cardinality being more important and we should provide a new semantic convention for the display name if we need one. Thus I'm reopening this for discussion.

[bogdandrutu]

@Oberon00 should this PR be removed?

[Oberon00]

If that's what you mean, the re-open was intentional, see the comment above. I would rather merge this PR than remove it 😃

[yurishkuro]

I'm still -1 on handler function name. Imagine you have two versions of REST APIs, /api/v1/xxx and /api/v2/xxx. Some routes in these APIs may not have changed between versions and may be implemented by the same handler - an information I consider very unimportant when looking at the trace, where the fact that /v1/doThis or /v2/doThis was called is much more interesting to the user of the trace (who may have never worked on this service). Using routes means we're referring to public API endpoints. Handler names are implementation detail.

[Oberon00]

@yurishkuro With #557 we would use the span name only as a default grouping. The route is still available as http.route. And we could add a display_name thing later. As #557 says, using the span name both as a display name and a grouping key is bad, so this PR ignores the usability of the span name as a display name and improves its value as a grouping key.

This is what the spec currently already says about the span name:

The span name should be the most general string that identifies a (statistically) interesting class of Spans, rather than individual Span instances.

(emphasis added by me).

[bogdandrutu]

Assigned @yurishkuro to make a decision on this PR, move forward or close.

[Oberon00]

This PR needs a decision on #557 first.
Only if we decide the span name is (mostly) a "grouping key" as opposed to a display name (which is the direction the discussion on #557 has been going lately), would it make sense to merge this PR.

[iNikem]

Don't we blur the line between data collection and backend too much?

[Oberon00]

How so?

[iNikem]

Seems like we try to dictate what backend should do. Yes, we define semantics of the data we collect, but actually saying "this should be used for displaying and this for grouping" is going much farther, no?

[Oberon00]

I see it that way: If we want backends to be able to use this as grouping key, we have to generate something that is suitable as grouping key. For example a full URL would be fine as display name but usually not so much as grouping key. Of course, we won't stop backends from using the grouping key also or only as display name (for which it is usually still usable, though maybe not optimal).

[github-actions]

This PR was marked stale due to lack of activity. It will be closed in 7 days.

[Oberon00]

Since #557 is now resolved via #810 to specify that human-readability is less important than being the "most general" string, I think this PR should now be merged as a direct consequence. Also, it has two approvals, no vetoes, and no changes since April.

[carlosalberto]

Ping @yurishkuro

[yurishkuro]

For the record, my disagreement in #548 (comment) had 3 upvotes. The discussion in #557 about grouping key vs. display name does not affect my argument, because the HTTP route is ultimately the API exposed by the server, it's the notion that users use to think about the service, express metrics and SLAs, reason about inter-service dependencies, etc. Those are not purely display concerns. The handler name is an implementation detail that is almost irrelevant to all those concerns.

[Oberon00]

@yurishkuro

The discussion in #557 about grouping key vs. display name does not affect my argument

Is the handler name (if available!) a more general name than the route? Yes, because you can map more than one route to the same handler. Is it specific enough to still be meaningful? I do think so. Is the handler name human readable? Yes (I agree that most will find the route more useful to read but as per #810, the generality is more important).

The purpose of #810 was precisely to have a guideline, criteria with which to avoid such a discussion.

the HTTP route is ultimately the API exposed by the server, it's the notion that users use to think about the service, express metrics and SLAs, reason about inter-service dependencies, etc. Those are not purely display concerns. The handler name is an implementation detail that is almost irrelevant to all those concerns.

Mostly agree on your points here, but we already have the http.route attribute for that. The span name has only one purpose: Group the largest meaningful set of spans together. If you need to track an SLA on a route, you can surely customize your sampler and backend to sample/group per http.route instead of the name. But usually if multiple routes map to the same handler, it does make sense to group them together, at least initially.

EDIT: I'm not sure about the inter-service dependencies. Maybe different calling services will use different inbound routes to access the service, but for the same handler we should always have the same outbound calls. So I think from an inter-service dependency view, grouping by handler does actually simplify the "graph" without real loss of fidelity.

[yurishkuro]

But usually if multiple routes map to the same handler, it does make sense to group them together, at least initially.

It may be sensible in some scenarios, but it is not unequivocal. I don't think the spec should be insisting on it. Some HTTP frameworks may not even have the notion of a handler name, it's fairly common to use anonymous lambda functions as handlers.

It is clear that we do not have consensus on this topic, so I suggest rewording the PR that using a handler name is an acceptable alternative, but stop short of saying that it is preferred over HTTP route.

[Oberon00]

We already have "MAY provide hooks to allow custom logic to override the default span name" here. We should specify a clear order of preference for the default.
We also already say "HTTP spans MUST follow the overall guidelines for span names.", so even if I don't spell the preference out here, it would follow from the general guidelines saying "The span name SHOULD be the most general string that identifies a (statistically) interesting class of Spans".

[github-actions]

This PR was marked stale due to lack of activity. It will be closed in 7 days.

[github-actions]

Closed as inactive. Feel free to reopen if this PR is still being worked on.

[Oberon00]

Note that I still think this is a good idea and I will re-open this PR after GA.