From 32452451967d532eab10693d03c39fee43c1ac49 Mon Sep 17 00:00:00 2001 From: Aaron Clawson Date: Wed, 2 Mar 2022 15:38:30 +0000 Subject: [PATCH 01/19] WIP: add global API --- metric/go.sum | 2 + metric/internal/global/instruments.go | 341 +++++++++++++++++++++ metric/internal/global/instruments_test.go | 73 +++++ metric/internal/global/meter.go | 296 ++++++++++++++++++ metric/internal/global/meter_test.go | 152 +++++++++ metric/internal/global/meter_types_test.go | 147 +++++++++ metric/internal/global/state.go | 60 ++++ 7 files changed, 1071 insertions(+) create mode 100644 metric/internal/global/instruments.go create mode 100644 metric/internal/global/instruments_test.go create mode 100644 metric/internal/global/meter.go create mode 100644 metric/internal/global/meter_test.go create mode 100644 metric/internal/global/meter_types_test.go create mode 100644 metric/internal/global/state.go diff --git a/metric/go.sum b/metric/go.sum index 5457c7626c5..d66fec17687 100644 --- a/metric/go.sum +++ b/metric/go.sum @@ -1,6 +1,8 @@ github.com/davecgh/go-spew v1.1.0 h1:ZDRjVQ15GmhC3fiQ8ni8+OwkZQO4DARzQgrnXU1Liz8= github.com/davecgh/go-spew v1.1.0/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38= +github.com/go-logr/logr v1.2.2 h1:ahHml/yUpnlb96Rp8HCvtYVPY8ZYpxq3g7UYchIYwbs= github.com/go-logr/logr v1.2.2/go.mod h1:jdQByPbusPIv2/zmleS9BjJVeZ6kBagPoEUsqbVz/1A= +github.com/go-logr/stdr v1.2.2 h1:hSWxHoqTgW2S2qGc0LTAI563KZ5YKYRhT3MFKZMbjag= github.com/go-logr/stdr v1.2.2/go.mod h1:mMo/vtBO5dYbehREoey6XUKy/eSumjCCveDpRre4VKE= github.com/google/go-cmp v0.5.7 h1:81/ik6ipDQS2aGcBfIN5dHDB36BwrStyeAQquSYCV4o= github.com/google/go-cmp v0.5.7/go.mod h1:n+brtR0CgQNWTVd5ZUFpTBC8YFBDLK/h/bpaJ8/DtOE= diff --git a/metric/internal/global/instruments.go b/metric/internal/global/instruments.go new file mode 100644 index 00000000000..6b2004af65f --- /dev/null +++ b/metric/internal/global/instruments.go @@ -0,0 +1,341 @@ +// Copyright The OpenTelemetry Authors +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package global // import "go.opentelemetry.io/otel/metric/internal/global" + +import ( + "context" + "sync/atomic" + + "go.opentelemetry.io/otel" + "go.opentelemetry.io/otel/attribute" + "go.opentelemetry.io/otel/metric" + "go.opentelemetry.io/otel/metric/instrument" + "go.opentelemetry.io/otel/metric/instrument/asyncfloat64" + "go.opentelemetry.io/otel/metric/instrument/asyncint64" + "go.opentelemetry.io/otel/metric/instrument/syncfloat64" + "go.opentelemetry.io/otel/metric/instrument/syncint64" +) + +type afCounter struct { + name string + opts []instrument.Option + + delegate atomic.Value //asyncfloat64.Counter + + instrument.Asynchronous +} + +func (i *afCounter) setDelegate(m metric.Meter) { + + ctr, err := m.AsyncFloat64().Counter(i.name, i.opts...) + if err != nil { + otel.Handle(err) + return + } + i.delegate.Store(ctr) + +} + +func (i *afCounter) Observe(ctx context.Context, x float64, attrs ...attribute.KeyValue) { + if ctr := i.delegate.Load(); ctr != nil { + ctr.(asyncfloat64.Counter).Observe(ctx, x, attrs...) + } +} + +type afUpDownCounter struct { + name string + opts []instrument.Option + + delegate atomic.Value //asyncfloat64.UpDownCounter + + instrument.Asynchronous +} + +func (i *afUpDownCounter) setDelegate(m metric.Meter) { + + ctr, err := m.AsyncFloat64().UpDownCounter(i.name, i.opts...) + if err != nil { + otel.Handle(err) + return + } + i.delegate.Store(ctr) + +} + +func (i *afUpDownCounter) Observe(ctx context.Context, x float64, attrs ...attribute.KeyValue) { + if ctr := i.delegate.Load(); ctr != nil { + ctr.(asyncfloat64.UpDownCounter).Observe(ctx, x, attrs...) + } +} + +type afGauge struct { + name string + opts []instrument.Option + + delegate atomic.Value //asyncfloat64.Gauge + + instrument.Asynchronous +} + +func (i *afGauge) setDelegate(m metric.Meter) { + + ctr, err := m.AsyncFloat64().Gauge(i.name, i.opts...) + if err != nil { + otel.Handle(err) + return + } + i.delegate.Store(ctr) + +} + +func (i *afGauge) Observe(ctx context.Context, x float64, attrs ...attribute.KeyValue) { + if ctr := i.delegate.Load(); ctr != nil { + ctr.(asyncfloat64.Gauge).Observe(ctx, x, attrs...) + } +} + +type aiCounter struct { + name string + opts []instrument.Option + + delegate atomic.Value //asyncint64.Counter + + instrument.Asynchronous +} + +func (i *aiCounter) setDelegate(m metric.Meter) { + + ctr, err := m.AsyncInt64().Counter(i.name, i.opts...) + if err != nil { + otel.Handle(err) + return + } + i.delegate.Store(ctr) + +} + +func (i *aiCounter) Observe(ctx context.Context, x int64, attrs ...attribute.KeyValue) { + if ctr := i.delegate.Load(); ctr != nil { + ctr.(asyncint64.Counter).Observe(ctx, x, attrs...) + } +} + +type aiUpDownCounter struct { + name string + opts []instrument.Option + + delegate atomic.Value //asyncint64.UpDownCounter + + instrument.Asynchronous +} + +func (i *aiUpDownCounter) setDelegate(m metric.Meter) { + + ctr, err := m.AsyncInt64().UpDownCounter(i.name, i.opts...) + if err != nil { + otel.Handle(err) + return + } + i.delegate.Store(ctr) + +} + +func (i *aiUpDownCounter) Observe(ctx context.Context, x int64, attrs ...attribute.KeyValue) { + if ctr := i.delegate.Load(); ctr != nil { + ctr.(asyncint64.UpDownCounter).Observe(ctx, x, attrs...) + } +} + +type aiGauge struct { + name string + opts []instrument.Option + + delegate atomic.Value //asyncint64.Gauge + + instrument.Asynchronous +} + +func (i *aiGauge) setDelegate(m metric.Meter) { + + ctr, err := m.AsyncInt64().Gauge(i.name, i.opts...) + if err != nil { + otel.Handle(err) + return + } + i.delegate.Store(ctr) + +} + +func (i *aiGauge) Observe(ctx context.Context, x int64, attrs ...attribute.KeyValue) { + if ctr := i.delegate.Load(); ctr != nil { + ctr.(asyncint64.Gauge).Observe(ctx, x, attrs...) + } +} + +//Sync Instruments +type sfCounter struct { + name string + opts []instrument.Option + + delegate atomic.Value //syncfloat64.Counter + + instrument.Synchronous +} + +func (i *sfCounter) setDelegate(m metric.Meter) { + + ctr, err := m.SyncFloat64().Counter(i.name, i.opts...) + if err != nil { + otel.Handle(err) + return + } + i.delegate.Store(ctr) + +} + +func (i *sfCounter) Add(ctx context.Context, incr float64, attrs ...attribute.KeyValue) { + if ctr := i.delegate.Load(); ctr != nil { + ctr.(syncfloat64.Counter).Add(ctx, incr, attrs...) + } +} + +type sfUpDownCounter struct { + name string + opts []instrument.Option + + delegate atomic.Value //syncfloat64.UpDownCounter + + instrument.Synchronous +} + +func (i *sfUpDownCounter) setDelegate(m metric.Meter) { + + ctr, err := m.SyncFloat64().UpDownCounter(i.name, i.opts...) + if err != nil { + otel.Handle(err) + return + } + i.delegate.Store(ctr) + +} + +func (i *sfUpDownCounter) Add(ctx context.Context, incr float64, attrs ...attribute.KeyValue) { + if ctr := i.delegate.Load(); ctr != nil { + ctr.(syncfloat64.UpDownCounter).Add(ctx, incr, attrs...) + } +} + +type sfHistogram struct { + name string + opts []instrument.Option + + delegate atomic.Value //syncfloat64.Histogram + + instrument.Synchronous +} + +func (i *sfHistogram) setDelegate(m metric.Meter) { + ctr, err := m.SyncFloat64().Histogram(i.name, i.opts...) + if err != nil { + otel.Handle(err) + return + } + i.delegate.Store(ctr) + +} + +func (i *sfHistogram) Record(ctx context.Context, x float64, attrs ...attribute.KeyValue) { + if ctr := i.delegate.Load(); ctr != nil { + ctr.(syncfloat64.Histogram).Record(ctx, x, attrs...) + } +} + +type siCounter struct { + name string + opts []instrument.Option + + delegate atomic.Value //syncint64.Counter + + instrument.Synchronous +} + +func (i *siCounter) setDelegate(m metric.Meter) { + + ctr, err := m.SyncInt64().Counter(i.name, i.opts...) + if err != nil { + otel.Handle(err) + return + } + i.delegate.Store(ctr) + +} + +func (i *siCounter) Add(ctx context.Context, x int64, attrs ...attribute.KeyValue) { + if ctr := i.delegate.Load(); ctr != nil { + ctr.(syncint64.Counter).Add(ctx, x, attrs...) + } +} + +type siUpDownCounter struct { + name string + opts []instrument.Option + + delegate atomic.Value //syncint64.UpDownCounter + + instrument.Synchronous +} + +func (i *siUpDownCounter) setDelegate(m metric.Meter) { + + ctr, err := m.SyncInt64().UpDownCounter(i.name, i.opts...) + if err != nil { + otel.Handle(err) + return + } + i.delegate.Store(ctr) + +} + +func (i *siUpDownCounter) Add(ctx context.Context, x int64, attrs ...attribute.KeyValue) { + if ctr := i.delegate.Load(); ctr != nil { + ctr.(syncint64.UpDownCounter).Add(ctx, x, attrs...) + } +} + +type siHistogram struct { + name string + opts []instrument.Option + + delegate atomic.Value //syncint64.Histogram + + instrument.Synchronous +} + +func (i *siHistogram) setDelegate(m metric.Meter) { + + ctr, err := m.SyncInt64().Histogram(i.name, i.opts...) + if err != nil { + otel.Handle(err) + return + } + i.delegate.Store(ctr) + +} + +func (i *siHistogram) Record(ctx context.Context, x int64, attrs ...attribute.KeyValue) { + if ctr := i.delegate.Load(); ctr != nil { + ctr.(syncint64.Histogram).Record(ctx, x, attrs...) + } +} diff --git a/metric/internal/global/instruments_test.go b/metric/internal/global/instruments_test.go new file mode 100644 index 00000000000..5c162961af3 --- /dev/null +++ b/metric/internal/global/instruments_test.go @@ -0,0 +1,73 @@ +// Copyright The OpenTelemetry Authors +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package global + +import ( + "context" + "testing" + + "go.opentelemetry.io/otel/attribute" + "go.opentelemetry.io/otel/metric/instrument" + "go.opentelemetry.io/otel/metric/nonrecording" +) + +func Test_afCounter_setDelegate(t *testing.T) { + delegate := afCounter{ + name: "testName", + opts: []instrument.Option{}, + } + + go func() { + for { + delegate.Observe(context.Background(), 1) + } + }() + + delegate.setDelegate(nonrecording.NewNoopMeter()) +} + +type test_counting_float_instrument struct { + count int + + instrument.Asynchronous + instrument.Synchronous +} + +func (i *test_counting_float_instrument) Observe(context.Context, float64, ...attribute.KeyValue) { + i.count++ +} +func (i *test_counting_float_instrument) Add(context.Context, float64, ...attribute.KeyValue) { + i.count++ +} +func (i *test_counting_float_instrument) Record(context.Context, float64, ...attribute.KeyValue) { + i.count++ +} + +type test_counting_int_instrument struct { + count int + + instrument.Asynchronous + instrument.Synchronous +} + +func (i *test_counting_int_instrument) Observe(context.Context, int64, ...attribute.KeyValue) { + i.count++ +} +func (i *test_counting_int_instrument) Add(context.Context, int64, ...attribute.KeyValue) { + i.count++ +} +func (i *test_counting_int_instrument) Record(context.Context, int64, ...attribute.KeyValue) { + i.count++ +} diff --git a/metric/internal/global/meter.go b/metric/internal/global/meter.go new file mode 100644 index 00000000000..0d2b088d5ac --- /dev/null +++ b/metric/internal/global/meter.go @@ -0,0 +1,296 @@ +// Copyright The OpenTelemetry Authors +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package global // import "go.opentelemetry.io/otel/metric/internal/global" + +import ( + "context" + "sync" + "sync/atomic" + + "go.opentelemetry.io/otel/metric" + "go.opentelemetry.io/otel/metric/instrument" + "go.opentelemetry.io/otel/metric/instrument/asyncfloat64" + "go.opentelemetry.io/otel/metric/instrument/asyncint64" + "go.opentelemetry.io/otel/metric/instrument/syncfloat64" + "go.opentelemetry.io/otel/metric/instrument/syncint64" +) + +// meterProvider is a placeholder for a configured SDK MeterProvider. +// +// All MeterProvider functionality is forwarded to a delegate once +// configured. +type meterProvider struct { + mtx sync.Mutex + meters map[il]*meter + + delegate metric.MeterProvider +} + +type il struct { + name string + version string +} + +// setDelegate configures p to delegate all MeterProvider functionality to +// provider. +// +// All Meters provided prior to this function call are switched out to be +// Meters provided by provider. All instruments and callbacks are recreated and +// delegated. +// +// It is guaranteed by the caller that this happens only once. +func (p *meterProvider) setDelegate(provider metric.MeterProvider) { + p.mtx.Lock() + defer p.mtx.Unlock() + + p.delegate = provider + + if len(p.meters) == 0 { + return + } + + for _, meter := range p.meters { + meter.setDelegate(provider) + } + + p.meters = nil +} + +// Meter implements MeterProvider. +func (p *meterProvider) Meter(name string, opts ...metric.MeterOption) metric.Meter { + p.mtx.Lock() + defer p.mtx.Unlock() + + if p.delegate != nil { + return p.delegate.Meter(name, opts...) + } + + // At this moment it is guaranteed that no sdk is installed, save the meter in the meters map. + + c := metric.NewMeterConfig(opts...) + key := il{ + name: name, + version: c.InstrumentationVersion(), + } + + if p.meters == nil { + p.meters = make(map[il]*meter) + } + + if val, ok := p.meters[key]; ok { + return val + } + + t := &meter{name: name, opts: opts} + p.meters[key] = t + return t +} + +// meter is a placeholder for a metric.Meter. +// +// All Meter functionality is forwarded to a delegate once configured. +// Otherwise, all functionality is forwarded to a NoopMeter. +type meter struct { + name string + opts []metric.MeterOption + + mtx sync.Mutex + instruments []delegatedInstrument + + // callbacks []callback + + delegate atomic.Value // metric.Meter +} + +type delegatedInstrument interface { + setDelegate(metric.Meter) +} + +// setDelegate configures t to delegate all Meter functionality to Meters +// created by provider. +// +// All subsequent calls to the Meter methods will be passed to the delegate. +// +// It is guaranteed by the caller that this happens only once. +func (m *meter) setDelegate(provider metric.MeterProvider) { + meter := provider.Meter(m.name, m.opts...) + m.delegate.Store(meter) + + for _, inst := range m.instruments { + inst.setDelegate(meter) + } + + m.instruments = nil +} + +// AsyncInt64 is the namespace for the Asynchronous Integer instruments. +// +// To Observe data with instruments it must be registered in a callback. +func (m *meter) AsyncInt64() asyncint64.InstrumentProvider { + if del, ok := m.delegate.Load().(metric.Meter); ok { + return del.AsyncInt64() + } + return (*aiInstProvider)(m) +} + +// AsyncFloat64 is the namespace for the Asynchronous Float instruments +// +// To Observe data with instruments it must be registered in a callback. +func (m *meter) AsyncFloat64() asyncfloat64.InstrumentProvider { + if del, ok := m.delegate.Load().(metric.Meter); ok { + return del.AsyncFloat64() + } + return (*afInstProvider)(m) +} + +// RegisterCallback captures the function that will be called during Collect. +// +// It is only valid to call Observe within the scope of the passed function, +// and only on the instruments that were registered with this call. +func (m *meter) RegisterCallback(insts []instrument.Asynchronous, function func(context.Context)) error { + panic("not implemented") // TODO: Implement +} + +// SyncInt64 is the namespace for the Synchronous Integer instruments +func (m *meter) SyncInt64() syncint64.InstrumentProvider { + if del, ok := m.delegate.Load().(metric.Meter); ok { + return del.SyncInt64() + } + return (*siInstProvider)(m) +} + +// SyncFloat64 is the namespace for the Synchronous Float instruments +func (m *meter) SyncFloat64() syncfloat64.InstrumentProvider { + if del, ok := m.delegate.Load().(metric.Meter); ok { + return del.SyncFloat64() + } + return (*sfInstProvider)(m) +} + +type afInstProvider meter + +// Counter creates an instrument for recording increasing values. +func (ip *afInstProvider) Counter(name string, opts ...instrument.Option) (asyncfloat64.Counter, error) { + ip.mtx.Lock() + defer ip.mtx.Unlock() + ctr := &afCounter{name: name, opts: opts} + ip.instruments = append(ip.instruments, ctr) + return ctr, nil +} + +// UpDownCounter creates an instrument for recording changes of a value. +func (ip *afInstProvider) UpDownCounter(name string, opts ...instrument.Option) (asyncfloat64.UpDownCounter, error) { + ip.mtx.Lock() + defer ip.mtx.Unlock() + ctr := &afUpDownCounter{name: name, opts: opts} + ip.instruments = append(ip.instruments, ctr) + return ctr, nil +} + +// Gauge creates an instrument for recording the current value. +func (ip *afInstProvider) Gauge(name string, opts ...instrument.Option) (asyncfloat64.Gauge, error) { + ip.mtx.Lock() + defer ip.mtx.Unlock() + ctr := &afGauge{name: name, opts: opts} + ip.instruments = append(ip.instruments, ctr) + return ctr, nil +} + +type aiInstProvider meter + +// Counter creates an instrument for recording increasing values. +func (ip *aiInstProvider) Counter(name string, opts ...instrument.Option) (asyncint64.Counter, error) { + ip.mtx.Lock() + defer ip.mtx.Unlock() + ctr := &aiCounter{name: name, opts: opts} + ip.instruments = append(ip.instruments, ctr) + return ctr, nil +} + +// UpDownCounter creates an instrument for recording changes of a value. +func (ip *aiInstProvider) UpDownCounter(name string, opts ...instrument.Option) (asyncint64.UpDownCounter, error) { + ip.mtx.Lock() + defer ip.mtx.Unlock() + ctr := &aiUpDownCounter{name: name, opts: opts} + ip.instruments = append(ip.instruments, ctr) + return ctr, nil +} + +// Gauge creates an instrument for recording the current value. +func (ip *aiInstProvider) Gauge(name string, opts ...instrument.Option) (asyncint64.Gauge, error) { + ip.mtx.Lock() + defer ip.mtx.Unlock() + ctr := &aiGauge{name: name, opts: opts} + ip.instruments = append(ip.instruments, ctr) + return ctr, nil +} + +type sfInstProvider meter + +// Counter creates an instrument for recording increasing values. +func (ip *sfInstProvider) Counter(name string, opts ...instrument.Option) (syncfloat64.Counter, error) { + ip.mtx.Lock() + defer ip.mtx.Unlock() + ctr := &sfCounter{name: name, opts: opts} + ip.instruments = append(ip.instruments, ctr) + return ctr, nil +} + +// UpDownCounter creates an instrument for recording changes of a value. +func (ip *sfInstProvider) UpDownCounter(name string, opts ...instrument.Option) (syncfloat64.UpDownCounter, error) { + ip.mtx.Lock() + defer ip.mtx.Unlock() + ctr := &sfUpDownCounter{name: name, opts: opts} + ip.instruments = append(ip.instruments, ctr) + return ctr, nil +} + +// Histogram creates an instrument for recording a distribution of values. +func (ip *sfInstProvider) Histogram(name string, opts ...instrument.Option) (syncfloat64.Histogram, error) { + ip.mtx.Lock() + defer ip.mtx.Unlock() + ctr := &sfHistogram{name: name, opts: opts} + ip.instruments = append(ip.instruments, ctr) + return ctr, nil +} + +type siInstProvider meter + +// Counter creates an instrument for recording increasing values. +func (ip *siInstProvider) Counter(name string, opts ...instrument.Option) (syncint64.Counter, error) { + ip.mtx.Lock() + defer ip.mtx.Unlock() + ctr := &siCounter{name: name, opts: opts} + ip.instruments = append(ip.instruments, ctr) + return ctr, nil +} + +// UpDownCounter creates an instrument for recording changes of a value. +func (ip *siInstProvider) UpDownCounter(name string, opts ...instrument.Option) (syncint64.UpDownCounter, error) { + ip.mtx.Lock() + defer ip.mtx.Unlock() + ctr := &siUpDownCounter{name: name, opts: opts} + ip.instruments = append(ip.instruments, ctr) + return ctr, nil +} + +// Histogram creates an instrument for recording a distribution of values. +func (ip *siInstProvider) Histogram(name string, opts ...instrument.Option) (syncint64.Histogram, error) { + ip.mtx.Lock() + defer ip.mtx.Unlock() + ctr := &siHistogram{name: name, opts: opts} + ip.instruments = append(ip.instruments, ctr) + return ctr, nil +} diff --git a/metric/internal/global/meter_test.go b/metric/internal/global/meter_test.go new file mode 100644 index 00000000000..1613dc65408 --- /dev/null +++ b/metric/internal/global/meter_test.go @@ -0,0 +1,152 @@ +// Copyright The OpenTelemetry Authors +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package global // import "go.opentelemetry.io/otel/metric/internal/global" + +import ( + "context" + "testing" + + "github.com/stretchr/testify/require" +) + +func Test_MeterProvider_delegates_calls(t *testing.T) { + + // The global MeterProvider should directly call the underlying MeterProvider + // if it is set prior to Meter() being called. + + // globalMeterProvider := otel.GetMeterProvider + globalMeterProvider := &meterProvider{} + + mp := &test_MeterProvider{} + + // otel.SetMeterProvider(mp) + globalMeterProvider.setDelegate(mp) + + require.Equal(t, 0, mp.count) + + meter := globalMeterProvider.Meter("go.opentelemetry.io/otel/metric/internal/global/meter_test") + _, _ = meter.AsyncFloat64().Counter("test_Async_Counter") + _, _ = meter.AsyncInt64().Counter("test_Async_Counter") + ctr, err := meter.SyncFloat64().Counter("testCounter") + require.NoError(t, err) + _, _ = meter.SyncInt64().Counter("test_sync_counter") + + ctr.Add(context.Background(), 5) + + // Calls to Meter() after setDelegate() should be executed by the delegate + require.IsType(t, &test_Meter{}, meter) + if t_meter, ok := meter.(*test_Meter); ok { + require.Equal(t, 1, t_meter.afCount) + require.Equal(t, 1, t_meter.aiCount) + require.Equal(t, 1, t_meter.sfCount) + require.Equal(t, 1, t_meter.siCount) + } + + // Because the Meter was provided by test_meterProvider it should also return our test instrument + require.IsType(t, &test_counting_float_instrument{}, ctr, "the meter did not delegate calls to the meter") + if test_ctr, ok := ctr.(*test_counting_float_instrument); ok { + require.Equal(t, 1, test_ctr.count) + } + + require.Equal(t, 1, mp.count) +} + +func Test_Meter_delegates_calls(t *testing.T) { + + // The global MeterProvider should directly provide a Meter instance that + // can be updated. If the SetMeterProvider is called after a Meter was + // obtained, but before instruments only the instrument should be generated + // by the delegated type. + + // globalMeterProvider := otel.GetMeterProvider + globalMeterProvider := &meterProvider{} + + mp := &test_MeterProvider{} + + require.Equal(t, 0, mp.count) + + m := globalMeterProvider.Meter("go.opentelemetry.io/otel/metric/internal/global/meter_test") + + // otel.SetMeterProvider(mp) + globalMeterProvider.setDelegate(mp) + + _, _ = m.AsyncFloat64().Counter("test_Async_Counter") + _, _ = m.AsyncInt64().Counter("test_Async_Counter") + ctr, err := m.SyncFloat64().Counter("testCounter") + require.NoError(t, err) + _, _ = m.SyncInt64().Counter("test_sync_counter") + + ctr.Add(context.Background(), 5) + + // Calls to Meter methods after setDelegate() should be executed by the delegate + require.IsType(t, &meter{}, m) + if d_meter, ok := m.(*meter); ok { + m := d_meter.delegate.Load().(*test_Meter) + require.NotNil(t, m) + require.Equal(t, 1, m.afCount) + require.Equal(t, 1, m.aiCount) + require.Equal(t, 1, m.sfCount) + require.Equal(t, 1, m.siCount) + } + + // Because the Meter was provided by test_meterProvider it should also return our test instrument + require.IsType(t, &test_counting_float_instrument{}, ctr, "the meter did not delegate calls to the meter") + if test_ctr, ok := ctr.(*test_counting_float_instrument); ok { + require.Equal(t, 1, test_ctr.count) + } + + require.Equal(t, 1, mp.count) +} + +func Test_Meter_defers_delegations(t *testing.T) { + + // If SetMeterProvider is called after insturments are registered, the + // instruments should be recreated with the new meter. + + // globalMeterProvider := otel.GetMeterProvider + globalMeterProvider := &meterProvider{} + + m := globalMeterProvider.Meter("go.opentelemetry.io/otel/metric/internal/global/meter_test") + _, _ = m.AsyncFloat64().Counter("test_Async_Counter") + _, _ = m.AsyncInt64().Counter("test_Async_Counter") + ctr, err := m.SyncFloat64().Counter("testCounter") + require.NoError(t, err) + _, _ = m.SyncInt64().Counter("test_sync_counter") + + ctr.Add(context.Background(), 5) + + mp := &test_MeterProvider{} + + // otel.SetMeterProvider(mp) + globalMeterProvider.setDelegate(mp) + + // Calls to Meter() before setDelegate() should be the delegated type + require.IsType(t, &meter{}, m) + + if d_meter, ok := m.(*meter); ok { + m := d_meter.delegate.Load().(*test_Meter) + require.NotNil(t, m) + require.Equal(t, 1, m.afCount) + require.Equal(t, 1, m.aiCount) + require.Equal(t, 1, m.sfCount) + require.Equal(t, 1, m.siCount) + } + + // Because the Meter was a delegate it should return a delegated instrument + + require.IsType(t, &sfCounter{}, ctr) + + require.Equal(t, 1, mp.count) +} diff --git a/metric/internal/global/meter_types_test.go b/metric/internal/global/meter_types_test.go new file mode 100644 index 00000000000..648c725f4c5 --- /dev/null +++ b/metric/internal/global/meter_types_test.go @@ -0,0 +1,147 @@ +// Copyright The OpenTelemetry Authors +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package global // import "go.opentelemetry.io/otel/metric/internal/global" + +import ( + "context" + + "go.opentelemetry.io/otel/metric" + "go.opentelemetry.io/otel/metric/instrument" + "go.opentelemetry.io/otel/metric/instrument/asyncfloat64" + "go.opentelemetry.io/otel/metric/instrument/asyncint64" + "go.opentelemetry.io/otel/metric/instrument/syncfloat64" + "go.opentelemetry.io/otel/metric/instrument/syncint64" +) + +type test_MeterProvider struct { + count int +} + +func (p *test_MeterProvider) Meter(name string, opts ...metric.MeterOption) metric.Meter { + p.count++ + + return &test_Meter{} +} + +type test_Meter struct { + afCount int + aiCount int + sfCount int + siCount int +} + +// AsyncInt64 is the namespace for the Asynchronous Integer instruments. +// +// To Observe data with instruments it must be registered in a callback. +func (m *test_Meter) AsyncInt64() asyncint64.InstrumentProvider { + m.aiCount++ + return &test_ai_InstrumentProvider{} +} + +// AsyncFloat64 is the namespace for the Asynchronous Float instruments +// +// To Observe data with instruments it must be registered in a callback. +func (m *test_Meter) AsyncFloat64() asyncfloat64.InstrumentProvider { + m.afCount++ + return &test_af_InstrumentProvider{} +} + +// RegisterCallback captures the function that will be called during Collect. +// +// It is only valid to call Observe within the scope of the passed function, +// and only on the instruments that were registered with this call. +func (m *test_Meter) RegisterCallback(insts []instrument.Asynchronous, function func(context.Context)) error { + panic("not implemented") // TODO: Implement +} + +// SyncInt64 is the namespace for the Synchronous Integer instruments +func (m *test_Meter) SyncInt64() syncint64.InstrumentProvider { + m.siCount++ + return &test_si_InstrumentProvider{} +} + +// SyncFloat64 is the namespace for the Synchronous Float instruments +func (m *test_Meter) SyncFloat64() syncfloat64.InstrumentProvider { + m.sfCount++ + return &test_sf_InstrumentProvider{} +} + +type test_af_InstrumentProvider struct{} + +// Counter creates an instrument for recording increasing values. +func (ip test_af_InstrumentProvider) Counter(name string, opts ...instrument.Option) (asyncfloat64.Counter, error) { + return &test_counting_float_instrument{}, nil +} + +// UpDownCounter creates an instrument for recording changes of a value. +func (ip test_af_InstrumentProvider) UpDownCounter(name string, opts ...instrument.Option) (asyncfloat64.UpDownCounter, error) { + return &test_counting_float_instrument{}, nil +} + +// Gauge creates an instrument for recording the current value. +func (ip test_af_InstrumentProvider) Gauge(name string, opts ...instrument.Option) (asyncfloat64.Gauge, error) { + return &test_counting_float_instrument{}, nil +} + +type test_ai_InstrumentProvider struct{} + +// Counter creates an instrument for recording increasing values. +func (ip test_ai_InstrumentProvider) Counter(name string, opts ...instrument.Option) (asyncint64.Counter, error) { + return &test_counting_int_instrument{}, nil +} + +// UpDownCounter creates an instrument for recording changes of a value. +func (ip test_ai_InstrumentProvider) UpDownCounter(name string, opts ...instrument.Option) (asyncint64.UpDownCounter, error) { + return &test_counting_int_instrument{}, nil +} + +// Gauge creates an instrument for recording the current value. +func (ip test_ai_InstrumentProvider) Gauge(name string, opts ...instrument.Option) (asyncint64.Gauge, error) { + return &test_counting_int_instrument{}, nil +} + +type test_sf_InstrumentProvider struct{} + +// Counter creates an instrument for recording increasing values. +func (ip test_sf_InstrumentProvider) Counter(name string, opts ...instrument.Option) (syncfloat64.Counter, error) { + return &test_counting_float_instrument{}, nil +} + +// UpDownCounter creates an instrument for recording changes of a value. +func (ip test_sf_InstrumentProvider) UpDownCounter(name string, opts ...instrument.Option) (syncfloat64.UpDownCounter, error) { + return &test_counting_float_instrument{}, nil +} + +// Gauge creates an instrument for recording the current value. +func (ip test_sf_InstrumentProvider) Histogram(name string, opts ...instrument.Option) (syncfloat64.Histogram, error) { + return &test_counting_float_instrument{}, nil +} + +type test_si_InstrumentProvider struct{} + +// Counter creates an instrument for recording increasing values. +func (ip test_si_InstrumentProvider) Counter(name string, opts ...instrument.Option) (syncint64.Counter, error) { + return &test_counting_int_instrument{}, nil +} + +// UpDownCounter creates an instrument for recording changes of a value. +func (ip test_si_InstrumentProvider) UpDownCounter(name string, opts ...instrument.Option) (syncint64.UpDownCounter, error) { + return &test_counting_int_instrument{}, nil +} + +// Gauge creates an instrument for recording the current value. +func (ip test_si_InstrumentProvider) Histogram(name string, opts ...instrument.Option) (syncint64.Histogram, error) { + return &test_counting_int_instrument{}, nil +} diff --git a/metric/internal/global/state.go b/metric/internal/global/state.go new file mode 100644 index 00000000000..149e37b86da --- /dev/null +++ b/metric/internal/global/state.go @@ -0,0 +1,60 @@ +// Copyright The OpenTelemetry Authors +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// htmp://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package global // import "go.opentelemetry.io/otel/metric/internal/global" + +import ( + "sync" + "sync/atomic" + + "go.opentelemetry.io/otel/metric" +) + +var ( + globalMeter = defaultMeterValue() + + delegateMeterOnce sync.Once +) + +type meterProviderHolder struct { + mp metric.MeterProvider +} + +// MeterProvider is the internal implementation for global.MeterProvider. +func MeterProvider() metric.MeterProvider { + return globalMeter.Load().(meterProviderHolder).mp +} + +// SetMeterProvider is the internal implementation for global.SetMeterProvider. +func SetMeterProvider(mp metric.MeterProvider) { + delegateMeterOnce.Do(func() { + current := MeterProvider() + if current == mp { + // Setting the provider to the prior default is nonsense, panic. + // Panic is acceptable because we are likely still early in the + // process lifetime. + panic("invalid MeterProvider, the global instance cannot be reinstalled") + } else if def, ok := current.(*meterProvider); ok { + def.setDelegate(mp) + } + + }) + globalMeter.Store(meterProviderHolder{mp: mp}) +} + +func defaultMeterValue() *atomic.Value { + v := &atomic.Value{} + v.Store(meterProviderHolder{mp: &meterProvider{}}) + return v +} From 2594638dafaddf36957ea7dcff3a83541473923b Mon Sep 17 00:00:00 2001 From: Aaron Clawson Date: Wed, 2 Mar 2022 19:10:00 +0000 Subject: [PATCH 02/19] WIP --- metric/internal/global/meter.go | 29 +++++- metric/internal/global/meter_test.go | 109 ++++++++++++++++----- metric/internal/global/meter_types_test.go | 13 ++- 3 files changed, 124 insertions(+), 27 deletions(-) diff --git a/metric/internal/global/meter.go b/metric/internal/global/meter.go index 0d2b088d5ac..e17a7f6a751 100644 --- a/metric/internal/global/meter.go +++ b/metric/internal/global/meter.go @@ -19,6 +19,7 @@ import ( "sync" "sync/atomic" + "go.opentelemetry.io/otel" "go.opentelemetry.io/otel/metric" "go.opentelemetry.io/otel/metric/instrument" "go.opentelemetry.io/otel/metric/instrument/asyncfloat64" @@ -108,8 +109,7 @@ type meter struct { mtx sync.Mutex instruments []delegatedInstrument - - // callbacks []callback + callbacks []delegatedCallback delegate atomic.Value // metric.Meter } @@ -160,7 +160,18 @@ func (m *meter) AsyncFloat64() asyncfloat64.InstrumentProvider { // It is only valid to call Observe within the scope of the passed function, // and only on the instruments that were registered with this call. func (m *meter) RegisterCallback(insts []instrument.Asynchronous, function func(context.Context)) error { - panic("not implemented") // TODO: Implement + if del, ok := m.delegate.Load().(metric.Meter); ok { + return del.RegisterCallback(insts, function) + } + + m.mtx.Lock() + defer m.mtx.Unlock() + m.callbacks = append(m.callbacks, delegatedCallback{ + instruments: insts, + function: function, + }) + + return nil } // SyncInt64 is the namespace for the Synchronous Integer instruments @@ -179,6 +190,18 @@ func (m *meter) SyncFloat64() syncfloat64.InstrumentProvider { return (*sfInstProvider)(m) } +type delegatedCallback struct { + instruments []instrument.Asynchronous + function func(context.Context) +} + +func (c *delegatedCallback) setDelegate(m metric.Meter) { + err := m.RegisterCallback(c.instruments, c.function) + if err != nil { + otel.Handle(err) + } +} + type afInstProvider meter // Counter creates an instrument for recording increasing values. diff --git a/metric/internal/global/meter_test.go b/metric/internal/global/meter_test.go index 1613dc65408..2a0534e84c8 100644 --- a/metric/internal/global/meter_test.go +++ b/metric/internal/global/meter_test.go @@ -19,6 +19,10 @@ import ( "testing" "github.com/stretchr/testify/require" + "go.opentelemetry.io/otel/metric" + "go.opentelemetry.io/otel/metric/instrument" + "go.opentelemetry.io/otel/metric/instrument/asyncfloat64" + "go.opentelemetry.io/otel/metric/instrument/syncfloat64" ) func Test_MeterProvider_delegates_calls(t *testing.T) { @@ -37,21 +41,21 @@ func Test_MeterProvider_delegates_calls(t *testing.T) { require.Equal(t, 0, mp.count) meter := globalMeterProvider.Meter("go.opentelemetry.io/otel/metric/internal/global/meter_test") - _, _ = meter.AsyncFloat64().Counter("test_Async_Counter") - _, _ = meter.AsyncInt64().Counter("test_Async_Counter") - ctr, err := meter.SyncFloat64().Counter("testCounter") - require.NoError(t, err) - _, _ = meter.SyncInt64().Counter("test_sync_counter") + + ctr, actr := test_setup_all_instrument_types(t, meter) ctr.Add(context.Background(), 5) + test_collect(t, meter) + // Calls to Meter() after setDelegate() should be executed by the delegate require.IsType(t, &test_Meter{}, meter) if t_meter, ok := meter.(*test_Meter); ok { - require.Equal(t, 1, t_meter.afCount) - require.Equal(t, 1, t_meter.aiCount) - require.Equal(t, 1, t_meter.sfCount) - require.Equal(t, 1, t_meter.siCount) + require.Equal(t, 3, t_meter.afCount) + require.Equal(t, 3, t_meter.aiCount) + require.Equal(t, 3, t_meter.sfCount) + require.Equal(t, 3, t_meter.siCount) + require.Equal(t, 1, len(t_meter.callbacks)) } // Because the Meter was provided by test_meterProvider it should also return our test instrument @@ -60,9 +64,67 @@ func Test_MeterProvider_delegates_calls(t *testing.T) { require.Equal(t, 1, test_ctr.count) } + require.IsType(t, &test_counting_float_instrument{}, actr, "the meter did not delegate calls to the meter") + if test_ctr, ok := actr.(*test_counting_float_instrument); ok { + require.Equal(t, 1, test_ctr.count) + } + require.Equal(t, 1, mp.count) } +func test_setup_all_instrument_types(t *testing.T, m metric.Meter) (syncfloat64.Counter, asyncfloat64.Counter) { + + afcounter, err := m.AsyncFloat64().Counter("test_Async_Counter") + require.NoError(t, err) + _, err = m.AsyncFloat64().UpDownCounter("test_Async_UpDownCounter") + require.NoError(t, err) + _, err = m.AsyncFloat64().Gauge("test_Async_Gauge") + require.NoError(t, err) + + _, err = m.AsyncInt64().Counter("test_Async_Counter") + require.NoError(t, err) + _, err = m.AsyncInt64().UpDownCounter("test_Async_UpDownCounter") + require.NoError(t, err) + _, err = m.AsyncInt64().Gauge("test_Async_Gauge") + require.NoError(t, err) + + m.RegisterCallback([]instrument.Asynchronous{afcounter}, func(ctx context.Context) { + afcounter.Observe(ctx, 3) + }) + + sfcounter, err := m.SyncFloat64().Counter("test_Async_Counter") + require.NoError(t, err) + _, err = m.SyncFloat64().UpDownCounter("test_Async_UpDownCounter") + require.NoError(t, err) + _, err = m.SyncFloat64().Histogram("test_Async_Histogram") + require.NoError(t, err) + + _, err = m.SyncInt64().Counter("test_Async_Counter") + require.NoError(t, err) + _, err = m.SyncInt64().UpDownCounter("test_Async_UpDownCounter") + require.NoError(t, err) + _, err = m.SyncInt64().Histogram("test_Async_Histogram") + require.NoError(t, err) + + return sfcounter, afcounter +} + +func test_collect(t *testing.T, m metric.Meter) { + if t_meter, ok := m.(*meter); ok { + m, ok = t_meter.delegate.Load().(metric.Meter) + if !ok { + t.Error("meter was not delegated") + return + } + } + t_meter, ok := m.(*test_Meter) + if !ok { + t.Error("collect called on non-test Meter") + return + } + t_meter.collect() +} + func Test_Meter_delegates_calls(t *testing.T) { // The global MeterProvider should directly provide a Meter instance that @@ -82,23 +144,21 @@ func Test_Meter_delegates_calls(t *testing.T) { // otel.SetMeterProvider(mp) globalMeterProvider.setDelegate(mp) - _, _ = m.AsyncFloat64().Counter("test_Async_Counter") - _, _ = m.AsyncInt64().Counter("test_Async_Counter") - ctr, err := m.SyncFloat64().Counter("testCounter") - require.NoError(t, err) - _, _ = m.SyncInt64().Counter("test_sync_counter") + ctr, actr := test_setup_all_instrument_types(t, m) ctr.Add(context.Background(), 5) + test_collect(t, m) + // Calls to Meter methods after setDelegate() should be executed by the delegate require.IsType(t, &meter{}, m) if d_meter, ok := m.(*meter); ok { m := d_meter.delegate.Load().(*test_Meter) require.NotNil(t, m) - require.Equal(t, 1, m.afCount) - require.Equal(t, 1, m.aiCount) - require.Equal(t, 1, m.sfCount) - require.Equal(t, 1, m.siCount) + require.Equal(t, 3, m.afCount) + require.Equal(t, 3, m.aiCount) + require.Equal(t, 3, m.sfCount) + require.Equal(t, 3, m.siCount) } // Because the Meter was provided by test_meterProvider it should also return our test instrument @@ -107,6 +167,12 @@ func Test_Meter_delegates_calls(t *testing.T) { require.Equal(t, 1, test_ctr.count) } + // Because the Meter was provided by test_meterProvider it should also return our test instrument + require.IsType(t, &test_counting_float_instrument{}, actr, "the meter did not delegate calls to the meter") + if test_ctr, ok := actr.(*test_counting_float_instrument); ok { + require.Equal(t, 1, test_ctr.count) + } + require.Equal(t, 1, mp.count) } @@ -119,11 +185,8 @@ func Test_Meter_defers_delegations(t *testing.T) { globalMeterProvider := &meterProvider{} m := globalMeterProvider.Meter("go.opentelemetry.io/otel/metric/internal/global/meter_test") - _, _ = m.AsyncFloat64().Counter("test_Async_Counter") - _, _ = m.AsyncInt64().Counter("test_Async_Counter") - ctr, err := m.SyncFloat64().Counter("testCounter") - require.NoError(t, err) - _, _ = m.SyncInt64().Counter("test_sync_counter") + + ctr, actr := test_setup_all_instrument_types(t, m) ctr.Add(context.Background(), 5) diff --git a/metric/internal/global/meter_types_test.go b/metric/internal/global/meter_types_test.go index 648c725f4c5..bafb0af51f5 100644 --- a/metric/internal/global/meter_types_test.go +++ b/metric/internal/global/meter_types_test.go @@ -40,6 +40,8 @@ type test_Meter struct { aiCount int sfCount int siCount int + + callbacks []func(context.Context) } // AsyncInt64 is the namespace for the Asynchronous Integer instruments. @@ -63,7 +65,8 @@ func (m *test_Meter) AsyncFloat64() asyncfloat64.InstrumentProvider { // It is only valid to call Observe within the scope of the passed function, // and only on the instruments that were registered with this call. func (m *test_Meter) RegisterCallback(insts []instrument.Asynchronous, function func(context.Context)) error { - panic("not implemented") // TODO: Implement + m.callbacks = append(m.callbacks, function) + return nil } // SyncInt64 is the namespace for the Synchronous Integer instruments @@ -78,6 +81,14 @@ func (m *test_Meter) SyncFloat64() syncfloat64.InstrumentProvider { return &test_sf_InstrumentProvider{} } +// This enables async collection +func (m *test_Meter) collect() { + ctx := context.Background() + for _, f := range m.callbacks { + f(ctx) + } +} + type test_af_InstrumentProvider struct{} // Counter creates an instrument for recording increasing values. From cac40eb046745cf79076a59171f964d55010443d Mon Sep 17 00:00:00 2001 From: Aaron Clawson Date: Fri, 4 Mar 2022 18:32:00 +0000 Subject: [PATCH 03/19] Add a global meter. --- metric/internal/global/meter.go | 13 +++ metric/internal/global/meter_test.go | 147 ++++++++++++++++++--------- metric/internal/global/state.go | 22 ++-- metric/internal/global/state_test.go | 74 ++++++++++++++ metric/meter.go | 13 +++ 5 files changed, 212 insertions(+), 57 deletions(-) create mode 100644 metric/internal/global/state_test.go diff --git a/metric/internal/global/meter.go b/metric/internal/global/meter.go index e17a7f6a751..0cef4cdf52d 100644 --- a/metric/internal/global/meter.go +++ b/metric/internal/global/meter.go @@ -69,6 +69,12 @@ func (p *meterProvider) setDelegate(provider metric.MeterProvider) { p.meters = nil } +func (p *meterProvider) isDelegated() bool { + p.mtx.Lock() + defer p.mtx.Unlock() + return p.delegate != nil +} + // Meter implements MeterProvider. func (p *meterProvider) Meter(name string, opts ...metric.MeterOption) metric.Meter { p.mtx.Lock() @@ -128,10 +134,17 @@ func (m *meter) setDelegate(provider metric.MeterProvider) { meter := provider.Meter(m.name, m.opts...) m.delegate.Store(meter) + m.mtx.Lock() + defer m.mtx.Unlock() + for _, inst := range m.instruments { inst.setDelegate(meter) } + for _, callback := range m.callbacks { + callback.setDelegate(meter) + } + m.instruments = nil } diff --git a/metric/internal/global/meter_test.go b/metric/internal/global/meter_test.go index 2a0534e84c8..dd4b590bdf3 100644 --- a/metric/internal/global/meter_test.go +++ b/metric/internal/global/meter_test.go @@ -16,6 +16,8 @@ package global // import "go.opentelemetry.io/otel/metric/internal/global" import ( "context" + "fmt" + "sync" "testing" "github.com/stretchr/testify/require" @@ -23,53 +25,55 @@ import ( "go.opentelemetry.io/otel/metric/instrument" "go.opentelemetry.io/otel/metric/instrument/asyncfloat64" "go.opentelemetry.io/otel/metric/instrument/syncfloat64" + "go.opentelemetry.io/otel/metric/nonrecording" ) -func Test_MeterProvider_delegates_calls(t *testing.T) { - - // The global MeterProvider should directly call the underlying MeterProvider - // if it is set prior to Meter() being called. - - // globalMeterProvider := otel.GetMeterProvider - globalMeterProvider := &meterProvider{} - - mp := &test_MeterProvider{} - - // otel.SetMeterProvider(mp) - globalMeterProvider.setDelegate(mp) +func Test_MeterProvider_race(t *testing.T) { + mp := &meterProvider{} - require.Equal(t, 0, mp.count) - - meter := globalMeterProvider.Meter("go.opentelemetry.io/otel/metric/internal/global/meter_test") - - ctr, actr := test_setup_all_instrument_types(t, meter) - - ctr.Add(context.Background(), 5) - - test_collect(t, meter) - - // Calls to Meter() after setDelegate() should be executed by the delegate - require.IsType(t, &test_Meter{}, meter) - if t_meter, ok := meter.(*test_Meter); ok { - require.Equal(t, 3, t_meter.afCount) - require.Equal(t, 3, t_meter.aiCount) - require.Equal(t, 3, t_meter.sfCount) - require.Equal(t, 3, t_meter.siCount) - require.Equal(t, 1, len(t_meter.callbacks)) - } + go func() { + i := 0 + for { + mp.Meter(fmt.Sprintf("a%d", i)) + i++ + } + }() - // Because the Meter was provided by test_meterProvider it should also return our test instrument - require.IsType(t, &test_counting_float_instrument{}, ctr, "the meter did not delegate calls to the meter") - if test_ctr, ok := ctr.(*test_counting_float_instrument); ok { - require.Equal(t, 1, test_ctr.count) - } + mp.setDelegate(nonrecording.NewNoopMeterProvider()) +} - require.IsType(t, &test_counting_float_instrument{}, actr, "the meter did not delegate calls to the meter") - if test_ctr, ok := actr.(*test_counting_float_instrument); ok { - require.Equal(t, 1, test_ctr.count) - } +func Test_meter_race(t *testing.T) { + mtr := &meter{} + + wg := &sync.WaitGroup{} + wg.Add(1) + go func() { + i := 0 + once := false + for { + name := fmt.Sprintf("a%d", i) + _, _ = mtr.AsyncFloat64().Counter(name) + _, _ = mtr.AsyncFloat64().UpDownCounter(name) + _, _ = mtr.AsyncFloat64().Gauge(name) + _, _ = mtr.AsyncInt64().Counter(name) + _, _ = mtr.AsyncInt64().UpDownCounter(name) + _, _ = mtr.AsyncInt64().Gauge(name) + _, _ = mtr.SyncFloat64().Counter(name) + _, _ = mtr.SyncFloat64().UpDownCounter(name) + _, _ = mtr.SyncFloat64().Histogram(name) + _, _ = mtr.SyncInt64().Counter(name) + _, _ = mtr.SyncInt64().UpDownCounter(name) + _, _ = mtr.SyncInt64().Histogram(name) + mtr.RegisterCallback(nil, func(ctx context.Context) {}) + if !once { + wg.Done() + once = true + } + } + }() - require.Equal(t, 1, mp.count) + wg.Wait() + mtr.setDelegate(nonrecording.NewNoopMeterProvider()) } func test_setup_all_instrument_types(t *testing.T, m metric.Meter) (syncfloat64.Counter, asyncfloat64.Counter) { @@ -125,6 +129,53 @@ func test_collect(t *testing.T, m metric.Meter) { t_meter.collect() } +func Test_MeterProvider_delegates_calls(t *testing.T) { + + // The global MeterProvider should directly call the underlying MeterProvider + // if it is set prior to Meter() being called. + + // globalMeterProvider := otel.GetMeterProvider + globalMeterProvider := &meterProvider{} + + mp := &test_MeterProvider{} + + // otel.SetMeterProvider(mp) + globalMeterProvider.setDelegate(mp) + + require.Equal(t, 0, mp.count) + + meter := globalMeterProvider.Meter("go.opentelemetry.io/otel/metric/internal/global/meter_test") + + ctr, actr := test_setup_all_instrument_types(t, meter) + + ctr.Add(context.Background(), 5) + + test_collect(t, meter) // This is a hacky way to emulate a read from an exporter + + // Calls to Meter() after setDelegate() should be executed by the delegate + require.IsType(t, &test_Meter{}, meter) + if t_meter, ok := meter.(*test_Meter); ok { + require.Equal(t, 3, t_meter.afCount) + require.Equal(t, 3, t_meter.aiCount) + require.Equal(t, 3, t_meter.sfCount) + require.Equal(t, 3, t_meter.siCount) + require.Equal(t, 1, len(t_meter.callbacks)) + } + + // Because the Meter was provided by test_meterProvider it should also return our test instrument + require.IsType(t, &test_counting_float_instrument{}, ctr, "the meter did not delegate calls to the meter") + if test_ctr, ok := ctr.(*test_counting_float_instrument); ok { + require.Equal(t, 1, test_ctr.count) + } + + require.IsType(t, &test_counting_float_instrument{}, actr, "the meter did not delegate calls to the meter") + if test_ctr, ok := actr.(*test_counting_float_instrument); ok { + require.Equal(t, 1, test_ctr.count) + } + + require.Equal(t, 1, mp.count) +} + func Test_Meter_delegates_calls(t *testing.T) { // The global MeterProvider should directly provide a Meter instance that @@ -148,7 +199,7 @@ func Test_Meter_delegates_calls(t *testing.T) { ctr.Add(context.Background(), 5) - test_collect(t, m) + test_collect(t, m) // This is a hacky way to emulate a read from an exporter // Calls to Meter methods after setDelegate() should be executed by the delegate require.IsType(t, &meter{}, m) @@ -195,21 +246,25 @@ func Test_Meter_defers_delegations(t *testing.T) { // otel.SetMeterProvider(mp) globalMeterProvider.setDelegate(mp) + test_collect(t, m) // This is a hacky way to emulate a read from an exporter + // Calls to Meter() before setDelegate() should be the delegated type require.IsType(t, &meter{}, m) if d_meter, ok := m.(*meter); ok { m := d_meter.delegate.Load().(*test_Meter) require.NotNil(t, m) - require.Equal(t, 1, m.afCount) - require.Equal(t, 1, m.aiCount) - require.Equal(t, 1, m.sfCount) - require.Equal(t, 1, m.siCount) + require.Equal(t, 3, m.afCount) + require.Equal(t, 3, m.aiCount) + require.Equal(t, 3, m.sfCount) + require.Equal(t, 3, m.siCount) } // Because the Meter was a delegate it should return a delegated instrument require.IsType(t, &sfCounter{}, ctr) + require.IsType(t, &afCounter{}, actr) + require.Equal(t, 1, mp.count) } diff --git a/metric/internal/global/state.go b/metric/internal/global/state.go index 149e37b86da..21b71a36be0 100644 --- a/metric/internal/global/state.go +++ b/metric/internal/global/state.go @@ -22,7 +22,7 @@ import ( ) var ( - globalMeter = defaultMeterValue() + globalMeterProvider = defaultMeterProvider() delegateMeterOnce sync.Once ) @@ -33,27 +33,27 @@ type meterProviderHolder struct { // MeterProvider is the internal implementation for global.MeterProvider. func MeterProvider() metric.MeterProvider { - return globalMeter.Load().(meterProviderHolder).mp + return globalMeterProvider.Load().(meterProviderHolder).mp } // SetMeterProvider is the internal implementation for global.SetMeterProvider. func SetMeterProvider(mp metric.MeterProvider) { + // Guard against SetMeterProvider(MeterProvider()) + current := MeterProvider() + if current == mp { + return + } + delegateMeterOnce.Do(func() { - current := MeterProvider() - if current == mp { - // Setting the provider to the prior default is nonsense, panic. - // Panic is acceptable because we are likely still early in the - // process lifetime. - panic("invalid MeterProvider, the global instance cannot be reinstalled") - } else if def, ok := current.(*meterProvider); ok { + if def, ok := current.(*meterProvider); ok { def.setDelegate(mp) } }) - globalMeter.Store(meterProviderHolder{mp: mp}) + globalMeterProvider.Store(meterProviderHolder{mp: mp}) } -func defaultMeterValue() *atomic.Value { +func defaultMeterProvider() *atomic.Value { v := &atomic.Value{} v.Store(meterProviderHolder{mp: &meterProvider{}}) return v diff --git a/metric/internal/global/state_test.go b/metric/internal/global/state_test.go new file mode 100644 index 00000000000..b7670a1ef72 --- /dev/null +++ b/metric/internal/global/state_test.go @@ -0,0 +1,74 @@ +// Copyright The OpenTelemetry Authors +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// htmp://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package global // import "go.opentelemetry.io/otel/metric/internal/global" + +import ( + "sync" + "testing" + + "go.opentelemetry.io/otel/metric/nonrecording" +) + +func resetGlobalMeterProvider() { + globalMeterProvider = defaultMeterProvider() + delegateMeterOnce = sync.Once{} +} + +func TestSetMeterProvider(t *testing.T) { + t.Cleanup(resetGlobalMeterProvider) + + t.Run("Set With default is no op", func(t *testing.T) { + resetGlobalMeterProvider() + + // This action should have no effect, nothing should be delegated + SetMeterProvider(MeterProvider()) + + mp, ok := MeterProvider().(*meterProvider) + if !ok { + t.Error("Global Meter Provider was changed") + return + } + if mp.delegate != nil { + t.Error("meter provider should not delegat when setting itself") + } + + }) + + t.Run("First Set() should replace the delegate", func(t *testing.T) { + resetGlobalMeterProvider() + + SetMeterProvider(nonrecording.NewNoopMeterProvider()) + + _, ok := MeterProvider().(*meterProvider) + if ok { + t.Error("Global Meter Provider was changed") + return + } + }) + + t.Run("Set() should delegate existing Meter Providers", func(t *testing.T) { + resetGlobalMeterProvider() + + mp := MeterProvider() + + SetMeterProvider(nonrecording.NewNoopMeterProvider()) + + dmp := mp.(*meterProvider) + + if dmp.delegate == nil { + t.Error("The delegated meter providers should have a delegate") + } + }) +} diff --git a/metric/meter.go b/metric/meter.go index 21fc1c499fb..bb7a7151666 100644 --- a/metric/meter.go +++ b/metric/meter.go @@ -17,11 +17,13 @@ package metric // import "go.opentelemetry.io/otel/metric" import ( "context" + "go.opentelemetry.io/otel/metric/global" "go.opentelemetry.io/otel/metric/instrument" "go.opentelemetry.io/otel/metric/instrument/asyncfloat64" "go.opentelemetry.io/otel/metric/instrument/asyncint64" "go.opentelemetry.io/otel/metric/instrument/syncfloat64" "go.opentelemetry.io/otel/metric/instrument/syncint64" + "go.opentelemetry.io/otel/metric/internal/global" ) // MeterProvider provides access to named Meter instances, for instrumenting @@ -58,3 +60,14 @@ type Meter interface { // SyncFloat64 is the namespace for the Synchronous Float instruments SyncFloat64() syncfloat64.InstrumentProvider } + +// GetGlobalMeterProvider returns the registered global trace provider. +// If none is registered then a No-op MeterProvider is returned. +func GetGlobalMeterProvider() MeterProvider { + return global.MeterProvider() +} + +// SetGlobalMeterProvider registers `mp` as the global meter provider. +func SetGlobalMeterProvider(mp MeterProvider) { + global.SetMeterProvider(mp) +} From 888f67f05e69e33b75d3b6fbcca23cea183ca59a Mon Sep 17 00:00:00 2001 From: Aaron Clawson Date: Fri, 4 Mar 2022 18:43:06 +0000 Subject: [PATCH 04/19] moved global access out of metric because of loop imports --- metric/global/global.go | 31 +++++++++++++++++++++++++++++++ metric/go.mod | 5 ++++- metric/go.sum | 1 + metric/meter.go | 13 ------------- 4 files changed, 36 insertions(+), 14 deletions(-) create mode 100644 metric/global/global.go diff --git a/metric/global/global.go b/metric/global/global.go new file mode 100644 index 00000000000..f52d2bda7f1 --- /dev/null +++ b/metric/global/global.go @@ -0,0 +1,31 @@ +// Copyright The OpenTelemetry Authors +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package global // import "go.opentelemetry.io/otel/metric/global" + +import ( + "go.opentelemetry.io/otel/metric" + "go.opentelemetry.io/otel/metric/internal/global" +) + +// GetGlobalMeterProvider returns the registered global trace provider. +// If none is registered then a No-op MeterProvider is returned. +func GetGlobalMeterProvider() metric.MeterProvider { + return global.MeterProvider() +} + +// SetGlobalMeterProvider registers `mp` as the global meter provider. +func SetGlobalMeterProvider(mp metric.MeterProvider) { + global.SetMeterProvider(mp) +} diff --git a/metric/go.mod b/metric/go.mod index 7d510be1f32..ac50aaeaa64 100644 --- a/metric/go.mod +++ b/metric/go.mod @@ -2,7 +2,10 @@ module go.opentelemetry.io/otel/metric go 1.16 -require go.opentelemetry.io/otel v1.4.1 +require ( + github.com/stretchr/testify v1.7.0 + go.opentelemetry.io/otel v1.4.1 +) replace go.opentelemetry.io/otel => ../ diff --git a/metric/go.sum b/metric/go.sum index d66fec17687..4f1776cd92b 100644 --- a/metric/go.sum +++ b/metric/go.sum @@ -12,6 +12,7 @@ github.com/stretchr/objx v0.1.0/go.mod h1:HFkY916IF+rwdDfMAkV7OtwuqBVzrE8GR6GFx+ github.com/stretchr/testify v1.7.0 h1:nwc3DEeHmmLAfoZucVR881uASk0Mfjw8xYJ99tb5CcY= github.com/stretchr/testify v1.7.0/go.mod h1:6Fq8oRcR53rry900zMqJjRRixrwX3KX962/h/Wwjteg= golang.org/x/xerrors v0.0.0-20191204190536-9bdfabe68543/go.mod h1:I/5z698sn9Ka8TeJc9MKroUUfqBBauWjQqLJ2OPfmY0= +gopkg.in/check.v1 v0.0.0-20161208181325-20d25e280405 h1:yhCVgyC4o1eVCa2tZl7eS0r+SDo693bJlVdllGtEeKM= gopkg.in/check.v1 v0.0.0-20161208181325-20d25e280405/go.mod h1:Co6ibVJAznAaIkqp8huTwlJQCZ016jof/cbN4VW5Yz0= gopkg.in/yaml.v3 v3.0.0-20200313102051-9f266ea9e77c h1:dUUwHk2QECo/6vqA44rthZ8ie2QXMNeKRTHCNY2nXvo= gopkg.in/yaml.v3 v3.0.0-20200313102051-9f266ea9e77c/go.mod h1:K4uyk7z7BCEPqu6E+C64Yfv1cQ7kz7rIZviUmN+EgEM= diff --git a/metric/meter.go b/metric/meter.go index bb7a7151666..21fc1c499fb 100644 --- a/metric/meter.go +++ b/metric/meter.go @@ -17,13 +17,11 @@ package metric // import "go.opentelemetry.io/otel/metric" import ( "context" - "go.opentelemetry.io/otel/metric/global" "go.opentelemetry.io/otel/metric/instrument" "go.opentelemetry.io/otel/metric/instrument/asyncfloat64" "go.opentelemetry.io/otel/metric/instrument/asyncint64" "go.opentelemetry.io/otel/metric/instrument/syncfloat64" "go.opentelemetry.io/otel/metric/instrument/syncint64" - "go.opentelemetry.io/otel/metric/internal/global" ) // MeterProvider provides access to named Meter instances, for instrumenting @@ -60,14 +58,3 @@ type Meter interface { // SyncFloat64 is the namespace for the Synchronous Float instruments SyncFloat64() syncfloat64.InstrumentProvider } - -// GetGlobalMeterProvider returns the registered global trace provider. -// If none is registered then a No-op MeterProvider is returned. -func GetGlobalMeterProvider() MeterProvider { - return global.MeterProvider() -} - -// SetGlobalMeterProvider registers `mp` as the global meter provider. -func SetGlobalMeterProvider(mp MeterProvider) { - global.SetMeterProvider(mp) -} From 2c125e4f7ff97c2b83046dce0807c6a65cce215f Mon Sep 17 00:00:00 2001 From: Aaron Clawson Date: Fri, 4 Mar 2022 18:59:09 +0000 Subject: [PATCH 05/19] fix linting issues --- .../internal/otlpconfig/options_test.go | 6 +- .../internal/otlpconfig/options_test.go | 8 +- metric/internal/global/instruments_test.go | 16 ++-- metric/internal/global/meter.go | 6 -- metric/internal/global/meter_test.go | 90 ++++++++++--------- metric/internal/global/meter_types_test.go | 84 ++++++++--------- 6 files changed, 103 insertions(+), 107 deletions(-) diff --git a/exporters/otlp/otlpmetric/internal/otlpconfig/options_test.go b/exporters/otlp/otlpmetric/internal/otlpconfig/options_test.go index 44c9af4d94c..d7caa14c8bc 100644 --- a/exporters/otlp/otlpmetric/internal/otlpconfig/options_test.go +++ b/exporters/otlp/otlpmetric/internal/otlpconfig/options_test.go @@ -201,7 +201,7 @@ func TestConfigs(t *testing.T) { //TODO: make sure gRPC's credentials actually works assert.NotNil(t, c.Metrics.GRPCCredentials) } else { - assert.Equal(t, tlsCert.RootCAs.Subjects(), c.Metrics.TLSCfg.RootCAs.Subjects()) + assert.Equal(t, tlsCert.RootCAs.Subjects(), c.Metrics.TLSCfg.RootCAs.Subjects()) //nolint } }, }, @@ -217,7 +217,7 @@ func TestConfigs(t *testing.T) { if grpcOption { assert.NotNil(t, c.Metrics.GRPCCredentials) } else { - assert.Equal(t, tlsCert.RootCAs.Subjects(), c.Metrics.TLSCfg.RootCAs.Subjects()) + assert.Equal(t, tlsCert.RootCAs.Subjects(), c.Metrics.TLSCfg.RootCAs.Subjects()) //nolint } }, }, @@ -235,7 +235,7 @@ func TestConfigs(t *testing.T) { if grpcOption { assert.NotNil(t, c.Metrics.GRPCCredentials) } else { - assert.Equal(t, tlsCert.RootCAs.Subjects(), c.Metrics.TLSCfg.RootCAs.Subjects()) + assert.Equal(t, tlsCert.RootCAs.Subjects(), c.Metrics.TLSCfg.RootCAs.Subjects()) //nolint } }, }, diff --git a/exporters/otlp/otlptrace/internal/otlpconfig/options_test.go b/exporters/otlp/otlptrace/internal/otlpconfig/options_test.go index 4efa2f7c630..84e9a7a205c 100644 --- a/exporters/otlp/otlptrace/internal/otlpconfig/options_test.go +++ b/exporters/otlp/otlptrace/internal/otlpconfig/options_test.go @@ -201,7 +201,7 @@ func TestConfigs(t *testing.T) { //TODO: make sure gRPC's credentials actually works assert.NotNil(t, c.Traces.GRPCCredentials) } else { - assert.Equal(t, tlsCert.RootCAs.Subjects(), c.Traces.TLSCfg.RootCAs.Subjects()) + assert.Equal(t, tlsCert.RootCAs.Subjects(), c.Traces.TLSCfg.RootCAs.Subjects()) //nolint } }, }, @@ -217,7 +217,7 @@ func TestConfigs(t *testing.T) { if grpcOption { assert.NotNil(t, c.Traces.GRPCCredentials) } else { - assert.Equal(t, tlsCert.RootCAs.Subjects(), c.Traces.TLSCfg.RootCAs.Subjects()) + assert.Equal(t, tlsCert.RootCAs.Subjects(), c.Traces.TLSCfg.RootCAs.Subjects()) //nolint } }, }, @@ -235,7 +235,7 @@ func TestConfigs(t *testing.T) { if grpcOption { assert.NotNil(t, c.Traces.GRPCCredentials) } else { - assert.Equal(t, tlsCert.RootCAs.Subjects(), c.Traces.TLSCfg.RootCAs.Subjects()) + assert.Equal(t, tlsCert.RootCAs.Subjects(), c.Traces.TLSCfg.RootCAs.Subjects()) //nolint } }, }, @@ -252,7 +252,7 @@ func TestConfigs(t *testing.T) { if grpcOption { assert.NotNil(t, c.Traces.GRPCCredentials) } else { - assert.Equal(t, tlsCert.RootCAs.Subjects(), c.Traces.TLSCfg.RootCAs.Subjects()) + assert.Equal(t, tlsCert.RootCAs.Subjects(), c.Traces.TLSCfg.RootCAs.Subjects()) //nolint } }, }, diff --git a/metric/internal/global/instruments_test.go b/metric/internal/global/instruments_test.go index 5c162961af3..259503a57c0 100644 --- a/metric/internal/global/instruments_test.go +++ b/metric/internal/global/instruments_test.go @@ -38,36 +38,36 @@ func Test_afCounter_setDelegate(t *testing.T) { delegate.setDelegate(nonrecording.NewNoopMeter()) } -type test_counting_float_instrument struct { +type testCountingFloatInstrument struct { count int instrument.Asynchronous instrument.Synchronous } -func (i *test_counting_float_instrument) Observe(context.Context, float64, ...attribute.KeyValue) { +func (i *testCountingFloatInstrument) Observe(context.Context, float64, ...attribute.KeyValue) { i.count++ } -func (i *test_counting_float_instrument) Add(context.Context, float64, ...attribute.KeyValue) { +func (i *testCountingFloatInstrument) Add(context.Context, float64, ...attribute.KeyValue) { i.count++ } -func (i *test_counting_float_instrument) Record(context.Context, float64, ...attribute.KeyValue) { +func (i *testCountingFloatInstrument) Record(context.Context, float64, ...attribute.KeyValue) { i.count++ } -type test_counting_int_instrument struct { +type testCountingIntInstrument struct { count int instrument.Asynchronous instrument.Synchronous } -func (i *test_counting_int_instrument) Observe(context.Context, int64, ...attribute.KeyValue) { +func (i *testCountingIntInstrument) Observe(context.Context, int64, ...attribute.KeyValue) { i.count++ } -func (i *test_counting_int_instrument) Add(context.Context, int64, ...attribute.KeyValue) { +func (i *testCountingIntInstrument) Add(context.Context, int64, ...attribute.KeyValue) { i.count++ } -func (i *test_counting_int_instrument) Record(context.Context, int64, ...attribute.KeyValue) { +func (i *testCountingIntInstrument) Record(context.Context, int64, ...attribute.KeyValue) { i.count++ } diff --git a/metric/internal/global/meter.go b/metric/internal/global/meter.go index 0cef4cdf52d..ea3b40e5248 100644 --- a/metric/internal/global/meter.go +++ b/metric/internal/global/meter.go @@ -69,12 +69,6 @@ func (p *meterProvider) setDelegate(provider metric.MeterProvider) { p.meters = nil } -func (p *meterProvider) isDelegated() bool { - p.mtx.Lock() - defer p.mtx.Unlock() - return p.delegate != nil -} - // Meter implements MeterProvider. func (p *meterProvider) Meter(name string, opts ...metric.MeterOption) metric.Meter { p.mtx.Lock() diff --git a/metric/internal/global/meter_test.go b/metric/internal/global/meter_test.go index dd4b590bdf3..2dcf2b76c2a 100644 --- a/metric/internal/global/meter_test.go +++ b/metric/internal/global/meter_test.go @@ -21,6 +21,7 @@ import ( "testing" "github.com/stretchr/testify/require" + "go.opentelemetry.io/otel/metric" "go.opentelemetry.io/otel/metric/instrument" "go.opentelemetry.io/otel/metric/instrument/asyncfloat64" @@ -64,7 +65,7 @@ func Test_meter_race(t *testing.T) { _, _ = mtr.SyncInt64().Counter(name) _, _ = mtr.SyncInt64().UpDownCounter(name) _, _ = mtr.SyncInt64().Histogram(name) - mtr.RegisterCallback(nil, func(ctx context.Context) {}) + _ = mtr.RegisterCallback(nil, func(ctx context.Context) {}) if !once { wg.Done() once = true @@ -76,7 +77,7 @@ func Test_meter_race(t *testing.T) { mtr.setDelegate(nonrecording.NewNoopMeterProvider()) } -func test_setup_all_instrument_types(t *testing.T, m metric.Meter) (syncfloat64.Counter, asyncfloat64.Counter) { +func testSetupAllInstrumentTypes(t *testing.T, m metric.Meter) (syncfloat64.Counter, asyncfloat64.Counter) { afcounter, err := m.AsyncFloat64().Counter("test_Async_Counter") require.NoError(t, err) @@ -92,7 +93,7 @@ func test_setup_all_instrument_types(t *testing.T, m metric.Meter) (syncfloat64. _, err = m.AsyncInt64().Gauge("test_Async_Gauge") require.NoError(t, err) - m.RegisterCallback([]instrument.Asynchronous{afcounter}, func(ctx context.Context) { + _ = m.RegisterCallback([]instrument.Asynchronous{afcounter}, func(ctx context.Context) { afcounter.Observe(ctx, 3) }) @@ -113,20 +114,21 @@ func test_setup_all_instrument_types(t *testing.T, m metric.Meter) (syncfloat64. return sfcounter, afcounter } -func test_collect(t *testing.T, m metric.Meter) { - if t_meter, ok := m.(*meter); ok { - m, ok = t_meter.delegate.Load().(metric.Meter) +// This is to emulate a read from an exporter +func testCollect(t *testing.T, m metric.Meter) { + if tMeter, ok := m.(*meter); ok { + m, ok = tMeter.delegate.Load().(metric.Meter) if !ok { t.Error("meter was not delegated") return } } - t_meter, ok := m.(*test_Meter) + tMeter, ok := m.(*testMeter) if !ok { t.Error("collect called on non-test Meter") return } - t_meter.collect() + tMeter.collect() } func Test_MeterProvider_delegates_calls(t *testing.T) { @@ -137,7 +139,7 @@ func Test_MeterProvider_delegates_calls(t *testing.T) { // globalMeterProvider := otel.GetMeterProvider globalMeterProvider := &meterProvider{} - mp := &test_MeterProvider{} + mp := &testMeterProvider{} // otel.SetMeterProvider(mp) globalMeterProvider.setDelegate(mp) @@ -146,31 +148,31 @@ func Test_MeterProvider_delegates_calls(t *testing.T) { meter := globalMeterProvider.Meter("go.opentelemetry.io/otel/metric/internal/global/meter_test") - ctr, actr := test_setup_all_instrument_types(t, meter) + ctr, actr := testSetupAllInstrumentTypes(t, meter) ctr.Add(context.Background(), 5) - test_collect(t, meter) // This is a hacky way to emulate a read from an exporter + testCollect(t, meter) // This is a hacky way to emulate a read from an exporter // Calls to Meter() after setDelegate() should be executed by the delegate - require.IsType(t, &test_Meter{}, meter) - if t_meter, ok := meter.(*test_Meter); ok { - require.Equal(t, 3, t_meter.afCount) - require.Equal(t, 3, t_meter.aiCount) - require.Equal(t, 3, t_meter.sfCount) - require.Equal(t, 3, t_meter.siCount) - require.Equal(t, 1, len(t_meter.callbacks)) + require.IsType(t, &testMeter{}, meter) + if tMeter, ok := meter.(*testMeter); ok { + require.Equal(t, 3, tMeter.afCount) + require.Equal(t, 3, tMeter.aiCount) + require.Equal(t, 3, tMeter.sfCount) + require.Equal(t, 3, tMeter.siCount) + require.Equal(t, 1, len(tMeter.callbacks)) } - // Because the Meter was provided by test_meterProvider it should also return our test instrument - require.IsType(t, &test_counting_float_instrument{}, ctr, "the meter did not delegate calls to the meter") - if test_ctr, ok := ctr.(*test_counting_float_instrument); ok { - require.Equal(t, 1, test_ctr.count) + // Because the Meter was provided by testmeterProvider it should also return our test instrument + require.IsType(t, &testCountingFloatInstrument{}, ctr, "the meter did not delegate calls to the meter") + if testCtr, ok := ctr.(*testCountingFloatInstrument); ok { + require.Equal(t, 1, testCtr.count) } - require.IsType(t, &test_counting_float_instrument{}, actr, "the meter did not delegate calls to the meter") - if test_ctr, ok := actr.(*test_counting_float_instrument); ok { - require.Equal(t, 1, test_ctr.count) + require.IsType(t, &testCountingFloatInstrument{}, actr, "the meter did not delegate calls to the meter") + if testCtr, ok := actr.(*testCountingFloatInstrument); ok { + require.Equal(t, 1, testCtr.count) } require.Equal(t, 1, mp.count) @@ -186,7 +188,7 @@ func Test_Meter_delegates_calls(t *testing.T) { // globalMeterProvider := otel.GetMeterProvider globalMeterProvider := &meterProvider{} - mp := &test_MeterProvider{} + mp := &testMeterProvider{} require.Equal(t, 0, mp.count) @@ -195,16 +197,16 @@ func Test_Meter_delegates_calls(t *testing.T) { // otel.SetMeterProvider(mp) globalMeterProvider.setDelegate(mp) - ctr, actr := test_setup_all_instrument_types(t, m) + ctr, actr := testSetupAllInstrumentTypes(t, m) ctr.Add(context.Background(), 5) - test_collect(t, m) // This is a hacky way to emulate a read from an exporter + testCollect(t, m) // This is a hacky way to emulate a read from an exporter // Calls to Meter methods after setDelegate() should be executed by the delegate require.IsType(t, &meter{}, m) - if d_meter, ok := m.(*meter); ok { - m := d_meter.delegate.Load().(*test_Meter) + if dMeter, ok := m.(*meter); ok { + m := dMeter.delegate.Load().(*testMeter) require.NotNil(t, m) require.Equal(t, 3, m.afCount) require.Equal(t, 3, m.aiCount) @@ -212,16 +214,16 @@ func Test_Meter_delegates_calls(t *testing.T) { require.Equal(t, 3, m.siCount) } - // Because the Meter was provided by test_meterProvider it should also return our test instrument - require.IsType(t, &test_counting_float_instrument{}, ctr, "the meter did not delegate calls to the meter") - if test_ctr, ok := ctr.(*test_counting_float_instrument); ok { - require.Equal(t, 1, test_ctr.count) + // Because the Meter was provided by testmeterProvider it should also return our test instrument + require.IsType(t, &testCountingFloatInstrument{}, ctr, "the meter did not delegate calls to the meter") + if testCtr, ok := ctr.(*testCountingFloatInstrument); ok { + require.Equal(t, 1, testCtr.count) } - // Because the Meter was provided by test_meterProvider it should also return our test instrument - require.IsType(t, &test_counting_float_instrument{}, actr, "the meter did not delegate calls to the meter") - if test_ctr, ok := actr.(*test_counting_float_instrument); ok { - require.Equal(t, 1, test_ctr.count) + // Because the Meter was provided by testmeterProvider it should also return our test instrument + require.IsType(t, &testCountingFloatInstrument{}, actr, "the meter did not delegate calls to the meter") + if testCtr, ok := actr.(*testCountingFloatInstrument); ok { + require.Equal(t, 1, testCtr.count) } require.Equal(t, 1, mp.count) @@ -229,7 +231,7 @@ func Test_Meter_delegates_calls(t *testing.T) { func Test_Meter_defers_delegations(t *testing.T) { - // If SetMeterProvider is called after insturments are registered, the + // If SetMeterProvider is called after instruments are registered, the // instruments should be recreated with the new meter. // globalMeterProvider := otel.GetMeterProvider @@ -237,22 +239,22 @@ func Test_Meter_defers_delegations(t *testing.T) { m := globalMeterProvider.Meter("go.opentelemetry.io/otel/metric/internal/global/meter_test") - ctr, actr := test_setup_all_instrument_types(t, m) + ctr, actr := testSetupAllInstrumentTypes(t, m) ctr.Add(context.Background(), 5) - mp := &test_MeterProvider{} + mp := &testMeterProvider{} // otel.SetMeterProvider(mp) globalMeterProvider.setDelegate(mp) - test_collect(t, m) // This is a hacky way to emulate a read from an exporter + testCollect(t, m) // This is a hacky way to emulate a read from an exporter // Calls to Meter() before setDelegate() should be the delegated type require.IsType(t, &meter{}, m) - if d_meter, ok := m.(*meter); ok { - m := d_meter.delegate.Load().(*test_Meter) + if dMeter, ok := m.(*meter); ok { + m := dMeter.delegate.Load().(*testMeter) require.NotNil(t, m) require.Equal(t, 3, m.afCount) require.Equal(t, 3, m.aiCount) diff --git a/metric/internal/global/meter_types_test.go b/metric/internal/global/meter_types_test.go index bafb0af51f5..8a01116e8cb 100644 --- a/metric/internal/global/meter_types_test.go +++ b/metric/internal/global/meter_types_test.go @@ -25,17 +25,17 @@ import ( "go.opentelemetry.io/otel/metric/instrument/syncint64" ) -type test_MeterProvider struct { +type testMeterProvider struct { count int } -func (p *test_MeterProvider) Meter(name string, opts ...metric.MeterOption) metric.Meter { +func (p *testMeterProvider) Meter(name string, opts ...metric.MeterOption) metric.Meter { p.count++ - return &test_Meter{} + return &testMeter{} } -type test_Meter struct { +type testMeter struct { afCount int aiCount int sfCount int @@ -47,112 +47,112 @@ type test_Meter struct { // AsyncInt64 is the namespace for the Asynchronous Integer instruments. // // To Observe data with instruments it must be registered in a callback. -func (m *test_Meter) AsyncInt64() asyncint64.InstrumentProvider { +func (m *testMeter) AsyncInt64() asyncint64.InstrumentProvider { m.aiCount++ - return &test_ai_InstrumentProvider{} + return &testAIInstrumentProvider{} } // AsyncFloat64 is the namespace for the Asynchronous Float instruments // // To Observe data with instruments it must be registered in a callback. -func (m *test_Meter) AsyncFloat64() asyncfloat64.InstrumentProvider { +func (m *testMeter) AsyncFloat64() asyncfloat64.InstrumentProvider { m.afCount++ - return &test_af_InstrumentProvider{} + return &testAFInstrumentProvider{} } // RegisterCallback captures the function that will be called during Collect. // // It is only valid to call Observe within the scope of the passed function, // and only on the instruments that were registered with this call. -func (m *test_Meter) RegisterCallback(insts []instrument.Asynchronous, function func(context.Context)) error { +func (m *testMeter) RegisterCallback(insts []instrument.Asynchronous, function func(context.Context)) error { m.callbacks = append(m.callbacks, function) return nil } // SyncInt64 is the namespace for the Synchronous Integer instruments -func (m *test_Meter) SyncInt64() syncint64.InstrumentProvider { +func (m *testMeter) SyncInt64() syncint64.InstrumentProvider { m.siCount++ - return &test_si_InstrumentProvider{} + return &testSIInstrumentProvider{} } // SyncFloat64 is the namespace for the Synchronous Float instruments -func (m *test_Meter) SyncFloat64() syncfloat64.InstrumentProvider { +func (m *testMeter) SyncFloat64() syncfloat64.InstrumentProvider { m.sfCount++ - return &test_sf_InstrumentProvider{} + return &testSFInstrumentProvider{} } // This enables async collection -func (m *test_Meter) collect() { +func (m *testMeter) collect() { ctx := context.Background() for _, f := range m.callbacks { f(ctx) } } -type test_af_InstrumentProvider struct{} +type testAFInstrumentProvider struct{} // Counter creates an instrument for recording increasing values. -func (ip test_af_InstrumentProvider) Counter(name string, opts ...instrument.Option) (asyncfloat64.Counter, error) { - return &test_counting_float_instrument{}, nil +func (ip testAFInstrumentProvider) Counter(name string, opts ...instrument.Option) (asyncfloat64.Counter, error) { + return &testCountingFloatInstrument{}, nil } // UpDownCounter creates an instrument for recording changes of a value. -func (ip test_af_InstrumentProvider) UpDownCounter(name string, opts ...instrument.Option) (asyncfloat64.UpDownCounter, error) { - return &test_counting_float_instrument{}, nil +func (ip testAFInstrumentProvider) UpDownCounter(name string, opts ...instrument.Option) (asyncfloat64.UpDownCounter, error) { + return &testCountingFloatInstrument{}, nil } // Gauge creates an instrument for recording the current value. -func (ip test_af_InstrumentProvider) Gauge(name string, opts ...instrument.Option) (asyncfloat64.Gauge, error) { - return &test_counting_float_instrument{}, nil +func (ip testAFInstrumentProvider) Gauge(name string, opts ...instrument.Option) (asyncfloat64.Gauge, error) { + return &testCountingFloatInstrument{}, nil } -type test_ai_InstrumentProvider struct{} +type testAIInstrumentProvider struct{} // Counter creates an instrument for recording increasing values. -func (ip test_ai_InstrumentProvider) Counter(name string, opts ...instrument.Option) (asyncint64.Counter, error) { - return &test_counting_int_instrument{}, nil +func (ip testAIInstrumentProvider) Counter(name string, opts ...instrument.Option) (asyncint64.Counter, error) { + return &testCountingIntInstrument{}, nil } // UpDownCounter creates an instrument for recording changes of a value. -func (ip test_ai_InstrumentProvider) UpDownCounter(name string, opts ...instrument.Option) (asyncint64.UpDownCounter, error) { - return &test_counting_int_instrument{}, nil +func (ip testAIInstrumentProvider) UpDownCounter(name string, opts ...instrument.Option) (asyncint64.UpDownCounter, error) { + return &testCountingIntInstrument{}, nil } // Gauge creates an instrument for recording the current value. -func (ip test_ai_InstrumentProvider) Gauge(name string, opts ...instrument.Option) (asyncint64.Gauge, error) { - return &test_counting_int_instrument{}, nil +func (ip testAIInstrumentProvider) Gauge(name string, opts ...instrument.Option) (asyncint64.Gauge, error) { + return &testCountingIntInstrument{}, nil } -type test_sf_InstrumentProvider struct{} +type testSFInstrumentProvider struct{} // Counter creates an instrument for recording increasing values. -func (ip test_sf_InstrumentProvider) Counter(name string, opts ...instrument.Option) (syncfloat64.Counter, error) { - return &test_counting_float_instrument{}, nil +func (ip testSFInstrumentProvider) Counter(name string, opts ...instrument.Option) (syncfloat64.Counter, error) { + return &testCountingFloatInstrument{}, nil } // UpDownCounter creates an instrument for recording changes of a value. -func (ip test_sf_InstrumentProvider) UpDownCounter(name string, opts ...instrument.Option) (syncfloat64.UpDownCounter, error) { - return &test_counting_float_instrument{}, nil +func (ip testSFInstrumentProvider) UpDownCounter(name string, opts ...instrument.Option) (syncfloat64.UpDownCounter, error) { + return &testCountingFloatInstrument{}, nil } // Gauge creates an instrument for recording the current value. -func (ip test_sf_InstrumentProvider) Histogram(name string, opts ...instrument.Option) (syncfloat64.Histogram, error) { - return &test_counting_float_instrument{}, nil +func (ip testSFInstrumentProvider) Histogram(name string, opts ...instrument.Option) (syncfloat64.Histogram, error) { + return &testCountingFloatInstrument{}, nil } -type test_si_InstrumentProvider struct{} +type testSIInstrumentProvider struct{} // Counter creates an instrument for recording increasing values. -func (ip test_si_InstrumentProvider) Counter(name string, opts ...instrument.Option) (syncint64.Counter, error) { - return &test_counting_int_instrument{}, nil +func (ip testSIInstrumentProvider) Counter(name string, opts ...instrument.Option) (syncint64.Counter, error) { + return &testCountingIntInstrument{}, nil } // UpDownCounter creates an instrument for recording changes of a value. -func (ip test_si_InstrumentProvider) UpDownCounter(name string, opts ...instrument.Option) (syncint64.UpDownCounter, error) { - return &test_counting_int_instrument{}, nil +func (ip testSIInstrumentProvider) UpDownCounter(name string, opts ...instrument.Option) (syncint64.UpDownCounter, error) { + return &testCountingIntInstrument{}, nil } // Gauge creates an instrument for recording the current value. -func (ip test_si_InstrumentProvider) Histogram(name string, opts ...instrument.Option) (syncint64.Histogram, error) { - return &test_counting_int_instrument{}, nil +func (ip testSIInstrumentProvider) Histogram(name string, opts ...instrument.Option) (syncint64.Histogram, error) { + return &testCountingIntInstrument{}, nil } From dec12e695a35336f7abb6dfd1f395b61315dd0a4 Mon Sep 17 00:00:00 2001 From: Aaron Clawson Date: Mon, 7 Mar 2022 15:10:45 +0000 Subject: [PATCH 06/19] remove changes from other lint failures. --- .../otlp/otlpmetric/internal/otlpconfig/options_test.go | 6 +++--- .../otlp/otlptrace/internal/otlpconfig/options_test.go | 8 ++++---- 2 files changed, 7 insertions(+), 7 deletions(-) diff --git a/exporters/otlp/otlpmetric/internal/otlpconfig/options_test.go b/exporters/otlp/otlpmetric/internal/otlpconfig/options_test.go index d7caa14c8bc..44c9af4d94c 100644 --- a/exporters/otlp/otlpmetric/internal/otlpconfig/options_test.go +++ b/exporters/otlp/otlpmetric/internal/otlpconfig/options_test.go @@ -201,7 +201,7 @@ func TestConfigs(t *testing.T) { //TODO: make sure gRPC's credentials actually works assert.NotNil(t, c.Metrics.GRPCCredentials) } else { - assert.Equal(t, tlsCert.RootCAs.Subjects(), c.Metrics.TLSCfg.RootCAs.Subjects()) //nolint + assert.Equal(t, tlsCert.RootCAs.Subjects(), c.Metrics.TLSCfg.RootCAs.Subjects()) } }, }, @@ -217,7 +217,7 @@ func TestConfigs(t *testing.T) { if grpcOption { assert.NotNil(t, c.Metrics.GRPCCredentials) } else { - assert.Equal(t, tlsCert.RootCAs.Subjects(), c.Metrics.TLSCfg.RootCAs.Subjects()) //nolint + assert.Equal(t, tlsCert.RootCAs.Subjects(), c.Metrics.TLSCfg.RootCAs.Subjects()) } }, }, @@ -235,7 +235,7 @@ func TestConfigs(t *testing.T) { if grpcOption { assert.NotNil(t, c.Metrics.GRPCCredentials) } else { - assert.Equal(t, tlsCert.RootCAs.Subjects(), c.Metrics.TLSCfg.RootCAs.Subjects()) //nolint + assert.Equal(t, tlsCert.RootCAs.Subjects(), c.Metrics.TLSCfg.RootCAs.Subjects()) } }, }, diff --git a/exporters/otlp/otlptrace/internal/otlpconfig/options_test.go b/exporters/otlp/otlptrace/internal/otlpconfig/options_test.go index 84e9a7a205c..4efa2f7c630 100644 --- a/exporters/otlp/otlptrace/internal/otlpconfig/options_test.go +++ b/exporters/otlp/otlptrace/internal/otlpconfig/options_test.go @@ -201,7 +201,7 @@ func TestConfigs(t *testing.T) { //TODO: make sure gRPC's credentials actually works assert.NotNil(t, c.Traces.GRPCCredentials) } else { - assert.Equal(t, tlsCert.RootCAs.Subjects(), c.Traces.TLSCfg.RootCAs.Subjects()) //nolint + assert.Equal(t, tlsCert.RootCAs.Subjects(), c.Traces.TLSCfg.RootCAs.Subjects()) } }, }, @@ -217,7 +217,7 @@ func TestConfigs(t *testing.T) { if grpcOption { assert.NotNil(t, c.Traces.GRPCCredentials) } else { - assert.Equal(t, tlsCert.RootCAs.Subjects(), c.Traces.TLSCfg.RootCAs.Subjects()) //nolint + assert.Equal(t, tlsCert.RootCAs.Subjects(), c.Traces.TLSCfg.RootCAs.Subjects()) } }, }, @@ -235,7 +235,7 @@ func TestConfigs(t *testing.T) { if grpcOption { assert.NotNil(t, c.Traces.GRPCCredentials) } else { - assert.Equal(t, tlsCert.RootCAs.Subjects(), c.Traces.TLSCfg.RootCAs.Subjects()) //nolint + assert.Equal(t, tlsCert.RootCAs.Subjects(), c.Traces.TLSCfg.RootCAs.Subjects()) } }, }, @@ -252,7 +252,7 @@ func TestConfigs(t *testing.T) { if grpcOption { assert.NotNil(t, c.Traces.GRPCCredentials) } else { - assert.Equal(t, tlsCert.RootCAs.Subjects(), c.Traces.TLSCfg.RootCAs.Subjects()) //nolint + assert.Equal(t, tlsCert.RootCAs.Subjects(), c.Traces.TLSCfg.RootCAs.Subjects()) } }, }, From 17598c5897cb630458a6a58f589d4d949ab4dddb Mon Sep 17 00:00:00 2001 From: Aaron Clawson Date: Mon, 7 Mar 2022 15:25:42 +0000 Subject: [PATCH 07/19] Add changelog --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 52cc0c413e7..f517a97089e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -35,6 +35,7 @@ This update is a breaking change of the unstable Metrics API. Code instrumented Zero or negative values will not be changed to the default value like `WithSpanLimits` does. Setting a limit to zero will effectively disable the related resource it limits and setting to a negative value will mean that resource is unlimited. Consequentially, limits should be constructed using `NewSpanLimits` and updated accordingly. (#2637) +- Add the `metric/global` for obtaining and setting the global `MeterProvider` (#2660) ### Changed From 6944af37178bd59a90c9b8cc8563a59a6a2a544b Mon Sep 17 00:00:00 2001 From: Aaron Clawson Date: Wed, 9 Mar 2022 15:54:56 +0000 Subject: [PATCH 08/19] Fixes for comments. Changed name of global API. Added stop to all race tests go routine. Added race tests for other instruments. --- metric/global/global.go | 6 +- metric/internal/global/instruments_test.go | 282 ++++++++++++++++++++- metric/internal/global/meter_test.go | 16 +- 3 files changed, 287 insertions(+), 17 deletions(-) diff --git a/metric/global/global.go b/metric/global/global.go index f52d2bda7f1..56b7c26e78b 100644 --- a/metric/global/global.go +++ b/metric/global/global.go @@ -19,13 +19,13 @@ import ( "go.opentelemetry.io/otel/metric/internal/global" ) -// GetGlobalMeterProvider returns the registered global trace provider. +// MeterProvider returns the registered global trace provider. // If none is registered then a No-op MeterProvider is returned. -func GetGlobalMeterProvider() metric.MeterProvider { +func MeterProvider() metric.MeterProvider { return global.MeterProvider() } // SetGlobalMeterProvider registers `mp` as the global meter provider. -func SetGlobalMeterProvider(mp metric.MeterProvider) { +func SetMeterProvider(mp metric.MeterProvider) { global.SetMeterProvider(mp) } diff --git a/metric/internal/global/instruments_test.go b/metric/internal/global/instruments_test.go index 259503a57c0..a0d46dd3990 100644 --- a/metric/internal/global/instruments_test.go +++ b/metric/internal/global/instruments_test.go @@ -23,19 +23,275 @@ import ( "go.opentelemetry.io/otel/metric/nonrecording" ) -func Test_afCounter_setDelegate(t *testing.T) { - delegate := afCounter{ - name: "testName", - opts: []instrument.Option{}, - } - - go func() { - for { - delegate.Observe(context.Background(), 1) - } - }() - - delegate.setDelegate(nonrecording.NewNoopMeter()) +func Test_asyncInstrument_setDelegate_race(t *testing.T) { + // Float64 Instruments + t.Run("Float64 Instruments", func(t *testing.T) { + t.Run("Async Counter", func(t *testing.T) { + delegate := &afCounter{ + name: "testName", + opts: []instrument.Option{}, + } + finish := make(chan struct{}) + go func() { + for { + delegate.Observe(context.Background(), 1) + select { + case <-finish: + return + default: + } + } + }() + + delegate.setDelegate(nonrecording.NewNoopMeter()) + close(finish) + }) + + t.Run("Async UpDownCounter", func(t *testing.T) { + delegate := &afUpDownCounter{ + name: "testName", + opts: []instrument.Option{}, + } + finish := make(chan struct{}) + go func() { + for { + delegate.Observe(context.Background(), 1) + select { + case <-finish: + return + default: + } + } + }() + + delegate.setDelegate(nonrecording.NewNoopMeter()) + close(finish) + }) + + t.Run("Async Gauge", func(t *testing.T) { + delegate := &afGauge{ + name: "testName", + opts: []instrument.Option{}, + } + finish := make(chan struct{}) + go func() { + for { + delegate.Observe(context.Background(), 1) + select { + case <-finish: + return + default: + } + } + }() + + delegate.setDelegate(nonrecording.NewNoopMeter()) + close(finish) + }) + }) + + // Int64 Instruments + + t.Run("int64 Instruments", func(t *testing.T) { + t.Run("Async Counter", func(t *testing.T) { + delegate := &aiCounter{ + name: "testName", + opts: []instrument.Option{}, + } + finish := make(chan struct{}) + go func() { + for { + delegate.Observe(context.Background(), 1) + select { + case <-finish: + return + default: + } + } + }() + + delegate.setDelegate(nonrecording.NewNoopMeter()) + close(finish) + }) + + t.Run("Async UpDownCounter", func(t *testing.T) { + delegate := &aiUpDownCounter{ + name: "testName", + opts: []instrument.Option{}, + } + finish := make(chan struct{}) + go func() { + for { + delegate.Observe(context.Background(), 1) + select { + case <-finish: + return + default: + } + } + }() + + delegate.setDelegate(nonrecording.NewNoopMeter()) + close(finish) + }) + + t.Run("Async Gauge", func(t *testing.T) { + delegate := &aiGauge{ + name: "testName", + opts: []instrument.Option{}, + } + finish := make(chan struct{}) + go func() { + for { + delegate.Observe(context.Background(), 1) + select { + case <-finish: + return + default: + } + } + }() + + delegate.setDelegate(nonrecording.NewNoopMeter()) + close(finish) + }) + }) +} + +func Test_syncInstrument_setDelegate_race(t *testing.T) { + // Float64 Instruments + // Float64 Instruments + t.Run("Float64 Instruments", func(t *testing.T) { + t.Run("Sync Counter", func(t *testing.T) { + delegate := &sfCounter{ + name: "testName", + opts: []instrument.Option{}, + } + finish := make(chan struct{}) + go func() { + for { + delegate.Add(context.Background(), 1) + select { + case <-finish: + return + default: + } + } + }() + + delegate.setDelegate(nonrecording.NewNoopMeter()) + close(finish) + }) + + t.Run("Sync UpDownCounter", func(t *testing.T) { + delegate := &sfUpDownCounter{ + name: "testName", + opts: []instrument.Option{}, + } + finish := make(chan struct{}) + go func() { + for { + delegate.Add(context.Background(), 1) + select { + case <-finish: + return + default: + } + } + }() + + delegate.setDelegate(nonrecording.NewNoopMeter()) + close(finish) + }) + + t.Run("Sync Histogram", func(t *testing.T) { + delegate := &sfHistogram{ + name: "testName", + opts: []instrument.Option{}, + } + finish := make(chan struct{}) + go func() { + for { + delegate.Record(context.Background(), 1) + select { + case <-finish: + return + default: + } + } + }() + + delegate.setDelegate(nonrecording.NewNoopMeter()) + close(finish) + }) + }) + + // Int64 Instruments + + t.Run("Int64 Instruments", func(t *testing.T) { + t.Run("Sync Counter", func(t *testing.T) { + delegate := &siCounter{ + name: "testName", + opts: []instrument.Option{}, + } + finish := make(chan struct{}) + go func() { + for { + delegate.Add(context.Background(), 1) + select { + case <-finish: + return + default: + } + } + }() + + delegate.setDelegate(nonrecording.NewNoopMeter()) + close(finish) + }) + + t.Run("Sync UpDownCounter", func(t *testing.T) { + delegate := &siUpDownCounter{ + name: "testName", + opts: []instrument.Option{}, + } + finish := make(chan struct{}) + go func() { + for { + delegate.Add(context.Background(), 1) + select { + case <-finish: + return + default: + } + } + }() + + delegate.setDelegate(nonrecording.NewNoopMeter()) + close(finish) + }) + + t.Run("Sync Histogram", func(t *testing.T) { + delegate := &siHistogram{ + name: "testName", + opts: []instrument.Option{}, + } + finish := make(chan struct{}) + go func() { + for { + delegate.Record(context.Background(), 1) + select { + case <-finish: + return + default: + } + } + }() + + delegate.setDelegate(nonrecording.NewNoopMeter()) + close(finish) + }) + }) } type testCountingFloatInstrument struct { diff --git a/metric/internal/global/meter_test.go b/metric/internal/global/meter_test.go index 2dcf2b76c2a..13e5cf6d485 100644 --- a/metric/internal/global/meter_test.go +++ b/metric/internal/global/meter_test.go @@ -31,16 +31,23 @@ import ( func Test_MeterProvider_race(t *testing.T) { mp := &meterProvider{} - + finish := make(chan struct{}) go func() { i := 0 for { mp.Meter(fmt.Sprintf("a%d", i)) i++ + select { + case <-finish: + return + default: + } } }() mp.setDelegate(nonrecording.NewNoopMeterProvider()) + close(finish) + } func Test_meter_race(t *testing.T) { @@ -48,6 +55,7 @@ func Test_meter_race(t *testing.T) { wg := &sync.WaitGroup{} wg.Add(1) + finish := make(chan struct{}) go func() { i := 0 once := false @@ -70,11 +78,17 @@ func Test_meter_race(t *testing.T) { wg.Done() once = true } + select { + case <-finish: + return + default: + } } }() wg.Wait() mtr.setDelegate(nonrecording.NewNoopMeterProvider()) + close(finish) } func testSetupAllInstrumentTypes(t *testing.T, m metric.Meter) (syncfloat64.Counter, asyncfloat64.Counter) { From 8a360ddf14a422662aba5e95046501ec6bb8b7bd Mon Sep 17 00:00:00 2001 From: Aaron Clawson Date: Thu, 10 Mar 2022 18:40:24 -0600 Subject: [PATCH 09/19] Apply suggestions from code review Co-authored-by: Tyler Yahn --- metric/global/global.go | 2 +- metric/internal/global/instruments_test.go | 37 +++++++++++----------- metric/internal/global/meter.go | 5 +-- metric/internal/global/meter_test.go | 26 ++++++--------- metric/internal/global/meter_types_test.go | 4 +-- metric/internal/global/state_test.go | 2 +- 6 files changed, 35 insertions(+), 41 deletions(-) diff --git a/metric/global/global.go b/metric/global/global.go index 56b7c26e78b..8578c99ae5a 100644 --- a/metric/global/global.go +++ b/metric/global/global.go @@ -25,7 +25,7 @@ func MeterProvider() metric.MeterProvider { return global.MeterProvider() } -// SetGlobalMeterProvider registers `mp` as the global meter provider. +// SetMeterProvider registers `mp` as the global meter provider. func SetMeterProvider(mp metric.MeterProvider) { global.SetMeterProvider(mp) } diff --git a/metric/internal/global/instruments_test.go b/metric/internal/global/instruments_test.go index a0d46dd3990..d655cb91ead 100644 --- a/metric/internal/global/instruments_test.go +++ b/metric/internal/global/instruments_test.go @@ -23,10 +23,10 @@ import ( "go.opentelemetry.io/otel/metric/nonrecording" ) -func Test_asyncInstrument_setDelegate_race(t *testing.T) { +func TestAsyncInstrumentSetDelegateRace(t *testing.T) { // Float64 Instruments - t.Run("Float64 Instruments", func(t *testing.T) { - t.Run("Async Counter", func(t *testing.T) { + t.Run("Float64", func(t *testing.T) { + t.Run("Counter", func(t *testing.T) { delegate := &afCounter{ name: "testName", opts: []instrument.Option{}, @@ -47,7 +47,7 @@ func Test_asyncInstrument_setDelegate_race(t *testing.T) { close(finish) }) - t.Run("Async UpDownCounter", func(t *testing.T) { + t.Run("UpDownCounter", func(t *testing.T) { delegate := &afUpDownCounter{ name: "testName", opts: []instrument.Option{}, @@ -68,7 +68,7 @@ func Test_asyncInstrument_setDelegate_race(t *testing.T) { close(finish) }) - t.Run("Async Gauge", func(t *testing.T) { + t.Run("Gauge", func(t *testing.T) { delegate := &afGauge{ name: "testName", opts: []instrument.Option{}, @@ -92,8 +92,8 @@ func Test_asyncInstrument_setDelegate_race(t *testing.T) { // Int64 Instruments - t.Run("int64 Instruments", func(t *testing.T) { - t.Run("Async Counter", func(t *testing.T) { + t.Run("Int64", func(t *testing.T) { + t.Run("Counter", func(t *testing.T) { delegate := &aiCounter{ name: "testName", opts: []instrument.Option{}, @@ -114,7 +114,7 @@ func Test_asyncInstrument_setDelegate_race(t *testing.T) { close(finish) }) - t.Run("Async UpDownCounter", func(t *testing.T) { + t.Run("UpDownCounter", func(t *testing.T) { delegate := &aiUpDownCounter{ name: "testName", opts: []instrument.Option{}, @@ -135,7 +135,7 @@ func Test_asyncInstrument_setDelegate_race(t *testing.T) { close(finish) }) - t.Run("Async Gauge", func(t *testing.T) { + t.Run("Gauge", func(t *testing.T) { delegate := &aiGauge{ name: "testName", opts: []instrument.Option{}, @@ -158,11 +158,10 @@ func Test_asyncInstrument_setDelegate_race(t *testing.T) { }) } -func Test_syncInstrument_setDelegate_race(t *testing.T) { +func TestSyncInstrumentSetDelegateRace(t *testing.T) { // Float64 Instruments - // Float64 Instruments - t.Run("Float64 Instruments", func(t *testing.T) { - t.Run("Sync Counter", func(t *testing.T) { + t.Run("Float64", func(t *testing.T) { + t.Run("Counter", func(t *testing.T) { delegate := &sfCounter{ name: "testName", opts: []instrument.Option{}, @@ -183,7 +182,7 @@ func Test_syncInstrument_setDelegate_race(t *testing.T) { close(finish) }) - t.Run("Sync UpDownCounter", func(t *testing.T) { + t.Run("UpDownCounter", func(t *testing.T) { delegate := &sfUpDownCounter{ name: "testName", opts: []instrument.Option{}, @@ -204,7 +203,7 @@ func Test_syncInstrument_setDelegate_race(t *testing.T) { close(finish) }) - t.Run("Sync Histogram", func(t *testing.T) { + t.Run("Histogram", func(t *testing.T) { delegate := &sfHistogram{ name: "testName", opts: []instrument.Option{}, @@ -228,8 +227,8 @@ func Test_syncInstrument_setDelegate_race(t *testing.T) { // Int64 Instruments - t.Run("Int64 Instruments", func(t *testing.T) { - t.Run("Sync Counter", func(t *testing.T) { + t.Run("Int64", func(t *testing.T) { + t.Run("Counter", func(t *testing.T) { delegate := &siCounter{ name: "testName", opts: []instrument.Option{}, @@ -250,7 +249,7 @@ func Test_syncInstrument_setDelegate_race(t *testing.T) { close(finish) }) - t.Run("Sync UpDownCounter", func(t *testing.T) { + t.Run("UpDownCounter", func(t *testing.T) { delegate := &siUpDownCounter{ name: "testName", opts: []instrument.Option{}, @@ -271,7 +270,7 @@ func Test_syncInstrument_setDelegate_race(t *testing.T) { close(finish) }) - t.Run("Sync Histogram", func(t *testing.T) { + t.Run("Histogram", func(t *testing.T) { delegate := &siHistogram{ name: "testName", opts: []instrument.Option{}, diff --git a/metric/internal/global/meter.go b/metric/internal/global/meter.go index ea3b40e5248..9001ae9427b 100644 --- a/metric/internal/global/meter.go +++ b/metric/internal/global/meter.go @@ -118,7 +118,7 @@ type delegatedInstrument interface { setDelegate(metric.Meter) } -// setDelegate configures t to delegate all Meter functionality to Meters +// setDelegate configures m to delegate all Meter functionality to Meters // created by provider. // // All subsequent calls to the Meter methods will be passed to the delegate. @@ -140,6 +140,7 @@ func (m *meter) setDelegate(provider metric.MeterProvider) { } m.instruments = nil + m.callbacks = nil } // AsyncInt64 is the namespace for the Asynchronous Integer instruments. @@ -152,7 +153,7 @@ func (m *meter) AsyncInt64() asyncint64.InstrumentProvider { return (*aiInstProvider)(m) } -// AsyncFloat64 is the namespace for the Asynchronous Float instruments +// AsyncFloat64 is the namespace for the Asynchronous Float instruments. // // To Observe data with instruments it must be registered in a callback. func (m *meter) AsyncFloat64() asyncfloat64.InstrumentProvider { diff --git a/metric/internal/global/meter_test.go b/metric/internal/global/meter_test.go index 13e5cf6d485..2cfa66eca88 100644 --- a/metric/internal/global/meter_test.go +++ b/metric/internal/global/meter_test.go @@ -29,14 +29,12 @@ import ( "go.opentelemetry.io/otel/metric/nonrecording" ) -func Test_MeterProvider_race(t *testing.T) { +func TestMeterProviderRace(t *testing.T) { mp := &meterProvider{} finish := make(chan struct{}) go func() { - i := 0 - for { + for i := 0; ; i++ { mp.Meter(fmt.Sprintf("a%d", i)) - i++ select { case <-finish: return @@ -50,16 +48,14 @@ func Test_MeterProvider_race(t *testing.T) { } -func Test_meter_race(t *testing.T) { +func TestMeterRace(t *testing.T) { mtr := &meter{} wg := &sync.WaitGroup{} wg.Add(1) finish := make(chan struct{}) go func() { - i := 0 - once := false - for { + for i, once := 0, false; ; i++{ name := fmt.Sprintf("a%d", i) _, _ = mtr.AsyncFloat64().Counter(name) _, _ = mtr.AsyncFloat64().UpDownCounter(name) @@ -107,9 +103,9 @@ func testSetupAllInstrumentTypes(t *testing.T, m metric.Meter) (syncfloat64.Coun _, err = m.AsyncInt64().Gauge("test_Async_Gauge") require.NoError(t, err) - _ = m.RegisterCallback([]instrument.Asynchronous{afcounter}, func(ctx context.Context) { + require.NoError(t, m.RegisterCallback([]instrument.Asynchronous{afcounter}, func(ctx context.Context) { afcounter.Observe(ctx, 3) - }) + })) sfcounter, err := m.SyncFloat64().Counter("test_Async_Counter") require.NoError(t, err) @@ -128,7 +124,7 @@ func testSetupAllInstrumentTypes(t *testing.T, m metric.Meter) (syncfloat64.Coun return sfcounter, afcounter } -// This is to emulate a read from an exporter +// This is to emulate a read from an exporter. func testCollect(t *testing.T, m metric.Meter) { if tMeter, ok := m.(*meter); ok { m, ok = tMeter.delegate.Load().(metric.Meter) @@ -145,7 +141,7 @@ func testCollect(t *testing.T, m metric.Meter) { tMeter.collect() } -func Test_MeterProvider_delegates_calls(t *testing.T) { +func TestMeterProviderDelegatesCalls(t *testing.T) { // The global MeterProvider should directly call the underlying MeterProvider // if it is set prior to Meter() being called. @@ -192,14 +188,13 @@ func Test_MeterProvider_delegates_calls(t *testing.T) { require.Equal(t, 1, mp.count) } -func Test_Meter_delegates_calls(t *testing.T) { +func TestMeterDelegatesCalls(t *testing.T) { // The global MeterProvider should directly provide a Meter instance that // can be updated. If the SetMeterProvider is called after a Meter was // obtained, but before instruments only the instrument should be generated // by the delegated type. - // globalMeterProvider := otel.GetMeterProvider globalMeterProvider := &meterProvider{} mp := &testMeterProvider{} @@ -208,7 +203,6 @@ func Test_Meter_delegates_calls(t *testing.T) { m := globalMeterProvider.Meter("go.opentelemetry.io/otel/metric/internal/global/meter_test") - // otel.SetMeterProvider(mp) globalMeterProvider.setDelegate(mp) ctr, actr := testSetupAllInstrumentTypes(t, m) @@ -243,7 +237,7 @@ func Test_Meter_delegates_calls(t *testing.T) { require.Equal(t, 1, mp.count) } -func Test_Meter_defers_delegations(t *testing.T) { +func TestMeterDefersDelegations(t *testing.T) { // If SetMeterProvider is called after instruments are registered, the // instruments should be recreated with the new meter. diff --git a/metric/internal/global/meter_types_test.go b/metric/internal/global/meter_types_test.go index 8a01116e8cb..acd07de1847 100644 --- a/metric/internal/global/meter_types_test.go +++ b/metric/internal/global/meter_types_test.go @@ -135,7 +135,7 @@ func (ip testSFInstrumentProvider) UpDownCounter(name string, opts ...instrument return &testCountingFloatInstrument{}, nil } -// Gauge creates an instrument for recording the current value. +// Histogram creates an instrument for recording a distribution of values. func (ip testSFInstrumentProvider) Histogram(name string, opts ...instrument.Option) (syncfloat64.Histogram, error) { return &testCountingFloatInstrument{}, nil } @@ -152,7 +152,7 @@ func (ip testSIInstrumentProvider) UpDownCounter(name string, opts ...instrument return &testCountingIntInstrument{}, nil } -// Gauge creates an instrument for recording the current value. +// Histogram creates an instrument for recording a distribution of values. func (ip testSIInstrumentProvider) Histogram(name string, opts ...instrument.Option) (syncint64.Histogram, error) { return &testCountingIntInstrument{}, nil } diff --git a/metric/internal/global/state_test.go b/metric/internal/global/state_test.go index b7670a1ef72..d7e7ed46900 100644 --- a/metric/internal/global/state_test.go +++ b/metric/internal/global/state_test.go @@ -53,7 +53,7 @@ func TestSetMeterProvider(t *testing.T) { _, ok := MeterProvider().(*meterProvider) if ok { - t.Error("Global Meter Provider was changed") + t.Error("Global Meter Provider was not changed") return } }) From 3bcc00a992fea7e6a7b401cc9028da55cf5396cc Mon Sep 17 00:00:00 2001 From: Aaron Clawson Date: Fri, 11 Mar 2022 14:56:39 +0000 Subject: [PATCH 10/19] Consolidated instrument tests --- metric/internal/global/instruments_test.go | 275 +++++---------------- 1 file changed, 59 insertions(+), 216 deletions(-) diff --git a/metric/internal/global/instruments_test.go b/metric/internal/global/instruments_test.go index d655cb91ead..33c88f6b6f1 100644 --- a/metric/internal/global/instruments_test.go +++ b/metric/internal/global/instruments_test.go @@ -19,74 +19,61 @@ import ( "testing" "go.opentelemetry.io/otel/attribute" + "go.opentelemetry.io/otel/metric" "go.opentelemetry.io/otel/metric/instrument" "go.opentelemetry.io/otel/metric/nonrecording" ) +func testFloat64Race(interact func(context.Context, float64, ...attribute.KeyValue), setDelegate func(metric.Meter)) { + finish := make(chan struct{}) + go func() { + for { + interact(context.Background(), 1) + select { + case <-finish: + return + default: + } + } + }() + + setDelegate(nonrecording.NewNoopMeter()) + close(finish) +} + +func testInt64Race(interact func(context.Context, int64, ...attribute.KeyValue), setDelegate func(metric.Meter)) { + finish := make(chan struct{}) + go func() { + for { + interact(context.Background(), 1) + select { + case <-finish: + return + default: + } + } + }() + + setDelegate(nonrecording.NewNoopMeter()) + close(finish) +} + func TestAsyncInstrumentSetDelegateRace(t *testing.T) { // Float64 Instruments t.Run("Float64", func(t *testing.T) { t.Run("Counter", func(t *testing.T) { - delegate := &afCounter{ - name: "testName", - opts: []instrument.Option{}, - } - finish := make(chan struct{}) - go func() { - for { - delegate.Observe(context.Background(), 1) - select { - case <-finish: - return - default: - } - } - }() - - delegate.setDelegate(nonrecording.NewNoopMeter()) - close(finish) + delegate := &afCounter{} + testFloat64Race(delegate.Observe, delegate.setDelegate) }) t.Run("UpDownCounter", func(t *testing.T) { - delegate := &afUpDownCounter{ - name: "testName", - opts: []instrument.Option{}, - } - finish := make(chan struct{}) - go func() { - for { - delegate.Observe(context.Background(), 1) - select { - case <-finish: - return - default: - } - } - }() - - delegate.setDelegate(nonrecording.NewNoopMeter()) - close(finish) + delegate := &afUpDownCounter{} + testFloat64Race(delegate.Observe, delegate.setDelegate) }) t.Run("Gauge", func(t *testing.T) { - delegate := &afGauge{ - name: "testName", - opts: []instrument.Option{}, - } - finish := make(chan struct{}) - go func() { - for { - delegate.Observe(context.Background(), 1) - select { - case <-finish: - return - default: - } - } - }() - - delegate.setDelegate(nonrecording.NewNoopMeter()) - close(finish) + delegate := &afGauge{} + testFloat64Race(delegate.Observe, delegate.setDelegate) }) }) @@ -94,66 +81,18 @@ func TestAsyncInstrumentSetDelegateRace(t *testing.T) { t.Run("Int64", func(t *testing.T) { t.Run("Counter", func(t *testing.T) { - delegate := &aiCounter{ - name: "testName", - opts: []instrument.Option{}, - } - finish := make(chan struct{}) - go func() { - for { - delegate.Observe(context.Background(), 1) - select { - case <-finish: - return - default: - } - } - }() - - delegate.setDelegate(nonrecording.NewNoopMeter()) - close(finish) + delegate := &aiCounter{} + testInt64Race(delegate.Observe, delegate.setDelegate) }) t.Run("UpDownCounter", func(t *testing.T) { - delegate := &aiUpDownCounter{ - name: "testName", - opts: []instrument.Option{}, - } - finish := make(chan struct{}) - go func() { - for { - delegate.Observe(context.Background(), 1) - select { - case <-finish: - return - default: - } - } - }() - - delegate.setDelegate(nonrecording.NewNoopMeter()) - close(finish) + delegate := &aiUpDownCounter{} + testInt64Race(delegate.Observe, delegate.setDelegate) }) t.Run("Gauge", func(t *testing.T) { - delegate := &aiGauge{ - name: "testName", - opts: []instrument.Option{}, - } - finish := make(chan struct{}) - go func() { - for { - delegate.Observe(context.Background(), 1) - select { - case <-finish: - return - default: - } - } - }() - - delegate.setDelegate(nonrecording.NewNoopMeter()) - close(finish) + delegate := &aiGauge{} + testInt64Race(delegate.Observe, delegate.setDelegate) }) }) } @@ -162,66 +101,18 @@ func TestSyncInstrumentSetDelegateRace(t *testing.T) { // Float64 Instruments t.Run("Float64", func(t *testing.T) { t.Run("Counter", func(t *testing.T) { - delegate := &sfCounter{ - name: "testName", - opts: []instrument.Option{}, - } - finish := make(chan struct{}) - go func() { - for { - delegate.Add(context.Background(), 1) - select { - case <-finish: - return - default: - } - } - }() - - delegate.setDelegate(nonrecording.NewNoopMeter()) - close(finish) + delegate := &sfCounter{} + testFloat64Race(delegate.Add, delegate.setDelegate) }) t.Run("UpDownCounter", func(t *testing.T) { - delegate := &sfUpDownCounter{ - name: "testName", - opts: []instrument.Option{}, - } - finish := make(chan struct{}) - go func() { - for { - delegate.Add(context.Background(), 1) - select { - case <-finish: - return - default: - } - } - }() - - delegate.setDelegate(nonrecording.NewNoopMeter()) - close(finish) + delegate := &sfUpDownCounter{} + testFloat64Race(delegate.Add, delegate.setDelegate) }) t.Run("Histogram", func(t *testing.T) { - delegate := &sfHistogram{ - name: "testName", - opts: []instrument.Option{}, - } - finish := make(chan struct{}) - go func() { - for { - delegate.Record(context.Background(), 1) - select { - case <-finish: - return - default: - } - } - }() - - delegate.setDelegate(nonrecording.NewNoopMeter()) - close(finish) + delegate := &sfHistogram{} + testFloat64Race(delegate.Record, delegate.setDelegate) }) }) @@ -229,66 +120,18 @@ func TestSyncInstrumentSetDelegateRace(t *testing.T) { t.Run("Int64", func(t *testing.T) { t.Run("Counter", func(t *testing.T) { - delegate := &siCounter{ - name: "testName", - opts: []instrument.Option{}, - } - finish := make(chan struct{}) - go func() { - for { - delegate.Add(context.Background(), 1) - select { - case <-finish: - return - default: - } - } - }() - - delegate.setDelegate(nonrecording.NewNoopMeter()) - close(finish) + delegate := &siCounter{} + testInt64Race(delegate.Add, delegate.setDelegate) }) t.Run("UpDownCounter", func(t *testing.T) { - delegate := &siUpDownCounter{ - name: "testName", - opts: []instrument.Option{}, - } - finish := make(chan struct{}) - go func() { - for { - delegate.Add(context.Background(), 1) - select { - case <-finish: - return - default: - } - } - }() - - delegate.setDelegate(nonrecording.NewNoopMeter()) - close(finish) + delegate := &siUpDownCounter{} + testInt64Race(delegate.Add, delegate.setDelegate) }) t.Run("Histogram", func(t *testing.T) { - delegate := &siHistogram{ - name: "testName", - opts: []instrument.Option{}, - } - finish := make(chan struct{}) - go func() { - for { - delegate.Record(context.Background(), 1) - select { - case <-finish: - return - default: - } - } - }() - - delegate.setDelegate(nonrecording.NewNoopMeter()) - close(finish) + delegate := &siHistogram{} + testInt64Race(delegate.Record, delegate.setDelegate) }) }) } From f075b204a23357066ee6ab207a97748f55783bed Mon Sep 17 00:00:00 2001 From: Aaron Clawson Date: Fri, 11 Mar 2022 15:39:58 +0000 Subject: [PATCH 11/19] fixed lint, and removed unneeded type checking --- metric/internal/global/meter_test.go | 62 +++++++++++----------------- 1 file changed, 23 insertions(+), 39 deletions(-) diff --git a/metric/internal/global/meter_test.go b/metric/internal/global/meter_test.go index 2cfa66eca88..78ab03b2a54 100644 --- a/metric/internal/global/meter_test.go +++ b/metric/internal/global/meter_test.go @@ -55,7 +55,7 @@ func TestMeterRace(t *testing.T) { wg.Add(1) finish := make(chan struct{}) go func() { - for i, once := 0, false; ; i++{ + for i, once := 0, false; ; i++ { name := fmt.Sprintf("a%d", i) _, _ = mtr.AsyncFloat64().Counter(name) _, _ = mtr.AsyncFloat64().UpDownCounter(name) @@ -166,24 +166,19 @@ func TestMeterProviderDelegatesCalls(t *testing.T) { // Calls to Meter() after setDelegate() should be executed by the delegate require.IsType(t, &testMeter{}, meter) - if tMeter, ok := meter.(*testMeter); ok { - require.Equal(t, 3, tMeter.afCount) - require.Equal(t, 3, tMeter.aiCount) - require.Equal(t, 3, tMeter.sfCount) - require.Equal(t, 3, tMeter.siCount) - require.Equal(t, 1, len(tMeter.callbacks)) - } + tMeter := meter.(*testMeter) + require.Equal(t, 3, tMeter.afCount) + require.Equal(t, 3, tMeter.aiCount) + require.Equal(t, 3, tMeter.sfCount) + require.Equal(t, 3, tMeter.siCount) + require.Equal(t, 1, len(tMeter.callbacks)) // Because the Meter was provided by testmeterProvider it should also return our test instrument require.IsType(t, &testCountingFloatInstrument{}, ctr, "the meter did not delegate calls to the meter") - if testCtr, ok := ctr.(*testCountingFloatInstrument); ok { - require.Equal(t, 1, testCtr.count) - } + require.Equal(t, 1, ctr.(*testCountingFloatInstrument).count) require.IsType(t, &testCountingFloatInstrument{}, actr, "the meter did not delegate calls to the meter") - if testCtr, ok := actr.(*testCountingFloatInstrument); ok { - require.Equal(t, 1, testCtr.count) - } + require.Equal(t, 1, actr.(*testCountingFloatInstrument).count) require.Equal(t, 1, mp.count) } @@ -213,26 +208,20 @@ func TestMeterDelegatesCalls(t *testing.T) { // Calls to Meter methods after setDelegate() should be executed by the delegate require.IsType(t, &meter{}, m) - if dMeter, ok := m.(*meter); ok { - m := dMeter.delegate.Load().(*testMeter) - require.NotNil(t, m) - require.Equal(t, 3, m.afCount) - require.Equal(t, 3, m.aiCount) - require.Equal(t, 3, m.sfCount) - require.Equal(t, 3, m.siCount) - } + tMeter := m.(*meter).delegate.Load().(*testMeter) + require.NotNil(t, tMeter) + require.Equal(t, 3, tMeter.afCount) + require.Equal(t, 3, tMeter.aiCount) + require.Equal(t, 3, tMeter.sfCount) + require.Equal(t, 3, tMeter.siCount) // Because the Meter was provided by testmeterProvider it should also return our test instrument require.IsType(t, &testCountingFloatInstrument{}, ctr, "the meter did not delegate calls to the meter") - if testCtr, ok := ctr.(*testCountingFloatInstrument); ok { - require.Equal(t, 1, testCtr.count) - } + require.Equal(t, 1, ctr.(*testCountingFloatInstrument).count) // Because the Meter was provided by testmeterProvider it should also return our test instrument require.IsType(t, &testCountingFloatInstrument{}, actr, "the meter did not delegate calls to the meter") - if testCtr, ok := actr.(*testCountingFloatInstrument); ok { - require.Equal(t, 1, testCtr.count) - } + require.Equal(t, 1, actr.(*testCountingFloatInstrument).count) require.Equal(t, 1, mp.count) } @@ -260,21 +249,16 @@ func TestMeterDefersDelegations(t *testing.T) { // Calls to Meter() before setDelegate() should be the delegated type require.IsType(t, &meter{}, m) - - if dMeter, ok := m.(*meter); ok { - m := dMeter.delegate.Load().(*testMeter) - require.NotNil(t, m) - require.Equal(t, 3, m.afCount) - require.Equal(t, 3, m.aiCount) - require.Equal(t, 3, m.sfCount) - require.Equal(t, 3, m.siCount) - } + tMeter := m.(*meter).delegate.Load().(*testMeter) + require.NotNil(t, tMeter) + require.Equal(t, 3, tMeter.afCount) + require.Equal(t, 3, tMeter.aiCount) + require.Equal(t, 3, tMeter.sfCount) + require.Equal(t, 3, tMeter.siCount) // Because the Meter was a delegate it should return a delegated instrument require.IsType(t, &sfCounter{}, ctr) - require.IsType(t, &afCounter{}, actr) - require.Equal(t, 1, mp.count) } From 3f666696ebe35e195a6d080ab552b8e6fe55b1e3 Mon Sep 17 00:00:00 2001 From: Aaron Clawson Date: Fri, 11 Mar 2022 19:43:15 +0000 Subject: [PATCH 12/19] change require's to asserts. --- metric/internal/global/meter_test.go | 71 ++++++++++++++-------------- 1 file changed, 36 insertions(+), 35 deletions(-) diff --git a/metric/internal/global/meter_test.go b/metric/internal/global/meter_test.go index 78ab03b2a54..8efd89af60c 100644 --- a/metric/internal/global/meter_test.go +++ b/metric/internal/global/meter_test.go @@ -20,6 +20,7 @@ import ( "sync" "testing" + "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" "go.opentelemetry.io/otel/metric" @@ -82,7 +83,7 @@ func TestMeterRace(t *testing.T) { } }() - wg.Wait() + // wg.Wait() mtr.setDelegate(nonrecording.NewNoopMeterProvider()) close(finish) } @@ -92,16 +93,16 @@ func testSetupAllInstrumentTypes(t *testing.T, m metric.Meter) (syncfloat64.Coun afcounter, err := m.AsyncFloat64().Counter("test_Async_Counter") require.NoError(t, err) _, err = m.AsyncFloat64().UpDownCounter("test_Async_UpDownCounter") - require.NoError(t, err) + assert.NoError(t, err) _, err = m.AsyncFloat64().Gauge("test_Async_Gauge") - require.NoError(t, err) + assert.NoError(t, err) _, err = m.AsyncInt64().Counter("test_Async_Counter") - require.NoError(t, err) + assert.NoError(t, err) _, err = m.AsyncInt64().UpDownCounter("test_Async_UpDownCounter") - require.NoError(t, err) + assert.NoError(t, err) _, err = m.AsyncInt64().Gauge("test_Async_Gauge") - require.NoError(t, err) + assert.NoError(t, err) require.NoError(t, m.RegisterCallback([]instrument.Asynchronous{afcounter}, func(ctx context.Context) { afcounter.Observe(ctx, 3) @@ -110,16 +111,16 @@ func testSetupAllInstrumentTypes(t *testing.T, m metric.Meter) (syncfloat64.Coun sfcounter, err := m.SyncFloat64().Counter("test_Async_Counter") require.NoError(t, err) _, err = m.SyncFloat64().UpDownCounter("test_Async_UpDownCounter") - require.NoError(t, err) + assert.NoError(t, err) _, err = m.SyncFloat64().Histogram("test_Async_Histogram") - require.NoError(t, err) + assert.NoError(t, err) _, err = m.SyncInt64().Counter("test_Async_Counter") - require.NoError(t, err) + assert.NoError(t, err) _, err = m.SyncInt64().UpDownCounter("test_Async_UpDownCounter") - require.NoError(t, err) + assert.NoError(t, err) _, err = m.SyncInt64().Histogram("test_Async_Histogram") - require.NoError(t, err) + assert.NoError(t, err) return sfcounter, afcounter } @@ -154,7 +155,7 @@ func TestMeterProviderDelegatesCalls(t *testing.T) { // otel.SetMeterProvider(mp) globalMeterProvider.setDelegate(mp) - require.Equal(t, 0, mp.count) + assert.Equal(t, 0, mp.count) meter := globalMeterProvider.Meter("go.opentelemetry.io/otel/metric/internal/global/meter_test") @@ -167,20 +168,20 @@ func TestMeterProviderDelegatesCalls(t *testing.T) { // Calls to Meter() after setDelegate() should be executed by the delegate require.IsType(t, &testMeter{}, meter) tMeter := meter.(*testMeter) - require.Equal(t, 3, tMeter.afCount) - require.Equal(t, 3, tMeter.aiCount) - require.Equal(t, 3, tMeter.sfCount) - require.Equal(t, 3, tMeter.siCount) - require.Equal(t, 1, len(tMeter.callbacks)) + assert.Equal(t, 3, tMeter.afCount) + assert.Equal(t, 3, tMeter.aiCount) + assert.Equal(t, 3, tMeter.sfCount) + assert.Equal(t, 3, tMeter.siCount) + assert.Equal(t, 1, len(tMeter.callbacks)) // Because the Meter was provided by testmeterProvider it should also return our test instrument require.IsType(t, &testCountingFloatInstrument{}, ctr, "the meter did not delegate calls to the meter") - require.Equal(t, 1, ctr.(*testCountingFloatInstrument).count) + assert.Equal(t, 1, ctr.(*testCountingFloatInstrument).count) require.IsType(t, &testCountingFloatInstrument{}, actr, "the meter did not delegate calls to the meter") - require.Equal(t, 1, actr.(*testCountingFloatInstrument).count) + assert.Equal(t, 1, actr.(*testCountingFloatInstrument).count) - require.Equal(t, 1, mp.count) + assert.Equal(t, 1, mp.count) } func TestMeterDelegatesCalls(t *testing.T) { @@ -194,7 +195,7 @@ func TestMeterDelegatesCalls(t *testing.T) { mp := &testMeterProvider{} - require.Equal(t, 0, mp.count) + assert.Equal(t, 0, mp.count) m := globalMeterProvider.Meter("go.opentelemetry.io/otel/metric/internal/global/meter_test") @@ -210,20 +211,20 @@ func TestMeterDelegatesCalls(t *testing.T) { require.IsType(t, &meter{}, m) tMeter := m.(*meter).delegate.Load().(*testMeter) require.NotNil(t, tMeter) - require.Equal(t, 3, tMeter.afCount) - require.Equal(t, 3, tMeter.aiCount) - require.Equal(t, 3, tMeter.sfCount) - require.Equal(t, 3, tMeter.siCount) + assert.Equal(t, 3, tMeter.afCount) + assert.Equal(t, 3, tMeter.aiCount) + assert.Equal(t, 3, tMeter.sfCount) + assert.Equal(t, 3, tMeter.siCount) // Because the Meter was provided by testmeterProvider it should also return our test instrument require.IsType(t, &testCountingFloatInstrument{}, ctr, "the meter did not delegate calls to the meter") - require.Equal(t, 1, ctr.(*testCountingFloatInstrument).count) + assert.Equal(t, 1, ctr.(*testCountingFloatInstrument).count) // Because the Meter was provided by testmeterProvider it should also return our test instrument require.IsType(t, &testCountingFloatInstrument{}, actr, "the meter did not delegate calls to the meter") - require.Equal(t, 1, actr.(*testCountingFloatInstrument).count) + assert.Equal(t, 1, actr.(*testCountingFloatInstrument).count) - require.Equal(t, 1, mp.count) + assert.Equal(t, 1, mp.count) } func TestMeterDefersDelegations(t *testing.T) { @@ -251,14 +252,14 @@ func TestMeterDefersDelegations(t *testing.T) { require.IsType(t, &meter{}, m) tMeter := m.(*meter).delegate.Load().(*testMeter) require.NotNil(t, tMeter) - require.Equal(t, 3, tMeter.afCount) - require.Equal(t, 3, tMeter.aiCount) - require.Equal(t, 3, tMeter.sfCount) - require.Equal(t, 3, tMeter.siCount) + assert.Equal(t, 3, tMeter.afCount) + assert.Equal(t, 3, tMeter.aiCount) + assert.Equal(t, 3, tMeter.sfCount) + assert.Equal(t, 3, tMeter.siCount) // Because the Meter was a delegate it should return a delegated instrument - require.IsType(t, &sfCounter{}, ctr) - require.IsType(t, &afCounter{}, actr) - require.Equal(t, 1, mp.count) + assert.IsType(t, &sfCounter{}, ctr) + assert.IsType(t, &afCounter{}, actr) + assert.Equal(t, 1, mp.count) } From 503228ad53a6118a0ddc8b2f0abf801a88465f2f Mon Sep 17 00:00:00 2001 From: Aaron Clawson Date: Fri, 11 Mar 2022 13:44:13 -0600 Subject: [PATCH 13/19] Update misspelling Co-authored-by: Tyler Yahn --- metric/internal/global/state_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/metric/internal/global/state_test.go b/metric/internal/global/state_test.go index d7e7ed46900..34e7d0cb16d 100644 --- a/metric/internal/global/state_test.go +++ b/metric/internal/global/state_test.go @@ -41,7 +41,7 @@ func TestSetMeterProvider(t *testing.T) { return } if mp.delegate != nil { - t.Error("meter provider should not delegat when setting itself") + t.Error("meter provider should not delegate when setting itself") } }) From d63de58152223a684d75a99e72461813a8538908 Mon Sep 17 00:00:00 2001 From: Aaron Clawson <3766680+MadVikingGod@users.noreply.github.com> Date: Mon, 14 Mar 2022 15:12:29 +0000 Subject: [PATCH 14/19] Fix meter race test. --- metric/internal/global/meter_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/metric/internal/global/meter_test.go b/metric/internal/global/meter_test.go index 8efd89af60c..481c55e2273 100644 --- a/metric/internal/global/meter_test.go +++ b/metric/internal/global/meter_test.go @@ -83,7 +83,7 @@ func TestMeterRace(t *testing.T) { } }() - // wg.Wait() + wg.Wait() mtr.setDelegate(nonrecording.NewNoopMeterProvider()) close(finish) } From c90fb7e6f56515a8dd73c57921d78baad1f45499 Mon Sep 17 00:00:00 2001 From: Aaron Clawson <3766680+MadVikingGod@users.noreply.github.com> Date: Fri, 18 Mar 2022 13:42:29 +0000 Subject: [PATCH 15/19] Copy SetTracerProvider logic. --- metric/internal/global/state.go | 15 +++++++-------- 1 file changed, 7 insertions(+), 8 deletions(-) diff --git a/metric/internal/global/state.go b/metric/internal/global/state.go index 21b71a36be0..29a67c5dbe4 100644 --- a/metric/internal/global/state.go +++ b/metric/internal/global/state.go @@ -38,17 +38,16 @@ func MeterProvider() metric.MeterProvider { // SetMeterProvider is the internal implementation for global.SetMeterProvider. func SetMeterProvider(mp metric.MeterProvider) { - // Guard against SetMeterProvider(MeterProvider()) - current := MeterProvider() - if current == mp { - return - } - delegateMeterOnce.Do(func() { - if def, ok := current.(*meterProvider); ok { + current := MeterProvider() + if current == mp { + // Setting the provider to the prior default is nonsense, panic. + // Panic is acceptable because we are likely still early in the + // process lifetime. + panic("invalid MeterProvider, the global instance cannot be reinstalled") + } else if def, ok := current.(*meterProvider); ok { def.setDelegate(mp) } - }) globalMeterProvider.Store(meterProviderHolder{mp: mp}) } From 713a5b104dc41d95adbf0efb3845a86188331640 Mon Sep 17 00:00:00 2001 From: Aaron Clawson <3766680+MadVikingGod@users.noreply.github.com> Date: Mon, 21 Mar 2022 13:37:10 +0000 Subject: [PATCH 16/19] Fix global test for panic. --- metric/internal/global/state_test.go | 17 +++++------------ 1 file changed, 5 insertions(+), 12 deletions(-) diff --git a/metric/internal/global/state_test.go b/metric/internal/global/state_test.go index 34e7d0cb16d..b3d2c40c751 100644 --- a/metric/internal/global/state_test.go +++ b/metric/internal/global/state_test.go @@ -18,6 +18,7 @@ import ( "sync" "testing" + "github.com/stretchr/testify/assert" "go.opentelemetry.io/otel/metric/nonrecording" ) @@ -29,20 +30,12 @@ func resetGlobalMeterProvider() { func TestSetMeterProvider(t *testing.T) { t.Cleanup(resetGlobalMeterProvider) - t.Run("Set With default is no op", func(t *testing.T) { + t.Run("Set With default panics", func(t *testing.T) { resetGlobalMeterProvider() - // This action should have no effect, nothing should be delegated - SetMeterProvider(MeterProvider()) - - mp, ok := MeterProvider().(*meterProvider) - if !ok { - t.Error("Global Meter Provider was changed") - return - } - if mp.delegate != nil { - t.Error("meter provider should not delegate when setting itself") - } + assert.Panics(t, func() { + SetMeterProvider(MeterProvider()) + }) }) From 560b92b70bdabbbd2f5156f8ea2becdaeb93bdc9 Mon Sep 17 00:00:00 2001 From: Aaron Clawson <3766680+MadVikingGod@users.noreply.github.com> Date: Mon, 21 Mar 2022 13:49:55 +0000 Subject: [PATCH 17/19] Fix linting error --- metric/internal/global/state_test.go | 1 + 1 file changed, 1 insertion(+) diff --git a/metric/internal/global/state_test.go b/metric/internal/global/state_test.go index b3d2c40c751..69cb9b917d6 100644 --- a/metric/internal/global/state_test.go +++ b/metric/internal/global/state_test.go @@ -19,6 +19,7 @@ import ( "testing" "github.com/stretchr/testify/assert" + "go.opentelemetry.io/otel/metric/nonrecording" ) From a6f16ef3d60b08f83df5f512cf3808261be6126f Mon Sep 17 00:00:00 2001 From: Aaron Clawson <3766680+MadVikingGod@users.noreply.github.com> Date: Mon, 21 Mar 2022 18:37:21 +0000 Subject: [PATCH 18/19] bump testify version --- metric/go.mod | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/metric/go.mod b/metric/go.mod index 0cf2d2bd084..9714c1150a6 100644 --- a/metric/go.mod +++ b/metric/go.mod @@ -3,7 +3,7 @@ module go.opentelemetry.io/otel/metric go 1.16 require ( - github.com/stretchr/testify v1.7.0 + github.com/stretchr/testify v1.7.1 go.opentelemetry.io/otel v1.5.0 ) From dd15b7c60b9cc60fdfdabb1c05e8330025910b66 Mon Sep 17 00:00:00 2001 From: Aaron Clawson <3766680+MadVikingGod@users.noreply.github.com> Date: Tue, 22 Mar 2022 15:11:07 +0000 Subject: [PATCH 19/19] moved changelog into unreleased --- CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index e255ca55ae3..c6465e99e34 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -17,6 +17,7 @@ Code instrumented with the `go.opentelemetry.io/otel/metric` will need to be mod - Add go 1.18 to our compatibility tests. (#2679) - Allow configuring the Sampler with the `OTEL_TRACES_SAMPLER` and `OTEL_TRACES_SAMPLER_ARG` environment variables. (#2305, #2517) +- Add the `metric/global` for obtaining and setting the global `MeterProvider` (#2660) ### Changed @@ -51,7 +52,6 @@ Code instrumented with the `go.opentelemetry.io/otel/metric` will need to be mod Zero or negative values will not be changed to the default value like `WithSpanLimits` does. Setting a limit to zero will effectively disable the related resource it limits and setting to a negative value will mean that resource is unlimited. Consequentially, limits should be constructed using `NewSpanLimits` and updated accordingly. (#2637) -- Add the `metric/global` for obtaining and setting the global `MeterProvider` (#2660) ### Changed