diff --git a/go.mod b/go.mod index c220dcc2236..55622eb37ce 100644 --- a/go.mod +++ b/go.mod @@ -112,3 +112,5 @@ replace github.com/Sirupsen/logrus => github.com/sirupsen/logrus v1.2.0 replace github.com/influxdata/platform => /dev/null replace github.com/goreleaser/goreleaser => github.com/influxdata/goreleaser v0.97.0-influx + +replace github.com/julienschmidt/httprouter => github.com/georgemac/httprouter v1.3.1-0.20191121142037-34e411acc501 diff --git a/go.sum b/go.sum index f1460462b6d..993937e69e2 100644 --- a/go.sum +++ b/go.sum @@ -113,6 +113,10 @@ github.com/flynn/go-shlex v0.0.0-20150515145356-3f9db97f8568 h1:BHsljHzVlRcyQhjr github.com/flynn/go-shlex v0.0.0-20150515145356-3f9db97f8568/go.mod h1:xEzjJPgXI435gkrCt3MPfRiAkVrwSbHsst4LCFVfpJc= github.com/fsnotify/fsnotify v1.4.7 h1:IXs+QLmnXW2CcXuY+8Mzv/fWEsPGWxqefPtCP5CnV9I= github.com/fsnotify/fsnotify v1.4.7/go.mod h1:jwhsz4b93w/PPRr/qN1Yymfu8t87LnFCMoQvtojpjFo= +github.com/georgemac/httprouter v1.3.1-0.20191005171706-08a3b3d20bbe h1:AQVSvv8BnA3cE3JKyxbuQMTREPrfFQ3FO3t0WX0OHT4= +github.com/georgemac/httprouter v1.3.1-0.20191005171706-08a3b3d20bbe/go.mod h1:JR6WtHb+2LUe8TCKY3cZOxFyyO8IZAc4RVcycCCAKdM= +github.com/georgemac/httprouter v1.3.1-0.20191121142037-34e411acc501 h1:eSe2KTNWsu40zA8mT/lJ7y+XnthfQ5umHyHUiLXnKb8= +github.com/georgemac/httprouter v1.3.1-0.20191121142037-34e411acc501/go.mod h1:JR6WtHb+2LUe8TCKY3cZOxFyyO8IZAc4RVcycCCAKdM= github.com/getkin/kin-openapi v0.2.0 h1:PbHHtYZpjKwZtGlIyELgA2DploRrsaXztoNNx9HjwNY= github.com/getkin/kin-openapi v0.2.0/go.mod h1:V1z9xl9oF5Wt7v32ne4FmiF1alpS4dM6mNzoywPOXlk= github.com/ghodss/yaml v1.0.0 h1:wQHKEahhL6wmXdzwWG11gIVCkOv05bNOh+Rxn0yngAk= @@ -267,8 +271,6 @@ github.com/jsternberg/zap-logfmt v1.2.0 h1:1v+PK4/B48cy8cfQbxL4FmmNZrjnIMr2BsnyE github.com/jsternberg/zap-logfmt v1.2.0/go.mod h1:kz+1CUmCutPWABnNkOu9hOHKdT2q3TDYCcsFy9hpqb0= github.com/jtolds/gls v4.20.0+incompatible h1:xdiiI2gbIgH/gLH7ADydsJ1uDOEzR8yvV7C0MuV77Wo= github.com/jtolds/gls v4.20.0+incompatible/go.mod h1:QJZ7F/aHp+rZTRtaJ1ow/lLfFfVYBRgL+9YlvaHOwJU= -github.com/julienschmidt/httprouter v1.2.0 h1:TDTW5Yz1mjftljbcKqRcrYhd4XeOoI98t+9HbQbYf7g= -github.com/julienschmidt/httprouter v1.2.0/go.mod h1:SYymIcj16QtmaHHD7aYtjjsJG7VTCxuUUipMqKk8s4w= github.com/jwilder/encoding v0.0.0-20170811194829-b4e1701a28ef h1:2jNeR4YUziVtswNP9sEFAI913cVrzH85T+8Q6LpYbT0= github.com/jwilder/encoding v0.0.0-20170811194829-b4e1701a28ef/go.mod h1:Ct9fl0F6iIOGgxJ5npU/IUOhOhqlVrGjyIZc8/MagT0= github.com/k0kubun/colorstring v0.0.0-20150214042306-9440f1994b88 h1:uC1QfSlInpQF+M0ao65imhwqKnz3Q2z/d8PWZRMQvDM= diff --git a/http/router.go b/http/router.go index 348fcd3dd16..93dfdc1ded8 100644 --- a/http/router.go +++ b/http/router.go @@ -22,6 +22,7 @@ func NewRouter(h platform.HTTPErrorHandler) *httprouter.Router { router.NotFound = http.HandlerFunc(b.notFound) router.MethodNotAllowed = http.HandlerFunc(b.methodNotAllowed) router.PanicHandler = b.panic + router.AddMatchedRouteToContext = true return router } diff --git a/kit/tracing/tracing.go b/kit/tracing/tracing.go index e3f9deb56d3..9bbbfea89f6 100644 --- a/kit/tracing/tracing.go +++ b/kit/tracing/tracing.go @@ -7,6 +7,7 @@ import ( "runtime" "strings" + "github.com/julienschmidt/httprouter" "github.com/opentracing/opentracing-go" "github.com/opentracing/opentracing-go/ext" "github.com/opentracing/opentracing-go/log" @@ -50,15 +51,26 @@ 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) - LogError(span, err) - req = req.WithContext(ctx) - return span, req + span, ctx := opentracing.StartSpanFromContext(req.Context(), "request") + annotateSpan(span, handlerName, req) + + _ = LogError(span, err) + + return span, req.WithContext(ctx) } - span := opentracing.StartSpan(handlerName+":"+req.URL.Path, opentracing.ChildOf(spanContext), ext.RPCServerOption(spanContext)) - req = req.WithContext(opentracing.ContextWithSpan(req.Context(), span)) - return span, req + span := opentracing.StartSpan("request", opentracing.ChildOf(spanContext), ext.RPCServerOption(spanContext)) + annotateSpan(span, handlerName, req) + + return span, req.WithContext(opentracing.ContextWithSpan(req.Context(), span)) +} + +func annotateSpan(span opentracing.Span, handlerName string, req *http.Request) { + if route := httprouter.MatchedRouteFromContext(req.Context()); route != "" { + span.SetTag("route", route) + } + span.SetTag("handler", handlerName) + span.LogKV("path", req.URL.Path) } // StartSpanFromContext is an improved opentracing.StartSpanFromContext. diff --git a/kit/tracing/tracing_test.go b/kit/tracing/tracing_test.go index 26bb20e710c..5a7a09a7a92 100644 --- a/kit/tracing/tracing_test.go +++ b/kit/tracing/tracing_test.go @@ -4,9 +4,11 @@ import ( "context" "fmt" "net/http" + "net/url" "runtime" "testing" + "github.com/julienschmidt/httprouter" "github.com/opentracing/opentracing-go" "github.com/opentracing/opentracing-go/mocktracer" ) @@ -37,6 +39,83 @@ func TestInjectAndExtractHTTPRequest(t *testing.T) { } } +func TestExtractHTTPRequest(t *testing.T) { + var ( + tracer = mocktracer.New() + oldTracer = opentracing.GlobalTracer() + ctx = context.Background() + ) + + opentracing.SetGlobalTracer(tracer) + defer opentracing.SetGlobalTracer(oldTracer) + + for _, test := range []struct { + name string + handlerName string + path string + ctx context.Context + tags map[string]interface{} + }{ + { + name: "happy path", + handlerName: "WriteHandler", + ctx: context.WithValue(ctx, httprouter.MatchedRouteKey, "/api/v2/write"), + path: "/api/v2/write", + tags: map[string]interface{}{ + "route": "/api/v2/write", + "handler": "WriteHandler", + }, + }, + { + name: "happy path bucket handler", + handlerName: "BucketHandler", + ctx: context.WithValue(ctx, httprouter.MatchedRouteKey, "/api/v2/buckets/:bucket_id"), + path: "/api/v2/buckets/12345", + tags: map[string]interface{}{ + "route": "/api/v2/buckets/:bucket_id", + "handler": "BucketHandler", + }, + }, + { + name: "empty path", + handlerName: "Home", + ctx: ctx, + tags: map[string]interface{}{ + "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.WithContext(test.ctx), 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 +298,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") + } +}