Fix delegation for global MeterProviders (#5828)

Fixes #5827 Fixes #5852 --------- Co-authored-by: Robert Pająk <pellared@hotmail.com> Co-authored-by: David Ashpole <dashpole@google.com> Co-authored-by: Damien Mathieu <42@dmathieu.com>
open-telemetry · Oct 4, 2024 · 1333b2f · 1333b2f
1 parent 19877b1
commit 1333b2f
Show file tree

Hide file tree

Showing 4 changed files with 128 additions and 28 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -28,6 +28,8 @@ This project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.htm
 - The race condition for multiple `FixedSize` exemplar reservoirs identified in #5814 is resolved. (#5819)
 - Fix log records duplication in case of heterogeneous resource attributes by correctly mapping each log record to it's resource and scope. (#5803)
 - Fix timer channel drain to avoid hanging on Go 1.23. (#5868)
+- Fix delegation for global meter providers. (#5827)
+  Change the `reflect.TypeOf` to use a nil pointer to not allocate on the heap unless necessary.
 
 <!-- Released section -->
 <!-- Don't change this section unless doing release -->

diff --git a/internal/global/meter.go b/internal/global/meter.go
@@ -152,14 +152,17 @@ func (m *meter) Int64Counter(name string, options ...metric.Int64CounterOption)
 		return m.delegate.Int64Counter(name, options...)
 	}
 
-	i := &siCounter{name: name, opts: options}
 	cfg := metric.NewInt64CounterConfig(options...)
 	id := instID{
 		name:        name,
-		kind:        reflect.TypeOf(i),
+		kind:        reflect.TypeOf((*siCounter)(nil)),
 		description: cfg.Description(),
 		unit:        cfg.Unit(),
 	}
+	if f, ok := m.instruments[id]; ok {
+		return f.(metric.Int64Counter), nil
+	}
+	i := &siCounter{name: name, opts: options}
 	m.instruments[id] = i
 	return i, nil
 }
@@ -172,14 +175,17 @@ func (m *meter) Int64UpDownCounter(name string, options ...metric.Int64UpDownCou
 		return m.delegate.Int64UpDownCounter(name, options...)
 	}
 
-	i := &siUpDownCounter{name: name, opts: options}
 	cfg := metric.NewInt64UpDownCounterConfig(options...)
 	id := instID{
 		name:        name,
-		kind:        reflect.TypeOf(i),
+		kind:        reflect.TypeOf((*siUpDownCounter)(nil)),
 		description: cfg.Description(),
 		unit:        cfg.Unit(),
 	}
+	if f, ok := m.instruments[id]; ok {
+		return f.(metric.Int64UpDownCounter), nil
+	}
+	i := &siUpDownCounter{name: name, opts: options}
 	m.instruments[id] = i
 	return i, nil
 }
@@ -192,14 +198,17 @@ func (m *meter) Int64Histogram(name string, options ...metric.Int64HistogramOpti
 		return m.delegate.Int64Histogram(name, options...)
 	}
 
-	i := &siHistogram{name: name, opts: options}
 	cfg := metric.NewInt64HistogramConfig(options...)
 	id := instID{
 		name:        name,
-		kind:        reflect.TypeOf(i),
+		kind:        reflect.TypeOf((*siHistogram)(nil)),
 		description: cfg.Description(),
 		unit:        cfg.Unit(),
 	}
+	if f, ok := m.instruments[id]; ok {
+		return f.(metric.Int64Histogram), nil
+	}
+	i := &siHistogram{name: name, opts: options}
 	m.instruments[id] = i
 	return i, nil
 }
@@ -212,14 +221,17 @@ func (m *meter) Int64Gauge(name string, options ...metric.Int64GaugeOption) (met
 		return m.delegate.Int64Gauge(name, options...)
 	}
 
-	i := &siGauge{name: name, opts: options}
 	cfg := metric.NewInt64GaugeConfig(options...)
 	id := instID{
 		name:        name,
-		kind:        reflect.TypeOf(i),
+		kind:        reflect.TypeOf((*siGauge)(nil)),
 		description: cfg.Description(),
 		unit:        cfg.Unit(),
 	}
