From 11ebd19c46c39896a790db15825d7c769c52577f Mon Sep 17 00:00:00 2001 From: Tyler Yahn Date: Wed, 7 Feb 2024 11:59:58 -0800 Subject: [PATCH] Fix callback registration bug (#4888) * Fix cback reg bug Register all callbacks not just the last passed. * Add changelog entry --- CHANGELOG.md | 4 ++ sdk/metric/meter.go | 6 +- sdk/metric/meter_test.go | 128 +++++++++++++++++++++++++++------------ 3 files changed, 98 insertions(+), 40 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index ea1f723e264..c7d94884622 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,6 +8,10 @@ This project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.htm ## [Unreleased] +### Fixed + +- Register all callbacks passed during observable instrument creation instead of just the last one multiple times in `go.opentelemetry.io/otel/sdk/metric`. (#4888) + ## [1.23.0] 2024-02-06 This release contains the first stable, `v1`, release of the following modules: diff --git a/sdk/metric/meter.go b/sdk/metric/meter.go index 88710563517..beb7876ec40 100644 --- a/sdk/metric/meter.go +++ b/sdk/metric/meter.go @@ -148,7 +148,8 @@ func (m *meter) int64ObservableInstrument(id Instrument, callbacks []metric.Int6 inst.appendMeasures(in) for _, cback := range callbacks { inst := int64Observer{measures: in} - insert.addCallback(func(ctx context.Context) error { return cback(ctx, inst) }) + fn := cback + insert.addCallback(func(ctx context.Context) error { return fn(ctx, inst) }) } } return inst, validateInstrumentName(id.Name) @@ -281,7 +282,8 @@ func (m *meter) float64ObservableInstrument(id Instrument, callbacks []metric.Fl inst.appendMeasures(in) for _, cback := range callbacks { inst := float64Observer{measures: in} - insert.addCallback(func(ctx context.Context) error { return cback(ctx, inst) }) + fn := cback + insert.addCallback(func(ctx context.Context) error { return fn(ctx, inst) }) } } return inst, validateInstrumentName(id.Name) diff --git a/sdk/metric/meter_test.go b/sdk/metric/meter_test.go index 037f0cad555..5bec7c2ce38 100644 --- a/sdk/metric/meter_test.go +++ b/sdk/metric/meter_test.go @@ -174,8 +174,18 @@ func TestMeterCreatesInstruments(t *testing.T) { ctx, cancel := context.WithCancel(context.Background()) cancel() - attrs := attribute.NewSet(attribute.String("name", "alice")) - opt := metric.WithAttributeSet(attrs) + alice := attribute.NewSet( + attribute.String("name", "Alice"), + attribute.Bool("admin", true), + ) + optAlice := metric.WithAttributeSet(alice) + + bob := attribute.NewSet( + attribute.String("name", "Bob"), + attribute.Bool("admin", false), + ) + optBob := metric.WithAttributeSet(bob) + testCases := []struct { name string fn func(*testing.T, metric.Meter) @@ -184,11 +194,17 @@ func TestMeterCreatesInstruments(t *testing.T) { { name: "ObservableInt64Count", fn: func(t *testing.T, m metric.Meter) { - cback := func(_ context.Context, o metric.Int64Observer) error { - o.Observe(4, opt) - return nil - } - ctr, err := m.Int64ObservableCounter("aint", metric.WithInt64Callback(cback)) + ctr, err := m.Int64ObservableCounter( + "aint", + metric.WithInt64Callback(func(_ context.Context, o metric.Int64Observer) error { + o.Observe(4, optAlice) + return nil + }), + metric.WithInt64Callback(func(_ context.Context, o metric.Int64Observer) error { + o.Observe(5, optBob) + return nil + }), + ) assert.NoError(t, err) _, err = m.RegisterCallback(func(_ context.Context, o metric.Observer) error { o.ObserveInt64(ctr, 3) @@ -202,7 +218,8 @@ func TestMeterCreatesInstruments(t *testing.T) { Temporality: metricdata.CumulativeTemporality, IsMonotonic: true, DataPoints: []metricdata.DataPoint[int64]{ - {Attributes: attrs, Value: 4}, + {Attributes: alice, Value: 4}, + {Attributes: bob, Value: 5}, {Value: 3}, }, }, @@ -211,11 +228,17 @@ func TestMeterCreatesInstruments(t *testing.T) { { name: "ObservableInt64UpDownCount", fn: func(t *testing.T, m metric.Meter) { - cback := func(_ context.Context, o metric.Int64Observer) error { - o.Observe(4, opt) - return nil - } - ctr, err := m.Int64ObservableUpDownCounter("aint", metric.WithInt64Callback(cback)) + ctr, err := m.Int64ObservableUpDownCounter( + "aint", + metric.WithInt64Callback(func(_ context.Context, o metric.Int64Observer) error { + o.Observe(4, optAlice) + return nil + }), + metric.WithInt64Callback(func(_ context.Context, o metric.Int64Observer) error { + o.Observe(5, optBob) + return nil + }), + ) assert.NoError(t, err) _, err = m.RegisterCallback(func(_ context.Context, o metric.Observer) error { o.ObserveInt64(ctr, 11) @@ -229,7 +252,8 @@ func TestMeterCreatesInstruments(t *testing.T) { Temporality: metricdata.CumulativeTemporality, IsMonotonic: false, DataPoints: []metricdata.DataPoint[int64]{ - {Attributes: attrs, Value: 4}, + {Attributes: alice, Value: 4}, + {Attributes: bob, Value: 5}, {Value: 11}, }, }, @@ -238,11 +262,17 @@ func TestMeterCreatesInstruments(t *testing.T) { { name: "ObservableInt64Gauge", fn: func(t *testing.T, m metric.Meter) { - cback := func(_ context.Context, o metric.Int64Observer) error { - o.Observe(4, opt) - return nil - } - gauge, err := m.Int64ObservableGauge("agauge", metric.WithInt64Callback(cback)) + gauge, err := m.Int64ObservableGauge( + "agauge", + metric.WithInt64Callback(func(_ context.Context, o metric.Int64Observer) error { + o.Observe(4, optAlice) + return nil + }), + metric.WithInt64Callback(func(_ context.Context, o metric.Int64Observer) error { + o.Observe(5, optBob) + return nil + }), + ) assert.NoError(t, err) _, err = m.RegisterCallback(func(_ context.Context, o metric.Observer) error { o.ObserveInt64(gauge, 11) @@ -254,7 +284,8 @@ func TestMeterCreatesInstruments(t *testing.T) { Name: "agauge", Data: metricdata.Gauge[int64]{ DataPoints: []metricdata.DataPoint[int64]{ - {Attributes: attrs, Value: 4}, + {Attributes: alice, Value: 4}, + {Attributes: bob, Value: 5}, {Value: 11}, }, }, @@ -263,11 +294,17 @@ func TestMeterCreatesInstruments(t *testing.T) { { name: "ObservableFloat64Count", fn: func(t *testing.T, m metric.Meter) { - cback := func(_ context.Context, o metric.Float64Observer) error { - o.Observe(4, opt) - return nil - } - ctr, err := m.Float64ObservableCounter("afloat", metric.WithFloat64Callback(cback)) + ctr, err := m.Float64ObservableCounter( + "afloat", + metric.WithFloat64Callback(func(_ context.Context, o metric.Float64Observer) error { + o.Observe(4, optAlice) + return nil + }), + metric.WithFloat64Callback(func(_ context.Context, o metric.Float64Observer) error { + o.Observe(5, optBob) + return nil + }), + ) assert.NoError(t, err) _, err = m.RegisterCallback(func(_ context.Context, o metric.Observer) error { o.ObserveFloat64(ctr, 3) @@ -281,7 +318,8 @@ func TestMeterCreatesInstruments(t *testing.T) { Temporality: metricdata.CumulativeTemporality, IsMonotonic: true, DataPoints: []metricdata.DataPoint[float64]{ - {Attributes: attrs, Value: 4}, + {Attributes: alice, Value: 4}, + {Attributes: bob, Value: 5}, {Value: 3}, }, }, @@ -290,11 +328,17 @@ func TestMeterCreatesInstruments(t *testing.T) { { name: "ObservableFloat64UpDownCount", fn: func(t *testing.T, m metric.Meter) { - cback := func(_ context.Context, o metric.Float64Observer) error { - o.Observe(4, opt) - return nil - } - ctr, err := m.Float64ObservableUpDownCounter("afloat", metric.WithFloat64Callback(cback)) + ctr, err := m.Float64ObservableUpDownCounter( + "afloat", + metric.WithFloat64Callback(func(_ context.Context, o metric.Float64Observer) error { + o.Observe(4, optAlice) + return nil + }), + metric.WithFloat64Callback(func(_ context.Context, o metric.Float64Observer) error { + o.Observe(5, optBob) + return nil + }), + ) assert.NoError(t, err) _, err = m.RegisterCallback(func(_ context.Context, o metric.Observer) error { o.ObserveFloat64(ctr, 11) @@ -308,7 +352,8 @@ func TestMeterCreatesInstruments(t *testing.T) { Temporality: metricdata.CumulativeTemporality, IsMonotonic: false, DataPoints: []metricdata.DataPoint[float64]{ - {Attributes: attrs, Value: 4}, + {Attributes: alice, Value: 4}, + {Attributes: bob, Value: 5}, {Value: 11}, }, }, @@ -317,11 +362,17 @@ func TestMeterCreatesInstruments(t *testing.T) { { name: "ObservableFloat64Gauge", fn: func(t *testing.T, m metric.Meter) { - cback := func(_ context.Context, o metric.Float64Observer) error { - o.Observe(4, opt) - return nil - } - gauge, err := m.Float64ObservableGauge("agauge", metric.WithFloat64Callback(cback)) + gauge, err := m.Float64ObservableGauge( + "agauge", + metric.WithFloat64Callback(func(_ context.Context, o metric.Float64Observer) error { + o.Observe(4, optAlice) + return nil + }), + metric.WithFloat64Callback(func(_ context.Context, o metric.Float64Observer) error { + o.Observe(5, optBob) + return nil + }), + ) assert.NoError(t, err) _, err = m.RegisterCallback(func(_ context.Context, o metric.Observer) error { o.ObserveFloat64(gauge, 11) @@ -333,7 +384,8 @@ func TestMeterCreatesInstruments(t *testing.T) { Name: "agauge", Data: metricdata.Gauge[float64]{ DataPoints: []metricdata.DataPoint[float64]{ - {Attributes: attrs, Value: 4}, + {Attributes: alice, Value: 4}, + {Attributes: bob, Value: 5}, {Value: 11}, }, },