From 0ffe14b750f9e4e1477a6f1f0a15d4ce0345274a Mon Sep 17 00:00:00 2001 From: Robert Lin Date: Fri, 26 Aug 2022 09:51:03 -0700 Subject: [PATCH 01/10] bridge/opentracing: add NewDynamicWrappedTracerProvider for named tracers --- bridge/opentracing/wrapper.go | 24 +++++++++++++++++++----- 1 file changed, 19 insertions(+), 5 deletions(-) diff --git a/bridge/opentracing/wrapper.go b/bridge/opentracing/wrapper.go index 8016ea2a87c..5137414ef9a 100644 --- a/bridge/opentracing/wrapper.go +++ b/bridge/opentracing/wrapper.go @@ -24,21 +24,35 @@ import ( // WrapperTracerProvider is an OpenTelemetry TracerProvider that wraps an // OpenTracing Tracer. type WrapperTracerProvider struct { - wTracer *WrapperTracer + getWrappedTracer func(name string, opts ...trace.TracerOption) *WrapperTracer } var _ trace.TracerProvider = (*WrapperTracerProvider)(nil) // Tracer returns the WrapperTracer associated with the WrapperTracerProvider. -func (p *WrapperTracerProvider) Tracer(_ string, _ ...trace.TracerOption) trace.Tracer { - return p.wTracer +func (p *WrapperTracerProvider) Tracer(name string, opts ...trace.TracerOption) trace.Tracer { + return p.getWrappedTracer(name, opts...) } // NewWrappedTracerProvider creates a new trace provider that creates a single -// instance of WrapperTracer that wraps OpenTelemetry tracer. +// instance of WrapperTracer that wraps OpenTelemetry tracer, and always returns +// it unmodified from Tracer(). func NewWrappedTracerProvider(bridge *BridgeTracer, tracer trace.Tracer) *WrapperTracerProvider { + wTracer := NewWrapperTracer(bridge, tracer) return &WrapperTracerProvider{ - wTracer: NewWrapperTracer(bridge, tracer), + getWrappedTracer: func(_ string, _ ...trace.TracerOption) *WrapperTracer { + return wTracer + }, + } +} + +// NewDynamicWrappedTracerProvider creates a new trace provider that creates new +// instances of WrapperTracer that wraps OpenTelemetry tracer for each call to Tracer(). +func NewDynamicWrappedTracerProvider(bridge *BridgeTracer, provider trace.TracerProvider) *WrapperTracerProvider { + return &WrapperTracerProvider{ + getWrappedTracer: func(name string, opts ...trace.TracerOption) *WrapperTracer { + return NewWrapperTracer(bridge, provider.Tracer(name, opts...)) + }, } } From 8a83e1f9408b5e853c2d268a8bc0fc6b0ab719d7 Mon Sep 17 00:00:00 2001 From: Robert Lin Date: Mon, 29 Aug 2022 14:31:45 -0700 Subject: [PATCH 02/10] bridge/opentelmeetry: cache created Tracers, add tests --- bridge/opentracing/wrapper.go | 28 +++++++++++- bridge/opentracing/wrapper_test.go | 71 ++++++++++++++++++++++++++++++ 2 files changed, 98 insertions(+), 1 deletion(-) create mode 100644 bridge/opentracing/wrapper_test.go diff --git a/bridge/opentracing/wrapper.go b/bridge/opentracing/wrapper.go index 5137414ef9a..5f10819809f 100644 --- a/bridge/opentracing/wrapper.go +++ b/bridge/opentracing/wrapper.go @@ -16,6 +16,7 @@ package opentracing // import "go.opentelemetry.io/otel/bridge/opentracing" import ( "context" + "sync" "go.opentelemetry.io/otel/bridge/opentracing/migration" "go.opentelemetry.io/otel/trace" @@ -46,12 +47,37 @@ func NewWrappedTracerProvider(bridge *BridgeTracer, tracer trace.Tracer) *Wrappe } } +type wrappedTracerKey struct { + name string + version string +} + // NewDynamicWrappedTracerProvider creates a new trace provider that creates new // instances of WrapperTracer that wraps OpenTelemetry tracer for each call to Tracer(). func NewDynamicWrappedTracerProvider(bridge *BridgeTracer, provider trace.TracerProvider) *WrapperTracerProvider { + var ( + mtx sync.Mutex + tracers = make(map[wrappedTracerKey]*WrapperTracer) + ) + return &WrapperTracerProvider{ getWrappedTracer: func(name string, opts ...trace.TracerOption) *WrapperTracer { - return NewWrapperTracer(bridge, provider.Tracer(name, opts...)) + mtx.Lock() + defer mtx.Unlock() + + c := trace.NewTracerConfig(opts...) + key := wrappedTracerKey{ + name: name, + version: c.InstrumentationVersion(), + } + + if t, ok := tracers[key]; ok { + return t + } + + wrapper := NewWrapperTracer(bridge, provider.Tracer(name, opts...)) + tracers[key] = wrapper + return wrapper }, } } diff --git a/bridge/opentracing/wrapper_test.go b/bridge/opentracing/wrapper_test.go new file mode 100644 index 00000000000..f98277fc2e2 --- /dev/null +++ b/bridge/opentracing/wrapper_test.go @@ -0,0 +1,71 @@ +package opentracing + +import ( + "testing" + + "go.opentelemetry.io/otel/bridge/opentracing/internal" + "go.opentelemetry.io/otel/trace" +) + +type namedMockTracer struct { + name string + *internal.MockTracer +} + +type namedMockTracerProvider struct{} + +var _ trace.TracerProvider = (*namedMockTracerProvider)(nil) + +// Tracer returns the WrapperTracer associated with the WrapperTracerProvider. +func (p *namedMockTracerProvider) Tracer(name string, opts ...trace.TracerOption) trace.Tracer { + return &namedMockTracer{ + name: name, + MockTracer: internal.NewMockTracer(), + } +} + +func TestDynamicWrappedTracerProvider(t *testing.T) { + // assertMockTracerName casts tracer into a named mock tracer provided by + // namedMockTracerProvider, and asserts against its name + assertMockTracerName := func(t *testing.T, tracer trace.Tracer, name string) { + // Unwrap the tracer + wrapped := tracer.(*WrapperTracer) + tracer = wrapped.tracer + + // Cast into the underlying type and assert + if mock, ok := tracer.(*namedMockTracer); ok { + if name != mock.name { + t.Errorf("expected name %q, got %q", name, mock.name) + } + } else if !ok { + t.Errorf("expected *namedMockTracer, got %T", mock) + } + } + + var ( + foobar = "foobar" + bazbar = "bazbar" + provider = NewDynamicWrappedTracerProvider(nil, &namedMockTracerProvider{}) + ) + + t.Run("Tracers should be created with foobar from provider", func(t *testing.T) { + tracer := provider.Tracer(foobar) + assertMockTracerName(t, tracer, foobar) + }) + + t.Run("Repeated requests to create a tracer should provide the existing tracer", func(t *testing.T) { + tracer1 := provider.Tracer(foobar) + assertMockTracerName(t, tracer1, foobar) + tracer2 := provider.Tracer(foobar) + assertMockTracerName(t, tracer2, foobar) + tracer3 := provider.Tracer(bazbar) + assertMockTracerName(t, tracer3, bazbar) + + if tracer1 != tracer2 { + t.Errorf("expected the same tracer, got different tracers") + } + if tracer1 == tracer3 || tracer2 == tracer3 { + t.Errorf("expected different tracers, got the same tracer") + } + }) +} From 59f2c6bb8879ea50dcc7adfa51667a6a6175af8a Mon Sep 17 00:00:00 2001 From: Robert Lin Date: Thu, 1 Sep 2022 08:40:54 -0700 Subject: [PATCH 03/10] bridge/opentracing: rename constructor to NewTracerProvider --- bridge/opentracing/wrapper.go | 10 ++++++---- bridge/opentracing/wrapper_test.go | 4 ++-- 2 files changed, 8 insertions(+), 6 deletions(-) diff --git a/bridge/opentracing/wrapper.go b/bridge/opentracing/wrapper.go index 5f10819809f..bac427f1bbc 100644 --- a/bridge/opentracing/wrapper.go +++ b/bridge/opentracing/wrapper.go @@ -30,11 +30,13 @@ type WrapperTracerProvider struct { var _ trace.TracerProvider = (*WrapperTracerProvider)(nil) -// Tracer returns the WrapperTracer associated with the WrapperTracerProvider. +// Tracer creates a WrapperTracer associated with the WrapperTracerProvider. func (p *WrapperTracerProvider) Tracer(name string, opts ...trace.TracerOption) trace.Tracer { return p.getWrappedTracer(name, opts...) } +// Deprecated: Use NewTracerProvider(...) instead. +// // NewWrappedTracerProvider creates a new trace provider that creates a single // instance of WrapperTracer that wraps OpenTelemetry tracer, and always returns // it unmodified from Tracer(). @@ -52,9 +54,9 @@ type wrappedTracerKey struct { version string } -// NewDynamicWrappedTracerProvider creates a new trace provider that creates new -// instances of WrapperTracer that wraps OpenTelemetry tracer for each call to Tracer(). -func NewDynamicWrappedTracerProvider(bridge *BridgeTracer, provider trace.TracerProvider) *WrapperTracerProvider { +// NewTracerProvider creates a new trace provider that creates new instances of +// WrapperTracer that wraps OpenTelemetry tracer for each call to Tracer(). +func NewTracerProvider(bridge *BridgeTracer, provider trace.TracerProvider) *WrapperTracerProvider { var ( mtx sync.Mutex tracers = make(map[wrappedTracerKey]*WrapperTracer) diff --git a/bridge/opentracing/wrapper_test.go b/bridge/opentracing/wrapper_test.go index f98277fc2e2..be1c760af33 100644 --- a/bridge/opentracing/wrapper_test.go +++ b/bridge/opentracing/wrapper_test.go @@ -24,7 +24,7 @@ func (p *namedMockTracerProvider) Tracer(name string, opts ...trace.TracerOption } } -func TestDynamicWrappedTracerProvider(t *testing.T) { +func TestTracerProvider(t *testing.T) { // assertMockTracerName casts tracer into a named mock tracer provided by // namedMockTracerProvider, and asserts against its name assertMockTracerName := func(t *testing.T, tracer trace.Tracer, name string) { @@ -45,7 +45,7 @@ func TestDynamicWrappedTracerProvider(t *testing.T) { var ( foobar = "foobar" bazbar = "bazbar" - provider = NewDynamicWrappedTracerProvider(nil, &namedMockTracerProvider{}) + provider = NewTracerProvider(nil, &namedMockTracerProvider{}) ) t.Run("Tracers should be created with foobar from provider", func(t *testing.T) { From 26f25f0836e009e84039898437be376e420cdf83 Mon Sep 17 00:00:00 2001 From: Robert Lin Date: Wed, 12 Oct 2022 11:27:17 -0700 Subject: [PATCH 04/10] add license header --- bridge/opentracing/wrapper_test.go | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/bridge/opentracing/wrapper_test.go b/bridge/opentracing/wrapper_test.go index be1c760af33..8af3796e031 100644 --- a/bridge/opentracing/wrapper_test.go +++ b/bridge/opentracing/wrapper_test.go @@ -1,3 +1,17 @@ +// Copyright The OpenTelemetry Authors +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + package opentracing import ( From 848a0a3623ff6a054614065918f50fabf47ba14e Mon Sep 17 00:00:00 2001 From: Robert Lin Date: Sun, 20 Nov 2022 15:36:27 -0800 Subject: [PATCH 05/10] Update docstring Co-authored-by: Tyler Yahn --- bridge/opentracing/wrapper.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/bridge/opentracing/wrapper.go b/bridge/opentracing/wrapper.go index bac427f1bbc..d15ec237872 100644 --- a/bridge/opentracing/wrapper.go +++ b/bridge/opentracing/wrapper.go @@ -54,7 +54,7 @@ type wrappedTracerKey struct { version string } -// NewTracerProvider creates a new trace provider that creates new instances of +// NewTracerProvider returns a new TracerProvider that creates new instances of // WrapperTracer that wraps OpenTelemetry tracer for each call to Tracer(). func NewTracerProvider(bridge *BridgeTracer, provider trace.TracerProvider) *WrapperTracerProvider { var ( From af4b04455736fa4e6e939dcad8345310ecb7ecee Mon Sep 17 00:00:00 2001 From: Robert Lin Date: Sun, 20 Nov 2022 15:37:16 -0800 Subject: [PATCH 06/10] fix deprecated docstring --- bridge/opentracing/wrapper.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/bridge/opentracing/wrapper.go b/bridge/opentracing/wrapper.go index d15ec237872..8cd6240a1f3 100644 --- a/bridge/opentracing/wrapper.go +++ b/bridge/opentracing/wrapper.go @@ -35,11 +35,11 @@ func (p *WrapperTracerProvider) Tracer(name string, opts ...trace.TracerOption) return p.getWrappedTracer(name, opts...) } -// Deprecated: Use NewTracerProvider(...) instead. -// // NewWrappedTracerProvider creates a new trace provider that creates a single // instance of WrapperTracer that wraps OpenTelemetry tracer, and always returns // it unmodified from Tracer(). +// +// Deprecated: Use NewTracerProvider(...) instead. func NewWrappedTracerProvider(bridge *BridgeTracer, tracer trace.Tracer) *WrapperTracerProvider { wTracer := NewWrapperTracer(bridge, tracer) return &WrapperTracerProvider{ From 850f48435b5a954fa4dcd3e2e8fd1c60c473937b Mon Sep 17 00:00:00 2001 From: Robert Lin Date: Sun, 20 Nov 2022 15:51:04 -0800 Subject: [PATCH 07/10] rewrite new lookup TracerProvider as separate type --- bridge/opentracing/provider.go | 72 +++++++++++++++++++ .../{wrapper_test.go => provider_test.go} | 0 bridge/opentracing/wrapper.go | 53 +++----------- 3 files changed, 80 insertions(+), 45 deletions(-) create mode 100644 bridge/opentracing/provider.go rename bridge/opentracing/{wrapper_test.go => provider_test.go} (100%) diff --git a/bridge/opentracing/provider.go b/bridge/opentracing/provider.go new file mode 100644 index 00000000000..aefcf5127c0 --- /dev/null +++ b/bridge/opentracing/provider.go @@ -0,0 +1,72 @@ +// Copyright The OpenTelemetry Authors +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package opentracing + +import ( + "sync" + + "go.opentelemetry.io/otel/trace" +) + +// TracerProvider is an OpenTelemetry TracerProvider that wraps an OpenTracing +// Tracer. +type TracerProvider struct { + bridge *BridgeTracer + provider trace.TracerProvider + + tracers map[wrappedTracerKey]*WrapperTracer + mtx sync.Mutex +} + +var _ trace.TracerProvider = (*TracerProvider)(nil) + +// NewTracerProvider returns a new TracerProvider that creates new instances of +// WrapperTracer that wraps OpenTelemetry tracer for each call to Tracer() with +// new configuration. +// +// Repeated calls to Tracer() with the same configuration will look up and return +// an existing instance of WrapperTracer. +func NewTracerProvider(bridge *BridgeTracer, provider trace.TracerProvider) *TracerProvider { + return &TracerProvider{ + bridge: bridge, + provider: provider, + + tracers: make(map[wrappedTracerKey]*WrapperTracer), + } +} + +type wrappedTracerKey struct { + name string + version string +} + +func (p *TracerProvider) Tracer(name string, opts ...trace.TracerOption) trace.Tracer { + p.mtx.Lock() + defer p.mtx.Unlock() + + c := trace.NewTracerConfig(opts...) + key := wrappedTracerKey{ + name: name, + version: c.InstrumentationVersion(), + } + + if t, ok := p.tracers[key]; ok { + return t + } + + wrapper := NewWrapperTracer(p.bridge, p.provider.Tracer(name, opts...)) + p.tracers[key] = wrapper + return wrapper +} diff --git a/bridge/opentracing/wrapper_test.go b/bridge/opentracing/provider_test.go similarity index 100% rename from bridge/opentracing/wrapper_test.go rename to bridge/opentracing/provider_test.go diff --git a/bridge/opentracing/wrapper.go b/bridge/opentracing/wrapper.go index 8cd6240a1f3..3e348c45523 100644 --- a/bridge/opentracing/wrapper.go +++ b/bridge/opentracing/wrapper.go @@ -16,23 +16,24 @@ package opentracing // import "go.opentelemetry.io/otel/bridge/opentracing" import ( "context" - "sync" "go.opentelemetry.io/otel/bridge/opentracing/migration" "go.opentelemetry.io/otel/trace" ) // WrapperTracerProvider is an OpenTelemetry TracerProvider that wraps an -// OpenTracing Tracer. +// OpenTracing Tracer, created by the deprecated NewWrappedTracerProvider. +// +// Deprecated: Use the TracerProvider from NewTracerProvider(...) instead. type WrapperTracerProvider struct { - getWrappedTracer func(name string, opts ...trace.TracerOption) *WrapperTracer + wTracer *WrapperTracer } var _ trace.TracerProvider = (*WrapperTracerProvider)(nil) -// Tracer creates a WrapperTracer associated with the WrapperTracerProvider. -func (p *WrapperTracerProvider) Tracer(name string, opts ...trace.TracerOption) trace.Tracer { - return p.getWrappedTracer(name, opts...) +// Tracer returns the WrapperTracer associated with the WrapperTracerProvider. +func (p *WrapperTracerProvider) Tracer(_ string, _ ...trace.TracerOption) trace.Tracer { + return p.wTracer } // NewWrappedTracerProvider creates a new trace provider that creates a single @@ -41,46 +42,8 @@ func (p *WrapperTracerProvider) Tracer(name string, opts ...trace.TracerOption) // // Deprecated: Use NewTracerProvider(...) instead. func NewWrappedTracerProvider(bridge *BridgeTracer, tracer trace.Tracer) *WrapperTracerProvider { - wTracer := NewWrapperTracer(bridge, tracer) return &WrapperTracerProvider{ - getWrappedTracer: func(_ string, _ ...trace.TracerOption) *WrapperTracer { - return wTracer - }, - } -} - -type wrappedTracerKey struct { - name string - version string -} - -// NewTracerProvider returns a new TracerProvider that creates new instances of -// WrapperTracer that wraps OpenTelemetry tracer for each call to Tracer(). -func NewTracerProvider(bridge *BridgeTracer, provider trace.TracerProvider) *WrapperTracerProvider { - var ( - mtx sync.Mutex - tracers = make(map[wrappedTracerKey]*WrapperTracer) - ) - - return &WrapperTracerProvider{ - getWrappedTracer: func(name string, opts ...trace.TracerOption) *WrapperTracer { - mtx.Lock() - defer mtx.Unlock() - - c := trace.NewTracerConfig(opts...) - key := wrappedTracerKey{ - name: name, - version: c.InstrumentationVersion(), - } - - if t, ok := tracers[key]; ok { - return t - } - - wrapper := NewWrapperTracer(bridge, provider.Tracer(name, opts...)) - tracers[key] = wrapper - return wrapper - }, + wTracer: NewWrapperTracer(bridge, tracer), } } From 4d385da66e9562bbd6fa17aa69c626f39fe830cd Mon Sep 17 00:00:00 2001 From: Robert Lin Date: Tue, 10 Jan 2023 21:21:08 -0800 Subject: [PATCH 08/10] update docstring --- bridge/opentracing/provider.go | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/bridge/opentracing/provider.go b/bridge/opentracing/provider.go index aefcf5127c0..74c7b6a40b7 100644 --- a/bridge/opentracing/provider.go +++ b/bridge/opentracing/provider.go @@ -33,11 +33,7 @@ type TracerProvider struct { var _ trace.TracerProvider = (*TracerProvider)(nil) // NewTracerProvider returns a new TracerProvider that creates new instances of -// WrapperTracer that wraps OpenTelemetry tracer for each call to Tracer() with -// new configuration. -// -// Repeated calls to Tracer() with the same configuration will look up and return -// an existing instance of WrapperTracer. +// WrapperTracer from the given TracerProvider. func NewTracerProvider(bridge *BridgeTracer, provider trace.TracerProvider) *TracerProvider { return &TracerProvider{ bridge: bridge, @@ -52,6 +48,9 @@ type wrappedTracerKey struct { version string } +// Tracer creates a WrappedTracer that wraps the OpenTelemetry tracer for each call to +// Tracer(). Repeated calls to Tracer() with the same configuration will look up and +// return an existing instance of WrapperTracer. func (p *TracerProvider) Tracer(name string, opts ...trace.TracerOption) trace.Tracer { p.mtx.Lock() defer p.mtx.Unlock() From 3345f1243a8ce6f309949e9ff663c7953d74f72f Mon Sep 17 00:00:00 2001 From: Robert Lin Date: Tue, 10 Jan 2023 21:24:45 -0800 Subject: [PATCH 09/10] add changelog entries --- CHANGELOG.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 33d3cf58bde..4bc79e6543d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -49,6 +49,7 @@ This project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.htm - `Int64Counter` replaces the `syncint64.Counter` - `Int64UpDownCounter` replaces the `syncint64.UpDownCounter` - `Int64Histogram` replaces the `syncint64.Histogram` +- Add `NewTracerProvider` to `go.opentelemetry.io/otel/bridge/opentracing` to create `WrapperTracer` instances from a `TracerProvider`. (#3316) ### Changed @@ -111,6 +112,7 @@ This project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.htm Use the instruments from `go.opentelemetry.io/otel/metric/instrument` instead. (#3575) - The `go.opentelemetry.io/otel/metric/instrument/syncint64` package is deprecated. Use the instruments from `go.opentelemetry.io/otel/metric/instrument` instead. (#3575) +- The `NewWrappedTracerProvider` in `go.opentelemetry.io/otel/bridge/opentracing` is now deprecated. Use `NewTracerProvider` instead. (#3316) ### Removed From a0d303fedf35e2bebe72dab89b2b3e22bb182875 Mon Sep 17 00:00:00 2001 From: Robert Lin Date: Wed, 11 Jan 2023 08:26:39 -0800 Subject: [PATCH 10/10] Update bridge/opentracing/provider.go Co-authored-by: Damien Mathieu <42@dmathieu.com> --- bridge/opentracing/provider.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/bridge/opentracing/provider.go b/bridge/opentracing/provider.go index 74c7b6a40b7..941e277baf8 100644 --- a/bridge/opentracing/provider.go +++ b/bridge/opentracing/provider.go @@ -12,7 +12,7 @@ // See the License for the specific language governing permissions and // limitations under the License. -package opentracing +package opentracing // import "go.opentelemetry.io/otel/bridge/opentracing" import ( "sync"