From 2a686c02c0c6fc4bb97945d56ae85c4d33517cbe Mon Sep 17 00:00:00 2001 From: Mahad Zaryab Date: Thu, 31 Oct 2024 21:26:53 -0400 Subject: [PATCH 1/3] Add Trace Response Handler To Server Signed-off-by: Mahad Zaryab --- cmd/query/app/http_handler.go | 17 ----------------- cmd/query/app/server.go | 1 + cmd/query/app/trace_response_handler.go | 25 +++++++++++++++++++++++++ 3 files changed, 26 insertions(+), 17 deletions(-) create mode 100644 cmd/query/app/trace_response_handler.go diff --git a/cmd/query/app/http_handler.go b/cmd/query/app/http_handler.go index e00472babb5..3eec204720e 100644 --- a/cmd/query/app/http_handler.go +++ b/cmd/query/app/http_handler.go @@ -18,7 +18,6 @@ import ( "github.com/gogo/protobuf/proto" "github.com/gorilla/mux" "go.opentelemetry.io/contrib/instrumentation/net/http/otelhttp" - "go.opentelemetry.io/otel/propagation" "go.opentelemetry.io/otel/trace" "go.uber.org/zap" @@ -132,7 +131,6 @@ func (aH *APIHandler) handleFunc( ) *mux.Route { route := aH.formatRoute(routeFmt, args...) var handler http.Handler = http.HandlerFunc(f) - handler = traceResponseHandler(handler) handler = otelhttp.WithRouteTag(route, handler) handler = spanNameHandler(route, handler) return router.HandleFunc(route, handler.ServeHTTP) @@ -517,21 +515,6 @@ func (aH *APIHandler) writeJSON(w http.ResponseWriter, r *http.Request, response } } -// Returns a handler that generates a traceresponse header. -// https://github.com/w3c/trace-context/blob/main/spec/21-http_response_header_format.md -func traceResponseHandler(handler http.Handler) http.Handler { - // We use the standard TraceContext propagator, since the formats are identical. - // But the propagator uses "traceparent" header name, so we inject it into a map - // `carrier` and then use the result to set the "tracereponse" header. - var prop propagation.TraceContext - return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { - carrier := make(map[string]string) - prop.Inject(r.Context(), propagation.MapCarrier(carrier)) - w.Header().Add("traceresponse", carrier["traceparent"]) - handler.ServeHTTP(w, r) - }) -} - func spanNameHandler(spanName string, handler http.Handler) http.Handler { return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { span := trace.SpanFromContext(r.Context()) diff --git a/cmd/query/app/server.go b/cmd/query/app/server.go index 543ede956ee..043fe978d3c 100644 --- a/cmd/query/app/server.go +++ b/cmd/query/app/server.go @@ -201,6 +201,7 @@ func initRouter( if tenancyMgr.Enabled { handler = tenancy.ExtractTenantHTTPHandler(tenancyMgr, handler) } + handler = traceResponseHandler(handler) return handler, staticHandlerCloser } diff --git a/cmd/query/app/trace_response_handler.go b/cmd/query/app/trace_response_handler.go new file mode 100644 index 00000000000..f01dc4a7315 --- /dev/null +++ b/cmd/query/app/trace_response_handler.go @@ -0,0 +1,25 @@ +// Copyright (c) 2024 The Jaeger Authors. +// SPDX-License-Identifier: Apache-2.0 + +package app + +import ( + "net/http" + + "go.opentelemetry.io/otel/propagation" +) + +// Returns a handler that generates a traceresponse header. +// https://github.com/w3c/trace-context/blob/main/spec/21-http_response_header_format.md +func traceResponseHandler(handler http.Handler) http.Handler { + // We use the standard TraceContext propagator, since the formats are identical. + // But the propagator uses "traceparent" header name, so we inject it into a map + // `carrier` and then use the result to set the "tracereponse" header. + var prop propagation.TraceContext + return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + carrier := make(map[string]string) + prop.Inject(r.Context(), propagation.MapCarrier(carrier)) + w.Header().Add("traceresponse", carrier["traceparent"]) + handler.ServeHTTP(w, r) + }) +} From c45555991b246c19782436bceeea0288a43fab01 Mon Sep 17 00:00:00 2001 From: Mahad Zaryab Date: Thu, 31 Oct 2024 21:27:13 -0400 Subject: [PATCH 2/3] Add Unit Test For Trace Response Handler Signed-off-by: Mahad Zaryab --- cmd/query/app/trace_response_handler_test.go | 49 ++++++++++++++++++++ 1 file changed, 49 insertions(+) create mode 100644 cmd/query/app/trace_response_handler_test.go diff --git a/cmd/query/app/trace_response_handler_test.go b/cmd/query/app/trace_response_handler_test.go new file mode 100644 index 00000000000..5bd32de5266 --- /dev/null +++ b/cmd/query/app/trace_response_handler_test.go @@ -0,0 +1,49 @@ +// Copyright (c) 2024 The Jaeger Authors. +// SPDX-License-Identifier: Apache-2.0 + +package app + +import ( + "net/http" + "net/http/httptest" + "testing" + + "github.com/stretchr/testify/require" + sdktrace "go.opentelemetry.io/otel/sdk/trace" + "go.opentelemetry.io/otel/sdk/trace/tracetest" + + "github.com/jaegertracing/jaeger/pkg/jtracer" +) + +func TestTraceResponseHandler(t *testing.T) { + exporter := tracetest.NewInMemoryExporter() + tracerProvider := sdktrace.NewTracerProvider( + sdktrace.WithSyncer(exporter), + sdktrace.WithSampler(sdktrace.AlwaysSample()), + ) + jTracer := jtracer.JTracer{OTEL: tracerProvider} + tracer := jTracer.OTEL.Tracer("instrumentation/trace_response_handler_test") + + emptyHandler := http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {}) + handlerWithTracing := traceResponseHandler(emptyHandler) + hanlderWithInstrumentation := http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + _, span := tracer.Start(r.Context(), "operation") + defer span.End() + + handlerWithTracing.ServeHTTP(w, r) + }) + + server := httptest.NewServer(hanlderWithInstrumentation) + defer server.Close() + + req, err := http.NewRequest(http.MethodGet, server.URL, nil) + require.NoError(t, err) + + resp, err := server.Client().Do(req) + require.NoError(t, err) + + traceResponse := resp.Header.Get("traceresponse") + + // TODO: why isn't this populated? + require.Equal(t, "", traceResponse) +} From fe50c78e05edc8b57126941c524b03b2e98e0715 Mon Sep 17 00:00:00 2001 From: Mahad Zaryab Date: Fri, 1 Nov 2024 17:23:40 -0400 Subject: [PATCH 3/3] Fix Test For Trace Response Handler Signed-off-by: Mahad Zaryab --- cmd/query/app/trace_response_handler_test.go | 46 +++++++++----------- 1 file changed, 20 insertions(+), 26 deletions(-) diff --git a/cmd/query/app/trace_response_handler_test.go b/cmd/query/app/trace_response_handler_test.go index 5bd32de5266..59f988f2a37 100644 --- a/cmd/query/app/trace_response_handler_test.go +++ b/cmd/query/app/trace_response_handler_test.go @@ -4,46 +4,40 @@ package app import ( + "context" "net/http" "net/http/httptest" + "strings" "testing" "github.com/stretchr/testify/require" - sdktrace "go.opentelemetry.io/otel/sdk/trace" + "go.opentelemetry.io/otel/sdk/trace" "go.opentelemetry.io/otel/sdk/trace/tracetest" - - "github.com/jaegertracing/jaeger/pkg/jtracer" ) func TestTraceResponseHandler(t *testing.T) { - exporter := tracetest.NewInMemoryExporter() - tracerProvider := sdktrace.NewTracerProvider( - sdktrace.WithSyncer(exporter), - sdktrace.WithSampler(sdktrace.AlwaysSample()), - ) - jTracer := jtracer.JTracer{OTEL: tracerProvider} - tracer := jTracer.OTEL.Tracer("instrumentation/trace_response_handler_test") - - emptyHandler := http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {}) - handlerWithTracing := traceResponseHandler(emptyHandler) - hanlderWithInstrumentation := http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { - _, span := tracer.Start(r.Context(), "operation") - defer span.End() - - handlerWithTracing.ServeHTTP(w, r) + emptyHandler := http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { + w.Write([]byte{}) }) + handler := traceResponseHandler(emptyHandler) - server := httptest.NewServer(hanlderWithInstrumentation) - defer server.Close() + exporter := tracetest.NewInMemoryExporter() + tracerProvider := trace.NewTracerProvider( + trace.WithSyncer(exporter), + trace.WithSampler(trace.AlwaysSample()), + ) + tracer := tracerProvider.Tracer("test-tracer") + ctx, span := tracer.Start(context.Background(), "test-span") + defer span.End() - req, err := http.NewRequest(http.MethodGet, server.URL, nil) + req, err := http.NewRequestWithContext(ctx, http.MethodGet, "http://localhost:8080", nil) require.NoError(t, err) - resp, err := server.Client().Do(req) - require.NoError(t, err) + w := httptest.NewRecorder() - traceResponse := resp.Header.Get("traceresponse") + handler.ServeHTTP(w, req) - // TODO: why isn't this populated? - require.Equal(t, "", traceResponse) + traceResponse := w.Header().Get("traceresponse") + parts := strings.Split(traceResponse, "-") + require.Len(t, parts, 4) }