diff --git a/kit/tracing/tracing.go b/kit/tracing/tracing.go index e3f9deb56d3..3cac6e45fc8 100644 --- a/kit/tracing/tracing.go +++ b/kit/tracing/tracing.go @@ -50,15 +50,44 @@ func InjectToHTTPRequest(span opentracing.Span, req *http.Request) { func ExtractFromHTTPRequest(req *http.Request, handlerName string) (opentracing.Span, *http.Request) { spanContext, err := opentracing.GlobalTracer().Extract(opentracing.HTTPHeaders, opentracing.HTTPHeadersCarrier(req.Header)) if err != nil { - span, ctx := opentracing.StartSpanFromContext(req.Context(), handlerName+":"+req.URL.Path) + span, ctx := opentracing.StartSpanFromContext(req.Context(), "request") + annotateSpan(span, handlerName, req.URL.Path) + LogError(span, err) - req = req.WithContext(ctx) - return span, req + + return span, req.WithContext(ctx) + } + + span := opentracing.StartSpan("request", opentracing.ChildOf(spanContext), ext.RPCServerOption(spanContext)) + annotateSpan(span, handlerName, req.URL.Path) + + return span, req.WithContext(opentracing.ContextWithSpan(req.Context(), span)) +} + +func annotateSpan(span opentracing.Span, handlerName, path string) { + span.SetTag("handler", handlerName) + span.SetTag("base_path", basePath(path)) + span.LogKV("path", path) +} + +// basePath derives a suitable base path +// for example: +// /foo/bar/1234 -> /foo +// /api/v2/buckets/1234 -> /api/v2/buckets +// /api/v1 -> /api +// / -> / +// "" -> / +func basePath(path string) string { + // combine path root into span name e.g. /api/v2 + if parts := strings.SplitN(path, "/", 5); len(parts) > 1 { + if len(parts) < 4 || !(parts[1] == "api" && parts[2] == "v2") { + return "/" + parts[1] + } + + return "/api/v2/" + parts[3] } - span := opentracing.StartSpan(handlerName+":"+req.URL.Path, opentracing.ChildOf(spanContext), ext.RPCServerOption(spanContext)) - req = req.WithContext(opentracing.ContextWithSpan(req.Context(), span)) - return span, req + return "/" } // StartSpanFromContext is an improved opentracing.StartSpanFromContext. diff --git a/kit/tracing/tracing_test.go b/kit/tracing/tracing_test.go index 26bb20e710c..ef4d64d5b39 100644 --- a/kit/tracing/tracing_test.go +++ b/kit/tracing/tracing_test.go @@ -4,6 +4,7 @@ import ( "context" "fmt" "net/http" + "net/url" "runtime" "testing" @@ -37,6 +38,86 @@ func TestInjectAndExtractHTTPRequest(t *testing.T) { } } +func TestExtractHTTPRequest(t *testing.T) { + tracer := mocktracer.New() + + oldTracer := opentracing.GlobalTracer() + opentracing.SetGlobalTracer(tracer) + defer opentracing.SetGlobalTracer(oldTracer) + + for _, test := range []struct { + name string + handlerName string + path string + tags map[string]interface{} + }{ + { + name: "happy path", + handlerName: "WriteHandler", + path: "/api/v2/write", + tags: map[string]interface{}{ + "base_path": "/api/v2/write", + "handler": "WriteHandler", + }, + }, + { + name: "happy path bucket handler", + handlerName: "BucketHandler", + path: "/api/v2/buckets/12345", + tags: map[string]interface{}{ + "base_path": "/api/v2/buckets", + "handler": "BucketHandler", + }, + }, + { + name: "happy path not prefix with /api/v2", + handlerName: "LegacyWriteHandler", + path: "/write", + tags: map[string]interface{}{ + "base_path": "/write", + "handler": "LegacyWriteHandler", + }, + }, + { + name: "empty path", + handlerName: "Home", + tags: map[string]interface{}{ + "base_path": "/", + "handler": "Home", + }, + }, + } { + t.Run(test.name, func(t *testing.T) { + request, err := http.NewRequest(http.MethodPost, "http://localhost"+test.path, nil) + if err != nil { + t.Fatal(err) + } + + span := tracer.StartSpan("operation name") + + InjectToHTTPRequest(span, request) + gotSpan, _ := ExtractFromHTTPRequest(request, test.handlerName) + + if op := gotSpan.(*mocktracer.MockSpan).OperationName; op != "request" { + t.Fatalf("operation name %q != request", op) + } + + tags := gotSpan.(*mocktracer.MockSpan).Tags() + for k, v := range test.tags { + found, ok := tags[k] + if !ok { + t.Errorf("tag not found in span %q", k) + continue + } + + if found != v { + t.Errorf("expected %v, found %v for tag %q", v, found, k) + } + } + }) + } +} + func TestStartSpanFromContext(t *testing.T) { tracer := mocktracer.New() @@ -219,3 +300,15 @@ func BenchmarkOpentracing_StartSpan_child(b *testing.B) { _ = opentracing.StartSpan("operation name", opentracing.ChildOf(parentSpan.Context())) } } + +func BenchmarkOpentracing_ExtractFromHTTPRequest(b *testing.B) { + b.ReportAllocs() + + req := &http.Request{ + URL: &url.URL{Path: "/api/v2/organization/12345"}, + } + + for n := 0; n < b.N; n++ { + _, _ = ExtractFromHTTPRequest(req, "OrganizationHandler") + } +}