From 04845621bc9da4ba6c2c30a9a2c3c3328e786d79 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=96mer=20Faruk=20IRMAK?= Date: Wed, 4 Jun 2025 13:02:03 +0300 Subject: [PATCH] metrics: hardcode lazy metric construction codepath motivation is to remove the dependency on reflect.Call Throughout the codebase, GetOrRegister() is mostly used with lazy construction while Register() is strictly used with eagerly constructed metric objects. So it makes sense to isolate the lazy construction to GetOrRegister() and make caller decide on which function makes sense for them. --- metrics/counter.go | 5 +---- metrics/counter_float64.go | 5 +---- metrics/gauge.go | 5 +---- metrics/gauge_float64.go | 5 +---- metrics/gauge_info.go | 5 +---- metrics/histogram.go | 10 ++-------- metrics/meter.go | 5 +---- metrics/registry.go | 32 ++++++++++++-------------------- metrics/registry_test.go | 14 +++++++------- metrics/resetting_timer.go | 5 +---- metrics/runtimehistogram.go | 5 +---- metrics/timer.go | 5 +---- 12 files changed, 30 insertions(+), 71 deletions(-) diff --git a/metrics/counter.go b/metrics/counter.go index 0f373b0d92..2531496c68 100644 --- a/metrics/counter.go +++ b/metrics/counter.go @@ -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).(*Counter) } // NewCounter constructs a new Counter. diff --git a/metrics/counter_float64.go b/metrics/counter_float64.go index 91c4215c4d..82ec01f7cb 100644 --- a/metrics/counter_float64.go +++ b/metrics/counter_float64.go @@ -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).(*CounterFloat64) } // NewCounterFloat64 constructs a new CounterFloat64. diff --git a/metrics/gauge.go b/metrics/gauge.go index ba7843e03b..82190d1a61 100644 --- a/metrics/gauge.go +++ b/metrics/gauge.go @@ -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).(*Gauge) } // NewGauge constructs a new Gauge. diff --git a/metrics/gauge_float64.go b/metrics/gauge_float64.go index 05b401ef9c..a27d35dcb4 100644 --- a/metrics/gauge_float64.go +++ b/metrics/gauge_float64.go @@ -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).(*GaugeFloat64) } // GaugeFloat64Snapshot is a read-only copy of a GaugeFloat64. diff --git a/metrics/gauge_info.go b/metrics/gauge_info.go index 1862ed55c5..861f37df56 100644 --- a/metrics/gauge_info.go +++ b/metrics/gauge_info.go @@ -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).(*GaugeInfo) } // NewGaugeInfo constructs a new GaugeInfo. diff --git a/metrics/histogram.go b/metrics/histogram.go index 7c27bcc928..b222730d0c 100644 --- a/metrics/histogram.go +++ b/metrics/histogram.go @@ -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).(Histogram) } // 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).(Histogram) } // NewHistogram constructs a new StandardHistogram from a Sample. diff --git a/metrics/meter.go b/metrics/meter.go index 197e5abbed..585134d49e 100644 --- a/metrics/meter.go +++ b/metrics/meter.go @@ -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).(*Meter) } // NewMeter constructs a new Meter and launches a goroutine. diff --git a/metrics/registry.go b/metrics/registry.go index 527da6238d..b562144554 100644 --- a/metrics/registry.go +++ b/metrics/registry.go @@ -3,7 +3,6 @@ package metrics import ( "errors" "fmt" - "reflect" "sort" "strings" "sync" @@ -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 @@ -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 } @@ -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) @@ -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. @@ -338,8 +327,11 @@ 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{} { - return DefaultRegistry.GetOrRegister(name, i) +func GetOrRegister[T any](name string, ctor func() T, r Registry) interface{} { + if nil == r { + r = DefaultRegistry + } + return r.GetOrRegister(name, func() any { return ctor() }) } // Register the given metric under the given name. Returns a ErrDuplicateMetric diff --git a/metrics/registry_test.go b/metrics/registry_test.go index bdc58fee6c..2572f92a0a 100644 --- a/metrics/registry_test.go +++ b/metrics/registry_test.go @@ -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) + GetOrRegister("foo", NewMeter, r) } wg.Done() }() @@ -98,8 +98,8 @@ func TestRegistryGetOrRegister(t *testing.T) { r := NewRegistry() // First metric wins with GetOrRegister - _ = r.GetOrRegister("foo", NewCounter()) - m := r.GetOrRegister("foo", NewGauge()) + _ = GetOrRegister("foo", NewCounter, r) + m := GetOrRegister("foo", NewGauge, r) if _, ok := m.(*Counter); !ok { t.Fatal(m) } @@ -123,8 +123,8 @@ func TestRegistryGetOrRegisterWithLazyInstantiation(t *testing.T) { r := NewRegistry() // First metric wins with GetOrRegister - _ = r.GetOrRegister("foo", NewCounter) - m := r.GetOrRegister("foo", NewGauge) + _ = GetOrRegister("foo", NewCounter, r) + m := GetOrRegister("foo", NewGauge, r) if _, ok := m.(*Counter); !ok { t.Fatal(m) } @@ -165,7 +165,7 @@ func TestPrefixedChildRegistryGetOrRegister(t *testing.T) { r := NewRegistry() pr := NewPrefixedChildRegistry(r, "prefix.") - _ = pr.GetOrRegister("foo", NewCounter()) + _ = GetOrRegister("foo", NewCounter, pr) i := 0 r.Each(func(name string, m interface{}) { @@ -182,7 +182,7 @@ func TestPrefixedChildRegistryGetOrRegister(t *testing.T) { func TestPrefixedRegistryGetOrRegister(t *testing.T) { r := NewPrefixedRegistry("prefix.") - _ = r.GetOrRegister("foo", NewCounter()) + _ = GetOrRegister("foo", NewCounter, r) i := 0 r.Each(func(name string, m interface{}) { diff --git a/metrics/resetting_timer.go b/metrics/resetting_timer.go index 66458bdb91..35923aa841 100644 --- a/metrics/resetting_timer.go +++ b/metrics/resetting_timer.go @@ -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).(*ResettingTimer) } // NewRegisteredResettingTimer constructs and registers a new ResettingTimer. diff --git a/metrics/runtimehistogram.go b/metrics/runtimehistogram.go index 92fcbcc281..b688fed44f 100644 --- a/metrics/runtimehistogram.go +++ b/metrics/runtimehistogram.go @@ -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. diff --git a/metrics/timer.go b/metrics/timer.go index 9df15c967a..4c0e723962 100644 --- a/metrics/timer.go +++ b/metrics/timer.go @@ -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).(*Timer) } // NewCustomTimer constructs a new Timer from a Histogram and a Meter.