otelhttp should collect url information

[bjornbyte]

Problem Statement

When I instrument my application using otelhttp.NewHandler(...) the spans it produces are missing method and path information which is important for understanding my system. The spans for requests it sends to other services (as a client) include this information, but the spans for requests it receives do not.

Proposed Solution

Either by default, or by some WithUrlAttributes option, it spans for incoming http requests should include Url information and set the standard http.url attributes.

[dmathieu]

Could you provide a sample of the issue?
The Handler sets Server attributes from semconv, which does add the http.method.

As for http.target, it was removed a few months ago due to cardinality issues (which seems weird for traces, I agree): open-telemetry/opentelemetry-go#3687

I'll let @MrAlias chime in, but I think the intent was to remove it for metrics (where high cardinality is indeed an issue). So we could maybe add it only for traces?

[bjornbyte]

My mistake, yes, it does include method.

What I'm really missing is the url path in the incoming request, but could also get that from http.url like it has on outgoing (client) requests.

My current workaround is to add an additional filter in my request pipeline and add the attribute,

trace.SpanFromContext(req.Request.Context()).SetAttributes(
		attribute.String("http.url.path", req.Request.URL.Path),
	)

which just now looking at https://github.com/open-telemetry/opentelemetry-specification/blob/main/specification/trace/semantic_conventions/http.md I realize should be http.target or http.route, but anyway, it would be really nice to have an easy way to have that added for me already when using the otelhttp handler.

[dmathieu]

net/http has no notion of route, so it should be http.target, which the spec does mention as a required field.
I do agree this attribute should be there for traces (so does the spec).

[dashpole]

I ran into this, and came up with a workaround:

I changed this:

otelhttp.NewHandler(handler, serviceName, opts...)

To this:

wrappedHandler := http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
  // Add the http.target attribute to the otelhttp span
  if r.URL != nil {
	  oteltrace.SpanFromContext(r.Context()).SetAttributes(semconv.HTTPTarget(r.URL.RequestURI()))
  }
  handler.ServeHTTP(w, r)
})
otelhttp.NewHandler(wrappedHandler, serviceName, opts...)

[alexchowle]

net/http has no notion of route, so it should be http.target, which the spec does mention as a required field. I do agree this attribute should be there for traces (so does the spec).

Looking at the latest specification, the HTTP Server Span should definitely provide url.path. I can't find a reference to http.target as a required field.

[tonglil]

related:

• Misleading variable 'operation' #2645
• Clarify where to use NewHandler() #953

[dennis-jekubczyk]

We would love to use x-ray based sampling in respect of different routes on a reverse proxy (caddy). Sadly the missing propagation of http.targetUrl leads to the point where this is not possible.

Even the great workaround from @dashpole did not work here, as those context is added after the sampling decision.

Does anybody have an idea how to add the targetUrl so the sampler will respect that?

[dashpole]

Migrating to the new semconv is tracked in https://github.com/orgs/open-telemetry/projects/87. We are required to implement the migration plan specified here: https://github.com/open-telemetry/semantic-conventions/tree/main/docs/http#semantic-conventions-for-http.

[dennis-jekubczyk]

Thanks for your quick reply. Sadly I have no access to the project (seems to be private).

As we are not as experienced in OTEL / go instrumentation we found a hacky solution by providing a proxied TraceProvider which will read the http.url from a specific value provided within the request context before calling the Tracer.

That workaround should be sufficient to enable the request based sampling 😄

[dashpole]

Ah, you can follow along here: #5331