From d24c90e3ad2138645241426d8efd2521e5929208 Mon Sep 17 00:00:00 2001 From: Sam Xie Date: Wed, 26 Aug 2020 14:08:44 +0800 Subject: [PATCH 01/17] Unify instrumentation options for mongo-driver --- .../go.mongodb.org/mongo-driver/config.go | 21 ++++++++++--------- .../go.mongodb.org/mongo-driver/mongo_test.go | 4 ++-- internal/trace/mock_tracer.go | 8 +++++++ 3 files changed, 21 insertions(+), 12 deletions(-) diff --git a/instrumentation/go.mongodb.org/mongo-driver/config.go b/instrumentation/go.mongodb.org/mongo-driver/config.go index b4941fa0d55..5093e1cb539 100644 --- a/instrumentation/go.mongodb.org/mongo-driver/config.go +++ b/instrumentation/go.mongodb.org/mongo-driver/config.go @@ -25,30 +25,31 @@ const ( // Config is used to configure the mongo tracer. type Config struct { + TraceProvider trace.Provider + Tracer trace.Tracer } // newConfig returns a Config with all Options set. func newConfig(opts ...Option) Config { - cfg := Config{} + cfg := Config{ + TraceProvider: global.TraceProvider(), + } for _, opt := range opts { opt(&cfg) } - if cfg.Tracer == nil { - cfg.Tracer = global.Tracer(defaultTracerName) - } + + cfg.Tracer = cfg.TraceProvider.Tracer(defaultTracerName) return cfg } // Option specifies instrumentation configuration options. type Option func(*Config) -// WithTracer specifies a tracer to use for creating spans. If none is -// specified, a tracer named -// "go.opentelemetry.io/contrib/instrumentation/go.mongodb.org/mongo-driver" -// from the global provider is used. -func WithTracer(tracer trace.Tracer) Option { +// WithTracerProvider specifies a tracer provider to use for creating a tracer. +// If none is specified, the global provider is used. +func WithTracerProvider(provider trace.Provider) Option { return func(cfg *Config) { - cfg.Tracer = tracer + cfg.TraceProvider = provider } } diff --git a/instrumentation/go.mongodb.org/mongo-driver/mongo_test.go b/instrumentation/go.mongodb.org/mongo-driver/mongo_test.go index 2e29ccdc32e..c17d60483c5 100644 --- a/instrumentation/go.mongodb.org/mongo-driver/mongo_test.go +++ b/instrumentation/go.mongodb.org/mongo-driver/mongo_test.go @@ -36,7 +36,7 @@ func TestMain(m *testing.M) { } func Test(t *testing.T) { - mt := mocktracer.NewTracer("mongodb") + provider, mt := mocktracer.NewProviderAndTracer(defaultTracerName) hostname, port := "localhost", "27017" @@ -47,7 +47,7 @@ func Test(t *testing.T) { addr := "mongodb://localhost:27017/?connect=direct" opts := options.Client() - opts.Monitor = NewMonitor("mongo", WithTracer(mt)) + opts.Monitor = NewMonitor("mongo", WithTracerProvider(provider)) opts.ApplyURI(addr) client, err := mongo.Connect(ctx, opts) if err != nil { diff --git a/internal/trace/mock_tracer.go b/internal/trace/mock_tracer.go index 0ecbcc5dd9d..6963bd5f400 100644 --- a/internal/trace/mock_tracer.go +++ b/internal/trace/mock_tracer.go @@ -158,3 +158,11 @@ func (mt *Tracer) Start(ctx context.Context, name string, o ...oteltrace.StartOp return oteltrace.ContextWithSpan(ctx, span), span } + +// NewProviderAndTracer return mock provider and tracer. +func NewProviderAndTracer(tracerName string) (*Provider, *Tracer) { + var provider Provider + tracer := provider.Tracer(tracerName) + + return &provider, tracer.(*Tracer) +} From 61fe1d156c4527590eaee0c41144b09f9e318717 Mon Sep 17 00:00:00 2001 From: Sam Xie Date: Wed, 26 Aug 2020 14:11:22 +0800 Subject: [PATCH 02/17] Unify instrumentation options for gin --- .../github.com/gin-gonic/gin/gintrace.go | 5 ++-- .../github.com/gin-gonic/gin/gintrace_test.go | 26 +++++++++---------- .../github.com/gin-gonic/gin/option.go | 23 ++++++++-------- 3 files changed, 27 insertions(+), 27 deletions(-) diff --git a/instrumentation/github.com/gin-gonic/gin/gintrace.go b/instrumentation/github.com/gin-gonic/gin/gintrace.go index f86a1bc50b2..ed1e0ee4d89 100644 --- a/instrumentation/github.com/gin-gonic/gin/gintrace.go +++ b/instrumentation/github.com/gin-gonic/gin/gintrace.go @@ -43,9 +43,10 @@ func Middleware(service string, opts ...Option) gin.HandlerFunc { for _, opt := range opts { opt(&cfg) } - if cfg.Tracer == nil { - cfg.Tracer = otelglobal.Tracer(tracerName) + if cfg.TraceProvider == nil { + cfg.TraceProvider = otelglobal.TraceProvider() } + cfg.Tracer = cfg.TraceProvider.Tracer(tracerName) if cfg.Propagators == nil { cfg.Propagators = otelglobal.Propagators() } diff --git a/instrumentation/github.com/gin-gonic/gin/gintrace_test.go b/instrumentation/github.com/gin-gonic/gin/gintrace_test.go index 3afe60ff212..92b4ec37df4 100644 --- a/instrumentation/github.com/gin-gonic/gin/gintrace_test.go +++ b/instrumentation/github.com/gin-gonic/gin/gintrace_test.go @@ -63,10 +63,10 @@ func TestChildSpanFromGlobalTracer(t *testing.T) { } func TestChildSpanFromCustomTracer(t *testing.T) { - tracer := mocktrace.NewTracer("test-tracer") + provider, _ := mocktrace.NewProviderAndTracer(tracerName) router := gin.New() - router.Use(Middleware("foobar", WithTracer(tracer))) + router.Use(Middleware("foobar", WithTracerProvider(provider))) router.GET("/user/:id", func(c *gin.Context) { span := oteltrace.SpanFromContext(c.Request.Context()) _, ok := span.(*mocktrace.Span) @@ -74,7 +74,7 @@ func TestChildSpanFromCustomTracer(t *testing.T) { spanTracer := span.Tracer() mockTracer, ok := spanTracer.(*mocktrace.Tracer) require.True(t, ok) - assert.Equal(t, "test-tracer", mockTracer.Name) + assert.Equal(t, tracerName, mockTracer.Name) }) r := httptest.NewRequest("GET", "/user/123", nil) @@ -84,10 +84,10 @@ func TestChildSpanFromCustomTracer(t *testing.T) { } func TestTrace200(t *testing.T) { - tracer := mocktrace.NewTracer("test-tracer") + provider, tracer := mocktrace.NewProviderAndTracer(tracerName) router := gin.New() - router.Use(Middleware("foobar", WithTracer(tracer))) + router.Use(Middleware("foobar", WithTracerProvider(provider))) router.GET("/user/:id", func(c *gin.Context) { span := oteltrace.SpanFromContext(c.Request.Context()) mspan, ok := span.(*mocktrace.Span) @@ -119,11 +119,11 @@ func TestTrace200(t *testing.T) { } func TestError(t *testing.T) { - tracer := mocktrace.NewTracer("test-tracer") + provider, tracer := mocktrace.NewProviderAndTracer(tracerName) // setup router := gin.New() - router.Use(Middleware("foobar", WithTracer(tracer))) + router.Use(Middleware("foobar", WithTracerProvider(provider))) // configure a handler that returns an error and 5xx status // code @@ -149,11 +149,11 @@ func TestError(t *testing.T) { } func TestHTML(t *testing.T) { - tracer := mocktrace.NewTracer("test-tracer") + provider, tracer := mocktrace.NewProviderAndTracer(tracerName) // setup router := gin.New() - router.Use(Middleware("foobar", WithTracer(tracer))) + router.Use(Middleware("foobar", WithTracerProvider(provider))) // add a template tmpl := template.Must(template.New("hello").Parse("hello {{.}}")) @@ -203,7 +203,7 @@ func TestGetSpanNotInstrumented(t *testing.T) { } func TestPropagationWithGlobalPropagators(t *testing.T) { - tracer := mocktrace.NewTracer("test-tracer") + provider, tracer := mocktrace.NewProviderAndTracer(tracerName) r := httptest.NewRequest("GET", "/user/123", nil) w := httptest.NewRecorder() @@ -212,7 +212,7 @@ func TestPropagationWithGlobalPropagators(t *testing.T) { otelpropagation.InjectHTTP(ctx, otelglobal.Propagators(), r.Header) router := gin.New() - router.Use(Middleware("foobar", WithTracer(tracer))) + router.Use(Middleware("foobar", WithTracerProvider(provider))) router.GET("/user/:id", func(c *gin.Context) { span := oteltrace.SpanFromContext(c.Request.Context()) mspan, ok := span.(*mocktrace.Span) @@ -225,7 +225,7 @@ func TestPropagationWithGlobalPropagators(t *testing.T) { } func TestPropagationWithCustomPropagators(t *testing.T) { - tracer := mocktrace.NewTracer("test-tracer") + provider, tracer := mocktrace.NewProviderAndTracer(tracerName) b3 := oteltrace.B3{} props := otelpropagation.New( otelpropagation.WithExtractors(b3), @@ -239,7 +239,7 @@ func TestPropagationWithCustomPropagators(t *testing.T) { otelpropagation.InjectHTTP(ctx, props, r.Header) router := gin.New() - router.Use(Middleware("foobar", WithTracer(tracer), WithPropagators(props))) + router.Use(Middleware("foobar", WithTracerProvider(provider), WithPropagators(props))) router.GET("/user/:id", func(c *gin.Context) { span := oteltrace.SpanFromContext(c.Request.Context()) mspan, ok := span.(*mocktrace.Span) diff --git a/instrumentation/github.com/gin-gonic/gin/option.go b/instrumentation/github.com/gin-gonic/gin/option.go index d8195dee03d..25b44c73627 100644 --- a/instrumentation/github.com/gin-gonic/gin/option.go +++ b/instrumentation/github.com/gin-gonic/gin/option.go @@ -22,23 +22,14 @@ import ( ) type Config struct { - Tracer oteltrace.Tracer - Propagators otelpropagation.Propagators + TraceProvider oteltrace.Provider + Tracer oteltrace.Tracer + Propagators otelpropagation.Propagators } // Option specifies instrumentation configuration options. type Option func(*Config) -// WithTracer specifies a tracer to use for creating spans. If none is -// specified, a tracer named -// "go.opentelemetry.io/contrib/instrumentation/github.com/gin-gonic/gin" -// from the global provider is used. -func WithTracer(tracer oteltrace.Tracer) Option { - return func(cfg *Config) { - cfg.Tracer = tracer - } -} - // WithPropagators specifies propagators to use for extracting // information from the HTTP requests. If none are specified, global // ones will be used. @@ -47,3 +38,11 @@ func WithPropagators(propagators otelpropagation.Propagators) Option { cfg.Propagators = propagators } } + +// WithTracerProvider specifies a tracer provider to use for creating a tracer. +// If none is specified, the global provider is used. +func WithTracerProvider(provider oteltrace.Provider) Option { + return func(cfg *Config) { + cfg.TraceProvider = provider + } +} From fab48f6ba98f4afe5e3019ad1974a088920c86a7 Mon Sep 17 00:00:00 2001 From: Sam Xie Date: Wed, 26 Aug 2020 14:13:39 +0800 Subject: [PATCH 03/17] Unify instrumentation options for gorilla --- .../github.com/gorilla/mux/config.go | 23 +++++++++---------- instrumentation/github.com/gorilla/mux/mux.go | 5 ++-- .../github.com/gorilla/mux/mux_test.go | 19 +++++++-------- 3 files changed, 24 insertions(+), 23 deletions(-) diff --git a/instrumentation/github.com/gorilla/mux/config.go b/instrumentation/github.com/gorilla/mux/config.go index a126e738183..77ccecff68e 100644 --- a/instrumentation/github.com/gorilla/mux/config.go +++ b/instrumentation/github.com/gorilla/mux/config.go @@ -21,23 +21,14 @@ import ( // Config is used to configure the mux middleware. type Config struct { - Tracer oteltrace.Tracer - Propagators otelpropagation.Propagators + TraceProvider oteltrace.Provider + Tracer oteltrace.Tracer + Propagators otelpropagation.Propagators } // Option specifies instrumentation configuration options. type Option func(*Config) -// WithTracer specifies a tracer to use for creating spans. If none is -// specified, a tracer named -// "go.opentelemetry.io/contrib/instrumentation/github.com/gorilla/mux" -// from the global provider is used. -func WithTracer(tracer oteltrace.Tracer) Option { - return func(cfg *Config) { - cfg.Tracer = tracer - } -} - // WithPropagators specifies propagators to use for extracting // information from the HTTP requests. If none are specified, global // ones will be used. @@ -46,3 +37,11 @@ func WithPropagators(propagators otelpropagation.Propagators) Option { cfg.Propagators = propagators } } + +// WithTracerProvider specifies a tracer provider to use for creating a tracer. +// If none is specified, the global provider is used. +func WithTracerProvider(provider oteltrace.Provider) Option { + return func(cfg *Config) { + cfg.TraceProvider = provider + } +} diff --git a/instrumentation/github.com/gorilla/mux/mux.go b/instrumentation/github.com/gorilla/mux/mux.go index c371659c2ef..f91aacb95e7 100644 --- a/instrumentation/github.com/gorilla/mux/mux.go +++ b/instrumentation/github.com/gorilla/mux/mux.go @@ -39,9 +39,10 @@ func Middleware(service string, opts ...Option) mux.MiddlewareFunc { for _, opt := range opts { opt(&cfg) } - if cfg.Tracer == nil { - cfg.Tracer = otelglobal.Tracer(tracerName) + if cfg.TraceProvider == nil { + cfg.TraceProvider = otelglobal.TraceProvider() } + cfg.Tracer = cfg.TraceProvider.Tracer(tracerName) if cfg.Propagators == nil { cfg.Propagators = otelglobal.Propagators() } diff --git a/instrumentation/github.com/gorilla/mux/mux_test.go b/instrumentation/github.com/gorilla/mux/mux_test.go index cd1d2646844..d131d504283 100644 --- a/instrumentation/github.com/gorilla/mux/mux_test.go +++ b/instrumentation/github.com/gorilla/mux/mux_test.go @@ -54,10 +54,10 @@ func TestChildSpanFromGlobalTracer(t *testing.T) { } func TestChildSpanFromCustomTracer(t *testing.T) { - tracer := mocktrace.NewTracer("test-tracer") + provider, _ := mocktrace.NewProviderAndTracer(tracerName) router := mux.NewRouter() - router.Use(Middleware("foobar", WithTracer(tracer))) + router.Use(Middleware("foobar", WithTracerProvider(provider))) router.HandleFunc("/user/{id}", http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { span := oteltrace.SpanFromContext(r.Context()) _, ok := span.(*mocktrace.Span) @@ -65,7 +65,7 @@ func TestChildSpanFromCustomTracer(t *testing.T) { spanTracer := span.Tracer() mockTracer, ok := spanTracer.(*mocktrace.Tracer) require.True(t, ok) - assert.Equal(t, "test-tracer", mockTracer.Name) + assert.Equal(t, tracerName, mockTracer.Name) w.WriteHeader(http.StatusOK) })) @@ -76,10 +76,10 @@ func TestChildSpanFromCustomTracer(t *testing.T) { } func TestChildSpanNames(t *testing.T) { - tracer := mocktrace.NewTracer("test-tracer") + provider, tracer := mocktrace.NewProviderAndTracer(tracerName) router := mux.NewRouter() - router.Use(Middleware("foobar", WithTracer(tracer))) + router.Use(Middleware("foobar", WithTracerProvider(provider))) router.HandleFunc("/user/{id:[0-9]+}", http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { w.WriteHeader(http.StatusOK) })) @@ -132,7 +132,7 @@ func TestGetSpanNotInstrumented(t *testing.T) { } func TestPropagationWithGlobalPropagators(t *testing.T) { - tracer := mocktrace.NewTracer("test-tracer") + provider, tracer := mocktrace.NewProviderAndTracer(tracerName) r := httptest.NewRequest("GET", "/user/123", nil) w := httptest.NewRecorder() @@ -141,7 +141,7 @@ func TestPropagationWithGlobalPropagators(t *testing.T) { otelpropagation.InjectHTTP(ctx, otelglobal.Propagators(), r.Header) router := mux.NewRouter() - router.Use(Middleware("foobar", WithTracer(tracer))) + router.Use(Middleware("foobar", WithTracerProvider(provider))) router.HandleFunc("/user/{id}", http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { span := oteltrace.SpanFromContext(r.Context()) mspan, ok := span.(*mocktrace.Span) @@ -155,7 +155,8 @@ func TestPropagationWithGlobalPropagators(t *testing.T) { } func TestPropagationWithCustomPropagators(t *testing.T) { - tracer := mocktrace.NewTracer("test-tracer") + provider, tracer := mocktrace.NewProviderAndTracer(tracerName) + b3 := oteltrace.B3{} props := otelpropagation.New( otelpropagation.WithExtractors(b3), @@ -169,7 +170,7 @@ func TestPropagationWithCustomPropagators(t *testing.T) { otelpropagation.InjectHTTP(ctx, props, r.Header) router := mux.NewRouter() - router.Use(Middleware("foobar", WithTracer(tracer), WithPropagators(props))) + router.Use(Middleware("foobar", WithTracerProvider(provider), WithPropagators(props))) router.HandleFunc("/user/{id}", http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { span := oteltrace.SpanFromContext(r.Context()) mspan, ok := span.(*mocktrace.Span) From 2b9cf5986716c5ecd090625bb3c22bc7b586728c Mon Sep 17 00:00:00 2001 From: Sam Xie Date: Wed, 26 Aug 2020 14:14:52 +0800 Subject: [PATCH 04/17] Unify instrumentation options for labstack echo --- .../github.com/labstack/echo/config.go | 23 +++++++++---------- .../github.com/labstack/echo/echo.go | 5 ++-- .../github.com/labstack/echo/echo_test.go | 23 ++++++++++--------- 3 files changed, 26 insertions(+), 25 deletions(-) diff --git a/instrumentation/github.com/labstack/echo/config.go b/instrumentation/github.com/labstack/echo/config.go index cf90b04ed10..b7fa83c2857 100644 --- a/instrumentation/github.com/labstack/echo/config.go +++ b/instrumentation/github.com/labstack/echo/config.go @@ -21,23 +21,14 @@ import ( // Config is used to configure the mux middleware. type Config struct { - Tracer oteltrace.Tracer - Propagators otelpropagation.Propagators + TraceProvider oteltrace.Provider + Tracer oteltrace.Tracer + Propagators otelpropagation.Propagators } // Option specifies instrumentation configuration options. type Option func(*Config) -// WithTracer specifies a tracer to use for creating spans. If none is -// specified, a tracer named -// "go.opentelemetry.io/contrib/instrumentation/github.com/labstack/echo" -// from the global provider is used. -func WithTracer(tracer oteltrace.Tracer) Option { - return func(cfg *Config) { - cfg.Tracer = tracer - } -} - // WithPropagators specifies propagators to use for extracting // information from the HTTP requests. If none are specified, global // ones will be used. @@ -46,3 +37,11 @@ func WithPropagators(propagators otelpropagation.Propagators) Option { cfg.Propagators = propagators } } + +// WithTracerProvider specifies a tracer provider to use for creating a tracer. +// If none is specified, the global provider is used. +func WithTracerProvider(provider oteltrace.Provider) Option { + return func(cfg *Config) { + cfg.TraceProvider = provider + } +} diff --git a/instrumentation/github.com/labstack/echo/echo.go b/instrumentation/github.com/labstack/echo/echo.go index 82d3eec65c8..cd730b4e0ad 100644 --- a/instrumentation/github.com/labstack/echo/echo.go +++ b/instrumentation/github.com/labstack/echo/echo.go @@ -37,9 +37,10 @@ func Middleware(service string, opts ...Option) echo.MiddlewareFunc { for _, opt := range opts { opt(&cfg) } - if cfg.Tracer == nil { - cfg.Tracer = otelglobal.Tracer(tracerName) + if cfg.TraceProvider == nil { + cfg.TraceProvider = otelglobal.TraceProvider() } + cfg.Tracer = cfg.TraceProvider.Tracer(tracerName) if cfg.Propagators == nil { cfg.Propagators = otelglobal.Propagators() } diff --git a/instrumentation/github.com/labstack/echo/echo_test.go b/instrumentation/github.com/labstack/echo/echo_test.go index 35fe292a221..e6cc7c09ec4 100644 --- a/instrumentation/github.com/labstack/echo/echo_test.go +++ b/instrumentation/github.com/labstack/echo/echo_test.go @@ -60,10 +60,10 @@ func TestChildSpanFromGlobalTracer(t *testing.T) { } func TestChildSpanFromCustomTracer(t *testing.T) { - tracer := mocktrace.NewTracer("test-tracer") + provider, _ := mocktrace.NewProviderAndTracer(tracerName) router := echo.New() - router.Use(Middleware("foobar", WithTracer(tracer))) + router.Use(Middleware("foobar", WithTracerProvider(provider))) router.GET("/user/:id", func(c echo.Context) error { span := oteltrace.SpanFromContext(c.Request().Context()) _, ok := span.(*mocktrace.Span) @@ -71,7 +71,7 @@ func TestChildSpanFromCustomTracer(t *testing.T) { spanTracer := span.Tracer() mockTracer, ok := spanTracer.(*mocktrace.Tracer) require.True(t, ok) - assert.Equal(t, "test-tracer", mockTracer.Name) + assert.Equal(t, tracerName, mockTracer.Name) return c.NoContent(200) }) @@ -82,10 +82,10 @@ func TestChildSpanFromCustomTracer(t *testing.T) { } func TestTrace200(t *testing.T) { - tracer := mocktrace.NewTracer("test-tracer") + provider, tracer := mocktrace.NewProviderAndTracer(tracerName) router := echo.New() - router.Use(Middleware("foobar", WithTracer(tracer))) + router.Use(Middleware("foobar", WithTracerProvider(provider))) router.GET("/user/:id", func(c echo.Context) error { span := oteltrace.SpanFromContext(c.Request().Context()) mspan, ok := span.(*mocktrace.Span) @@ -117,11 +117,11 @@ func TestTrace200(t *testing.T) { } func TestError(t *testing.T) { - tracer := mocktrace.NewTracer("test-tracer") + provider, tracer := mocktrace.NewProviderAndTracer(tracerName) // setup router := echo.New() - router.Use(Middleware("foobar", WithTracer(tracer))) + router.Use(Middleware("foobar", WithTracerProvider(provider))) wantErr := errors.New("oh no") // configure a handler that returns an error and 5xx status // code @@ -163,7 +163,7 @@ func TestGetSpanNotInstrumented(t *testing.T) { } func TestPropagationWithGlobalPropagators(t *testing.T) { - tracer := mocktrace.NewTracer("test-tracer") + provider, tracer := mocktrace.NewProviderAndTracer(tracerName) r := httptest.NewRequest("GET", "/user/123", nil) w := httptest.NewRecorder() @@ -172,7 +172,7 @@ func TestPropagationWithGlobalPropagators(t *testing.T) { otelpropagation.InjectHTTP(ctx, otelglobal.Propagators(), r.Header) router := echo.New() - router.Use(Middleware("foobar", WithTracer(tracer))) + router.Use(Middleware("foobar", WithTracerProvider(provider))) router.GET("/user/:id", func(c echo.Context) error { span := oteltrace.SpanFromContext(c.Request().Context()) mspan, ok := span.(*mocktrace.Span) @@ -186,7 +186,8 @@ func TestPropagationWithGlobalPropagators(t *testing.T) { } func TestPropagationWithCustomPropagators(t *testing.T) { - tracer := mocktrace.NewTracer("test-tracer") + provider, tracer := mocktrace.NewProviderAndTracer(tracerName) + b3 := oteltrace.B3{} props := otelpropagation.New( otelpropagation.WithExtractors(b3), @@ -200,7 +201,7 @@ func TestPropagationWithCustomPropagators(t *testing.T) { otelpropagation.InjectHTTP(ctx, props, r.Header) router := echo.New() - router.Use(Middleware("foobar", WithTracer(tracer), WithPropagators(props))) + router.Use(Middleware("foobar", WithTracerProvider(provider), WithPropagators(props))) router.GET("/user/:id", func(c echo.Context) error { span := oteltrace.SpanFromContext(c.Request().Context()) mspan, ok := span.(*mocktrace.Span) From c1a28d6b8732a079da7267615ba857a1ca727b40 Mon Sep 17 00:00:00 2001 From: Sam Xie Date: Wed, 26 Aug 2020 14:15:37 +0800 Subject: [PATCH 05/17] Unify instrumentation options for go-restful --- .../github.com/emicklei/go-restful/config.go | 23 ++++++----- .../github.com/emicklei/go-restful/restful.go | 5 ++- .../emicklei/go-restful/restful_test.go | 38 +++++++++---------- 3 files changed, 33 insertions(+), 33 deletions(-) diff --git a/instrumentation/github.com/emicklei/go-restful/config.go b/instrumentation/github.com/emicklei/go-restful/config.go index f983dbed1b8..d561a056da8 100644 --- a/instrumentation/github.com/emicklei/go-restful/config.go +++ b/instrumentation/github.com/emicklei/go-restful/config.go @@ -21,23 +21,14 @@ import ( // Config is used to configure the go-restful middleware. type Config struct { - Tracer oteltrace.Tracer - Propagators otelpropagation.Propagators + TraceProvider oteltrace.Provider + Tracer oteltrace.Tracer + Propagators otelpropagation.Propagators } // Option specifies instrumentation configuration options. type Option func(*Config) -// WithTracer specifies a tracer to use for creating spans. If none is -// specified, a tracer named -// "go.opentelemetry.io/contrib/instrumentation/emicklei/go-restful" from the global -// provider is used. -func WithTracer(tracer oteltrace.Tracer) Option { - return func(cfg *Config) { - cfg.Tracer = tracer - } -} - // WithPropagators specifies propagators to use for extracting // information from the HTTP requests. If none are specified, global // ones will be used. @@ -46,3 +37,11 @@ func WithPropagators(propagators otelpropagation.Propagators) Option { cfg.Propagators = propagators } } + +// WithTracerProvider specifies a tracer provider to use for creating a tracer. +// If none is specified, the global provider is used. +func WithTracerProvider(provider oteltrace.Provider) Option { + return func(cfg *Config) { + cfg.TraceProvider = provider + } +} diff --git a/instrumentation/github.com/emicklei/go-restful/restful.go b/instrumentation/github.com/emicklei/go-restful/restful.go index 3314c13d243..5d9b7a9cff1 100644 --- a/instrumentation/github.com/emicklei/go-restful/restful.go +++ b/instrumentation/github.com/emicklei/go-restful/restful.go @@ -38,9 +38,10 @@ func OTelFilter(service string, opts ...Option) restful.FilterFunction { for _, opt := range opts { opt(&cfg) } - if cfg.Tracer == nil { - cfg.Tracer = otelglobal.TraceProvider().Tracer(tracerName, oteltrace.WithInstrumentationVersion(tracerVersion)) + if cfg.TraceProvider == nil { + cfg.TraceProvider = otelglobal.TraceProvider() } + cfg.Tracer = cfg.TraceProvider.Tracer(tracerName, oteltrace.WithInstrumentationVersion(tracerVersion)) if cfg.Propagators == nil { cfg.Propagators = otelglobal.Propagators() } diff --git a/instrumentation/github.com/emicklei/go-restful/restful_test.go b/instrumentation/github.com/emicklei/go-restful/restful_test.go index ca65537c46a..450bef81286 100644 --- a/instrumentation/github.com/emicklei/go-restful/restful_test.go +++ b/instrumentation/github.com/emicklei/go-restful/restful_test.go @@ -33,6 +33,8 @@ import ( otelkv "go.opentelemetry.io/otel/label" ) +const tracerName = "go.opentelemetry.io/contrib/instrumentation/github.com/emicklei/go-restful" + func TestChildSpanFromGlobalTracer(t *testing.T) { otelglobal.SetTraceProvider(&mocktrace.Provider{}) @@ -43,7 +45,7 @@ func TestChildSpanFromGlobalTracer(t *testing.T) { spanTracer := span.Tracer() mockTracer, ok := spanTracer.(*mocktrace.Tracer) require.True(t, ok) - assert.Equal(t, "go.opentelemetry.io/contrib/instrumentation/github.com/emicklei/go-restful", mockTracer.Name) + assert.Equal(t, tracerName, mockTracer.Name) resp.WriteHeader(http.StatusOK) } ws := &restful.WebService{} @@ -61,7 +63,7 @@ func TestChildSpanFromGlobalTracer(t *testing.T) { } func TestChildSpanFromCustomTracer(t *testing.T) { - tracer := mocktrace.NewTracer("test-tracer") + provider, _ := mocktrace.NewProviderAndTracer(tracerName) handlerFunc := func(req *restful.Request, resp *restful.Response) { span := oteltrace.SpanFromContext(req.Request.Context()) @@ -70,14 +72,14 @@ func TestChildSpanFromCustomTracer(t *testing.T) { spanTracer := span.Tracer() mockTracer, ok := spanTracer.(*mocktrace.Tracer) require.True(t, ok) - assert.Equal(t, "test-tracer", mockTracer.Name) + assert.Equal(t, tracerName, mockTracer.Name) resp.WriteHeader(http.StatusOK) } ws := &restful.WebService{} ws.Route(ws.GET("/user/{id}").To(handlerFunc)) container := restful.NewContainer() - container.Filter(restfultrace.OTelFilter("my-service", restfultrace.WithTracer(tracer))) + container.Filter(restfultrace.OTelFilter("my-service", restfultrace.WithTracerProvider(provider))) container.Add(ws) r := httptest.NewRequest("GET", "/user/123", nil) @@ -87,7 +89,7 @@ func TestChildSpanFromCustomTracer(t *testing.T) { } func TestChildSpanNames(t *testing.T) { - tracer := mocktrace.NewTracer("test-tracer") + provider, tracer := mocktrace.NewProviderAndTracer(tracerName) handlerFunc := func(req *restful.Request, resp *restful.Response) { resp.WriteHeader(http.StatusOK) @@ -96,7 +98,7 @@ func TestChildSpanNames(t *testing.T) { ws.Route(ws.GET("/user/{id:[0-9]+}").To(handlerFunc)) container := restful.NewContainer() - container.Filter(restfultrace.OTelFilter("foobar", restfultrace.WithTracer(tracer))) + container.Filter(restfultrace.OTelFilter("foobar", restfultrace.WithTracerProvider(provider))) container.Add(ws) ws.Route(ws.GET("/book/{title}").To(func(req *restful.Request, resp *restful.Response) { @@ -152,7 +154,7 @@ func TestGetSpanNotInstrumented(t *testing.T) { } func TestPropagationWithGlobalPropagators(t *testing.T) { - tracer := mocktrace.NewTracer("test-tracer") + provider, tracer := mocktrace.NewProviderAndTracer(tracerName) r := httptest.NewRequest("GET", "/user/123", nil) w := httptest.NewRecorder() @@ -172,14 +174,14 @@ func TestPropagationWithGlobalPropagators(t *testing.T) { ws.Route(ws.GET("/user/{id}").To(handlerFunc)) container := restful.NewContainer() - container.Filter(restfultrace.OTelFilter("foobar", restfultrace.WithTracer(tracer))) + container.Filter(restfultrace.OTelFilter("foobar", restfultrace.WithTracerProvider(provider))) container.Add(ws) container.ServeHTTP(w, r) } func TestPropagationWithCustomPropagators(t *testing.T) { - tracer := mocktrace.NewTracer("test-tracer") + provider, tracer := mocktrace.NewProviderAndTracer(tracerName) b3 := oteltrace.B3{} props := otelpropagation.New( otelpropagation.WithExtractors(b3), @@ -205,7 +207,7 @@ func TestPropagationWithCustomPropagators(t *testing.T) { container := restful.NewContainer() container.Filter(restfultrace.OTelFilter("foobar", - restfultrace.WithTracer(tracer), + restfultrace.WithTracerProvider(provider), restfultrace.WithPropagators(props))) container.Add(ws) @@ -213,9 +215,7 @@ func TestPropagationWithCustomPropagators(t *testing.T) { } func TestMultiFilters(t *testing.T) { - tracer1 := mocktrace.NewTracer("tracer1") - tracer2 := mocktrace.NewTracer("tracer2") - tracer3 := mocktrace.NewTracer("tracer3") + provider, _ := mocktrace.NewProviderAndTracer(tracerName) wrappedFunc := func(tracerName string) restful.RouteFunction { return func(req *restful.Request, resp *restful.Response) { @@ -232,16 +232,16 @@ func TestMultiFilters(t *testing.T) { ws1 := &restful.WebService{} ws1.Path("/user") ws1.Route(ws1.GET("/{id}"). - Filter(restfultrace.OTelFilter("my-service", restfultrace.WithTracer(tracer1))). - To(wrappedFunc("tracer1"))) + Filter(restfultrace.OTelFilter("my-service", restfultrace.WithTracerProvider(provider))). + To(wrappedFunc(tracerName))) ws1.Route(ws1.GET("/{id}/books"). - Filter(restfultrace.OTelFilter("book-service", restfultrace.WithTracer(tracer2))). - To(wrappedFunc("tracer2"))) + Filter(restfultrace.OTelFilter("book-service", restfultrace.WithTracerProvider(provider))). + To(wrappedFunc(tracerName))) ws2 := &restful.WebService{} ws2.Path("/library") - ws2.Filter(restfultrace.OTelFilter("library-service", restfultrace.WithTracer(tracer3))) - ws2.Route(ws2.GET("/{name}").To(wrappedFunc("tracer3"))) + ws2.Filter(restfultrace.OTelFilter("library-service", restfultrace.WithTracerProvider(provider))) + ws2.Route(ws2.GET("/{name}").To(wrappedFunc(tracerName))) container := restful.NewContainer() container.Add(ws1) From c20a7b05285f2b3e13001120ac828c496729af7d Mon Sep 17 00:00:00 2001 From: Sam Xie Date: Wed, 26 Aug 2020 14:17:09 +0800 Subject: [PATCH 06/17] Unify instrumentation options for gomemcache --- instrumentation/github.com/bradfitz/gomemcache/config.go | 5 +++-- .../github.com/bradfitz/gomemcache/example/client.go | 2 +- .../github.com/bradfitz/gomemcache/gomemcache_test.go | 2 +- 3 files changed, 5 insertions(+), 4 deletions(-) diff --git a/instrumentation/github.com/bradfitz/gomemcache/config.go b/instrumentation/github.com/bradfitz/gomemcache/config.go index ed8529f8e8c..3b4e177a4c7 100644 --- a/instrumentation/github.com/bradfitz/gomemcache/config.go +++ b/instrumentation/github.com/bradfitz/gomemcache/config.go @@ -26,8 +26,9 @@ type config struct { // Option is used to configure the client. type Option func(*config) -// WithTracer configures the client with the provided trace provider. -func WithTraceProvider(traceProvider oteltrace.Provider) Option { +// WithTracerProvider specifies a tracer provider to use for creating a tracer. +// If none is specified, the global provider is used. +func WithTracerProvider(traceProvider oteltrace.Provider) Option { return func(cfg *config) { cfg.traceProvider = traceProvider } diff --git a/instrumentation/github.com/bradfitz/gomemcache/example/client.go b/instrumentation/github.com/bradfitz/gomemcache/example/client.go index cb99dc196b5..247521ef0ab 100644 --- a/instrumentation/github.com/bradfitz/gomemcache/example/client.go +++ b/instrumentation/github.com/bradfitz/gomemcache/example/client.go @@ -39,7 +39,7 @@ func main() { memcache.New( host+":"+port, ), - gomemcache.WithTraceProvider(tp), + gomemcache.WithTracerProvider(tp), ) ctx, s := tp.Tracer("example-tracer").Start(ctx, "test-operations") diff --git a/instrumentation/github.com/bradfitz/gomemcache/gomemcache_test.go b/instrumentation/github.com/bradfitz/gomemcache/gomemcache_test.go index 7f52b3bfd73..a90d76cf206 100644 --- a/instrumentation/github.com/bradfitz/gomemcache/gomemcache_test.go +++ b/instrumentation/github.com/bradfitz/gomemcache/gomemcache_test.go @@ -110,7 +110,7 @@ func initClientWithMockTraceProvider(t *testing.T) (*Client, *mocktracer.Provide c := NewClientWithTracing( mc, - WithTraceProvider(mt), + WithTracerProvider(mt), ) return c, mt From d3c9346949569c1585d1187d0d393fc4d3d82bff Mon Sep 17 00:00:00 2001 From: Sam Xie Date: Wed, 26 Aug 2020 14:17:47 +0800 Subject: [PATCH 07/17] Unify instrumentation options for sarama --- .../github.com/Shopify/sarama/consumer_test.go | 6 +++--- instrumentation/github.com/Shopify/sarama/option.go | 4 ++-- .../github.com/Shopify/sarama/option_test.go | 2 +- .../github.com/Shopify/sarama/producer_test.go | 12 ++++++------ 4 files changed, 12 insertions(+), 12 deletions(-) diff --git a/instrumentation/github.com/Shopify/sarama/consumer_test.go b/instrumentation/github.com/Shopify/sarama/consumer_test.go index 81eec4bda9f..c3697dea44b 100644 --- a/instrumentation/github.com/Shopify/sarama/consumer_test.go +++ b/instrumentation/github.com/Shopify/sarama/consumer_test.go @@ -53,7 +53,7 @@ func TestWrapPartitionConsumer(t *testing.T) { partitionConsumer, err := consumer.ConsumePartition(topic, 0, 0) require.NoError(t, err) - partitionConsumer = WrapPartitionConsumer(partitionConsumer, WithTraceProvider(provider)) + partitionConsumer = WrapPartitionConsumer(partitionConsumer, WithTracerProvider(provider)) consumeAndCheck(t, mt, mockPartitionConsumer, partitionConsumer) } @@ -67,7 +67,7 @@ func TestWrapConsumer(t *testing.T) { mockPartitionConsumer := mockConsumer.ExpectConsumePartition(topic, 0, 0) // Wrap consumer - consumer := WrapConsumer(mockConsumer, WithTraceProvider(provider)) + consumer := WrapConsumer(mockConsumer, WithTracerProvider(provider)) // Create partition consumer partitionConsumer, err := consumer.ConsumePartition(topic, 0, 0) @@ -170,7 +170,7 @@ func BenchmarkWrapPartitionConsumer(b *testing.B) { mockPartitionConsumer, partitionConsumer := createMockPartitionConsumer(b) - partitionConsumer = WrapPartitionConsumer(partitionConsumer, WithTraceProvider(provider)) + partitionConsumer = WrapPartitionConsumer(partitionConsumer, WithTracerProvider(provider)) message := sarama.ConsumerMessage{Key: []byte("foo")} b.ReportAllocs() diff --git a/instrumentation/github.com/Shopify/sarama/option.go b/instrumentation/github.com/Shopify/sarama/option.go index cdca4010342..a59b6e70720 100644 --- a/instrumentation/github.com/Shopify/sarama/option.go +++ b/instrumentation/github.com/Shopify/sarama/option.go @@ -52,9 +52,9 @@ func newConfig(opts ...Option) config { // Option specifies instrumentation configuration options. type Option func(*config) -// WithTraceProvider specifies a trace provider to use for creating a tracer for spans. +// WithTracerProvider specifies a tracer provider to use for creating a tracer. // If none is specified, the global provider is used. -func WithTraceProvider(provider trace.Provider) Option { +func WithTracerProvider(provider trace.Provider) Option { return func(cfg *config) { cfg.TraceProvider = provider } diff --git a/instrumentation/github.com/Shopify/sarama/option_test.go b/instrumentation/github.com/Shopify/sarama/option_test.go index d66532abf30..244c8b0bbb6 100644 --- a/instrumentation/github.com/Shopify/sarama/option_test.go +++ b/instrumentation/github.com/Shopify/sarama/option_test.go @@ -31,7 +31,7 @@ func TestNewConfig(t *testing.T) { { name: "with provider", opts: []Option{ - WithTraceProvider(global.TraceProvider()), + WithTracerProvider(global.TraceProvider()), }, expected: config{ TraceProvider: global.TraceProvider(), diff --git a/instrumentation/github.com/Shopify/sarama/producer_test.go b/instrumentation/github.com/Shopify/sarama/producer_test.go index 99270f64bbd..bc8733af71f 100644 --- a/instrumentation/github.com/Shopify/sarama/producer_test.go +++ b/instrumentation/github.com/Shopify/sarama/producer_test.go @@ -52,7 +52,7 @@ func TestWrapSyncProducer(t *testing.T) { mockSyncProducer := mocks.NewSyncProducer(t, cfg) // Wrap sync producer - syncProducer := WrapSyncProducer(cfg, mockSyncProducer, WithTraceProvider(provider)) + syncProducer := WrapSyncProducer(cfg, mockSyncProducer, WithTracerProvider(provider)) // Create message with span context ctx, _ := mt.Start(context.Background(), "") @@ -167,7 +167,7 @@ func TestWrapAsyncProducer(t *testing.T) { cfg := newSaramaConfig() mockAsyncProducer := mocks.NewAsyncProducer(t, cfg) - ap := WrapAsyncProducer(cfg, mockAsyncProducer, WithTraceProvider(provider)) + ap := WrapAsyncProducer(cfg, mockAsyncProducer, WithTracerProvider(provider)) msgList := createMessages(mt) // Send message @@ -237,7 +237,7 @@ func TestWrapAsyncProducer(t *testing.T) { cfg.Producer.Return.Successes = true mockAsyncProducer := mocks.NewAsyncProducer(t, cfg) - ap := WrapAsyncProducer(cfg, mockAsyncProducer, WithTraceProvider(provider)) + ap := WrapAsyncProducer(cfg, mockAsyncProducer, WithTracerProvider(provider)) msgList := createMessages(mt) // Send message @@ -315,7 +315,7 @@ func TestWrapAsyncProducerError(t *testing.T) { cfg.Producer.Return.Successes = true mockAsyncProducer := mocks.NewAsyncProducer(t, cfg) - ap := WrapAsyncProducer(cfg, mockAsyncProducer, WithTraceProvider(provider)) + ap := WrapAsyncProducer(cfg, mockAsyncProducer, WithTracerProvider(provider)) mockAsyncProducer.ExpectInputAndFail(errors.New("test")) ap.Input() <- &sarama.ProducerMessage{Topic: topic, Key: sarama.StringEncoder("foo2")} @@ -349,7 +349,7 @@ func BenchmarkWrapSyncProducer(b *testing.B) { mockSyncProducer := mocks.NewSyncProducer(b, cfg) // Wrap sync producer - syncProducer := WrapSyncProducer(cfg, mockSyncProducer, WithTraceProvider(provider)) + syncProducer := WrapSyncProducer(cfg, mockSyncProducer, WithTracerProvider(provider)) message := sarama.ProducerMessage{Key: sarama.StringEncoder("foo")} b.ReportAllocs() @@ -390,7 +390,7 @@ func BenchmarkWrapAsyncProducer(b *testing.B) { mockAsyncProducer := mocks.NewAsyncProducer(b, cfg) // Wrap sync producer - asyncProducer := WrapAsyncProducer(cfg, mockAsyncProducer, WithTraceProvider(provider)) + asyncProducer := WrapAsyncProducer(cfg, mockAsyncProducer, WithTracerProvider(provider)) message := sarama.ProducerMessage{Key: sarama.StringEncoder("foo")} b.ReportAllocs() From 31d07564d8a8c29ec48750994fc21c08e914dee0 Mon Sep 17 00:00:00 2001 From: Sam Xie Date: Wed, 26 Aug 2020 14:19:25 +0800 Subject: [PATCH 08/17] Unify instrumentation options for net/http --- instrumentation/net/http/config.go | 38 +++++++++++++++------- instrumentation/net/http/config_test.go | 23 +++++++------ instrumentation/net/http/handler.go | 5 --- instrumentation/net/http/handler_test.go | 16 ++++----- instrumentation/net/http/transport.go | 3 -- instrumentation/net/http/transport_test.go | 4 +-- 6 files changed, 48 insertions(+), 41 deletions(-) diff --git a/instrumentation/net/http/config.go b/instrumentation/net/http/config.go index 15fce125d5b..d6d6f9cb337 100644 --- a/instrumentation/net/http/config.go +++ b/instrumentation/net/http/config.go @@ -17,11 +17,16 @@ package http import ( "net/http" + "go.opentelemetry.io/otel/api/global" "go.opentelemetry.io/otel/api/metric" "go.opentelemetry.io/otel/api/propagation" "go.opentelemetry.io/otel/api/trace" ) +const ( + instrumentationName = "go.opentelemetry.io/contrib/instrumentation/net/http" +) + // Config represents the configuration options available for the http.Handler // and http.Transport types. type Config struct { @@ -33,6 +38,9 @@ type Config struct { WriteEvent bool Filters []Filter SpanNameFormatter func(string, *http.Request) string + + TraceProvider trace.Provider + MeterProvider metric.Provider } // Option Interface used for setting *optional* Config properties @@ -50,26 +58,34 @@ func (o OptionFunc) Apply(c *Config) { // NewConfig creates a new Config struct and applies opts to it. func NewConfig(opts ...Option) *Config { - c := &Config{} + c := &Config{ + Propagators: global.Propagators(), + TraceProvider: global.TraceProvider(), + MeterProvider: global.MeterProvider(), + } for _, opt := range opts { opt.Apply(c) } + + c.Tracer = c.TraceProvider.Tracer(instrumentationName) + c.Meter = c.MeterProvider.Meter(instrumentationName) + return c } -// WithTracer configures a specific tracer. If this option -// isn't specified then the global tracer is used. -func WithTracer(tracer trace.Tracer) Option { - return OptionFunc(func(c *Config) { - c.Tracer = tracer +// WithTracerProvider specifies a tracer provider to use for creating a tracer. +// If none is specified, the global provider is used. +func WithTracerProvider(provider trace.Provider) Option { + return OptionFunc(func(cfg *Config) { + cfg.TraceProvider = provider }) } -// WithMeter configures a specific meter. If this option -// isn't specified then the global meter is used. -func WithMeter(meter metric.Meter) Option { - return OptionFunc(func(c *Config) { - c.Meter = meter +// WithMeterProvider specifies a meter provider to use for creating a meter. +// If none is specified, the global provider is used. +func WithMeterProvider(provider metric.Provider) Option { + return OptionFunc(func(cfg *Config) { + cfg.MeterProvider = provider }) } diff --git a/instrumentation/net/http/config_test.go b/instrumentation/net/http/config_test.go index 49297b3a72a..a0836b119c3 100644 --- a/instrumentation/net/http/config_test.go +++ b/instrumentation/net/http/config_test.go @@ -21,13 +21,15 @@ import ( "net/http/httptest" "testing" + "github.com/stretchr/testify/assert" + mocktrace "go.opentelemetry.io/contrib/internal/trace" ) func TestBasicFilter(t *testing.T) { rr := httptest.NewRecorder() - tracer := mocktrace.Tracer{} + provider, tracer := mocktrace.NewProviderAndTracer(instrumentationName) h := NewHandler( http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { @@ -35,7 +37,7 @@ func TestBasicFilter(t *testing.T) { t.Fatal(err) } }), "test_handler", - WithTracer(&tracer), + WithTracerProvider(provider), WithFilter(func(r *http.Request) bool { return false }), @@ -95,12 +97,8 @@ func TestSpanNameFormatter(t *testing.T) { for _, tc := range testCases { t.Run(tc.name, func(t *testing.T) { rr := httptest.NewRecorder() - var spanName string - tracer := mocktrace.Tracer{ - OnSpanStarted: func(span *mocktrace.Span) { - spanName = span.Name - }, - } + + provider, tracer := mocktrace.NewProviderAndTracer(instrumentationName) handler := http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { if _, err := io.WriteString(w, "hello world"); err != nil { t.Fatal(err) @@ -109,7 +107,7 @@ func TestSpanNameFormatter(t *testing.T) { h := NewHandler( handler, tc.operation, - WithTracer(&tracer), + WithTracerProvider(provider), WithSpanNameFormatter(tc.formatter), ) r, err := http.NewRequest(http.MethodGet, "http://localhost/hello", nil) @@ -120,9 +118,10 @@ func TestSpanNameFormatter(t *testing.T) { if got, expected := rr.Result().StatusCode, http.StatusOK; got != expected { t.Fatalf("got %d, expected %d", got, expected) } - if got, expected := spanName, tc.expected; got != expected { - t.Fatalf("got %q, expected %q", got, expected) - } + + spans := tracer.EndedSpans() + assert.Len(t, spans, 1) + assert.Equal(t, tc.expected, spans[0].Name) }) } } diff --git a/instrumentation/net/http/handler.go b/instrumentation/net/http/handler.go index 29bf79e7604..72fddbd891d 100644 --- a/instrumentation/net/http/handler.go +++ b/instrumentation/net/http/handler.go @@ -63,12 +63,7 @@ func NewHandler(handler http.Handler, operation string, opts ...Option) http.Han operation: operation, } - const domain = "go.opentelemetry.io/contrib/instrumentation/net/http" - defaultOpts := []Option{ - WithTracer(global.Tracer(domain)), - WithMeter(global.Meter(domain)), - WithPropagators(global.Propagators()), WithSpanOptions(trace.WithSpanKind(trace.SpanKindServer)), WithSpanNameFormatter(defaultHandlerFormatter), } diff --git a/instrumentation/net/http/handler_test.go b/instrumentation/net/http/handler_test.go index 3d15ce6d866..3f4bee07eb8 100644 --- a/instrumentation/net/http/handler_test.go +++ b/instrumentation/net/http/handler_test.go @@ -43,8 +43,8 @@ func assertMetricLabels(t *testing.T, expectedLabels []label.KeyValue, measureme func TestHandlerBasics(t *testing.T) { rr := httptest.NewRecorder() - tracer := mocktrace.Tracer{} - meterimpl, meter := mockmeter.NewMeter() + tracerProvider, tracer := mocktrace.NewProviderAndTracer(instrumentationName) + meterimpl, meterProvider := mockmeter.NewProvider() operation := "test_handler" @@ -54,8 +54,8 @@ func TestHandlerBasics(t *testing.T) { t.Fatal(err) } }), operation, - WithTracer(&tracer), - WithMeter(meter), + WithTracerProvider(tracerProvider), + WithMeterProvider(meterProvider), ) r, err := http.NewRequest(http.MethodGet, "http://localhost/", strings.NewReader("foo")) @@ -98,7 +98,7 @@ func TestHandlerBasics(t *testing.T) { func TestHandlerNoWrite(t *testing.T) { rr := httptest.NewRecorder() - tracer := mocktrace.Tracer{} + tracerProvider, tracer := mocktrace.NewProviderAndTracer(instrumentationName) operation := "test_handler" var span trace.Span @@ -107,7 +107,7 @@ func TestHandlerNoWrite(t *testing.T) { http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { span = trace.SpanFromContext(r.Context()) }), operation, - WithTracer(&tracer), + WithTracerProvider(tracerProvider), ) r, err := http.NewRequest(http.MethodGet, "http://localhost/", nil) @@ -137,7 +137,7 @@ func TestHandlerNoWrite(t *testing.T) { func TestResponseWriterOptionalInterfaces(t *testing.T) { rr := httptest.NewRecorder() - tracer := mocktrace.Tracer{} + tracerProvider, _ := mocktrace.NewProviderAndTracer(instrumentationName) // ResponseRecorder implements the Flusher interface. Make sure the // wrapped ResponseWriter passed to the handler still implements @@ -151,7 +151,7 @@ func TestResponseWriterOptionalInterfaces(t *testing.T) { t.Fatal(err) } }), "test_handler", - WithTracer(&tracer)) + WithTracerProvider(tracerProvider)) r, err := http.NewRequest(http.MethodGet, "http://localhost/", nil) if err != nil { diff --git a/instrumentation/net/http/transport.go b/instrumentation/net/http/transport.go index f8048197bc0..c66e753a6d4 100644 --- a/instrumentation/net/http/transport.go +++ b/instrumentation/net/http/transport.go @@ -19,7 +19,6 @@ import ( "io" "net/http" - "go.opentelemetry.io/otel/api/global" "go.opentelemetry.io/otel/api/propagation" "go.opentelemetry.io/otel/api/trace" "go.opentelemetry.io/otel/semconv" @@ -49,8 +48,6 @@ func NewTransport(base http.RoundTripper, opts ...Option) *Transport { } defaultOpts := []Option{ - WithTracer(global.Tracer("go.opentelemetry.io/contrib/instrumentation/net/http")), - WithPropagators(global.Propagators()), WithSpanOptions(trace.WithSpanKind(trace.SpanKindClient)), WithSpanNameFormatter(defaultTransportFormatter), } diff --git a/instrumentation/net/http/transport_test.go b/instrumentation/net/http/transport_test.go index 5e2b8c18d18..7ec00b99ea4 100644 --- a/instrumentation/net/http/transport_test.go +++ b/instrumentation/net/http/transport_test.go @@ -30,7 +30,7 @@ import ( ) func TestTransportBasics(t *testing.T) { - tracer := mocktrace.Tracer{} + provider, tracer := mocktrace.NewProviderAndTracer(instrumentationName) content := []byte("Hello, world!") ts := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { @@ -56,7 +56,7 @@ func TestTransportBasics(t *testing.T) { tr := NewTransport( http.DefaultTransport, - WithTracer(&tracer), + WithTracerProvider(provider), ) c := http.Client{Transport: tr} From 6b661766df2c26b415e003ac00f5aeae094c3403 Mon Sep 17 00:00:00 2001 From: Sam Xie Date: Wed, 26 Aug 2020 14:22:05 +0800 Subject: [PATCH 09/17] Unify instrumentation options for beego --- .../github.com/astaxie/beego/beego.go | 4 ++-- .../github.com/astaxie/beego/beego_test.go | 8 +++---- .../github.com/astaxie/beego/config.go | 24 ++++++++----------- .../github.com/astaxie/beego/go.mod | 2 +- 4 files changed, 17 insertions(+), 21 deletions(-) diff --git a/instrumentation/github.com/astaxie/beego/beego.go b/instrumentation/github.com/astaxie/beego/beego.go index 2f748fccbd8..637662b1396 100644 --- a/instrumentation/github.com/astaxie/beego/beego.go +++ b/instrumentation/github.com/astaxie/beego/beego.go @@ -67,8 +67,8 @@ func NewOTelBeegoMiddleWare(service string, options ...Option) beego.MiddleWare cfg := configure(options...) httpOptions := []otelhttp.Option{ - otelhttp.WithTracer(cfg.traceProvider.Tracer(packageName)), - otelhttp.WithMeter(cfg.meterProvider.Meter(packageName)), + otelhttp.WithTracerProvider(cfg.traceProvider), + otelhttp.WithMeterProvider(cfg.meterProvider), otelhttp.WithPropagators(cfg.propagators), } diff --git a/instrumentation/github.com/astaxie/beego/beego_test.go b/instrumentation/github.com/astaxie/beego/beego_test.go index 2c3ca3f2c70..b11300bd956 100644 --- a/instrumentation/github.com/astaxie/beego/beego_test.go +++ b/instrumentation/github.com/astaxie/beego/beego_test.go @@ -238,7 +238,7 @@ func TestSpanFromContextCustomProvider(t *testing.T) { mw := NewOTelBeegoMiddleWare( middleWareName, - WithTraceProvider(NewTraceProvider()), + WithTracerProvider(NewTraceProvider()), WithMeterProvider(provider), ) @@ -261,7 +261,7 @@ func TestStatic(t *testing.T) { defer beego.SetStaticPath("/", "") mw := NewOTelBeegoMiddleWare(middleWareName, - WithTraceProvider(traceProvider), + WithTracerProvider(traceProvider), WithMeterProvider(meterProvider), ) @@ -312,7 +312,7 @@ func TestRender(t *testing.T) { mw := NewOTelBeegoMiddleWare( middleWareName, - WithTraceProvider(traceProvider), + WithTracerProvider(traceProvider), ) for _, str := range []string{"/render", "/renderstring", "/renderbytes"} { rr := httptest.NewRecorder() @@ -366,7 +366,7 @@ func runTest(t *testing.T, tc *testCase, url string) { middleWareName, append( tc.options, - WithTraceProvider(traceProvider), + WithTracerProvider(traceProvider), WithMeterProvider(meterProvider), )..., ) diff --git a/instrumentation/github.com/astaxie/beego/config.go b/instrumentation/github.com/astaxie/beego/config.go index 1c1792d0f62..ce0e9d25323 100644 --- a/instrumentation/github.com/astaxie/beego/config.go +++ b/instrumentation/github.com/astaxie/beego/config.go @@ -47,23 +47,19 @@ func (o OptionFunc) Apply(c *Config) { // ------------------------------------------ Options -// WithTraceProvider sets the trace provider to be used by the middleware -// to create a tracer for the spans. -// Defaults to calling global.TraceProvider(). -// Tracer name is set to "go.opentelemetry.io/contrib/instrumentation/github.com/astaxie/beego". -func WithTraceProvider(provider trace.Provider) OptionFunc { - return OptionFunc(func(c *Config) { - c.traceProvider = provider +// WithTracerProvider specifies a tracer provider to use for creating a tracer. +// If none is specified, the global provider is used. +func WithTracerProvider(traceProvider trace.Provider) Option { + return OptionFunc(func(cfg *Config) { + cfg.traceProvider = traceProvider }) } -// WithMeterProvider sets the meter provider to be used to create a meter -// by the middleware. -// Defaults to calling global.MeterProvider(). -// Meter name is set to "go.opentelemetry.io/contrib/instrumentation/github.com/astaxie/beego". -func WithMeterProvider(provider metric.Provider) OptionFunc { - return OptionFunc(func(c *Config) { - c.meterProvider = provider +// WithMeterProvider specifies a meter provider to use for creating a meter. +// If none is specified, the global provider is used. +func WithMeterProvider(provider metric.Provider) Option { + return OptionFunc(func(cfg *Config) { + cfg.meterProvider = provider }) } diff --git a/instrumentation/github.com/astaxie/beego/go.mod b/instrumentation/github.com/astaxie/beego/go.mod index 9a3fef9f8ba..def5ddda37f 100644 --- a/instrumentation/github.com/astaxie/beego/go.mod +++ b/instrumentation/github.com/astaxie/beego/go.mod @@ -6,7 +6,7 @@ require ( github.com/astaxie/beego v1.12.2 github.com/stretchr/testify v1.6.1 go.opentelemetry.io/contrib v0.11.0 - go.opentelemetry.io/contrib/instrumentation/net/http v0.11.0 + go.opentelemetry.io/contrib/instrumentation/net/http v0.10.1 go.opentelemetry.io/otel v0.11.0 golang.org/x/net v0.0.0-20200707034311-ab3426394381 // indirect golang.org/x/sys v0.0.0-20200803210538-64077c9b5642 // indirect From 7f05147d516c1771acfc58e7b09700fe1cc9383d Mon Sep 17 00:00:00 2001 From: Sam Xie Date: Wed, 26 Aug 2020 14:36:01 +0800 Subject: [PATCH 10/17] Update instrumentation guidelines about uniform *Provider options --- instrumentation/README.md | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/instrumentation/README.md b/instrumentation/README.md index e5348a40ab3..b77ce5455c8 100644 --- a/instrumentation/README.md +++ b/instrumentation/README.md @@ -35,6 +35,10 @@ Additionally the following guidelines for package composition need to be followe This documentation SHOULD be in a dedicated `doc.go` file if the package is more than one file. It SHOULD contain useful information like what the purpose of the instrumentation is, how to use it, and any compatibility restrictions that might exist. - Examples of how to actually use the instrumentation SHOULD be included. +- All instrumentation packages MUST provide an option to accept a `TracerProvider` if it uses a Tracer, a `MeterProvider` if it uses a Meter, and `Propagators` if it handles any context propagation. + Also, packages MUST use the default `TracerProvider`, `MeterProvider`, and `Propagators` supplied by the `global` package if no optional one is provided. +- All instrumentation packages MUST not provide an option to accept a `Tracer` or `Meter`. +- All instrumentation packages MUST create any used `Tracer` or `Meter` with a name matching the instrumentation package name. ## Additional Instrumentation Packages From 123971d8c7b3894dd3566ee8913fbcc5d79e04c2 Mon Sep 17 00:00:00 2001 From: Sam Xie Date: Wed, 26 Aug 2020 15:08:10 +0800 Subject: [PATCH 11/17] update CHANGELOG --- CHANGELOG.md | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 79187be307f..2b4e69f3ccc 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,6 +8,12 @@ This project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.htm ## [Unreleased] +### Changed + +- Unify instrumentation about provider options for `go.mongodb.org/mongo-driver`, `gin-gonic/gin`, `gorilla/mux`, + `labstack/echo`, `emicklei/go-restful`, `bradfitz/gomemcache`, `Shopify/sarama`, `net/http` and `beego`. (#303) +- Update instrumentation guidelines about uniform provider options. (#303) + ## [0.11.0] - 2020-08-25 ### Added From 19379165ae8a26499c2c42256ef1d01a07048863 Mon Sep 17 00:00:00 2001 From: Sam Xie Date: Thu, 27 Aug 2020 11:16:44 +0800 Subject: [PATCH 12/17] Update guidelines --- instrumentation/README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/instrumentation/README.md b/instrumentation/README.md index b77ce5455c8..b37c1f9fb2c 100644 --- a/instrumentation/README.md +++ b/instrumentation/README.md @@ -37,7 +37,7 @@ Additionally the following guidelines for package composition need to be followe - Examples of how to actually use the instrumentation SHOULD be included. - All instrumentation packages MUST provide an option to accept a `TracerProvider` if it uses a Tracer, a `MeterProvider` if it uses a Meter, and `Propagators` if it handles any context propagation. Also, packages MUST use the default `TracerProvider`, `MeterProvider`, and `Propagators` supplied by the `global` package if no optional one is provided. -- All instrumentation packages MUST not provide an option to accept a `Tracer` or `Meter`. +- All instrumentation packages MUST NOT provide an option to accept a `Tracer` or `Meter`. - All instrumentation packages MUST create any used `Tracer` or `Meter` with a name matching the instrumentation package name. ## Additional Instrumentation Packages From 94c136e123ae14bac9d49d820d27316a23ec14a6 Mon Sep 17 00:00:00 2001 From: Sam Xie Date: Thu, 27 Aug 2020 11:19:18 +0800 Subject: [PATCH 13/17] Fix naming and remove unnecessary fields --- .../github.com/Shopify/sarama/option.go | 12 ++++---- .../github.com/Shopify/sarama/option_test.go | 12 ++++---- .../github.com/astaxie/beego/beego.go | 2 +- .../github.com/astaxie/beego/beego_test.go | 30 +++++++++---------- .../github.com/astaxie/beego/config.go | 24 +++++++-------- .../github.com/astaxie/beego/go.mod | 2 +- .../github.com/bradfitz/gomemcache/config.go | 8 ++--- .../bradfitz/gomemcache/gomemcache.go | 6 ++-- .../bradfitz/gomemcache/gomemcache_test.go | 8 ++--- .../github.com/emicklei/go-restful/config.go | 7 ++--- .../github.com/emicklei/go-restful/restful.go | 8 ++--- .../github.com/gin-gonic/gin/gintrace.go | 10 +++---- .../github.com/gin-gonic/gin/option.go | 7 ++--- .../github.com/gorilla/mux/config.go | 7 ++--- instrumentation/github.com/gorilla/mux/mux.go | 8 ++--- .../github.com/labstack/echo/config.go | 7 ++--- .../github.com/labstack/echo/echo.go | 10 +++---- .../go.mongodb.org/mongo-driver/config.go | 8 ++--- instrumentation/net/http/config.go | 14 ++++----- 19 files changed, 93 insertions(+), 97 deletions(-) diff --git a/instrumentation/github.com/Shopify/sarama/option.go b/instrumentation/github.com/Shopify/sarama/option.go index a59b6e70720..6805e15bfe2 100644 --- a/instrumentation/github.com/Shopify/sarama/option.go +++ b/instrumentation/github.com/Shopify/sarama/option.go @@ -28,8 +28,8 @@ const ( ) type config struct { - TraceProvider trace.Provider - Propagators otelpropagation.Propagators + TracerProvider trace.Provider + Propagators otelpropagation.Propagators Tracer trace.Tracer } @@ -37,14 +37,14 @@ type config struct { // newConfig returns a config with all Options set. func newConfig(opts ...Option) config { cfg := config{ - Propagators: global.Propagators(), - TraceProvider: global.TraceProvider(), + Propagators: global.Propagators(), + TracerProvider: global.TraceProvider(), } for _, opt := range opts { opt(&cfg) } - cfg.Tracer = cfg.TraceProvider.Tracer(defaultTracerName) + cfg.Tracer = cfg.TracerProvider.Tracer(defaultTracerName) return cfg } @@ -56,7 +56,7 @@ type Option func(*config) // If none is specified, the global provider is used. func WithTracerProvider(provider trace.Provider) Option { return func(cfg *config) { - cfg.TraceProvider = provider + cfg.TracerProvider = provider } } diff --git a/instrumentation/github.com/Shopify/sarama/option_test.go b/instrumentation/github.com/Shopify/sarama/option_test.go index 244c8b0bbb6..2a3870a8fb4 100644 --- a/instrumentation/github.com/Shopify/sarama/option_test.go +++ b/instrumentation/github.com/Shopify/sarama/option_test.go @@ -34,9 +34,9 @@ func TestNewConfig(t *testing.T) { WithTracerProvider(global.TraceProvider()), }, expected: config{ - TraceProvider: global.TraceProvider(), - Tracer: global.TraceProvider().Tracer(defaultTracerName), - Propagators: global.Propagators(), + TracerProvider: global.TraceProvider(), + Tracer: global.TraceProvider().Tracer(defaultTracerName), + Propagators: global.Propagators(), }, }, { @@ -45,9 +45,9 @@ func TestNewConfig(t *testing.T) { WithPropagators(nil), }, expected: config{ - TraceProvider: global.TraceProvider(), - Tracer: global.TraceProvider().Tracer(defaultTracerName), - Propagators: nil, + TracerProvider: global.TraceProvider(), + Tracer: global.TraceProvider().Tracer(defaultTracerName), + Propagators: nil, }, }, } diff --git a/instrumentation/github.com/astaxie/beego/beego.go b/instrumentation/github.com/astaxie/beego/beego.go index 637662b1396..5768365843d 100644 --- a/instrumentation/github.com/astaxie/beego/beego.go +++ b/instrumentation/github.com/astaxie/beego/beego.go @@ -67,7 +67,7 @@ func NewOTelBeegoMiddleWare(service string, options ...Option) beego.MiddleWare cfg := configure(options...) httpOptions := []otelhttp.Option{ - otelhttp.WithTracerProvider(cfg.traceProvider), + otelhttp.WithTracerProvider(cfg.tracerProvider), otelhttp.WithMeterProvider(cfg.meterProvider), otelhttp.WithPropagators(cfg.propagators), } diff --git a/instrumentation/github.com/astaxie/beego/beego_test.go b/instrumentation/github.com/astaxie/beego/beego_test.go index b11300bd956..880724dbde9 100644 --- a/instrumentation/github.com/astaxie/beego/beego_test.go +++ b/instrumentation/github.com/astaxie/beego/beego_test.go @@ -43,16 +43,16 @@ import ( // ------------------------------------------ Mock Trace Provider -type MockTraceProvider struct { +type MockTracerProvider struct { tracer *mocktrace.Tracer } -func (m *MockTraceProvider) Tracer(name string, options ...trace.TracerOption) trace.Tracer { +func (m *MockTracerProvider) Tracer(name string, options ...trace.TracerOption) trace.Tracer { return m.tracer } -func NewTraceProvider() *MockTraceProvider { - return &MockTraceProvider{ +func NewTracerProvider() *MockTracerProvider { + return &MockTracerProvider{ tracer: mocktrace.NewTracer(packageName), } } @@ -205,7 +205,7 @@ func TestSpanFromContextDefaultProvider(t *testing.T) { defer replaceBeego() _, provider := mockmeter.NewProvider() global.SetMeterProvider(provider) - global.SetTraceProvider(NewTraceProvider()) + global.SetTraceProvider(NewTracerProvider()) router := beego.NewControllerRegister() router.Get("/hello-with-span", func(ctx *beegoCtx.Context) { assertSpanFromContext(ctx.Request.Context(), t) @@ -238,7 +238,7 @@ func TestSpanFromContextCustomProvider(t *testing.T) { mw := NewOTelBeegoMiddleWare( middleWareName, - WithTracerProvider(NewTraceProvider()), + WithTracerProvider(NewTracerProvider()), WithMeterProvider(provider), ) @@ -249,7 +249,7 @@ func TestSpanFromContextCustomProvider(t *testing.T) { func TestStatic(t *testing.T) { defer replaceBeego() - traceProvider := NewTraceProvider() + tracerProvider := NewTracerProvider() meterimpl, meterProvider := mockmeter.NewProvider() file, err := ioutil.TempFile("", "static-*.html") require.NoError(t, err) @@ -261,7 +261,7 @@ func TestStatic(t *testing.T) { defer beego.SetStaticPath("/", "") mw := NewOTelBeegoMiddleWare(middleWareName, - WithTracerProvider(traceProvider), + WithTracerProvider(tracerProvider), WithMeterProvider(meterProvider), ) @@ -278,7 +278,7 @@ func TestStatic(t *testing.T) { body, err := ioutil.ReadAll(rr.Result().Body) require.NoError(t, err) require.Equal(t, "

Hello, world!

", string(body)) - spans := traceProvider.tracer.EndedSpans() + spans := tracerProvider.tracer.EndedSpans() require.Len(t, spans, 1) assertSpan(t, spans[0], tc) assertMetrics(t, meterimpl.MeasurementBatches, tc) @@ -308,11 +308,11 @@ func TestRender(t *testing.T) { beego.SetViewsPath(dir) _, tplName = filepath.Split(file.Name()) - traceProvider := NewTraceProvider() + tracerProvider := NewTracerProvider() mw := NewOTelBeegoMiddleWare( middleWareName, - WithTracerProvider(traceProvider), + WithTracerProvider(tracerProvider), ) for _, str := range []string{"/render", "/renderstring", "/renderbytes"} { rr := httptest.NewRecorder() @@ -324,7 +324,7 @@ func TestRender(t *testing.T) { require.NoError(t, err) } - spans := traceProvider.tracer.EndedSpans() + spans := tracerProvider.tracer.EndedSpans() require.Len(t, spans, 6) // 3 HTTP requests, each creating 2 spans for _, span := range spans { switch span.Name { @@ -347,7 +347,7 @@ func TestRender(t *testing.T) { // ------------------------------------------ Utilities func runTest(t *testing.T, tc *testCase, url string) { - traceProvider := NewTraceProvider() + tracerProvider := NewTracerProvider() meterimpl, meterProvider := mockmeter.NewProvider() addTestRoutes(t) defer replaceBeego() @@ -366,7 +366,7 @@ func runTest(t *testing.T, tc *testCase, url string) { middleWareName, append( tc.options, - WithTracerProvider(traceProvider), + WithTracerProvider(tracerProvider), WithMeterProvider(meterProvider), )..., ) @@ -380,7 +380,7 @@ func runTest(t *testing.T, tc *testCase, url string) { require.NoError(t, json.Unmarshal(body, &message)) require.Equal(t, tc.expectedResponse, message) - spans := traceProvider.tracer.EndedSpans() + spans := tracerProvider.tracer.EndedSpans() if tc.hasSpan { require.Len(t, spans, 1) assertSpan(t, spans[0], tc) diff --git a/instrumentation/github.com/astaxie/beego/config.go b/instrumentation/github.com/astaxie/beego/config.go index ce0e9d25323..f4e0870eb51 100644 --- a/instrumentation/github.com/astaxie/beego/config.go +++ b/instrumentation/github.com/astaxie/beego/config.go @@ -24,11 +24,11 @@ import ( // Config provides configuration for the beego OpenTelemetry // middleware. Configuration is modified using the provided Options. type Config struct { - traceProvider trace.Provider - meterProvider metric.Provider - propagators propagation.Propagators - filters []Filter - formatter SpanNameFormatter + tracerProvider trace.Provider + meterProvider metric.Provider + propagators propagation.Propagators + filters []Filter + formatter SpanNameFormatter } // Option applies a configuration to the given Config. @@ -49,9 +49,9 @@ func (o OptionFunc) Apply(c *Config) { // WithTracerProvider specifies a tracer provider to use for creating a tracer. // If none is specified, the global provider is used. -func WithTracerProvider(traceProvider trace.Provider) Option { +func WithTracerProvider(provider trace.Provider) Option { return OptionFunc(func(cfg *Config) { - cfg.traceProvider = traceProvider + cfg.tracerProvider = provider }) } @@ -91,11 +91,11 @@ func WithSpanNameFormatter(f SpanNameFormatter) OptionFunc { func configure(options ...Option) *Config { config := &Config{ - traceProvider: global.TraceProvider(), - meterProvider: global.MeterProvider(), - propagators: global.Propagators(), - filters: []Filter{}, - formatter: defaultSpanNameFormatter, + tracerProvider: global.TraceProvider(), + meterProvider: global.MeterProvider(), + propagators: global.Propagators(), + filters: []Filter{}, + formatter: defaultSpanNameFormatter, } for _, option := range options { option.Apply(config) diff --git a/instrumentation/github.com/astaxie/beego/go.mod b/instrumentation/github.com/astaxie/beego/go.mod index def5ddda37f..9a3fef9f8ba 100644 --- a/instrumentation/github.com/astaxie/beego/go.mod +++ b/instrumentation/github.com/astaxie/beego/go.mod @@ -6,7 +6,7 @@ require ( github.com/astaxie/beego v1.12.2 github.com/stretchr/testify v1.6.1 go.opentelemetry.io/contrib v0.11.0 - go.opentelemetry.io/contrib/instrumentation/net/http v0.10.1 + go.opentelemetry.io/contrib/instrumentation/net/http v0.11.0 go.opentelemetry.io/otel v0.11.0 golang.org/x/net v0.0.0-20200707034311-ab3426394381 // indirect golang.org/x/sys v0.0.0-20200803210538-64077c9b5642 // indirect diff --git a/instrumentation/github.com/bradfitz/gomemcache/config.go b/instrumentation/github.com/bradfitz/gomemcache/config.go index 3b4e177a4c7..fd1af46db10 100644 --- a/instrumentation/github.com/bradfitz/gomemcache/config.go +++ b/instrumentation/github.com/bradfitz/gomemcache/config.go @@ -19,8 +19,8 @@ import ( ) type config struct { - serviceName string - traceProvider oteltrace.Provider + serviceName string + tracerProvider oteltrace.Provider } // Option is used to configure the client. @@ -28,9 +28,9 @@ type Option func(*config) // WithTracerProvider specifies a tracer provider to use for creating a tracer. // If none is specified, the global provider is used. -func WithTracerProvider(traceProvider oteltrace.Provider) Option { +func WithTracerProvider(provider oteltrace.Provider) Option { return func(cfg *config) { - cfg.traceProvider = traceProvider + cfg.tracerProvider = provider } } diff --git a/instrumentation/github.com/bradfitz/gomemcache/gomemcache.go b/instrumentation/github.com/bradfitz/gomemcache/gomemcache.go index 411250e2a6f..978267b2b3a 100644 --- a/instrumentation/github.com/bradfitz/gomemcache/gomemcache.go +++ b/instrumentation/github.com/bradfitz/gomemcache/gomemcache.go @@ -58,14 +58,14 @@ func NewClientWithTracing(client *memcache.Client, opts ...Option) *Client { cfg.serviceName = defaultServiceName } - if cfg.traceProvider == nil { - cfg.traceProvider = otelglobal.TraceProvider() + if cfg.tracerProvider == nil { + cfg.tracerProvider = otelglobal.TraceProvider() } return &Client{ client, cfg, - cfg.traceProvider.Tracer( + cfg.tracerProvider.Tracer( defaultTracerName, oteltrace.WithInstrumentationVersion(contrib.SemVersion()), ), diff --git a/instrumentation/github.com/bradfitz/gomemcache/gomemcache_test.go b/instrumentation/github.com/bradfitz/gomemcache/gomemcache_test.go index a90d76cf206..bf0389be6ec 100644 --- a/instrumentation/github.com/bradfitz/gomemcache/gomemcache_test.go +++ b/instrumentation/github.com/bradfitz/gomemcache/gomemcache_test.go @@ -43,13 +43,13 @@ func TestNewClientWithTracing(t *testing.T) { assert.NotNil(t, c.Client) assert.NotNil(t, c.cfg) - assert.NotNil(t, c.cfg.traceProvider) + assert.NotNil(t, c.cfg.tracerProvider) assert.NotNil(t, c.tracer) assert.Equal(t, defaultServiceName, c.cfg.serviceName) } func TestOperation(t *testing.T) { - c, mtp := initClientWithMockTraceProvider(t) + c, mtp := initClientWithMockTracerProvider(t) mi := &memcache.Item{ Key: "foo", @@ -76,7 +76,7 @@ func TestOperation(t *testing.T) { func TestOperationWithCacheMissError(t *testing.T) { key := "foo" - c, mtp := initClientWithMockTraceProvider(t) + c, mtp := initClientWithMockTracerProvider(t) _, err := c.Get(key) assert.Error(t, err) @@ -101,7 +101,7 @@ func TestOperationWithCacheMissError(t *testing.T) { } // tests require running memcached instance -func initClientWithMockTraceProvider(t *testing.T) (*Client, *mocktracer.Provider) { +func initClientWithMockTracerProvider(t *testing.T) (*Client, *mocktracer.Provider) { mt := &mocktracer.Provider{} host, port := "localhost", "11211" diff --git a/instrumentation/github.com/emicklei/go-restful/config.go b/instrumentation/github.com/emicklei/go-restful/config.go index d561a056da8..d7196c05270 100644 --- a/instrumentation/github.com/emicklei/go-restful/config.go +++ b/instrumentation/github.com/emicklei/go-restful/config.go @@ -21,9 +21,8 @@ import ( // Config is used to configure the go-restful middleware. type Config struct { - TraceProvider oteltrace.Provider - Tracer oteltrace.Tracer - Propagators otelpropagation.Propagators + TracerProvider oteltrace.Provider + Propagators otelpropagation.Propagators } // Option specifies instrumentation configuration options. @@ -42,6 +41,6 @@ func WithPropagators(propagators otelpropagation.Propagators) Option { // If none is specified, the global provider is used. func WithTracerProvider(provider oteltrace.Provider) Option { return func(cfg *Config) { - cfg.TraceProvider = provider + cfg.TracerProvider = provider } } diff --git a/instrumentation/github.com/emicklei/go-restful/restful.go b/instrumentation/github.com/emicklei/go-restful/restful.go index 5d9b7a9cff1..30c8bc63ca4 100644 --- a/instrumentation/github.com/emicklei/go-restful/restful.go +++ b/instrumentation/github.com/emicklei/go-restful/restful.go @@ -38,10 +38,10 @@ func OTelFilter(service string, opts ...Option) restful.FilterFunction { for _, opt := range opts { opt(&cfg) } - if cfg.TraceProvider == nil { - cfg.TraceProvider = otelglobal.TraceProvider() + if cfg.TracerProvider == nil { + cfg.TracerProvider = otelglobal.TraceProvider() } - cfg.Tracer = cfg.TraceProvider.Tracer(tracerName, oteltrace.WithInstrumentationVersion(tracerVersion)) + tracer := cfg.TracerProvider.Tracer(tracerName, oteltrace.WithInstrumentationVersion(tracerVersion)) if cfg.Propagators == nil { cfg.Propagators = otelglobal.Propagators() } @@ -57,7 +57,7 @@ func OTelFilter(service string, opts ...Option) restful.FilterFunction { oteltrace.WithAttributes(semconv.HTTPServerAttributesFromHTTPRequest(service, route, r)...), oteltrace.WithSpanKind(oteltrace.SpanKindServer), } - ctx, span := cfg.Tracer.Start(ctx, spanName, opts...) + ctx, span := tracer.Start(ctx, spanName, opts...) defer span.End() // pass the span through the request context diff --git a/instrumentation/github.com/gin-gonic/gin/gintrace.go b/instrumentation/github.com/gin-gonic/gin/gintrace.go index ed1e0ee4d89..8ddabdfdb42 100644 --- a/instrumentation/github.com/gin-gonic/gin/gintrace.go +++ b/instrumentation/github.com/gin-gonic/gin/gintrace.go @@ -43,15 +43,15 @@ func Middleware(service string, opts ...Option) gin.HandlerFunc { for _, opt := range opts { opt(&cfg) } - if cfg.TraceProvider == nil { - cfg.TraceProvider = otelglobal.TraceProvider() + if cfg.TracerProvider == nil { + cfg.TracerProvider = otelglobal.TraceProvider() } - cfg.Tracer = cfg.TraceProvider.Tracer(tracerName) + tracer := cfg.TracerProvider.Tracer(tracerName) if cfg.Propagators == nil { cfg.Propagators = otelglobal.Propagators() } return func(c *gin.Context) { - c.Set(tracerKey, cfg.Tracer) + c.Set(tracerKey, tracer) savedCtx := c.Request.Context() defer func() { c.Request = c.Request.WithContext(savedCtx) @@ -67,7 +67,7 @@ func Middleware(service string, opts ...Option) gin.HandlerFunc { if spanName == "" { spanName = fmt.Sprintf("HTTP %s route not found", c.Request.Method) } - ctx, span := cfg.Tracer.Start(ctx, spanName, opts...) + ctx, span := tracer.Start(ctx, spanName, opts...) defer span.End() // pass the span through the request context diff --git a/instrumentation/github.com/gin-gonic/gin/option.go b/instrumentation/github.com/gin-gonic/gin/option.go index 25b44c73627..420681dfd22 100644 --- a/instrumentation/github.com/gin-gonic/gin/option.go +++ b/instrumentation/github.com/gin-gonic/gin/option.go @@ -22,9 +22,8 @@ import ( ) type Config struct { - TraceProvider oteltrace.Provider - Tracer oteltrace.Tracer - Propagators otelpropagation.Propagators + TracerProvider oteltrace.Provider + Propagators otelpropagation.Propagators } // Option specifies instrumentation configuration options. @@ -43,6 +42,6 @@ func WithPropagators(propagators otelpropagation.Propagators) Option { // If none is specified, the global provider is used. func WithTracerProvider(provider oteltrace.Provider) Option { return func(cfg *Config) { - cfg.TraceProvider = provider + cfg.TracerProvider = provider } } diff --git a/instrumentation/github.com/gorilla/mux/config.go b/instrumentation/github.com/gorilla/mux/config.go index 77ccecff68e..dce194aae42 100644 --- a/instrumentation/github.com/gorilla/mux/config.go +++ b/instrumentation/github.com/gorilla/mux/config.go @@ -21,9 +21,8 @@ import ( // Config is used to configure the mux middleware. type Config struct { - TraceProvider oteltrace.Provider - Tracer oteltrace.Tracer - Propagators otelpropagation.Propagators + TracerProvider oteltrace.Provider + Propagators otelpropagation.Propagators } // Option specifies instrumentation configuration options. @@ -42,6 +41,6 @@ func WithPropagators(propagators otelpropagation.Propagators) Option { // If none is specified, the global provider is used. func WithTracerProvider(provider oteltrace.Provider) Option { return func(cfg *Config) { - cfg.TraceProvider = provider + cfg.TracerProvider = provider } } diff --git a/instrumentation/github.com/gorilla/mux/mux.go b/instrumentation/github.com/gorilla/mux/mux.go index f91aacb95e7..81bb3976b51 100644 --- a/instrumentation/github.com/gorilla/mux/mux.go +++ b/instrumentation/github.com/gorilla/mux/mux.go @@ -39,17 +39,17 @@ func Middleware(service string, opts ...Option) mux.MiddlewareFunc { for _, opt := range opts { opt(&cfg) } - if cfg.TraceProvider == nil { - cfg.TraceProvider = otelglobal.TraceProvider() + if cfg.TracerProvider == nil { + cfg.TracerProvider = otelglobal.TraceProvider() } - cfg.Tracer = cfg.TraceProvider.Tracer(tracerName) + tracer := cfg.TracerProvider.Tracer(tracerName) if cfg.Propagators == nil { cfg.Propagators = otelglobal.Propagators() } return func(handler http.Handler) http.Handler { return traceware{ service: service, - tracer: cfg.Tracer, + tracer: tracer, propagators: cfg.Propagators, handler: handler, } diff --git a/instrumentation/github.com/labstack/echo/config.go b/instrumentation/github.com/labstack/echo/config.go index b7fa83c2857..4d74234c9d6 100644 --- a/instrumentation/github.com/labstack/echo/config.go +++ b/instrumentation/github.com/labstack/echo/config.go @@ -21,9 +21,8 @@ import ( // Config is used to configure the mux middleware. type Config struct { - TraceProvider oteltrace.Provider - Tracer oteltrace.Tracer - Propagators otelpropagation.Propagators + TracerProvider oteltrace.Provider + Propagators otelpropagation.Propagators } // Option specifies instrumentation configuration options. @@ -42,6 +41,6 @@ func WithPropagators(propagators otelpropagation.Propagators) Option { // If none is specified, the global provider is used. func WithTracerProvider(provider oteltrace.Provider) Option { return func(cfg *Config) { - cfg.TraceProvider = provider + cfg.TracerProvider = provider } } diff --git a/instrumentation/github.com/labstack/echo/echo.go b/instrumentation/github.com/labstack/echo/echo.go index cd730b4e0ad..781c4bf6869 100644 --- a/instrumentation/github.com/labstack/echo/echo.go +++ b/instrumentation/github.com/labstack/echo/echo.go @@ -37,16 +37,16 @@ func Middleware(service string, opts ...Option) echo.MiddlewareFunc { for _, opt := range opts { opt(&cfg) } - if cfg.TraceProvider == nil { - cfg.TraceProvider = otelglobal.TraceProvider() + if cfg.TracerProvider == nil { + cfg.TracerProvider = otelglobal.TraceProvider() } - cfg.Tracer = cfg.TraceProvider.Tracer(tracerName) + tracer := cfg.TracerProvider.Tracer(tracerName) if cfg.Propagators == nil { cfg.Propagators = otelglobal.Propagators() } return func(next echo.HandlerFunc) echo.HandlerFunc { return func(c echo.Context) error { - c.Set(tracerKey, cfg.Tracer) + c.Set(tracerKey, tracer) request := c.Request() savedCtx := request.Context() defer func() { @@ -65,7 +65,7 @@ func Middleware(service string, opts ...Option) echo.MiddlewareFunc { spanName = fmt.Sprintf("HTTP %s route not found", request.Method) } - ctx, span := cfg.Tracer.Start(ctx, spanName, opts...) + ctx, span := tracer.Start(ctx, spanName, opts...) defer span.End() // pass the span through the request context diff --git a/instrumentation/go.mongodb.org/mongo-driver/config.go b/instrumentation/go.mongodb.org/mongo-driver/config.go index 5093e1cb539..4b5e503cf9e 100644 --- a/instrumentation/go.mongodb.org/mongo-driver/config.go +++ b/instrumentation/go.mongodb.org/mongo-driver/config.go @@ -25,7 +25,7 @@ const ( // Config is used to configure the mongo tracer. type Config struct { - TraceProvider trace.Provider + TracerProvider trace.Provider Tracer trace.Tracer } @@ -33,13 +33,13 @@ type Config struct { // newConfig returns a Config with all Options set. func newConfig(opts ...Option) Config { cfg := Config{ - TraceProvider: global.TraceProvider(), + TracerProvider: global.TraceProvider(), } for _, opt := range opts { opt(&cfg) } - cfg.Tracer = cfg.TraceProvider.Tracer(defaultTracerName) + cfg.Tracer = cfg.TracerProvider.Tracer(defaultTracerName) return cfg } @@ -50,6 +50,6 @@ type Option func(*Config) // If none is specified, the global provider is used. func WithTracerProvider(provider trace.Provider) Option { return func(cfg *Config) { - cfg.TraceProvider = provider + cfg.TracerProvider = provider } } diff --git a/instrumentation/net/http/config.go b/instrumentation/net/http/config.go index d6d6f9cb337..a869bd472d1 100644 --- a/instrumentation/net/http/config.go +++ b/instrumentation/net/http/config.go @@ -39,8 +39,8 @@ type Config struct { Filters []Filter SpanNameFormatter func(string, *http.Request) string - TraceProvider trace.Provider - MeterProvider metric.Provider + TracerProvider trace.Provider + MeterProvider metric.Provider } // Option Interface used for setting *optional* Config properties @@ -59,15 +59,15 @@ func (o OptionFunc) Apply(c *Config) { // NewConfig creates a new Config struct and applies opts to it. func NewConfig(opts ...Option) *Config { c := &Config{ - Propagators: global.Propagators(), - TraceProvider: global.TraceProvider(), - MeterProvider: global.MeterProvider(), + Propagators: global.Propagators(), + TracerProvider: global.TraceProvider(), + MeterProvider: global.MeterProvider(), } for _, opt := range opts { opt.Apply(c) } - c.Tracer = c.TraceProvider.Tracer(instrumentationName) + c.Tracer = c.TracerProvider.Tracer(instrumentationName) c.Meter = c.MeterProvider.Meter(instrumentationName) return c @@ -77,7 +77,7 @@ func NewConfig(opts ...Option) *Config { // If none is specified, the global provider is used. func WithTracerProvider(provider trace.Provider) Option { return OptionFunc(func(cfg *Config) { - cfg.TraceProvider = provider + cfg.TracerProvider = provider }) } From e8eef931aa73c55d16813fd8993f9825e2bcb623 Mon Sep 17 00:00:00 2001 From: Sam Xie Date: Thu, 27 Aug 2020 11:23:31 +0800 Subject: [PATCH 14/17] Avoid the escalation of a test failure to panic --- instrumentation/net/http/config_test.go | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/instrumentation/net/http/config_test.go b/instrumentation/net/http/config_test.go index a0836b119c3..9a7abd3b6b8 100644 --- a/instrumentation/net/http/config_test.go +++ b/instrumentation/net/http/config_test.go @@ -120,8 +120,9 @@ func TestSpanNameFormatter(t *testing.T) { } spans := tracer.EndedSpans() - assert.Len(t, spans, 1) - assert.Equal(t, tc.expected, spans[0].Name) + if assert.Len(t, spans, 1) { + assert.Equal(t, tc.expected, spans[0].Name) + } }) } } From 2600f46e61b969a0da00939b391c5006d8de6a74 Mon Sep 17 00:00:00 2001 From: Sam Xie Date: Mon, 31 Aug 2020 10:29:02 +0800 Subject: [PATCH 15/17] Make config struct of instrumentation unexported --- .../github.com/astaxie/beego/config.go | 28 +++++++-------- .../github.com/emicklei/go-restful/config.go | 10 +++--- .../github.com/emicklei/go-restful/restful.go | 2 +- .../github.com/gin-gonic/gin/gintrace.go | 2 +- .../github.com/gin-gonic/gin/option.go | 8 ++--- .../github.com/gorilla/mux/config.go | 10 +++--- instrumentation/github.com/gorilla/mux/mux.go | 2 +- .../github.com/labstack/echo/config.go | 10 +++--- .../github.com/labstack/echo/echo.go | 2 +- .../go.mongodb.org/mongo-driver/config.go | 14 ++++---- .../go.mongodb.org/mongo-driver/mongo.go | 2 +- instrumentation/gopkg.in/macaron.v1/config.go | 10 +++--- .../gopkg.in/macaron.v1/macaron.go | 2 +- instrumentation/host/host.go | 20 +++++------ instrumentation/net/http/config.go | 34 +++++++++---------- instrumentation/net/http/handler.go | 4 +-- instrumentation/net/http/transport.go | 4 +-- instrumentation/runtime/runtime.go | 22 ++++++------ 18 files changed, 93 insertions(+), 93 deletions(-) diff --git a/instrumentation/github.com/astaxie/beego/config.go b/instrumentation/github.com/astaxie/beego/config.go index f4e0870eb51..7056fbcee6f 100644 --- a/instrumentation/github.com/astaxie/beego/config.go +++ b/instrumentation/github.com/astaxie/beego/config.go @@ -21,9 +21,9 @@ import ( "go.opentelemetry.io/otel/api/trace" ) -// Config provides configuration for the beego OpenTelemetry +// config provides configuration for the beego OpenTelemetry // middleware. Configuration is modified using the provided Options. -type Config struct { +type config struct { tracerProvider trace.Provider meterProvider metric.Provider propagators propagation.Propagators @@ -31,17 +31,17 @@ type Config struct { formatter SpanNameFormatter } -// Option applies a configuration to the given Config. +// Option applies a configuration to the given config. type Option interface { - Apply(*Config) + Apply(*config) } // OptionFunc is a function type that applies a particular // configuration to the beego middleware in question. -type OptionFunc func(c *Config) +type OptionFunc func(c *config) -// Apply will apply the option to the Config, c. -func (o OptionFunc) Apply(c *Config) { +// Apply will apply the option to the config, c. +func (o OptionFunc) Apply(c *config) { o(c) } @@ -50,7 +50,7 @@ func (o OptionFunc) Apply(c *Config) { // WithTracerProvider specifies a tracer provider to use for creating a tracer. // If none is specified, the global provider is used. func WithTracerProvider(provider trace.Provider) Option { - return OptionFunc(func(cfg *Config) { + return OptionFunc(func(cfg *config) { cfg.tracerProvider = provider }) } @@ -58,7 +58,7 @@ func WithTracerProvider(provider trace.Provider) Option { // WithMeterProvider specifies a meter provider to use for creating a meter. // If none is specified, the global provider is used. func WithMeterProvider(provider metric.Provider) Option { - return OptionFunc(func(cfg *Config) { + return OptionFunc(func(cfg *config) { cfg.meterProvider = provider }) } @@ -66,7 +66,7 @@ func WithMeterProvider(provider metric.Provider) Option { // WithPropagators sets the propagators used in the middleware. // Defaults to global.Propagators(). func WithPropagators(propagators propagation.Propagators) OptionFunc { - return OptionFunc(func(c *Config) { + return OptionFunc(func(c *config) { c.propagators = propagators }) } @@ -74,7 +74,7 @@ func WithPropagators(propagators propagation.Propagators) OptionFunc { // WithFilter adds the given filter for use in the middleware. // Defaults to no filters. func WithFilter(f Filter) OptionFunc { - return OptionFunc(func(c *Config) { + return OptionFunc(func(c *config) { c.filters = append(c.filters, f) }) } @@ -82,15 +82,15 @@ func WithFilter(f Filter) OptionFunc { // WithSpanNameFormatter sets the formatter to be used to format // span names. Defaults to the path template. func WithSpanNameFormatter(f SpanNameFormatter) OptionFunc { - return OptionFunc(func(c *Config) { + return OptionFunc(func(c *config) { c.formatter = f }) } // ------------------------------------------ Private Functions -func configure(options ...Option) *Config { - config := &Config{ +func configure(options ...Option) *config { + config := &config{ tracerProvider: global.TraceProvider(), meterProvider: global.MeterProvider(), propagators: global.Propagators(), diff --git a/instrumentation/github.com/emicklei/go-restful/config.go b/instrumentation/github.com/emicklei/go-restful/config.go index d7196c05270..164aac49bba 100644 --- a/instrumentation/github.com/emicklei/go-restful/config.go +++ b/instrumentation/github.com/emicklei/go-restful/config.go @@ -19,20 +19,20 @@ import ( oteltrace "go.opentelemetry.io/otel/api/trace" ) -// Config is used to configure the go-restful middleware. -type Config struct { +// config is used to configure the go-restful middleware. +type config struct { TracerProvider oteltrace.Provider Propagators otelpropagation.Propagators } // Option specifies instrumentation configuration options. -type Option func(*Config) +type Option func(*config) // WithPropagators specifies propagators to use for extracting // information from the HTTP requests. If none are specified, global // ones will be used. func WithPropagators(propagators otelpropagation.Propagators) Option { - return func(cfg *Config) { + return func(cfg *config) { cfg.Propagators = propagators } } @@ -40,7 +40,7 @@ func WithPropagators(propagators otelpropagation.Propagators) Option { // WithTracerProvider specifies a tracer provider to use for creating a tracer. // If none is specified, the global provider is used. func WithTracerProvider(provider oteltrace.Provider) Option { - return func(cfg *Config) { + return func(cfg *config) { cfg.TracerProvider = provider } } diff --git a/instrumentation/github.com/emicklei/go-restful/restful.go b/instrumentation/github.com/emicklei/go-restful/restful.go index 30c8bc63ca4..837892b40d5 100644 --- a/instrumentation/github.com/emicklei/go-restful/restful.go +++ b/instrumentation/github.com/emicklei/go-restful/restful.go @@ -34,7 +34,7 @@ const ( // the request. Options can be applied to configure the tracer and propagators // used for this filter. func OTelFilter(service string, opts ...Option) restful.FilterFunction { - cfg := Config{} + cfg := config{} for _, opt := range opts { opt(&cfg) } diff --git a/instrumentation/github.com/gin-gonic/gin/gintrace.go b/instrumentation/github.com/gin-gonic/gin/gintrace.go index 8ddabdfdb42..d55ec20b6cc 100644 --- a/instrumentation/github.com/gin-gonic/gin/gintrace.go +++ b/instrumentation/github.com/gin-gonic/gin/gintrace.go @@ -39,7 +39,7 @@ const ( // The service parameter should describe the name of the (virtual) // server handling the request. func Middleware(service string, opts ...Option) gin.HandlerFunc { - cfg := Config{} + cfg := config{} for _, opt := range opts { opt(&cfg) } diff --git a/instrumentation/github.com/gin-gonic/gin/option.go b/instrumentation/github.com/gin-gonic/gin/option.go index 420681dfd22..8e99067a99e 100644 --- a/instrumentation/github.com/gin-gonic/gin/option.go +++ b/instrumentation/github.com/gin-gonic/gin/option.go @@ -21,19 +21,19 @@ import ( oteltrace "go.opentelemetry.io/otel/api/trace" ) -type Config struct { +type config struct { TracerProvider oteltrace.Provider Propagators otelpropagation.Propagators } // Option specifies instrumentation configuration options. -type Option func(*Config) +type Option func(*config) // WithPropagators specifies propagators to use for extracting // information from the HTTP requests. If none are specified, global // ones will be used. func WithPropagators(propagators otelpropagation.Propagators) Option { - return func(cfg *Config) { + return func(cfg *config) { cfg.Propagators = propagators } } @@ -41,7 +41,7 @@ func WithPropagators(propagators otelpropagation.Propagators) Option { // WithTracerProvider specifies a tracer provider to use for creating a tracer. // If none is specified, the global provider is used. func WithTracerProvider(provider oteltrace.Provider) Option { - return func(cfg *Config) { + return func(cfg *config) { cfg.TracerProvider = provider } } diff --git a/instrumentation/github.com/gorilla/mux/config.go b/instrumentation/github.com/gorilla/mux/config.go index dce194aae42..528e5edb3d0 100644 --- a/instrumentation/github.com/gorilla/mux/config.go +++ b/instrumentation/github.com/gorilla/mux/config.go @@ -19,20 +19,20 @@ import ( oteltrace "go.opentelemetry.io/otel/api/trace" ) -// Config is used to configure the mux middleware. -type Config struct { +// config is used to configure the mux middleware. +type config struct { TracerProvider oteltrace.Provider Propagators otelpropagation.Propagators } // Option specifies instrumentation configuration options. -type Option func(*Config) +type Option func(*config) // WithPropagators specifies propagators to use for extracting // information from the HTTP requests. If none are specified, global // ones will be used. func WithPropagators(propagators otelpropagation.Propagators) Option { - return func(cfg *Config) { + return func(cfg *config) { cfg.Propagators = propagators } } @@ -40,7 +40,7 @@ func WithPropagators(propagators otelpropagation.Propagators) Option { // WithTracerProvider specifies a tracer provider to use for creating a tracer. // If none is specified, the global provider is used. func WithTracerProvider(provider oteltrace.Provider) Option { - return func(cfg *Config) { + return func(cfg *config) { cfg.TracerProvider = provider } } diff --git a/instrumentation/github.com/gorilla/mux/mux.go b/instrumentation/github.com/gorilla/mux/mux.go index 81bb3976b51..261f09b88c9 100644 --- a/instrumentation/github.com/gorilla/mux/mux.go +++ b/instrumentation/github.com/gorilla/mux/mux.go @@ -35,7 +35,7 @@ const ( // requests. The service parameter should describe the name of the // (virtual) server handling the request. func Middleware(service string, opts ...Option) mux.MiddlewareFunc { - cfg := Config{} + cfg := config{} for _, opt := range opts { opt(&cfg) } diff --git a/instrumentation/github.com/labstack/echo/config.go b/instrumentation/github.com/labstack/echo/config.go index 4d74234c9d6..400dc3e9cc8 100644 --- a/instrumentation/github.com/labstack/echo/config.go +++ b/instrumentation/github.com/labstack/echo/config.go @@ -19,20 +19,20 @@ import ( oteltrace "go.opentelemetry.io/otel/api/trace" ) -// Config is used to configure the mux middleware. -type Config struct { +// config is used to configure the mux middleware. +type config struct { TracerProvider oteltrace.Provider Propagators otelpropagation.Propagators } // Option specifies instrumentation configuration options. -type Option func(*Config) +type Option func(*config) // WithPropagators specifies propagators to use for extracting // information from the HTTP requests. If none are specified, global // ones will be used. func WithPropagators(propagators otelpropagation.Propagators) Option { - return func(cfg *Config) { + return func(cfg *config) { cfg.Propagators = propagators } } @@ -40,7 +40,7 @@ func WithPropagators(propagators otelpropagation.Propagators) Option { // WithTracerProvider specifies a tracer provider to use for creating a tracer. // If none is specified, the global provider is used. func WithTracerProvider(provider oteltrace.Provider) Option { - return func(cfg *Config) { + return func(cfg *config) { cfg.TracerProvider = provider } } diff --git a/instrumentation/github.com/labstack/echo/echo.go b/instrumentation/github.com/labstack/echo/echo.go index 781c4bf6869..d805de47833 100644 --- a/instrumentation/github.com/labstack/echo/echo.go +++ b/instrumentation/github.com/labstack/echo/echo.go @@ -33,7 +33,7 @@ const ( // Middleware returns echo middleware which will trace incoming requests. func Middleware(service string, opts ...Option) echo.MiddlewareFunc { - cfg := Config{} + cfg := config{} for _, opt := range opts { opt(&cfg) } diff --git a/instrumentation/go.mongodb.org/mongo-driver/config.go b/instrumentation/go.mongodb.org/mongo-driver/config.go index 4b5e503cf9e..f21f85f9b6f 100644 --- a/instrumentation/go.mongodb.org/mongo-driver/config.go +++ b/instrumentation/go.mongodb.org/mongo-driver/config.go @@ -23,16 +23,16 @@ const ( defaultTracerName = "go.opentelemetry.io/contrib/instrumentation/go.mongodb.org/mongo-driver" ) -// Config is used to configure the mongo tracer. -type Config struct { +// config is used to configure the mongo tracer. +type config struct { TracerProvider trace.Provider Tracer trace.Tracer } -// newConfig returns a Config with all Options set. -func newConfig(opts ...Option) Config { - cfg := Config{ +// newConfig returns a config with all Options set. +func newConfig(opts ...Option) config { + cfg := config{ TracerProvider: global.TraceProvider(), } for _, opt := range opts { @@ -44,12 +44,12 @@ func newConfig(opts ...Option) Config { } // Option specifies instrumentation configuration options. -type Option func(*Config) +type Option func(*config) // WithTracerProvider specifies a tracer provider to use for creating a tracer. // If none is specified, the global provider is used. func WithTracerProvider(provider trace.Provider) Option { - return func(cfg *Config) { + return func(cfg *config) { cfg.TracerProvider = provider } } diff --git a/instrumentation/go.mongodb.org/mongo-driver/mongo.go b/instrumentation/go.mongodb.org/mongo-driver/mongo.go index 257d3d0397b..43102e61437 100644 --- a/instrumentation/go.mongodb.org/mongo-driver/mongo.go +++ b/instrumentation/go.mongodb.org/mongo-driver/mongo.go @@ -36,7 +36,7 @@ type monitor struct { sync.Mutex spans map[spanKey]trace.Span serviceName string - cfg Config + cfg config } func (m *monitor) Started(ctx context.Context, evt *event.CommandStartedEvent) { diff --git a/instrumentation/gopkg.in/macaron.v1/config.go b/instrumentation/gopkg.in/macaron.v1/config.go index 809d78f332d..30109182ae6 100644 --- a/instrumentation/gopkg.in/macaron.v1/config.go +++ b/instrumentation/gopkg.in/macaron.v1/config.go @@ -19,21 +19,21 @@ import ( "go.opentelemetry.io/otel/api/trace" ) -// Config is a helper struct to configure Macaron options -type Config struct { +// config is a helper struct to configure Macaron options +type config struct { Tracer trace.Tracer Propagators propagation.Propagators } // Option specifies instrumentation configuration options. -type Option func(*Config) +type Option func(*config) // WithTracer specifies a tracer to use for creating spans. If none is // specified, a tracer named // "go.opentelemetry.io/contrib/instrumentation/macaron" from the global // provider is used. func WithTracer(tracer trace.Tracer) Option { - return func(cfg *Config) { + return func(cfg *config) { cfg.Tracer = tracer } } @@ -42,7 +42,7 @@ func WithTracer(tracer trace.Tracer) Option { // information from the HTTP requests. If none are specified, global // ones will be used. func WithPropagators(propagators propagation.Propagators) Option { - return func(cfg *Config) { + return func(cfg *config) { cfg.Propagators = propagators } } diff --git a/instrumentation/gopkg.in/macaron.v1/macaron.go b/instrumentation/gopkg.in/macaron.v1/macaron.go index ae38b548553..2b61f34ea9b 100644 --- a/instrumentation/gopkg.in/macaron.v1/macaron.go +++ b/instrumentation/gopkg.in/macaron.v1/macaron.go @@ -32,7 +32,7 @@ const ( // Middleware returns a macaron Handler to trace requests to the server. func Middleware(service string, opts ...Option) macaron.Handler { - cfg := Config{} + cfg := config{} for _, opt := range opts { opt(&cfg) } diff --git a/instrumentation/host/host.go b/instrumentation/host/host.go index 1618130d7d2..31fa2a02b9b 100644 --- a/instrumentation/host/host.go +++ b/instrumentation/host/host.go @@ -34,12 +34,12 @@ import ( // Host reports the work-in-progress conventional host metrics specified by OpenTelemetry type host struct { - config Config + config config meter metric.Meter } -// Config contains optional settings for reporting host metrics. -type Config struct { +// config contains optional settings for reporting host metrics. +type config struct { // MeterProvider sets the metric.Provider. If nil, the global // Provider will be used. MeterProvider metric.Provider @@ -47,8 +47,8 @@ type Config struct { // Option supports configuring optional settings for host metrics. type Option interface { - // ApplyHost updates *Config. - ApplyHost(*Config) + // ApplyHost updates *config. + ApplyHost(*config) } // WithMeterProvider sets the Metric implementation to use for @@ -61,7 +61,7 @@ func WithMeterProvider(provider metric.Provider) Option { type metricProviderOption struct{ metric.Provider } // ApplyHost implements Option. -func (o metricProviderOption) ApplyHost(c *Config) { +func (o metricProviderOption) ApplyHost(c *config) { c.MeterProvider = o.Provider } @@ -84,9 +84,9 @@ var ( LabelNetworkReceive = []label.KeyValue{label.String("direction", "receive")} ) -// configure computes a Config from a list of Options. -func configure(opts ...Option) Config { - c := Config{ +// configure computes a config from a list of Options. +func configure(opts ...Option) config { + c := config{ MeterProvider: global.MeterProvider(), } for _, opt := range opts { @@ -95,7 +95,7 @@ func configure(opts ...Option) Config { return c } -// Start initializes reporting of host metrics using the supplied Config. +// Start initializes reporting of host metrics using the supplied config. func Start(opts ...Option) error { c := configure(opts...) if c.MeterProvider == nil { diff --git a/instrumentation/net/http/config.go b/instrumentation/net/http/config.go index a869bd472d1..316e005b058 100644 --- a/instrumentation/net/http/config.go +++ b/instrumentation/net/http/config.go @@ -27,9 +27,9 @@ const ( instrumentationName = "go.opentelemetry.io/contrib/instrumentation/net/http" ) -// Config represents the configuration options available for the http.Handler +// config represents the configuration options available for the http.Handler // and http.Transport types. -type Config struct { +type config struct { Tracer trace.Tracer Meter metric.Meter Propagators propagation.Propagators @@ -43,22 +43,22 @@ type Config struct { MeterProvider metric.Provider } -// Option Interface used for setting *optional* Config properties +// Option Interface used for setting *optional* config properties type Option interface { - Apply(*Config) + Apply(*config) } // OptionFunc provides a convenience wrapper for simple Options // that can be represented as functions. -type OptionFunc func(*Config) +type OptionFunc func(*config) -func (o OptionFunc) Apply(c *Config) { +func (o OptionFunc) Apply(c *config) { o(c) } -// NewConfig creates a new Config struct and applies opts to it. -func NewConfig(opts ...Option) *Config { - c := &Config{ +// newConfig creates a new config struct and applies opts to it. +func newConfig(opts ...Option) *config { + c := &config{ Propagators: global.Propagators(), TracerProvider: global.TraceProvider(), MeterProvider: global.MeterProvider(), @@ -76,7 +76,7 @@ func NewConfig(opts ...Option) *Config { // WithTracerProvider specifies a tracer provider to use for creating a tracer. // If none is specified, the global provider is used. func WithTracerProvider(provider trace.Provider) Option { - return OptionFunc(func(cfg *Config) { + return OptionFunc(func(cfg *config) { cfg.TracerProvider = provider }) } @@ -84,7 +84,7 @@ func WithTracerProvider(provider trace.Provider) Option { // WithMeterProvider specifies a meter provider to use for creating a meter. // If none is specified, the global provider is used. func WithMeterProvider(provider metric.Provider) Option { - return OptionFunc(func(cfg *Config) { + return OptionFunc(func(cfg *config) { cfg.MeterProvider = provider }) } @@ -93,7 +93,7 @@ func WithMeterProvider(provider metric.Provider) Option { // span context. If this option is not provided, then the association is a child // association instead of a link. func WithPublicEndpoint() Option { - return OptionFunc(func(c *Config) { + return OptionFunc(func(c *config) { c.SpanStartOptions = append(c.SpanStartOptions, trace.WithNewRoot()) }) } @@ -102,7 +102,7 @@ func WithPublicEndpoint() Option { // option isn't specified then // go.opentelemetry.io/otel/api/global.Propagators are used. func WithPropagators(ps propagation.Propagators) Option { - return OptionFunc(func(c *Config) { + return OptionFunc(func(c *config) { c.Propagators = ps }) } @@ -110,7 +110,7 @@ func WithPropagators(ps propagation.Propagators) Option { // WithSpanOptions configures an additional set of // trace.StartOptions, which are applied to each new span. func WithSpanOptions(opts ...trace.StartOption) Option { - return OptionFunc(func(c *Config) { + return OptionFunc(func(c *config) { c.SpanStartOptions = append(c.SpanStartOptions, opts...) }) } @@ -122,7 +122,7 @@ func WithSpanOptions(opts ...trace.StartOption) Option { // Filters will be invoked for each processed request, it is advised to make them // simple and fast. func WithFilter(f Filter) Option { - return OptionFunc(func(c *Config) { + return OptionFunc(func(c *config) { c.Filters = append(c.Filters, f) }) } @@ -145,7 +145,7 @@ const ( // * WriteEvents: Record the number of bytes written after every http.ResponeWriter.Write // using the WriteBytesKey func WithMessageEvents(events ...event) Option { - return OptionFunc(func(c *Config) { + return OptionFunc(func(c *config) { for _, e := range events { switch e { case ReadEvents: @@ -160,7 +160,7 @@ func WithMessageEvents(events ...event) Option { // WithSpanNameFormatter takes a function that will be called on every // request and the returned string will become the Span Name func WithSpanNameFormatter(f func(operation string, r *http.Request) string) Option { - return OptionFunc(func(c *Config) { + return OptionFunc(func(c *config) { c.SpanNameFormatter = f }) } diff --git a/instrumentation/net/http/handler.go b/instrumentation/net/http/handler.go index 72fddbd891d..dc28c757b95 100644 --- a/instrumentation/net/http/handler.go +++ b/instrumentation/net/http/handler.go @@ -68,14 +68,14 @@ func NewHandler(handler http.Handler, operation string, opts ...Option) http.Han WithSpanNameFormatter(defaultHandlerFormatter), } - c := NewConfig(append(defaultOpts, opts...)...) + c := newConfig(append(defaultOpts, opts...)...) h.configure(c) h.createMeasures() return &h } -func (h *Handler) configure(c *Config) { +func (h *Handler) configure(c *config) { h.tracer = c.Tracer h.meter = c.Meter h.propagators = c.Propagators diff --git a/instrumentation/net/http/transport.go b/instrumentation/net/http/transport.go index c66e753a6d4..e6df9882ee0 100644 --- a/instrumentation/net/http/transport.go +++ b/instrumentation/net/http/transport.go @@ -52,13 +52,13 @@ func NewTransport(base http.RoundTripper, opts ...Option) *Transport { WithSpanNameFormatter(defaultTransportFormatter), } - c := NewConfig(append(defaultOpts, opts...)...) + c := newConfig(append(defaultOpts, opts...)...) t.configure(c) return &t } -func (t *Transport) configure(c *Config) { +func (t *Transport) configure(c *config) { t.tracer = c.Tracer t.propagators = c.Propagators t.spanStartOptions = c.SpanStartOptions diff --git a/instrumentation/runtime/runtime.go b/instrumentation/runtime/runtime.go index 25e0bad9654..4f3b69fb898 100644 --- a/instrumentation/runtime/runtime.go +++ b/instrumentation/runtime/runtime.go @@ -28,12 +28,12 @@ import ( // Runtime reports the work-in-progress conventional runtime metrics specified by OpenTelemetry type runtime struct { - config Config + config config meter metric.Meter } -// Config contains optional settings for reporting runtime metrics. -type Config struct { +// config contains optional settings for reporting runtime metrics. +type config struct { // MinimumReadMemStatsInterval sets the mininum interval // between calls to runtime.ReadMemStats(). Negative values // are ignored. @@ -46,8 +46,8 @@ type Config struct { // Option supports configuring optional settings for runtime metrics. type Option interface { - // ApplyRuntime updates *Config. - ApplyRuntime(*Config) + // ApplyRuntime updates *config. + ApplyRuntime(*config) } // DefaultMinimumReadMemStatsInterval is the default minimum interval @@ -66,7 +66,7 @@ func WithMinimumReadMemStatsInterval(d time.Duration) Option { type minimumReadMemStatsIntervalOption time.Duration // ApplyRuntime implements Option. -func (o minimumReadMemStatsIntervalOption) ApplyRuntime(c *Config) { +func (o minimumReadMemStatsIntervalOption) ApplyRuntime(c *config) { if o >= 0 { c.MinimumReadMemStatsInterval = time.Duration(o) } @@ -82,13 +82,13 @@ func WithMeterProvider(provider metric.Provider) Option { type metricProviderOption struct{ metric.Provider } // ApplyRuntime implements Option. -func (o metricProviderOption) ApplyRuntime(c *Config) { +func (o metricProviderOption) ApplyRuntime(c *config) { c.MeterProvider = o.Provider } -// configure computes a Config from the supplied Options. -func configure(opts ...Option) Config { - c := Config{ +// configure computes a config from the supplied Options. +func configure(opts ...Option) config { + c := config{ MeterProvider: global.MeterProvider(), MinimumReadMemStatsInterval: DefaultMinimumReadMemStatsInterval, } @@ -98,7 +98,7 @@ func configure(opts ...Option) Config { return c } -// Start initializes reporting of runtime metrics using the supplied Config. +// Start initializes reporting of runtime metrics using the supplied config. func Start(opts ...Option) error { c := configure(opts...) if c.MinimumReadMemStatsInterval < 0 { From 81be81b1dc832c601001246a5ecb941764129932 Mon Sep 17 00:00:00 2001 From: Sam Xie Date: Mon, 31 Aug 2020 10:42:54 +0800 Subject: [PATCH 16/17] Update style guide --- CONTRIBUTING.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 9bf9b4b04dd..51220138e1f 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -91,6 +91,7 @@ Any Maintainer can merge the PR once it is **ready to merge**. * Make sure to run `make precommit` - this will find and fix the code formatting. +* Check [opentelemetry-go Style Guide](https://github.com/open-telemetry/opentelemetry-go/blob/master/CONTRIBUTING.md#style-guide) ## Adding a new Contrib package From d82cf32510d30f48589ad1cfef5541340d88dd3a Mon Sep 17 00:00:00 2001 From: Sam Xie Date: Mon, 31 Aug 2020 10:43:46 +0800 Subject: [PATCH 17/17] Update CHANGELOG --- CHANGELOG.md | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index bd2d9c1a7b3..1d4a9fd05cb 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -17,7 +17,8 @@ This project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.htm - Unify instrumentation about provider options for `go.mongodb.org/mongo-driver`, `gin-gonic/gin`, `gorilla/mux`, `labstack/echo`, `emicklei/go-restful`, `bradfitz/gomemcache`, `Shopify/sarama`, `net/http` and `beego`. (#303) -- Update instrumentation guidelines about uniform provider options. (#303) +- Update instrumentation guidelines about uniform provider options. Also, update style guide. (#303) +- Make config struct of instrumentation unexported. (#303) ## [0.11.0] - 2020-08-25