+	if f, ok := m.instruments[id]; ok {
+		return f.(metric.Int64Gauge), nil
+	}
+	i := &siGauge{name: name, opts: options}
 	m.instruments[id] = i
 	return i, nil
 }
@@ -232,14 +244,17 @@ func (m *meter) Int64ObservableCounter(name string, options ...metric.Int64Obser
 		return m.delegate.Int64ObservableCounter(name, options...)
 	}
 
-	i := &aiCounter{name: name, opts: options}
 	cfg := metric.NewInt64ObservableCounterConfig(options...)
 	id := instID{
 		name:        name,
-		kind:        reflect.TypeOf(i),
+		kind:        reflect.TypeOf((*aiCounter)(nil)),
 		description: cfg.Description(),
 		unit:        cfg.Unit(),
 	}
+	if f, ok := m.instruments[id]; ok {
+		return f.(metric.Int64ObservableCounter), nil
+	}
+	i := &aiCounter{name: name, opts: options}
 	m.instruments[id] = i
 	return i, nil
 }
@@ -252,14 +267,17 @@ func (m *meter) Int64ObservableUpDownCounter(name string, options ...metric.Int6
 		return m.delegate.Int64ObservableUpDownCounter(name, options...)
 	}
 
-	i := &aiUpDownCounter{name: name, opts: options}
 	cfg := metric.NewInt64ObservableUpDownCounterConfig(options...)
 	id := instID{
 		name:        name,
-		kind:        reflect.TypeOf(i),
+		kind:        reflect.TypeOf((*aiUpDownCounter)(nil)),
 		description: cfg.Description(),
 		unit:        cfg.Unit(),
 	}
+	if f, ok := m.instruments[id]; ok {
+		return f.(metric.Int64ObservableUpDownCounter), nil
+	}
+	i := &aiUpDownCounter{name: name, opts: options}
 	m.instruments[id] = i
 	return i, nil
 }
@@ -272,14 +290,17 @@ func (m *meter) Int64ObservableGauge(name string, options ...metric.Int64Observa
 		return m.delegate.Int64ObservableGauge(name, options...)
 	}
 
-	i := &aiGauge{name: name, opts: options}
 	cfg := metric.NewInt64ObservableGaugeConfig(options...)
 	id := instID{
 		name:        name,
-		kind:        reflect.TypeOf(i),
+		kind:        reflect.TypeOf((*aiGauge)(nil)),
 		description: cfg.Description(),
 		unit:        cfg.Unit(),
 	}
+	if f, ok := m.instruments[id]; ok {
+		return f.(metric.Int64ObservableGauge), nil
+	}
+	i := &aiGauge{name: name, opts: options}
 	m.instruments[id] = i
 	return i, nil
 }
@@ -292,14 +313,17 @@ func (m *meter) Float64Counter(name string, options ...metric.Float64CounterOpti
 		return m.delegate.Float64Counter(name, options...)
 	}
 
-	i := &sfCounter{name: name, opts: options}
 	cfg := metric.NewFloat64CounterConfig(options...)
 	id := instID{
 		name:        name,
-		kind:        reflect.TypeOf(i),
+		kind:        reflect.TypeOf((*sfCounter)(nil)),
 		description: cfg.Description(),
 		unit:        cfg.Unit(),
 	}
+	if f, ok := m.instruments[id]; ok {
+		return f.(metric.Float64Counter), nil
+	}
+	i := &sfCounter{name: name, opts: options}
 	m.instruments[id] = i
 	return i, nil
 }
@@ -312,14 +336,17 @@ func (m *meter) Float64UpDownCounter(name string, options ...metric.Float64UpDow
 		return m.delegate.Float64UpDownCounter(name, options...)
 	}
 
