Skip to content

Commit

Permalink
feat(tracing): dont trace spans with full URL path names in ExtractFr…
Browse files Browse the repository at this point in the history
…omHTTPRequest
  • Loading branch information
GeorgeMac committed Nov 21, 2019
1 parent 6b19ea7 commit dbbe687
Show file tree
Hide file tree
Showing 5 changed files with 117 additions and 9 deletions.
2 changes: 2 additions & 0 deletions go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -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
6 changes: 4 additions & 2 deletions go.sum
Original file line number Diff line number Diff line change
Expand Up @@ -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=
Expand Down Expand Up @@ -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=
Expand Down
1 change: 1 addition & 0 deletions http/router.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}

Expand Down
26 changes: 19 additions & 7 deletions kit/tracing/tracing.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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.
Expand Down
91 changes: 91 additions & 0 deletions kit/tracing/tracing_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
)
Expand Down Expand Up @@ -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()

Expand Down Expand Up @@ -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")
}
}

0 comments on commit dbbe687

Please sign in to comment.