Skip to content

Commit 435caad

Browse files
fix otelhttp setup in activator (#16046)
Co-authored-by: Dave Protasowski <[email protected]>
1 parent c76d7ae commit 435caad

File tree

6 files changed

+25
-18
lines changed

6 files changed

+25
-18
lines changed

cmd/activator/main.go

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -35,6 +35,8 @@ import (
3535
"k8s.io/apimachinery/pkg/util/wait"
3636

3737
// Injection related imports.
38+
"go.opentelemetry.io/contrib/instrumentation/net/http/otelhttp"
39+
3840
network "knative.dev/networking/pkg"
3941
netcfg "knative.dev/networking/pkg/config"
4042
netprobe "knative.dev/networking/pkg/http/probe"
@@ -230,7 +232,7 @@ func main() {
230232
apiconfig.DefaultRevisionIdleTimeoutSeconds * time.Second
231233
})
232234
ah = concurrencyReporter.Handler(ah)
233-
ah = activatorhandler.NewTracingHandler(tp, ah)
235+
ah = activatorhandler.NewTracingAttributeHandler(tp, ah)
234236
reqLogHandler, err := pkghttp.NewRequestLogHandler(ah, logging.NewSyncFileWriter(os.Stdout), "",
235237
requestLogTemplateInputGetter, false /*enableProbeRequestLog*/)
236238
if err != nil {
@@ -240,11 +242,16 @@ func main() {
240242

241243
// NOTE: MetricHandler is being used as the outermost handler of the meaty bits. We're not interested in measuring
242244
// the healthchecks or probes.
243-
ah = activatorhandler.NewMetricHandler(env.PodName, ah)
245+
ah = activatorhandler.NewMetricAttributeHandler(env.PodName, ah)
244246
// We need the context handler to run first so ctx gets the revision info.
245247
ah = activatorhandler.WrapActivatorHandlerWithFullDuplex(ah, logger)
246248
ah = activatorhandler.NewContextHandler(ctx, ah, configStore)
247249

250+
ah = otelhttp.NewHandler(ah, "handle",
251+
otelhttp.WithTracerProvider(tp),
252+
otelhttp.WithMeterProvider(mp),
253+
)
254+
248255
// Network probe handlers.
249256
ah = &activatorhandler.ProbeHandler{NextHandler: ah}
250257
ah = netprobe.NewHandler(ah)

pkg/activator/handler/main_test.go

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -88,9 +88,9 @@ func BenchmarkHandlerChain(b *testing.B) {
8888
// Make sure to update this if the activator's main file changes.
8989
ah := New(ctx, fakeThrottler{}, rt, false, logger, false /* TLS */, tp)
9090
ah = concurrencyReporter.Handler(ah)
91-
ah = NewTracingHandler(tp, ah)
91+
ah = NewTracingAttributeHandler(tp, ah)
9292
ah, _ = pkghttp.NewRequestLogHandler(ah, io.Discard, "", nil, false)
93-
ah = NewMetricHandler(activatorPodName, ah)
93+
ah = NewMetricAttributeHandler(activatorPodName, ah)
9494
ah = NewContextHandler(ctx, ah, configStore)
9595
ah = &ProbeHandler{NextHandler: ah}
9696
ah = netprobe.NewHandler(ah)
@@ -204,9 +204,9 @@ func TestActivatorChainHandlerWithFullDuplex(t *testing.T) {
204204
})
205205
var ah http.Handler
206206
ah = concurrencyReporter.Handler(proxyWithMiddleware)
207-
ah = NewTracingHandler(tp, ah)
207+
ah = NewTracingAttributeHandler(tp, ah)
208208
ah, _ = pkghttp.NewRequestLogHandler(ah, io.Discard, "", nil, false)
209-
ah = NewMetricHandler(activatorPodName, ah)
209+
ah = NewMetricAttributeHandler(activatorPodName, ah)
210210
ah = WrapActivatorHandlerWithFullDuplex(ah, logger)
211211
ah = NewContextHandler(ctx, ah, configStore)
212212
ah = &ProbeHandler{NextHandler: ah}

pkg/activator/handler/metric_handler.go

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -24,8 +24,9 @@ import (
2424
"knative.dev/serving/pkg/metrics"
2525
)
2626

27-
// NewMetricHandler creates a handler that collects and reports request metrics.
28-
func NewMetricHandler(podName string, next http.Handler) *MetricHandler {
27+
// NewMetricAttributeHandler creates a handler that adds serving attributes
28+
// to the otelhttp labeler
29+
func NewMetricAttributeHandler(podName string, next http.Handler) *MetricHandler {
2930
return &MetricHandler{
3031
nextHandler: next,
3132
podName: podName,

pkg/activator/handler/metric_handler_test.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -64,7 +64,7 @@ func TestRequestMetricHandler(t *testing.T) {
6464

6565
for _, test := range tests {
6666
t.Run(test.label, func(t *testing.T) {
67-
handler := NewMetricHandler(testPod, test.baseHandler)
67+
handler := NewMetricAttributeHandler(testPod, test.baseHandler)
6868

6969
labeler := &otelhttp.Labeler{}
7070

@@ -121,7 +121,7 @@ func BenchmarkMetricHandler(b *testing.B) {
121121

122122
reqCtx = otelhttp.ContextWithLabeler(reqCtx, &otelhttp.Labeler{})
123123

124-
handler := NewMetricHandler("benchPod", baseHandler)
124+
handler := NewMetricAttributeHandler("benchPod", baseHandler)
125125

126126
resp := httptest.NewRecorder()
127127
b.Run("sequential", func(b *testing.B) {

pkg/activator/handler/tracing_handler.go

Lines changed: 4 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -23,8 +23,10 @@ import (
2323
"go.opentelemetry.io/otel/trace"
2424
)
2525

26-
func NewTracingHandler(tp trace.TracerProvider, next http.Handler) http.Handler {
27-
shim := http.HandlerFunc(func(rw http.ResponseWriter, r *http.Request) {
26+
// NewTracingAttributeHandler creates a handler that adds serving attributes
27+
// to the span
28+
func NewTracingAttributeHandler(tp trace.TracerProvider, next http.Handler) http.Handler {
29+
return http.HandlerFunc(func(rw http.ResponseWriter, r *http.Request) {
2830
defer func() {
2931
// otelhttp middleware creates the labeler
3032
labeler, _ := otelhttp.LabelerFromContext(r.Context())
@@ -36,8 +38,4 @@ func NewTracingHandler(tp trace.TracerProvider, next http.Handler) http.Handler
3638

3739
next.ServeHTTP(rw, r)
3840
})
39-
40-
return otelhttp.NewHandler(shim, "activate",
41-
otelhttp.WithTracerProvider(tp),
42-
)
4341
}

pkg/activator/handler/tracing_handler_test.go

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -30,7 +30,7 @@ import (
3030
rtesting "knative.dev/pkg/reconciler/testing"
3131
)
3232

33-
func TestTracingHandler(t *testing.T) {
33+
func TestTracingAttributeHandler(t *testing.T) {
3434
exporter := tracetest.NewInMemoryExporter()
3535
tp := trace.NewTracerProvider(
3636
trace.WithSyncer(exporter),
@@ -43,7 +43,8 @@ func TestTracingHandler(t *testing.T) {
4343
labeler, _ := otelhttp.LabelerFromContext(r.Context())
4444
labeler.Add(attribute.Bool("x", true))
4545
})
46-
handler := NewTracingHandler(tp, baseHandler)
46+
handler := NewTracingAttributeHandler(tp, baseHandler)
47+
handler = otelhttp.NewHandler(handler, "op", otelhttp.WithTracerProvider(tp))
4748

4849
resp := httptest.NewRecorder()
4950
req := httptest.NewRequest(http.MethodPost, "http://example.com", nil)

0 commit comments

Comments
 (0)