opentelemetry-go-contrib
opentelemetry-go-contrib copied to clipboard
Request to include request headers into span attributes for net/http
Background
Package Link: go.opentelemetry.io/contrib/instrumentation/net/http/otelhttp
Many Go http services use the stdlib provided net/http for which go.opentelemetry.io/contrib/instrumentation/net/http/otelhttp provides an easy to use middleware.
Looking into the Python and Java equivalents, each supports an environment variable OTEL_INSTRUMENTATION_HTTP_CAPTURE_HEADERS_SERVER_REQUEST which allows for specified HTTP Headers to be automatically included in spans as attributes. However the Go implementation does not yet support this.
Why can this instrumentation not be included in the package itself?
Instrumentation is presently outside of the scope of the Go standard library. net/http/otelhttp already exists; it's reasonable to simply extend it further.
Why can this instrumentation not be hosted in a dedicated repository?
Same answer as above.
Proposed Solution
I think there are at least a few options:
- Provide a new
otelhttp.Optionthat configures a(*otelhttp.Config).requestAttributesfield (with a type like[]func(*http.Request) attribute.KeyValue, which gets incorporated where the other span attributes are set. Users would be able to provide this as an explicit option when invokingotelhttp.NewHandler. E.g.
// new code
for _, requestAttribute := range h.requestAttributes {
opts = append(opts, trace.WithAttributes(requestAttribute(r))
}
// existing code
opts = append([]trace.SpanStartOption{
trace.WithAttributes(semconv.NetAttributesFromHTTPRequest("tcp", r)...),
trace.WithAttributes(semconv.EndUserAttributesFromHTTPRequest(r)...),
trace.WithAttributes(semconv.HTTPServerAttributesFromHTTPRequest(h.operation, "", r)...),
}, opts...) // start with the configured options
- Similarly, implement support for an environment variable
OTEL_INSTRUMENTATION_HTTP_CAPTURE_HEADERS_SERVER_REQUEST, which could be read and processed as part ofotelhttp.NewHandler. An upside is parity between languages, but at the cost of introducing a side-effect in code that's otherwise pretty explicit. This could likely be balanced by hiding this behind anOptionwhich could potentially be explicitly provided by users. - allow it to somehow provided as part of setting up the TracerProvider
- in support: this is where many global settings are currently configured, and it would allow the setting to be explicit
- however: this would mean somehow registering a function that accepts an
*http.Request, and potentially needing to smuggle it through acontext.Context, for which there may not be precedent and/or it might be a pain
Tracing
- attributes:
- the headers would be incorporated in span attributes using the semantic conventions of
http.request.header.<key>with values of type[]string
- the headers would be incorporated in span attributes using the semantic conventions of
- events:
- n/a
- links:
- n/a
Metrics
n/a
Prior Art
For Python, see https://github.com/open-telemetry/opentelemetry-python-contrib/blob/cca90db86f177d3b9affa54a2089d3a28c58e888/instrumentation/opentelemetry-instrumentation-wsgi/src/opentelemetry/instrumentation/wsgi/init.py#L529-L533
For Java see https://opentelemetry.io/docs/instrumentation/java/automatic/agent-config/#capturing-http-request-and-response-headers
Tasks
- Code complete:
- [ ] Comprehensive unit tests.
- [ ] End-to-end integration tests.
- [ ] Tests all passing.
- [ ] Instrumentation functionality verified.
- Documented
- [ ] Added to the OpenTelemetry Registry
- [ ] README included for the module describing high-level purpose.
- [ ] Complete documentation of all public API including package documentation.
- [ ] Instrumentation documentation updated.
- Examples
- [ ]
Dockerfilefile to build example application. - [ ]
docker-compose.ymlto run example in a docker environment to demonstrate instrumentation.
- [ ]
Thank you for your interest and proposal. I think I could support the explicit configuration proposed in option 1, but I'd really prefer if this were worked through the semantic conventions working group to provide a consistent view on how to handle these attributes across all OTel instrumentations.
I'd really prefer if this were worked through the semantic conventions working group to provide a consistent view on how to handle these attributes across all OTel instrumentations.
Sounds great. Is that something I should plan to attend to raise the issue, or is it enough to drop a link to this issue into the agenda of the next meeting?
FWIW the Node implementation has the same (called headersToSpanAttributes).
https://github.com/open-telemetry/opentelemetry-js/tree/main/experimental/packages/opentelemetry-instrumentation-http#http-instrumentation-options
At the risk of me-tooing, I just ran into this issue myself and was disappointed (and surprised!) to see that the Go library was the odd man out here. @danielmmetz was there any progress to report from bringing this to the WG?
I ran into this issue just today. May I ask why does this require a WG decision when there is a clear trend what the users want? I really don't see the point of waiting for a WG stamp in an ivory tower, while this feature has been implemented across multiple platforms and languages. In my opinion what the WG group can do at this point is just bowing their head and admit that this feature is necessary.
The Go SIG is not in an "ivory tower". We review every pull request, keep the project on track and up to date, and we do that in the open since weekly meetings can be attended by everyone and (per definition), the project is open.
I am currently the sole code owner of the otelhttp instrumentation. You're more than welcome to start contributing, and give a hand.
Note that this is achievable within your code (and in a more powerful way, since you could do any computing you want from the request, and add this as attributes).
func extendHTTPSpan(next http.Handler) http.Handler {
return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
next.ServeHTTP(w, r)
span := trace.SpanFromContext(r.Context())
span.SetAttributes(
attribute.String("authorization", r.Header.Get("Authorization"),
)
})
}
Then, to use it (order of the handlers matter):
handler := [...] // whatever your application does when serving HTTP requests.
handler = otelhttp.NewHandler(extendHTTPSpan(http.HandlerFunc(handler), "HTTP"))
http.Handle("/", handler)
_ = http.ListenAndServe(":7777", nil)
This is not to say the option can't be added. You are welcome to open a PR adding.