Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Allow non-comparable global types #2773

Merged
merged 4 commits into from Apr 7, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
4 changes: 4 additions & 0 deletions CHANGELOG.md
Expand Up @@ -25,6 +25,10 @@ This project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.htm
- Upgrade `go.opentelemetry.io/proto/otlp` in `go.opentelemetry.io/otel/exporters/otlp/otlpmetric` from `v0.12.1` to `v0.15.0`.
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)

### Fixed

- Allow non-comparable global `MeterProvider`, `TracerProvider`, and `TextMapPropagator` types to be set. (#2772, #2773)

## [1.6.2] - 2022-04-06

### Changed
Expand Down
38 changes: 22 additions & 16 deletions internal/global/state.go
Expand Up @@ -50,14 +50,17 @@ func TracerProvider() trace.TracerProvider {
// SetTracerProvider is the internal implementation for global.SetTracerProvider.
func SetTracerProvider(tp trace.TracerProvider) {
current := TracerProvider()
if current == tp {
// Setting the provider to the prior default results in a noop. Return
// early.
Error(
errors.New("no delegate configured in tracer provider"),
"Setting tracer provider to it's current value. No delegate will be configured",
)
return

if _, cOk := current.(*tracerProvider); cOk {
if _, tpOk := tp.(*tracerProvider); tpOk && current == tp {
// Do not assign the default delegating TracerProvider to delegate
// to itself.
Error(
errors.New("no delegate configured in tracer provider"),
"Setting tracer provider to it's current value. No delegate will be configured",
)
return
}
}

delegateTraceOnce.Do(func() {
Expand All @@ -76,14 +79,17 @@ func TextMapPropagator() propagation.TextMapPropagator {
// SetTextMapPropagator is the internal implementation for global.SetTextMapPropagator.
func SetTextMapPropagator(p propagation.TextMapPropagator) {
current := TextMapPropagator()
if current == p {
// Setting the provider to the prior default results in a noop. Return
// early.
Error(
errors.New("no delegate configured in text map propagator"),
"Setting text map propagator to it's current value. No delegate will be configured",
)
return

if _, cOk := current.(*textMapPropagator); cOk {
if _, pOk := p.(*textMapPropagator); pOk && current == p {
// Do not assign the default delegating TextMapPropagator to
// delegate to itself.
Error(
errors.New("no delegate configured in text map propagator"),
"Setting text map propagator to it's current value. No delegate will be configured",
)
return
}
}

// For the textMapPropagator already returned by TextMapPropagator
Expand Down
25 changes: 25 additions & 0 deletions internal/global/state_test.go
Expand Up @@ -17,10 +17,18 @@ package global
import (
"testing"

"github.com/stretchr/testify/assert"

"go.opentelemetry.io/otel/propagation"
"go.opentelemetry.io/otel/trace"
)

type nonComparableTracerProvider struct {
trace.TracerProvider

nonComparable func() //nolint:structcheck,unused // This is not called.
}

func TestSetTracerProvider(t *testing.T) {
t.Run("Set With default is a noop", func(t *testing.T) {
ResetForTest(t)
Expand Down Expand Up @@ -59,6 +67,14 @@ func TestSetTracerProvider(t *testing.T) {
t.Fatal("The delegated tracer providers should have a delegate")
}
})

t.Run("non-comparable types should not panic", func(t *testing.T) {
ResetForTest(t)

tp := nonComparableTracerProvider{}
SetTracerProvider(tp)
assert.NotPanics(t, func() { SetTracerProvider(tp) })
})
}

func TestSetTextMapPropagator(t *testing.T) {
Expand Down Expand Up @@ -99,4 +115,13 @@ func TestSetTextMapPropagator(t *testing.T) {
t.Fatal("The delegated TextMapPropagators should have a delegate")
}
})

t.Run("non-comparable types should not panic", func(t *testing.T) {
ResetForTest(t)

// A composite TextMapPropagator is not comparable.
prop := propagation.NewCompositeTextMapPropagator(propagation.TraceContext{})
SetTextMapPropagator(prop)
assert.NotPanics(t, func() { SetTextMapPropagator(prop) })
})
}
18 changes: 10 additions & 8 deletions metric/internal/global/state.go
Expand Up @@ -41,14 +41,16 @@ func MeterProvider() metric.MeterProvider {
// SetMeterProvider is the internal implementation for global.SetMeterProvider.
func SetMeterProvider(mp metric.MeterProvider) {
current := MeterProvider()
if current == mp {
// Setting the provider to the prior default results in a noop. Return
// early.
global.Error(
errors.New("no delegate configured in meter provider"),
"Setting meter provider to it's current value. No delegate will be configured",
)
return
if _, cOk := current.(*meterProvider); cOk {
if _, mpOk := mp.(*meterProvider); mpOk && current == mp {
// Do not assign the default delegating MeterProvider to delegate
// to itself.
global.Error(
errors.New("no delegate configured in meter provider"),
"Setting meter provider to it's current value. No delegate will be configured",
)
return
}
}

delegateMeterOnce.Do(func() {
Expand Down
17 changes: 17 additions & 0 deletions metric/internal/global/state_test.go
Expand Up @@ -18,6 +18,9 @@ import (
"sync"
"testing"

"github.com/stretchr/testify/assert"

"go.opentelemetry.io/otel/metric"
"go.opentelemetry.io/otel/metric/nonrecording"
)

Expand All @@ -26,6 +29,12 @@ func resetGlobalMeterProvider() {
delegateMeterOnce = sync.Once{}
}

type nonComparableMeterProvider struct {
metric.MeterProvider

nonComparable func() //nolint:structcheck,unused // This is not called.
}

func TestSetMeterProvider(t *testing.T) {
t.Cleanup(resetGlobalMeterProvider)

Expand Down Expand Up @@ -67,4 +76,12 @@ func TestSetMeterProvider(t *testing.T) {
t.Fatal("The delegated meter providers should have a delegate")
}
})

t.Run("non-comparable types should not panic", func(t *testing.T) {
resetGlobalMeterProvider()

mp := nonComparableMeterProvider{}
SetMeterProvider(mp)
assert.NotPanics(t, func() { SetMeterProvider(mp) })
})
}