http.route attribue

[iNikem]

Semantic convention for HTTP server spans defines attribute http.route:

The matched route (path template). E.g. "/users/:userID".

Currently instrumentations don't create this attribute. I think we should, but I am not quite sure which span should have this attribute. In case of a typical Java web application the first span created will usually come from generic middleware, such as Servlets or similar. It has no notion of route yet, just the full URL. Then, when request processing reaches actual web framework, such as Spring or jax-rs, second span will be created (potentially with kind=INTERNAL). At this point instrumentation can get used route information from the framework.

Where this attribute should now go? To parent span of kind SERVER (which has all other http attributes like http.url and http.method) or to a current span of kind INTERNAL, which has no other http attributes?

[trask]

I believe it should go on the SERVER span.

We already have instrumentation to capture the route as part of the SERVER span name (to conform with the current span name guidelines), so I think this would be added in the same places in the code, calling now span.setAttribute("http.route", ...) in addition to span.updateName(...).

[iNikem]

@tylerbenson said during last Java SIG meeting that he really dislikes this inter-instrumentation communication, when we modify already existing spans. And I tend to agree with him. If we continue to do this, we HAVE to do that in more explicit and transparent way. Which I don't know yet :)

Can we always be sure that SERVER span is still present when we create child spans? Can it happen that in some async processing server span will be already sent out to collector?

[iNikem]

Also, modifying existing spans goes against e.g. #388. Also https://github.com/open-telemetry/opentelemetry-specification/blob/master/specification/trace/api.md#span says:

It is highly discouraged to update the name of a Span after its creation. Span name is often used to group, filter and identify the logical groups of spans. And often, filtering logic will be implemented before the Span creation for performance reasons. Thus the name update may interfere with this logic.

[trask]

The primary reason that updateName() exists is to handle this example, where you don't know the route when the SERVER span starts, and so initially you set the span name to something generic (like in #428), and then later when you know the route, you call updateName() on the SERVER span to set the span name to the route.

(also, see open-telemetry/opentelemetry-specification#468 which is proposing to remove that warning about updating the name of a Span)

[iNikem]

Ok, I see your point. But in this case I think we still need much clearer and more explicit way for any span to find parent/root span. Current approach is too well hidden and non-obvious.

[trask]

Nice, #465 will also solve this 👍

[sirianni]

The absence of the http.route label is one of the things blocking us from migrating off of https://github.com/DataDog/dd-trace-java to this library. According to Datadog, the http.route label is used to categorize endpoints for the APM UI.

[mateuszrzeszutek]

A couple of TODOs from #5086:

• Move and rename ServerSpanNaming Rename ServerSpanNaming to HttpRouteHolder #5211
• Use http.route in HttpServerMetrics Use http.route in HttpServerMetrics #5266
• Consider extracting the route from Context in HttpServerAttributesExtractor#onEnd() - that'd require adding Context to the parameter list though Refactor AttributesExtractor so that it extracts route from Context #5288
• Use ServerSpanNaming (or whatever it's renamed to) in apache-camel instrumentation Use HttpRouteHolder in camel instrumentation #5242
• Use ServerSpanNaming (or whatever it's renamed to) in spark instrumentation Use HttpRouteHolder in spark instrumentation #5241
• Use ServerSpanNaming (or whatever it's renamed to) in vertx-web instrumentation Use HttpRouteHolder in vertx-web instrumentation #5240
• Use ServerSpanNaming (or whatever it's renamed to) in spring-webflux instrumentation Use HttpRouteHolder in spring-webflux instrumentation #5239
• Refactor HttpServerTest and get rid of extraAttributes() Remove HttpServerTest#extraAttributes() method #5176
• Expected server span name for null route should probably be HTTP GET, not getContextPath() + "/*" (HttpServerTest) Expected server span name for null route should be HTTP GET #5197