-	i := &sfUpDownCounter{name: name, opts: options}
 	cfg := metric.NewFloat64UpDownCounterConfig(options...)
 	id := instID{
 		name:        name,
-		kind:        reflect.TypeOf(i),
+		kind:        reflect.TypeOf((*sfUpDownCounter)(nil)),
 		description: cfg.Description(),
 		unit:        cfg.Unit(),
 	}
+	if f, ok := m.instruments[id]; ok {
+		return f.(metric.Float64UpDownCounter), nil
+	}
+	i := &sfUpDownCounter{name: name, opts: options}
 	m.instruments[id] = i
 	return i, nil
 }
@@ -332,14 +359,17 @@ func (m *meter) Float64Histogram(name string, options ...metric.Float64Histogram
 		return m.delegate.Float64Histogram(name, options...)
 	}
 
-	i := &sfHistogram{name: name, opts: options}
 	cfg := metric.NewFloat64HistogramConfig(options...)
 	id := instID{
 		name:        name,
-		kind:        reflect.TypeOf(i),
+		kind:        reflect.TypeOf((*sfHistogram)(nil)),
 		description: cfg.Description(),
 		unit:        cfg.Unit(),
 	}
+	if f, ok := m.instruments[id]; ok {
+		return f.(metric.Float64Histogram), nil
+	}
+	i := &sfHistogram{name: name, opts: options}
 	m.instruments[id] = i
 	return i, nil
 }
@@ -352,14 +382,17 @@ func (m *meter) Float64Gauge(name string, options ...metric.Float64GaugeOption)
 		return m.delegate.Float64Gauge(name, options...)
 	}
 
-	i := &sfGauge{name: name, opts: options}
 	cfg := metric.NewFloat64GaugeConfig(options...)
 	id := instID{
 		name:        name,
-		kind:        reflect.TypeOf(i),
+		kind:        reflect.TypeOf((*sfGauge)(nil)),
 		description: cfg.Description(),
 		unit:        cfg.Unit(),
 	}
+	if f, ok := m.instruments[id]; ok {
+		return f.(metric.Float64Gauge), nil
+	}
+	i := &sfGauge{name: name, opts: options}
 	m.instruments[id] = i
 	return i, nil
 }
@@ -372,14 +405,17 @@ func (m *meter) Float64ObservableCounter(name string, options ...metric.Float64O
 		return m.delegate.Float64ObservableCounter(name, options...)
 	}
 
-	i := &afCounter{name: name, opts: options}
 	cfg := metric.NewFloat64ObservableCounterConfig(options...)
 	id := instID{
 		name:        name,
-		kind:        reflect.TypeOf(i),
+		kind:        reflect.TypeOf((*afCounter)(nil)),
 		description: cfg.Description(),
 		unit:        cfg.Unit(),
 	}
+	if f, ok := m.instruments[id]; ok {
+		return f.(metric.Float64ObservableCounter), nil
+	}
+	i := &afCounter{name: name, opts: options}
 	m.instruments[id] = i
 	return i, nil
 }
@@ -392,14 +428,17 @@ func (m *meter) Float64ObservableUpDownCounter(name string, options ...metric.Fl
 		return m.delegate.Float64ObservableUpDownCounter(name, options...)
 	}
 
-	i := &afUpDownCounter{name: name, opts: options}
 	cfg := metric.NewFloat64ObservableUpDownCounterConfig(options...)
 	id := instID{
 		name:        name,
-		kind:        reflect.TypeOf(i),
+		kind:        reflect.TypeOf((*afUpDownCounter)(nil)),
 		description: cfg.Description(),
 		unit:        cfg.Unit(),
 	}
+	if f, ok := m.instruments[id]; ok {
+		return f.(metric.Float64ObservableUpDownCounter), nil
+	}
+	i := &afUpDownCounter{name: name, opts: options}
 	m.instruments[id] = i
 	return i, nil
 }
@@ -412,14 +451,17 @@ func (m *meter) Float64ObservableGauge(name string, options ...metric.Float64Obs
 		return m.delegate.Float64ObservableGauge(name, options...)
 	}
 
