Skip to content

Commit 9838bba

Browse files
authored
Allow non-comparable global types (#2773)
* Fix #2772: allow non-comparable global types The global MeterProvider, TracerProvider, and TextMapPropagator should not panic when they are set to a non-comparable implementation of each. * Add changes to changelog * No lint unused field for testing
1 parent 376c23c commit 9838bba

5 files changed

Lines changed: 78 additions & 24 deletions

File tree

CHANGELOG.md

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,10 @@ This project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.htm
2525
- Upgrade `go.opentelemetry.io/proto/otlp` in `go.opentelemetry.io/otel/exporters/otlp/otlpmetric` from `v0.12.1` to `v0.15.0`.
2626
This replaces the use of the now deprecated `InstrumentationLibrary` and `InstrumentationLibraryMetrics` types and fields in the proto library with the equivalent `InstrumentationScope` and `ScopeMetrics`. (#2748)
2727

28+
### Fixed
29+
30+
- Allow non-comparable global `MeterProvider`, `TracerProvider`, and `TextMapPropagator` types to be set. (#2772, #2773)
31+
2832
## [1.6.2] - 2022-04-06
2933

3034
### Changed

internal/global/state.go

Lines changed: 22 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -50,14 +50,17 @@ func TracerProvider() trace.TracerProvider {
5050
// SetTracerProvider is the internal implementation for global.SetTracerProvider.
5151
func SetTracerProvider(tp trace.TracerProvider) {
5252
current := TracerProvider()
53-
if current == tp {
54-
// Setting the provider to the prior default results in a noop. Return
55-
// early.
56-
Error(
57-
errors.New("no delegate configured in tracer provider"),
58-
"Setting tracer provider to it's current value. No delegate will be configured",
59-
)
60-
return
53+
54+
if _, cOk := current.(*tracerProvider); cOk {
55+
if _, tpOk := tp.(*tracerProvider); tpOk && current == tp {
56+
// Do not assign the default delegating TracerProvider to delegate
57+
// to itself.
58+
Error(
59+
errors.New("no delegate configured in tracer provider"),
60+
"Setting tracer provider to it's current value. No delegate will be configured",
61+
)
62+
return
63+
}
6164
}
6265

6366
delegateTraceOnce.Do(func() {
@@ -76,14 +79,17 @@ func TextMapPropagator() propagation.TextMapPropagator {
7679
// SetTextMapPropagator is the internal implementation for global.SetTextMapPropagator.
7780
func SetTextMapPropagator(p propagation.TextMapPropagator) {
7881
current := TextMapPropagator()
79-
if current == p {
80-
// Setting the provider to the prior default results in a noop. Return
81-
// early.
82-
Error(
83-
errors.New("no delegate configured in text map propagator"),
84-
"Setting text map propagator to it's current value. No delegate will be configured",
85-
)
86-
return
82+
83+
if _, cOk := current.(*textMapPropagator); cOk {
84+
if _, pOk := p.(*textMapPropagator); pOk && current == p {
85+
// Do not assign the default delegating TextMapPropagator to
86+
// delegate to itself.
87+
Error(
88+
errors.New("no delegate configured in text map propagator"),
89+
"Setting text map propagator to it's current value. No delegate will be configured",
90+
)
91+
return
92+
}
8793
}
8894

8995
// For the textMapPropagator already returned by TextMapPropagator

internal/global/state_test.go

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -17,10 +17,18 @@ package global
1717
import (
1818
"testing"
1919

20+
"github.com/stretchr/testify/assert"
21+
2022
"go.opentelemetry.io/otel/propagation"
2123
"go.opentelemetry.io/otel/trace"
2224
)
2325

26+
type nonComparableTracerProvider struct {
27+
trace.TracerProvider
28+
29+
nonComparable func() //nolint:structcheck,unused // This is not called.
30+
}
31+
2432
func TestSetTracerProvider(t *testing.T) {
2533
t.Run("Set With default is a noop", func(t *testing.T) {
2634
ResetForTest(t)
@@ -59,6 +67,14 @@ func TestSetTracerProvider(t *testing.T) {
5967
t.Fatal("The delegated tracer providers should have a delegate")
6068
}
6169
})
70+
71+
t.Run("non-comparable types should not panic", func(t *testing.T) {
72+
ResetForTest(t)
73+
74+
tp := nonComparableTracerProvider{}
75+
SetTracerProvider(tp)
76+
assert.NotPanics(t, func() { SetTracerProvider(tp) })
77+
})
6278
}
6379

6480
func TestSetTextMapPropagator(t *testing.T) {
@@ -99,4 +115,13 @@ func TestSetTextMapPropagator(t *testing.T) {
99115
t.Fatal("The delegated TextMapPropagators should have a delegate")
100116
}
101117
})
118+
119+
t.Run("non-comparable types should not panic", func(t *testing.T) {
120+
ResetForTest(t)
121+
122+
// A composite TextMapPropagator is not comparable.
123+
prop := propagation.NewCompositeTextMapPropagator(propagation.TraceContext{})
124+
SetTextMapPropagator(prop)
125+
assert.NotPanics(t, func() { SetTextMapPropagator(prop) })
126+
})
102127
}

metric/internal/global/state.go

Lines changed: 10 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -41,14 +41,16 @@ func MeterProvider() metric.MeterProvider {
4141
// SetMeterProvider is the internal implementation for global.SetMeterProvider.
4242
func SetMeterProvider(mp metric.MeterProvider) {
4343
current := MeterProvider()
44-
if current == mp {
45-
// Setting the provider to the prior default results in a noop. Return
46-
// early.
47-
global.Error(
48-
errors.New("no delegate configured in meter provider"),
49-
"Setting meter provider to it's current value. No delegate will be configured",
50-
)
51-
return
44+
if _, cOk := current.(*meterProvider); cOk {
45+
if _, mpOk := mp.(*meterProvider); mpOk && current == mp {
46+
// Do not assign the default delegating MeterProvider to delegate
47+
// to itself.
48+
global.Error(
49+
errors.New("no delegate configured in meter provider"),
50+
"Setting meter provider to it's current value. No delegate will be configured",
51+
)
52+
return
53+
}
5254
}
5355

5456
delegateMeterOnce.Do(func() {

metric/internal/global/state_test.go

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,9 @@ import (
1818
"sync"
1919
"testing"
2020

21+
"github.com/stretchr/testify/assert"
22+
23+
"go.opentelemetry.io/otel/metric"
2124
"go.opentelemetry.io/otel/metric/nonrecording"
2225
)
2326

@@ -26,6 +29,12 @@ func resetGlobalMeterProvider() {
2629
delegateMeterOnce = sync.Once{}
2730
}
2831

32+
type nonComparableMeterProvider struct {
33+
metric.MeterProvider
34+
35+
nonComparable func() //nolint:structcheck,unused // This is not called.
36+
}
37+
2938
func TestSetMeterProvider(t *testing.T) {
3039
t.Cleanup(resetGlobalMeterProvider)
3140

@@ -67,4 +76,12 @@ func TestSetMeterProvider(t *testing.T) {
6776
t.Fatal("The delegated meter providers should have a delegate")
6877
}
6978
})
79+
80+
t.Run("non-comparable types should not panic", func(t *testing.T) {
81+
resetGlobalMeterProvider()
82+
83+
mp := nonComparableMeterProvider{}
84+
SetMeterProvider(mp)
85+
assert.NotPanics(t, func() { SetMeterProvider(mp) })
86+
})
7087
}

0 commit comments

Comments
 (0)