From 3235f89a9caeb3ef7e64e3102d1b201569f71648 Mon Sep 17 00:00:00 2001 From: yura Date: Wed, 27 Mar 2019 21:28:19 +0300 Subject: [PATCH] tracer autodetection for client interceptors Ability to automatically get tracer from request context if no tracer were passed to constructor. --- tracing/opentracing/client_interceptors.go | 15 +++++++++++++ tracing/opentracing/interceptors_test.go | 26 ++++++++++++++++------ tracing/opentracing/options.go | 9 ++++---- tracing/opentracing/server_interceptors.go | 4 ++++ 4 files changed, 43 insertions(+), 11 deletions(-) diff --git a/tracing/opentracing/client_interceptors.go b/tracing/opentracing/client_interceptors.go index f8fdecf5a..4fa9625ac 100644 --- a/tracing/opentracing/client_interceptors.go +++ b/tracing/opentracing/client_interceptors.go @@ -25,6 +25,9 @@ func UnaryClientInterceptor(opts ...Option) grpc.UnaryClientInterceptor { return invoker(parentCtx, method, req, reply, cc, opts...) } newCtx, clientSpan := newClientSpanFromContext(parentCtx, o.tracer, method) + if clientSpan == nil { + return invoker(parentCtx, method, req, reply, cc, opts...) + } err := invoker(newCtx, method, req, reply, cc, opts...) finishClientSpan(clientSpan, err) return err @@ -39,6 +42,9 @@ func StreamClientInterceptor(opts ...Option) grpc.StreamClientInterceptor { return streamer(parentCtx, desc, cc, method, opts...) } newCtx, clientSpan := newClientSpanFromContext(parentCtx, o.tracer, method) + if clientSpan == nil { + return streamer(parentCtx, desc, cc, method, opts...) + } clientStream, err := streamer(newCtx, desc, cc, method, opts...) if err != nil { finishClientSpan(clientSpan, err) @@ -112,6 +118,15 @@ func newClientSpanFromContext(ctx context.Context, tracer opentracing.Tracer, fu var parentSpanCtx opentracing.SpanContext if parent := opentracing.SpanFromContext(ctx); parent != nil { parentSpanCtx = parent.Context() + if tracer == nil { + tracer = parent.Tracer() + } + } + if tracer == nil { + tracer = opentracing.GlobalTracer() + if _, ok := tracer.(opentracing.NoopTracer); ok { + return ctx, nil + } } opts := []opentracing.StartSpanOption{ opentracing.ChildOf(parentSpanCtx), diff --git a/tracing/opentracing/interceptors_test.go b/tracing/opentracing/interceptors_test.go index fec8b0661..ba4dbfff9 100644 --- a/tracing/opentracing/interceptors_test.go +++ b/tracing/opentracing/interceptors_test.go @@ -82,7 +82,19 @@ func TestTaggingSuite(t *testing.T) { } s := &OpentracingSuite{ mockTracer: mockTracer, - InterceptorTestSuite: makeInterceptorTestSuite(t, opts), + InterceptorTestSuite: makeInterceptorTestSuite(t, opts, opts), + } + suite.Run(t, s) +} + +func TestTaggingSuite2(t *testing.T) { + mockTracer := mocktracer.New() + opts := []grpc_opentracing.Option{ + grpc_opentracing.WithTracer(mockTracer), + } + s := &OpentracingSuite{ + mockTracer: mockTracer, + InterceptorTestSuite: makeInterceptorTestSuite(t, nil, opts), } suite.Run(t, s) } @@ -96,26 +108,26 @@ func TestTaggingSuiteJaeger(t *testing.T) { } s := &OpentracingSuite{ mockTracer: mockTracer, - InterceptorTestSuite: makeInterceptorTestSuite(t, opts), + InterceptorTestSuite: makeInterceptorTestSuite(t, opts, opts), } suite.Run(t, s) } -func makeInterceptorTestSuite(t *testing.T, opts []grpc_opentracing.Option) *grpc_testing.InterceptorTestSuite { +func makeInterceptorTestSuite(t *testing.T, clientOpts, serverOpts []grpc_opentracing.Option) *grpc_testing.InterceptorTestSuite { return &grpc_testing.InterceptorTestSuite{ TestService: &tracingAssertService{TestServiceServer: &grpc_testing.TestPingService{T: t}, T: t}, ClientOpts: []grpc.DialOption{ - grpc.WithUnaryInterceptor(grpc_opentracing.UnaryClientInterceptor(opts...)), - grpc.WithStreamInterceptor(grpc_opentracing.StreamClientInterceptor(opts...)), + grpc.WithUnaryInterceptor(grpc_opentracing.UnaryClientInterceptor(clientOpts...)), + grpc.WithStreamInterceptor(grpc_opentracing.StreamClientInterceptor(clientOpts...)), }, ServerOpts: []grpc.ServerOption{ grpc_middleware.WithStreamServerChain( grpc_ctxtags.StreamServerInterceptor(grpc_ctxtags.WithFieldExtractor(grpc_ctxtags.CodeGenRequestFieldExtractor)), - grpc_opentracing.StreamServerInterceptor(opts...)), + grpc_opentracing.StreamServerInterceptor(serverOpts...)), grpc_middleware.WithUnaryServerChain( grpc_ctxtags.UnaryServerInterceptor(grpc_ctxtags.WithFieldExtractor(grpc_ctxtags.CodeGenRequestFieldExtractor)), - grpc_opentracing.UnaryServerInterceptor(opts...)), + grpc_opentracing.UnaryServerInterceptor(serverOpts...)), }, } } diff --git a/tracing/opentracing/options.go b/tracing/opentracing/options.go index e75102b25..4d91554de 100644 --- a/tracing/opentracing/options.go +++ b/tracing/opentracing/options.go @@ -32,9 +32,6 @@ func evaluateOptions(opts []Option) *options { for _, o := range opts { o(optCopy) } - if optCopy.tracer == nil { - optCopy.tracer = opentracing.GlobalTracer() - } return optCopy } @@ -50,6 +47,10 @@ func WithFilterFunc(f FilterFunc) Option { // WithTracer sets a custom tracer to be used for this middleware, otherwise the opentracing.GlobalTracer is used. func WithTracer(tracer opentracing.Tracer) Option { return func(o *options) { - o.tracer = tracer + if tracer == nil { + o.tracer = opentracing.GlobalTracer() + } else { + o.tracer = tracer + } } } diff --git a/tracing/opentracing/server_interceptors.go b/tracing/opentracing/server_interceptors.go index 53764a083..7f641f707 100644 --- a/tracing/opentracing/server_interceptors.go +++ b/tracing/opentracing/server_interceptors.go @@ -56,6 +56,10 @@ func newServerSpanFromInbound(ctx context.Context, tracer opentracing.Tracer, fu grpclog.Printf("grpc_opentracing: failed parsing trace information: %v", err) } + if tracer == nil { + tracer = opentracing.GlobalTracer() + } + serverSpan := tracer.StartSpan( fullMethodName, // this is magical, it attaches the new span to the parent parentSpanContext, and creates an unparented one if empty.