Skip to content

Commit

Permalink
Fix callback registration bug (#4888)
Browse files Browse the repository at this point in the history 
* Fix cback reg bug

Register all callbacks not just the last passed.

* Add changelog entry
  • Loading branch information
@MrAlias
MrAlias authored Feb 7, 2024
1 parent e3eb3f7 commit 11ebd19
Show file tree
Hide file tree
Showing 3 changed files with 98 additions and 40 deletions.
4 changes: 4 additions & 0 deletions CHANGELOG.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down
6 changes: 4 additions & 2 deletions sdk/metric/meter.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -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)
Expand Down
128 changes: 90 additions & 38 deletions sdk/metric/meter_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand All @@ -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)
Expand All @@ -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},
},
},
Expand All @@ -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)
Expand All @@ -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},
},
},
Expand All @@ -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)
Expand All @@ -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},
},
},
Expand All @@ -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)
Expand All @@ -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},
},
},
Expand All @@ -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)
Expand All @@ -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},
},
},
Expand All @@ -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)
Expand All @@ -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},
},
},
Expand Down

0 comments on commit 11ebd19

Please sign in to comment.