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

Replace meterRegistry with cache #3255

Merged
merged 3 commits into from
Oct 12, 2022
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.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
44 changes: 0 additions & 44 deletions sdk/metric/meter.go
Expand Up @@ -16,7 +16,6 @@ package metric // import "go.opentelemetry.io/otel/sdk/metric"

import (
"context"
"sync"

"go.opentelemetry.io/otel/metric"
"go.opentelemetry.io/otel/metric/instrument"
Expand All @@ -27,49 +26,6 @@ import (
"go.opentelemetry.io/otel/sdk/instrumentation"
)

// meterRegistry keeps a record of initialized meters for instrumentation
// scopes. A meter is unique to an instrumentation scope and if multiple
// requests for that meter are made a meterRegistry ensure the same instance
// is used.
//
// The zero meterRegistry is empty and ready for use.
//
// A meterRegistry must not be copied after first use.
//
// All methods of a meterRegistry are safe to call concurrently.
type meterRegistry struct {
sync.Mutex

meters map[instrumentation.Scope]*meter

pipes pipelines
}

// Get returns a registered meter matching the instrumentation scope if it
// exists in the meterRegistry. Otherwise, a new meter configured for the
// instrumentation scope is registered and then returned.
//
// Get is safe to call concurrently.
func (r *meterRegistry) Get(s instrumentation.Scope) *meter {
r.Lock()
defer r.Unlock()

if r.meters == nil {
m := newMeter(s, r.pipes)
r.meters = map[instrumentation.Scope]*meter{s: m}
return m
}

m, ok := r.meters[s]
if ok {
return m
}

m = newMeter(s, r.pipes)
r.meters[s] = m
return m
}

// meter handles the creation and coordination of all metric instruments. A
// meter represents a single instrumentation scope; all metric telemetry
// produced by an instrumentation scope will use metric instruments from a
Expand Down
22 changes: 0 additions & 22 deletions sdk/metric/meter_test.go
Expand Up @@ -31,28 +31,6 @@ import (
"go.opentelemetry.io/otel/sdk/resource"
)

func TestMeterRegistry(t *testing.T) {
is0 := instrumentation.Scope{Name: "zero"}
is1 := instrumentation.Scope{Name: "one"}

r := meterRegistry{}
var m0 *meter
t.Run("ZeroValueGetDoesNotPanic", func(t *testing.T) {
assert.NotPanics(t, func() { m0 = r.Get(is0) })
assert.Equal(t, is0, m0.Scope, "uninitialized meter returned")
})

m01 := r.Get(is0)
t.Run("GetSameMeter", func(t *testing.T) {
assert.Samef(t, m0, m01, "returned different meters: %v", is0)
})

m1 := r.Get(is1)
t.Run("GetDifferentMeter", func(t *testing.T) {
assert.NotSamef(t, m0, m1, "returned same meters: %v", is1)
})
}

// A meter should be able to make instruments concurrently.
func TestMeterInstrumentConcurrency(t *testing.T) {
wg := &sync.WaitGroup{}
Expand Down
17 changes: 7 additions & 10 deletions sdk/metric/provider.go
Expand Up @@ -26,7 +26,8 @@ import (
// the same Views applied to them, and have their produced metric telemetry
// passed to the configured Readers.
type MeterProvider struct {
meters meterRegistry
pipes pipelines
meters cache[instrumentation.Scope, *meter]

forceFlush, shutdown func(context.Context) error
}
Expand All @@ -42,16 +43,9 @@ var _ metric.MeterProvider = (*MeterProvider)(nil)
// Readers, will perform no operations.
func NewMeterProvider(options ...Option) *MeterProvider {
conf := newConfig(options)

flush, sdown := conf.readerSignals()

registry := newPipelines(conf.res, conf.readers)

return &MeterProvider{
meters: meterRegistry{
pipes: registry,
},

pipes: newPipelines(conf.res, conf.readers),
forceFlush: flush,
shutdown: sdown,
}
Expand All @@ -72,10 +66,13 @@ func NewMeterProvider(options ...Option) *MeterProvider {
// This method is safe to call concurrently.
func (mp *MeterProvider) Meter(name string, options ...metric.MeterOption) metric.Meter {
c := metric.NewMeterConfig(options...)
return mp.meters.Get(instrumentation.Scope{
s := instrumentation.Scope{
Name: name,
Version: c.InstrumentationVersion(),
SchemaURL: c.SchemaURL(),
}
return mp.meters.Lookup(s, func() *meter {
return newMeter(s, mp.pipes)
})
}

Expand Down