open-telemetry / opentelemetry-go-contrib

Collection of extensions for OpenTelemetry-Go.
https://opentelemetry.io/
Apache License 2.0
1.18k stars 555 forks source link

otelrestful: panic in header extract #6252

Closed dashpole closed 3 days ago

dashpole commented 1 week ago

Description

Stack trace:

goroutine 51790938 [running]:
 net/http.(*conn).serve.func1()
         net/http/server.go:1868 +0xb9
 panic({0x3e0acc0?, 0x70936b0?})
        runtime/panic.go:920 +0x270
 go.opentelemetry.io/otel/internal/global.(*textMapPropagator).Extract(0x98?, {0x4de3208, 0xc0011e7d10}, {0x4dd3260, 0xc002f502d0})
         go.opentelemetry.io/otel@v1.19.0/internal/global/propagator.go:76 +0x27
go.opentelemetry.io/contrib/instrumentation/github.com/emicklei/go-restful/otelrestful.OTelFilter.func1(0xc002e82d80, 0xc0003605b0, 0xc0011e8140?)
         go.opentelemetry.io/contrib/instrumentation/github.com/emicklei/go-restful/otelrestful@v0.42.0/restful.go:51 +0xcc
 github.com/emicklei/go-restful/v3.(*FilterChain).ProcessFilter(0x3db?, 0x4de3208?, 0xc0011e7d10?)
         github.com/emicklei/go-restful/v3@v3.11.0/filter.go:21 +0x42
...

Environment

This occurred in the kubelet of a GKE cluster.

Steps To Reproduce

Not easily reproducible

Expected behavior

Whether the request header is nil or not, kubelet shouldn't panic.

dashpole commented 1 week ago

Hmmm... This seems to imply that the requests Header is always required to be initialized: https://pkg.go.dev/net/http#RoundTripper. I've also looked through many other instrumentation libraries (prometheus and other otel http libraries), and none of them check for nil req.Headers