feat(tracing): dont trace spans with full URL path names in ExtractFr…

…omHTTPRequest

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.

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")
+	}
+}