From 1333b2f73acc30139c0903be1c95eedf25802670 Mon Sep 17 00:00:00 2001 From: Jesse Bank <159821964+Jesse-Bonfire@users.noreply.github.com> Date: Fri, 4 Oct 2024 01:19:41 -0700 Subject: [PATCH] Fix delegation for global MeterProviders (#5828) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Fixes #5827 Fixes #5852 --------- Co-authored-by: Robert Pająk Co-authored-by: David Ashpole Co-authored-by: Damien Mathieu <42@dmathieu.com> --- CHANGELOG.md | 2 + internal/global/meter.go | 99 +++++++++++++++++++++++++---------- internal/global/meter_test.go | 20 +++++++ sdk/metric/meter_test.go | 35 +++++++++++++ 4 files changed, 128 insertions(+), 28 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index b52b4749f2e..56f4b33e670 100644 --- 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. diff --git a/internal/global/meter.go b/internal/global/meter.go index f2fc3929b11..e3db438a09f 100644 --- 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 index aec31e1e92c..3a700924e34 100644 --- 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 := ®istration{ + 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 index e79ef030bf4..906182ad13d 100644 --- 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) + }) +}