Skip to content

Commit

Permalink
Fix registration of multiple callbacks when using the global meter pr…
Browse files Browse the repository at this point in the history
…ovider (#4945)
  • Loading branch information
ntriamme committed Feb 21, 2024
1 parent c2fdbca commit f793a05
Show file tree
Hide file tree
Showing 3 changed files with 21 additions and 8 deletions.
4 changes: 4 additions & 0 deletions CHANGELOG.md
Expand Up @@ -17,6 +17,10 @@ The next release will require at least [Go 1.21].
- Add exemplar support to `go.opentelemetry.io/otel/exporters/otlp/otlpmetric/otlpmetricgrpc`. (#4900)
- Add exemplar support to `go.opentelemetry.io/otel/exporters/otlp/otlpmetric/otlpmetrichttp`. (#4900)

### Fixed

- Fix registration of multiple callbacks when using the global meter provider from `go.opentelemetry.io/otel`. (#4945)

## [1.23.1] 2024-02-07

### Fixed
Expand Down
4 changes: 3 additions & 1 deletion internal/global/meter.go
Expand Up @@ -130,9 +130,11 @@ func (m *meter) setDelegate(provider metric.MeterProvider) {
inst.setDelegate(meter)
}

for e := m.registry.Front(); e != nil; e = e.Next() {
var n *list.Element
for e := m.registry.Front(); e != nil; e = n {
r := e.Value.(*registration)
r.setDelegate(meter)
n = e.Next()
m.registry.Remove(e)
}

Expand Down
21 changes: 14 additions & 7 deletions internal/global/meter_test.go
Expand Up @@ -354,20 +354,27 @@ func TestRegistrationDelegation(t *testing.T) {
require.NoError(t, err)
require.Equal(t, 1, mImpl.registry.Len(), "second callback not registered")

mp := &testMeterProvider{}
var called2 bool
_, err = m.RegisterCallback(func(context.Context, metric.Observer) error {
called2 = true
return nil
}, actr)
require.NoError(t, err)
require.Equal(t, 2, mImpl.registry.Len(), "third callback not registered")

// otel.SetMeterProvider(mp)
mp := &testMeterProvider{}
globalMeterProvider.setDelegate(mp)

testCollect(t, m) // This is a hacky way to emulate a read from an exporter
require.False(t, called0, "pre-delegation unregistered callback called")
require.True(t, called1, "callback not called")
require.True(t, called1, "second callback not called")
require.True(t, called2, "third callback not called")

called1 = false
assert.NoError(t, reg1.Unregister(), "unregister second callback")

testCollect(t, m) // This is a hacky way to emulate a read from an exporter
assert.False(t, called1, "unregistered callback called")
called1, called2 = false, false // reset called capture
testCollect(t, m) // This is a hacky way to emulate a read from an exporter
assert.False(t, called1, "unregistered second callback called")
require.True(t, called2, "third callback not called")

assert.NotPanics(t, func() {
assert.NoError(t, reg1.Unregister(), "duplicate unregister calls")
Expand Down

0 comments on commit f793a05

Please sign in to comment.