-	i := &afGauge{name: name, opts: options}
 	cfg := metric.NewFloat64ObservableGaugeConfig(options...)
 	id := instID{
 		name:        name,
-		kind:        reflect.TypeOf(i),
+		kind:        reflect.TypeOf((*afGauge)(nil)),
 		description: cfg.Description(),
 		unit:        cfg.Unit(),
 	}
+	if f, ok := m.instruments[id]; ok {
+		return f.(metric.Float64ObservableGauge), nil
+	}
+	i := &afGauge{name: name, opts: options}
 	m.instruments[id] = i
 	return i, nil
 }
@@ -487,6 +529,7 @@ func (c *registration) setDelegate(m metric.Meter) {
 	reg, err := m.RegisterCallback(c.function, insts...)
 	if err != nil {
 		GetErrorHandler().Handle(err)
+		return
 	}
 
 	c.unreg = reg.Unregister

diff --git a/internal/global/meter_test.go b/internal/global/meter_test.go
@@ -5,6 +5,7 @@ package global // import "go.opentelemetry.io/otel/internal/global"
 
 import (
 	"context"
+	"errors"
 	"fmt"
 	"sync"
 	"testing"
@@ -430,3 +431,22 @@ func TestMeterIdentity(t *testing.T) {
 		}
 	}
 }
+
+type failingRegisterCallbackMeter struct {
+	noop.Meter
+}
+
+func (m *failingRegisterCallbackMeter) RegisterCallback(metric.Callback, ...metric.Observable) (metric.Registration, error) {
+	return nil, errors.New("an error occurred")
+}
+
+func TestRegistrationDelegateFailingCallback(t *testing.T) {
+	r := &registration{
+		unreg: func() error { return nil },
+	}
+	m := &failingRegisterCallbackMeter{}
+
+	assert.NotPanics(t, func() {
+		r.setDelegate(m)
+	})
+}
diff --git a/sdk/metric/meter_test.go b/sdk/metric/meter_test.go
@@ -2427,3 +2427,38 @@ func TestDuplicateInstrumentCreation(t *testing.T) {
 		})
 	}
 }
+
+func TestMeterProviderDelegation(t *testing.T) {
+	meter := otel.Meter("go.opentelemetry.io/otel/metric/internal/global/meter_test")
+	otel.SetErrorHandler(otel.ErrorHandlerFunc(func(err error) { require.NoError(t, err) }))
+	for i := 0; i < 5; i++ {
+		int64Counter, err := meter.Int64ObservableCounter("observable.int64.counter")
+		require.NoError(t, err)
+		int64UpDownCounter, err := meter.Int64ObservableUpDownCounter("observable.int64.up.down.counter")
+		require.NoError(t, err)
+		int64Gauge, err := meter.Int64ObservableGauge("observable.int64.gauge")
+		require.NoError(t, err)
+		floatCounter, err := meter.Float64ObservableCounter("observable.float.counter")
+		require.NoError(t, err)
+		floatUpDownCounter, err := meter.Float64ObservableUpDownCounter("observable.float.up.down.counter")
+		require.NoError(t, err)
+		floatGauge, err := meter.Float64ObservableGauge("observable.float.gauge")
+		require.NoError(t, err)
+		_, err = meter.RegisterCallback(func(ctx context.Context, o metric.Observer) error {
+			o.ObserveInt64(int64Counter, int64(10))
+			o.ObserveInt64(int64UpDownCounter, int64(10))
+			o.ObserveInt64(int64Gauge, int64(10))
+
+			o.ObserveFloat64(floatCounter, float64(10))
+			o.ObserveFloat64(floatUpDownCounter, float64(10))
+			o.ObserveFloat64(floatGauge, float64(10))
+			return nil
+		}, int64Counter, int64UpDownCounter, int64Gauge, floatCounter, floatUpDownCounter, floatGauge)
+		require.NoError(t, err)
+	}
+	provider := NewMeterProvider()
+
+	assert.NotPanics(t, func() {
+		otel.SetMeterProvider(provider)
+	})
+}