From 14a7c30b23200c6f7f7cf9d3b6a1412779913237 Mon Sep 17 00:00:00 2001 From: Tyler Yahn Date: Tue, 10 Jan 2023 11:32:14 -0800 Subject: [PATCH 01/31] Update RegisterCallback and Callback decls RegisterCallback accept variadic Asynchronous instruments instead of a slice. Callback accept an observation result recorder to ensure instruments that are observed by a callback. --- metric/meter.go | 16 ++++++++++++++-- 1 file changed, 14 insertions(+), 2 deletions(-) diff --git a/metric/meter.go b/metric/meter.go index 41cbf91f255..559f55e67f4 100644 --- a/metric/meter.go +++ b/metric/meter.go @@ -17,6 +17,7 @@ package metric // import "go.opentelemetry.io/otel/metric" import ( "context" + "go.opentelemetry.io/otel/attribute" "go.opentelemetry.io/otel/metric/instrument" ) @@ -102,12 +103,15 @@ type Meter interface { // // If no instruments are passed, f should not be registered nor called // during collection. - RegisterCallback(instruments []instrument.Asynchronous, f Callback) (Registration, error) + RegisterCallback(f Callback, instruments ...instrument.Asynchronous) (Registration, error) } // Callback is a function registered with a Meter that makes observations for // the set of instruments it is registered with. // +// The ObservationRecorder parameter is used to record measurment observations. +// Measurments should not be recorded directly with an Observer. +// // The function needs to complete in a finite amount of time and the deadline // of the passed context is expected to be honored. // @@ -116,7 +120,15 @@ type Meter interface { // the same attributes as another Callback will report. // // The function needs to be concurrent safe. -type Callback func(context.Context) error +type Callback func(context.Context, ObservationRecorder) error + +// ObservationRecorder records the measurements in a Callback. +type ObservationRecorder interface { + // Float64 records the float64 value with attributes for inst. + Float64(inst instrument.Float64Observer, value float64, attributes ...attribute.KeyValue) + // Int64 records the int64 value with attributes for inst. + Int64(inst instrument.Int64Observer, value int64, attributes ...attribute.KeyValue) +} // Registration is an token representing the unique registration of a callback // for a set of instruments with a Meter. From 4a0d32ff457dfe5e11853f89040ee21922a4dc82 Mon Sep 17 00:00:00 2001 From: Tyler Yahn Date: Tue, 10 Jan 2023 11:33:23 -0800 Subject: [PATCH 02/31] Update global impl --- metric/internal/global/meter.go | 6 +++--- metric/internal/global/meter_test.go | 22 +++++++++++++--------- metric/internal/global/meter_types_test.go | 18 ++++++++++++++++-- 3 files changed, 32 insertions(+), 14 deletions(-) diff --git a/metric/internal/global/meter.go b/metric/internal/global/meter.go index 97ad2735bea..92f35e9730b 100644 --- a/metric/internal/global/meter.go +++ b/metric/internal/global/meter.go @@ -278,10 +278,10 @@ func (m *meter) Float64ObservableGauge(name string, options ...instrument.Float6 // // 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, f metric.Callback) (metric.Registration, error) { +func (m *meter) RegisterCallback(f metric.Callback, insts ...instrument.Asynchronous) (metric.Registration, error) { if del, ok := m.delegate.Load().(metric.Meter); ok { insts = unwrapInstruments(insts) - return del.RegisterCallback(insts, f) + return del.RegisterCallback(f, insts...) } m.mtx.Lock() @@ -335,7 +335,7 @@ func (c *registration) setDelegate(m metric.Meter) { return } - reg, err := m.RegisterCallback(insts, c.function) + reg, err := m.RegisterCallback(c.function, insts...) if err != nil { otel.Handle(err) } diff --git a/metric/internal/global/meter_test.go b/metric/internal/global/meter_test.go index 0260a7dedb3..1286176ca2e 100644 --- a/metric/internal/global/meter_test.go +++ b/metric/internal/global/meter_test.go @@ -45,6 +45,10 @@ func TestMeterProviderRace(t *testing.T) { close(finish) } +var zeroCallback metric.Callback = func(ctx context.Context, or metric.ObservationRecorder) error { + return nil +} + func TestMeterRace(t *testing.T) { mtr := &meter{} @@ -66,7 +70,7 @@ func TestMeterRace(t *testing.T) { _, _ = mtr.Int64Counter(name) _, _ = mtr.Int64UpDownCounter(name) _, _ = mtr.Int64Histogram(name) - _, _ = mtr.RegisterCallback(nil, func(ctx context.Context) error { return nil }) + _, _ = mtr.RegisterCallback(zeroCallback) if !once { wg.Done() once = true @@ -86,7 +90,7 @@ func TestMeterRace(t *testing.T) { func TestUnregisterRace(t *testing.T) { mtr := &meter{} - reg, err := mtr.RegisterCallback(nil, func(ctx context.Context) error { return nil }) + reg, err := mtr.RegisterCallback(zeroCallback) require.NoError(t, err) wg := &sync.WaitGroup{} @@ -128,10 +132,10 @@ func testSetupAllInstrumentTypes(t *testing.T, m metric.Meter) (instrument.Float _, err = m.Int64ObservableGauge("test_Async_Gauge") assert.NoError(t, err) - _, err = m.RegisterCallback([]instrument.Asynchronous{afcounter}, func(ctx context.Context) error { - afcounter.Observe(ctx, 3) + _, err = m.RegisterCallback(func(ctx context.Context, obs metric.ObservationRecorder) error { + obs.Float64(afcounter, 3) return nil - }) + }, afcounter) require.NoError(t, err) sfcounter, err := m.Float64Counter("test_Async_Counter") @@ -323,10 +327,10 @@ func TestRegistrationDelegation(t *testing.T) { require.NoError(t, err) var called0 bool - reg0, err := m.RegisterCallback([]instrument.Asynchronous{actr}, func(context.Context) error { + reg0, err := m.RegisterCallback(func(context.Context, metric.ObservationRecorder) error { called0 = true return nil - }) + }, actr) require.NoError(t, err) require.Equal(t, 1, mImpl.registry.Len(), "callback not registered") // This means reg0 should not be delegated. @@ -334,10 +338,10 @@ func TestRegistrationDelegation(t *testing.T) { assert.Equal(t, 0, mImpl.registry.Len(), "callback not unregistered") var called1 bool - reg1, err := m.RegisterCallback([]instrument.Asynchronous{actr}, func(context.Context) error { + reg1, err := m.RegisterCallback(func(context.Context, metric.ObservationRecorder) error { called1 = true return nil - }) + }, actr) require.NoError(t, err) require.Equal(t, 1, mImpl.registry.Len(), "second callback not registered") diff --git a/metric/internal/global/meter_types_test.go b/metric/internal/global/meter_types_test.go index 57711f29e62..57e5214bdaa 100644 --- a/metric/internal/global/meter_types_test.go +++ b/metric/internal/global/meter_types_test.go @@ -17,6 +17,7 @@ package global // import "go.opentelemetry.io/otel/metric/internal/global" import ( "context" + "go.opentelemetry.io/otel/attribute" "go.opentelemetry.io/otel/metric" "go.opentelemetry.io/otel/metric/instrument" ) @@ -115,7 +116,7 @@ func (m *testMeter) Float64ObservableGauge(name string, options ...instrument.Fl // // 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 *testMeter) RegisterCallback(i []instrument.Asynchronous, f metric.Callback) (metric.Registration, error) { +func (m *testMeter) RegisterCallback(f metric.Callback, i ...instrument.Asynchronous) (metric.Registration, error) { m.callbacks = append(m.callbacks, f) return testReg{ f: func(idx int) func() { @@ -136,11 +137,24 @@ func (r testReg) Unregister() error { // This enables async collection. func (m *testMeter) collect() { ctx := context.Background() + o := observationRecorder{ctx} for _, f := range m.callbacks { if f == nil { // Unregister. continue } - _ = f(ctx) + _ = f(ctx, o) } } + +type observationRecorder struct { + ctx context.Context +} + +func (o observationRecorder) Float64(i instrument.Float64Observer, value float64, attr ...attribute.KeyValue) { + i.Observe(o.ctx, value, attr...) +} + +func (o observationRecorder) Int64(i instrument.Int64Observer, value int64, attr ...attribute.KeyValue) { + i.Observe(o.ctx, value, attr...) +} From 7110c780342b591dcc97936ef9d3e0ab2c7be60d Mon Sep 17 00:00:00 2001 From: Tyler Yahn Date: Tue, 10 Jan 2023 11:33:38 -0800 Subject: [PATCH 03/31] Update noop impl --- metric/noop.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/metric/noop.go b/metric/noop.go index 98652b4fac7..409268ecc98 100644 --- a/metric/noop.go +++ b/metric/noop.go @@ -88,7 +88,7 @@ func (noopMeter) Float64ObservableGauge(string, ...instrument.Float64ObserverOpt } // RegisterCallback creates a register callback that does not record any metrics. -func (noopMeter) RegisterCallback([]instrument.Asynchronous, Callback) (Registration, error) { +func (noopMeter) RegisterCallback(Callback, ...instrument.Asynchronous) (Registration, error) { return noopReg{}, nil } From e87ea5f64cad26625be59674fbd3ba0ca97ab400 Mon Sep 17 00:00:00 2001 From: Tyler Yahn Date: Tue, 10 Jan 2023 16:17:42 -0800 Subject: [PATCH 04/31] Update SDK impl --- sdk/metric/instrument.go | 109 ++++++++++++++++----- sdk/metric/meter.go | 186 ++++++++++++++++++++++++++++++------ sdk/metric/meter_test.go | 133 +++++++++++++------------- sdk/metric/pipeline.go | 8 +- sdk/metric/pipeline_test.go | 6 +- 5 files changed, 318 insertions(+), 124 deletions(-) diff --git a/sdk/metric/instrument.go b/sdk/metric/instrument.go index 5414a8db7e6..e808c1564e5 100644 --- a/sdk/metric/instrument.go +++ b/sdk/metric/instrument.go @@ -16,8 +16,11 @@ package metric // import "go.opentelemetry.io/otel/sdk/metric" import ( "context" + "errors" + "fmt" "go.opentelemetry.io/otel/attribute" + "go.opentelemetry.io/otel/internal/global" "go.opentelemetry.io/otel/metric/instrument" "go.opentelemetry.io/otel/metric/unit" "go.opentelemetry.io/otel/sdk/instrumentation" @@ -170,33 +173,17 @@ type instrumentID struct { } type instrumentImpl[N int64 | float64] struct { - instrument.Asynchronous instrument.Synchronous aggregators []internal.Aggregator[N] } -var _ instrument.Float64ObservableCounter = &instrumentImpl[float64]{} -var _ instrument.Float64ObservableUpDownCounter = &instrumentImpl[float64]{} -var _ instrument.Float64ObservableGauge = &instrumentImpl[float64]{} -var _ instrument.Int64ObservableCounter = &instrumentImpl[int64]{} -var _ instrument.Int64ObservableUpDownCounter = &instrumentImpl[int64]{} -var _ instrument.Int64ObservableGauge = &instrumentImpl[int64]{} -var _ instrument.Float64Counter = &instrumentImpl[float64]{} -var _ instrument.Float64UpDownCounter = &instrumentImpl[float64]{} -var _ instrument.Float64Histogram = &instrumentImpl[float64]{} -var _ instrument.Int64Counter = &instrumentImpl[int64]{} -var _ instrument.Int64UpDownCounter = &instrumentImpl[int64]{} -var _ instrument.Int64Histogram = &instrumentImpl[int64]{} - -func (i *instrumentImpl[N]) Observe(ctx context.Context, val N, attrs ...attribute.KeyValue) { - // Only record a value if this is being called from the MetricProvider. - _, ok := ctx.Value(produceKey).(struct{}) - if !ok { - return - } - i.aggregate(ctx, val, attrs) -} +var _ instrument.Float64Counter = (*instrumentImpl[float64])(nil) +var _ instrument.Float64UpDownCounter = (*instrumentImpl[float64])(nil) +var _ instrument.Float64Histogram = (*instrumentImpl[float64])(nil) +var _ instrument.Int64Counter = (*instrumentImpl[int64])(nil) +var _ instrument.Int64UpDownCounter = (*instrumentImpl[int64])(nil) +var _ instrument.Int64Histogram = (*instrumentImpl[int64])(nil) func (i *instrumentImpl[N]) Add(ctx context.Context, val N, attrs ...attribute.KeyValue) { i.aggregate(ctx, val, attrs) @@ -214,3 +201,81 @@ func (i *instrumentImpl[N]) aggregate(ctx context.Context, val N, attrs []attrib agg.Aggregate(val, attribute.NewSet(attrs...)) } } + +// observerID is a comparable unique identifier of an observer. +type observerID[N int64 | float64] struct { + name string + description string + kind InstrumentKind + unit unit.Unit + scope instrumentation.Scope +} + +type observer[N int64 | float64] struct { + instrument.Asynchronous + observerID[N] + + aggregators []internal.Aggregator[N] +} + +func newObserver[N int64 | float64](scope instrumentation.Scope, kind InstrumentKind, name, desc string, u unit.Unit, agg []internal.Aggregator[N]) *observer[N] { + return &observer[N]{ + observerID: observerID[N]{ + name: name, + description: desc, + kind: kind, + unit: u, + scope: scope, + }, + aggregators: agg, + } +} + +var _ instrument.Float64ObservableCounter = (*observer[float64])(nil) +var _ instrument.Float64ObservableUpDownCounter = (*observer[float64])(nil) +var _ instrument.Float64ObservableGauge = (*observer[float64])(nil) +var _ instrument.Int64ObservableCounter = (*observer[int64])(nil) +var _ instrument.Int64ObservableUpDownCounter = (*observer[int64])(nil) +var _ instrument.Int64ObservableGauge = (*observer[int64])(nil) + +// Observe logs an error. +func (o *observer[N]) Observe(ctx context.Context, val N, attrs ...attribute.KeyValue) { + var zero N + err := errors.New("invalid observation recording") + global.Error(err, "dropping measurement", + "name", o.name, + "description", o.description, + "unit", o.unit, + "number", fmt.Sprintf("%T", zero), + ) +} + +func (o *observer[N]) observe(ctx context.Context, val N, attrs []attribute.KeyValue) { + if err := ctx.Err(); err != nil { + return + } + for _, agg := range o.aggregators { + agg.Aggregate(val, attribute.NewSet(attrs...)) + } +} + +var errEmptyAgg = errors.New("no aggregators for observer") + +func (o *observer[N]) registerable(scope instrumentation.Scope) error { + if len(o.aggregators) == 0 { + return errEmptyAgg + } + if scope != o.scope { + return fmt.Errorf( + "invalid registration: observer %q from Meter %q, registered with Meter %q", + o.name, + scope.Name, + o.scope.Name, + ) + } + return nil +} + +type registerabler interface { + registerable(instrumentation.Scope) error +} diff --git a/sdk/metric/meter.go b/sdk/metric/meter.go index 7aabb2a999e..07294a04d67 100644 --- a/sdk/metric/meter.go +++ b/sdk/metric/meter.go @@ -16,11 +16,16 @@ package metric // import "go.opentelemetry.io/otel/sdk/metric" import ( "context" + "errors" + "fmt" + "go.opentelemetry.io/otel/attribute" + "go.opentelemetry.io/otel/internal/global" "go.opentelemetry.io/otel/metric" "go.opentelemetry.io/otel/metric/instrument" "go.opentelemetry.io/otel/metric/unit" "go.opentelemetry.io/otel/sdk/instrumentation" + "go.opentelemetry.io/otel/sdk/metric/internal" ) // meter handles the creation and coordination of all metric instruments. A @@ -28,6 +33,7 @@ import ( // produced by an instrumentation scope will use metric instruments from a // single meter. type meter struct { + scope instrumentation.Scope pipes pipelines int64IP *instProvider[int64] @@ -45,6 +51,7 @@ func newMeter(s instrumentation.Scope, p pipelines) *meter { fc := newInstrumentCache[float64](nil, &viewCache) return &meter{ + scope: s, pipes: p, int64IP: newInstProvider(s, p, ic), float64IP: newInstProvider(s, p, fc), @@ -200,29 +207,126 @@ func (m *meter) Float64ObservableGauge(name string, options ...instrument.Float6 // RegisterCallback registers the function f to be called when any of the // insts Collect method is called. -func (m *meter) RegisterCallback(insts []instrument.Asynchronous, f metric.Callback) (metric.Registration, error) { +func (m *meter) RegisterCallback(f metric.Callback, insts ...instrument.Asynchronous) (metric.Registration, error) { + if len(insts) == 0 { + // Don't allocate an obeservationRegistry if not needed. + return noopRegister{}, nil + } + + reg := newObservationRegistry() + var errs multierror for _, inst := range insts { - // Only register if at least one instrument has a non-drop aggregation. - // Otherwise, calling f during collection will be wasted computation. - switch t := inst.(type) { - case *instrumentImpl[int64]: - if len(t.aggregators) > 0 { - return m.registerMultiCallback(f) + switch o := inst.(type) { + case *observer[int64]: + if err := o.registerable(m.scope); err != nil { + if !errors.Is(err, errEmptyAgg) { + errs.append(err) + } + continue } - case *instrumentImpl[float64]: - if len(t.aggregators) > 0 { - return m.registerMultiCallback(f) + reg.registerInt64(o.observerID) + case *observer[float64]: + if err := o.registerable(m.scope); err != nil { + if !errors.Is(err, errEmptyAgg) { + errs.append(err) + } + continue } + reg.registerFloat64(o.observerID) default: - // Instrument external to the SDK. For example, an instrument from - // the "go.opentelemetry.io/otel/metric/internal/global" package. - // - // Fail gracefully here, assume a valid instrument. - return m.registerMultiCallback(f) + // Instrument external to the SDK. + return nil, fmt.Errorf("invalid observer: from different implementation") } } - // All insts use drop aggregation. - return noopRegister{}, nil + + if err := errs.errorOrNil(); err != nil { + return nil, err + } + + if reg.len() == 0 { + // All insts use drop aggregation. + return noopRegister{}, nil + } + + cback := func(ctx context.Context) error { + return f(ctx, reg.recorder(ctx)) + } + return m.pipes.registerMultiCallback(cback), nil +} + +type obeservationRegistry struct { + float64 map[observerID[float64]]struct{} + int64 map[observerID[int64]]struct{} +} + +func newObservationRegistry() obeservationRegistry { + return obeservationRegistry{ + float64: make(map[observerID[float64]]struct{}), + int64: make(map[observerID[int64]]struct{}), + } +} + +func (r obeservationRegistry) len() int { + return len(r.float64) + len(r.int64) +} + +func (r obeservationRegistry) registerFloat64(id observerID[float64]) { + r.float64[id] = struct{}{} +} + +func (r obeservationRegistry) registerInt64(id observerID[int64]) { + r.int64[id] = struct{}{} +} + +func (r obeservationRegistry) recorder(ctx context.Context) metric.ObservationRecorder { + return obeservationRecorder{obeservationRegistry: r, ctx: ctx} +} + +type obeservationRecorder struct { + obeservationRegistry + + ctx context.Context +} + +var ( + errUnknownObserver = errors.New("unknown observer") + errUnregObserver = errors.New("observer not registered for callback") +) + +func (r obeservationRecorder) Float64(o instrument.Float64Observer, v float64, a ...attribute.KeyValue) { + oImpl, ok := o.(*observer[float64]) + if !ok { + global.Error(errUnknownObserver, "failed to record") + return + } + if _, registered := r.float64[oImpl.observerID]; !registered { + global.Error(errUnregObserver, "failed to record", + "name", oImpl.name, + "description", oImpl.description, + "unit", oImpl.unit, + "number", fmt.Sprintf("%T", float64(0)), + ) + return + } + oImpl.observe(r.ctx, v, a) +} + +func (r obeservationRecorder) Int64(o instrument.Int64Observer, v int64, a ...attribute.KeyValue) { + oImpl, ok := o.(*observer[int64]) + if !ok { + global.Error(errUnknownObserver, "failed to record") + return + } + if _, registered := r.int64[oImpl.observerID]; !registered { + global.Error(errUnregObserver, "failed to record", + "name", oImpl.name, + "description", oImpl.description, + "unit", oImpl.unit, + "number", fmt.Sprintf("%T", int64(0)), + ) + return + } + oImpl.observe(r.ctx, v, a) } type noopRegister struct{} @@ -231,10 +335,6 @@ func (noopRegister) Unregister() error { return nil } -func (m *meter) registerMultiCallback(c metric.Callback) (metric.Registration, error) { - return m.pipes.registerMultiCallback(c), nil -} - // instProvider provides all OpenTelemetry instruments. type instProvider[N int64 | float64] struct { scope instrumentation.Scope @@ -246,8 +346,7 @@ func newInstProvider[N int64 | float64](s instrumentation.Scope, p pipelines, c return &instProvider[N]{scope: s, pipes: p, resolve: newResolver(p, c)} } -// lookup returns the resolved instrumentImpl. -func (p *instProvider[N]) lookup(kind InstrumentKind, name, desc string, u unit.Unit) (*instrumentImpl[N], error) { +func (p *instProvider[N]) aggs(kind InstrumentKind, name, desc string, u unit.Unit) ([]internal.Aggregator[N], error) { inst := Instrument{ Name: name, Description: desc, @@ -255,13 +354,23 @@ func (p *instProvider[N]) lookup(kind InstrumentKind, name, desc string, u unit. Kind: kind, Scope: p.scope, } - aggs, err := p.resolve.Aggregators(inst) + return p.resolve.Aggregators(inst) +} + +// lookup returns the resolved instrumentImpl. +func (p *instProvider[N]) lookup(kind InstrumentKind, name, desc string, u unit.Unit) (*instrumentImpl[N], error) { + aggs, err := p.aggs(kind, name, desc, u) return &instrumentImpl[N]{aggregators: aggs}, err } type int64ObservProvider struct{ *instProvider[int64] } -func (p int64ObservProvider) registerCallbacks(inst *instrumentImpl[int64], cBacks []instrument.Int64Callback) { +func (p int64ObservProvider) lookup(kind InstrumentKind, name, desc string, u unit.Unit) (*observer[int64], error) { + aggs, err := p.aggs(kind, name, desc, u) + return newObserver(p.scope, kind, name, desc, u, aggs), err +} + +func (p int64ObservProvider) registerCallbacks(inst *observer[int64], cBacks []instrument.Int64Callback) { if inst == nil { // Drop aggregator. return @@ -272,13 +381,19 @@ func (p int64ObservProvider) registerCallbacks(inst *instrumentImpl[int64], cBac } } -func (p int64ObservProvider) callback(i *instrumentImpl[int64], f instrument.Int64Callback) func(context.Context) error { - return func(ctx context.Context) error { return f(ctx, i) } +func (p int64ObservProvider) callback(i *observer[int64], f instrument.Int64Callback) func(context.Context) error { + inst := callbackObserver[int64]{i} + return func(ctx context.Context) error { return f(ctx, inst) } } type float64ObservProvider struct{ *instProvider[float64] } -func (p float64ObservProvider) registerCallbacks(inst *instrumentImpl[float64], cBacks []instrument.Float64Callback) { +func (p float64ObservProvider) lookup(kind InstrumentKind, name, desc string, u unit.Unit) (*observer[float64], error) { + aggs, err := p.aggs(kind, name, desc, u) + return newObserver(p.scope, kind, name, desc, u, aggs), err +} + +func (p float64ObservProvider) registerCallbacks(inst *observer[float64], cBacks []instrument.Float64Callback) { if inst == nil { // Drop aggregator. return @@ -289,6 +404,17 @@ func (p float64ObservProvider) registerCallbacks(inst *instrumentImpl[float64], } } -func (p float64ObservProvider) callback(i *instrumentImpl[float64], f instrument.Float64Callback) func(context.Context) error { - return func(ctx context.Context) error { return f(ctx, i) } +func (p float64ObservProvider) callback(i *observer[float64], f instrument.Float64Callback) func(context.Context) error { + inst := callbackObserver[float64]{i} + return func(ctx context.Context) error { return f(ctx, inst) } +} + +// callbackObserver is passed to a callback where a users is expected to call +// Observe directly to record a measurement. +type callbackObserver[N int64 | float64] struct { + *observer[N] +} + +func (o callbackObserver[N]) Observe(ctx context.Context, val N, attrs ...attribute.KeyValue) { + o.observe(ctx, val, attrs) } diff --git a/sdk/metric/meter_test.go b/sdk/metric/meter_test.go index 44209078fa6..7ede255360e 100644 --- a/sdk/metric/meter_test.go +++ b/sdk/metric/meter_test.go @@ -91,7 +91,7 @@ func TestMeterInstrumentConcurrency(t *testing.T) { wg.Wait() } -var emptyCallback metric.Callback = func(ctx context.Context) error { return nil } +var emptyCallback metric.Callback = func(context.Context, metric.ObservationRecorder) error { return nil } // A Meter Should be able register Callbacks Concurrently. func TestMeterCallbackCreationConcurrency(t *testing.T) { @@ -101,11 +101,11 @@ func TestMeterCallbackCreationConcurrency(t *testing.T) { m := NewMeterProvider().Meter("callback-concurrency") go func() { - _, _ = m.RegisterCallback([]instrument.Asynchronous{}, emptyCallback) + _, _ = m.RegisterCallback(emptyCallback) wg.Done() }() go func() { - _, _ = m.RegisterCallback([]instrument.Asynchronous{}, emptyCallback) + _, _ = m.RegisterCallback(emptyCallback) wg.Done() }() wg.Wait() @@ -113,7 +113,7 @@ func TestMeterCallbackCreationConcurrency(t *testing.T) { func TestNoopCallbackUnregisterConcurrency(t *testing.T) { m := NewMeterProvider().Meter("noop-unregister-concurrency") - reg, err := m.RegisterCallback(nil, emptyCallback) + reg, err := m.RegisterCallback(emptyCallback) require.NoError(t, err) wg := &sync.WaitGroup{} @@ -140,12 +140,10 @@ func TestCallbackUnregisterConcurrency(t *testing.T) { ag, err := meter.Int64ObservableGauge("gauge") require.NoError(t, err) - i := []instrument.Asynchronous{actr} - regCtr, err := meter.RegisterCallback(i, emptyCallback) + regCtr, err := meter.RegisterCallback(emptyCallback, actr) require.NoError(t, err) - i = []instrument.Asynchronous{ag} - regG, err := meter.RegisterCallback(i, emptyCallback) + regG, err := meter.RegisterCallback(emptyCallback, ag) require.NoError(t, err) wg := &sync.WaitGroup{} @@ -181,10 +179,10 @@ func TestMeterCreatesInstruments(t *testing.T) { } ctr, err := m.Int64ObservableCounter("aint", instrument.WithInt64Callback(cback)) assert.NoError(t, err) - _, err = m.RegisterCallback([]instrument.Asynchronous{ctr}, func(ctx context.Context) error { - ctr.Observe(ctx, 3) + _, err = m.RegisterCallback(func(_ context.Context, o metric.ObservationRecorder) error { + o.Int64(ctr, 3) return nil - }) + }, ctr) assert.NoError(t, err) // Observed outside of a callback, it should be ignored. @@ -211,10 +209,10 @@ func TestMeterCreatesInstruments(t *testing.T) { } ctr, err := m.Int64ObservableUpDownCounter("aint", instrument.WithInt64Callback(cback)) assert.NoError(t, err) - _, err = m.RegisterCallback([]instrument.Asynchronous{ctr}, func(ctx context.Context) error { - ctr.Observe(ctx, 11) + _, err = m.RegisterCallback(func(ctx context.Context, o metric.ObservationRecorder) error { + o.Int64(ctr, 11) return nil - }) + }, ctr) assert.NoError(t, err) // Observed outside of a callback, it should be ignored. @@ -241,10 +239,10 @@ func TestMeterCreatesInstruments(t *testing.T) { } gauge, err := m.Int64ObservableGauge("agauge", instrument.WithInt64Callback(cback)) assert.NoError(t, err) - _, err = m.RegisterCallback([]instrument.Asynchronous{gauge}, func(ctx context.Context) error { - gauge.Observe(ctx, 11) + _, err = m.RegisterCallback(func(ctx context.Context, o metric.ObservationRecorder) error { + o.Int64(gauge, 11) return nil - }) + }, gauge) assert.NoError(t, err) // Observed outside of a callback, it should be ignored. @@ -269,10 +267,10 @@ func TestMeterCreatesInstruments(t *testing.T) { } ctr, err := m.Float64ObservableCounter("afloat", instrument.WithFloat64Callback(cback)) assert.NoError(t, err) - _, err = m.RegisterCallback([]instrument.Asynchronous{ctr}, func(ctx context.Context) error { - ctr.Observe(ctx, 3) + _, err = m.RegisterCallback(func(ctx context.Context, o metric.ObservationRecorder) error { + o.Float64(ctr, 3) return nil - }) + }, ctr) assert.NoError(t, err) // Observed outside of a callback, it should be ignored. @@ -299,10 +297,10 @@ func TestMeterCreatesInstruments(t *testing.T) { } ctr, err := m.Float64ObservableUpDownCounter("afloat", instrument.WithFloat64Callback(cback)) assert.NoError(t, err) - _, err = m.RegisterCallback([]instrument.Asynchronous{ctr}, func(ctx context.Context) error { - ctr.Observe(ctx, 11) + _, err = m.RegisterCallback(func(ctx context.Context, o metric.ObservationRecorder) error { + o.Float64(ctr, 11) return nil - }) + }, ctr) assert.NoError(t, err) // Observed outside of a callback, it should be ignored. @@ -329,10 +327,10 @@ func TestMeterCreatesInstruments(t *testing.T) { } gauge, err := m.Float64ObservableGauge("agauge", instrument.WithFloat64Callback(cback)) assert.NoError(t, err) - _, err = m.RegisterCallback([]instrument.Asynchronous{gauge}, func(ctx context.Context) error { - gauge.Observe(ctx, 11) + _, err = m.RegisterCallback(func(ctx context.Context, o metric.ObservationRecorder) error { + o.Float64(gauge, 11) return nil - }) + }, gauge) assert.NoError(t, err) // Observed outside of a callback, it should be ignored. @@ -505,20 +503,21 @@ func TestMetersProvideScope(t *testing.T) { m1 := mp.Meter("scope1") ctr1, err := m1.Float64ObservableCounter("ctr1") assert.NoError(t, err) - _, err = m1.RegisterCallback([]instrument.Asynchronous{ctr1}, func(ctx context.Context) error { - ctr1.Observe(ctx, 5) + _, err = m1.RegisterCallback(func(_ context.Context, o metric.ObservationRecorder) error { + o.Float64(ctr1, 5) return nil - }) + }, ctr1) assert.NoError(t, err) m2 := mp.Meter("scope2") ctr2, err := m2.Int64ObservableCounter("ctr2") assert.NoError(t, err) - _, err = m1.RegisterCallback([]instrument.Asynchronous{ctr2}, func(ctx context.Context) error { - ctr2.Observe(ctx, 7) + _, err = m2.RegisterCallback(func(_ context.Context, o metric.ObservationRecorder) error { + o.Int64(ctr2, 7) return nil - }) + }, ctr2) assert.NoError(t, err) + // TODO: verify this should be an error and then fix this test. want := metricdata.ResourceMetrics{ Resource: resource.Default(), @@ -593,17 +592,18 @@ func TestUnregisterUnregisters(t *testing.T) { require.NoError(t, err) var called bool - reg, err := m.RegisterCallback([]instrument.Asynchronous{ + reg, err := m.RegisterCallback( + func(context.Context, metric.ObservationRecorder) error { + called = true + return nil + }, int64Counter, int64UpDownCounter, int64Gauge, floag64Counter, floag64UpDownCounter, floag64Gauge, - }, func(context.Context) error { - called = true - return nil - }) + ) require.NoError(t, err) ctx := context.Background() @@ -646,17 +646,18 @@ func TestRegisterCallbackDropAggregations(t *testing.T) { require.NoError(t, err) var called bool - _, err = m.RegisterCallback([]instrument.Asynchronous{ + _, err = m.RegisterCallback( + func(context.Context, metric.ObservationRecorder) error { + called = true + return nil + }, int64Counter, int64UpDownCounter, int64Gauge, floag64Counter, floag64UpDownCounter, floag64Gauge, - }, func(context.Context) error { - called = true - return nil - }) + ) require.NoError(t, err) data, err := r.Collect(context.Background()) @@ -681,11 +682,11 @@ func TestAttributeFilter(t *testing.T) { if err != nil { return err } - _, err = mtr.RegisterCallback([]instrument.Asynchronous{ctr}, func(ctx context.Context) error { - ctr.Observe(ctx, 1.0, attribute.String("foo", "bar"), attribute.Int("version", 1)) - ctr.Observe(ctx, 2.0, attribute.String("foo", "bar"), attribute.Int("version", 2)) + _, err = mtr.RegisterCallback(func(_ context.Context, o metric.ObservationRecorder) error { + o.Float64(ctr, 1.0, attribute.String("foo", "bar"), attribute.Int("version", 1)) + o.Float64(ctr, 2.0, attribute.String("foo", "bar"), attribute.Int("version", 2)) return nil - }) + }, ctr) return err }, wantMetric: metricdata.Metrics{ @@ -709,11 +710,11 @@ func TestAttributeFilter(t *testing.T) { if err != nil { return err } - _, err = mtr.RegisterCallback([]instrument.Asynchronous{ctr}, func(ctx context.Context) error { - ctr.Observe(ctx, 1.0, attribute.String("foo", "bar"), attribute.Int("version", 1)) - ctr.Observe(ctx, 2.0, attribute.String("foo", "bar"), attribute.Int("version", 2)) + _, err = mtr.RegisterCallback(func(_ context.Context, o metric.ObservationRecorder) error { + o.Float64(ctr, 1.0, attribute.String("foo", "bar"), attribute.Int("version", 1)) + o.Float64(ctr, 2.0, attribute.String("foo", "bar"), attribute.Int("version", 2)) return nil - }) + }, ctr) return err }, wantMetric: metricdata.Metrics{ @@ -737,11 +738,11 @@ func TestAttributeFilter(t *testing.T) { if err != nil { return err } - _, err = mtr.RegisterCallback([]instrument.Asynchronous{ctr}, func(ctx context.Context) error { - ctr.Observe(ctx, 1.0, attribute.String("foo", "bar"), attribute.Int("version", 1)) - ctr.Observe(ctx, 2.0, attribute.String("foo", "bar"), attribute.Int("version", 2)) + _, err = mtr.RegisterCallback(func(_ context.Context, o metric.ObservationRecorder) error { + o.Float64(ctr, 1.0, attribute.String("foo", "bar"), attribute.Int("version", 1)) + o.Float64(ctr, 2.0, attribute.String("foo", "bar"), attribute.Int("version", 2)) return nil - }) + }, ctr) return err }, wantMetric: metricdata.Metrics{ @@ -763,11 +764,11 @@ func TestAttributeFilter(t *testing.T) { if err != nil { return err } - _, err = mtr.RegisterCallback([]instrument.Asynchronous{ctr}, func(ctx context.Context) error { - ctr.Observe(ctx, 10, attribute.String("foo", "bar"), attribute.Int("version", 1)) - ctr.Observe(ctx, 20, attribute.String("foo", "bar"), attribute.Int("version", 2)) + _, err = mtr.RegisterCallback(func(_ context.Context, o metric.ObservationRecorder) error { + o.Int64(ctr, 10, attribute.String("foo", "bar"), attribute.Int("version", 1)) + o.Int64(ctr, 20, attribute.String("foo", "bar"), attribute.Int("version", 2)) return nil - }) + }, ctr) return err }, wantMetric: metricdata.Metrics{ @@ -791,11 +792,11 @@ func TestAttributeFilter(t *testing.T) { if err != nil { return err } - _, err = mtr.RegisterCallback([]instrument.Asynchronous{ctr}, func(ctx context.Context) error { - ctr.Observe(ctx, 10, attribute.String("foo", "bar"), attribute.Int("version", 1)) - ctr.Observe(ctx, 20, attribute.String("foo", "bar"), attribute.Int("version", 2)) + _, err = mtr.RegisterCallback(func(_ context.Context, o metric.ObservationRecorder) error { + o.Int64(ctr, 10, attribute.String("foo", "bar"), attribute.Int("version", 1)) + o.Int64(ctr, 20, attribute.String("foo", "bar"), attribute.Int("version", 2)) return nil - }) + }, ctr) return err }, wantMetric: metricdata.Metrics{ @@ -819,11 +820,11 @@ func TestAttributeFilter(t *testing.T) { if err != nil { return err } - _, err = mtr.RegisterCallback([]instrument.Asynchronous{ctr}, func(ctx context.Context) error { - ctr.Observe(ctx, 10, attribute.String("foo", "bar"), attribute.Int("version", 1)) - ctr.Observe(ctx, 20, attribute.String("foo", "bar"), attribute.Int("version", 2)) + _, err = mtr.RegisterCallback(func(_ context.Context, o metric.ObservationRecorder) error { + o.Int64(ctr, 10, attribute.String("foo", "bar"), attribute.Int("version", 1)) + o.Int64(ctr, 20, attribute.String("foo", "bar"), attribute.Int("version", 2)) return nil - }) + }, ctr) return err }, wantMetric: metricdata.Metrics{ diff --git a/sdk/metric/pipeline.go b/sdk/metric/pipeline.go index 32b9125340b..666e095c169 100644 --- a/sdk/metric/pipeline.go +++ b/sdk/metric/pipeline.go @@ -104,9 +104,11 @@ func (p *pipeline) addCallback(cback func(context.Context) error) { p.callbacks = append(p.callbacks, cback) } +type multiCallback func(context.Context) error + // addMultiCallback registers a multi-instrument callback to be run when // `produce()` is called. -func (p *pipeline) addMultiCallback(c metric.Callback) (unregister func()) { +func (p *pipeline) addMultiCallback(c multiCallback) (unregister func()) { p.Lock() defer p.Unlock() e := p.multiCallbacks.PushBack(c) @@ -146,7 +148,7 @@ func (p *pipeline) produce(ctx context.Context) (metricdata.ResourceMetrics, err } for e := p.multiCallbacks.Front(); e != nil; e = e.Next() { // TODO make the callbacks parallel. ( #3034 ) - f := e.Value.(metric.Callback) + f := e.Value.(multiCallback) if err := f(ctx); err != nil { errs.append(err) } @@ -475,7 +477,7 @@ func (p pipelines) registerCallback(cback func(context.Context) error) { } } -func (p pipelines) registerMultiCallback(c metric.Callback) metric.Registration { +func (p pipelines) registerMultiCallback(c multiCallback) metric.Registration { unregs := make([]func(), len(p)) for i, pipe := range p { unregs[i] = pipe.addMultiCallback(c) diff --git a/sdk/metric/pipeline_test.go b/sdk/metric/pipeline_test.go index 7b9f89585dc..3c5e19b8b7d 100644 --- a/sdk/metric/pipeline_test.go +++ b/sdk/metric/pipeline_test.go @@ -54,7 +54,7 @@ func TestEmptyPipeline(t *testing.T) { }) require.NotPanics(t, func() { - pipe.addMultiCallback(emptyCallback) + pipe.addMultiCallback(func(context.Context) error { return nil }) }) output, err = pipe.produce(context.Background()) @@ -78,7 +78,7 @@ func TestNewPipeline(t *testing.T) { }) require.NotPanics(t, func() { - pipe.addMultiCallback(emptyCallback) + pipe.addMultiCallback(func(context.Context) error { return nil }) }) output, err = pipe.produce(context.Background()) @@ -121,7 +121,7 @@ func TestPipelineConcurrency(t *testing.T) { wg.Add(1) go func() { defer wg.Done() - pipe.addMultiCallback(emptyCallback) + pipe.addMultiCallback(func(context.Context) error { return nil }) }() } wg.Wait() From 68d8224ff178c6094a1e476a0b2ef5025c637e23 Mon Sep 17 00:00:00 2001 From: Tyler Yahn Date: Tue, 10 Jan 2023 16:20:03 -0800 Subject: [PATCH 05/31] Fix prometheus example --- example/prometheus/main.go | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/example/prometheus/main.go b/example/prometheus/main.go index 68be2f8d6cb..254ed83a3e4 100644 --- a/example/prometheus/main.go +++ b/example/prometheus/main.go @@ -28,6 +28,7 @@ import ( "go.opentelemetry.io/otel/attribute" "go.opentelemetry.io/otel/exporters/prometheus" + api "go.opentelemetry.io/otel/metric" "go.opentelemetry.io/otel/metric/instrument" "go.opentelemetry.io/otel/sdk/metric" ) @@ -68,11 +69,11 @@ func main() { if err != nil { log.Fatal(err) } - _, err = meter.RegisterCallback([]instrument.Asynchronous{gauge}, func(ctx context.Context) error { + _, err = meter.RegisterCallback(func(_ context.Context, o api.ObservationRecorder) error { n := -10. + rand.Float64()*(90.) // [-10, 100) - gauge.Observe(ctx, n, attrs...) + o.Float64(gauge, n, attrs...) return nil - }) + }, gauge) if err != nil { log.Fatal(err) } From 8c88dea03583ddbe94e34ba3824949f5f51f716d Mon Sep 17 00:00:00 2001 From: Tyler Yahn Date: Tue, 10 Jan 2023 16:22:16 -0800 Subject: [PATCH 06/31] Fix metric API example_test --- metric/example_test.go | 13 ++++++------- 1 file changed, 6 insertions(+), 7 deletions(-) diff --git a/metric/example_test.go b/metric/example_test.go index 64441c23ace..5c981329a41 100644 --- a/metric/example_test.go +++ b/metric/example_test.go @@ -89,22 +89,21 @@ func ExampleMeter_asynchronous_multiple() { gcCount, _ := meter.Int64ObservableCounter("gcCount") gcPause, _ := meter.Float64Histogram("gcPause") - _, err := meter.RegisterCallback([]instrument.Asynchronous{ - heapAlloc, - gcCount, - }, - func(ctx context.Context) error { + _, err := meter.RegisterCallback( + func(ctx context.Context, o metric.ObservationRecorder) error { memStats := &runtime.MemStats{} // This call does work runtime.ReadMemStats(memStats) - heapAlloc.Observe(ctx, int64(memStats.HeapAlloc)) - gcCount.Observe(ctx, int64(memStats.NumGC)) + o.Int64(heapAlloc, int64(memStats.HeapAlloc)) + o.Int64(gcCount, int64(memStats.NumGC)) // This function synchronously records the pauses computeGCPauses(ctx, gcPause, memStats.PauseNs[:]) return nil }, + heapAlloc, + gcCount, ) if err != nil { From 93a5d85f51b959fcf2f0e35ba9c684be750c2332 Mon Sep 17 00:00:00 2001 From: Tyler Yahn Date: Tue, 10 Jan 2023 16:23:20 -0800 Subject: [PATCH 07/31] Remove unused registerabler --- sdk/metric/instrument.go | 4 ---- 1 file changed, 4 deletions(-) diff --git a/sdk/metric/instrument.go b/sdk/metric/instrument.go index e808c1564e5..2b669226b96 100644 --- a/sdk/metric/instrument.go +++ b/sdk/metric/instrument.go @@ -275,7 +275,3 @@ func (o *observer[N]) registerable(scope instrumentation.Scope) error { } return nil } - -type registerabler interface { - registerable(instrumentation.Scope) error -} From 0a2362680d26462854d24f3e2427a701fe9594f5 Mon Sep 17 00:00:00 2001 From: Tyler Yahn Date: Wed, 11 Jan 2023 08:08:34 -0800 Subject: [PATCH 08/31] Rename ObservationRecorder to MultiObserver --- example/prometheus/main.go | 2 +- metric/example_test.go | 2 +- metric/internal/global/meter_test.go | 8 +++---- metric/meter.go | 16 ++++++------- sdk/metric/meter.go | 28 +++++++++++------------ sdk/metric/meter_test.go | 34 ++++++++++++++-------------- 6 files changed, 45 insertions(+), 45 deletions(-) diff --git a/example/prometheus/main.go b/example/prometheus/main.go index 254ed83a3e4..4d7be8b776d 100644 --- a/example/prometheus/main.go +++ b/example/prometheus/main.go @@ -69,7 +69,7 @@ func main() { if err != nil { log.Fatal(err) } - _, err = meter.RegisterCallback(func(_ context.Context, o api.ObservationRecorder) error { + _, err = meter.RegisterCallback(func(_ context.Context, o api.MultiObserver) error { n := -10. + rand.Float64()*(90.) // [-10, 100) o.Float64(gauge, n, attrs...) return nil diff --git a/metric/example_test.go b/metric/example_test.go index 5c981329a41..42abfe2cfd1 100644 --- a/metric/example_test.go +++ b/metric/example_test.go @@ -90,7 +90,7 @@ func ExampleMeter_asynchronous_multiple() { gcPause, _ := meter.Float64Histogram("gcPause") _, err := meter.RegisterCallback( - func(ctx context.Context, o metric.ObservationRecorder) error { + func(ctx context.Context, o metric.MultiObserver) error { memStats := &runtime.MemStats{} // This call does work runtime.ReadMemStats(memStats) diff --git a/metric/internal/global/meter_test.go b/metric/internal/global/meter_test.go index 1286176ca2e..27fd68f1a5d 100644 --- a/metric/internal/global/meter_test.go +++ b/metric/internal/global/meter_test.go @@ -45,7 +45,7 @@ func TestMeterProviderRace(t *testing.T) { close(finish) } -var zeroCallback metric.Callback = func(ctx context.Context, or metric.ObservationRecorder) error { +var zeroCallback metric.Callback = func(ctx context.Context, or metric.MultiObserver) error { return nil } @@ -132,7 +132,7 @@ func testSetupAllInstrumentTypes(t *testing.T, m metric.Meter) (instrument.Float _, err = m.Int64ObservableGauge("test_Async_Gauge") assert.NoError(t, err) - _, err = m.RegisterCallback(func(ctx context.Context, obs metric.ObservationRecorder) error { + _, err = m.RegisterCallback(func(ctx context.Context, obs metric.MultiObserver) error { obs.Float64(afcounter, 3) return nil }, afcounter) @@ -327,7 +327,7 @@ func TestRegistrationDelegation(t *testing.T) { require.NoError(t, err) var called0 bool - reg0, err := m.RegisterCallback(func(context.Context, metric.ObservationRecorder) error { + reg0, err := m.RegisterCallback(func(context.Context, metric.MultiObserver) error { called0 = true return nil }, actr) @@ -338,7 +338,7 @@ func TestRegistrationDelegation(t *testing.T) { assert.Equal(t, 0, mImpl.registry.Len(), "callback not unregistered") var called1 bool - reg1, err := m.RegisterCallback(func(context.Context, metric.ObservationRecorder) error { + reg1, err := m.RegisterCallback(func(context.Context, metric.MultiObserver) error { called1 = true return nil }, actr) diff --git a/metric/meter.go b/metric/meter.go index 559f55e67f4..d6f91ce00d0 100644 --- a/metric/meter.go +++ b/metric/meter.go @@ -109,7 +109,7 @@ type Meter interface { // Callback is a function registered with a Meter that makes observations for // the set of instruments it is registered with. // -// The ObservationRecorder parameter is used to record measurment observations. +// The MultiObserver parameter is used to record measurment observations. // Measurments should not be recorded directly with an Observer. // // The function needs to complete in a finite amount of time and the deadline @@ -120,14 +120,14 @@ type Meter interface { // the same attributes as another Callback will report. // // The function needs to be concurrent safe. -type Callback func(context.Context, ObservationRecorder) error +type Callback func(context.Context, MultiObserver) error -// ObservationRecorder records the measurements in a Callback. -type ObservationRecorder interface { - // Float64 records the float64 value with attributes for inst. - Float64(inst instrument.Float64Observer, value float64, attributes ...attribute.KeyValue) - // Int64 records the int64 value with attributes for inst. - Int64(inst instrument.Int64Observer, value int64, attributes ...attribute.KeyValue) +// MultiObserver records measurements for multiple instruments in a Callback. +type MultiObserver interface { + // Float64 records the float64 value with attributes for obsrv. + Float64(obsrv instrument.Float64Observer, value float64, attributes ...attribute.KeyValue) + // Int64 records the int64 value with attributes for obsrv. + Int64(obsrv instrument.Int64Observer, value int64, attributes ...attribute.KeyValue) } // Registration is an token representing the unique registration of a callback diff --git a/sdk/metric/meter.go b/sdk/metric/meter.go index 07294a04d67..cf86ce85dd8 100644 --- a/sdk/metric/meter.go +++ b/sdk/metric/meter.go @@ -213,7 +213,7 @@ func (m *meter) RegisterCallback(f metric.Callback, insts ...instrument.Asynchro return noopRegister{}, nil } - reg := newObservationRegistry() + reg := newObserverRegistry() var errs multierror for _, inst := range insts { switch o := inst.(type) { @@ -249,41 +249,41 @@ func (m *meter) RegisterCallback(f metric.Callback, insts ...instrument.Asynchro } cback := func(ctx context.Context) error { - return f(ctx, reg.recorder(ctx)) + return f(ctx, reg.multiObserver(ctx)) } return m.pipes.registerMultiCallback(cback), nil } -type obeservationRegistry struct { +type obeserverRegistry struct { float64 map[observerID[float64]]struct{} int64 map[observerID[int64]]struct{} } -func newObservationRegistry() obeservationRegistry { - return obeservationRegistry{ +func newObserverRegistry() obeserverRegistry { + return obeserverRegistry{ float64: make(map[observerID[float64]]struct{}), int64: make(map[observerID[int64]]struct{}), } } -func (r obeservationRegistry) len() int { +func (r obeserverRegistry) len() int { return len(r.float64) + len(r.int64) } -func (r obeservationRegistry) registerFloat64(id observerID[float64]) { +func (r obeserverRegistry) registerFloat64(id observerID[float64]) { r.float64[id] = struct{}{} } -func (r obeservationRegistry) registerInt64(id observerID[int64]) { +func (r obeserverRegistry) registerInt64(id observerID[int64]) { r.int64[id] = struct{}{} } -func (r obeservationRegistry) recorder(ctx context.Context) metric.ObservationRecorder { - return obeservationRecorder{obeservationRegistry: r, ctx: ctx} +func (r obeserverRegistry) multiObserver(ctx context.Context) metric.MultiObserver { + return multiObserver{obeserverRegistry: r, ctx: ctx} } -type obeservationRecorder struct { - obeservationRegistry +type multiObserver struct { + obeserverRegistry ctx context.Context } @@ -293,7 +293,7 @@ var ( errUnregObserver = errors.New("observer not registered for callback") ) -func (r obeservationRecorder) Float64(o instrument.Float64Observer, v float64, a ...attribute.KeyValue) { +func (r multiObserver) Float64(o instrument.Float64Observer, v float64, a ...attribute.KeyValue) { oImpl, ok := o.(*observer[float64]) if !ok { global.Error(errUnknownObserver, "failed to record") @@ -311,7 +311,7 @@ func (r obeservationRecorder) Float64(o instrument.Float64Observer, v float64, a oImpl.observe(r.ctx, v, a) } -func (r obeservationRecorder) Int64(o instrument.Int64Observer, v int64, a ...attribute.KeyValue) { +func (r multiObserver) Int64(o instrument.Int64Observer, v int64, a ...attribute.KeyValue) { oImpl, ok := o.(*observer[int64]) if !ok { global.Error(errUnknownObserver, "failed to record") diff --git a/sdk/metric/meter_test.go b/sdk/metric/meter_test.go index 7ede255360e..0634bdb6512 100644 --- a/sdk/metric/meter_test.go +++ b/sdk/metric/meter_test.go @@ -91,7 +91,7 @@ func TestMeterInstrumentConcurrency(t *testing.T) { wg.Wait() } -var emptyCallback metric.Callback = func(context.Context, metric.ObservationRecorder) error { return nil } +var emptyCallback metric.Callback = func(context.Context, metric.MultiObserver) error { return nil } // A Meter Should be able register Callbacks Concurrently. func TestMeterCallbackCreationConcurrency(t *testing.T) { @@ -179,7 +179,7 @@ func TestMeterCreatesInstruments(t *testing.T) { } ctr, err := m.Int64ObservableCounter("aint", instrument.WithInt64Callback(cback)) assert.NoError(t, err) - _, err = m.RegisterCallback(func(_ context.Context, o metric.ObservationRecorder) error { + _, err = m.RegisterCallback(func(_ context.Context, o metric.MultiObserver) error { o.Int64(ctr, 3) return nil }, ctr) @@ -209,7 +209,7 @@ func TestMeterCreatesInstruments(t *testing.T) { } ctr, err := m.Int64ObservableUpDownCounter("aint", instrument.WithInt64Callback(cback)) assert.NoError(t, err) - _, err = m.RegisterCallback(func(ctx context.Context, o metric.ObservationRecorder) error { + _, err = m.RegisterCallback(func(ctx context.Context, o metric.MultiObserver) error { o.Int64(ctr, 11) return nil }, ctr) @@ -239,7 +239,7 @@ func TestMeterCreatesInstruments(t *testing.T) { } gauge, err := m.Int64ObservableGauge("agauge", instrument.WithInt64Callback(cback)) assert.NoError(t, err) - _, err = m.RegisterCallback(func(ctx context.Context, o metric.ObservationRecorder) error { + _, err = m.RegisterCallback(func(ctx context.Context, o metric.MultiObserver) error { o.Int64(gauge, 11) return nil }, gauge) @@ -267,7 +267,7 @@ func TestMeterCreatesInstruments(t *testing.T) { } ctr, err := m.Float64ObservableCounter("afloat", instrument.WithFloat64Callback(cback)) assert.NoError(t, err) - _, err = m.RegisterCallback(func(ctx context.Context, o metric.ObservationRecorder) error { + _, err = m.RegisterCallback(func(ctx context.Context, o metric.MultiObserver) error { o.Float64(ctr, 3) return nil }, ctr) @@ -297,7 +297,7 @@ func TestMeterCreatesInstruments(t *testing.T) { } ctr, err := m.Float64ObservableUpDownCounter("afloat", instrument.WithFloat64Callback(cback)) assert.NoError(t, err) - _, err = m.RegisterCallback(func(ctx context.Context, o metric.ObservationRecorder) error { + _, err = m.RegisterCallback(func(ctx context.Context, o metric.MultiObserver) error { o.Float64(ctr, 11) return nil }, ctr) @@ -327,7 +327,7 @@ func TestMeterCreatesInstruments(t *testing.T) { } gauge, err := m.Float64ObservableGauge("agauge", instrument.WithFloat64Callback(cback)) assert.NoError(t, err) - _, err = m.RegisterCallback(func(ctx context.Context, o metric.ObservationRecorder) error { + _, err = m.RegisterCallback(func(ctx context.Context, o metric.MultiObserver) error { o.Float64(gauge, 11) return nil }, gauge) @@ -503,7 +503,7 @@ func TestMetersProvideScope(t *testing.T) { m1 := mp.Meter("scope1") ctr1, err := m1.Float64ObservableCounter("ctr1") assert.NoError(t, err) - _, err = m1.RegisterCallback(func(_ context.Context, o metric.ObservationRecorder) error { + _, err = m1.RegisterCallback(func(_ context.Context, o metric.MultiObserver) error { o.Float64(ctr1, 5) return nil }, ctr1) @@ -512,7 +512,7 @@ func TestMetersProvideScope(t *testing.T) { m2 := mp.Meter("scope2") ctr2, err := m2.Int64ObservableCounter("ctr2") assert.NoError(t, err) - _, err = m2.RegisterCallback(func(_ context.Context, o metric.ObservationRecorder) error { + _, err = m2.RegisterCallback(func(_ context.Context, o metric.MultiObserver) error { o.Int64(ctr2, 7) return nil }, ctr2) @@ -593,7 +593,7 @@ func TestUnregisterUnregisters(t *testing.T) { var called bool reg, err := m.RegisterCallback( - func(context.Context, metric.ObservationRecorder) error { + func(context.Context, metric.MultiObserver) error { called = true return nil }, @@ -647,7 +647,7 @@ func TestRegisterCallbackDropAggregations(t *testing.T) { var called bool _, err = m.RegisterCallback( - func(context.Context, metric.ObservationRecorder) error { + func(context.Context, metric.MultiObserver) error { called = true return nil }, @@ -682,7 +682,7 @@ func TestAttributeFilter(t *testing.T) { if err != nil { return err } - _, err = mtr.RegisterCallback(func(_ context.Context, o metric.ObservationRecorder) error { + _, err = mtr.RegisterCallback(func(_ context.Context, o metric.MultiObserver) error { o.Float64(ctr, 1.0, attribute.String("foo", "bar"), attribute.Int("version", 1)) o.Float64(ctr, 2.0, attribute.String("foo", "bar"), attribute.Int("version", 2)) return nil @@ -710,7 +710,7 @@ func TestAttributeFilter(t *testing.T) { if err != nil { return err } - _, err = mtr.RegisterCallback(func(_ context.Context, o metric.ObservationRecorder) error { + _, err = mtr.RegisterCallback(func(_ context.Context, o metric.MultiObserver) error { o.Float64(ctr, 1.0, attribute.String("foo", "bar"), attribute.Int("version", 1)) o.Float64(ctr, 2.0, attribute.String("foo", "bar"), attribute.Int("version", 2)) return nil @@ -738,7 +738,7 @@ func TestAttributeFilter(t *testing.T) { if err != nil { return err } - _, err = mtr.RegisterCallback(func(_ context.Context, o metric.ObservationRecorder) error { + _, err = mtr.RegisterCallback(func(_ context.Context, o metric.MultiObserver) error { o.Float64(ctr, 1.0, attribute.String("foo", "bar"), attribute.Int("version", 1)) o.Float64(ctr, 2.0, attribute.String("foo", "bar"), attribute.Int("version", 2)) return nil @@ -764,7 +764,7 @@ func TestAttributeFilter(t *testing.T) { if err != nil { return err } - _, err = mtr.RegisterCallback(func(_ context.Context, o metric.ObservationRecorder) error { + _, err = mtr.RegisterCallback(func(_ context.Context, o metric.MultiObserver) error { o.Int64(ctr, 10, attribute.String("foo", "bar"), attribute.Int("version", 1)) o.Int64(ctr, 20, attribute.String("foo", "bar"), attribute.Int("version", 2)) return nil @@ -792,7 +792,7 @@ func TestAttributeFilter(t *testing.T) { if err != nil { return err } - _, err = mtr.RegisterCallback(func(_ context.Context, o metric.ObservationRecorder) error { + _, err = mtr.RegisterCallback(func(_ context.Context, o metric.MultiObserver) error { o.Int64(ctr, 10, attribute.String("foo", "bar"), attribute.Int("version", 1)) o.Int64(ctr, 20, attribute.String("foo", "bar"), attribute.Int("version", 2)) return nil @@ -820,7 +820,7 @@ func TestAttributeFilter(t *testing.T) { if err != nil { return err } - _, err = mtr.RegisterCallback(func(_ context.Context, o metric.ObservationRecorder) error { + _, err = mtr.RegisterCallback(func(_ context.Context, o metric.MultiObserver) error { o.Int64(ctr, 10, attribute.String("foo", "bar"), attribute.Int("version", 1)) o.Int64(ctr, 20, attribute.String("foo", "bar"), attribute.Int("version", 2)) return nil From d0947fb54b863afa643ccf5dfde30911b713e7bd Mon Sep 17 00:00:00 2001 From: Tyler Yahn Date: Wed, 11 Jan 2023 08:45:49 -0800 Subject: [PATCH 09/31] Update Callback documentation about MultiObserver --- metric/meter.go | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/metric/meter.go b/metric/meter.go index d6f91ce00d0..45010c496ce 100644 --- a/metric/meter.go +++ b/metric/meter.go @@ -107,10 +107,8 @@ type Meter interface { } // Callback is a function registered with a Meter that makes observations for -// the set of instruments it is registered with. -// -// The MultiObserver parameter is used to record measurment observations. -// Measurments should not be recorded directly with an Observer. +// the set of instruments it is registered with. The MultiObserver parameter is +// used to record measurment observations for these instruments. // // The function needs to complete in a finite amount of time and the deadline // of the passed context is expected to be honored. From 053e7d168816b5ee8043e7a8e9f00deaf0ec3f38 Mon Sep 17 00:00:00 2001 From: Tyler Yahn Date: Wed, 11 Jan 2023 11:46:20 -0800 Subject: [PATCH 10/31] Add changes to changelog --- CHANGELOG.md | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 33d3cf58bde..d2fe4f74100 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -99,6 +99,16 @@ This project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.htm - The exporter from `go.opentelemetry.io/otel/exporters/zipkin` is updated to use the `v1.16.0` version of semantic conventions. This means it no longer uses the removed `net.peer.ip` or `http.host` attributes to determine the remote endpoint. Instead it uses the `net.sock.peer` attributes. (#3581) +- The parameters for the `RegisterCallback` method of the `Meter` from `go.opentelemetry.io/otel/metric` are changed. + The slice of `instrument.Asynchronous` parameter is now passed as a variadic argument. (#3584) +- The `Callback` in `go.opentelemetry.io/otel/metric` has the added `MultiObserver` parameter added. + This new parameter is used by `Callback` implementations to observe values for asynchronous instruments instead of calling the `Observer` method of the instrument directly. (#3584) + +### Fixed + +- The `RegisterCallback` method of the `Meter` from `go.opentelemetry.io/otel/sdk/metric` only registers a callback once per call, instead of once per instrument the method is called with. (#3584) +- The `RegisterCallback` method of the `Meter` from `go.opentelemetry.io/otel/sdk/metric` only registers a callback for instruments created by that meter. + Trying to register a callback with instruments from a different meter will result in an error being returned. (#3584) ### Deprecated From ac2911eb071767bdd7f4350a494193dd85c0db4f Mon Sep 17 00:00:00 2001 From: Tyler Yahn Date: Wed, 11 Jan 2023 11:50:27 -0800 Subject: [PATCH 11/31] Comment observer impl method --- sdk/metric/instrument.go | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/sdk/metric/instrument.go b/sdk/metric/instrument.go index 2b669226b96..ab483024a7b 100644 --- a/sdk/metric/instrument.go +++ b/sdk/metric/instrument.go @@ -250,6 +250,7 @@ func (o *observer[N]) Observe(ctx context.Context, val N, attrs ...attribute.Key ) } +// observe records the val for the set of attrs. func (o *observer[N]) observe(ctx context.Context, val N, attrs []attribute.KeyValue) { if err := ctx.Err(); err != nil { return @@ -261,6 +262,10 @@ func (o *observer[N]) observe(ctx context.Context, val N, attrs []attribute.KeyV var errEmptyAgg = errors.New("no aggregators for observer") +// registerable returns an error if the observer o should not be registered, +// and nil if it should. An errEmptyAgg error is returned if o is effecively a +// no-op because it does not have any aggregators. Also, an error is returned +// if scope defines a Meter other than the one o was created by. func (o *observer[N]) registerable(scope instrumentation.Scope) error { if len(o.aggregators) == 0 { return errEmptyAgg From 2dc0046c0a6a0d94acd7fad6171e0ab61a440a8f Mon Sep 17 00:00:00 2001 From: Tyler Yahn Date: Wed, 11 Jan 2023 11:54:32 -0800 Subject: [PATCH 12/31] Fix comment type ref --- sdk/metric/meter.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/sdk/metric/meter.go b/sdk/metric/meter.go index cf86ce85dd8..95b441f9c6a 100644 --- a/sdk/metric/meter.go +++ b/sdk/metric/meter.go @@ -209,7 +209,7 @@ func (m *meter) Float64ObservableGauge(name string, options ...instrument.Float6 // insts Collect method is called. func (m *meter) RegisterCallback(f metric.Callback, insts ...instrument.Asynchronous) (metric.Registration, error) { if len(insts) == 0 { - // Don't allocate an obeservationRegistry if not needed. + // Don't allocate an obeserverRegistry if not needed. return noopRegister{}, nil } From b5fe5f4ed5ba104912364e17b04ac2e5cae38926 Mon Sep 17 00:00:00 2001 From: Tyler Yahn Date: Wed, 11 Jan 2023 12:25:58 -0800 Subject: [PATCH 13/31] Fix err msg for registerable --- sdk/metric/instrument.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/sdk/metric/instrument.go b/sdk/metric/instrument.go index ab483024a7b..c2ffcf358e6 100644 --- a/sdk/metric/instrument.go +++ b/sdk/metric/instrument.go @@ -274,8 +274,8 @@ func (o *observer[N]) registerable(scope instrumentation.Scope) error { return fmt.Errorf( "invalid registration: observer %q from Meter %q, registered with Meter %q", o.name, - scope.Name, o.scope.Name, + scope.Name, ) } return nil From 2a4a70e9c0e1e982f72a723baba5d2f97b0418eb Mon Sep 17 00:00:00 2001 From: Tyler Yahn Date: Wed, 11 Jan 2023 12:27:16 -0800 Subject: [PATCH 14/31] Remove ctx eval in observer observe --- sdk/metric/instrument.go | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/sdk/metric/instrument.go b/sdk/metric/instrument.go index c2ffcf358e6..568bac8b2ac 100644 --- a/sdk/metric/instrument.go +++ b/sdk/metric/instrument.go @@ -251,10 +251,7 @@ func (o *observer[N]) Observe(ctx context.Context, val N, attrs ...attribute.Key } // observe records the val for the set of attrs. -func (o *observer[N]) observe(ctx context.Context, val N, attrs []attribute.KeyValue) { - if err := ctx.Err(); err != nil { - return - } +func (o *observer[N]) observe(_ context.Context, val N, attrs []attribute.KeyValue) { for _, agg := range o.aggregators { agg.Aggregate(val, attribute.NewSet(attrs...)) } From 65ab9df8a0e7158c457705265f1a04f3cba0f6c2 Mon Sep 17 00:00:00 2001 From: Tyler Yahn Date: Wed, 11 Jan 2023 12:32:00 -0800 Subject: [PATCH 15/31] Don't pass unneeded ctx to observer observe --- sdk/metric/instrument.go | 2 +- sdk/metric/meter.go | 36 +++++++++++++----------------------- 2 files changed, 14 insertions(+), 24 deletions(-) diff --git a/sdk/metric/instrument.go b/sdk/metric/instrument.go index 568bac8b2ac..b93a7bb84c1 100644 --- a/sdk/metric/instrument.go +++ b/sdk/metric/instrument.go @@ -251,7 +251,7 @@ func (o *observer[N]) Observe(ctx context.Context, val N, attrs ...attribute.Key } // observe records the val for the set of attrs. -func (o *observer[N]) observe(_ context.Context, val N, attrs []attribute.KeyValue) { +func (o *observer[N]) observe(val N, attrs []attribute.KeyValue) { for _, agg := range o.aggregators { agg.Aggregate(val, attribute.NewSet(attrs...)) } diff --git a/sdk/metric/meter.go b/sdk/metric/meter.go index 95b441f9c6a..7c0cc449510 100644 --- a/sdk/metric/meter.go +++ b/sdk/metric/meter.go @@ -209,11 +209,11 @@ func (m *meter) Float64ObservableGauge(name string, options ...instrument.Float6 // insts Collect method is called. func (m *meter) RegisterCallback(f metric.Callback, insts ...instrument.Asynchronous) (metric.Registration, error) { if len(insts) == 0 { - // Don't allocate an obeserverRegistry if not needed. + // Don't allocate a multiObserver if not needed. return noopRegister{}, nil } - reg := newObserverRegistry() + reg := newMultiObserver() var errs multierror for _, inst := range insts { switch o := inst.(type) { @@ -249,45 +249,35 @@ func (m *meter) RegisterCallback(f metric.Callback, insts ...instrument.Asynchro } cback := func(ctx context.Context) error { - return f(ctx, reg.multiObserver(ctx)) + return f(ctx, reg) } return m.pipes.registerMultiCallback(cback), nil } -type obeserverRegistry struct { +type multiObserver struct { float64 map[observerID[float64]]struct{} int64 map[observerID[int64]]struct{} } -func newObserverRegistry() obeserverRegistry { - return obeserverRegistry{ +func newMultiObserver() multiObserver { + return multiObserver{ float64: make(map[observerID[float64]]struct{}), int64: make(map[observerID[int64]]struct{}), } } -func (r obeserverRegistry) len() int { +func (r multiObserver) len() int { return len(r.float64) + len(r.int64) } -func (r obeserverRegistry) registerFloat64(id observerID[float64]) { +func (r multiObserver) registerFloat64(id observerID[float64]) { r.float64[id] = struct{}{} } -func (r obeserverRegistry) registerInt64(id observerID[int64]) { +func (r multiObserver) registerInt64(id observerID[int64]) { r.int64[id] = struct{}{} } -func (r obeserverRegistry) multiObserver(ctx context.Context) metric.MultiObserver { - return multiObserver{obeserverRegistry: r, ctx: ctx} -} - -type multiObserver struct { - obeserverRegistry - - ctx context.Context -} - var ( errUnknownObserver = errors.New("unknown observer") errUnregObserver = errors.New("observer not registered for callback") @@ -308,7 +298,7 @@ func (r multiObserver) Float64(o instrument.Float64Observer, v float64, a ...att ) return } - oImpl.observe(r.ctx, v, a) + oImpl.observe(v, a) } func (r multiObserver) Int64(o instrument.Int64Observer, v int64, a ...attribute.KeyValue) { @@ -326,7 +316,7 @@ func (r multiObserver) Int64(o instrument.Int64Observer, v int64, a ...attribute ) return } - oImpl.observe(r.ctx, v, a) + oImpl.observe(v, a) } type noopRegister struct{} @@ -415,6 +405,6 @@ type callbackObserver[N int64 | float64] struct { *observer[N] } -func (o callbackObserver[N]) Observe(ctx context.Context, val N, attrs ...attribute.KeyValue) { - o.observe(ctx, val, attrs) +func (o callbackObserver[N]) Observe(_ context.Context, val N, attrs ...attribute.KeyValue) { + o.observe(val, attrs) } From 63e9ad05fb2f29c3bc2105acf436c0377839733b Mon Sep 17 00:00:00 2001 From: Tyler Yahn Date: Wed, 11 Jan 2023 13:36:52 -0800 Subject: [PATCH 16/31] Revert restructure to RegisterCallback --- CHANGELOG.md | 2 - example/prometheus/main.go | 4 +- metric/example_test.go | 7 +- metric/internal/global/meter.go | 6 +- metric/internal/global/meter_test.go | 18 +++-- metric/internal/global/meter_types_test.go | 2 +- metric/meter.go | 2 +- metric/noop.go | 2 +- sdk/metric/meter.go | 2 +- sdk/metric/meter_test.go | 90 +++++++++++----------- 10 files changed, 68 insertions(+), 67 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index d2fe4f74100..1433b54e420 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -99,8 +99,6 @@ This project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.htm - The exporter from `go.opentelemetry.io/otel/exporters/zipkin` is updated to use the `v1.16.0` version of semantic conventions. This means it no longer uses the removed `net.peer.ip` or `http.host` attributes to determine the remote endpoint. Instead it uses the `net.sock.peer` attributes. (#3581) -- The parameters for the `RegisterCallback` method of the `Meter` from `go.opentelemetry.io/otel/metric` are changed. - The slice of `instrument.Asynchronous` parameter is now passed as a variadic argument. (#3584) - The `Callback` in `go.opentelemetry.io/otel/metric` has the added `MultiObserver` parameter added. This new parameter is used by `Callback` implementations to observe values for asynchronous instruments instead of calling the `Observer` method of the instrument directly. (#3584) diff --git a/example/prometheus/main.go b/example/prometheus/main.go index 4d7be8b776d..4bfeb036089 100644 --- a/example/prometheus/main.go +++ b/example/prometheus/main.go @@ -69,11 +69,11 @@ func main() { if err != nil { log.Fatal(err) } - _, err = meter.RegisterCallback(func(_ context.Context, o api.MultiObserver) error { + _, err = meter.RegisterCallback([]instrument.Asynchronous{gauge}, func(_ context.Context, o api.MultiObserver) error { n := -10. + rand.Float64()*(90.) // [-10, 100) o.Float64(gauge, n, attrs...) return nil - }, gauge) + }) if err != nil { log.Fatal(err) } diff --git a/metric/example_test.go b/metric/example_test.go index 42abfe2cfd1..225569bffc6 100644 --- a/metric/example_test.go +++ b/metric/example_test.go @@ -89,7 +89,10 @@ func ExampleMeter_asynchronous_multiple() { gcCount, _ := meter.Int64ObservableCounter("gcCount") gcPause, _ := meter.Float64Histogram("gcPause") - _, err := meter.RegisterCallback( + _, err := meter.RegisterCallback([]instrument.Asynchronous{ + heapAlloc, + gcCount, + }, func(ctx context.Context, o metric.MultiObserver) error { memStats := &runtime.MemStats{} // This call does work @@ -102,8 +105,6 @@ func ExampleMeter_asynchronous_multiple() { computeGCPauses(ctx, gcPause, memStats.PauseNs[:]) return nil }, - heapAlloc, - gcCount, ) if err != nil { diff --git a/metric/internal/global/meter.go b/metric/internal/global/meter.go index 92f35e9730b..97ad2735bea 100644 --- a/metric/internal/global/meter.go +++ b/metric/internal/global/meter.go @@ -278,10 +278,10 @@ func (m *meter) Float64ObservableGauge(name string, options ...instrument.Float6 // // 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(f metric.Callback, insts ...instrument.Asynchronous) (metric.Registration, error) { +func (m *meter) RegisterCallback(insts []instrument.Asynchronous, f metric.Callback) (metric.Registration, error) { if del, ok := m.delegate.Load().(metric.Meter); ok { insts = unwrapInstruments(insts) - return del.RegisterCallback(f, insts...) + return del.RegisterCallback(insts, f) } m.mtx.Lock() @@ -335,7 +335,7 @@ func (c *registration) setDelegate(m metric.Meter) { return } - reg, err := m.RegisterCallback(c.function, insts...) + reg, err := m.RegisterCallback(insts, c.function) if err != nil { otel.Handle(err) } diff --git a/metric/internal/global/meter_test.go b/metric/internal/global/meter_test.go index 27fd68f1a5d..4f16496fd90 100644 --- a/metric/internal/global/meter_test.go +++ b/metric/internal/global/meter_test.go @@ -70,7 +70,7 @@ func TestMeterRace(t *testing.T) { _, _ = mtr.Int64Counter(name) _, _ = mtr.Int64UpDownCounter(name) _, _ = mtr.Int64Histogram(name) - _, _ = mtr.RegisterCallback(zeroCallback) + _, _ = mtr.RegisterCallback(nil, zeroCallback) if !once { wg.Done() once = true @@ -90,7 +90,7 @@ func TestMeterRace(t *testing.T) { func TestUnregisterRace(t *testing.T) { mtr := &meter{} - reg, err := mtr.RegisterCallback(zeroCallback) + reg, err := mtr.RegisterCallback(nil, zeroCallback) require.NoError(t, err) wg := &sync.WaitGroup{} @@ -132,10 +132,12 @@ func testSetupAllInstrumentTypes(t *testing.T, m metric.Meter) (instrument.Float _, err = m.Int64ObservableGauge("test_Async_Gauge") assert.NoError(t, err) - _, err = m.RegisterCallback(func(ctx context.Context, obs metric.MultiObserver) error { + _, err = m.RegisterCallback([]instrument.Asynchronous{ + afcounter, + }, func(ctx context.Context, obs metric.MultiObserver) error { obs.Float64(afcounter, 3) return nil - }, afcounter) + }) require.NoError(t, err) sfcounter, err := m.Float64Counter("test_Async_Counter") @@ -327,10 +329,10 @@ func TestRegistrationDelegation(t *testing.T) { require.NoError(t, err) var called0 bool - reg0, err := m.RegisterCallback(func(context.Context, metric.MultiObserver) error { + reg0, err := m.RegisterCallback([]instrument.Asynchronous{actr}, func(context.Context, metric.MultiObserver) error { called0 = true return nil - }, actr) + }) require.NoError(t, err) require.Equal(t, 1, mImpl.registry.Len(), "callback not registered") // This means reg0 should not be delegated. @@ -338,10 +340,10 @@ func TestRegistrationDelegation(t *testing.T) { assert.Equal(t, 0, mImpl.registry.Len(), "callback not unregistered") var called1 bool - reg1, err := m.RegisterCallback(func(context.Context, metric.MultiObserver) error { + reg1, err := m.RegisterCallback([]instrument.Asynchronous{actr}, func(context.Context, metric.MultiObserver) error { called1 = true return nil - }, actr) + }) require.NoError(t, err) require.Equal(t, 1, mImpl.registry.Len(), "second callback not registered") diff --git a/metric/internal/global/meter_types_test.go b/metric/internal/global/meter_types_test.go index 57e5214bdaa..70985680a55 100644 --- a/metric/internal/global/meter_types_test.go +++ b/metric/internal/global/meter_types_test.go @@ -116,7 +116,7 @@ func (m *testMeter) Float64ObservableGauge(name string, options ...instrument.Fl // // 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 *testMeter) RegisterCallback(f metric.Callback, i ...instrument.Asynchronous) (metric.Registration, error) { +func (m *testMeter) RegisterCallback(i []instrument.Asynchronous, f metric.Callback) (metric.Registration, error) { m.callbacks = append(m.callbacks, f) return testReg{ f: func(idx int) func() { diff --git a/metric/meter.go b/metric/meter.go index 45010c496ce..910c0c7765b 100644 --- a/metric/meter.go +++ b/metric/meter.go @@ -103,7 +103,7 @@ type Meter interface { // // If no instruments are passed, f should not be registered nor called // during collection. - RegisterCallback(f Callback, instruments ...instrument.Asynchronous) (Registration, error) + RegisterCallback(instruments []instrument.Asynchronous, f Callback) (Registration, error) } // Callback is a function registered with a Meter that makes observations for diff --git a/metric/noop.go b/metric/noop.go index 409268ecc98..98652b4fac7 100644 --- a/metric/noop.go +++ b/metric/noop.go @@ -88,7 +88,7 @@ func (noopMeter) Float64ObservableGauge(string, ...instrument.Float64ObserverOpt } // RegisterCallback creates a register callback that does not record any metrics. -func (noopMeter) RegisterCallback(Callback, ...instrument.Asynchronous) (Registration, error) { +func (noopMeter) RegisterCallback([]instrument.Asynchronous, Callback) (Registration, error) { return noopReg{}, nil } diff --git a/sdk/metric/meter.go b/sdk/metric/meter.go index 7c0cc449510..50893ca449b 100644 --- a/sdk/metric/meter.go +++ b/sdk/metric/meter.go @@ -207,7 +207,7 @@ func (m *meter) Float64ObservableGauge(name string, options ...instrument.Float6 // RegisterCallback registers the function f to be called when any of the // insts Collect method is called. -func (m *meter) RegisterCallback(f metric.Callback, insts ...instrument.Asynchronous) (metric.Registration, error) { +func (m *meter) RegisterCallback(insts []instrument.Asynchronous, f metric.Callback) (metric.Registration, error) { if len(insts) == 0 { // Don't allocate a multiObserver if not needed. return noopRegister{}, nil diff --git a/sdk/metric/meter_test.go b/sdk/metric/meter_test.go index 0634bdb6512..565c030037c 100644 --- a/sdk/metric/meter_test.go +++ b/sdk/metric/meter_test.go @@ -101,11 +101,11 @@ func TestMeterCallbackCreationConcurrency(t *testing.T) { m := NewMeterProvider().Meter("callback-concurrency") go func() { - _, _ = m.RegisterCallback(emptyCallback) + _, _ = m.RegisterCallback([]instrument.Asynchronous{}, emptyCallback) wg.Done() }() go func() { - _, _ = m.RegisterCallback(emptyCallback) + _, _ = m.RegisterCallback([]instrument.Asynchronous{}, emptyCallback) wg.Done() }() wg.Wait() @@ -113,7 +113,7 @@ func TestMeterCallbackCreationConcurrency(t *testing.T) { func TestNoopCallbackUnregisterConcurrency(t *testing.T) { m := NewMeterProvider().Meter("noop-unregister-concurrency") - reg, err := m.RegisterCallback(emptyCallback) + reg, err := m.RegisterCallback(nil, emptyCallback) require.NoError(t, err) wg := &sync.WaitGroup{} @@ -140,10 +140,12 @@ func TestCallbackUnregisterConcurrency(t *testing.T) { ag, err := meter.Int64ObservableGauge("gauge") require.NoError(t, err) - regCtr, err := meter.RegisterCallback(emptyCallback, actr) + i := []instrument.Asynchronous{actr} + regCtr, err := meter.RegisterCallback(i, emptyCallback) require.NoError(t, err) - regG, err := meter.RegisterCallback(emptyCallback, ag) + i = []instrument.Asynchronous{ag} + regG, err := meter.RegisterCallback(i, emptyCallback) require.NoError(t, err) wg := &sync.WaitGroup{} @@ -179,10 +181,10 @@ func TestMeterCreatesInstruments(t *testing.T) { } ctr, err := m.Int64ObservableCounter("aint", instrument.WithInt64Callback(cback)) assert.NoError(t, err) - _, err = m.RegisterCallback(func(_ context.Context, o metric.MultiObserver) error { + _, err = m.RegisterCallback([]instrument.Asynchronous{ctr}, func(_ context.Context, o metric.MultiObserver) error { o.Int64(ctr, 3) return nil - }, ctr) + }) assert.NoError(t, err) // Observed outside of a callback, it should be ignored. @@ -209,10 +211,10 @@ func TestMeterCreatesInstruments(t *testing.T) { } ctr, err := m.Int64ObservableUpDownCounter("aint", instrument.WithInt64Callback(cback)) assert.NoError(t, err) - _, err = m.RegisterCallback(func(ctx context.Context, o metric.MultiObserver) error { + _, err = m.RegisterCallback([]instrument.Asynchronous{ctr}, func(_ context.Context, o metric.MultiObserver) error { o.Int64(ctr, 11) return nil - }, ctr) + }) assert.NoError(t, err) // Observed outside of a callback, it should be ignored. @@ -239,10 +241,10 @@ func TestMeterCreatesInstruments(t *testing.T) { } gauge, err := m.Int64ObservableGauge("agauge", instrument.WithInt64Callback(cback)) assert.NoError(t, err) - _, err = m.RegisterCallback(func(ctx context.Context, o metric.MultiObserver) error { + _, err = m.RegisterCallback([]instrument.Asynchronous{gauge}, func(_ context.Context, o metric.MultiObserver) error { o.Int64(gauge, 11) return nil - }, gauge) + }) assert.NoError(t, err) // Observed outside of a callback, it should be ignored. @@ -267,10 +269,10 @@ func TestMeterCreatesInstruments(t *testing.T) { } ctr, err := m.Float64ObservableCounter("afloat", instrument.WithFloat64Callback(cback)) assert.NoError(t, err) - _, err = m.RegisterCallback(func(ctx context.Context, o metric.MultiObserver) error { + _, err = m.RegisterCallback([]instrument.Asynchronous{ctr}, func(_ context.Context, o metric.MultiObserver) error { o.Float64(ctr, 3) return nil - }, ctr) + }) assert.NoError(t, err) // Observed outside of a callback, it should be ignored. @@ -297,10 +299,10 @@ func TestMeterCreatesInstruments(t *testing.T) { } ctr, err := m.Float64ObservableUpDownCounter("afloat", instrument.WithFloat64Callback(cback)) assert.NoError(t, err) - _, err = m.RegisterCallback(func(ctx context.Context, o metric.MultiObserver) error { + _, err = m.RegisterCallback([]instrument.Asynchronous{ctr}, func(_ context.Context, o metric.MultiObserver) error { o.Float64(ctr, 11) return nil - }, ctr) + }) assert.NoError(t, err) // Observed outside of a callback, it should be ignored. @@ -327,10 +329,10 @@ func TestMeterCreatesInstruments(t *testing.T) { } gauge, err := m.Float64ObservableGauge("agauge", instrument.WithFloat64Callback(cback)) assert.NoError(t, err) - _, err = m.RegisterCallback(func(ctx context.Context, o metric.MultiObserver) error { + _, err = m.RegisterCallback([]instrument.Asynchronous{gauge}, func(_ context.Context, o metric.MultiObserver) error { o.Float64(gauge, 11) return nil - }, gauge) + }) assert.NoError(t, err) // Observed outside of a callback, it should be ignored. @@ -503,19 +505,19 @@ func TestMetersProvideScope(t *testing.T) { m1 := mp.Meter("scope1") ctr1, err := m1.Float64ObservableCounter("ctr1") assert.NoError(t, err) - _, err = m1.RegisterCallback(func(_ context.Context, o metric.MultiObserver) error { + _, err = m1.RegisterCallback([]instrument.Asynchronous{ctr1}, func(_ context.Context, o metric.MultiObserver) error { o.Float64(ctr1, 5) return nil - }, ctr1) + }) assert.NoError(t, err) m2 := mp.Meter("scope2") ctr2, err := m2.Int64ObservableCounter("ctr2") assert.NoError(t, err) - _, err = m2.RegisterCallback(func(_ context.Context, o metric.MultiObserver) error { + _, err = m2.RegisterCallback([]instrument.Asynchronous{ctr2}, func(_ context.Context, o metric.MultiObserver) error { o.Int64(ctr2, 7) return nil - }, ctr2) + }) assert.NoError(t, err) // TODO: verify this should be an error and then fix this test. @@ -592,18 +594,17 @@ func TestUnregisterUnregisters(t *testing.T) { require.NoError(t, err) var called bool - reg, err := m.RegisterCallback( - func(context.Context, metric.MultiObserver) error { - called = true - return nil - }, + reg, err := m.RegisterCallback([]instrument.Asynchronous{ int64Counter, int64UpDownCounter, int64Gauge, floag64Counter, floag64UpDownCounter, floag64Gauge, - ) + }, func(context.Context, metric.MultiObserver) error { + called = true + return nil + }) require.NoError(t, err) ctx := context.Background() @@ -646,18 +647,17 @@ func TestRegisterCallbackDropAggregations(t *testing.T) { require.NoError(t, err) var called bool - _, err = m.RegisterCallback( - func(context.Context, metric.MultiObserver) error { - called = true - return nil - }, + _, err = m.RegisterCallback([]instrument.Asynchronous{ int64Counter, int64UpDownCounter, int64Gauge, floag64Counter, floag64UpDownCounter, floag64Gauge, - ) + }, func(context.Context, metric.MultiObserver) error { + called = true + return nil + }) require.NoError(t, err) data, err := r.Collect(context.Background()) @@ -682,11 +682,11 @@ func TestAttributeFilter(t *testing.T) { if err != nil { return err } - _, err = mtr.RegisterCallback(func(_ context.Context, o metric.MultiObserver) error { + _, err = mtr.RegisterCallback([]instrument.Asynchronous{ctr}, func(_ context.Context, o metric.MultiObserver) error { o.Float64(ctr, 1.0, attribute.String("foo", "bar"), attribute.Int("version", 1)) o.Float64(ctr, 2.0, attribute.String("foo", "bar"), attribute.Int("version", 2)) return nil - }, ctr) + }) return err }, wantMetric: metricdata.Metrics{ @@ -710,11 +710,11 @@ func TestAttributeFilter(t *testing.T) { if err != nil { return err } - _, err = mtr.RegisterCallback(func(_ context.Context, o metric.MultiObserver) error { + _, err = mtr.RegisterCallback([]instrument.Asynchronous{ctr}, func(_ context.Context, o metric.MultiObserver) error { o.Float64(ctr, 1.0, attribute.String("foo", "bar"), attribute.Int("version", 1)) o.Float64(ctr, 2.0, attribute.String("foo", "bar"), attribute.Int("version", 2)) return nil - }, ctr) + }) return err }, wantMetric: metricdata.Metrics{ @@ -738,11 +738,11 @@ func TestAttributeFilter(t *testing.T) { if err != nil { return err } - _, err = mtr.RegisterCallback(func(_ context.Context, o metric.MultiObserver) error { + _, err = mtr.RegisterCallback([]instrument.Asynchronous{ctr}, func(_ context.Context, o metric.MultiObserver) error { o.Float64(ctr, 1.0, attribute.String("foo", "bar"), attribute.Int("version", 1)) o.Float64(ctr, 2.0, attribute.String("foo", "bar"), attribute.Int("version", 2)) return nil - }, ctr) + }) return err }, wantMetric: metricdata.Metrics{ @@ -764,11 +764,11 @@ func TestAttributeFilter(t *testing.T) { if err != nil { return err } - _, err = mtr.RegisterCallback(func(_ context.Context, o metric.MultiObserver) error { + _, err = mtr.RegisterCallback([]instrument.Asynchronous{ctr}, func(_ context.Context, o metric.MultiObserver) error { o.Int64(ctr, 10, attribute.String("foo", "bar"), attribute.Int("version", 1)) o.Int64(ctr, 20, attribute.String("foo", "bar"), attribute.Int("version", 2)) return nil - }, ctr) + }) return err }, wantMetric: metricdata.Metrics{ @@ -792,11 +792,11 @@ func TestAttributeFilter(t *testing.T) { if err != nil { return err } - _, err = mtr.RegisterCallback(func(_ context.Context, o metric.MultiObserver) error { + _, err = mtr.RegisterCallback([]instrument.Asynchronous{ctr}, func(_ context.Context, o metric.MultiObserver) error { o.Int64(ctr, 10, attribute.String("foo", "bar"), attribute.Int("version", 1)) o.Int64(ctr, 20, attribute.String("foo", "bar"), attribute.Int("version", 2)) return nil - }, ctr) + }) return err }, wantMetric: metricdata.Metrics{ @@ -820,11 +820,11 @@ func TestAttributeFilter(t *testing.T) { if err != nil { return err } - _, err = mtr.RegisterCallback(func(_ context.Context, o metric.MultiObserver) error { + _, err = mtr.RegisterCallback([]instrument.Asynchronous{ctr}, func(_ context.Context, o metric.MultiObserver) error { o.Int64(ctr, 10, attribute.String("foo", "bar"), attribute.Int("version", 1)) o.Int64(ctr, 20, attribute.String("foo", "bar"), attribute.Int("version", 2)) return nil - }, ctr) + }) return err }, wantMetric: metricdata.Metrics{ From 04e202060ec2bc543dc13c361462827db5cfcb71 Mon Sep 17 00:00:00 2001 From: Tyler Yahn Date: Wed, 11 Jan 2023 13:48:30 -0800 Subject: [PATCH 17/31] Remove erroneous bug --- CHANGELOG.md | 1 - 1 file changed, 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 1433b54e420..90a41b5c554 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -104,7 +104,6 @@ This project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.htm ### Fixed -- The `RegisterCallback` method of the `Meter` from `go.opentelemetry.io/otel/sdk/metric` only registers a callback once per call, instead of once per instrument the method is called with. (#3584) - The `RegisterCallback` method of the `Meter` from `go.opentelemetry.io/otel/sdk/metric` only registers a callback for instruments created by that meter. Trying to register a callback with instruments from a different meter will result in an error being returned. (#3584) From d5c54780420d89d735288d5b0c0723a039f2c39d Mon Sep 17 00:00:00 2001 From: Tyler Yahn Date: Wed, 11 Jan 2023 14:18:44 -0800 Subject: [PATCH 18/31] Test RegisterCallback for invalid obsrvs --- sdk/metric/meter_test.go | 49 +++++++++++++++++++++++++++++++++++++++- 1 file changed, 48 insertions(+), 1 deletion(-) diff --git a/sdk/metric/meter_test.go b/sdk/metric/meter_test.go index 565c030037c..7a1cb04f26f 100644 --- a/sdk/metric/meter_test.go +++ b/sdk/metric/meter_test.go @@ -498,6 +498,54 @@ func TestMeterCreatesInstruments(t *testing.T) { } } +func TestRegisterNonSDKObserverErrors(t *testing.T) { + rdr := NewManualReader() + mp := NewMeterProvider(WithReader(rdr)) + meter := mp.Meter("scope") + + type obsrv struct{ instrument.Asynchronous } + o := obsrv{} + + _, err := meter.RegisterCallback( + []instrument.Asynchronous{o}, + func(context.Context, metric.MultiObserver) error { return nil }, + ) + assert.ErrorContains( + t, + err, + "invalid observer: from different implementation", + "External instrument registred", + ) +} + +func TestMeterMixingOnRegisterErrors(t *testing.T) { + rdr := NewManualReader() + mp := NewMeterProvider(WithReader(rdr)) + + m1 := mp.Meter("scope1") + m2 := mp.Meter("scope2") + iCtr, err := m2.Int64ObservableCounter("int64 ctr") + require.NoError(t, err) + fCtr, err := m2.Float64ObservableCounter("float64 ctr") + require.NoError(t, err) + _, err = m1.RegisterCallback( + []instrument.Asynchronous{iCtr, fCtr}, + func(context.Context, metric.MultiObserver) error { return nil }, + ) + assert.ErrorContains( + t, + err, + `invalid registration: observer "int64 ctr" from Meter "scope2", registered with Meter "scope1"`, + "Instrument registred with non-creation Meter", + ) + assert.ErrorContains( + t, + err, + `invalid registration: observer "float64 ctr" from Meter "scope2", registered with Meter "scope1"`, + "Instrument registred with non-creation Meter", + ) +} + func TestMetersProvideScope(t *testing.T) { rdr := NewManualReader() mp := NewMeterProvider(WithReader(rdr)) @@ -519,7 +567,6 @@ func TestMetersProvideScope(t *testing.T) { return nil }) assert.NoError(t, err) - // TODO: verify this should be an error and then fix this test. want := metricdata.ResourceMetrics{ Resource: resource.Default(), From ee01dcd609a2fd971eca2a7bfb68fde15b036200 Mon Sep 17 00:00:00 2001 From: Tyler Yahn Date: Wed, 11 Jan 2023 14:29:53 -0800 Subject: [PATCH 19/31] Test callbacks from foreign sources not collected --- sdk/metric/meter_test.go | 66 ++++++++++++++++++++++++++++++++++++++++ 1 file changed, 66 insertions(+) diff --git a/sdk/metric/meter_test.go b/sdk/metric/meter_test.go index 7a1cb04f26f..4765fb62f5b 100644 --- a/sdk/metric/meter_test.go +++ b/sdk/metric/meter_test.go @@ -546,6 +546,72 @@ func TestMeterMixingOnRegisterErrors(t *testing.T) { ) } +func TestCallbackObserverNonRegistered(t *testing.T) { + rdr := NewManualReader() + mp := NewMeterProvider(WithReader(rdr)) + + m1 := mp.Meter("scope1") + valid, err := m1.Int64ObservableCounter("ctr") + require.NoError(t, err) + + m2 := mp.Meter("scope2") + iCtr, err := m2.Int64ObservableCounter("int64 ctr") + require.NoError(t, err) + fCtr, err := m2.Float64ObservableCounter("float64 ctr") + require.NoError(t, err) + + // Panics if Observe is called. + type int64Obsrv struct{ instrument.Int64Observer } + int64Foreign := int64Obsrv{} + type float64Obsrv struct{ instrument.Float64Observer } + float64Foreign := float64Obsrv{} + + _, err = m1.RegisterCallback( + []instrument.Asynchronous{valid}, + func(_ context.Context, o metric.MultiObserver) error { + o.Int64(valid, 1) + o.Int64(iCtr, 1) + o.Float64(fCtr, 1) + o.Int64(int64Foreign, 1) + o.Float64(float64Foreign, 1) + return nil + }, + ) + require.NoError(t, err) + + var got metricdata.ResourceMetrics + assert.NotPanics(t, func() { + got, err = rdr.Collect(context.Background()) + }) + + assert.NoError(t, err) + want := metricdata.ResourceMetrics{ + Resource: resource.Default(), + ScopeMetrics: []metricdata.ScopeMetrics{ + { + Scope: instrumentation.Scope{ + Name: "scope1", + }, + Metrics: []metricdata.Metrics{ + { + Name: "ctr", + Data: metricdata.Sum[int64]{ + Temporality: metricdata.CumulativeTemporality, + IsMonotonic: true, + DataPoints: []metricdata.DataPoint[int64]{ + { + Value: 1, + }, + }, + }, + }, + }, + }, + }, + } + metricdatatest.AssertEqual(t, want, got, metricdatatest.IgnoreTimestamp()) +} + func TestMetersProvideScope(t *testing.T) { rdr := NewManualReader() mp := NewMeterProvider(WithReader(rdr)) From 9ca6b1a39a2a9b4cad5556ad0493c44ae6dccff0 Mon Sep 17 00:00:00 2001 From: Tyler Yahn Date: Thu, 12 Jan 2023 11:31:58 -0800 Subject: [PATCH 20/31] Rename Float64 and Int64 methods of MultiObserver Rename to ObserveFloat64 and ObserveInt64. --- example/prometheus/main.go | 2 +- metric/example_test.go | 4 +- metric/internal/global/meter_test.go | 2 +- metric/internal/global/meter_types_test.go | 4 +- metric/meter.go | 8 ++-- sdk/metric/meter.go | 4 +- sdk/metric/meter_test.go | 50 +++++++++++----------- 7 files changed, 37 insertions(+), 37 deletions(-) diff --git a/example/prometheus/main.go b/example/prometheus/main.go index 4bfeb036089..e27745cd150 100644 --- a/example/prometheus/main.go +++ b/example/prometheus/main.go @@ -71,7 +71,7 @@ func main() { } _, err = meter.RegisterCallback([]instrument.Asynchronous{gauge}, func(_ context.Context, o api.MultiObserver) error { n := -10. + rand.Float64()*(90.) // [-10, 100) - o.Float64(gauge, n, attrs...) + o.ObserveFloat64(gauge, n, attrs...) return nil }) if err != nil { diff --git a/metric/example_test.go b/metric/example_test.go index 225569bffc6..a76fe600325 100644 --- a/metric/example_test.go +++ b/metric/example_test.go @@ -98,8 +98,8 @@ func ExampleMeter_asynchronous_multiple() { // This call does work runtime.ReadMemStats(memStats) - o.Int64(heapAlloc, int64(memStats.HeapAlloc)) - o.Int64(gcCount, int64(memStats.NumGC)) + o.ObserveInt64(heapAlloc, int64(memStats.HeapAlloc)) + o.ObserveInt64(gcCount, int64(memStats.NumGC)) // This function synchronously records the pauses computeGCPauses(ctx, gcPause, memStats.PauseNs[:]) diff --git a/metric/internal/global/meter_test.go b/metric/internal/global/meter_test.go index 4f16496fd90..cf2f8328bab 100644 --- a/metric/internal/global/meter_test.go +++ b/metric/internal/global/meter_test.go @@ -135,7 +135,7 @@ func testSetupAllInstrumentTypes(t *testing.T, m metric.Meter) (instrument.Float _, err = m.RegisterCallback([]instrument.Asynchronous{ afcounter, }, func(ctx context.Context, obs metric.MultiObserver) error { - obs.Float64(afcounter, 3) + obs.ObserveFloat64(afcounter, 3) return nil }) require.NoError(t, err) diff --git a/metric/internal/global/meter_types_test.go b/metric/internal/global/meter_types_test.go index 70985680a55..4ad75e855db 100644 --- a/metric/internal/global/meter_types_test.go +++ b/metric/internal/global/meter_types_test.go @@ -151,10 +151,10 @@ type observationRecorder struct { ctx context.Context } -func (o observationRecorder) Float64(i instrument.Float64Observer, value float64, attr ...attribute.KeyValue) { +func (o observationRecorder) ObserveFloat64(i instrument.Float64Observer, value float64, attr ...attribute.KeyValue) { i.Observe(o.ctx, value, attr...) } -func (o observationRecorder) Int64(i instrument.Int64Observer, value int64, attr ...attribute.KeyValue) { +func (o observationRecorder) ObserveInt64(i instrument.Int64Observer, value int64, attr ...attribute.KeyValue) { i.Observe(o.ctx, value, attr...) } diff --git a/metric/meter.go b/metric/meter.go index 910c0c7765b..528854e28aa 100644 --- a/metric/meter.go +++ b/metric/meter.go @@ -122,10 +122,10 @@ type Callback func(context.Context, MultiObserver) error // MultiObserver records measurements for multiple instruments in a Callback. type MultiObserver interface { - // Float64 records the float64 value with attributes for obsrv. - Float64(obsrv instrument.Float64Observer, value float64, attributes ...attribute.KeyValue) - // Int64 records the int64 value with attributes for obsrv. - Int64(obsrv instrument.Int64Observer, value int64, attributes ...attribute.KeyValue) + // ObserveFloat64 records the float64 value with attributes for obsrv. + ObserveFloat64(obsrv instrument.Float64Observer, value float64, attributes ...attribute.KeyValue) + // ObserveInt64 records the int64 value with attributes for obsrv. + ObserveInt64(obsrv instrument.Int64Observer, value int64, attributes ...attribute.KeyValue) } // Registration is an token representing the unique registration of a callback diff --git a/sdk/metric/meter.go b/sdk/metric/meter.go index 50893ca449b..79b2c49b95a 100644 --- a/sdk/metric/meter.go +++ b/sdk/metric/meter.go @@ -283,7 +283,7 @@ var ( errUnregObserver = errors.New("observer not registered for callback") ) -func (r multiObserver) Float64(o instrument.Float64Observer, v float64, a ...attribute.KeyValue) { +func (r multiObserver) ObserveFloat64(o instrument.Float64Observer, v float64, a ...attribute.KeyValue) { oImpl, ok := o.(*observer[float64]) if !ok { global.Error(errUnknownObserver, "failed to record") @@ -301,7 +301,7 @@ func (r multiObserver) Float64(o instrument.Float64Observer, v float64, a ...att oImpl.observe(v, a) } -func (r multiObserver) Int64(o instrument.Int64Observer, v int64, a ...attribute.KeyValue) { +func (r multiObserver) ObserveInt64(o instrument.Int64Observer, v int64, a ...attribute.KeyValue) { oImpl, ok := o.(*observer[int64]) if !ok { global.Error(errUnknownObserver, "failed to record") diff --git a/sdk/metric/meter_test.go b/sdk/metric/meter_test.go index 4765fb62f5b..166db6f3718 100644 --- a/sdk/metric/meter_test.go +++ b/sdk/metric/meter_test.go @@ -182,7 +182,7 @@ func TestMeterCreatesInstruments(t *testing.T) { ctr, err := m.Int64ObservableCounter("aint", instrument.WithInt64Callback(cback)) assert.NoError(t, err) _, err = m.RegisterCallback([]instrument.Asynchronous{ctr}, func(_ context.Context, o metric.MultiObserver) error { - o.Int64(ctr, 3) + o.ObserveInt64(ctr, 3) return nil }) assert.NoError(t, err) @@ -212,7 +212,7 @@ func TestMeterCreatesInstruments(t *testing.T) { ctr, err := m.Int64ObservableUpDownCounter("aint", instrument.WithInt64Callback(cback)) assert.NoError(t, err) _, err = m.RegisterCallback([]instrument.Asynchronous{ctr}, func(_ context.Context, o metric.MultiObserver) error { - o.Int64(ctr, 11) + o.ObserveInt64(ctr, 11) return nil }) assert.NoError(t, err) @@ -242,7 +242,7 @@ func TestMeterCreatesInstruments(t *testing.T) { gauge, err := m.Int64ObservableGauge("agauge", instrument.WithInt64Callback(cback)) assert.NoError(t, err) _, err = m.RegisterCallback([]instrument.Asynchronous{gauge}, func(_ context.Context, o metric.MultiObserver) error { - o.Int64(gauge, 11) + o.ObserveInt64(gauge, 11) return nil }) assert.NoError(t, err) @@ -270,7 +270,7 @@ func TestMeterCreatesInstruments(t *testing.T) { ctr, err := m.Float64ObservableCounter("afloat", instrument.WithFloat64Callback(cback)) assert.NoError(t, err) _, err = m.RegisterCallback([]instrument.Asynchronous{ctr}, func(_ context.Context, o metric.MultiObserver) error { - o.Float64(ctr, 3) + o.ObserveFloat64(ctr, 3) return nil }) assert.NoError(t, err) @@ -300,7 +300,7 @@ func TestMeterCreatesInstruments(t *testing.T) { ctr, err := m.Float64ObservableUpDownCounter("afloat", instrument.WithFloat64Callback(cback)) assert.NoError(t, err) _, err = m.RegisterCallback([]instrument.Asynchronous{ctr}, func(_ context.Context, o metric.MultiObserver) error { - o.Float64(ctr, 11) + o.ObserveFloat64(ctr, 11) return nil }) assert.NoError(t, err) @@ -330,7 +330,7 @@ func TestMeterCreatesInstruments(t *testing.T) { gauge, err := m.Float64ObservableGauge("agauge", instrument.WithFloat64Callback(cback)) assert.NoError(t, err) _, err = m.RegisterCallback([]instrument.Asynchronous{gauge}, func(_ context.Context, o metric.MultiObserver) error { - o.Float64(gauge, 11) + o.ObserveFloat64(gauge, 11) return nil }) assert.NoError(t, err) @@ -569,11 +569,11 @@ func TestCallbackObserverNonRegistered(t *testing.T) { _, err = m1.RegisterCallback( []instrument.Asynchronous{valid}, func(_ context.Context, o metric.MultiObserver) error { - o.Int64(valid, 1) - o.Int64(iCtr, 1) - o.Float64(fCtr, 1) - o.Int64(int64Foreign, 1) - o.Float64(float64Foreign, 1) + o.ObserveInt64(valid, 1) + o.ObserveInt64(iCtr, 1) + o.ObserveFloat64(fCtr, 1) + o.ObserveInt64(int64Foreign, 1) + o.ObserveFloat64(float64Foreign, 1) return nil }, ) @@ -620,7 +620,7 @@ func TestMetersProvideScope(t *testing.T) { ctr1, err := m1.Float64ObservableCounter("ctr1") assert.NoError(t, err) _, err = m1.RegisterCallback([]instrument.Asynchronous{ctr1}, func(_ context.Context, o metric.MultiObserver) error { - o.Float64(ctr1, 5) + o.ObserveFloat64(ctr1, 5) return nil }) assert.NoError(t, err) @@ -629,7 +629,7 @@ func TestMetersProvideScope(t *testing.T) { ctr2, err := m2.Int64ObservableCounter("ctr2") assert.NoError(t, err) _, err = m2.RegisterCallback([]instrument.Asynchronous{ctr2}, func(_ context.Context, o metric.MultiObserver) error { - o.Int64(ctr2, 7) + o.ObserveInt64(ctr2, 7) return nil }) assert.NoError(t, err) @@ -796,8 +796,8 @@ func TestAttributeFilter(t *testing.T) { return err } _, err = mtr.RegisterCallback([]instrument.Asynchronous{ctr}, func(_ context.Context, o metric.MultiObserver) error { - o.Float64(ctr, 1.0, attribute.String("foo", "bar"), attribute.Int("version", 1)) - o.Float64(ctr, 2.0, attribute.String("foo", "bar"), attribute.Int("version", 2)) + o.ObserveFloat64(ctr, 1.0, attribute.String("foo", "bar"), attribute.Int("version", 1)) + o.ObserveFloat64(ctr, 2.0, attribute.String("foo", "bar"), attribute.Int("version", 2)) return nil }) return err @@ -824,8 +824,8 @@ func TestAttributeFilter(t *testing.T) { return err } _, err = mtr.RegisterCallback([]instrument.Asynchronous{ctr}, func(_ context.Context, o metric.MultiObserver) error { - o.Float64(ctr, 1.0, attribute.String("foo", "bar"), attribute.Int("version", 1)) - o.Float64(ctr, 2.0, attribute.String("foo", "bar"), attribute.Int("version", 2)) + o.ObserveFloat64(ctr, 1.0, attribute.String("foo", "bar"), attribute.Int("version", 1)) + o.ObserveFloat64(ctr, 2.0, attribute.String("foo", "bar"), attribute.Int("version", 2)) return nil }) return err @@ -852,8 +852,8 @@ func TestAttributeFilter(t *testing.T) { return err } _, err = mtr.RegisterCallback([]instrument.Asynchronous{ctr}, func(_ context.Context, o metric.MultiObserver) error { - o.Float64(ctr, 1.0, attribute.String("foo", "bar"), attribute.Int("version", 1)) - o.Float64(ctr, 2.0, attribute.String("foo", "bar"), attribute.Int("version", 2)) + o.ObserveFloat64(ctr, 1.0, attribute.String("foo", "bar"), attribute.Int("version", 1)) + o.ObserveFloat64(ctr, 2.0, attribute.String("foo", "bar"), attribute.Int("version", 2)) return nil }) return err @@ -878,8 +878,8 @@ func TestAttributeFilter(t *testing.T) { return err } _, err = mtr.RegisterCallback([]instrument.Asynchronous{ctr}, func(_ context.Context, o metric.MultiObserver) error { - o.Int64(ctr, 10, attribute.String("foo", "bar"), attribute.Int("version", 1)) - o.Int64(ctr, 20, attribute.String("foo", "bar"), attribute.Int("version", 2)) + o.ObserveInt64(ctr, 10, attribute.String("foo", "bar"), attribute.Int("version", 1)) + o.ObserveInt64(ctr, 20, attribute.String("foo", "bar"), attribute.Int("version", 2)) return nil }) return err @@ -906,8 +906,8 @@ func TestAttributeFilter(t *testing.T) { return err } _, err = mtr.RegisterCallback([]instrument.Asynchronous{ctr}, func(_ context.Context, o metric.MultiObserver) error { - o.Int64(ctr, 10, attribute.String("foo", "bar"), attribute.Int("version", 1)) - o.Int64(ctr, 20, attribute.String("foo", "bar"), attribute.Int("version", 2)) + o.ObserveInt64(ctr, 10, attribute.String("foo", "bar"), attribute.Int("version", 1)) + o.ObserveInt64(ctr, 20, attribute.String("foo", "bar"), attribute.Int("version", 2)) return nil }) return err @@ -934,8 +934,8 @@ func TestAttributeFilter(t *testing.T) { return err } _, err = mtr.RegisterCallback([]instrument.Asynchronous{ctr}, func(_ context.Context, o metric.MultiObserver) error { - o.Int64(ctr, 10, attribute.String("foo", "bar"), attribute.Int("version", 1)) - o.Int64(ctr, 20, attribute.String("foo", "bar"), attribute.Int("version", 2)) + o.ObserveInt64(ctr, 10, attribute.String("foo", "bar"), attribute.Int("version", 1)) + o.ObserveInt64(ctr, 20, attribute.String("foo", "bar"), attribute.Int("version", 2)) return nil }) return err From 04d2712c8d4a0cf2ef79d375eea0f1245177b5b9 Mon Sep 17 00:00:00 2001 From: Tyler Yahn Date: Thu, 12 Jan 2023 11:38:52 -0800 Subject: [PATCH 21/31] Rename MultiObserver to Observer --- CHANGELOG.md | 2 +- example/prometheus/main.go | 2 +- metric/example_test.go | 2 +- metric/internal/global/meter_test.go | 8 ++-- metric/meter.go | 10 ++-- sdk/metric/instrument.go | 38 ++++++++-------- sdk/metric/meter.go | 68 ++++++++++++++-------------- sdk/metric/meter_test.go | 46 +++++++++---------- 8 files changed, 88 insertions(+), 88 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index c05bd08b204..d01665b5847 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -100,7 +100,7 @@ This project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.htm - The exporter from `go.opentelemetry.io/otel/exporters/zipkin` is updated to use the `v1.16.0` version of semantic conventions. This means it no longer uses the removed `net.peer.ip` or `http.host` attributes to determine the remote endpoint. Instead it uses the `net.sock.peer` attributes. (#3581) -- The `Callback` in `go.opentelemetry.io/otel/metric` has the added `MultiObserver` parameter added. +- The `Callback` in `go.opentelemetry.io/otel/metric` has the added `Observer` parameter added. This new parameter is used by `Callback` implementations to observe values for asynchronous instruments instead of calling the `Observer` method of the instrument directly. (#3584) ### Fixed diff --git a/example/prometheus/main.go b/example/prometheus/main.go index e27745cd150..8fba3bd5d7a 100644 --- a/example/prometheus/main.go +++ b/example/prometheus/main.go @@ -69,7 +69,7 @@ func main() { if err != nil { log.Fatal(err) } - _, err = meter.RegisterCallback([]instrument.Asynchronous{gauge}, func(_ context.Context, o api.MultiObserver) error { + _, err = meter.RegisterCallback([]instrument.Asynchronous{gauge}, func(_ context.Context, o api.Observer) error { n := -10. + rand.Float64()*(90.) // [-10, 100) o.ObserveFloat64(gauge, n, attrs...) return nil diff --git a/metric/example_test.go b/metric/example_test.go index a76fe600325..96db543c35a 100644 --- a/metric/example_test.go +++ b/metric/example_test.go @@ -93,7 +93,7 @@ func ExampleMeter_asynchronous_multiple() { heapAlloc, gcCount, }, - func(ctx context.Context, o metric.MultiObserver) error { + func(ctx context.Context, o metric.Observer) error { memStats := &runtime.MemStats{} // This call does work runtime.ReadMemStats(memStats) diff --git a/metric/internal/global/meter_test.go b/metric/internal/global/meter_test.go index cf2f8328bab..91eea3411bb 100644 --- a/metric/internal/global/meter_test.go +++ b/metric/internal/global/meter_test.go @@ -45,7 +45,7 @@ func TestMeterProviderRace(t *testing.T) { close(finish) } -var zeroCallback metric.Callback = func(ctx context.Context, or metric.MultiObserver) error { +var zeroCallback metric.Callback = func(ctx context.Context, or metric.Observer) error { return nil } @@ -134,7 +134,7 @@ func testSetupAllInstrumentTypes(t *testing.T, m metric.Meter) (instrument.Float _, err = m.RegisterCallback([]instrument.Asynchronous{ afcounter, - }, func(ctx context.Context, obs metric.MultiObserver) error { + }, func(ctx context.Context, obs metric.Observer) error { obs.ObserveFloat64(afcounter, 3) return nil }) @@ -329,7 +329,7 @@ func TestRegistrationDelegation(t *testing.T) { require.NoError(t, err) var called0 bool - reg0, err := m.RegisterCallback([]instrument.Asynchronous{actr}, func(context.Context, metric.MultiObserver) error { + reg0, err := m.RegisterCallback([]instrument.Asynchronous{actr}, func(context.Context, metric.Observer) error { called0 = true return nil }) @@ -340,7 +340,7 @@ func TestRegistrationDelegation(t *testing.T) { assert.Equal(t, 0, mImpl.registry.Len(), "callback not unregistered") var called1 bool - reg1, err := m.RegisterCallback([]instrument.Asynchronous{actr}, func(context.Context, metric.MultiObserver) error { + reg1, err := m.RegisterCallback([]instrument.Asynchronous{actr}, func(context.Context, metric.Observer) error { called1 = true return nil }) diff --git a/metric/meter.go b/metric/meter.go index 528854e28aa..9248a40db84 100644 --- a/metric/meter.go +++ b/metric/meter.go @@ -107,8 +107,8 @@ type Meter interface { } // Callback is a function registered with a Meter that makes observations for -// the set of instruments it is registered with. The MultiObserver parameter is -// used to record measurment observations for these instruments. +// the set of instruments it is registered with. The Observer parameter is used +// to record measurment observations for these instruments. // // The function needs to complete in a finite amount of time and the deadline // of the passed context is expected to be honored. @@ -118,10 +118,10 @@ type Meter interface { // the same attributes as another Callback will report. // // The function needs to be concurrent safe. -type Callback func(context.Context, MultiObserver) error +type Callback func(context.Context, Observer) error -// MultiObserver records measurements for multiple instruments in a Callback. -type MultiObserver interface { +// Observer records measurements for multiple instruments in a Callback. +type Observer interface { // ObserveFloat64 records the float64 value with attributes for obsrv. ObserveFloat64(obsrv instrument.Float64Observer, value float64, attributes ...attribute.KeyValue) // ObserveInt64 records the int64 value with attributes for obsrv. diff --git a/sdk/metric/instrument.go b/sdk/metric/instrument.go index b93a7bb84c1..bdcb822f18e 100644 --- a/sdk/metric/instrument.go +++ b/sdk/metric/instrument.go @@ -202,8 +202,8 @@ func (i *instrumentImpl[N]) aggregate(ctx context.Context, val N, attrs []attrib } } -// observerID is a comparable unique identifier of an observer. -type observerID[N int64 | float64] struct { +// observablID is a comparable unique identifier of an observable. +type observablID[N int64 | float64] struct { name string description string kind InstrumentKind @@ -211,16 +211,16 @@ type observerID[N int64 | float64] struct { scope instrumentation.Scope } -type observer[N int64 | float64] struct { +type observable[N int64 | float64] struct { instrument.Asynchronous - observerID[N] + observablID[N] aggregators []internal.Aggregator[N] } -func newObserver[N int64 | float64](scope instrumentation.Scope, kind InstrumentKind, name, desc string, u unit.Unit, agg []internal.Aggregator[N]) *observer[N] { - return &observer[N]{ - observerID: observerID[N]{ +func newObservable[N int64 | float64](scope instrumentation.Scope, kind InstrumentKind, name, desc string, u unit.Unit, agg []internal.Aggregator[N]) *observable[N] { + return &observable[N]{ + observablID: observablID[N]{ name: name, description: desc, kind: kind, @@ -231,15 +231,15 @@ func newObserver[N int64 | float64](scope instrumentation.Scope, kind Instrument } } -var _ instrument.Float64ObservableCounter = (*observer[float64])(nil) -var _ instrument.Float64ObservableUpDownCounter = (*observer[float64])(nil) -var _ instrument.Float64ObservableGauge = (*observer[float64])(nil) -var _ instrument.Int64ObservableCounter = (*observer[int64])(nil) -var _ instrument.Int64ObservableUpDownCounter = (*observer[int64])(nil) -var _ instrument.Int64ObservableGauge = (*observer[int64])(nil) +var _ instrument.Float64ObservableCounter = (*observable[float64])(nil) +var _ instrument.Float64ObservableUpDownCounter = (*observable[float64])(nil) +var _ instrument.Float64ObservableGauge = (*observable[float64])(nil) +var _ instrument.Int64ObservableCounter = (*observable[int64])(nil) +var _ instrument.Int64ObservableUpDownCounter = (*observable[int64])(nil) +var _ instrument.Int64ObservableGauge = (*observable[int64])(nil) // Observe logs an error. -func (o *observer[N]) Observe(ctx context.Context, val N, attrs ...attribute.KeyValue) { +func (o *observable[N]) Observe(ctx context.Context, val N, attrs ...attribute.KeyValue) { var zero N err := errors.New("invalid observation recording") global.Error(err, "dropping measurement", @@ -251,25 +251,25 @@ func (o *observer[N]) Observe(ctx context.Context, val N, attrs ...attribute.Key } // observe records the val for the set of attrs. -func (o *observer[N]) observe(val N, attrs []attribute.KeyValue) { +func (o *observable[N]) observe(val N, attrs []attribute.KeyValue) { for _, agg := range o.aggregators { agg.Aggregate(val, attribute.NewSet(attrs...)) } } -var errEmptyAgg = errors.New("no aggregators for observer") +var errEmptyAgg = errors.New("no aggregators for observable") -// registerable returns an error if the observer o should not be registered, +// registerable returns an error if the observable o should not be registered, // and nil if it should. An errEmptyAgg error is returned if o is effecively a // no-op because it does not have any aggregators. Also, an error is returned // if scope defines a Meter other than the one o was created by. -func (o *observer[N]) registerable(scope instrumentation.Scope) error { +func (o *observable[N]) registerable(scope instrumentation.Scope) error { if len(o.aggregators) == 0 { return errEmptyAgg } if scope != o.scope { return fmt.Errorf( - "invalid registration: observer %q from Meter %q, registered with Meter %q", + "invalid registration: observable %q from Meter %q, registered with Meter %q", o.name, o.scope.Name, scope.Name, diff --git a/sdk/metric/meter.go b/sdk/metric/meter.go index 79b2c49b95a..cbf79b76830 100644 --- a/sdk/metric/meter.go +++ b/sdk/metric/meter.go @@ -209,33 +209,33 @@ func (m *meter) Float64ObservableGauge(name string, options ...instrument.Float6 // insts Collect method is called. func (m *meter) RegisterCallback(insts []instrument.Asynchronous, f metric.Callback) (metric.Registration, error) { if len(insts) == 0 { - // Don't allocate a multiObserver if not needed. + // Don't allocate a observer if not needed. return noopRegister{}, nil } - reg := newMultiObserver() + reg := newObserver() var errs multierror for _, inst := range insts { switch o := inst.(type) { - case *observer[int64]: + case *observable[int64]: if err := o.registerable(m.scope); err != nil { if !errors.Is(err, errEmptyAgg) { errs.append(err) } continue } - reg.registerInt64(o.observerID) - case *observer[float64]: + reg.registerInt64(o.observablID) + case *observable[float64]: if err := o.registerable(m.scope); err != nil { if !errors.Is(err, errEmptyAgg) { errs.append(err) } continue } - reg.registerFloat64(o.observerID) + reg.registerFloat64(o.observablID) default: // Instrument external to the SDK. - return nil, fmt.Errorf("invalid observer: from different implementation") + return nil, fmt.Errorf("invalid observable: from different implementation") } } @@ -254,42 +254,42 @@ func (m *meter) RegisterCallback(insts []instrument.Asynchronous, f metric.Callb return m.pipes.registerMultiCallback(cback), nil } -type multiObserver struct { - float64 map[observerID[float64]]struct{} - int64 map[observerID[int64]]struct{} +type observer struct { + float64 map[observablID[float64]]struct{} + int64 map[observablID[int64]]struct{} } -func newMultiObserver() multiObserver { - return multiObserver{ - float64: make(map[observerID[float64]]struct{}), - int64: make(map[observerID[int64]]struct{}), +func newObserver() observer { + return observer{ + float64: make(map[observablID[float64]]struct{}), + int64: make(map[observablID[int64]]struct{}), } } -func (r multiObserver) len() int { +func (r observer) len() int { return len(r.float64) + len(r.int64) } -func (r multiObserver) registerFloat64(id observerID[float64]) { +func (r observer) registerFloat64(id observablID[float64]) { r.float64[id] = struct{}{} } -func (r multiObserver) registerInt64(id observerID[int64]) { +func (r observer) registerInt64(id observablID[int64]) { r.int64[id] = struct{}{} } var ( - errUnknownObserver = errors.New("unknown observer") - errUnregObserver = errors.New("observer not registered for callback") + errUnknownObserver = errors.New("unknown observable") + errUnregObserver = errors.New("observable not registered for callback") ) -func (r multiObserver) ObserveFloat64(o instrument.Float64Observer, v float64, a ...attribute.KeyValue) { - oImpl, ok := o.(*observer[float64]) +func (r observer) ObserveFloat64(o instrument.Float64Observer, v float64, a ...attribute.KeyValue) { + oImpl, ok := o.(*observable[float64]) if !ok { global.Error(errUnknownObserver, "failed to record") return } - if _, registered := r.float64[oImpl.observerID]; !registered { + if _, registered := r.float64[oImpl.observablID]; !registered { global.Error(errUnregObserver, "failed to record", "name", oImpl.name, "description", oImpl.description, @@ -301,13 +301,13 @@ func (r multiObserver) ObserveFloat64(o instrument.Float64Observer, v float64, a oImpl.observe(v, a) } -func (r multiObserver) ObserveInt64(o instrument.Int64Observer, v int64, a ...attribute.KeyValue) { - oImpl, ok := o.(*observer[int64]) +func (r observer) ObserveInt64(o instrument.Int64Observer, v int64, a ...attribute.KeyValue) { + oImpl, ok := o.(*observable[int64]) if !ok { global.Error(errUnknownObserver, "failed to record") return } - if _, registered := r.int64[oImpl.observerID]; !registered { + if _, registered := r.int64[oImpl.observablID]; !registered { global.Error(errUnregObserver, "failed to record", "name", oImpl.name, "description", oImpl.description, @@ -355,12 +355,12 @@ func (p *instProvider[N]) lookup(kind InstrumentKind, name, desc string, u unit. type int64ObservProvider struct{ *instProvider[int64] } -func (p int64ObservProvider) lookup(kind InstrumentKind, name, desc string, u unit.Unit) (*observer[int64], error) { +func (p int64ObservProvider) lookup(kind InstrumentKind, name, desc string, u unit.Unit) (*observable[int64], error) { aggs, err := p.aggs(kind, name, desc, u) - return newObserver(p.scope, kind, name, desc, u, aggs), err + return newObservable(p.scope, kind, name, desc, u, aggs), err } -func (p int64ObservProvider) registerCallbacks(inst *observer[int64], cBacks []instrument.Int64Callback) { +func (p int64ObservProvider) registerCallbacks(inst *observable[int64], cBacks []instrument.Int64Callback) { if inst == nil { // Drop aggregator. return @@ -371,19 +371,19 @@ func (p int64ObservProvider) registerCallbacks(inst *observer[int64], cBacks []i } } -func (p int64ObservProvider) callback(i *observer[int64], f instrument.Int64Callback) func(context.Context) error { +func (p int64ObservProvider) callback(i *observable[int64], f instrument.Int64Callback) func(context.Context) error { inst := callbackObserver[int64]{i} return func(ctx context.Context) error { return f(ctx, inst) } } type float64ObservProvider struct{ *instProvider[float64] } -func (p float64ObservProvider) lookup(kind InstrumentKind, name, desc string, u unit.Unit) (*observer[float64], error) { +func (p float64ObservProvider) lookup(kind InstrumentKind, name, desc string, u unit.Unit) (*observable[float64], error) { aggs, err := p.aggs(kind, name, desc, u) - return newObserver(p.scope, kind, name, desc, u, aggs), err + return newObservable(p.scope, kind, name, desc, u, aggs), err } -func (p float64ObservProvider) registerCallbacks(inst *observer[float64], cBacks []instrument.Float64Callback) { +func (p float64ObservProvider) registerCallbacks(inst *observable[float64], cBacks []instrument.Float64Callback) { if inst == nil { // Drop aggregator. return @@ -394,7 +394,7 @@ func (p float64ObservProvider) registerCallbacks(inst *observer[float64], cBacks } } -func (p float64ObservProvider) callback(i *observer[float64], f instrument.Float64Callback) func(context.Context) error { +func (p float64ObservProvider) callback(i *observable[float64], f instrument.Float64Callback) func(context.Context) error { inst := callbackObserver[float64]{i} return func(ctx context.Context) error { return f(ctx, inst) } } @@ -402,7 +402,7 @@ func (p float64ObservProvider) callback(i *observer[float64], f instrument.Float // callbackObserver is passed to a callback where a users is expected to call // Observe directly to record a measurement. type callbackObserver[N int64 | float64] struct { - *observer[N] + *observable[N] } func (o callbackObserver[N]) Observe(_ context.Context, val N, attrs ...attribute.KeyValue) { diff --git a/sdk/metric/meter_test.go b/sdk/metric/meter_test.go index 166db6f3718..24202ba548a 100644 --- a/sdk/metric/meter_test.go +++ b/sdk/metric/meter_test.go @@ -91,7 +91,7 @@ func TestMeterInstrumentConcurrency(t *testing.T) { wg.Wait() } -var emptyCallback metric.Callback = func(context.Context, metric.MultiObserver) error { return nil } +var emptyCallback metric.Callback = func(context.Context, metric.Observer) error { return nil } // A Meter Should be able register Callbacks Concurrently. func TestMeterCallbackCreationConcurrency(t *testing.T) { @@ -181,7 +181,7 @@ func TestMeterCreatesInstruments(t *testing.T) { } ctr, err := m.Int64ObservableCounter("aint", instrument.WithInt64Callback(cback)) assert.NoError(t, err) - _, err = m.RegisterCallback([]instrument.Asynchronous{ctr}, func(_ context.Context, o metric.MultiObserver) error { + _, err = m.RegisterCallback([]instrument.Asynchronous{ctr}, func(_ context.Context, o metric.Observer) error { o.ObserveInt64(ctr, 3) return nil }) @@ -211,7 +211,7 @@ func TestMeterCreatesInstruments(t *testing.T) { } ctr, err := m.Int64ObservableUpDownCounter("aint", instrument.WithInt64Callback(cback)) assert.NoError(t, err) - _, err = m.RegisterCallback([]instrument.Asynchronous{ctr}, func(_ context.Context, o metric.MultiObserver) error { + _, err = m.RegisterCallback([]instrument.Asynchronous{ctr}, func(_ context.Context, o metric.Observer) error { o.ObserveInt64(ctr, 11) return nil }) @@ -241,7 +241,7 @@ func TestMeterCreatesInstruments(t *testing.T) { } gauge, err := m.Int64ObservableGauge("agauge", instrument.WithInt64Callback(cback)) assert.NoError(t, err) - _, err = m.RegisterCallback([]instrument.Asynchronous{gauge}, func(_ context.Context, o metric.MultiObserver) error { + _, err = m.RegisterCallback([]instrument.Asynchronous{gauge}, func(_ context.Context, o metric.Observer) error { o.ObserveInt64(gauge, 11) return nil }) @@ -269,7 +269,7 @@ func TestMeterCreatesInstruments(t *testing.T) { } ctr, err := m.Float64ObservableCounter("afloat", instrument.WithFloat64Callback(cback)) assert.NoError(t, err) - _, err = m.RegisterCallback([]instrument.Asynchronous{ctr}, func(_ context.Context, o metric.MultiObserver) error { + _, err = m.RegisterCallback([]instrument.Asynchronous{ctr}, func(_ context.Context, o metric.Observer) error { o.ObserveFloat64(ctr, 3) return nil }) @@ -299,7 +299,7 @@ func TestMeterCreatesInstruments(t *testing.T) { } ctr, err := m.Float64ObservableUpDownCounter("afloat", instrument.WithFloat64Callback(cback)) assert.NoError(t, err) - _, err = m.RegisterCallback([]instrument.Asynchronous{ctr}, func(_ context.Context, o metric.MultiObserver) error { + _, err = m.RegisterCallback([]instrument.Asynchronous{ctr}, func(_ context.Context, o metric.Observer) error { o.ObserveFloat64(ctr, 11) return nil }) @@ -329,7 +329,7 @@ func TestMeterCreatesInstruments(t *testing.T) { } gauge, err := m.Float64ObservableGauge("agauge", instrument.WithFloat64Callback(cback)) assert.NoError(t, err) - _, err = m.RegisterCallback([]instrument.Asynchronous{gauge}, func(_ context.Context, o metric.MultiObserver) error { + _, err = m.RegisterCallback([]instrument.Asynchronous{gauge}, func(_ context.Context, o metric.Observer) error { o.ObserveFloat64(gauge, 11) return nil }) @@ -508,12 +508,12 @@ func TestRegisterNonSDKObserverErrors(t *testing.T) { _, err := meter.RegisterCallback( []instrument.Asynchronous{o}, - func(context.Context, metric.MultiObserver) error { return nil }, + func(context.Context, metric.Observer) error { return nil }, ) assert.ErrorContains( t, err, - "invalid observer: from different implementation", + "invalid observable: from different implementation", "External instrument registred", ) } @@ -530,18 +530,18 @@ func TestMeterMixingOnRegisterErrors(t *testing.T) { require.NoError(t, err) _, err = m1.RegisterCallback( []instrument.Asynchronous{iCtr, fCtr}, - func(context.Context, metric.MultiObserver) error { return nil }, + func(context.Context, metric.Observer) error { return nil }, ) assert.ErrorContains( t, err, - `invalid registration: observer "int64 ctr" from Meter "scope2", registered with Meter "scope1"`, + `invalid registration: observable "int64 ctr" from Meter "scope2", registered with Meter "scope1"`, "Instrument registred with non-creation Meter", ) assert.ErrorContains( t, err, - `invalid registration: observer "float64 ctr" from Meter "scope2", registered with Meter "scope1"`, + `invalid registration: observable "float64 ctr" from Meter "scope2", registered with Meter "scope1"`, "Instrument registred with non-creation Meter", ) } @@ -568,7 +568,7 @@ func TestCallbackObserverNonRegistered(t *testing.T) { _, err = m1.RegisterCallback( []instrument.Asynchronous{valid}, - func(_ context.Context, o metric.MultiObserver) error { + func(_ context.Context, o metric.Observer) error { o.ObserveInt64(valid, 1) o.ObserveInt64(iCtr, 1) o.ObserveFloat64(fCtr, 1) @@ -619,7 +619,7 @@ func TestMetersProvideScope(t *testing.T) { m1 := mp.Meter("scope1") ctr1, err := m1.Float64ObservableCounter("ctr1") assert.NoError(t, err) - _, err = m1.RegisterCallback([]instrument.Asynchronous{ctr1}, func(_ context.Context, o metric.MultiObserver) error { + _, err = m1.RegisterCallback([]instrument.Asynchronous{ctr1}, func(_ context.Context, o metric.Observer) error { o.ObserveFloat64(ctr1, 5) return nil }) @@ -628,7 +628,7 @@ func TestMetersProvideScope(t *testing.T) { m2 := mp.Meter("scope2") ctr2, err := m2.Int64ObservableCounter("ctr2") assert.NoError(t, err) - _, err = m2.RegisterCallback([]instrument.Asynchronous{ctr2}, func(_ context.Context, o metric.MultiObserver) error { + _, err = m2.RegisterCallback([]instrument.Asynchronous{ctr2}, func(_ context.Context, o metric.Observer) error { o.ObserveInt64(ctr2, 7) return nil }) @@ -714,7 +714,7 @@ func TestUnregisterUnregisters(t *testing.T) { floag64Counter, floag64UpDownCounter, floag64Gauge, - }, func(context.Context, metric.MultiObserver) error { + }, func(context.Context, metric.Observer) error { called = true return nil }) @@ -767,7 +767,7 @@ func TestRegisterCallbackDropAggregations(t *testing.T) { floag64Counter, floag64UpDownCounter, floag64Gauge, - }, func(context.Context, metric.MultiObserver) error { + }, func(context.Context, metric.Observer) error { called = true return nil }) @@ -795,7 +795,7 @@ func TestAttributeFilter(t *testing.T) { if err != nil { return err } - _, err = mtr.RegisterCallback([]instrument.Asynchronous{ctr}, func(_ context.Context, o metric.MultiObserver) error { + _, err = mtr.RegisterCallback([]instrument.Asynchronous{ctr}, func(_ context.Context, o metric.Observer) error { o.ObserveFloat64(ctr, 1.0, attribute.String("foo", "bar"), attribute.Int("version", 1)) o.ObserveFloat64(ctr, 2.0, attribute.String("foo", "bar"), attribute.Int("version", 2)) return nil @@ -823,7 +823,7 @@ func TestAttributeFilter(t *testing.T) { if err != nil { return err } - _, err = mtr.RegisterCallback([]instrument.Asynchronous{ctr}, func(_ context.Context, o metric.MultiObserver) error { + _, err = mtr.RegisterCallback([]instrument.Asynchronous{ctr}, func(_ context.Context, o metric.Observer) error { o.ObserveFloat64(ctr, 1.0, attribute.String("foo", "bar"), attribute.Int("version", 1)) o.ObserveFloat64(ctr, 2.0, attribute.String("foo", "bar"), attribute.Int("version", 2)) return nil @@ -851,7 +851,7 @@ func TestAttributeFilter(t *testing.T) { if err != nil { return err } - _, err = mtr.RegisterCallback([]instrument.Asynchronous{ctr}, func(_ context.Context, o metric.MultiObserver) error { + _, err = mtr.RegisterCallback([]instrument.Asynchronous{ctr}, func(_ context.Context, o metric.Observer) error { o.ObserveFloat64(ctr, 1.0, attribute.String("foo", "bar"), attribute.Int("version", 1)) o.ObserveFloat64(ctr, 2.0, attribute.String("foo", "bar"), attribute.Int("version", 2)) return nil @@ -877,7 +877,7 @@ func TestAttributeFilter(t *testing.T) { if err != nil { return err } - _, err = mtr.RegisterCallback([]instrument.Asynchronous{ctr}, func(_ context.Context, o metric.MultiObserver) error { + _, err = mtr.RegisterCallback([]instrument.Asynchronous{ctr}, func(_ context.Context, o metric.Observer) error { o.ObserveInt64(ctr, 10, attribute.String("foo", "bar"), attribute.Int("version", 1)) o.ObserveInt64(ctr, 20, attribute.String("foo", "bar"), attribute.Int("version", 2)) return nil @@ -905,7 +905,7 @@ func TestAttributeFilter(t *testing.T) { if err != nil { return err } - _, err = mtr.RegisterCallback([]instrument.Asynchronous{ctr}, func(_ context.Context, o metric.MultiObserver) error { + _, err = mtr.RegisterCallback([]instrument.Asynchronous{ctr}, func(_ context.Context, o metric.Observer) error { o.ObserveInt64(ctr, 10, attribute.String("foo", "bar"), attribute.Int("version", 1)) o.ObserveInt64(ctr, 20, attribute.String("foo", "bar"), attribute.Int("version", 2)) return nil @@ -933,7 +933,7 @@ func TestAttributeFilter(t *testing.T) { if err != nil { return err } - _, err = mtr.RegisterCallback([]instrument.Asynchronous{ctr}, func(_ context.Context, o metric.MultiObserver) error { + _, err = mtr.RegisterCallback([]instrument.Asynchronous{ctr}, func(_ context.Context, o metric.Observer) error { o.ObserveInt64(ctr, 10, attribute.String("foo", "bar"), attribute.Int("version", 1)) o.ObserveInt64(ctr, 20, attribute.String("foo", "bar"), attribute.Int("version", 2)) return nil From 355064fd052727e9f83201e8a0b8aef82cfb2b3c Mon Sep 17 00:00:00 2001 From: Tyler Yahn Date: Thu, 12 Jan 2023 11:40:58 -0800 Subject: [PATCH 22/31] Fix method name in changelog --- CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index d01665b5847..63ed83bc409 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -101,7 +101,7 @@ This project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.htm This means it no longer uses the removed `net.peer.ip` or `http.host` attributes to determine the remote endpoint. Instead it uses the `net.sock.peer` attributes. (#3581) - The `Callback` in `go.opentelemetry.io/otel/metric` has the added `Observer` parameter added. - This new parameter is used by `Callback` implementations to observe values for asynchronous instruments instead of calling the `Observer` method of the instrument directly. (#3584) + This new parameter is used by `Callback` implementations to observe values for asynchronous instruments instead of calling the `Observe` method of the instrument directly. (#3584) ### Fixed From fa2a29e213f9a684082defa027e3c015419774f6 Mon Sep 17 00:00:00 2001 From: Tyler Yahn Date: Tue, 17 Jan 2023 08:21:34 -0800 Subject: [PATCH 23/31] Support registering delegating insts --- metric/internal/global/instruments.go | 46 +++++++++++++++++++++++---- sdk/metric/meter.go | 7 ++++ 2 files changed, 47 insertions(+), 6 deletions(-) diff --git a/metric/internal/global/instruments.go b/metric/internal/global/instruments.go index c4b3d1ff5ab..d4112b77711 100644 --- a/metric/internal/global/instruments.go +++ b/metric/internal/global/instruments.go @@ -24,6 +24,10 @@ import ( "go.opentelemetry.io/otel/metric/instrument" ) +type Unwrapper interface { + Unwrap() instrument.Asynchronous +} + type afCounter struct { name string opts []instrument.Float64ObserverOption @@ -33,6 +37,9 @@ type afCounter struct { instrument.Asynchronous } +var _ Unwrapper = (*afCounter)(nil) +var _ instrument.Float64ObservableCounter = (*afCounter)(nil) + func (i *afCounter) setDelegate(m metric.Meter) { ctr, err := m.Float64ObservableCounter(i.name, i.opts...) if err != nil { @@ -48,7 +55,7 @@ func (i *afCounter) Observe(ctx context.Context, x float64, attrs ...attribute.K } } -func (i *afCounter) unwrap() instrument.Asynchronous { +func (i *afCounter) Unwrap() instrument.Asynchronous { if ctr := i.delegate.Load(); ctr != nil { return ctr.(instrument.Float64ObservableCounter) } @@ -64,6 +71,9 @@ type afUpDownCounter struct { instrument.Asynchronous } +var _ Unwrapper = (*afUpDownCounter)(nil) +var _ instrument.Float64ObservableUpDownCounter = (*afUpDownCounter)(nil) + func (i *afUpDownCounter) setDelegate(m metric.Meter) { ctr, err := m.Float64ObservableUpDownCounter(i.name, i.opts...) if err != nil { @@ -79,7 +89,7 @@ func (i *afUpDownCounter) Observe(ctx context.Context, x float64, attrs ...attri } } -func (i *afUpDownCounter) unwrap() instrument.Asynchronous { +func (i *afUpDownCounter) Unwrap() instrument.Asynchronous { if ctr := i.delegate.Load(); ctr != nil { return ctr.(instrument.Float64ObservableUpDownCounter) } @@ -104,13 +114,16 @@ func (i *afGauge) setDelegate(m metric.Meter) { i.delegate.Store(ctr) } +var _ Unwrapper = (*afGauge)(nil) +var _ instrument.Float64ObservableGauge = (*afGauge)(nil) + func (i *afGauge) Observe(ctx context.Context, x float64, attrs ...attribute.KeyValue) { if ctr := i.delegate.Load(); ctr != nil { ctr.(instrument.Float64ObservableGauge).Observe(ctx, x, attrs...) } } -func (i *afGauge) unwrap() instrument.Asynchronous { +func (i *afGauge) Unwrap() instrument.Asynchronous { if ctr := i.delegate.Load(); ctr != nil { return ctr.(instrument.Float64ObservableGauge) } @@ -126,6 +139,9 @@ type aiCounter struct { instrument.Asynchronous } +var _ Unwrapper = (*aiCounter)(nil) +var _ instrument.Int64ObservableCounter = (*aiCounter)(nil) + func (i *aiCounter) setDelegate(m metric.Meter) { ctr, err := m.Int64ObservableCounter(i.name, i.opts...) if err != nil { @@ -141,7 +157,7 @@ func (i *aiCounter) Observe(ctx context.Context, x int64, attrs ...attribute.Key } } -func (i *aiCounter) unwrap() instrument.Asynchronous { +func (i *aiCounter) Unwrap() instrument.Asynchronous { if ctr := i.delegate.Load(); ctr != nil { return ctr.(instrument.Int64ObservableCounter) } @@ -157,6 +173,9 @@ type aiUpDownCounter struct { instrument.Asynchronous } +var _ Unwrapper = (*aiUpDownCounter)(nil) +var _ instrument.Int64ObservableUpDownCounter = (*aiUpDownCounter)(nil) + func (i *aiUpDownCounter) setDelegate(m metric.Meter) { ctr, err := m.Int64ObservableUpDownCounter(i.name, i.opts...) if err != nil { @@ -172,7 +191,7 @@ func (i *aiUpDownCounter) Observe(ctx context.Context, x int64, attrs ...attribu } } -func (i *aiUpDownCounter) unwrap() instrument.Asynchronous { +func (i *aiUpDownCounter) Unwrap() instrument.Asynchronous { if ctr := i.delegate.Load(); ctr != nil { return ctr.(instrument.Int64ObservableUpDownCounter) } @@ -188,6 +207,9 @@ type aiGauge struct { instrument.Asynchronous } +var _ Unwrapper = (*aiGauge)(nil) +var _ instrument.Int64ObservableGauge = (*aiGauge)(nil) + func (i *aiGauge) setDelegate(m metric.Meter) { ctr, err := m.Int64ObservableGauge(i.name, i.opts...) if err != nil { @@ -203,7 +225,7 @@ func (i *aiGauge) Observe(ctx context.Context, x int64, attrs ...attribute.KeyVa } } -func (i *aiGauge) unwrap() instrument.Asynchronous { +func (i *aiGauge) Unwrap() instrument.Asynchronous { if ctr := i.delegate.Load(); ctr != nil { return ctr.(instrument.Int64ObservableGauge) } @@ -220,6 +242,8 @@ type sfCounter struct { instrument.Synchronous } +var _ instrument.Float64Counter = (*sfCounter)(nil) + func (i *sfCounter) setDelegate(m metric.Meter) { ctr, err := m.Float64Counter(i.name, i.opts...) if err != nil { @@ -244,6 +268,8 @@ type sfUpDownCounter struct { instrument.Synchronous } +var _ instrument.Float64UpDownCounter = (*sfUpDownCounter)(nil) + func (i *sfUpDownCounter) setDelegate(m metric.Meter) { ctr, err := m.Float64UpDownCounter(i.name, i.opts...) if err != nil { @@ -268,6 +294,8 @@ type sfHistogram struct { instrument.Synchronous } +var _ instrument.Float64Histogram = (*sfHistogram)(nil) + func (i *sfHistogram) setDelegate(m metric.Meter) { ctr, err := m.Float64Histogram(i.name, i.opts...) if err != nil { @@ -292,6 +320,8 @@ type siCounter struct { instrument.Synchronous } +var _ instrument.Int64Counter = (*siCounter)(nil) + func (i *siCounter) setDelegate(m metric.Meter) { ctr, err := m.Int64Counter(i.name, i.opts...) if err != nil { @@ -316,6 +346,8 @@ type siUpDownCounter struct { instrument.Synchronous } +var _ instrument.Int64UpDownCounter = (*siUpDownCounter)(nil) + func (i *siUpDownCounter) setDelegate(m metric.Meter) { ctr, err := m.Int64UpDownCounter(i.name, i.opts...) if err != nil { @@ -340,6 +372,8 @@ type siHistogram struct { instrument.Synchronous } +var _ instrument.Int64Histogram = (*siHistogram)(nil) + func (i *siHistogram) setDelegate(m metric.Meter) { ctr, err := m.Int64Histogram(i.name, i.opts...) if err != nil { diff --git a/sdk/metric/meter.go b/sdk/metric/meter.go index 59392242456..704f8d313a0 100644 --- a/sdk/metric/meter.go +++ b/sdk/metric/meter.go @@ -216,6 +216,13 @@ func (m *meter) RegisterCallback(f metric.Callback, insts ...instrument.Asynchro reg := newObserver() var errs multierror for _, inst := range insts { + // Unwrap any global. + if u, ok := inst.(interface { + Unwrap() instrument.Asynchronous + }); ok { + inst = u.Unwrap() + } + switch o := inst.(type) { case *observable[int64]: if err := o.registerable(m.scope); err != nil { From 40bdf13ab9be141170999a6cfb7c5db520b83a67 Mon Sep 17 00:00:00 2001 From: Tyler Yahn Date: Tue, 17 Jan 2023 08:45:27 -0800 Subject: [PATCH 24/31] Add TestGlobalInstRegisterCallback Check global instruments inter-op with RegisterCallback. --- sdk/metric/meter_test.go | 36 ++++++++++++++++++++++++++++++++++++ 1 file changed, 36 insertions(+) diff --git a/sdk/metric/meter_test.go b/sdk/metric/meter_test.go index 2f3e586cc6a..8e378e5547e 100644 --- a/sdk/metric/meter_test.go +++ b/sdk/metric/meter_test.go @@ -24,6 +24,7 @@ import ( "go.opentelemetry.io/otel/attribute" "go.opentelemetry.io/otel/metric" + "go.opentelemetry.io/otel/metric/global" "go.opentelemetry.io/otel/metric/instrument" "go.opentelemetry.io/otel/sdk/instrumentation" "go.opentelemetry.io/otel/sdk/metric/aggregation" @@ -1143,6 +1144,41 @@ func TestAttributeFilter(t *testing.T) { } } +func TestGlobalInstRegisterCallback(t *testing.T) { + const mtrName = "TestGlobalInstRegisterCallback" + preMtr := global.Meter(mtrName) + preCtr, err := preMtr.Int64ObservableCounter("pre.counter") + require.NoError(t, err) + + rdr := NewManualReader() + mp := NewMeterProvider(WithReader(rdr)) + global.SetMeterProvider(mp) + + postMtr := global.Meter(mtrName) + postCtr, err := postMtr.Int64ObservableCounter("post.counter") + require.NoError(t, err) + + cb := func(_ context.Context, o metric.Observer) error { + o.ObserveInt64(preCtr, 2) + return nil + } + + _, err = preMtr.RegisterCallback(cb, preCtr) + assert.NoError(t, err) + + _, err = preMtr.RegisterCallback(cb, postCtr) + assert.NoError(t, err) + + _, err = postMtr.RegisterCallback(cb, preCtr) + assert.NoError(t, err) + + _, err = postMtr.RegisterCallback(cb, postCtr) + assert.NoError(t, err) + + _, err = rdr.Collect(context.Background()) + assert.NoError(t, err) +} + var ( aiCounter instrument.Int64ObservableCounter aiUpDownCounter instrument.Int64ObservableUpDownCounter From bf4c1561414c31a02846e8772f1e36af18584257 Mon Sep 17 00:00:00 2001 From: Tyler Yahn Date: Tue, 17 Jan 2023 08:48:38 -0800 Subject: [PATCH 25/31] Change Observe err msg --- sdk/metric/instrument.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/sdk/metric/instrument.go b/sdk/metric/instrument.go index bdcb822f18e..d8e9b45e488 100644 --- a/sdk/metric/instrument.go +++ b/sdk/metric/instrument.go @@ -241,8 +241,8 @@ var _ instrument.Int64ObservableGauge = (*observable[int64])(nil) // Observe logs an error. func (o *observable[N]) Observe(ctx context.Context, val N, attrs ...attribute.KeyValue) { var zero N - err := errors.New("invalid observation recording") - global.Error(err, "dropping measurement", + err := errors.New("invalid observation") + global.Error(err, "dropping observation made outside a callback", "name", o.name, "description", o.description, "unit", o.unit, From f798ed806325e4bffcd9c66562061af6fcda6f30 Mon Sep 17 00:00:00 2001 From: Tyler Yahn Date: Tue, 17 Jan 2023 08:55:13 -0800 Subject: [PATCH 26/31] Unexport Unwrapper from metric global --- metric/internal/global/instruments.go | 15 ++++++++------- 1 file changed, 8 insertions(+), 7 deletions(-) diff --git a/metric/internal/global/instruments.go b/metric/internal/global/instruments.go index d4112b77711..1398ada26be 100644 --- a/metric/internal/global/instruments.go +++ b/metric/internal/global/instruments.go @@ -24,7 +24,8 @@ import ( "go.opentelemetry.io/otel/metric/instrument" ) -type Unwrapper interface { +// unwrapper unwraps to return the underlying instrument implementation. +type unwrapper interface { Unwrap() instrument.Asynchronous } @@ -37,7 +38,7 @@ type afCounter struct { instrument.Asynchronous } -var _ Unwrapper = (*afCounter)(nil) +var _ unwrapper = (*afCounter)(nil) var _ instrument.Float64ObservableCounter = (*afCounter)(nil) func (i *afCounter) setDelegate(m metric.Meter) { @@ -71,7 +72,7 @@ type afUpDownCounter struct { instrument.Asynchronous } -var _ Unwrapper = (*afUpDownCounter)(nil) +var _ unwrapper = (*afUpDownCounter)(nil) var _ instrument.Float64ObservableUpDownCounter = (*afUpDownCounter)(nil) func (i *afUpDownCounter) setDelegate(m metric.Meter) { @@ -114,7 +115,7 @@ func (i *afGauge) setDelegate(m metric.Meter) { i.delegate.Store(ctr) } -var _ Unwrapper = (*afGauge)(nil) +var _ unwrapper = (*afGauge)(nil) var _ instrument.Float64ObservableGauge = (*afGauge)(nil) func (i *afGauge) Observe(ctx context.Context, x float64, attrs ...attribute.KeyValue) { @@ -139,7 +140,7 @@ type aiCounter struct { instrument.Asynchronous } -var _ Unwrapper = (*aiCounter)(nil) +var _ unwrapper = (*aiCounter)(nil) var _ instrument.Int64ObservableCounter = (*aiCounter)(nil) func (i *aiCounter) setDelegate(m metric.Meter) { @@ -173,7 +174,7 @@ type aiUpDownCounter struct { instrument.Asynchronous } -var _ Unwrapper = (*aiUpDownCounter)(nil) +var _ unwrapper = (*aiUpDownCounter)(nil) var _ instrument.Int64ObservableUpDownCounter = (*aiUpDownCounter)(nil) func (i *aiUpDownCounter) setDelegate(m metric.Meter) { @@ -207,7 +208,7 @@ type aiGauge struct { instrument.Asynchronous } -var _ Unwrapper = (*aiGauge)(nil) +var _ unwrapper = (*aiGauge)(nil) var _ instrument.Int64ObservableGauge = (*aiGauge)(nil) func (i *aiGauge) setDelegate(m metric.Meter) { From 5bf6707e94fee25d650754aee776d59a4ee3e5f0 Mon Sep 17 00:00:00 2001 From: Tyler Yahn Date: Tue, 17 Jan 2023 08:59:44 -0800 Subject: [PATCH 27/31] Clarify callbackObserver doc --- sdk/metric/meter.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/sdk/metric/meter.go b/sdk/metric/meter.go index 704f8d313a0..b301c96e2ff 100644 --- a/sdk/metric/meter.go +++ b/sdk/metric/meter.go @@ -406,8 +406,8 @@ func (p float64ObservProvider) callback(i *observable[float64], f instrument.Flo return func(ctx context.Context) error { return f(ctx, inst) } } -// callbackObserver is passed to a callback where a users is expected to call -// Observe directly to record a measurement. +// callbackObserver is an observer that records values for a wrapped +// observable. type callbackObserver[N int64 | float64] struct { *observable[N] } From 8142b9e8fadfbfd7eff556fb847263488c0d8c27 Mon Sep 17 00:00:00 2001 From: Tyler Yahn Date: Tue, 17 Jan 2023 09:19:59 -0800 Subject: [PATCH 28/31] Move TestGlobalInstRegisterCallback --- sdk/metric/meter_test.go | 70 ++++++++++++++++++++-------------------- 1 file changed, 35 insertions(+), 35 deletions(-) diff --git a/sdk/metric/meter_test.go b/sdk/metric/meter_test.go index 8e378e5547e..cb68ca3f8e6 100644 --- a/sdk/metric/meter_test.go +++ b/sdk/metric/meter_test.go @@ -611,6 +611,41 @@ func TestCallbackObserverNonRegistered(t *testing.T) { metricdatatest.AssertEqual(t, want, got, metricdatatest.IgnoreTimestamp()) } +func TestGlobalInstRegisterCallback(t *testing.T) { + const mtrName = "TestGlobalInstRegisterCallback" + preMtr := global.Meter(mtrName) + preCtr, err := preMtr.Int64ObservableCounter("pre.counter") + require.NoError(t, err) + + rdr := NewManualReader() + mp := NewMeterProvider(WithReader(rdr)) + global.SetMeterProvider(mp) + + postMtr := global.Meter(mtrName) + postCtr, err := postMtr.Int64ObservableCounter("post.counter") + require.NoError(t, err) + + cb := func(_ context.Context, o metric.Observer) error { + o.ObserveInt64(preCtr, 2) + return nil + } + + _, err = preMtr.RegisterCallback(cb, preCtr) + assert.NoError(t, err) + + _, err = preMtr.RegisterCallback(cb, postCtr) + assert.NoError(t, err) + + _, err = postMtr.RegisterCallback(cb, preCtr) + assert.NoError(t, err) + + _, err = postMtr.RegisterCallback(cb, postCtr) + assert.NoError(t, err) + + _, err = rdr.Collect(context.Background()) + assert.NoError(t, err) +} + func TestMetersProvideScope(t *testing.T) { rdr := NewManualReader() mp := NewMeterProvider(WithReader(rdr)) @@ -1144,41 +1179,6 @@ func TestAttributeFilter(t *testing.T) { } } -func TestGlobalInstRegisterCallback(t *testing.T) { - const mtrName = "TestGlobalInstRegisterCallback" - preMtr := global.Meter(mtrName) - preCtr, err := preMtr.Int64ObservableCounter("pre.counter") - require.NoError(t, err) - - rdr := NewManualReader() - mp := NewMeterProvider(WithReader(rdr)) - global.SetMeterProvider(mp) - - postMtr := global.Meter(mtrName) - postCtr, err := postMtr.Int64ObservableCounter("post.counter") - require.NoError(t, err) - - cb := func(_ context.Context, o metric.Observer) error { - o.ObserveInt64(preCtr, 2) - return nil - } - - _, err = preMtr.RegisterCallback(cb, preCtr) - assert.NoError(t, err) - - _, err = preMtr.RegisterCallback(cb, postCtr) - assert.NoError(t, err) - - _, err = postMtr.RegisterCallback(cb, preCtr) - assert.NoError(t, err) - - _, err = postMtr.RegisterCallback(cb, postCtr) - assert.NoError(t, err) - - _, err = rdr.Collect(context.Background()) - assert.NoError(t, err) -} - var ( aiCounter instrument.Int64ObservableCounter aiUpDownCounter instrument.Int64ObservableUpDownCounter From 27f50ecf3f39fd4bc761bdfafbc08fb5f671d211 Mon Sep 17 00:00:00 2001 From: Tyler Yahn Date: Wed, 18 Jan 2023 13:35:15 -0800 Subject: [PATCH 29/31] Fix observation of global inst --- sdk/metric/meter.go | 36 +++++++++++++++++++++--- sdk/metric/meter_test.go | 60 ++++++++++++++++++++++++++++++++-------- 2 files changed, 81 insertions(+), 15 deletions(-) diff --git a/sdk/metric/meter.go b/sdk/metric/meter.go index b301c96e2ff..f1107b2ef23 100644 --- a/sdk/metric/meter.go +++ b/sdk/metric/meter.go @@ -291,11 +291,25 @@ var ( ) func (r observer) ObserveFloat64(o instrument.Float64Observer, v float64, a ...attribute.KeyValue) { - oImpl, ok := o.(*observable[float64]) - if !ok { + var oImpl *observable[float64] + switch conv := o.(type) { + case *observable[float64]: + oImpl = conv + case interface { + Unwrap() instrument.Asynchronous + }: + // Unwrap any global. + async := conv.Unwrap() + var ok bool + if oImpl, ok = async.(*observable[float64]); !ok { + global.Error(errUnknownObserver, "failed to record asynchronous") + return + } + default: global.Error(errUnknownObserver, "failed to record") return } + if _, registered := r.float64[oImpl.observablID]; !registered { global.Error(errUnregObserver, "failed to record", "name", oImpl.name, @@ -309,11 +323,25 @@ func (r observer) ObserveFloat64(o instrument.Float64Observer, v float64, a ...a } func (r observer) ObserveInt64(o instrument.Int64Observer, v int64, a ...attribute.KeyValue) { - oImpl, ok := o.(*observable[int64]) - if !ok { + var oImpl *observable[int64] + switch conv := o.(type) { + case *observable[int64]: + oImpl = conv + case interface { + Unwrap() instrument.Asynchronous + }: + // Unwrap any global. + async := conv.Unwrap() + var ok bool + if oImpl, ok = async.(*observable[int64]); !ok { + global.Error(errUnknownObserver, "failed to record asynchronous") + return + } + default: global.Error(errUnknownObserver, "failed to record") return } + if _, registered := r.int64[oImpl.observablID]; !registered { global.Error(errUnregObserver, "failed to record", "name", oImpl.name, diff --git a/sdk/metric/meter_test.go b/sdk/metric/meter_test.go index cb68ca3f8e6..845bc04a5c0 100644 --- a/sdk/metric/meter_test.go +++ b/sdk/metric/meter_test.go @@ -16,12 +16,17 @@ package metric import ( "context" + "fmt" + "strings" "sync" "testing" + "github.com/go-logr/logr" + "github.com/go-logr/logr/testr" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" + "go.opentelemetry.io/otel" "go.opentelemetry.io/otel/attribute" "go.opentelemetry.io/otel/metric" "go.opentelemetry.io/otel/metric/global" @@ -611,10 +616,43 @@ func TestCallbackObserverNonRegistered(t *testing.T) { metricdatatest.AssertEqual(t, want, got, metricdatatest.IgnoreTimestamp()) } +type logSink struct { + logr.LogSink + + messages []string +} + +func newLogSink(t *testing.T) *logSink { + return &logSink{LogSink: testr.New(t).GetSink()} +} + +func (l *logSink) Info(level int, msg string, keysAndValues ...interface{}) { + l.messages = append(l.messages, msg) + l.LogSink.Info(level, msg, keysAndValues...) +} + +func (l *logSink) Error(err error, msg string, keysAndValues ...interface{}) { + l.messages = append(l.messages, fmt.Sprintf("%s: %s", err, msg)) + l.LogSink.Error(err, msg, keysAndValues...) +} + +func (l *logSink) String() string { + out := make([]string, len(l.messages)) + for i := range l.messages { + out[i] = "\t-" + l.messages[i] + } + return strings.Join(out, "\n") +} + func TestGlobalInstRegisterCallback(t *testing.T) { + l := newLogSink(t) + otel.SetLogger(logr.New(l)) + const mtrName = "TestGlobalInstRegisterCallback" preMtr := global.Meter(mtrName) - preCtr, err := preMtr.Int64ObservableCounter("pre.counter") + preInt64Ctr, err := preMtr.Int64ObservableCounter("pre.int64.counter") + require.NoError(t, err) + preFloat64Ctr, err := preMtr.Float64ObservableCounter("pre.float64.counter") require.NoError(t, err) rdr := NewManualReader() @@ -622,28 +660,28 @@ func TestGlobalInstRegisterCallback(t *testing.T) { global.SetMeterProvider(mp) postMtr := global.Meter(mtrName) - postCtr, err := postMtr.Int64ObservableCounter("post.counter") + postInt64Ctr, err := postMtr.Int64ObservableCounter("post.int64.counter") + require.NoError(t, err) + postFloat64Ctr, err := postMtr.Float64ObservableCounter("post.float64.counter") require.NoError(t, err) cb := func(_ context.Context, o metric.Observer) error { - o.ObserveInt64(preCtr, 2) + o.ObserveInt64(preInt64Ctr, 1) + o.ObserveFloat64(preFloat64Ctr, 2) + o.ObserveInt64(postInt64Ctr, 3) + o.ObserveFloat64(postFloat64Ctr, 4) return nil } - _, err = preMtr.RegisterCallback(cb, preCtr) - assert.NoError(t, err) - - _, err = preMtr.RegisterCallback(cb, postCtr) - assert.NoError(t, err) - - _, err = postMtr.RegisterCallback(cb, preCtr) + _, err = preMtr.RegisterCallback(cb, preInt64Ctr, preFloat64Ctr, postInt64Ctr, postFloat64Ctr) assert.NoError(t, err) - _, err = postMtr.RegisterCallback(cb, postCtr) + _, err = preMtr.RegisterCallback(cb, preInt64Ctr, preFloat64Ctr, postInt64Ctr, postFloat64Ctr) assert.NoError(t, err) _, err = rdr.Collect(context.Background()) assert.NoError(t, err) + assert.Lenf(t, l.messages, 0, "Warnings and errors logged:\n%s", l) } func TestMetersProvideScope(t *testing.T) { From e25195d0a4b3e41641213e0db1c264a922113105 Mon Sep 17 00:00:00 2001 From: Tyler Yahn Date: Wed, 18 Jan 2023 13:46:35 -0800 Subject: [PATCH 30/31] Update err msgs --- sdk/metric/instrument.go | 2 +- sdk/metric/meter.go | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/sdk/metric/instrument.go b/sdk/metric/instrument.go index d8e9b45e488..2b3c2356d3a 100644 --- a/sdk/metric/instrument.go +++ b/sdk/metric/instrument.go @@ -257,7 +257,7 @@ func (o *observable[N]) observe(val N, attrs []attribute.KeyValue) { } } -var errEmptyAgg = errors.New("no aggregators for observable") +var errEmptyAgg = errors.New("no aggregators for observable instrument") // registerable returns an error if the observable o should not be registered, // and nil if it should. An errEmptyAgg error is returned if o is effecively a diff --git a/sdk/metric/meter.go b/sdk/metric/meter.go index f1107b2ef23..7a4d6f9d377 100644 --- a/sdk/metric/meter.go +++ b/sdk/metric/meter.go @@ -286,8 +286,8 @@ func (r observer) registerInt64(id observablID[int64]) { } var ( - errUnknownObserver = errors.New("unknown observable") - errUnregObserver = errors.New("observable not registered for callback") + errUnknownObserver = errors.New("unknown observable instrument") + errUnregObserver = errors.New("observable instrument not registered for callback") ) func (r observer) ObserveFloat64(o instrument.Float64Observer, v float64, a ...attribute.KeyValue) { From eec9b941c40e604fb704f5b093c704f97c87715e Mon Sep 17 00:00:00 2001 From: Tyler Yahn Date: Wed, 18 Jan 2023 16:00:58 -0800 Subject: [PATCH 31/31] Assert collection in TestGlobalInstRegisterCallback --- sdk/metric/meter_test.go | 45 ++++++++++++++++++++++++++++++++++++++-- 1 file changed, 43 insertions(+), 2 deletions(-) diff --git a/sdk/metric/meter_test.go b/sdk/metric/meter_test.go index 845bc04a5c0..15c86d63156 100644 --- a/sdk/metric/meter_test.go +++ b/sdk/metric/meter_test.go @@ -656,7 +656,7 @@ func TestGlobalInstRegisterCallback(t *testing.T) { require.NoError(t, err) rdr := NewManualReader() - mp := NewMeterProvider(WithReader(rdr)) + mp := NewMeterProvider(WithReader(rdr), WithResource(resource.Empty())) global.SetMeterProvider(mp) postMtr := global.Meter(mtrName) @@ -679,9 +679,50 @@ func TestGlobalInstRegisterCallback(t *testing.T) { _, err = preMtr.RegisterCallback(cb, preInt64Ctr, preFloat64Ctr, postInt64Ctr, postFloat64Ctr) assert.NoError(t, err) - _, err = rdr.Collect(context.Background()) + got, err := rdr.Collect(context.Background()) assert.NoError(t, err) assert.Lenf(t, l.messages, 0, "Warnings and errors logged:\n%s", l) + metricdatatest.AssertEqual(t, metricdata.ResourceMetrics{ + ScopeMetrics: []metricdata.ScopeMetrics{ + { + Scope: instrumentation.Scope{Name: "TestGlobalInstRegisterCallback"}, + Metrics: []metricdata.Metrics{ + { + Name: "pre.int64.counter", + Data: metricdata.Sum[int64]{ + Temporality: metricdata.CumulativeTemporality, + IsMonotonic: true, + DataPoints: []metricdata.DataPoint[int64]{{Value: 1}}, + }, + }, + { + Name: "pre.float64.counter", + Data: metricdata.Sum[float64]{ + DataPoints: []metricdata.DataPoint[float64]{{Value: 2}}, + Temporality: metricdata.CumulativeTemporality, + IsMonotonic: true, + }, + }, + { + Name: "post.int64.counter", + Data: metricdata.Sum[int64]{ + Temporality: metricdata.CumulativeTemporality, + IsMonotonic: true, + DataPoints: []metricdata.DataPoint[int64]{{Value: 3}}, + }, + }, + { + Name: "post.float64.counter", + Data: metricdata.Sum[float64]{ + DataPoints: []metricdata.DataPoint[float64]{{Value: 4}}, + Temporality: metricdata.CumulativeTemporality, + IsMonotonic: true, + }, + }, + }, + }, + }, + }, got, metricdatatest.IgnoreTimestamp()) } func TestMetersProvideScope(t *testing.T) {