Skip to content

metrics: hardcode lazy metric construction codepath #31962

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

Open
wants to merge 4 commits into
base: master
Choose a base branch
from
Open
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
5 changes: 1 addition & 4 deletions metrics/counter.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,10 +7,7 @@ import (
// GetOrRegisterCounter returns an existing Counter or constructs and registers
// a new Counter.
func GetOrRegisterCounter(name string, r Registry) *Counter {
if r == nil {
r = DefaultRegistry
}
return r.GetOrRegister(name, NewCounter).(*Counter)
return getOrRegister(name, NewCounter, r)
}

// NewCounter constructs a new Counter.
Expand Down
5 changes: 1 addition & 4 deletions metrics/counter_float64.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,10 +8,7 @@ import (
// GetOrRegisterCounterFloat64 returns an existing *CounterFloat64 or constructs and registers
// a new CounterFloat64.
func GetOrRegisterCounterFloat64(name string, r Registry) *CounterFloat64 {
if nil == r {
r = DefaultRegistry
}
return r.GetOrRegister(name, NewCounterFloat64).(*CounterFloat64)
return getOrRegister(name, NewCounterFloat64, r)
}

// NewCounterFloat64 constructs a new CounterFloat64.
Expand Down
5 changes: 1 addition & 4 deletions metrics/gauge.go
Original file line number Diff line number Diff line change
Expand Up @@ -11,10 +11,7 @@ func (g GaugeSnapshot) Value() int64 { return int64(g) }
// GetOrRegisterGauge returns an existing Gauge or constructs and registers a
// new Gauge.
func GetOrRegisterGauge(name string, r Registry) *Gauge {
if r == nil {
r = DefaultRegistry
}
return r.GetOrRegister(name, NewGauge).(*Gauge)
return getOrRegister(name, NewGauge, r)
}

// NewGauge constructs a new Gauge.
Expand Down
5 changes: 1 addition & 4 deletions metrics/gauge_float64.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,10 +8,7 @@ import (
// GetOrRegisterGaugeFloat64 returns an existing GaugeFloat64 or constructs and registers a
// new GaugeFloat64.
func GetOrRegisterGaugeFloat64(name string, r Registry) *GaugeFloat64 {
if nil == r {
r = DefaultRegistry
}
return r.GetOrRegister(name, NewGaugeFloat64()).(*GaugeFloat64)
return getOrRegister(name, NewGaugeFloat64, r)
}

// GaugeFloat64Snapshot is a read-only copy of a GaugeFloat64.
Expand Down
5 changes: 1 addition & 4 deletions metrics/gauge_info.go
Original file line number Diff line number Diff line change
Expand Up @@ -16,10 +16,7 @@ func (val GaugeInfoValue) String() string {
// GetOrRegisterGaugeInfo returns an existing GaugeInfo or constructs and registers a
// new GaugeInfo.
func GetOrRegisterGaugeInfo(name string, r Registry) *GaugeInfo {
if nil == r {
r = DefaultRegistry
}
return r.GetOrRegister(name, NewGaugeInfo()).(*GaugeInfo)
return getOrRegister(name, NewGaugeInfo, r)
}

// NewGaugeInfo constructs a new GaugeInfo.
Expand Down
10 changes: 2 additions & 8 deletions metrics/histogram.go
Original file line number Diff line number Diff line change
Expand Up @@ -23,19 +23,13 @@ type Histogram interface {
// GetOrRegisterHistogram returns an existing Histogram or constructs and
// registers a new StandardHistogram.
func GetOrRegisterHistogram(name string, r Registry, s Sample) Histogram {
if nil == r {
r = DefaultRegistry
}
return r.GetOrRegister(name, func() Histogram { return NewHistogram(s) }).(Histogram)
return getOrRegister(name, func() Histogram { return NewHistogram(s) }, r)
}

// GetOrRegisterHistogramLazy returns an existing Histogram or constructs and
// registers a new StandardHistogram.
func GetOrRegisterHistogramLazy(name string, r Registry, s func() Sample) Histogram {
if nil == r {
r = DefaultRegistry
}
return r.GetOrRegister(name, func() Histogram { return NewHistogram(s()) }).(Histogram)
return getOrRegister(name, func() Histogram { return NewHistogram(s()) }, r)
}

// NewHistogram constructs a new StandardHistogram from a Sample.
Expand Down
5 changes: 1 addition & 4 deletions metrics/meter.go
Original file line number Diff line number Diff line change
Expand Up @@ -12,10 +12,7 @@ import (
// Be sure to unregister the meter from the registry once it is of no use to
// allow for garbage collection.
func GetOrRegisterMeter(name string, r Registry) *Meter {
if r == nil {
r = DefaultRegistry
}
return r.GetOrRegister(name, NewMeter).(*Meter)
return getOrRegister(name, NewMeter, r)
}

// NewMeter constructs a new Meter and launches a goroutine.
Expand Down
34 changes: 15 additions & 19 deletions metrics/registry.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@ package metrics
import (
"errors"
"fmt"
"reflect"
"sort"
"strings"
"sync"
Expand All @@ -30,10 +29,9 @@ type Registry interface {
// GetAll metrics in the Registry.
GetAll() map[string]map[string]interface{}

// GetOrRegister gets an existing metric or registers the given one.
// The interface can be the metric to register if not found in registry,
// or a function returning the metric for lazy instantiation.
GetOrRegister(string, interface{}) interface{}
// GetOrRegister returns an existing metric or registers the one returned
// by the given constructor.
GetOrRegister(string, func() interface{}) interface{}

// Register the given metric under the given name.
Register(string, interface{}) error
Expand Down Expand Up @@ -95,19 +93,13 @@ func (r *StandardRegistry) Get(name string) interface{} {
// alternative to calling Get and Register on failure.
// The interface can be the metric to register if not found in registry,
// or a function returning the metric for lazy instantiation.
func (r *StandardRegistry) GetOrRegister(name string, i interface{}) interface{} {
func (r *StandardRegistry) GetOrRegister(name string, ctor func() interface{}) interface{} {
// fast path
cached, ok := r.metrics.Load(name)
if ok {
return cached
}
if v := reflect.ValueOf(i); v.Kind() == reflect.Func {
i = v.Call(nil)[0].Interface()
}
item, _, ok := r.loadOrRegister(name, i)
if !ok {
return i
}
item, _, _ := r.loadOrRegister(name, ctor())
return item
}

Expand All @@ -120,9 +112,6 @@ func (r *StandardRegistry) Register(name string, i interface{}) error {
return fmt.Errorf("%w: %v", ErrDuplicateMetric, name)
}

if v := reflect.ValueOf(i); v.Kind() == reflect.Func {
i = v.Call(nil)[0].Interface()
}
_, loaded, _ := r.loadOrRegister(name, i)
if loaded {
return fmt.Errorf("%w: %v", ErrDuplicateMetric, name)
Expand Down Expand Up @@ -295,9 +284,9 @@ func (r *PrefixedRegistry) Get(name string) interface{} {
// GetOrRegister gets an existing metric or registers the given one.
// The interface can be the metric to register if not found in registry,
// or a function returning the metric for lazy instantiation.
func (r *PrefixedRegistry) GetOrRegister(name string, metric interface{}) interface{} {
func (r *PrefixedRegistry) GetOrRegister(name string, ctor func() interface{}) interface{} {
realName := r.prefix + name
return r.underlying.GetOrRegister(realName, metric)
return r.underlying.GetOrRegister(realName, ctor)
}

// Register the given metric under the given name. The name will be prefixed.
Expand Down Expand Up @@ -338,10 +327,17 @@ func Get(name string) interface{} {

// GetOrRegister gets an existing metric or creates and registers a new one. Threadsafe
// alternative to calling Get and Register on failure.
func GetOrRegister(name string, i interface{}) interface{} {
func GetOrRegister(name string, i func() interface{}) interface{} {
return DefaultRegistry.GetOrRegister(name, i)
}

func getOrRegister[T any](name string, ctor func() T, r Registry) T {
if r == nil {
r = DefaultRegistry
}
return r.GetOrRegister(name, func() any { return ctor() }).(T)
}

// Register the given metric under the given name. Returns a ErrDuplicateMetric
// if a metric by the given name is already registered.
func Register(name string, i interface{}) error {
Expand Down
22 changes: 11 additions & 11 deletions metrics/registry_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ func benchmarkRegistryGetOrRegisterParallel(b *testing.B, amount int) {
wg.Add(1)
go func() {
for i := 0; i < b.N; i++ {
r.GetOrRegister("foo", NewMeter)
GetOrRegisterMeter("foo", r)
}
wg.Done()
}()
Expand Down Expand Up @@ -98,10 +98,10 @@ func TestRegistryGetOrRegister(t *testing.T) {
r := NewRegistry()

// First metric wins with GetOrRegister
_ = r.GetOrRegister("foo", NewCounter())
m := r.GetOrRegister("foo", NewGauge())
if _, ok := m.(*Counter); !ok {
t.Fatal(m)
c1 := GetOrRegisterCounter("foo", r)
c2 := GetOrRegisterCounter("foo", r)
if c1 != c2 {
t.Fatal("counters should've matched")
}

i := 0
Expand All @@ -123,10 +123,10 @@ func TestRegistryGetOrRegisterWithLazyInstantiation(t *testing.T) {
r := NewRegistry()

// First metric wins with GetOrRegister
_ = r.GetOrRegister("foo", NewCounter)
m := r.GetOrRegister("foo", NewGauge)
if _, ok := m.(*Counter); !ok {
t.Fatal(m)
c1 := GetOrRegisterCounter("foo", r)
c2 := GetOrRegisterCounter("foo", r)
if c1 != c2 {
t.Fatal("counters should've matched")
}

i := 0
Expand Down Expand Up @@ -165,7 +165,7 @@ func TestPrefixedChildRegistryGetOrRegister(t *testing.T) {
r := NewRegistry()
pr := NewPrefixedChildRegistry(r, "prefix.")

_ = pr.GetOrRegister("foo", NewCounter())
_ = GetOrRegisterCounter("foo", pr)

i := 0
r.Each(func(name string, m interface{}) {
Expand All @@ -182,7 +182,7 @@ func TestPrefixedChildRegistryGetOrRegister(t *testing.T) {
func TestPrefixedRegistryGetOrRegister(t *testing.T) {
r := NewPrefixedRegistry("prefix.")

_ = r.GetOrRegister("foo", NewCounter())
_ = GetOrRegisterCounter("foo", r)

i := 0
r.Each(func(name string, m interface{}) {
Expand Down
5 changes: 1 addition & 4 deletions metrics/resetting_timer.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,10 +8,7 @@ import (
// GetOrRegisterResettingTimer returns an existing ResettingTimer or constructs and registers a
// new ResettingTimer.
func GetOrRegisterResettingTimer(name string, r Registry) *ResettingTimer {
if nil == r {
r = DefaultRegistry
}
return r.GetOrRegister(name, NewResettingTimer).(*ResettingTimer)
return getOrRegister(name, NewResettingTimer, r)
}

// NewRegisteredResettingTimer constructs and registers a new ResettingTimer.
Expand Down
5 changes: 1 addition & 4 deletions metrics/runtimehistogram.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,11 +8,8 @@ import (
)

func getOrRegisterRuntimeHistogram(name string, scale float64, r Registry) *runtimeHistogram {
if r == nil {
r = DefaultRegistry
}
constructor := func() Histogram { return newRuntimeHistogram(scale) }
return r.GetOrRegister(name, constructor).(*runtimeHistogram)
return getOrRegister(name, constructor, r).(*runtimeHistogram)
}

// runtimeHistogram wraps a runtime/metrics histogram.
Expand Down
5 changes: 1 addition & 4 deletions metrics/timer.go
Original file line number Diff line number Diff line change
Expand Up @@ -10,10 +10,7 @@ import (
// Be sure to unregister the meter from the registry once it is of no use to
// allow for garbage collection.
func GetOrRegisterTimer(name string, r Registry) *Timer {
if nil == r {
r = DefaultRegistry
}
return r.GetOrRegister(name, NewTimer).(*Timer)
return getOrRegister(name, NewTimer, r)
}

// NewCustomTimer constructs a new Timer from a Histogram and a Meter.
Expand Down