Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Restructure RegisterCallback method #3587

Merged
merged 5 commits into from
Jan 13, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -100,6 +100,8 @@ 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. (#3587)

### Deprecated

Expand Down
4 changes: 2 additions & 2 deletions example/prometheus/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -68,11 +68,11 @@ func main() {
if err != nil {
log.Fatal(err)
}
_, err = meter.RegisterCallback([]instrument.Asynchronous{gauge}, func(ctx context.Context) error {
_, err = meter.RegisterCallback(func(ctx context.Context) error {
n := -10. + rand.Float64()*(90.) // [-10, 100)
gauge.Observe(ctx, n, attrs...)
return nil
})
}, gauge)
if err != nil {
log.Fatal(err)
}
Expand Down
7 changes: 3 additions & 4 deletions metric/example_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -89,10 +89,7 @@ func ExampleMeter_asynchronous_multiple() {
gcCount, _ := meter.Int64ObservableCounter("gcCount")
gcPause, _ := meter.Float64Histogram("gcPause")

_, err := meter.RegisterCallback([]instrument.Asynchronous{
heapAlloc,
gcCount,
},
_, err := meter.RegisterCallback(
func(ctx context.Context) error {
memStats := &runtime.MemStats{}
// This call does work
Expand All @@ -105,6 +102,8 @@ func ExampleMeter_asynchronous_multiple() {
computeGCPauses(ctx, gcPause, memStats.PauseNs[:])
return nil
},
heapAlloc,
gcCount,
)

if err != nil {
Expand Down
6 changes: 3 additions & 3 deletions metric/internal/global/meter.go
Original file line number Diff line number Diff line change
Expand Up @@ -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()
Expand Down Expand Up @@ -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)
}
Expand Down
16 changes: 8 additions & 8 deletions metric/internal/global/meter_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,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(func(ctx context.Context) error { return nil })
if !once {
wg.Done()
once = true
Expand All @@ -86,7 +86,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(func(ctx context.Context) error { return nil })
require.NoError(t, err)

wg := &sync.WaitGroup{}
Expand Down Expand Up @@ -128,10 +128,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 {
_, err = m.RegisterCallback(func(ctx context.Context) error {
afcounter.Observe(ctx, 3)
return nil
})
}, afcounter)
require.NoError(t, err)

sfcounter, err := m.Float64Counter("test_Async_Counter")
Expand Down Expand Up @@ -323,21 +323,21 @@ 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) 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.
assert.NoError(t, reg0.Unregister())
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) error {
called1 = true
return nil
})
}, actr)
require.NoError(t, err)
require.Equal(t, 1, mImpl.registry.Len(), "second callback not registered")

Expand Down
2 changes: 1 addition & 1 deletion metric/internal/global/meter_types_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -115,7 +115,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() {
Expand Down
2 changes: 1 addition & 1 deletion metric/meter.go
Original file line number Diff line number Diff line change
Expand Up @@ -102,7 +102,7 @@ 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
Expand Down
2 changes: 1 addition & 1 deletion metric/noop.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}

Expand Down
2 changes: 1 addition & 1 deletion sdk/metric/meter.go
Original file line number Diff line number Diff line change
Expand Up @@ -200,7 +200,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(insts []instrument.Asynchronous, f metric.Callback) (metric.Registration, error) {
func (m *meter) RegisterCallback(f metric.Callback, insts ...instrument.Asynchronous) (metric.Registration, error) {
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.
Expand Down
Loading