Skip to content

Commit b37e8a9

Browse files
XSAMXSAM
authored
SetMeterProvider might miss the delegation for instruments and registries (#5780)
Closes #5757 This PR fixes an issue where `SetMeterProvider` might miss the delegation for instruments and registries. This bug brings a concurrent issue that could possibly make instruments and registries unable to operate correctly, such as recording, after using the `SetMeterProvider` method. The data put on these instruments and registries might be lost.
1 parent 9e1b015 commit b37e8a9

File tree

3 files changed

+92
-57
lines changed

3 files changed

+92
-57
lines changed

‎CHANGELOG.md

+1
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@ This project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.htm
1818

1919
- Fix memory leak in the global `MeterProvider` when identical instruments are repeatedly created. (#5754)
2020
- Fix panic instruments creation when setting meter provider. (#5758)
21+
- Fix an issue where `SetMeterProvider` in `go.opentelemetry.io/otel` might miss the delegation for instruments and registries. (#5780)
2122

2223
### Removed
2324

‎internal/global/meter.go

+80-53
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,6 @@ import (
77
"container/list"
88
"reflect"
99
"sync"
10-
"sync/atomic"
1110

1211
"go.opentelemetry.io/otel/metric"
1312
"go.opentelemetry.io/otel/metric/embedded"
@@ -97,7 +96,7 @@ type meter struct {
9796

9897
registry list.List
9998

100-
delegate atomic.Value // metric.Meter
99+
delegate metric.Meter
101100
}
102101

103102
type delegatedInstrument interface {
@@ -123,12 +122,12 @@ type instID struct {
123122
//
124123
// It is guaranteed by the caller that this happens only once.
125124
func (m *meter) setDelegate(provider metric.MeterProvider) {
126-
meter := provider.Meter(m.name, m.opts...)
127-
m.delegate.Store(meter)
128-
129125
m.mtx.Lock()
130126
defer m.mtx.Unlock()
131127

128+
meter := provider.Meter(m.name, m.opts...)
129+
m.delegate = meter
130+
132131
for _, inst := range m.instruments {
133132
inst.setDelegate(meter)
134133
}
@@ -141,16 +140,18 @@ func (m *meter) setDelegate(provider metric.MeterProvider) {
141140
m.registry.Remove(e)
142141
}
143142

144-
clear(m.instruments)
143+
m.instruments = nil
145144
m.registry.Init()
146145
}
147146

148147
func (m *meter) Int64Counter(name string, options ...metric.Int64CounterOption) (metric.Int64Counter, error) {
149-
if del, ok := m.delegate.Load().(metric.Meter); ok {
150-
return del.Int64Counter(name, options...)
151-
}
152148
m.mtx.Lock()
153149
defer m.mtx.Unlock()
150+
151+
if m.delegate != nil {
152+
return m.delegate.Int64Counter(name, options...)
153+
}
154+
154155
i := &siCounter{name: name, opts: options}
155156
cfg := metric.NewInt64CounterConfig(options...)
156157
id := instID{
@@ -164,11 +165,13 @@ func (m *meter) Int64Counter(name string, options ...metric.Int64CounterOption)
164165
}
165166

166167
func (m *meter) Int64UpDownCounter(name string, options ...metric.Int64UpDownCounterOption) (metric.Int64UpDownCounter, error) {
167-
if del, ok := m.delegate.Load().(metric.Meter); ok {
168-
return del.Int64UpDownCounter(name, options...)
169-
}
170168
m.mtx.Lock()
171169
defer m.mtx.Unlock()
170+
171+
if m.delegate != nil {
172+
return m.delegate.Int64UpDownCounter(name, options...)
173+
}
174+
172175
i := &siUpDownCounter{name: name, opts: options}
173176
cfg := metric.NewInt64UpDownCounterConfig(options...)
174177
id := instID{
@@ -182,11 +185,13 @@ func (m *meter) Int64UpDownCounter(name string, options ...metric.Int64UpDownCou
182185
}
183186

184187
func (m *meter) Int64Histogram(name string, options ...metric.Int64HistogramOption) (metric.Int64Histogram, error) {
185-
if del, ok := m.delegate.Load().(metric.Meter); ok {
186-
return del.Int64Histogram(name, options...)
187-
}
188188
m.mtx.Lock()
189189
defer m.mtx.Unlock()
190+
191+
if m.delegate != nil {
192+
return m.delegate.Int64Histogram(name, options...)
193+
}
194+
190195
i := &siHistogram{name: name, opts: options}
191196
cfg := metric.NewInt64HistogramConfig(options...)
192197
id := instID{
@@ -200,11 +205,13 @@ func (m *meter) Int64Histogram(name string, options ...metric.Int64HistogramOpti
200205
}
201206

202207
func (m *meter) Int64Gauge(name string, options ...metric.Int64GaugeOption) (metric.Int64Gauge, error) {
203-
if del, ok := m.delegate.Load().(metric.Meter); ok {
204-
return del.Int64Gauge(name, options...)
205-
}
206208
m.mtx.Lock()
207209
defer m.mtx.Unlock()
210+
211+
if m.delegate != nil {
212+
return m.delegate.Int64Gauge(name, options...)
213+
}
214+
208215
i := &siGauge{name: name, opts: options}
209216
cfg := metric.NewInt64GaugeConfig(options...)
210217
id := instID{
@@ -218,11 +225,13 @@ func (m *meter) Int64Gauge(name string, options ...metric.Int64GaugeOption) (met
218225
}
219226

220227
func (m *meter) Int64ObservableCounter(name string, options ...metric.Int64ObservableCounterOption) (metric.Int64ObservableCounter, error) {
221-
if del, ok := m.delegate.Load().(metric.Meter); ok {
222-
return del.Int64ObservableCounter(name, options...)
223-
}
224228
m.mtx.Lock()
225229
defer m.mtx.Unlock()
230+
231+
if m.delegate != nil {
232+
return m.delegate.Int64ObservableCounter(name, options...)
233+
}
234+
226235
i := &aiCounter{name: name, opts: options}
227236
cfg := metric.NewInt64ObservableCounterConfig(options...)
228237
id := instID{
@@ -236,11 +245,13 @@ func (m *meter) Int64ObservableCounter(name string, options ...metric.Int64Obser
236245
}
237246

238247
func (m *meter) Int64ObservableUpDownCounter(name string, options ...metric.Int64ObservableUpDownCounterOption) (metric.Int64ObservableUpDownCounter, error) {
239-
if del, ok := m.delegate.Load().(metric.Meter); ok {
240-
return del.Int64ObservableUpDownCounter(name, options...)
241-
}
242248
m.mtx.Lock()
243249
defer m.mtx.Unlock()
250+
251+
if m.delegate != nil {
252+
return m.delegate.Int64ObservableUpDownCounter(name, options...)
253+
}
254+
244255
i := &aiUpDownCounter{name: name, opts: options}
245256
cfg := metric.NewInt64ObservableUpDownCounterConfig(options...)
246257
id := instID{
@@ -254,11 +265,13 @@ func (m *meter) Int64ObservableUpDownCounter(name string, options ...metric.Int6
254265
}
255266

256267
func (m *meter) Int64ObservableGauge(name string, options ...metric.Int64ObservableGaugeOption) (metric.Int64ObservableGauge, error) {
257-
if del, ok := m.delegate.Load().(metric.Meter); ok {
258-
return del.Int64ObservableGauge(name, options...)
259-
}
260268
m.mtx.Lock()
261269
defer m.mtx.Unlock()
270+
271+
if m.delegate != nil {
272+
return m.delegate.Int64ObservableGauge(name, options...)
273+
}
274+
262275
i := &aiGauge{name: name, opts: options}
263276
cfg := metric.NewInt64ObservableGaugeConfig(options...)
264277
id := instID{
@@ -272,11 +285,13 @@ func (m *meter) Int64ObservableGauge(name string, options ...metric.Int64Observa
272285
}
273286

274287
func (m *meter) Float64Counter(name string, options ...metric.Float64CounterOption) (metric.Float64Counter, error) {
275-
if del, ok := m.delegate.Load().(metric.Meter); ok {
276-
return del.Float64Counter(name, options...)
277-
}
278288
m.mtx.Lock()
279289
defer m.mtx.Unlock()
290+
291+
if m.delegate != nil {
292+
return m.delegate.Float64Counter(name, options...)
293+
}
294+
280295
i := &sfCounter{name: name, opts: options}
281296
cfg := metric.NewFloat64CounterConfig(options...)
282297
id := instID{
@@ -290,11 +305,13 @@ func (m *meter) Float64Counter(name string, options ...metric.Float64CounterOpti
290305
}
291306

292307
func (m *meter) Float64UpDownCounter(name string, options ...metric.Float64UpDownCounterOption) (metric.Float64UpDownCounter, error) {
293-
if del, ok := m.delegate.Load().(metric.Meter); ok {
294-
return del.Float64UpDownCounter(name, options...)
295-
}
296308
m.mtx.Lock()
297309
defer m.mtx.Unlock()
310+
311+
if m.delegate != nil {
312+
return m.delegate.Float64UpDownCounter(name, options...)
313+
}
314+
298315
i := &sfUpDownCounter{name: name, opts: options}
299316
cfg := metric.NewFloat64UpDownCounterConfig(options...)
300317
id := instID{
@@ -308,11 +325,13 @@ func (m *meter) Float64UpDownCounter(name string, options ...metric.Float64UpDow
308325
}
309326

310327
func (m *meter) Float64Histogram(name string, options ...metric.Float64HistogramOption) (metric.Float64Histogram, error) {
311-
if del, ok := m.delegate.Load().(metric.Meter); ok {
312-
return del.Float64Histogram(name, options...)
313-
}
314328
m.mtx.Lock()
315329
defer m.mtx.Unlock()
330+
331+
if m.delegate != nil {
332+
return m.delegate.Float64Histogram(name, options...)
333+
}
334+
316335
i := &sfHistogram{name: name, opts: options}
317336
cfg := metric.NewFloat64HistogramConfig(options...)
318337
id := instID{
@@ -326,11 +345,13 @@ func (m *meter) Float64Histogram(name string, options ...metric.Float64Histogram
326345
}
327346

328347
func (m *meter) Float64Gauge(name string, options ...metric.Float64GaugeOption) (metric.Float64Gauge, error) {
329-
if del, ok := m.delegate.Load().(metric.Meter); ok {
330-
return del.Float64Gauge(name, options...)
331-
}
332348
m.mtx.Lock()
333349
defer m.mtx.Unlock()
350+
351+
if m.delegate != nil {
352+
return m.delegate.Float64Gauge(name, options...)
353+
}
354+
334355
i := &sfGauge{name: name, opts: options}
335356
cfg := metric.NewFloat64GaugeConfig(options...)
336357
id := instID{
@@ -344,11 +365,13 @@ func (m *meter) Float64Gauge(name string, options ...metric.Float64GaugeOption)
344365
}
345366

346367
func (m *meter) Float64ObservableCounter(name string, options ...metric.Float64ObservableCounterOption) (metric.Float64ObservableCounter, error) {
347-
if del, ok := m.delegate.Load().(metric.Meter); ok {
348-
return del.Float64ObservableCounter(name, options...)
349-
}
350368
m.mtx.Lock()
351369
defer m.mtx.Unlock()
370+
371+
if m.delegate != nil {
372+
return m.delegate.Float64ObservableCounter(name, options...)
373+
}
374+
352375
i := &afCounter{name: name, opts: options}
353376
cfg := metric.NewFloat64ObservableCounterConfig(options...)
354377
id := instID{
@@ -362,11 +385,13 @@ func (m *meter) Float64ObservableCounter(name string, options ...metric.Float64O
362385
}
363386

364387
func (m *meter) Float64ObservableUpDownCounter(name string, options ...metric.Float64ObservableUpDownCounterOption) (metric.Float64ObservableUpDownCounter, error) {
365-
if del, ok := m.delegate.Load().(metric.Meter); ok {
366-
return del.Float64ObservableUpDownCounter(name, options...)
367-
}
368388
m.mtx.Lock()
369389
defer m.mtx.Unlock()
390+
391+
if m.delegate != nil {
392+
return m.delegate.Float64ObservableUpDownCounter(name, options...)
393+
}
394+
370395
i := &afUpDownCounter{name: name, opts: options}
371396
cfg := metric.NewFloat64ObservableUpDownCounterConfig(options...)
372397
id := instID{
@@ -380,11 +405,13 @@ func (m *meter) Float64ObservableUpDownCounter(name string, options ...metric.Fl
380405
}
381406

382407
func (m *meter) Float64ObservableGauge(name string, options ...metric.Float64ObservableGaugeOption) (metric.Float64ObservableGauge, error) {
383-
if del, ok := m.delegate.Load().(metric.Meter); ok {
384-
return del.Float64ObservableGauge(name, options...)
385-
}
386408
m.mtx.Lock()
387409
defer m.mtx.Unlock()
410+
411+
if m.delegate != nil {
412+
return m.delegate.Float64ObservableGauge(name, options...)
413+
}
414+
388415
i := &afGauge{name: name, opts: options}
389416
cfg := metric.NewFloat64ObservableGaugeConfig(options...)
390417
id := instID{
@@ -399,14 +426,14 @@ func (m *meter) Float64ObservableGauge(name string, options ...metric.Float64Obs
399426

400427
// RegisterCallback captures the function that will be called during Collect.
401428
func (m *meter) RegisterCallback(f metric.Callback, insts ...metric.Observable) (metric.Registration, error) {
402-
if del, ok := m.delegate.Load().(metric.Meter); ok {
403-
insts = unwrapInstruments(insts)
404-
return del.RegisterCallback(f, insts...)
405-
}
406-
407429
m.mtx.Lock()
408430
defer m.mtx.Unlock()
409431

432+
if m.delegate != nil {
433+
insts = unwrapInstruments(insts)
434+
return m.delegate.RegisterCallback(f, insts...)
435+
}
436+
410437
reg := &registration{instruments: insts, function: f}
411438
e := m.registry.PushBack(reg)
412439
reg.unreg = func() error {

‎internal/global/meter_test.go

+11-4
Original file line numberDiff line numberDiff line change
@@ -83,6 +83,12 @@ func TestMeterConcurrentSafe(t *testing.T) {
8383
mtr.setDelegate(noop.NewMeterProvider())
8484
close(finish)
8585
<-done
86+
87+
// No instruments should be left after the meter is replaced.
88+
assert.Empty(t, mtr.instruments)
89+
90+
// No callbacks should be left after the meter is replaced.
91+
assert.Zero(t, mtr.registry.Len())
8692
}
8793

8894
func TestUnregisterConcurrentSafe(t *testing.T) {
@@ -162,8 +168,9 @@ func testSetupAllInstrumentTypes(t *testing.T, m metric.Meter) (metric.Float64Co
162168
// This is to emulate a read from an exporter.
163169
func testCollect(t *testing.T, m metric.Meter) {
164170
if tMeter, ok := m.(*meter); ok {
165-
m, ok = tMeter.delegate.Load().(metric.Meter)
166-
if !ok {
171+
// This changes the input m to the delegate.
172+
m = tMeter.delegate
173+
if m == nil {
167174
t.Error("meter was not delegated")
168175
return
169176
}
@@ -261,7 +268,7 @@ func TestMeterDelegatesCalls(t *testing.T) {
261268

262269
// Calls to Meter methods after setDelegate() should be executed by the delegate
263270
require.IsType(t, &meter{}, m)
264-
tMeter := m.(*meter).delegate.Load().(*testMeter)
271+
tMeter := m.(*meter).delegate.(*testMeter)
265272
require.NotNil(t, tMeter)
266273
assert.Equal(t, 1, tMeter.afCount)
267274
assert.Equal(t, 1, tMeter.afUDCount)
@@ -309,7 +316,7 @@ func TestMeterDefersDelegations(t *testing.T) {
309316

310317
// Calls to Meter() before setDelegate() should be the delegated type
311318
require.IsType(t, &meter{}, m)
312-
tMeter := m.(*meter).delegate.Load().(*testMeter)
319+
tMeter := m.(*meter).delegate.(*testMeter)
313320
require.NotNil(t, tMeter)
314321
assert.Equal(t, 1, tMeter.afCount)
315322
assert.Equal(t, 1, tMeter.afUDCount)

0 commit comments

Comments
 (0)