From 769aad4126f6591b7746a5f534a1d7cb987dacbf Mon Sep 17 00:00:00 2001 From: Martin Holst Swende Date: Wed, 30 Aug 2023 14:22:44 +0200 Subject: [PATCH 01/26] metrics: add benchmarks --- metrics/ewma_test.go | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/metrics/ewma_test.go b/metrics/ewma_test.go index 5b244191616e..73e77f852f76 100644 --- a/metrics/ewma_test.go +++ b/metrics/ewma_test.go @@ -14,6 +14,18 @@ func BenchmarkEWMA(b *testing.B) { } } +func BenchmarkEWMAParallel(b *testing.B) { + a := NewEWMA1() + b.ResetTimer() + + b.RunParallel(func(pb *testing.PB) { + for pb.Next() { + a.Update(1) + a.Tick() + } + }) +} + func TestEWMA1(t *testing.T) { a := NewEWMA1() a.Update(3) From 1fc2e832a1c13ecf9a9f98119cd54793ef72247f Mon Sep 17 00:00:00 2001 From: Martin Holst Swende Date: Wed, 30 Aug 2023 14:23:31 +0200 Subject: [PATCH 02/26] metrics: reduce lock contention in ewma ref: https://github.com/rcrowley/go-metrics/commit/b08b7421c5a1684c85903d3a185332b51dcf064e --- metrics/ewma.go | 43 ++++++++++++++++++++++++++++++------------- 1 file changed, 30 insertions(+), 13 deletions(-) diff --git a/metrics/ewma.go b/metrics/ewma.go index ed95cba19b4f..ffe4fe83a54d 100644 --- a/metrics/ewma.go +++ b/metrics/ewma.go @@ -77,16 +77,14 @@ func (NilEWMA) Update(n int64) {} type StandardEWMA struct { uncounted atomic.Int64 alpha float64 - rate float64 - init bool + rate atomic.Uint64 + init atomic.Bool mutex sync.Mutex } // Rate returns the moving average rate of events per second. func (a *StandardEWMA) Rate() float64 { - a.mutex.Lock() - defer a.mutex.Unlock() - return a.rate * float64(time.Second) + return math.Float64frombits(a.rate.Load()) * float64(time.Second) } // Snapshot returns a read-only copy of the EWMA. @@ -97,17 +95,36 @@ func (a *StandardEWMA) Snapshot() EWMA { // Tick ticks the clock to update the moving average. It assumes it is called // every five seconds. func (a *StandardEWMA) Tick() { - count := a.uncounted.Load() - a.uncounted.Add(-count) - instantRate := float64(count) / float64(5*time.Second) + // Optimization to avoid mutex locking in the hot-path. + if a.init.Load() { + a.updateRate(a.fetchInstantRate()) + return + } + // Slow-path: this is only needed on the first Tick() and preserves transactional updating + // of init and rate in the else block. The first conditional is needed below because + // a different thread could have set a.init = 1 between the time of the first atomic load and when + // the lock was acquired. a.mutex.Lock() - defer a.mutex.Unlock() - if a.init { - a.rate += a.alpha * (instantRate - a.rate) + if a.init.Load() { + // The fetchInstantRate() uses atomic loading, which is unnecessary in this critical section + // but again, this section is only invoked on the first successful Tick() operation. + a.updateRate(a.fetchInstantRate()) } else { - a.init = true - a.rate = instantRate + a.init.Store(true) + a.rate.Store(math.Float64bits(a.fetchInstantRate())) } + a.mutex.Unlock() +} + +func (a *StandardEWMA) fetchInstantRate() float64 { + count := a.uncounted.Swap(0) + return float64(count) / float64(5*time.Second) +} + +func (a *StandardEWMA) updateRate(instantRate float64) { + currentRate := math.Float64frombits(a.rate.Load()) + currentRate += a.alpha * (instantRate - currentRate) + a.rate.Store(math.Float64bits(currentRate)) } // Update adds n uncounted events. From 16d39ddfcdbcefaab420bd685440c51dc2b84748 Mon Sep 17 00:00:00 2001 From: Martin Holst Swende Date: Wed, 30 Aug 2023 15:01:43 +0200 Subject: [PATCH 03/26] metrics: update meter.go ref https://github.com/rcrowley/go-metrics/commit/90735333ef07a37936f09df83fb886a43bd83cc6 Remove locks from the hot-path in meter.go and add parallel benchmark metrics: simplify meter.go metrics: refactor meter --- metrics/meter.go | 147 ++++++++++++----------------------------------- metrics/timer.go | 4 +- 2 files changed, 39 insertions(+), 112 deletions(-) diff --git a/metrics/meter.go b/metrics/meter.go index 8a89dc4275f1..25e08d3629ce 100644 --- a/metrics/meter.go +++ b/metrics/meter.go @@ -1,6 +1,7 @@ package metrics import ( + "math" "sync" "sync/atomic" "time" @@ -30,17 +31,6 @@ func GetOrRegisterMeter(name string, r Registry) Meter { return r.GetOrRegister(name, NewMeter).(Meter) } -// GetOrRegisterMeterForced returns an existing Meter or constructs and registers a -// new StandardMeter no matter the global switch is enabled or not. -// Be sure to unregister the meter from the registry once it is of no use to -// allow for garbage collection. -func GetOrRegisterMeterForced(name string, r Registry) Meter { - if nil == r { - r = DefaultRegistry - } - return r.GetOrRegister(name, NewMeterForced).(Meter) -} - // NewMeter constructs a new StandardMeter and launches a goroutine. // Be sure to call Stop() once the meter is of no use to allow for garbage collection. func NewMeter() Meter { @@ -68,83 +58,49 @@ func NewInactiveMeter() Meter { return m } -// NewMeterForced constructs a new StandardMeter and launches a goroutine no matter -// the global switch is enabled or not. -// Be sure to call Stop() once the meter is of no use to allow for garbage collection. -func NewMeterForced() Meter { - m := newStandardMeter() - arbiter.Lock() - defer arbiter.Unlock() - arbiter.meters[m] = struct{}{} - if !arbiter.started { - arbiter.started = true - go arbiter.tick() - } - return m -} - // NewRegisteredMeter constructs and registers a new StandardMeter // and launches a goroutine. // Be sure to unregister the meter from the registry once it is of no use to // allow for garbage collection. func NewRegisteredMeter(name string, r Registry) Meter { - c := NewMeter() - if nil == r { - r = DefaultRegistry - } - r.Register(name, c) - return c -} - -// NewRegisteredMeterForced constructs and registers a new StandardMeter -// and launches a goroutine no matter the global switch is enabled or not. -// Be sure to unregister the meter from the registry once it is of no use to -// allow for garbage collection. -func NewRegisteredMeterForced(name string, r Registry) Meter { - c := NewMeterForced() - if nil == r { - r = DefaultRegistry - } - r.Register(name, c) - return c + return GetOrRegisterMeter(name, r) } -// MeterSnapshot is a read-only copy of another Meter. -type MeterSnapshot struct { - temp atomic.Int64 +// meterSnapshot is a read-only copy of the meter's internal values. +type meterSnapshot struct { count int64 rate1, rate5, rate15, rateMean float64 } // Count returns the count of events at the time the snapshot was taken. -func (m *MeterSnapshot) Count() int64 { return m.count } +func (m *meterSnapshot) Count() int64 { return m.count } // Mark panics. -func (*MeterSnapshot) Mark(n int64) { - panic("Mark called on a MeterSnapshot") +func (*meterSnapshot) Mark(n int64) { + panic("Mark called on a meterSnapshot") } // Rate1 returns the one-minute moving average rate of events per second at the // time the snapshot was taken. -func (m *MeterSnapshot) Rate1() float64 { return m.rate1 } +func (m *meterSnapshot) Rate1() float64 { return m.rate1 } // Rate5 returns the five-minute moving average rate of events per second at // the time the snapshot was taken. -func (m *MeterSnapshot) Rate5() float64 { return m.rate5 } +func (m *meterSnapshot) Rate5() float64 { return m.rate5 } // Rate15 returns the fifteen-minute moving average rate of events per second // at the time the snapshot was taken. -func (m *MeterSnapshot) Rate15() float64 { return m.rate15 } +func (m *meterSnapshot) Rate15() float64 { return m.rate15 } // RateMean returns the meter's mean rate of events per second at the time the // snapshot was taken. -func (m *MeterSnapshot) RateMean() float64 { return m.rateMean } +func (m *meterSnapshot) RateMean() float64 { return m.rateMean } // Snapshot returns the snapshot. -func (m *MeterSnapshot) Snapshot() Meter { return m } +func (m *meterSnapshot) Snapshot() Meter { return m } // Stop is a no-op. -func (m *MeterSnapshot) Stop() {} +func (m *meterSnapshot) Stop() {} // NilMeter is a no-op Meter. type NilMeter struct{} @@ -175,8 +131,10 @@ func (NilMeter) Stop() {} // StandardMeter is the standard implementation of a Meter. type StandardMeter struct { - lock sync.RWMutex - snapshot *MeterSnapshot + count atomic.Int64 + temp atomic.Int64 + rateMean atomic.Uint64 + a1, a5, a15 EWMA startTime time.Time stopped atomic.Bool @@ -184,7 +142,6 @@ type StandardMeter struct { func newStandardMeter() *StandardMeter { return &StandardMeter{ - snapshot: &MeterSnapshot{}, a1: NewEWMA1(), a5: NewEWMA5(), a15: NewEWMA15(), @@ -194,8 +151,7 @@ func newStandardMeter() *StandardMeter { // Stop stops the meter, Mark() will be a no-op if you use it after being stopped. func (m *StandardMeter) Stop() { - stopped := m.stopped.Swap(true) - if !stopped { + if stopped := m.stopped.Swap(true); !stopped { arbiter.Lock() delete(arbiter.meters, m) arbiter.Unlock() @@ -203,88 +159,59 @@ func (m *StandardMeter) Stop() { } // Count returns the number of events recorded. -// It updates the meter to be as accurate as possible func (m *StandardMeter) Count() int64 { - m.lock.Lock() - defer m.lock.Unlock() - m.updateMeter() - return m.snapshot.count + return m.count.Load() + m.temp.Load() } // Mark records the occurrence of n events. func (m *StandardMeter) Mark(n int64) { - m.snapshot.temp.Add(n) + m.temp.Add(n) } // Rate1 returns the one-minute moving average rate of events per second. func (m *StandardMeter) Rate1() float64 { - m.lock.RLock() - defer m.lock.RUnlock() - return m.snapshot.rate1 + return m.a1.Rate() } // Rate5 returns the five-minute moving average rate of events per second. func (m *StandardMeter) Rate5() float64 { - m.lock.RLock() - defer m.lock.RUnlock() - return m.snapshot.rate5 + return m.a5.Rate() } // Rate15 returns the fifteen-minute moving average rate of events per second. func (m *StandardMeter) Rate15() float64 { - m.lock.RLock() - defer m.lock.RUnlock() - return m.snapshot.rate15 + return m.a15.Rate() } // RateMean returns the meter's mean rate of events per second. func (m *StandardMeter) RateMean() float64 { - m.lock.RLock() - defer m.lock.RUnlock() - return m.snapshot.rateMean + return math.Float64frombits(m.rateMean.Load()) } // Snapshot returns a read-only copy of the meter. func (m *StandardMeter) Snapshot() Meter { - m.lock.RLock() - snapshot := MeterSnapshot{ - count: m.snapshot.count, - rate1: m.snapshot.rate1, - rate5: m.snapshot.rate5, - rate15: m.snapshot.rate15, - rateMean: m.snapshot.rateMean, + return &meterSnapshot{ + count: m.count.Load(), + rate1: m.Rate1(), + rate5: m.Rate5(), + rate15: m.Rate15(), + rateMean: m.RateMean(), } - snapshot.temp.Store(m.snapshot.temp.Load()) - m.lock.RUnlock() - return &snapshot -} - -func (m *StandardMeter) updateSnapshot() { - // should run with write lock held on m.lock - snapshot := m.snapshot - snapshot.rate1 = m.a1.Rate() - snapshot.rate5 = m.a5.Rate() - snapshot.rate15 = m.a15.Rate() - snapshot.rateMean = float64(snapshot.count) / time.Since(m.startTime).Seconds() } -func (m *StandardMeter) updateMeter() { - // should only run with write lock held on m.lock - n := m.snapshot.temp.Swap(0) - m.snapshot.count += n +func (m *StandardMeter) tick() { + // Take the uncounted values, add to count + n := m.temp.Swap(0) + count := m.count.Add(n) + m.rateMean.Store(math.Float64bits(float64(count) / time.Since(m.startTime).Seconds())) + // Update the EWMA's internal state m.a1.Update(n) m.a5.Update(n) m.a15.Update(n) -} - -func (m *StandardMeter) tick() { - m.lock.Lock() - defer m.lock.Unlock() - m.updateMeter() + // And trigger them to calculate the rates m.a1.Tick() m.a5.Tick() m.a15.Tick() - m.updateSnapshot() } // meterArbiter ticks meters every 5s from a single goroutine. diff --git a/metrics/timer.go b/metrics/timer.go index 2e1a9be47295..ac88e24ad1d9 100644 --- a/metrics/timer.go +++ b/metrics/timer.go @@ -199,7 +199,7 @@ func (t *StandardTimer) Snapshot() Timer { defer t.mutex.Unlock() return &TimerSnapshot{ histogram: t.histogram.Snapshot().(*HistogramSnapshot), - meter: t.meter.Snapshot().(*MeterSnapshot), + meter: t.meter.Snapshot().(*meterSnapshot), } } @@ -249,7 +249,7 @@ func (t *StandardTimer) Variance() float64 { // TimerSnapshot is a read-only copy of another Timer. type TimerSnapshot struct { histogram *HistogramSnapshot - meter *MeterSnapshot + meter *meterSnapshot } // Count returns the number of events recorded at the time the snapshot was From 7e6f1a6869c50ddf2c1559464acb199eccd5cb7e Mon Sep 17 00:00:00 2001 From: Martin Holst Swende Date: Thu, 31 Aug 2023 09:06:06 +0200 Subject: [PATCH 04/26] metrics: separate updatable/readonly meter, timer and histogram --- metrics/histogram.go | 91 ++++++-------------- metrics/meter.go | 61 ++++---------- metrics/prometheus/collector.go | 6 +- metrics/runtimehistogram.go | 4 +- metrics/timer.go | 142 +++++++------------------------- 5 files changed, 72 insertions(+), 232 deletions(-) diff --git a/metrics/histogram.go b/metrics/histogram.go index 2c54ce8b4063..db294a9d3e46 100644 --- a/metrics/histogram.go +++ b/metrics/histogram.go @@ -1,8 +1,6 @@ package metrics -// Histograms calculate distribution statistics from a series of int64 values. -type Histogram interface { - Clear() +type HistogramSnapshot interface { Count() int64 Max() int64 Mean() float64 @@ -10,13 +8,18 @@ type Histogram interface { Percentile(float64) float64 Percentiles([]float64) []float64 Sample() Sample - Snapshot() Histogram StdDev() float64 Sum() int64 - Update(int64) Variance() float64 } +// Histograms calculate distribution statistics from a series of int64 values. +type Histogram interface { + Clear() + Update(int64) + Snapshot() HistogramSnapshot +} + // GetOrRegisterHistogram returns an existing Histogram or constructs and // registers a new StandardHistogram. func GetOrRegisterHistogram(name string, r Registry, s Sample) Histogram { @@ -54,64 +57,54 @@ func NewRegisteredHistogram(name string, r Registry, s Sample) Histogram { return c } -// HistogramSnapshot is a read-only copy of another Histogram. -type HistogramSnapshot struct { +// histogramSnapshot is a read-only copy of another Histogram. +type histogramSnapshot struct { sample *SampleSnapshot } -// Clear panics. -func (*HistogramSnapshot) Clear() { - panic("Clear called on a HistogramSnapshot") -} - // Count returns the number of samples recorded at the time the snapshot was // taken. -func (h *HistogramSnapshot) Count() int64 { return h.sample.Count() } +func (h *histogramSnapshot) Count() int64 { return h.sample.Count() } // Max returns the maximum value in the sample at the time the snapshot was // taken. -func (h *HistogramSnapshot) Max() int64 { return h.sample.Max() } +func (h *histogramSnapshot) Max() int64 { return h.sample.Max() } // Mean returns the mean of the values in the sample at the time the snapshot // was taken. -func (h *HistogramSnapshot) Mean() float64 { return h.sample.Mean() } +func (h *histogramSnapshot) Mean() float64 { return h.sample.Mean() } // Min returns the minimum value in the sample at the time the snapshot was // taken. -func (h *HistogramSnapshot) Min() int64 { return h.sample.Min() } +func (h *histogramSnapshot) Min() int64 { return h.sample.Min() } // Percentile returns an arbitrary percentile of values in the sample at the // time the snapshot was taken. -func (h *HistogramSnapshot) Percentile(p float64) float64 { +func (h *histogramSnapshot) Percentile(p float64) float64 { return h.sample.Percentile(p) } // Percentiles returns a slice of arbitrary percentiles of values in the sample // at the time the snapshot was taken. -func (h *HistogramSnapshot) Percentiles(ps []float64) []float64 { +func (h *histogramSnapshot) Percentiles(ps []float64) []float64 { return h.sample.Percentiles(ps) } // Sample returns the Sample underlying the histogram. -func (h *HistogramSnapshot) Sample() Sample { return h.sample } +func (h *histogramSnapshot) Sample() Sample { return h.sample } // Snapshot returns the snapshot. -func (h *HistogramSnapshot) Snapshot() Histogram { return h } +func (h *histogramSnapshot) Snapshot() HistogramSnapshot { return h } // StdDev returns the standard deviation of the values in the sample at the // time the snapshot was taken. -func (h *HistogramSnapshot) StdDev() float64 { return h.sample.StdDev() } +func (h *histogramSnapshot) StdDev() float64 { return h.sample.StdDev() } // Sum returns the sum in the sample at the time the snapshot was taken. -func (h *HistogramSnapshot) Sum() int64 { return h.sample.Sum() } - -// Update panics. -func (*HistogramSnapshot) Update(int64) { - panic("Update called on a HistogramSnapshot") -} +func (h *histogramSnapshot) Sum() int64 { return h.sample.Sum() } // Variance returns the variance of inputs at the time the snapshot was taken. -func (h *HistogramSnapshot) Variance() float64 { return h.sample.Variance() } +func (h *histogramSnapshot) Variance() float64 { return h.sample.Variance() } // NilHistogram is a no-op Histogram. type NilHistogram struct{} @@ -143,7 +136,7 @@ func (NilHistogram) Percentiles(ps []float64) []float64 { func (NilHistogram) Sample() Sample { return NilSample{} } // Snapshot is a no-op. -func (NilHistogram) Snapshot() Histogram { return NilHistogram{} } +func (NilHistogram) Snapshot() HistogramSnapshot { return NilHistogram{} } // StdDev is a no-op. func (NilHistogram) StdDev() float64 { return 0.0 } @@ -166,46 +159,10 @@ type StandardHistogram struct { // Clear clears the histogram and its sample. func (h *StandardHistogram) Clear() { h.sample.Clear() } -// Count returns the number of samples recorded since the histogram was last -// cleared. -func (h *StandardHistogram) Count() int64 { return h.sample.Count() } - -// Max returns the maximum value in the sample. -func (h *StandardHistogram) Max() int64 { return h.sample.Max() } - -// Mean returns the mean of the values in the sample. -func (h *StandardHistogram) Mean() float64 { return h.sample.Mean() } - -// Min returns the minimum value in the sample. -func (h *StandardHistogram) Min() int64 { return h.sample.Min() } - -// Percentile returns an arbitrary percentile of the values in the sample. -func (h *StandardHistogram) Percentile(p float64) float64 { - return h.sample.Percentile(p) -} - -// Percentiles returns a slice of arbitrary percentiles of the values in the -// sample. -func (h *StandardHistogram) Percentiles(ps []float64) []float64 { - return h.sample.Percentiles(ps) -} - -// Sample returns the Sample underlying the histogram. -func (h *StandardHistogram) Sample() Sample { return h.sample } - // Snapshot returns a read-only copy of the histogram. -func (h *StandardHistogram) Snapshot() Histogram { - return &HistogramSnapshot{sample: h.sample.Snapshot().(*SampleSnapshot)} +func (h *StandardHistogram) Snapshot() HistogramSnapshot { + return &histogramSnapshot{sample: h.sample.Snapshot().(*SampleSnapshot)} } -// StdDev returns the standard deviation of the values in the sample. -func (h *StandardHistogram) StdDev() float64 { return h.sample.StdDev() } - -// Sum returns the sum in the sample. -func (h *StandardHistogram) Sum() int64 { return h.sample.Sum() } - // Update samples a new value. func (h *StandardHistogram) Update(v int64) { h.sample.Update(v) } - -// Variance returns the variance of the values in the sample. -func (h *StandardHistogram) Variance() float64 { return h.sample.Variance() } diff --git a/metrics/meter.go b/metrics/meter.go index 25e08d3629ce..6eff96e675a2 100644 --- a/metrics/meter.go +++ b/metrics/meter.go @@ -7,16 +7,19 @@ import ( "time" ) -// Meters count events to produce exponentially-weighted moving average rates -// at one-, five-, and fifteen-minutes and a mean rate. -type Meter interface { +type MeterSnapshot interface { Count() int64 - Mark(int64) Rate1() float64 Rate5() float64 Rate15() float64 RateMean() float64 - Snapshot() Meter +} + +// Meters count events to produce exponentially-weighted moving average rates +// at one-, five-, and fifteen-minutes and a mean rate. +type Meter interface { + Mark(int64) + Snapshot() MeterSnapshot Stop() } @@ -75,11 +78,6 @@ type meterSnapshot struct { // Count returns the count of events at the time the snapshot was taken. func (m *meterSnapshot) Count() int64 { return m.count } -// Mark panics. -func (*meterSnapshot) Mark(n int64) { - panic("Mark called on a meterSnapshot") -} - // Rate1 returns the one-minute moving average rate of events per second at the // time the snapshot was taken. func (m *meterSnapshot) Rate1() float64 { return m.rate1 } @@ -96,12 +94,6 @@ func (m *meterSnapshot) Rate15() float64 { return m.rate15 } // snapshot was taken. func (m *meterSnapshot) RateMean() float64 { return m.rateMean } -// Snapshot returns the snapshot. -func (m *meterSnapshot) Snapshot() Meter { return m } - -// Stop is a no-op. -func (m *meterSnapshot) Stop() {} - // NilMeter is a no-op Meter. type NilMeter struct{} @@ -124,7 +116,7 @@ func (NilMeter) Rate15() float64 { return 0.0 } func (NilMeter) RateMean() float64 { return 0.0 } // Snapshot is a no-op. -func (NilMeter) Snapshot() Meter { return NilMeter{} } +func (NilMeter) Snapshot() MeterSnapshot { return NilMeter{} } // Stop is a no-op. func (NilMeter) Stop() {} @@ -158,44 +150,19 @@ func (m *StandardMeter) Stop() { } } -// Count returns the number of events recorded. -func (m *StandardMeter) Count() int64 { - return m.count.Load() + m.temp.Load() -} - // Mark records the occurrence of n events. func (m *StandardMeter) Mark(n int64) { m.temp.Add(n) } -// Rate1 returns the one-minute moving average rate of events per second. -func (m *StandardMeter) Rate1() float64 { - return m.a1.Rate() -} - -// Rate5 returns the five-minute moving average rate of events per second. -func (m *StandardMeter) Rate5() float64 { - return m.a5.Rate() -} - -// Rate15 returns the fifteen-minute moving average rate of events per second. -func (m *StandardMeter) Rate15() float64 { - return m.a15.Rate() -} - -// RateMean returns the meter's mean rate of events per second. -func (m *StandardMeter) RateMean() float64 { - return math.Float64frombits(m.rateMean.Load()) -} - // Snapshot returns a read-only copy of the meter. -func (m *StandardMeter) Snapshot() Meter { +func (m *StandardMeter) Snapshot() MeterSnapshot { return &meterSnapshot{ count: m.count.Load(), - rate1: m.Rate1(), - rate5: m.Rate5(), - rate15: m.Rate15(), - rateMean: m.RateMean(), + rate1: m.a1.Rate(), + rate5: m.a5.Rate(), + rate15: m.a15.Rate(), + rateMean: math.Float64frombits(m.rateMean.Load()), } } diff --git a/metrics/prometheus/collector.go b/metrics/prometheus/collector.go index 8624311c4b8e..90aa37f33811 100644 --- a/metrics/prometheus/collector.go +++ b/metrics/prometheus/collector.go @@ -95,7 +95,7 @@ func (c *collector) addGaugeInfo(name string, m metrics.GaugeInfo) { c.writeGaugeInfo(name, m.Value()) } -func (c *collector) addHistogram(name string, m metrics.Histogram) { +func (c *collector) addHistogram(name string, m metrics.HistogramSnapshot) { pv := []float64{0.5, 0.75, 0.95, 0.99, 0.999, 0.9999} ps := m.Percentiles(pv) c.writeSummaryCounter(name, m.Count()) @@ -106,11 +106,11 @@ func (c *collector) addHistogram(name string, m metrics.Histogram) { c.buff.WriteRune('\n') } -func (c *collector) addMeter(name string, m metrics.Meter) { +func (c *collector) addMeter(name string, m metrics.MeterSnapshot) { c.writeGaugeCounter(name, m.Count()) } -func (c *collector) addTimer(name string, m metrics.Timer) { +func (c *collector) addTimer(name string, m metrics.TimerSnapshot) { pv := []float64{0.5, 0.75, 0.95, 0.99, 0.999, 0.9999} ps := m.Percentiles(pv) c.writeSummaryCounter(name, m.Count()) diff --git a/metrics/runtimehistogram.go b/metrics/runtimehistogram.go index c68939af1ef7..f93a7188d912 100644 --- a/metrics/runtimehistogram.go +++ b/metrics/runtimehistogram.go @@ -62,7 +62,7 @@ func (h *runtimeHistogram) Sample() Sample { } // Snapshot returns a non-changing cop of the histogram. -func (h *runtimeHistogram) Snapshot() Histogram { +func (h *runtimeHistogram) Snapshot() HistogramSnapshot { return h.load() } @@ -123,7 +123,7 @@ func (h *runtimeHistogramSnapshot) Sample() Sample { return NilSample{} } -func (h *runtimeHistogramSnapshot) Snapshot() Histogram { +func (h *runtimeHistogramSnapshot) Snapshot() HistogramSnapshot { return h } diff --git a/metrics/timer.go b/metrics/timer.go index ac88e24ad1d9..c35bbc01b4e3 100644 --- a/metrics/timer.go +++ b/metrics/timer.go @@ -5,8 +5,7 @@ import ( "time" ) -// Timers capture the duration and rate of events. -type Timer interface { +type TimerSnapshot interface { Count() int64 Max() int64 Mean() float64 @@ -17,14 +16,18 @@ type Timer interface { Rate5() float64 Rate15() float64 RateMean() float64 - Snapshot() Timer + Sum() int64 + Variance() float64 StdDev() float64 +} + +// Timers capture the duration and rate of events. +type Timer interface { + Snapshot() TimerSnapshot Stop() - Sum() int64 Time(func()) - Update(time.Duration) UpdateSince(time.Time) - Variance() float64 + Update(time.Duration) } // GetOrRegisterTimer returns an existing Timer or constructs and registers a @@ -111,7 +114,7 @@ func (NilTimer) Rate15() float64 { return 0.0 } func (NilTimer) RateMean() float64 { return 0.0 } // Snapshot is a no-op. -func (NilTimer) Snapshot() Timer { return NilTimer{} } +func (NilTimer) Snapshot() TimerSnapshot { return NilTimer{} } // StdDev is a no-op. func (NilTimer) StdDev() float64 { return 0.0 } @@ -142,82 +145,21 @@ type StandardTimer struct { mutex sync.Mutex } -// Count returns the number of events recorded. -func (t *StandardTimer) Count() int64 { - return t.histogram.Count() -} - -// Max returns the maximum value in the sample. -func (t *StandardTimer) Max() int64 { - return t.histogram.Max() -} - -// Mean returns the mean of the values in the sample. -func (t *StandardTimer) Mean() float64 { - return t.histogram.Mean() -} - -// Min returns the minimum value in the sample. -func (t *StandardTimer) Min() int64 { - return t.histogram.Min() -} - -// Percentile returns an arbitrary percentile of the values in the sample. -func (t *StandardTimer) Percentile(p float64) float64 { - return t.histogram.Percentile(p) -} - -// Percentiles returns a slice of arbitrary percentiles of the values in the -// sample. -func (t *StandardTimer) Percentiles(ps []float64) []float64 { - return t.histogram.Percentiles(ps) -} - -// Rate1 returns the one-minute moving average rate of events per second. -func (t *StandardTimer) Rate1() float64 { - return t.meter.Rate1() -} - -// Rate5 returns the five-minute moving average rate of events per second. -func (t *StandardTimer) Rate5() float64 { - return t.meter.Rate5() -} - -// Rate15 returns the fifteen-minute moving average rate of events per second. -func (t *StandardTimer) Rate15() float64 { - return t.meter.Rate15() -} - -// RateMean returns the meter's mean rate of events per second. -func (t *StandardTimer) RateMean() float64 { - return t.meter.RateMean() -} - // Snapshot returns a read-only copy of the timer. -func (t *StandardTimer) Snapshot() Timer { +func (t *StandardTimer) Snapshot() TimerSnapshot { t.mutex.Lock() defer t.mutex.Unlock() - return &TimerSnapshot{ - histogram: t.histogram.Snapshot().(*HistogramSnapshot), + return &timerSnapshot{ + histogram: t.histogram.Snapshot().(*histogramSnapshot), meter: t.meter.Snapshot().(*meterSnapshot), } } -// StdDev returns the standard deviation of the values in the sample. -func (t *StandardTimer) StdDev() float64 { - return t.histogram.StdDev() -} - // Stop stops the meter. func (t *StandardTimer) Stop() { t.meter.Stop() } -// Sum returns the sum in the sample. -func (t *StandardTimer) Sum() int64 { - return t.histogram.Sum() -} - // Record the duration of the execution of the given function. func (t *StandardTimer) Time(f func()) { ts := time.Now() @@ -241,86 +183,60 @@ func (t *StandardTimer) UpdateSince(ts time.Time) { t.meter.Mark(1) } -// Variance returns the variance of the values in the sample. -func (t *StandardTimer) Variance() float64 { - return t.histogram.Variance() -} - -// TimerSnapshot is a read-only copy of another Timer. -type TimerSnapshot struct { - histogram *HistogramSnapshot +// timerSnapshot is a read-only copy of another Timer. +type timerSnapshot struct { + histogram *histogramSnapshot meter *meterSnapshot } // Count returns the number of events recorded at the time the snapshot was // taken. -func (t *TimerSnapshot) Count() int64 { return t.histogram.Count() } +func (t *timerSnapshot) Count() int64 { return t.histogram.Count() } // Max returns the maximum value at the time the snapshot was taken. -func (t *TimerSnapshot) Max() int64 { return t.histogram.Max() } +func (t *timerSnapshot) Max() int64 { return t.histogram.Max() } // Mean returns the mean value at the time the snapshot was taken. -func (t *TimerSnapshot) Mean() float64 { return t.histogram.Mean() } +func (t *timerSnapshot) Mean() float64 { return t.histogram.Mean() } // Min returns the minimum value at the time the snapshot was taken. -func (t *TimerSnapshot) Min() int64 { return t.histogram.Min() } +func (t *timerSnapshot) Min() int64 { return t.histogram.Min() } // Percentile returns an arbitrary percentile of sampled values at the time the // snapshot was taken. -func (t *TimerSnapshot) Percentile(p float64) float64 { +func (t *timerSnapshot) Percentile(p float64) float64 { return t.histogram.Percentile(p) } // Percentiles returns a slice of arbitrary percentiles of sampled values at // the time the snapshot was taken. -func (t *TimerSnapshot) Percentiles(ps []float64) []float64 { +func (t *timerSnapshot) Percentiles(ps []float64) []float64 { return t.histogram.Percentiles(ps) } // Rate1 returns the one-minute moving average rate of events per second at the // time the snapshot was taken. -func (t *TimerSnapshot) Rate1() float64 { return t.meter.Rate1() } +func (t *timerSnapshot) Rate1() float64 { return t.meter.Rate1() } // Rate5 returns the five-minute moving average rate of events per second at // the time the snapshot was taken. -func (t *TimerSnapshot) Rate5() float64 { return t.meter.Rate5() } +func (t *timerSnapshot) Rate5() float64 { return t.meter.Rate5() } // Rate15 returns the fifteen-minute moving average rate of events per second // at the time the snapshot was taken. -func (t *TimerSnapshot) Rate15() float64 { return t.meter.Rate15() } +func (t *timerSnapshot) Rate15() float64 { return t.meter.Rate15() } // RateMean returns the meter's mean rate of events per second at the time the // snapshot was taken. -func (t *TimerSnapshot) RateMean() float64 { return t.meter.RateMean() } - -// Snapshot returns the snapshot. -func (t *TimerSnapshot) Snapshot() Timer { return t } +func (t *timerSnapshot) RateMean() float64 { return t.meter.RateMean() } // StdDev returns the standard deviation of the values at the time the snapshot // was taken. -func (t *TimerSnapshot) StdDev() float64 { return t.histogram.StdDev() } - -// Stop is a no-op. -func (t *TimerSnapshot) Stop() {} +func (t *timerSnapshot) StdDev() float64 { return t.histogram.StdDev() } // Sum returns the sum at the time the snapshot was taken. -func (t *TimerSnapshot) Sum() int64 { return t.histogram.Sum() } - -// Time panics. -func (*TimerSnapshot) Time(func()) { - panic("Time called on a TimerSnapshot") -} - -// Update panics. -func (*TimerSnapshot) Update(time.Duration) { - panic("Update called on a TimerSnapshot") -} - -// UpdateSince panics. -func (*TimerSnapshot) UpdateSince(time.Time) { - panic("UpdateSince called on a TimerSnapshot") -} +func (t *timerSnapshot) Sum() int64 { return t.histogram.Sum() } // Variance returns the variance of the values at the time the snapshot was // taken. -func (t *TimerSnapshot) Variance() float64 { return t.histogram.Variance() } +func (t *timerSnapshot) Variance() float64 { return t.histogram.Variance() } From 9591faf40d7d79dbf190d40d77e681db5bd1255d Mon Sep 17 00:00:00 2001 From: Martin Holst Swende Date: Thu, 31 Aug 2023 09:46:47 +0200 Subject: [PATCH 05/26] metrics: separate updatable/readonly sample, runtimehistogram + do early sample calculation to avoid later iterations --- metrics/histogram.go | 10 +- metrics/resetting_sample.go | 2 +- metrics/runtimehistogram.go | 56 +------ metrics/sample.go | 313 ++++++++---------------------------- metrics/sample_test.go | 11 +- 5 files changed, 81 insertions(+), 311 deletions(-) diff --git a/metrics/histogram.go b/metrics/histogram.go index db294a9d3e46..5f7e2996247a 100644 --- a/metrics/histogram.go +++ b/metrics/histogram.go @@ -7,7 +7,7 @@ type HistogramSnapshot interface { Min() int64 Percentile(float64) float64 Percentiles([]float64) []float64 - Sample() Sample + Sample() SampleSnapshot StdDev() float64 Sum() int64 Variance() float64 @@ -59,7 +59,7 @@ func NewRegisteredHistogram(name string, r Registry, s Sample) Histogram { // histogramSnapshot is a read-only copy of another Histogram. type histogramSnapshot struct { - sample *SampleSnapshot + sample *sampleSnapshot } // Count returns the number of samples recorded at the time the snapshot was @@ -91,7 +91,7 @@ func (h *histogramSnapshot) Percentiles(ps []float64) []float64 { } // Sample returns the Sample underlying the histogram. -func (h *histogramSnapshot) Sample() Sample { return h.sample } +func (h *histogramSnapshot) Sample() SampleSnapshot { return h.sample } // Snapshot returns the snapshot. func (h *histogramSnapshot) Snapshot() HistogramSnapshot { return h } @@ -133,7 +133,7 @@ func (NilHistogram) Percentiles(ps []float64) []float64 { } // Sample is a no-op. -func (NilHistogram) Sample() Sample { return NilSample{} } +func (NilHistogram) Sample() SampleSnapshot { return NilSample{} } // Snapshot is a no-op. func (NilHistogram) Snapshot() HistogramSnapshot { return NilHistogram{} } @@ -161,7 +161,7 @@ func (h *StandardHistogram) Clear() { h.sample.Clear() } // Snapshot returns a read-only copy of the histogram. func (h *StandardHistogram) Snapshot() HistogramSnapshot { - return &histogramSnapshot{sample: h.sample.Snapshot().(*SampleSnapshot)} + return &histogramSnapshot{sample: h.sample.Snapshot().(*sampleSnapshot)} } // Update samples a new value. diff --git a/metrics/resetting_sample.go b/metrics/resetting_sample.go index 43c1129cd0bc..c38ffcd3ec32 100644 --- a/metrics/resetting_sample.go +++ b/metrics/resetting_sample.go @@ -17,7 +17,7 @@ type resettingSample struct { } // Snapshot returns a read-only copy of the sample with the original reset. -func (rs *resettingSample) Snapshot() Sample { +func (rs *resettingSample) Snapshot() SampleSnapshot { s := rs.Sample.Snapshot() rs.Sample.Clear() return s diff --git a/metrics/runtimehistogram.go b/metrics/runtimehistogram.go index f93a7188d912..6abdb67d59d9 100644 --- a/metrics/runtimehistogram.go +++ b/metrics/runtimehistogram.go @@ -57,69 +57,15 @@ func (h *runtimeHistogram) Clear() { func (h *runtimeHistogram) Update(int64) { panic("runtimeHistogram does not support Update") } -func (h *runtimeHistogram) Sample() Sample { - return NilSample{} -} // Snapshot returns a non-changing cop of the histogram. func (h *runtimeHistogram) Snapshot() HistogramSnapshot { return h.load() } -// Count returns the sample count. -func (h *runtimeHistogram) Count() int64 { - return h.load().Count() -} - -// Mean returns an approximation of the mean. -func (h *runtimeHistogram) Mean() float64 { - return h.load().Mean() -} - -// StdDev approximates the standard deviation of the histogram. -func (h *runtimeHistogram) StdDev() float64 { - return h.load().StdDev() -} - -// Variance approximates the variance of the histogram. -func (h *runtimeHistogram) Variance() float64 { - return h.load().Variance() -} - -// Percentile computes the p'th percentile value. -func (h *runtimeHistogram) Percentile(p float64) float64 { - return h.load().Percentile(p) -} - -// Percentiles computes all requested percentile values. -func (h *runtimeHistogram) Percentiles(ps []float64) []float64 { - return h.load().Percentiles(ps) -} - -// Max returns the highest sample value. -func (h *runtimeHistogram) Max() int64 { - return h.load().Max() -} - -// Min returns the lowest sample value. -func (h *runtimeHistogram) Min() int64 { - return h.load().Min() -} - -// Sum returns the sum of all sample values. -func (h *runtimeHistogram) Sum() int64 { - return h.load().Sum() -} - type runtimeHistogramSnapshot metrics.Float64Histogram -func (h *runtimeHistogramSnapshot) Clear() { - panic("runtimeHistogram does not support Clear") -} -func (h *runtimeHistogramSnapshot) Update(int64) { - panic("runtimeHistogram does not support Update") -} -func (h *runtimeHistogramSnapshot) Sample() Sample { +func (h *runtimeHistogramSnapshot) Sample() SampleSnapshot { return NilSample{} } diff --git a/metrics/sample.go b/metrics/sample.go index 252a878f581b..42d3b619561a 100644 --- a/metrics/sample.go +++ b/metrics/sample.go @@ -11,10 +11,7 @@ import ( const rescaleThreshold = time.Hour -// Samples maintain a statistically-significant selection of values from -// a stream. -type Sample interface { - Clear() +type SampleSnapshot interface { Count() int64 Max() int64 Mean() float64 @@ -22,14 +19,20 @@ type Sample interface { Percentile(float64) float64 Percentiles([]float64) []float64 Size() int - Snapshot() Sample StdDev() float64 Sum() int64 - Update(int64) Values() []int64 Variance() float64 } +// Samples maintain a statistically-significant selection of values from +// a stream. +type Sample interface { + Snapshot() SampleSnapshot + Clear() + Update(int64) +} + // ExpDecaySample is an exponentially-decaying sample using a forward-decaying // priority reservoir. See Cormode et al's "Forward Decay: A Practical Time // Decay Model for Streaming Systems". @@ -77,72 +80,16 @@ func (s *ExpDecaySample) Clear() { s.values.Clear() } -// Count returns the number of samples recorded, which may exceed the -// reservoir size. -func (s *ExpDecaySample) Count() int64 { - s.mutex.Lock() - defer s.mutex.Unlock() - return s.count -} - -// Max returns the maximum value in the sample, which may not be the maximum -// value ever to be part of the sample. -func (s *ExpDecaySample) Max() int64 { - return SampleMax(s.Values()) -} - -// Mean returns the mean of the values in the sample. -func (s *ExpDecaySample) Mean() float64 { - return SampleMean(s.Values()) -} - -// Min returns the minimum value in the sample, which may not be the minimum -// value ever to be part of the sample. -func (s *ExpDecaySample) Min() int64 { - return SampleMin(s.Values()) -} - -// Percentile returns an arbitrary percentile of values in the sample. -func (s *ExpDecaySample) Percentile(p float64) float64 { - return SamplePercentile(s.Values(), p) -} - -// Percentiles returns a slice of arbitrary percentiles of values in the -// sample. -func (s *ExpDecaySample) Percentiles(ps []float64) []float64 { - return SamplePercentiles(s.Values(), ps) -} - -// Size returns the size of the sample, which is at most the reservoir size. -func (s *ExpDecaySample) Size() int { - s.mutex.Lock() - defer s.mutex.Unlock() - return s.values.Size() -} - // Snapshot returns a read-only copy of the sample. -func (s *ExpDecaySample) Snapshot() Sample { +func (s *ExpDecaySample) Snapshot() SampleSnapshot { s.mutex.Lock() - defer s.mutex.Unlock() vals := s.values.Values() values := make([]int64, len(vals)) for i, v := range vals { values[i] = v.v } - return &SampleSnapshot{ - count: s.count, - values: values, - } -} - -// StdDev returns the standard deviation of the values in the sample. -func (s *ExpDecaySample) StdDev() float64 { - return SampleStdDev(s.Values()) -} - -// Sum returns the sum of the values in the sample. -func (s *ExpDecaySample) Sum() int64 { - return SampleSum(s.Values()) + s.mutex.Unlock() + return newSampleSnapshot(s.count, values) } // Update samples a new value. @@ -150,23 +97,6 @@ func (s *ExpDecaySample) Update(v int64) { s.update(time.Now(), v) } -// Values returns a copy of the values in the sample. -func (s *ExpDecaySample) Values() []int64 { - s.mutex.Lock() - defer s.mutex.Unlock() - vals := s.values.Values() - values := make([]int64, len(vals)) - for i, v := range vals { - values[i] = v.v - } - return values -} - -// Variance returns the variance of the values in the sample. -func (s *ExpDecaySample) Variance() float64 { - return SampleVariance(s.Values()) -} - // update samples a new value at a particular timestamp. This is a method all // its own to facilitate testing. func (s *ExpDecaySample) update(t time.Time, v int64) { @@ -229,7 +159,7 @@ func (NilSample) Percentiles(ps []float64) []float64 { func (NilSample) Size() int { return 0 } // Sample is a no-op. -func (NilSample) Snapshot() Sample { return NilSample{} } +func (NilSample) Snapshot() SampleSnapshot { return NilSample{} } // StdDev is a no-op. func (NilSample) StdDev() float64 { return 0.0 } @@ -246,42 +176,6 @@ func (NilSample) Values() []int64 { return []int64{} } // Variance is a no-op. func (NilSample) Variance() float64 { return 0.0 } -// SampleMax returns the maximum value of the slice of int64. -func SampleMax(values []int64) int64 { - if len(values) == 0 { - return 0 - } - var max int64 = math.MinInt64 - for _, v := range values { - if max < v { - max = v - } - } - return max -} - -// SampleMean returns the mean value of the slice of int64. -func SampleMean(values []int64) float64 { - if len(values) == 0 { - return 0.0 - } - return float64(SampleSum(values)) / float64(len(values)) -} - -// SampleMin returns the minimum value of the slice of int64. -func SampleMin(values []int64) int64 { - if len(values) == 0 { - return 0 - } - var min int64 = math.MaxInt64 - for _, v := range values { - if min > v { - min = v - } - } - return min -} - // SamplePercentiles returns an arbitrary percentile of the slice of int64. func SamplePercentile(values []int64, p float64) float64 { return SamplePercentiles(values, []float64{p})[0] @@ -310,99 +204,108 @@ func SamplePercentiles(values []int64, ps []float64) []float64 { return scores } -// SampleSnapshot is a read-only copy of another Sample. -type SampleSnapshot struct { +// sampleSnapshot is a read-only copy of another Sample. +type sampleSnapshot struct { count int64 values []int64 + + max int64 + min int64 + mean float64 + sum int64 } -func NewSampleSnapshot(count int64, values []int64) *SampleSnapshot { - return &SampleSnapshot{ +// newSampleSnapshot creates a read-only sampleSnapShot, and calculates some +// numbers. +func newSampleSnapshot(count int64, values []int64) *sampleSnapshot { + s := &sampleSnapshot{ count: count, values: values, } -} - -// Clear panics. -func (*SampleSnapshot) Clear() { - panic("Clear called on a SampleSnapshot") + if len(values) == 0 { + return s + } + var ( + max int64 = math.MinInt64 + min int64 = math.MaxInt64 + sum int64 + ) + for _, v := range values { + sum += v + if v > max { + max = v + } + if v < min { + min = v + } + } + s.min = min + s.max = max + s.mean = float64(sum) / float64(len(values)) + s.sum = sum + return s } // Count returns the count of inputs at the time the snapshot was taken. -func (s *SampleSnapshot) Count() int64 { return s.count } +func (s *sampleSnapshot) Count() int64 { return s.count } // Max returns the maximal value at the time the snapshot was taken. -func (s *SampleSnapshot) Max() int64 { return SampleMax(s.values) } +func (s *sampleSnapshot) Max() int64 { return s.max } // Mean returns the mean value at the time the snapshot was taken. -func (s *SampleSnapshot) Mean() float64 { return SampleMean(s.values) } +func (s *sampleSnapshot) Mean() float64 { return s.mean } // Min returns the minimal value at the time the snapshot was taken. -func (s *SampleSnapshot) Min() int64 { return SampleMin(s.values) } +func (s *sampleSnapshot) Min() int64 { return s.min } // Percentile returns an arbitrary percentile of values at the time the // snapshot was taken. -func (s *SampleSnapshot) Percentile(p float64) float64 { +func (s *sampleSnapshot) Percentile(p float64) float64 { return SamplePercentile(s.values, p) } // Percentiles returns a slice of arbitrary percentiles of values at the time // the snapshot was taken. -func (s *SampleSnapshot) Percentiles(ps []float64) []float64 { +func (s *sampleSnapshot) Percentiles(ps []float64) []float64 { return SamplePercentiles(s.values, ps) } // Size returns the size of the sample at the time the snapshot was taken. -func (s *SampleSnapshot) Size() int { return len(s.values) } +func (s *sampleSnapshot) Size() int { return len(s.values) } // Snapshot returns the snapshot. -func (s *SampleSnapshot) Snapshot() Sample { return s } +func (s *sampleSnapshot) Snapshot() SampleSnapshot { return s } // StdDev returns the standard deviation of values at the time the snapshot was // taken. -func (s *SampleSnapshot) StdDev() float64 { return SampleStdDev(s.values) } +func (s *sampleSnapshot) StdDev() float64 { return SampleStdDev(s.mean, s.values) } // Sum returns the sum of values at the time the snapshot was taken. -func (s *SampleSnapshot) Sum() int64 { return SampleSum(s.values) } - -// Update panics. -func (*SampleSnapshot) Update(int64) { - panic("Update called on a SampleSnapshot") -} +func (s *sampleSnapshot) Sum() int64 { return s.sum } // Values returns a copy of the values in the sample. -func (s *SampleSnapshot) Values() []int64 { +func (s *sampleSnapshot) Values() []int64 { values := make([]int64, len(s.values)) copy(values, s.values) return values } // Variance returns the variance of values at the time the snapshot was taken. -func (s *SampleSnapshot) Variance() float64 { return SampleVariance(s.values) } +func (s *sampleSnapshot) Variance() float64 { return SampleVariance(s.mean, s.values) } // SampleStdDev returns the standard deviation of the slice of int64. -func SampleStdDev(values []int64) float64 { - return math.Sqrt(SampleVariance(values)) -} - -// SampleSum returns the sum of the slice of int64. -func SampleSum(values []int64) int64 { - var sum int64 - for _, v := range values { - sum += v - } - return sum +func SampleStdDev(mean float64, values []int64) float64 { + return math.Sqrt(SampleVariance(mean, values)) } // SampleVariance returns the variance of the slice of int64. -func SampleVariance(values []int64) float64 { +func SampleVariance(mean float64, values []int64) float64 { if len(values) == 0 { return 0.0 } - m := SampleMean(values) var sum float64 for _, v := range values { - d := float64(v) - m + d := float64(v) - mean sum += d * d } return sum / float64(len(values)) @@ -445,83 +348,13 @@ func (s *UniformSample) Clear() { s.values = make([]int64, 0, s.reservoirSize) } -// Count returns the number of samples recorded, which may exceed the -// reservoir size. -func (s *UniformSample) Count() int64 { - s.mutex.Lock() - defer s.mutex.Unlock() - return s.count -} - -// Max returns the maximum value in the sample, which may not be the maximum -// value ever to be part of the sample. -func (s *UniformSample) Max() int64 { - s.mutex.Lock() - defer s.mutex.Unlock() - return SampleMax(s.values) -} - -// Mean returns the mean of the values in the sample. -func (s *UniformSample) Mean() float64 { - s.mutex.Lock() - defer s.mutex.Unlock() - return SampleMean(s.values) -} - -// Min returns the minimum value in the sample, which may not be the minimum -// value ever to be part of the sample. -func (s *UniformSample) Min() int64 { - s.mutex.Lock() - defer s.mutex.Unlock() - return SampleMin(s.values) -} - -// Percentile returns an arbitrary percentile of values in the sample. -func (s *UniformSample) Percentile(p float64) float64 { - s.mutex.Lock() - defer s.mutex.Unlock() - return SamplePercentile(s.values, p) -} - -// Percentiles returns a slice of arbitrary percentiles of values in the -// sample. -func (s *UniformSample) Percentiles(ps []float64) []float64 { - s.mutex.Lock() - defer s.mutex.Unlock() - return SamplePercentiles(s.values, ps) -} - -// Size returns the size of the sample, which is at most the reservoir size. -func (s *UniformSample) Size() int { - s.mutex.Lock() - defer s.mutex.Unlock() - return len(s.values) -} - // Snapshot returns a read-only copy of the sample. -func (s *UniformSample) Snapshot() Sample { +func (s *UniformSample) Snapshot() SampleSnapshot { s.mutex.Lock() - defer s.mutex.Unlock() values := make([]int64, len(s.values)) copy(values, s.values) - return &SampleSnapshot{ - count: s.count, - values: values, - } -} - -// StdDev returns the standard deviation of the values in the sample. -func (s *UniformSample) StdDev() float64 { - s.mutex.Lock() - defer s.mutex.Unlock() - return SampleStdDev(s.values) -} - -// Sum returns the sum of the values in the sample. -func (s *UniformSample) Sum() int64 { - s.mutex.Lock() - defer s.mutex.Unlock() - return SampleSum(s.values) + s.mutex.Unlock() + return newSampleSnapshot(s.count, s.values) } // Update samples a new value. @@ -544,22 +377,6 @@ func (s *UniformSample) Update(v int64) { } } -// Values returns a copy of the values in the sample. -func (s *UniformSample) Values() []int64 { - s.mutex.Lock() - defer s.mutex.Unlock() - values := make([]int64, len(s.values)) - copy(values, s.values) - return values -} - -// Variance returns the variance of the values in the sample. -func (s *UniformSample) Variance() float64 { - s.mutex.Lock() - defer s.mutex.Unlock() - return SampleVariance(s.values) -} - // expDecaySample represents an individual sample in a heap. type expDecaySample struct { k float64 diff --git a/metrics/sample_test.go b/metrics/sample_test.go index 3ae128d56f67..018358d729dc 100644 --- a/metrics/sample_test.go +++ b/metrics/sample_test.go @@ -14,22 +14,29 @@ import ( // computation for small samples and only slightly less for large samples. func BenchmarkCompute1000(b *testing.B) { s := make([]int64, 1000) + var sum int64 for i := 0; i < len(s); i++ { s[i] = int64(i) + sum += int64(i) } + mean := float64(sum) / float64(len(s)) b.ResetTimer() for i := 0; i < b.N; i++ { - SampleVariance(s) + SampleVariance(mean, s) } } func BenchmarkCompute1000000(b *testing.B) { s := make([]int64, 1000000) + var sum int64 for i := 0; i < len(s); i++ { s[i] = int64(i) + sum += int64(i) + } + mean := float64(sum) / float64(len(s)) b.ResetTimer() for i := 0; i < b.N; i++ { - SampleVariance(s) + SampleVariance(mean, s) } } func BenchmarkCopy1000(b *testing.B) { From b7f8d9b4cef0a86e22a21accbd977901037fd462 Mon Sep 17 00:00:00 2001 From: Martin Holst Swende Date: Thu, 31 Aug 2023 11:50:17 +0200 Subject: [PATCH 06/26] metrics: split Gauge interface, fix tests, fix race on Sample --- core/state/statedb.go | 10 ++-- metrics/exp/exp.go | 4 +- metrics/gauge.go | 95 +++++++-------------------------- metrics/gauge_test.go | 39 +------------- metrics/graphite.go | 2 +- metrics/histogram_test.go | 8 +-- metrics/librato/librato.go | 52 +++++++++--------- metrics/log.go | 2 +- metrics/meter.go | 12 ++--- metrics/meter_test.go | 30 +++++------ metrics/metrics_test.go | 2 +- metrics/opentsdb.go | 2 +- metrics/prometheus/collector.go | 2 +- metrics/registry.go | 2 +- metrics/sample.go | 3 +- metrics/sample_test.go | 42 ++++++++------- metrics/syslog.go | 2 +- metrics/timer_test.go | 10 ++-- metrics/writer.go | 2 +- 19 files changed, 118 insertions(+), 203 deletions(-) diff --git a/core/state/statedb.go b/core/state/statedb.go index 17bf9918078e..e4b305b3e3bf 100644 --- a/core/state/statedb.go +++ b/core/state/statedb.go @@ -1103,12 +1103,10 @@ func (s *StateDB) deleteStorage(addr common.Address, addrHash common.Hash, root slotDeletionSkip.Inc(1) } n := int64(len(slots)) - if n > slotDeletionMaxCount.Value() { - slotDeletionMaxCount.Update(n) - } - if int64(size) > slotDeletionMaxSize.Value() { - slotDeletionMaxSize.Update(int64(size)) - } + + slotDeletionMaxCount.UpdateIfGt(int64(len(slots))) + slotDeletionMaxSize.UpdateIfGt(int64(size)) + slotDeletionTimer.UpdateSince(start) slotDeletionCount.Mark(n) slotDeletionSize.Mark(int64(size)) diff --git a/metrics/exp/exp.go b/metrics/exp/exp.go index 9e850f96b266..834a27a44aa1 100644 --- a/metrics/exp/exp.go +++ b/metrics/exp/exp.go @@ -119,7 +119,7 @@ func (exp *exp) publishCounterFloat64(name string, metric metrics.CounterFloat64 v.Set(metric.Count()) } -func (exp *exp) publishGauge(name string, metric metrics.Gauge) { +func (exp *exp) publishGauge(name string, metric metrics.GaugeSnapshot) { v := exp.getInt(name) v.Set(metric.Value()) } @@ -193,7 +193,7 @@ func (exp *exp) syncToExpvar() { case metrics.CounterFloat64: exp.publishCounterFloat64(name, i) case metrics.Gauge: - exp.publishGauge(name, i) + exp.publishGauge(name, i.Snapshot()) case metrics.GaugeFloat64: exp.publishGaugeFloat64(name, i) case metrics.GaugeInfo: diff --git a/metrics/gauge.go b/metrics/gauge.go index 81137d7f7c5e..c4aef478a5f2 100644 --- a/metrics/gauge.go +++ b/metrics/gauge.go @@ -2,13 +2,18 @@ package metrics import "sync/atomic" +// gaugeSnapshot contains a readonly int64. +type GaugeSnapshot interface { + Value() int64 +} + // Gauges hold an int64 value that can be set arbitrarily. type Gauge interface { - Snapshot() Gauge + Snapshot() GaugeSnapshot Update(int64) + UpdateIfGt(int64) Dec(int64) Inc(int64) - Value() int64 } // GetOrRegisterGauge returns an existing Gauge or constructs and registers a @@ -38,57 +43,23 @@ func NewRegisteredGauge(name string, r Registry) Gauge { return c } -// NewFunctionalGauge constructs a new FunctionalGauge. -func NewFunctionalGauge(f func() int64) Gauge { - if !Enabled { - return NilGauge{} - } - return &FunctionalGauge{value: f} -} - -// NewRegisteredFunctionalGauge constructs and registers a new StandardGauge. -func NewRegisteredFunctionalGauge(name string, r Registry, f func() int64) Gauge { - c := NewFunctionalGauge(f) - if nil == r { - r = DefaultRegistry - } - r.Register(name, c) - return c -} - -// GaugeSnapshot is a read-only copy of another Gauge. -type GaugeSnapshot int64 - -// Snapshot returns the snapshot. -func (g GaugeSnapshot) Snapshot() Gauge { return g } - -// Update panics. -func (GaugeSnapshot) Update(int64) { - panic("Update called on a GaugeSnapshot") -} - -// Dec panics. -func (GaugeSnapshot) Dec(int64) { - panic("Dec called on a GaugeSnapshot") -} - -// Inc panics. -func (GaugeSnapshot) Inc(int64) { - panic("Inc called on a GaugeSnapshot") -} +// gaugeSnapshot is a read-only copy of another Gauge. +type gaugeSnapshot int64 // Value returns the value at the time the snapshot was taken. -func (g GaugeSnapshot) Value() int64 { return int64(g) } +func (g gaugeSnapshot) Value() int64 { return int64(g) } // NilGauge is a no-op Gauge. type NilGauge struct{} // Snapshot is a no-op. -func (NilGauge) Snapshot() Gauge { return NilGauge{} } +func (NilGauge) Snapshot() GaugeSnapshot { return NilGauge{} } // Update is a no-op. func (NilGauge) Update(v int64) {} +func (NilGauge) UpdateIfGt(v int64) {} + // Dec is a no-op. func (NilGauge) Dec(i int64) {} @@ -105,8 +76,8 @@ type StandardGauge struct { } // Snapshot returns a read-only copy of the gauge. -func (g *StandardGauge) Snapshot() Gauge { - return GaugeSnapshot(g.Value()) +func (g *StandardGauge) Snapshot() GaugeSnapshot { + return gaugeSnapshot(g.value.Load()) } // Update updates the gauge's value. @@ -114,9 +85,11 @@ func (g *StandardGauge) Update(v int64) { g.value.Store(v) } -// Value returns the gauge's current value. -func (g *StandardGauge) Value() int64 { - return g.value.Load() +// Update updates the gauge's value if v is larger then the current valie. +func (g *StandardGauge) UpdateIfGt(v int64) { + if g.value.Load() < v { + g.value.Store(v) + } } // Dec decrements the gauge's current value by the given amount. @@ -128,31 +101,3 @@ func (g *StandardGauge) Dec(i int64) { func (g *StandardGauge) Inc(i int64) { g.value.Add(i) } - -// FunctionalGauge returns value from given function -type FunctionalGauge struct { - value func() int64 -} - -// Value returns the gauge's current value. -func (g FunctionalGauge) Value() int64 { - return g.value() -} - -// Snapshot returns the snapshot. -func (g FunctionalGauge) Snapshot() Gauge { return GaugeSnapshot(g.Value()) } - -// Update panics. -func (FunctionalGauge) Update(int64) { - panic("Update called on a FunctionalGauge") -} - -// Dec panics. -func (FunctionalGauge) Dec(int64) { - panic("Dec called on a FunctionalGauge") -} - -// Inc panics. -func (FunctionalGauge) Inc(int64) { - panic("Inc called on a FunctionalGauge") -} diff --git a/metrics/gauge_test.go b/metrics/gauge_test.go index a98fe985d8c2..f2ba930bc465 100644 --- a/metrics/gauge_test.go +++ b/metrics/gauge_test.go @@ -1,7 +1,6 @@ package metrics import ( - "fmt" "testing" ) @@ -13,14 +12,6 @@ func BenchmarkGauge(b *testing.B) { } } -func TestGauge(t *testing.T) { - g := NewGauge() - g.Update(int64(47)) - if v := g.Value(); v != 47 { - t.Errorf("g.Value(): 47 != %v\n", v) - } -} - func TestGaugeSnapshot(t *testing.T) { g := NewGauge() g.Update(int64(47)) @@ -34,35 +25,7 @@ func TestGaugeSnapshot(t *testing.T) { func TestGetOrRegisterGauge(t *testing.T) { r := NewRegistry() NewRegisteredGauge("foo", r).Update(47) - if g := GetOrRegisterGauge("foo", r); g.Value() != 47 { - t.Fatal(g) - } -} - -func TestFunctionalGauge(t *testing.T) { - var counter int64 - fg := NewFunctionalGauge(func() int64 { - counter++ - return counter - }) - fg.Value() - fg.Value() - if counter != 2 { - t.Error("counter != 2") - } -} - -func TestGetOrRegisterFunctionalGauge(t *testing.T) { - r := NewRegistry() - NewRegisteredFunctionalGauge("foo", r, func() int64 { return 47 }) - if g := GetOrRegisterGauge("foo", r); g.Value() != 47 { + if g := GetOrRegisterGauge("foo", r); g.Snapshot().Value() != 47 { t.Fatal(g) } } - -func ExampleGetOrRegisterGauge() { - m := "server.bytes_sent" - g := GetOrRegisterGauge(m, nil) - g.Update(47) - fmt.Println(g.Value()) // Output: 47 -} diff --git a/metrics/graphite.go b/metrics/graphite.go index 4e3dd3b3b894..d9cb8b533ccd 100644 --- a/metrics/graphite.go +++ b/metrics/graphite.go @@ -70,7 +70,7 @@ func graphite(c *GraphiteConfig) error { case CounterFloat64: fmt.Fprintf(w, "%s.%s.count %f %d\n", c.Prefix, name, metric.Count(), now) case Gauge: - fmt.Fprintf(w, "%s.%s.value %d %d\n", c.Prefix, name, metric.Value(), now) + fmt.Fprintf(w, "%s.%s.value %d %d\n", c.Prefix, name, metric.Snapshot().Value(), now) case GaugeFloat64: fmt.Fprintf(w, "%s.%s.value %f %d\n", c.Prefix, name, metric.Value(), now) case GaugeInfo: diff --git a/metrics/histogram_test.go b/metrics/histogram_test.go index 7c9f42fcec96..22fc5468b0b5 100644 --- a/metrics/histogram_test.go +++ b/metrics/histogram_test.go @@ -14,7 +14,7 @@ func TestGetOrRegisterHistogram(t *testing.T) { r := NewRegistry() s := NewUniformSample(100) NewRegisteredHistogram("foo", r, s).Update(47) - if h := GetOrRegisterHistogram("foo", r, s); h.Count() != 1 { + if h := GetOrRegisterHistogram("foo", r, s).Snapshot(); h.Count() != 1 { t.Fatal(h) } } @@ -24,11 +24,11 @@ func TestHistogram10000(t *testing.T) { for i := 1; i <= 10000; i++ { h.Update(int64(i)) } - testHistogram10000(t, h) + testHistogram10000(t, h.Snapshot()) } func TestHistogramEmpty(t *testing.T) { - h := NewHistogram(NewUniformSample(100)) + h := NewHistogram(NewUniformSample(100)).Snapshot() if count := h.Count(); count != 0 { t.Errorf("h.Count(): 0 != %v\n", count) } @@ -66,7 +66,7 @@ func TestHistogramSnapshot(t *testing.T) { testHistogram10000(t, snapshot) } -func testHistogram10000(t *testing.T, h Histogram) { +func testHistogram10000(t *testing.T, h HistogramSnapshot) { if count := h.Count(); count != 10000 { t.Errorf("h.Count(): 10000 != %v\n", count) } diff --git a/metrics/librato/librato.go b/metrics/librato/librato.go index fa98595991fa..e7385264e9b9 100644 --- a/metrics/librato/librato.go +++ b/metrics/librato/librato.go @@ -61,7 +61,7 @@ func (rep *Reporter) Run() { // calculate sum of squares from data provided by metrics.Histogram // see http://en.wikipedia.org/wiki/Standard_deviation#Rapid_calculation_methods -func sumSquares(s metrics.Sample) float64 { +func sumSquares(s metrics.SampleSnapshot) float64 { count := float64(s.Count()) sumSquared := math.Pow(count*s.Mean(), 2) sumSquares := math.Pow(count*s.StdDev(), 2) + sumSquared/count @@ -70,7 +70,7 @@ func sumSquares(s metrics.Sample) float64 { } return sumSquares } -func sumSquaresTimer(t metrics.Timer) float64 { +func sumSquaresTimer(t metrics.TimerSnapshot) float64 { count := float64(t.Count()) sumSquared := math.Pow(count*t.Mean(), 2) sumSquares := math.Pow(count*t.StdDev(), 2) + sumSquared/count @@ -97,9 +97,10 @@ func (rep *Reporter) BuildRequest(now time.Time, r metrics.Registry) (snapshot B measurement[Period] = rep.Interval.Seconds() switch m := metric.(type) { case metrics.Counter: + ms := m.Snapshot() if m.Count() > 0 { measurement[Name] = fmt.Sprintf("%s.%s", name, "count") - measurement[Value] = float64(m.Count()) + measurement[Value] = float64(ms.Count()) measurement[Attributes] = map[string]interface{}{ DisplayUnitsLong: Operations, DisplayUnitsShort: OperationsShort, @@ -108,9 +109,9 @@ func (rep *Reporter) BuildRequest(now time.Time, r metrics.Registry) (snapshot B snapshot.Counters = append(snapshot.Counters, measurement) } case metrics.CounterFloat64: - if m.Count() > 0 { + if count := m.Snapshot().Count(); count > 0 { measurement[Name] = fmt.Sprintf("%s.%s", name, "count") - measurement[Value] = m.Count() + measurement[Value] = count measurement[Attributes] = map[string]interface{}{ DisplayUnitsLong: Operations, DisplayUnitsShort: OperationsShort, @@ -120,20 +121,21 @@ func (rep *Reporter) BuildRequest(now time.Time, r metrics.Registry) (snapshot B } case metrics.Gauge: measurement[Name] = name - measurement[Value] = float64(m.Value()) + measurement[Value] = float64(m.Snapshot().Value()) snapshot.Gauges = append(snapshot.Gauges, measurement) case metrics.GaugeFloat64: measurement[Name] = name - measurement[Value] = m.Value() + measurement[Value] = m.Snapshot().Value() snapshot.Gauges = append(snapshot.Gauges, measurement) case metrics.GaugeInfo: measurement[Name] = name measurement[Value] = m.Value() snapshot.Gauges = append(snapshot.Gauges, measurement) case metrics.Histogram: - if m.Count() > 0 { + ms := m.Snapshot() + if ms.Count() > 0 { gauges := make([]Measurement, histogramGaugeCount) - s := m.Sample() + s := ms.Sample() measurement[Name] = fmt.Sprintf("%s.%s", name, "hist") measurement[Count] = uint64(s.Count()) measurement[Max] = float64(s.Max()) @@ -151,13 +153,14 @@ func (rep *Reporter) BuildRequest(now time.Time, r metrics.Registry) (snapshot B snapshot.Gauges = append(snapshot.Gauges, gauges...) } case metrics.Meter: + ms := m.Snapshot() measurement[Name] = name - measurement[Value] = float64(m.Count()) + measurement[Value] = float64(ms.Count()) snapshot.Counters = append(snapshot.Counters, measurement) snapshot.Gauges = append(snapshot.Gauges, Measurement{ Name: fmt.Sprintf("%s.%s", name, "1min"), - Value: m.Rate1(), + Value: ms.Rate1(), Period: int64(rep.Interval.Seconds()), Attributes: map[string]interface{}{ DisplayUnitsLong: Operations, @@ -167,7 +170,7 @@ func (rep *Reporter) BuildRequest(now time.Time, r metrics.Registry) (snapshot B }, Measurement{ Name: fmt.Sprintf("%s.%s", name, "5min"), - Value: m.Rate5(), + Value: ms.Rate5(), Period: int64(rep.Interval.Seconds()), Attributes: map[string]interface{}{ DisplayUnitsLong: Operations, @@ -177,7 +180,7 @@ func (rep *Reporter) BuildRequest(now time.Time, r metrics.Registry) (snapshot B }, Measurement{ Name: fmt.Sprintf("%s.%s", name, "15min"), - Value: m.Rate15(), + Value: ms.Rate15(), Period: int64(rep.Interval.Seconds()), Attributes: map[string]interface{}{ DisplayUnitsLong: Operations, @@ -187,26 +190,27 @@ func (rep *Reporter) BuildRequest(now time.Time, r metrics.Registry) (snapshot B }, ) case metrics.Timer: + ms := m.Snapshot() measurement[Name] = name - measurement[Value] = float64(m.Count()) + measurement[Value] = float64(ms.Count()) snapshot.Counters = append(snapshot.Counters, measurement) - if m.Count() > 0 { + if ms.Count() > 0 { libratoName := fmt.Sprintf("%s.%s", name, "timer.mean") gauges := make([]Measurement, histogramGaugeCount) gauges[0] = Measurement{ Name: libratoName, - Count: uint64(m.Count()), - Sum: m.Mean() * float64(m.Count()), - Max: float64(m.Max()), - Min: float64(m.Min()), - SumSquares: sumSquaresTimer(m), + Count: uint64(ms.Count()), + Sum: ms.Mean() * float64(ms.Count()), + Max: float64(ms.Max()), + Min: float64(ms.Min()), + SumSquares: sumSquaresTimer(ms), Period: int64(rep.Interval.Seconds()), Attributes: rep.TimerAttributes, } for i, p := range rep.Percentiles { gauges[i+1] = Measurement{ Name: fmt.Sprintf("%s.timer.%2.0f", name, p*100), - Value: m.Percentile(p), + Value: ms.Percentile(p), Period: int64(rep.Interval.Seconds()), Attributes: rep.TimerAttributes, } @@ -215,7 +219,7 @@ func (rep *Reporter) BuildRequest(now time.Time, r metrics.Registry) (snapshot B snapshot.Gauges = append(snapshot.Gauges, Measurement{ Name: fmt.Sprintf("%s.%s", name, "rate.1min"), - Value: m.Rate1(), + Value: ms.Rate1(), Period: int64(rep.Interval.Seconds()), Attributes: map[string]interface{}{ DisplayUnitsLong: Operations, @@ -225,7 +229,7 @@ func (rep *Reporter) BuildRequest(now time.Time, r metrics.Registry) (snapshot B }, Measurement{ Name: fmt.Sprintf("%s.%s", name, "rate.5min"), - Value: m.Rate5(), + Value: ms.Rate5(), Period: int64(rep.Interval.Seconds()), Attributes: map[string]interface{}{ DisplayUnitsLong: Operations, @@ -235,7 +239,7 @@ func (rep *Reporter) BuildRequest(now time.Time, r metrics.Registry) (snapshot B }, Measurement{ Name: fmt.Sprintf("%s.%s", name, "rate.15min"), - Value: m.Rate15(), + Value: ms.Rate15(), Period: int64(rep.Interval.Seconds()), Attributes: map[string]interface{}{ DisplayUnitsLong: Operations, diff --git a/metrics/log.go b/metrics/log.go index d71a1c3d9c66..1ef078ca9419 100644 --- a/metrics/log.go +++ b/metrics/log.go @@ -29,7 +29,7 @@ func LogScaled(r Registry, freq time.Duration, scale time.Duration, l Logger) { l.Printf(" count: %f\n", metric.Count()) case Gauge: l.Printf("gauge %s\n", name) - l.Printf(" value: %9d\n", metric.Value()) + l.Printf(" value: %9d\n", metric.Snapshot().Value()) case GaugeFloat64: l.Printf("gauge %s\n", name) l.Printf(" value: %f\n", metric.Value()) diff --git a/metrics/meter.go b/metrics/meter.go index 6eff96e675a2..28c86f35852c 100644 --- a/metrics/meter.go +++ b/metrics/meter.go @@ -123,9 +123,9 @@ func (NilMeter) Stop() {} // StandardMeter is the standard implementation of a Meter. type StandardMeter struct { - count atomic.Int64 - temp atomic.Int64 - rateMean atomic.Uint64 + count atomic.Int64 + uncounted atomic.Int64 // not yet added to the EWMAs + rateMean atomic.Uint64 a1, a5, a15 EWMA startTime time.Time @@ -152,13 +152,13 @@ func (m *StandardMeter) Stop() { // Mark records the occurrence of n events. func (m *StandardMeter) Mark(n int64) { - m.temp.Add(n) + m.uncounted.Add(n) } // Snapshot returns a read-only copy of the meter. func (m *StandardMeter) Snapshot() MeterSnapshot { return &meterSnapshot{ - count: m.count.Load(), + count: m.count.Load() + m.uncounted.Load(), rate1: m.a1.Rate(), rate5: m.a5.Rate(), rate15: m.a15.Rate(), @@ -168,7 +168,7 @@ func (m *StandardMeter) Snapshot() MeterSnapshot { func (m *StandardMeter) tick() { // Take the uncounted values, add to count - n := m.temp.Swap(0) + n := m.uncounted.Swap(0) count := m.count.Add(n) m.rateMean.Store(math.Float64bits(float64(count) / time.Since(m.startTime).Seconds())) // Update the EWMA's internal state diff --git a/metrics/meter_test.go b/metrics/meter_test.go index b3f6cb8c0c97..019c4d765b52 100644 --- a/metrics/meter_test.go +++ b/metrics/meter_test.go @@ -12,11 +12,17 @@ func BenchmarkMeter(b *testing.B) { m.Mark(1) } } - +func TestMeter(t *testing.T) { + m := NewMeter() + m.Mark(47) + if v := m.Snapshot().Count(); v != 47 { + t.Fatalf("have %d want %d", v, 47) + } +} func TestGetOrRegisterMeter(t *testing.T) { r := NewRegistry() NewRegisteredMeter("foo", r).Mark(47) - if m := GetOrRegisterMeter("foo", r); m.Count() != 47 { + if m := GetOrRegisterMeter("foo", r).Snapshot(); m.Count() != 47 { t.Fatal(m.Count()) } } @@ -31,10 +37,10 @@ func TestMeterDecay(t *testing.T) { ma.meters[m] = struct{}{} m.Mark(1) ma.tickMeters() - rateMean := m.RateMean() + rateMean := m.Snapshot().RateMean() time.Sleep(100 * time.Millisecond) ma.tickMeters() - if m.RateMean() >= rateMean { + if m.Snapshot().RateMean() >= rateMean { t.Error("m.RateMean() didn't decrease") } } @@ -42,7 +48,7 @@ func TestMeterDecay(t *testing.T) { func TestMeterNonzero(t *testing.T) { m := NewMeter() m.Mark(3) - if count := m.Count(); count != 3 { + if count := m.Snapshot().Count(); count != 3 { t.Errorf("m.Count(): 3 != %v\n", count) } } @@ -59,16 +65,8 @@ func TestMeterStop(t *testing.T) { } } -func TestMeterSnapshot(t *testing.T) { - m := NewMeter() - m.Mark(1) - if snapshot := m.Snapshot(); m.RateMean() != snapshot.RateMean() { - t.Fatal(snapshot) - } -} - func TestMeterZero(t *testing.T) { - m := NewMeter() + m := NewMeter().Snapshot() if count := m.Count(); count != 0 { t.Errorf("m.Count(): 0 != %v\n", count) } @@ -79,13 +77,13 @@ func TestMeterRepeat(t *testing.T) { for i := 0; i < 101; i++ { m.Mark(int64(i)) } - if count := m.Count(); count != 5050 { + if count := m.Snapshot().Count(); count != 5050 { t.Errorf("m.Count(): 5050 != %v\n", count) } for i := 0; i < 101; i++ { m.Mark(int64(i)) } - if count := m.Count(); count != 10100 { + if count := m.Snapshot().Count(); count != 10100 { t.Errorf("m.Count(): 10100 != %v\n", count) } } diff --git a/metrics/metrics_test.go b/metrics/metrics_test.go index 534c44139b36..4a1a07e6f1ad 100644 --- a/metrics/metrics_test.go +++ b/metrics/metrics_test.go @@ -99,7 +99,7 @@ func Example() { t.Update(1) fmt.Println(c.Count()) - fmt.Println(t.Min()) + fmt.Println(t.Snapshot().Min()) // Output: 17 // 1 } diff --git a/metrics/opentsdb.go b/metrics/opentsdb.go index 4d2e209238fa..8f9be1debf20 100644 --- a/metrics/opentsdb.go +++ b/metrics/opentsdb.go @@ -69,7 +69,7 @@ func (c *OpenTSDBConfig) writeRegistry(w io.Writer, now int64, shortHostname str case CounterFloat64: fmt.Fprintf(w, "put %s.%s.count %d %f host=%s\n", c.Prefix, name, now, metric.Count(), shortHostname) case Gauge: - fmt.Fprintf(w, "put %s.%s.value %d %d host=%s\n", c.Prefix, name, now, metric.Value(), shortHostname) + fmt.Fprintf(w, "put %s.%s.value %d %d host=%s\n", c.Prefix, name, now, metric.Snapshot().Value(), shortHostname) case GaugeFloat64: fmt.Fprintf(w, "put %s.%s.value %d %f host=%s\n", c.Prefix, name, now, metric.Value(), shortHostname) case GaugeInfo: diff --git a/metrics/prometheus/collector.go b/metrics/prometheus/collector.go index 90aa37f33811..831431e0d05e 100644 --- a/metrics/prometheus/collector.go +++ b/metrics/prometheus/collector.go @@ -83,7 +83,7 @@ func (c *collector) addCounterFloat64(name string, m metrics.CounterFloat64) { c.writeGaugeCounter(name, m.Count()) } -func (c *collector) addGauge(name string, m metrics.Gauge) { +func (c *collector) addGauge(name string, m metrics.GaugeSnapshot) { c.writeGaugeCounter(name, m.Value()) } diff --git a/metrics/registry.go b/metrics/registry.go index 66dbc890c064..85f1b82414b1 100644 --- a/metrics/registry.go +++ b/metrics/registry.go @@ -154,7 +154,7 @@ func (r *StandardRegistry) GetAll() map[string]map[string]interface{} { case CounterFloat64: values["count"] = metric.Count() case Gauge: - values["value"] = metric.Value() + values["value"] = metric.Snapshot().Value() case GaugeFloat64: values["value"] = metric.Value() case Healthcheck: diff --git a/metrics/sample.go b/metrics/sample.go index 42d3b619561a..d52a92c06829 100644 --- a/metrics/sample.go +++ b/metrics/sample.go @@ -353,8 +353,9 @@ func (s *UniformSample) Snapshot() SampleSnapshot { s.mutex.Lock() values := make([]int64, len(s.values)) copy(values, s.values) + count := s.count s.mutex.Unlock() - return newSampleSnapshot(s.count, s.values) + return newSampleSnapshot(count, values) } // Update samples a new value. diff --git a/metrics/sample_test.go b/metrics/sample_test.go index 018358d729dc..a57aba78681d 100644 --- a/metrics/sample_test.go +++ b/metrics/sample_test.go @@ -87,10 +87,11 @@ func BenchmarkUniformSample1028(b *testing.B) { } func TestExpDecaySample10(t *testing.T) { - s := NewExpDecaySample(100, 0.99) + sw := NewExpDecaySample(100, 0.99) for i := 0; i < 10; i++ { - s.Update(int64(i)) + sw.Update(int64(i)) } + s := sw.Snapshot() if size := s.Count(); size != 10 { t.Errorf("s.Count(): 10 != %v\n", size) } @@ -108,10 +109,11 @@ func TestExpDecaySample10(t *testing.T) { } func TestExpDecaySample100(t *testing.T) { - s := NewExpDecaySample(1000, 0.01) + sw := NewExpDecaySample(1000, 0.01) for i := 0; i < 100; i++ { - s.Update(int64(i)) + sw.Update(int64(i)) } + s := sw.Snapshot() if size := s.Count(); size != 100 { t.Errorf("s.Count(): 100 != %v\n", size) } @@ -129,10 +131,11 @@ func TestExpDecaySample100(t *testing.T) { } func TestExpDecaySample1000(t *testing.T) { - s := NewExpDecaySample(100, 0.99) + sw := NewExpDecaySample(100, 0.99) for i := 0; i < 1000; i++ { - s.Update(int64(i)) + sw.Update(int64(i)) } + s := sw.Snapshot() if size := s.Count(); size != 1000 { t.Errorf("s.Count(): 1000 != %v\n", size) } @@ -154,14 +157,15 @@ func TestExpDecaySample1000(t *testing.T) { // The priority becomes +Inf quickly after starting if this is done, // effectively freezing the set of samples until a rescale step happens. func TestExpDecaySampleNanosecondRegression(t *testing.T) { - s := NewExpDecaySample(100, 0.99) + sw := NewExpDecaySample(100, 0.99) for i := 0; i < 100; i++ { - s.Update(10) + sw.Update(10) } time.Sleep(1 * time.Millisecond) for i := 0; i < 100; i++ { - s.Update(20) + sw.Update(20) } + s := sw.Snapshot() v := s.Values() avg := float64(0) for i := 0; i < len(v); i++ { @@ -201,14 +205,15 @@ func TestExpDecaySampleStatistics(t *testing.T) { for i := 1; i <= 10000; i++ { s.(*ExpDecaySample).update(now.Add(time.Duration(i)), int64(i)) } - testExpDecaySampleStatistics(t, s) + testExpDecaySampleStatistics(t, s.Snapshot()) } func TestUniformSample(t *testing.T) { - s := NewUniformSample(100) + sw := NewUniformSample(100) for i := 0; i < 1000; i++ { - s.Update(int64(i)) + sw.Update(int64(i)) } + s := sw.Snapshot() if size := s.Count(); size != 1000 { t.Errorf("s.Count(): 1000 != %v\n", size) } @@ -226,11 +231,12 @@ func TestUniformSample(t *testing.T) { } func TestUniformSampleIncludesTail(t *testing.T) { - s := NewUniformSample(100) + sw := NewUniformSample(100) max := 100 for i := 0; i < max; i++ { - s.Update(int64(i)) + sw.Update(int64(i)) } + s := sw.Snapshot() v := s.Values() sum := 0 exp := (max - 1) * max / 2 @@ -257,7 +263,7 @@ func TestUniformSampleStatistics(t *testing.T) { for i := 1; i <= 10000; i++ { s.Update(int64(i)) } - testUniformSampleStatistics(t, s) + testUniformSampleStatistics(t, s.Snapshot()) } func benchmarkSample(b *testing.B, s Sample) { @@ -274,7 +280,7 @@ func benchmarkSample(b *testing.B, s Sample) { b.Logf("GC cost: %d ns/op", int(memStats.PauseTotalNs-pauseTotalNs)/b.N) } -func testExpDecaySampleStatistics(t *testing.T, s Sample) { +func testExpDecaySampleStatistics(t *testing.T, s SampleSnapshot) { if count := s.Count(); count != 10000 { t.Errorf("s.Count(): 10000 != %v\n", count) } @@ -302,7 +308,7 @@ func testExpDecaySampleStatistics(t *testing.T, s Sample) { } } -func testUniformSampleStatistics(t *testing.T, s Sample) { +func testUniformSampleStatistics(t *testing.T, s SampleSnapshot) { if count := s.Count(); count != 10000 { t.Errorf("s.Count(): 10000 != %v\n", count) } @@ -356,7 +362,7 @@ func TestUniformSampleConcurrentUpdateCount(t *testing.T) { } }() for i := 0; i < 1000; i++ { - s.Count() + s.Snapshot().Count() time.Sleep(5 * time.Millisecond) } quit <- struct{}{} diff --git a/metrics/syslog.go b/metrics/syslog.go index 76c849056757..949ef1df2f19 100644 --- a/metrics/syslog.go +++ b/metrics/syslog.go @@ -20,7 +20,7 @@ func Syslog(r Registry, d time.Duration, w *syslog.Writer) { case CounterFloat64: w.Info(fmt.Sprintf("counter %s: count: %f", name, metric.Count())) case Gauge: - w.Info(fmt.Sprintf("gauge %s: value: %d", name, metric.Value())) + w.Info(fmt.Sprintf("gauge %s: value: %d", name, metric.Snapshot().Value())) case GaugeFloat64: w.Info(fmt.Sprintf("gauge %s: value: %f", name, metric.Value())) case GaugeInfo: diff --git a/metrics/timer_test.go b/metrics/timer_test.go index 903e8e8d496e..f10de16c9c23 100644 --- a/metrics/timer_test.go +++ b/metrics/timer_test.go @@ -18,7 +18,7 @@ func BenchmarkTimer(b *testing.B) { func TestGetOrRegisterTimer(t *testing.T) { r := NewRegistry() NewRegisteredTimer("foo", r).Update(47) - if tm := GetOrRegisterTimer("foo", r); tm.Count() != 1 { + if tm := GetOrRegisterTimer("foo", r).Snapshot(); tm.Count() != 1 { t.Fatal(tm) } } @@ -27,7 +27,7 @@ func TestTimerExtremes(t *testing.T) { tm := NewTimer() tm.Update(math.MaxInt64) tm.Update(0) - if stdDev := tm.StdDev(); stdDev != 4.611686018427388e+18 { + if stdDev := tm.Snapshot().StdDev(); stdDev != 4.611686018427388e+18 { t.Errorf("tm.StdDev(): 4.611686018427388e+18 != %v\n", stdDev) } } @@ -56,7 +56,7 @@ func TestTimerFunc(t *testing.T) { }) var ( drift = time.Millisecond * 2 - measured = time.Duration(tm.Max()) + measured = time.Duration(tm.Snapshot().Max()) ceil = actualTime + drift floor = actualTime - drift ) @@ -66,7 +66,7 @@ func TestTimerFunc(t *testing.T) { } func TestTimerZero(t *testing.T) { - tm := NewTimer() + tm := NewTimer().Snapshot() if count := tm.Count(); count != 0 { t.Errorf("tm.Count(): 0 != %v\n", count) } @@ -110,5 +110,5 @@ func ExampleGetOrRegisterTimer() { m := "account.create.latency" t := GetOrRegisterTimer(m, nil) t.Update(47) - fmt.Println(t.Max()) // Output: 47 + fmt.Println(t.Snapshot().Max()) // Output: 47 } diff --git a/metrics/writer.go b/metrics/writer.go index ec2e4f8c6a60..9dfe3327f676 100644 --- a/metrics/writer.go +++ b/metrics/writer.go @@ -35,7 +35,7 @@ func WriteOnce(r Registry, w io.Writer) { fmt.Fprintf(w, " count: %f\n", metric.Count()) case Gauge: fmt.Fprintf(w, "gauge %s\n", namedMetric.name) - fmt.Fprintf(w, " value: %9d\n", metric.Value()) + fmt.Fprintf(w, " value: %9d\n", metric.Snapshot().Value()) case GaugeFloat64: fmt.Fprintf(w, "gauge %s\n", namedMetric.name) fmt.Fprintf(w, " value: %f\n", metric.Value()) From 36c31a691ac1d186eb032c0b4f5d575782ff0d28 Mon Sep 17 00:00:00 2001 From: Martin Holst Swende Date: Thu, 31 Aug 2023 11:56:08 +0200 Subject: [PATCH 07/26] metrics: split gaugefloat64 interface --- metrics/exp/exp.go | 4 +- metrics/gauge_float64.go | 71 ++++++--------------------------- metrics/gauge_float64_test.go | 33 +-------------- metrics/graphite.go | 2 +- metrics/log.go | 2 +- metrics/opentsdb.go | 2 +- metrics/opentsdb_test.go | 15 +++++++ metrics/prometheus/collector.go | 2 +- metrics/registry.go | 2 +- metrics/syslog.go | 2 +- metrics/testdata/opentsb.want | 2 +- metrics/writer.go | 2 +- 12 files changed, 40 insertions(+), 99 deletions(-) diff --git a/metrics/exp/exp.go b/metrics/exp/exp.go index 834a27a44aa1..89e49a5d36fb 100644 --- a/metrics/exp/exp.go +++ b/metrics/exp/exp.go @@ -123,7 +123,7 @@ func (exp *exp) publishGauge(name string, metric metrics.GaugeSnapshot) { v := exp.getInt(name) v.Set(metric.Value()) } -func (exp *exp) publishGaugeFloat64(name string, metric metrics.GaugeFloat64) { +func (exp *exp) publishGaugeFloat64(name string, metric metrics.GaugeFloat64Snapshot) { exp.getFloat(name).Set(metric.Value()) } @@ -195,7 +195,7 @@ func (exp *exp) syncToExpvar() { case metrics.Gauge: exp.publishGauge(name, i.Snapshot()) case metrics.GaugeFloat64: - exp.publishGaugeFloat64(name, i) + exp.publishGaugeFloat64(name, i.Snapshot()) case metrics.GaugeInfo: exp.publishGaugeInfo(name, i) case metrics.Histogram: diff --git a/metrics/gauge_float64.go b/metrics/gauge_float64.go index 237ff8036e01..2f509b1cec8c 100644 --- a/metrics/gauge_float64.go +++ b/metrics/gauge_float64.go @@ -5,11 +5,14 @@ import ( "sync/atomic" ) -// GaugeFloat64s hold a float64 value that can be set arbitrarily. +type GaugeFloat64Snapshot interface { + Value() float64 +} + +// GaugeFloat64 hold a float64 value that can be set arbitrarily. type GaugeFloat64 interface { - Snapshot() GaugeFloat64 + Snapshot() GaugeFloat64Snapshot Update(float64) - Value() float64 } // GetOrRegisterGaugeFloat64 returns an existing GaugeFloat64 or constructs and registers a @@ -39,43 +42,17 @@ func NewRegisteredGaugeFloat64(name string, r Registry) GaugeFloat64 { return c } -// NewFunctionalGauge constructs a new FunctionalGauge. -func NewFunctionalGaugeFloat64(f func() float64) GaugeFloat64 { - if !Enabled { - return NilGaugeFloat64{} - } - return &FunctionalGaugeFloat64{value: f} -} - -// NewRegisteredFunctionalGauge constructs and registers a new StandardGauge. -func NewRegisteredFunctionalGaugeFloat64(name string, r Registry, f func() float64) GaugeFloat64 { - c := NewFunctionalGaugeFloat64(f) - if nil == r { - r = DefaultRegistry - } - r.Register(name, c) - return c -} - -// GaugeFloat64Snapshot is a read-only copy of another GaugeFloat64. -type GaugeFloat64Snapshot float64 - -// Snapshot returns the snapshot. -func (g GaugeFloat64Snapshot) Snapshot() GaugeFloat64 { return g } - -// Update panics. -func (GaugeFloat64Snapshot) Update(float64) { - panic("Update called on a GaugeFloat64Snapshot") -} +// gaugeFloat64Snapshot is a read-only copy of another GaugeFloat64. +type gaugeFloat64Snapshot float64 // Value returns the value at the time the snapshot was taken. -func (g GaugeFloat64Snapshot) Value() float64 { return float64(g) } +func (g gaugeFloat64Snapshot) Value() float64 { return float64(g) } // NilGauge is a no-op Gauge. type NilGaugeFloat64 struct{} // Snapshot is a no-op. -func (NilGaugeFloat64) Snapshot() GaugeFloat64 { return NilGaugeFloat64{} } +func (NilGaugeFloat64) Snapshot() GaugeFloat64Snapshot { return NilGaugeFloat64{} } // Update is a no-op. func (NilGaugeFloat64) Update(v float64) {} @@ -90,34 +67,12 @@ type StandardGaugeFloat64 struct { } // Snapshot returns a read-only copy of the gauge. -func (g *StandardGaugeFloat64) Snapshot() GaugeFloat64 { - return GaugeFloat64Snapshot(g.Value()) +func (g *StandardGaugeFloat64) Snapshot() GaugeFloat64Snapshot { + v := math.Float64frombits(g.floatBits.Load()) + return gaugeFloat64Snapshot(v) } // Update updates the gauge's value. func (g *StandardGaugeFloat64) Update(v float64) { g.floatBits.Store(math.Float64bits(v)) } - -// Value returns the gauge's current value. -func (g *StandardGaugeFloat64) Value() float64 { - return math.Float64frombits(g.floatBits.Load()) -} - -// FunctionalGaugeFloat64 returns value from given function -type FunctionalGaugeFloat64 struct { - value func() float64 -} - -// Value returns the gauge's current value. -func (g FunctionalGaugeFloat64) Value() float64 { - return g.value() -} - -// Snapshot returns the snapshot. -func (g FunctionalGaugeFloat64) Snapshot() GaugeFloat64 { return GaugeFloat64Snapshot(g.Value()) } - -// Update panics. -func (FunctionalGaugeFloat64) Update(float64) { - panic("Update called on a FunctionalGaugeFloat64") -} diff --git a/metrics/gauge_float64_test.go b/metrics/gauge_float64_test.go index 647d09000935..f0ac7ea5e7be 100644 --- a/metrics/gauge_float64_test.go +++ b/metrics/gauge_float64_test.go @@ -26,19 +26,11 @@ func BenchmarkGaugeFloat64Parallel(b *testing.B) { }() } wg.Wait() - if have, want := c.Value(), float64(b.N-1); have != want { + if have, want := c.Snapshot().Value(), float64(b.N-1); have != want { b.Fatalf("have %f want %f", have, want) } } -func TestGaugeFloat64(t *testing.T) { - g := NewGaugeFloat64() - g.Update(47.0) - if v := g.Value(); 47.0 != v { - t.Errorf("g.Value(): 47.0 != %v\n", v) - } -} - func TestGaugeFloat64Snapshot(t *testing.T) { g := NewGaugeFloat64() g.Update(47.0) @@ -53,28 +45,7 @@ func TestGetOrRegisterGaugeFloat64(t *testing.T) { r := NewRegistry() NewRegisteredGaugeFloat64("foo", r).Update(47.0) t.Logf("registry: %v", r) - if g := GetOrRegisterGaugeFloat64("foo", r); 47.0 != g.Value() { - t.Fatal(g) - } -} - -func TestFunctionalGaugeFloat64(t *testing.T) { - var counter float64 - fg := NewFunctionalGaugeFloat64(func() float64 { - counter++ - return counter - }) - fg.Value() - fg.Value() - if counter != 2 { - t.Error("counter != 2") - } -} - -func TestGetOrRegisterFunctionalGaugeFloat64(t *testing.T) { - r := NewRegistry() - NewRegisteredFunctionalGaugeFloat64("foo", r, func() float64 { return 47 }) - if g := GetOrRegisterGaugeFloat64("foo", r); g.Value() != 47 { + if g := GetOrRegisterGaugeFloat64("foo", r).Snapshot(); 47.0 != g.Value() { t.Fatal(g) } } diff --git a/metrics/graphite.go b/metrics/graphite.go index d9cb8b533ccd..d1a4cc0fa0eb 100644 --- a/metrics/graphite.go +++ b/metrics/graphite.go @@ -72,7 +72,7 @@ func graphite(c *GraphiteConfig) error { case Gauge: fmt.Fprintf(w, "%s.%s.value %d %d\n", c.Prefix, name, metric.Snapshot().Value(), now) case GaugeFloat64: - fmt.Fprintf(w, "%s.%s.value %f %d\n", c.Prefix, name, metric.Value(), now) + fmt.Fprintf(w, "%s.%s.value %f %d\n", c.Prefix, name, metric.Snapshot().Value(), now) case GaugeInfo: fmt.Fprintf(w, "%s.%s.value %s %d\n", c.Prefix, name, metric.Value().String(), now) case Histogram: diff --git a/metrics/log.go b/metrics/log.go index 1ef078ca9419..c9938dd1866d 100644 --- a/metrics/log.go +++ b/metrics/log.go @@ -32,7 +32,7 @@ func LogScaled(r Registry, freq time.Duration, scale time.Duration, l Logger) { l.Printf(" value: %9d\n", metric.Snapshot().Value()) case GaugeFloat64: l.Printf("gauge %s\n", name) - l.Printf(" value: %f\n", metric.Value()) + l.Printf(" value: %f\n", metric.Snapshot().Value()) case GaugeInfo: l.Printf("gauge %s\n", name) l.Printf(" value: %s\n", metric.Value()) diff --git a/metrics/opentsdb.go b/metrics/opentsdb.go index 8f9be1debf20..4b1b4902e609 100644 --- a/metrics/opentsdb.go +++ b/metrics/opentsdb.go @@ -71,7 +71,7 @@ func (c *OpenTSDBConfig) writeRegistry(w io.Writer, now int64, shortHostname str case Gauge: fmt.Fprintf(w, "put %s.%s.value %d %d host=%s\n", c.Prefix, name, now, metric.Snapshot().Value(), shortHostname) case GaugeFloat64: - fmt.Fprintf(w, "put %s.%s.value %d %f host=%s\n", c.Prefix, name, now, metric.Value(), shortHostname) + fmt.Fprintf(w, "put %s.%s.value %d %f host=%s\n", c.Prefix, name, now, metric.Snapshot().Value(), shortHostname) case GaugeInfo: fmt.Fprintf(w, "put %s.%s.value %d %s host=%s\n", c.Prefix, name, now, metric.Value().String(), shortHostname) case Histogram: diff --git a/metrics/opentsdb_test.go b/metrics/opentsdb_test.go index c02b98af061e..4548309f9c23 100644 --- a/metrics/opentsdb_test.go +++ b/metrics/opentsdb_test.go @@ -1,6 +1,7 @@ package metrics import ( + "fmt" "net" "os" "strings" @@ -47,5 +48,19 @@ func TestExampleOpenTSB(t *testing.T) { } if have, want := w.String(), string(wantB); have != want { t.Errorf("\nhave:\n%v\nwant:\n%v\n", have, want) + t.Logf("have vs want:\n%v", findFirstDiffPos(have, want)) } } + +func findFirstDiffPos(a, b string) string { + yy := strings.Split(b, "\n") + for i, x := range strings.Split(a, "\n") { + if i >= len(yy) { + return fmt.Sprintf("have:%d: %s\nwant:%d: ", i, x, i) + } + if y := yy[i]; x != y { + return fmt.Sprintf("have:%d: %s\nwant:%d: %s", i, x, i, y) + } + } + return "" +} diff --git a/metrics/prometheus/collector.go b/metrics/prometheus/collector.go index 831431e0d05e..28ffb218e1cd 100644 --- a/metrics/prometheus/collector.go +++ b/metrics/prometheus/collector.go @@ -87,7 +87,7 @@ func (c *collector) addGauge(name string, m metrics.GaugeSnapshot) { c.writeGaugeCounter(name, m.Value()) } -func (c *collector) addGaugeFloat64(name string, m metrics.GaugeFloat64) { +func (c *collector) addGaugeFloat64(name string, m metrics.GaugeFloat64Snapshot) { c.writeGaugeCounter(name, m.Value()) } diff --git a/metrics/registry.go b/metrics/registry.go index 85f1b82414b1..cfe0f76240d7 100644 --- a/metrics/registry.go +++ b/metrics/registry.go @@ -156,7 +156,7 @@ func (r *StandardRegistry) GetAll() map[string]map[string]interface{} { case Gauge: values["value"] = metric.Snapshot().Value() case GaugeFloat64: - values["value"] = metric.Value() + values["value"] = metric.Snapshot().Value() case Healthcheck: values["error"] = nil metric.Check() diff --git a/metrics/syslog.go b/metrics/syslog.go index 949ef1df2f19..ecc5fe960842 100644 --- a/metrics/syslog.go +++ b/metrics/syslog.go @@ -22,7 +22,7 @@ func Syslog(r Registry, d time.Duration, w *syslog.Writer) { case Gauge: w.Info(fmt.Sprintf("gauge %s: value: %d", name, metric.Snapshot().Value())) case GaugeFloat64: - w.Info(fmt.Sprintf("gauge %s: value: %f", name, metric.Value())) + w.Info(fmt.Sprintf("gauge %s: value: %f", name, metric.Snapshot().Value())) case GaugeInfo: w.Info(fmt.Sprintf("gauge %s: value: %s", name, metric.Value())) case Healthcheck: diff --git a/metrics/testdata/opentsb.want b/metrics/testdata/opentsb.want index c8e40a525042..43fe1b2ac27a 100644 --- a/metrics/testdata/opentsb.want +++ b/metrics/testdata/opentsb.want @@ -1,4 +1,4 @@ -put pre.elite.count 978307200 0 host=hal9000 +put pre.elite.count 978307200 1337 host=hal9000 put pre.elite.one-minute 978307200 0.00 host=hal9000 put pre.elite.five-minute 978307200 0.00 host=hal9000 put pre.elite.fifteen-minute 978307200 0.00 host=hal9000 diff --git a/metrics/writer.go b/metrics/writer.go index 9dfe3327f676..d352dc25d372 100644 --- a/metrics/writer.go +++ b/metrics/writer.go @@ -38,7 +38,7 @@ func WriteOnce(r Registry, w io.Writer) { fmt.Fprintf(w, " value: %9d\n", metric.Snapshot().Value()) case GaugeFloat64: fmt.Fprintf(w, "gauge %s\n", namedMetric.name) - fmt.Fprintf(w, " value: %f\n", metric.Value()) + fmt.Fprintf(w, " value: %f\n", metric.Snapshot().Value()) case GaugeInfo: fmt.Fprintf(w, "gauge %s\n", namedMetric.name) fmt.Fprintf(w, " value: %s\n", metric.Value().String()) From 7b9dfc72c164fcff8d4560b4f206185057783088 Mon Sep 17 00:00:00 2001 From: Martin Holst Swende Date: Thu, 31 Aug 2023 12:15:20 +0200 Subject: [PATCH 08/26] metrics: split up counter, counterfloat64 --- metrics/counter.go | 56 +++++++++-------------------- metrics/counter_float64.go | 61 +++++++++----------------------- metrics/counter_float_64_test.go | 16 ++++----- metrics/counter_test.go | 14 ++++---- metrics/exp/exp.go | 8 ++--- metrics/graphite.go | 4 +-- metrics/influxdb/influxdb.go | 4 +-- metrics/log.go | 4 +-- metrics/metrics_test.go | 2 +- metrics/opentsdb.go | 4 +-- metrics/prometheus/collector.go | 4 +-- metrics/registry.go | 4 +-- metrics/registry_test.go | 4 +-- metrics/syslog.go | 4 +-- metrics/writer.go | 4 +-- 15 files changed, 71 insertions(+), 122 deletions(-) diff --git a/metrics/counter.go b/metrics/counter.go index 55e1c59540f6..ada642eeec24 100644 --- a/metrics/counter.go +++ b/metrics/counter.go @@ -4,13 +4,16 @@ import ( "sync/atomic" ) +type CounterSnapshot interface { + Count() int64 +} + // Counters hold an int64 value that can be incremented and decremented. type Counter interface { Clear() - Count() int64 Dec(int64) Inc(int64) - Snapshot() Counter + Snapshot() CounterSnapshot } // GetOrRegisterCounter returns an existing Counter or constructs and registers @@ -38,13 +41,13 @@ func NewCounter() Counter { if !Enabled { return NilCounter{} } - return &StandardCounter{} + return new(StandardCounter) } // NewCounterForced constructs a new StandardCounter and returns it no matter if // the global switch is enabled or not. func NewCounterForced() Counter { - return &StandardCounter{} + return new(StandardCounter) } // NewRegisteredCounter constructs and registers a new StandardCounter. @@ -70,29 +73,11 @@ func NewRegisteredCounterForced(name string, r Registry) Counter { return c } -// CounterSnapshot is a read-only copy of another Counter. -type CounterSnapshot int64 - -// Clear panics. -func (CounterSnapshot) Clear() { - panic("Clear called on a CounterSnapshot") -} +// counterSnapshot is a read-only copy of another Counter. +type counterSnapshot int64 // Count returns the count at the time the snapshot was taken. -func (c CounterSnapshot) Count() int64 { return int64(c) } - -// Dec panics. -func (CounterSnapshot) Dec(int64) { - panic("Dec called on a CounterSnapshot") -} - -// Inc panics. -func (CounterSnapshot) Inc(int64) { - panic("Inc called on a CounterSnapshot") -} - -// Snapshot returns the snapshot. -func (c CounterSnapshot) Snapshot() Counter { return c } +func (c counterSnapshot) Count() int64 { return int64(c) } // NilCounter is a no-op Counter. type NilCounter struct{} @@ -110,35 +95,28 @@ func (NilCounter) Dec(i int64) {} func (NilCounter) Inc(i int64) {} // Snapshot is a no-op. -func (NilCounter) Snapshot() Counter { return NilCounter{} } +func (NilCounter) Snapshot() CounterSnapshot { return NilCounter{} } // StandardCounter is the standard implementation of a Counter and uses the // sync/atomic package to manage a single int64 value. -type StandardCounter struct { - count atomic.Int64 -} +type StandardCounter atomic.Int64 // Clear sets the counter to zero. func (c *StandardCounter) Clear() { - c.count.Store(0) -} - -// Count returns the current count. -func (c *StandardCounter) Count() int64 { - return c.count.Load() + (*atomic.Int64)(c).Store(0) } // Dec decrements the counter by the given amount. func (c *StandardCounter) Dec(i int64) { - c.count.Add(-i) + (*atomic.Int64)(c).Add(-i) } // Inc increments the counter by the given amount. func (c *StandardCounter) Inc(i int64) { - c.count.Add(i) + (*atomic.Int64)(c).Add(i) } // Snapshot returns a read-only copy of the counter. -func (c *StandardCounter) Snapshot() Counter { - return CounterSnapshot(c.Count()) +func (c *StandardCounter) Snapshot() CounterSnapshot { + return counterSnapshot((*atomic.Int64)(c).Load()) } diff --git a/metrics/counter_float64.go b/metrics/counter_float64.go index d1197bb8e0ae..15c81494efb8 100644 --- a/metrics/counter_float64.go +++ b/metrics/counter_float64.go @@ -5,13 +5,16 @@ import ( "sync/atomic" ) +type CounterFloat64Snapshot interface { + Count() float64 +} + // CounterFloat64 holds a float64 value that can be incremented and decremented. type CounterFloat64 interface { Clear() - Count() float64 Dec(float64) Inc(float64) - Snapshot() CounterFloat64 + Snapshot() CounterFloat64Snapshot } // GetOrRegisterCounterFloat64 returns an existing CounterFloat64 or constructs and registers @@ -71,47 +74,19 @@ func NewRegisteredCounterFloat64Forced(name string, r Registry) CounterFloat64 { return c } -// CounterFloat64Snapshot is a read-only copy of another CounterFloat64. -type CounterFloat64Snapshot float64 - -// Clear panics. -func (CounterFloat64Snapshot) Clear() { - panic("Clear called on a CounterFloat64Snapshot") -} +// counterFloat64Snapshot is a read-only copy of another CounterFloat64. +type counterFloat64Snapshot float64 // Count returns the value at the time the snapshot was taken. -func (c CounterFloat64Snapshot) Count() float64 { return float64(c) } - -// Dec panics. -func (CounterFloat64Snapshot) Dec(float64) { - panic("Dec called on a CounterFloat64Snapshot") -} +func (c counterFloat64Snapshot) Count() float64 { return float64(c) } -// Inc panics. -func (CounterFloat64Snapshot) Inc(float64) { - panic("Inc called on a CounterFloat64Snapshot") -} - -// Snapshot returns the snapshot. -func (c CounterFloat64Snapshot) Snapshot() CounterFloat64 { return c } - -// NilCounterFloat64 is a no-op CounterFloat64. type NilCounterFloat64 struct{} -// Clear is a no-op. -func (NilCounterFloat64) Clear() {} - -// Count is a no-op. -func (NilCounterFloat64) Count() float64 { return 0.0 } - -// Dec is a no-op. -func (NilCounterFloat64) Dec(i float64) {} - -// Inc is a no-op. -func (NilCounterFloat64) Inc(i float64) {} - -// Snapshot is a no-op. -func (NilCounterFloat64) Snapshot() CounterFloat64 { return NilCounterFloat64{} } +func (NilCounterFloat64) Clear() {} +func (NilCounterFloat64) Count() float64 { return 0.0 } +func (NilCounterFloat64) Dec(i float64) {} +func (NilCounterFloat64) Inc(i float64) {} +func (NilCounterFloat64) Snapshot() CounterFloat64Snapshot { return NilCounterFloat64{} } // StandardCounterFloat64 is the standard implementation of a CounterFloat64 and uses the // atomic to manage a single float64 value. @@ -124,11 +99,6 @@ func (c *StandardCounterFloat64) Clear() { c.floatBits.Store(0) } -// Count returns the current value. -func (c *StandardCounterFloat64) Count() float64 { - return math.Float64frombits(c.floatBits.Load()) -} - // Dec decrements the counter by the given amount. func (c *StandardCounterFloat64) Dec(v float64) { atomicAddFloat(&c.floatBits, -v) @@ -140,8 +110,9 @@ func (c *StandardCounterFloat64) Inc(v float64) { } // Snapshot returns a read-only copy of the counter. -func (c *StandardCounterFloat64) Snapshot() CounterFloat64 { - return CounterFloat64Snapshot(c.Count()) +func (c *StandardCounterFloat64) Snapshot() CounterFloat64Snapshot { + v := math.Float64frombits(c.floatBits.Load()) + return counterFloat64Snapshot(v) } func atomicAddFloat(fbits *atomic.Uint64, v float64) { diff --git a/metrics/counter_float_64_test.go b/metrics/counter_float_64_test.go index f17aca330cbe..c21bd3307fa1 100644 --- a/metrics/counter_float_64_test.go +++ b/metrics/counter_float_64_test.go @@ -27,7 +27,7 @@ func BenchmarkCounterFloat64Parallel(b *testing.B) { }() } wg.Wait() - if have, want := c.Count(), 10.0*float64(b.N); have != want { + if have, want := c.Snapshot().Count(), 10.0*float64(b.N); have != want { b.Fatalf("have %f want %f", have, want) } } @@ -36,7 +36,7 @@ func TestCounterFloat64Clear(t *testing.T) { c := NewCounterFloat64() c.Inc(1.0) c.Clear() - if count := c.Count(); count != 0 { + if count := c.Snapshot().Count(); count != 0 { t.Errorf("c.Count(): 0 != %v\n", count) } } @@ -44,7 +44,7 @@ func TestCounterFloat64Clear(t *testing.T) { func TestCounterFloat64Dec1(t *testing.T) { c := NewCounterFloat64() c.Dec(1.0) - if count := c.Count(); count != -1.0 { + if count := c.Snapshot().Count(); count != -1.0 { t.Errorf("c.Count(): -1.0 != %v\n", count) } } @@ -52,7 +52,7 @@ func TestCounterFloat64Dec1(t *testing.T) { func TestCounterFloat64Dec2(t *testing.T) { c := NewCounterFloat64() c.Dec(2.0) - if count := c.Count(); count != -2.0 { + if count := c.Snapshot().Count(); count != -2.0 { t.Errorf("c.Count(): -2.0 != %v\n", count) } } @@ -60,7 +60,7 @@ func TestCounterFloat64Dec2(t *testing.T) { func TestCounterFloat64Inc1(t *testing.T) { c := NewCounterFloat64() c.Inc(1.0) - if count := c.Count(); count != 1.0 { + if count := c.Snapshot().Count(); count != 1.0 { t.Errorf("c.Count(): 1.0 != %v\n", count) } } @@ -68,7 +68,7 @@ func TestCounterFloat64Inc1(t *testing.T) { func TestCounterFloat64Inc2(t *testing.T) { c := NewCounterFloat64() c.Inc(2.0) - if count := c.Count(); count != 2.0 { + if count := c.Snapshot().Count(); count != 2.0 { t.Errorf("c.Count(): 2.0 != %v\n", count) } } @@ -85,7 +85,7 @@ func TestCounterFloat64Snapshot(t *testing.T) { func TestCounterFloat64Zero(t *testing.T) { c := NewCounterFloat64() - if count := c.Count(); count != 0 { + if count := c.Snapshot().Count(); count != 0 { t.Errorf("c.Count(): 0 != %v\n", count) } } @@ -93,7 +93,7 @@ func TestCounterFloat64Zero(t *testing.T) { func TestGetOrRegisterCounterFloat64(t *testing.T) { r := NewRegistry() NewRegisteredCounterFloat64("foo", r).Inc(47.0) - if c := GetOrRegisterCounterFloat64("foo", r); c.Count() != 47.0 { + if c := GetOrRegisterCounterFloat64("foo", r).Snapshot(); c.Count() != 47.0 { t.Fatal(c) } } diff --git a/metrics/counter_test.go b/metrics/counter_test.go index af26ef1548fe..1b15b23f215f 100644 --- a/metrics/counter_test.go +++ b/metrics/counter_test.go @@ -14,7 +14,7 @@ func TestCounterClear(t *testing.T) { c := NewCounter() c.Inc(1) c.Clear() - if count := c.Count(); count != 0 { + if count := c.Snapshot().Count(); count != 0 { t.Errorf("c.Count(): 0 != %v\n", count) } } @@ -22,7 +22,7 @@ func TestCounterClear(t *testing.T) { func TestCounterDec1(t *testing.T) { c := NewCounter() c.Dec(1) - if count := c.Count(); count != -1 { + if count := c.Snapshot().Count(); count != -1 { t.Errorf("c.Count(): -1 != %v\n", count) } } @@ -30,7 +30,7 @@ func TestCounterDec1(t *testing.T) { func TestCounterDec2(t *testing.T) { c := NewCounter() c.Dec(2) - if count := c.Count(); count != -2 { + if count := c.Snapshot().Count(); count != -2 { t.Errorf("c.Count(): -2 != %v\n", count) } } @@ -38,7 +38,7 @@ func TestCounterDec2(t *testing.T) { func TestCounterInc1(t *testing.T) { c := NewCounter() c.Inc(1) - if count := c.Count(); count != 1 { + if count := c.Snapshot().Count(); count != 1 { t.Errorf("c.Count(): 1 != %v\n", count) } } @@ -46,7 +46,7 @@ func TestCounterInc1(t *testing.T) { func TestCounterInc2(t *testing.T) { c := NewCounter() c.Inc(2) - if count := c.Count(); count != 2 { + if count := c.Snapshot().Count(); count != 2 { t.Errorf("c.Count(): 2 != %v\n", count) } } @@ -63,7 +63,7 @@ func TestCounterSnapshot(t *testing.T) { func TestCounterZero(t *testing.T) { c := NewCounter() - if count := c.Count(); count != 0 { + if count := c.Snapshot().Count(); count != 0 { t.Errorf("c.Count(): 0 != %v\n", count) } } @@ -71,7 +71,7 @@ func TestCounterZero(t *testing.T) { func TestGetOrRegisterCounter(t *testing.T) { r := NewRegistry() NewRegisteredCounter("foo", r).Inc(47) - if c := GetOrRegisterCounter("foo", r); c.Count() != 47 { + if c := GetOrRegisterCounter("foo", r).Snapshot(); c.Count() != 47 { t.Fatal(c) } } diff --git a/metrics/exp/exp.go b/metrics/exp/exp.go index 89e49a5d36fb..dd528d15254e 100644 --- a/metrics/exp/exp.go +++ b/metrics/exp/exp.go @@ -109,12 +109,12 @@ func (exp *exp) getInfo(name string) *expvar.String { return v } -func (exp *exp) publishCounter(name string, metric metrics.Counter) { +func (exp *exp) publishCounter(name string, metric metrics.CounterSnapshot) { v := exp.getInt(name) v.Set(metric.Count()) } -func (exp *exp) publishCounterFloat64(name string, metric metrics.CounterFloat64) { +func (exp *exp) publishCounterFloat64(name string, metric metrics.CounterFloat64Snapshot) { v := exp.getFloat(name) v.Set(metric.Count()) } @@ -189,9 +189,9 @@ func (exp *exp) syncToExpvar() { exp.registry.Each(func(name string, i interface{}) { switch i := i.(type) { case metrics.Counter: - exp.publishCounter(name, i) + exp.publishCounter(name, i.Snapshot()) case metrics.CounterFloat64: - exp.publishCounterFloat64(name, i) + exp.publishCounterFloat64(name, i.Snapshot()) case metrics.Gauge: exp.publishGauge(name, i.Snapshot()) case metrics.GaugeFloat64: diff --git a/metrics/graphite.go b/metrics/graphite.go index d1a4cc0fa0eb..61a73e217f39 100644 --- a/metrics/graphite.go +++ b/metrics/graphite.go @@ -66,9 +66,9 @@ func graphite(c *GraphiteConfig) error { c.Registry.Each(func(name string, i interface{}) { switch metric := i.(type) { case Counter: - fmt.Fprintf(w, "%s.%s.count %d %d\n", c.Prefix, name, metric.Count(), now) + fmt.Fprintf(w, "%s.%s.count %d %d\n", c.Prefix, name, metric.Snapshot().Count(), now) case CounterFloat64: - fmt.Fprintf(w, "%s.%s.count %f %d\n", c.Prefix, name, metric.Count(), now) + fmt.Fprintf(w, "%s.%s.count %f %d\n", c.Prefix, name, metric.Snapshot().Count(), now) case Gauge: fmt.Fprintf(w, "%s.%s.value %d %d\n", c.Prefix, name, metric.Snapshot().Value(), now) case GaugeFloat64: diff --git a/metrics/influxdb/influxdb.go b/metrics/influxdb/influxdb.go index 9354f1a63358..ef2e4154a969 100644 --- a/metrics/influxdb/influxdb.go +++ b/metrics/influxdb/influxdb.go @@ -11,13 +11,13 @@ func readMeter(namespace, name string, i interface{}) (string, map[string]interf case metrics.Counter: measurement := fmt.Sprintf("%s%s.count", namespace, name) fields := map[string]interface{}{ - "value": metric.Count(), + "value": metric.Snapshot().Count(), } return measurement, fields case metrics.CounterFloat64: measurement := fmt.Sprintf("%s%s.count", namespace, name) fields := map[string]interface{}{ - "value": metric.Count(), + "value": metric.Snapshot().Count(), } return measurement, fields case metrics.Gauge: diff --git a/metrics/log.go b/metrics/log.go index c9938dd1866d..008903da940d 100644 --- a/metrics/log.go +++ b/metrics/log.go @@ -23,10 +23,10 @@ func LogScaled(r Registry, freq time.Duration, scale time.Duration, l Logger) { switch metric := i.(type) { case Counter: l.Printf("counter %s\n", name) - l.Printf(" count: %9d\n", metric.Count()) + l.Printf(" count: %9d\n", metric.Snapshot().Count()) case CounterFloat64: l.Printf("counter %s\n", name) - l.Printf(" count: %f\n", metric.Count()) + l.Printf(" count: %f\n", metric.Snapshot().Count()) case Gauge: l.Printf("gauge %s\n", name) l.Printf(" value: %9d\n", metric.Snapshot().Value()) diff --git a/metrics/metrics_test.go b/metrics/metrics_test.go index 4a1a07e6f1ad..2861d5f2caf6 100644 --- a/metrics/metrics_test.go +++ b/metrics/metrics_test.go @@ -98,7 +98,7 @@ func Example() { t.Time(func() { time.Sleep(10 * time.Millisecond) }) t.Update(1) - fmt.Println(c.Count()) + fmt.Println(c.Snapshot().Count()) fmt.Println(t.Snapshot().Min()) // Output: 17 // 1 diff --git a/metrics/opentsdb.go b/metrics/opentsdb.go index 4b1b4902e609..67534ff677a0 100644 --- a/metrics/opentsdb.go +++ b/metrics/opentsdb.go @@ -65,9 +65,9 @@ func (c *OpenTSDBConfig) writeRegistry(w io.Writer, now int64, shortHostname str c.Registry.Each(func(name string, i interface{}) { switch metric := i.(type) { case Counter: - fmt.Fprintf(w, "put %s.%s.count %d %d host=%s\n", c.Prefix, name, now, metric.Count(), shortHostname) + fmt.Fprintf(w, "put %s.%s.count %d %d host=%s\n", c.Prefix, name, now, metric.Snapshot().Count(), shortHostname) case CounterFloat64: - fmt.Fprintf(w, "put %s.%s.count %d %f host=%s\n", c.Prefix, name, now, metric.Count(), shortHostname) + fmt.Fprintf(w, "put %s.%s.count %d %f host=%s\n", c.Prefix, name, now, metric.Snapshot().Count(), shortHostname) case Gauge: fmt.Fprintf(w, "put %s.%s.value %d %d host=%s\n", c.Prefix, name, now, metric.Snapshot().Value(), shortHostname) case GaugeFloat64: diff --git a/metrics/prometheus/collector.go b/metrics/prometheus/collector.go index 28ffb218e1cd..e36904815f18 100644 --- a/metrics/prometheus/collector.go +++ b/metrics/prometheus/collector.go @@ -75,11 +75,11 @@ func (c *collector) Add(name string, i any) error { return nil } -func (c *collector) addCounter(name string, m metrics.Counter) { +func (c *collector) addCounter(name string, m metrics.CounterSnapshot) { c.writeGaugeCounter(name, m.Count()) } -func (c *collector) addCounterFloat64(name string, m metrics.CounterFloat64) { +func (c *collector) addCounterFloat64(name string, m metrics.CounterFloat64Snapshot) { c.writeGaugeCounter(name, m.Count()) } diff --git a/metrics/registry.go b/metrics/registry.go index cfe0f76240d7..8bfbc080420f 100644 --- a/metrics/registry.go +++ b/metrics/registry.go @@ -150,9 +150,9 @@ func (r *StandardRegistry) GetAll() map[string]map[string]interface{} { values := make(map[string]interface{}) switch metric := i.(type) { case Counter: - values["count"] = metric.Count() + values["count"] = metric.Snapshot().Count() case CounterFloat64: - values["count"] = metric.Count() + values["count"] = metric.Snapshot().Count() case Gauge: values["value"] = metric.Snapshot().Value() case GaugeFloat64: diff --git a/metrics/registry_test.go b/metrics/registry_test.go index 7cc5cf14fe55..75012dd4ac00 100644 --- a/metrics/registry_test.go +++ b/metrics/registry_test.go @@ -85,11 +85,11 @@ func TestRegistryDuplicate(t *testing.T) { func TestRegistryGet(t *testing.T) { r := NewRegistry() r.Register("foo", NewCounter()) - if count := r.Get("foo").(Counter).Count(); count != 0 { + if count := r.Get("foo").(Counter).Snapshot().Count(); count != 0 { t.Fatal(count) } r.Get("foo").(Counter).Inc(1) - if count := r.Get("foo").(Counter).Count(); count != 1 { + if count := r.Get("foo").(Counter).Snapshot().Count(); count != 1 { t.Fatal(count) } } diff --git a/metrics/syslog.go b/metrics/syslog.go index ecc5fe960842..9fd89c5e2043 100644 --- a/metrics/syslog.go +++ b/metrics/syslog.go @@ -16,9 +16,9 @@ func Syslog(r Registry, d time.Duration, w *syslog.Writer) { r.Each(func(name string, i interface{}) { switch metric := i.(type) { case Counter: - w.Info(fmt.Sprintf("counter %s: count: %d", name, metric.Count())) + w.Info(fmt.Sprintf("counter %s: count: %d", name, metric.Snapshot().Count())) case CounterFloat64: - w.Info(fmt.Sprintf("counter %s: count: %f", name, metric.Count())) + w.Info(fmt.Sprintf("counter %s: count: %f", name, metric.Snapshot().Count())) case Gauge: w.Info(fmt.Sprintf("gauge %s: value: %d", name, metric.Snapshot().Value())) case GaugeFloat64: diff --git a/metrics/writer.go b/metrics/writer.go index d352dc25d372..a36918b3b736 100644 --- a/metrics/writer.go +++ b/metrics/writer.go @@ -29,10 +29,10 @@ func WriteOnce(r Registry, w io.Writer) { switch metric := namedMetric.m.(type) { case Counter: fmt.Fprintf(w, "counter %s\n", namedMetric.name) - fmt.Fprintf(w, " count: %9d\n", metric.Count()) + fmt.Fprintf(w, " count: %9d\n", metric.Snapshot().Count()) case CounterFloat64: fmt.Fprintf(w, "counter %s\n", namedMetric.name) - fmt.Fprintf(w, " count: %f\n", metric.Count()) + fmt.Fprintf(w, " count: %f\n", metric.Snapshot().Count()) case Gauge: fmt.Fprintf(w, "gauge %s\n", namedMetric.name) fmt.Fprintf(w, " value: %9d\n", metric.Snapshot().Value()) From 73687d9a60b616ea97cac75e5b2a5e8fac70f188 Mon Sep 17 00:00:00 2001 From: Martin Holst Swende Date: Thu, 31 Aug 2023 12:34:07 +0200 Subject: [PATCH 09/26] metrics: split up resetting timer interface --- metrics/prometheus/collector.go | 2 +- metrics/resetting_timer.go | 174 ++++++++++++-------------------- 2 files changed, 64 insertions(+), 112 deletions(-) diff --git a/metrics/prometheus/collector.go b/metrics/prometheus/collector.go index e36904815f18..c3b5cabfbce4 100644 --- a/metrics/prometheus/collector.go +++ b/metrics/prometheus/collector.go @@ -121,7 +121,7 @@ func (c *collector) addTimer(name string, m metrics.TimerSnapshot) { c.buff.WriteRune('\n') } -func (c *collector) addResettingTimer(name string, m metrics.ResettingTimer) { +func (c *collector) addResettingTimer(name string, m metrics.ResettingTimerSnapshot) { if len(m.Values()) <= 0 { return } diff --git a/metrics/resetting_timer.go b/metrics/resetting_timer.go index 8e23c8eeeaaa..7e609e480f4d 100644 --- a/metrics/resetting_timer.go +++ b/metrics/resetting_timer.go @@ -11,12 +11,15 @@ import ( // Initial slice capacity for the values stored in a ResettingTimer const InitialResettingTimerSliceCap = 10 -// ResettingTimer is used for storing aggregated values for timers, which are reset on every flush interval. -type ResettingTimer interface { +type ResettingTimerSnapshot interface { Values() []int64 - Snapshot() ResettingTimer - Percentiles([]float64) []int64 Mean() float64 + Percentiles([]float64) []int64 +} + +// ResettingTimer is used for storing aggregated values for timers, which are reset on every flush interval. +type ResettingTimer interface { + Snapshot() ResettingTimerSnapshot Time(func()) Update(time.Duration) UpdateSince(time.Time) @@ -52,37 +55,15 @@ func NewResettingTimer() ResettingTimer { } // NilResettingTimer is a no-op ResettingTimer. -type NilResettingTimer struct { -} +type NilResettingTimer struct{} -// Values is a no-op. -func (NilResettingTimer) Values() []int64 { return nil } - -// Snapshot is a no-op. -func (NilResettingTimer) Snapshot() ResettingTimer { - return &ResettingTimerSnapshot{ - values: []int64{}, - } -} - -// Time is a no-op. -func (NilResettingTimer) Time(f func()) { f() } - -// Update is a no-op. -func (NilResettingTimer) Update(time.Duration) {} - -// Percentiles panics. -func (NilResettingTimer) Percentiles([]float64) []int64 { - panic("Percentiles called on a NilResettingTimer") -} - -// Mean panics. -func (NilResettingTimer) Mean() float64 { - panic("Mean called on a NilResettingTimer") -} - -// UpdateSince is a no-op. -func (NilResettingTimer) UpdateSince(time.Time) {} +func (NilResettingTimer) Values() []int64 { return nil } +func (n NilResettingTimer) Snapshot() ResettingTimerSnapshot { return n } +func (NilResettingTimer) Time(f func()) { f() } +func (NilResettingTimer) Update(time.Duration) {} +func (NilResettingTimer) Percentiles([]float64) []int64 { return nil } +func (NilResettingTimer) Mean() float64 { return 0.0 } +func (NilResettingTimer) UpdateSince(time.Time) {} // StandardResettingTimer is the standard implementation of a ResettingTimer. // and Meter. @@ -91,33 +72,23 @@ type StandardResettingTimer struct { mutex sync.Mutex } -// Values returns a slice with all measurements. -func (t *StandardResettingTimer) Values() []int64 { - return t.values -} +//// Values returns a slice with all measurements. +//func (t *StandardResettingTimer) Values() []int64 { +// return t.values +//} // Snapshot resets the timer and returns a read-only copy of its contents. -func (t *StandardResettingTimer) Snapshot() ResettingTimer { +func (t *StandardResettingTimer) Snapshot() ResettingTimerSnapshot { t.mutex.Lock() defer t.mutex.Unlock() currentValues := t.values t.values = make([]int64, 0, InitialResettingTimerSliceCap) - return &ResettingTimerSnapshot{ + return &resettingTimerSnapshot{ values: currentValues, } } -// Percentiles panics. -func (t *StandardResettingTimer) Percentiles([]float64) []int64 { - panic("Percentiles called on a StandardResettingTimer") -} - -// Mean panics. -func (t *StandardResettingTimer) Mean() float64 { - panic("Mean called on a StandardResettingTimer") -} - // Record the duration of the execution of the given function. func (t *StandardResettingTimer) Time(f func()) { ts := time.Now() @@ -139,46 +110,29 @@ func (t *StandardResettingTimer) UpdateSince(ts time.Time) { t.values = append(t.values, int64(time.Since(ts))) } -// ResettingTimerSnapshot is a point-in-time copy of another ResettingTimer. -type ResettingTimerSnapshot struct { +// resettingTimerSnapshot is a point-in-time copy of another ResettingTimer. +type resettingTimerSnapshot struct { values []int64 mean float64 thresholdBoundaries []int64 calculated bool } -// Snapshot returns the snapshot. -func (t *ResettingTimerSnapshot) Snapshot() ResettingTimer { return t } - -// Time panics. -func (*ResettingTimerSnapshot) Time(func()) { - panic("Time called on a ResettingTimerSnapshot") -} - -// Update panics. -func (*ResettingTimerSnapshot) Update(time.Duration) { - panic("Update called on a ResettingTimerSnapshot") -} - -// UpdateSince panics. -func (*ResettingTimerSnapshot) UpdateSince(time.Time) { - panic("UpdateSince called on a ResettingTimerSnapshot") -} - // Values returns all values from snapshot. -func (t *ResettingTimerSnapshot) Values() []int64 { +func (t *resettingTimerSnapshot) Values() []int64 { return t.values } // Percentiles returns the boundaries for the input percentiles. -func (t *ResettingTimerSnapshot) Percentiles(percentiles []float64) []int64 { +// note: this method is not thread safe +func (t *resettingTimerSnapshot) Percentiles(percentiles []float64) []int64 { t.calc(percentiles) - return t.thresholdBoundaries } // Mean returns the mean of the snapshotted values -func (t *ResettingTimerSnapshot) Mean() float64 { +// note: this method is not thread safe +func (t *resettingTimerSnapshot) Mean() float64 { if !t.calculated { t.calc([]float64{}) } @@ -186,50 +140,48 @@ func (t *ResettingTimerSnapshot) Mean() float64 { return t.mean } -func (t *ResettingTimerSnapshot) calc(percentiles []float64) { +func (t *resettingTimerSnapshot) calc(percentiles []float64) { slices.Sort(t.values) - count := len(t.values) - if count > 0 { - min := t.values[0] - max := t.values[count-1] - - cumulativeValues := make([]int64, count) - cumulativeValues[0] = min - for i := 1; i < count; i++ { - cumulativeValues[i] = t.values[i] + cumulativeValues[i-1] - } - + if count == 0 { t.thresholdBoundaries = make([]int64, len(percentiles)) + t.mean = 0 + t.calculated = true + return + } + min := t.values[0] + max := t.values[count-1] - thresholdBoundary := max - - for i, pct := range percentiles { - if count > 1 { - var abs float64 - if pct >= 0 { - abs = pct - } else { - abs = 100 + pct - } - // poor man's math.Round(x): - // math.Floor(x + 0.5) - indexOfPerc := int(math.Floor(((abs / 100.0) * float64(count)) + 0.5)) - if pct >= 0 && indexOfPerc > 0 { - indexOfPerc -= 1 // index offset=0 - } - thresholdBoundary = t.values[indexOfPerc] - } + cumulativeValues := make([]int64, count) + cumulativeValues[0] = min + for i := 1; i < count; i++ { + cumulativeValues[i] = t.values[i] + cumulativeValues[i-1] + } + + t.thresholdBoundaries = make([]int64, len(percentiles)) - t.thresholdBoundaries[i] = thresholdBoundary + thresholdBoundary := max + + for i, pct := range percentiles { + if count > 1 { + var abs float64 + if pct >= 0 { + abs = pct + } else { + abs = 100 + pct + } + // poor man's math.Round(x): + // math.Floor(x + 0.5) + indexOfPerc := int(math.Floor(((abs / 100.0) * float64(count)) + 0.5)) + if pct >= 0 && indexOfPerc > 0 { + indexOfPerc -= 1 // index offset=0 + } + thresholdBoundary = t.values[indexOfPerc] } - sum := cumulativeValues[count-1] - t.mean = float64(sum) / float64(count) - } else { - t.thresholdBoundaries = make([]int64, len(percentiles)) - t.mean = 0 + t.thresholdBoundaries[i] = thresholdBoundary } - t.calculated = true + sum := cumulativeValues[count-1] + t.mean = float64(sum) / float64(count) } From fb8e300797810ce9625ca07841aa844ce36639dd Mon Sep 17 00:00:00 2001 From: Martin Holst Swende Date: Thu, 31 Aug 2023 13:07:54 +0200 Subject: [PATCH 10/26] metrics: make resettingtimer not expose internal values --- metrics/exp/exp.go | 2 +- metrics/influxdb/influxdb.go | 9 +++--- metrics/prometheus/collector.go | 5 ++-- metrics/resetting_timer.go | 50 ++++++++++++++++++++++++--------- metrics/resetting_timer_test.go | 28 +++++++----------- 5 files changed, 53 insertions(+), 41 deletions(-) diff --git a/metrics/exp/exp.go b/metrics/exp/exp.go index dd528d15254e..100fd9d16771 100644 --- a/metrics/exp/exp.go +++ b/metrics/exp/exp.go @@ -177,7 +177,7 @@ func (exp *exp) publishTimer(name string, metric metrics.Timer) { func (exp *exp) publishResettingTimer(name string, metric metrics.ResettingTimer) { t := metric.Snapshot() ps := t.Percentiles([]float64{50, 75, 95, 99}) - exp.getInt(name + ".count").Set(int64(len(t.Values()))) + exp.getInt(name + ".count").Set(int64(t.Count())) exp.getFloat(name + ".mean").Set(t.Mean()) exp.getInt(name + ".50-percentile").Set(ps[0]) exp.getInt(name + ".75-percentile").Set(ps[1]) diff --git a/metrics/influxdb/influxdb.go b/metrics/influxdb/influxdb.go index ef2e4154a969..83c2edfb0d06 100644 --- a/metrics/influxdb/influxdb.go +++ b/metrics/influxdb/influxdb.go @@ -99,17 +99,16 @@ func readMeter(namespace, name string, i interface{}) (string, map[string]interf return measurement, fields case metrics.ResettingTimer: t := metric.Snapshot() - if len(t.Values()) == 0 { + if t.Count() == 0 { break } ps := t.Percentiles([]float64{50, 95, 99}) - val := t.Values() measurement := fmt.Sprintf("%s%s.span", namespace, name) fields := map[string]interface{}{ - "count": len(val), - "max": val[len(val)-1], + "count": t.Count(), + "max": t.Max(), "mean": t.Mean(), - "min": val[0], + "min": t.Min(), "p50": ps[0], "p95": ps[1], "p99": ps[2], diff --git a/metrics/prometheus/collector.go b/metrics/prometheus/collector.go index c3b5cabfbce4..45e8a914bea2 100644 --- a/metrics/prometheus/collector.go +++ b/metrics/prometheus/collector.go @@ -122,12 +122,11 @@ func (c *collector) addTimer(name string, m metrics.TimerSnapshot) { } func (c *collector) addResettingTimer(name string, m metrics.ResettingTimerSnapshot) { - if len(m.Values()) <= 0 { + if m.Count() <= 0 { return } ps := m.Percentiles([]float64{50, 95, 99}) - val := m.Values() - c.writeSummaryCounter(name, len(val)) + c.writeSummaryCounter(name, m.Count()) c.buff.WriteString(fmt.Sprintf(typeSummaryTpl, mutateKey(name))) c.writeSummaryPercentile(name, "0.50", ps[0]) c.writeSummaryPercentile(name, "0.95", ps[1]) diff --git a/metrics/resetting_timer.go b/metrics/resetting_timer.go index 7e609e480f4d..b5bbc753cc7f 100644 --- a/metrics/resetting_timer.go +++ b/metrics/resetting_timer.go @@ -12,8 +12,10 @@ import ( const InitialResettingTimerSliceCap = 10 type ResettingTimerSnapshot interface { - Values() []int64 + Count() int Mean() float64 + Max() int64 + Min() int64 Percentiles([]float64) []int64 } @@ -63,7 +65,10 @@ func (NilResettingTimer) Time(f func()) { f() } func (NilResettingTimer) Update(time.Duration) {} func (NilResettingTimer) Percentiles([]float64) []int64 { return nil } func (NilResettingTimer) Mean() float64 { return 0.0 } +func (NilResettingTimer) Max() int64 { return 0 } +func (NilResettingTimer) Min() int64 { return 0 } func (NilResettingTimer) UpdateSince(time.Time) {} +func (NilResettingTimer) Count() int { return 0 } // StandardResettingTimer is the standard implementation of a ResettingTimer. // and Meter. @@ -72,11 +77,6 @@ type StandardResettingTimer struct { mutex sync.Mutex } -//// Values returns a slice with all measurements. -//func (t *StandardResettingTimer) Values() []int64 { -// return t.values -//} - // Snapshot resets the timer and returns a read-only copy of its contents. func (t *StandardResettingTimer) Snapshot() ResettingTimerSnapshot { t.mutex.Lock() @@ -114,13 +114,15 @@ func (t *StandardResettingTimer) UpdateSince(ts time.Time) { type resettingTimerSnapshot struct { values []int64 mean float64 + max int64 + min int64 thresholdBoundaries []int64 calculated bool } -// Values returns all values from snapshot. -func (t *resettingTimerSnapshot) Values() []int64 { - return t.values +// Count return the length of the values from snapshot. +func (t *resettingTimerSnapshot) Count() int { + return len(t.values) } // Percentiles returns the boundaries for the input percentiles. @@ -134,12 +136,32 @@ func (t *resettingTimerSnapshot) Percentiles(percentiles []float64) []int64 { // note: this method is not thread safe func (t *resettingTimerSnapshot) Mean() float64 { if !t.calculated { - t.calc([]float64{}) + t.calc(nil) } return t.mean } +// Max returns the max of the snapshotted values +// note: this method is not thread safe +func (t *resettingTimerSnapshot) Max() int64 { + if !t.calculated { + t.calc(nil) + } + + return t.max +} + +// Min returns the min of the snapshotted values +// note: this method is not thread safe +func (t *resettingTimerSnapshot) Min() int64 { + if !t.calculated { + t.calc(nil) + } + + return t.min +} + func (t *resettingTimerSnapshot) calc(percentiles []float64) { slices.Sort(t.values) count := len(t.values) @@ -149,18 +171,18 @@ func (t *resettingTimerSnapshot) calc(percentiles []float64) { t.calculated = true return } - min := t.values[0] - max := t.values[count-1] + t.min = t.values[0] + t.max = t.values[count-1] cumulativeValues := make([]int64, count) - cumulativeValues[0] = min + cumulativeValues[0] = t.min for i := 1; i < count; i++ { cumulativeValues[i] = t.values[i] + cumulativeValues[i-1] } t.thresholdBoundaries = make([]int64, len(percentiles)) - thresholdBoundary := max + thresholdBoundary := t.max for i, pct := range percentiles { if count > 1 { diff --git a/metrics/resetting_timer_test.go b/metrics/resetting_timer_test.go index 77c49dc3866a..5bc88e38bd32 100644 --- a/metrics/resetting_timer_test.go +++ b/metrics/resetting_timer_test.go @@ -75,16 +75,12 @@ func TestResettingTimer(t *testing.T) { ps := snap.Percentiles([]float64{50, 95, 99}) - val := snap.Values() - - if len(val) > 0 { - if tt.wantMin != val[0] { - t.Fatalf("%d: min: got %d, want %d", ind, val[0], tt.wantMin) - } + if tt.wantMin != snap.Min() { + t.Fatalf("%d: min: got %d, want %d", ind, snap.Min(), tt.wantMin) + } - if tt.wantMax != val[len(val)-1] { - t.Fatalf("%d: max: got %d, want %d", ind, val[len(val)-1], tt.wantMax) - } + if tt.wantMax != snap.Max() { + t.Fatalf("%d: max: got %d, want %d", ind, snap.Max(), tt.wantMax) } if tt.wantMean != snap.Mean() { @@ -177,16 +173,12 @@ func TestResettingTimerWithFivePercentiles(t *testing.T) { ps := snap.Percentiles([]float64{5, 20, 50, 95, 99}) - val := snap.Values() - - if len(val) > 0 { - if tt.wantMin != val[0] { - t.Fatalf("%d: min: got %d, want %d", ind, val[0], tt.wantMin) - } + if tt.wantMin != snap.Min() { + t.Fatalf("%d: min: got %d, want %d", ind, snap.Min(), tt.wantMin) + } - if tt.wantMax != val[len(val)-1] { - t.Fatalf("%d: max: got %d, want %d", ind, val[len(val)-1], tt.wantMax) - } + if tt.wantMax != snap.Max() { + t.Fatalf("%d: max: got %d, want %d", ind, snap.Max(), tt.wantMax) } if tt.wantMean != snap.Mean() { From 0abadecea19eca1bb964c341d259c4e911067e51 Mon Sep 17 00:00:00 2001 From: Martin Holst Swende Date: Thu, 31 Aug 2023 14:48:53 +0200 Subject: [PATCH 11/26] metrics: align percentiles handling in resetting timer with rest of the codebase --- metrics/exp/exp.go | 10 ++-- metrics/influxdb/influxdb.go | 2 +- metrics/prometheus/collector.go | 2 +- metrics/resetting_timer.go | 60 +++++------------------ metrics/resetting_timer_test.go | 85 ++++++++++++++------------------- metrics/sample.go | 44 +++++++++-------- metrics/sample_test.go | 14 ++++++ 7 files changed, 95 insertions(+), 122 deletions(-) diff --git a/metrics/exp/exp.go b/metrics/exp/exp.go index 100fd9d16771..b273ee03c7a6 100644 --- a/metrics/exp/exp.go +++ b/metrics/exp/exp.go @@ -176,13 +176,13 @@ func (exp *exp) publishTimer(name string, metric metrics.Timer) { func (exp *exp) publishResettingTimer(name string, metric metrics.ResettingTimer) { t := metric.Snapshot() - ps := t.Percentiles([]float64{50, 75, 95, 99}) + ps := t.Percentiles([]float64{0.50, 0.75, 0.95, 0.99}) exp.getInt(name + ".count").Set(int64(t.Count())) exp.getFloat(name + ".mean").Set(t.Mean()) - exp.getInt(name + ".50-percentile").Set(ps[0]) - exp.getInt(name + ".75-percentile").Set(ps[1]) - exp.getInt(name + ".95-percentile").Set(ps[2]) - exp.getInt(name + ".99-percentile").Set(ps[3]) + exp.getFloat(name + ".50-percentile").Set(ps[0]) + exp.getFloat(name + ".75-percentile").Set(ps[1]) + exp.getFloat(name + ".95-percentile").Set(ps[2]) + exp.getFloat(name + ".99-percentile").Set(ps[3]) } func (exp *exp) syncToExpvar() { diff --git a/metrics/influxdb/influxdb.go b/metrics/influxdb/influxdb.go index 83c2edfb0d06..227c2bd57292 100644 --- a/metrics/influxdb/influxdb.go +++ b/metrics/influxdb/influxdb.go @@ -102,7 +102,7 @@ func readMeter(namespace, name string, i interface{}) (string, map[string]interf if t.Count() == 0 { break } - ps := t.Percentiles([]float64{50, 95, 99}) + ps := t.Percentiles([]float64{0.50, 0.95, 0.99}) measurement := fmt.Sprintf("%s%s.span", namespace, name) fields := map[string]interface{}{ "count": t.Count(), diff --git a/metrics/prometheus/collector.go b/metrics/prometheus/collector.go index 45e8a914bea2..d6a0b4200c18 100644 --- a/metrics/prometheus/collector.go +++ b/metrics/prometheus/collector.go @@ -125,7 +125,7 @@ func (c *collector) addResettingTimer(name string, m metrics.ResettingTimerSnaps if m.Count() <= 0 { return } - ps := m.Percentiles([]float64{50, 95, 99}) + ps := m.Percentiles([]float64{0.50, 0.95, 0.99}) c.writeSummaryCounter(name, m.Count()) c.buff.WriteString(fmt.Sprintf(typeSummaryTpl, mutateKey(name))) c.writeSummaryPercentile(name, "0.50", ps[0]) diff --git a/metrics/resetting_timer.go b/metrics/resetting_timer.go index b5bbc753cc7f..8fb27c8a5c7c 100644 --- a/metrics/resetting_timer.go +++ b/metrics/resetting_timer.go @@ -1,11 +1,8 @@ package metrics import ( - "math" "sync" "time" - - "golang.org/x/exp/slices" ) // Initial slice capacity for the values stored in a ResettingTimer @@ -16,7 +13,7 @@ type ResettingTimerSnapshot interface { Mean() float64 Max() int64 Min() int64 - Percentiles([]float64) []int64 + Percentiles([]float64) []float64 } // ResettingTimer is used for storing aggregated values for timers, which are reset on every flush interval. @@ -63,7 +60,7 @@ func (NilResettingTimer) Values() []int64 { return nil } func (n NilResettingTimer) Snapshot() ResettingTimerSnapshot { return n } func (NilResettingTimer) Time(f func()) { f() } func (NilResettingTimer) Update(time.Duration) {} -func (NilResettingTimer) Percentiles([]float64) []int64 { return nil } +func (NilResettingTimer) Percentiles([]float64) []float64 { return nil } func (NilResettingTimer) Mean() float64 { return 0.0 } func (NilResettingTimer) Max() int64 { return 0 } func (NilResettingTimer) Min() int64 { return 0 } @@ -116,7 +113,7 @@ type resettingTimerSnapshot struct { mean float64 max int64 min int64 - thresholdBoundaries []int64 + thresholdBoundaries []float64 calculated bool } @@ -127,7 +124,7 @@ func (t *resettingTimerSnapshot) Count() int { // Percentiles returns the boundaries for the input percentiles. // note: this method is not thread safe -func (t *resettingTimerSnapshot) Percentiles(percentiles []float64) []int64 { +func (t *resettingTimerSnapshot) Percentiles(percentiles []float64) []float64 { t.calc(percentiles) return t.thresholdBoundaries } @@ -148,7 +145,6 @@ func (t *resettingTimerSnapshot) Max() int64 { if !t.calculated { t.calc(nil) } - return t.max } @@ -158,52 +154,20 @@ func (t *resettingTimerSnapshot) Min() int64 { if !t.calculated { t.calc(nil) } - return t.min } func (t *resettingTimerSnapshot) calc(percentiles []float64) { - slices.Sort(t.values) - count := len(t.values) - if count == 0 { - t.thresholdBoundaries = make([]int64, len(percentiles)) - t.mean = 0 - t.calculated = true + scores := CalculatePercentiles(t.values, percentiles) + t.thresholdBoundaries = scores + if len(t.values) == 0 { return } t.min = t.values[0] - t.max = t.values[count-1] - - cumulativeValues := make([]int64, count) - cumulativeValues[0] = t.min - for i := 1; i < count; i++ { - cumulativeValues[i] = t.values[i] + cumulativeValues[i-1] + t.max = t.values[len(t.values)-1] + var sum int64 + for _, v := range t.values { + sum += v } - - t.thresholdBoundaries = make([]int64, len(percentiles)) - - thresholdBoundary := t.max - - for i, pct := range percentiles { - if count > 1 { - var abs float64 - if pct >= 0 { - abs = pct - } else { - abs = 100 + pct - } - // poor man's math.Round(x): - // math.Floor(x + 0.5) - indexOfPerc := int(math.Floor(((abs / 100.0) * float64(count)) + 0.5)) - if pct >= 0 && indexOfPerc > 0 { - indexOfPerc -= 1 // index offset=0 - } - thresholdBoundary = t.values[indexOfPerc] - } - - t.thresholdBoundaries[i] = thresholdBoundary - } - - sum := cumulativeValues[count-1] - t.mean = float64(sum) / float64(count) + t.mean = float64(sum) / float64(len(t.values)) } diff --git a/metrics/resetting_timer_test.go b/metrics/resetting_timer_test.go index 5bc88e38bd32..4571fc8eb052 100644 --- a/metrics/resetting_timer_test.go +++ b/metrics/resetting_timer_test.go @@ -10,9 +10,9 @@ func TestResettingTimer(t *testing.T) { values []int64 start int end int - wantP50 int64 - wantP95 int64 - wantP99 int64 + wantP50 float64 + wantP95 float64 + wantP99 float64 wantMean float64 wantMin int64 wantMax int64 @@ -21,14 +21,14 @@ func TestResettingTimer(t *testing.T) { values: []int64{}, start: 1, end: 11, - wantP50: 5, wantP95: 10, wantP99: 10, + wantP50: 5.5, wantP95: 10, wantP99: 10, wantMin: 1, wantMax: 10, wantMean: 5.5, }, { values: []int64{}, start: 1, end: 101, - wantP50: 50, wantP95: 95, wantP99: 99, + wantP50: 50.5, wantP95: 95.94999999999999, wantP99: 99.99, wantMin: 1, wantMax: 100, wantMean: 50.5, }, { @@ -56,11 +56,11 @@ func TestResettingTimer(t *testing.T) { values: []int64{1, 10}, start: 0, end: 0, - wantP50: 1, wantP95: 10, wantP99: 10, + wantP50: 5.5, wantP95: 10, wantP99: 10, wantMin: 1, wantMax: 10, wantMean: 5.5, }, } - for ind, tt := range tests { + for i, tt := range tests { timer := NewResettingTimer() for i := tt.start; i < tt.end; i++ { @@ -70,33 +70,27 @@ func TestResettingTimer(t *testing.T) { for _, v := range tt.values { timer.Update(time.Duration(v)) } - snap := timer.Snapshot() - ps := snap.Percentiles([]float64{50, 95, 99}) + ps := snap.Percentiles([]float64{0.50, 0.95, 0.99}) - if tt.wantMin != snap.Min() { - t.Fatalf("%d: min: got %d, want %d", ind, snap.Min(), tt.wantMin) + if have, want := snap.Min(), tt.wantMin; have != want { + t.Fatalf("%d: min: have %d, want %d", i, have, want) } - - if tt.wantMax != snap.Max() { - t.Fatalf("%d: max: got %d, want %d", ind, snap.Max(), tt.wantMax) + if have, want := snap.Max(), tt.wantMax; have != want { + t.Fatalf("%d: max: have %d, want %d", i, have, want) } - - if tt.wantMean != snap.Mean() { - t.Fatalf("%d: mean: got %.2f, want %.2f", ind, snap.Mean(), tt.wantMean) + if have, want := snap.Mean(), tt.wantMean; have != want { + t.Fatalf("%d: mean: have %v, want %v", i, have, want) } - - if tt.wantP50 != ps[0] { - t.Fatalf("%d: p50: got %d, want %d", ind, ps[0], tt.wantP50) + if have, want := ps[0], tt.wantP50; have != want { + t.Errorf("%d: p50: have %v, want %v", i, have, want) } - - if tt.wantP95 != ps[1] { - t.Fatalf("%d: p95: got %d, want %d", ind, ps[1], tt.wantP95) + if have, want := ps[1], tt.wantP95; have != want { + t.Errorf("%d: p95: have %v, want %v", i, have, want) } - - if tt.wantP99 != ps[2] { - t.Fatalf("%d: p99: got %d, want %d", ind, ps[2], tt.wantP99) + if have, want := ps[2], tt.wantP99; have != want { + t.Errorf("%d: p99: have %v, want %v", i, have, want) } } } @@ -106,11 +100,11 @@ func TestResettingTimerWithFivePercentiles(t *testing.T) { values []int64 start int end int - wantP05 int64 - wantP20 int64 - wantP50 int64 - wantP95 int64 - wantP99 int64 + wantP05 float64 + wantP20 float64 + wantP50 float64 + wantP95 float64 + wantP99 float64 wantMean float64 wantMin int64 wantMax int64 @@ -119,14 +113,14 @@ func TestResettingTimerWithFivePercentiles(t *testing.T) { values: []int64{}, start: 1, end: 11, - wantP05: 1, wantP20: 2, wantP50: 5, wantP95: 10, wantP99: 10, + wantP05: 1, wantP20: 2.2, wantP50: 5.5, wantP95: 10, wantP99: 10, wantMin: 1, wantMax: 10, wantMean: 5.5, }, { values: []int64{}, start: 1, end: 101, - wantP05: 5, wantP20: 20, wantP50: 50, wantP95: 95, wantP99: 99, + wantP05: 5.050000000000001, wantP20: 20.200000000000003, wantP50: 50.5, wantP95: 95.94999999999999, wantP99: 99.99, wantMin: 1, wantMax: 100, wantMean: 50.5, }, { @@ -154,7 +148,7 @@ func TestResettingTimerWithFivePercentiles(t *testing.T) { values: []int64{1, 10}, start: 0, end: 0, - wantP05: 1, wantP20: 1, wantP50: 1, wantP95: 10, wantP99: 10, + wantP05: 1, wantP20: 1, wantP50: 5.5, wantP95: 10, wantP99: 10, wantMin: 1, wantMax: 10, wantMean: 5.5, }, } @@ -171,38 +165,33 @@ func TestResettingTimerWithFivePercentiles(t *testing.T) { snap := timer.Snapshot() - ps := snap.Percentiles([]float64{5, 20, 50, 95, 99}) + ps := snap.Percentiles([]float64{0.05, 0.20, 0.50, 0.95, 0.99}) if tt.wantMin != snap.Min() { - t.Fatalf("%d: min: got %d, want %d", ind, snap.Min(), tt.wantMin) + t.Errorf("%d: min: got %d, want %d", ind, snap.Min(), tt.wantMin) } if tt.wantMax != snap.Max() { - t.Fatalf("%d: max: got %d, want %d", ind, snap.Max(), tt.wantMax) + t.Errorf("%d: max: got %d, want %d", ind, snap.Max(), tt.wantMax) } if tt.wantMean != snap.Mean() { - t.Fatalf("%d: mean: got %.2f, want %.2f", ind, snap.Mean(), tt.wantMean) + t.Errorf("%d: mean: got %.2f, want %.2f", ind, snap.Mean(), tt.wantMean) } - if tt.wantP05 != ps[0] { - t.Fatalf("%d: p05: got %d, want %d", ind, ps[0], tt.wantP05) + t.Errorf("%d: p05: got %v, want %v", ind, ps[0], tt.wantP05) } - if tt.wantP20 != ps[1] { - t.Fatalf("%d: p20: got %d, want %d", ind, ps[1], tt.wantP20) + t.Errorf("%d: p20: got %v, want %v", ind, ps[1], tt.wantP20) } - if tt.wantP50 != ps[2] { - t.Fatalf("%d: p50: got %d, want %d", ind, ps[2], tt.wantP50) + t.Errorf("%d: p50: got %v, want %v", ind, ps[2], tt.wantP50) } - if tt.wantP95 != ps[3] { - t.Fatalf("%d: p95: got %d, want %d", ind, ps[3], tt.wantP95) + t.Errorf("%d: p95: got %v, want %v", ind, ps[3], tt.wantP95) } - if tt.wantP99 != ps[4] { - t.Fatalf("%d: p99: got %d, want %d", ind, ps[4], tt.wantP99) + t.Errorf("%d: p99: got %v, want %v", ind, ps[4], tt.wantP99) } } } diff --git a/metrics/sample.go b/metrics/sample.go index d52a92c06829..20a7997478ba 100644 --- a/metrics/sample.go +++ b/metrics/sample.go @@ -176,29 +176,35 @@ func (NilSample) Values() []int64 { return []int64{} } // Variance is a no-op. func (NilSample) Variance() float64 { return 0.0 } -// SamplePercentiles returns an arbitrary percentile of the slice of int64. +// CalculatePercentiles returns an arbitrary percentile of the slice of int64. func SamplePercentile(values []int64, p float64) float64 { - return SamplePercentiles(values, []float64{p})[0] + return CalculatePercentiles(values, []float64{p})[0] } -// SamplePercentiles returns a slice of arbitrary percentiles of the slice of -// int64. -func SamplePercentiles(values []int64, ps []float64) []float64 { +// CalculatePercentiles returns a slice of arbitrary percentiles of the slice of +// int64. This method returns interpolated results, so e.g if there are only two +// values, [0, 10], a 50% percentile will land between them. +// +// Note: As a side-effect, this method will also sort the slice of values. +// Note2: The input format for percentiles is NOT percent! To express 50%, use 0.5, not 50. +func CalculatePercentiles(values []int64, ps []float64) []float64 { scores := make([]float64, len(ps)) size := len(values) - if size > 0 { - slices.Sort(values) - for i, p := range ps { - pos := p * float64(size+1) - if pos < 1.0 { - scores[i] = float64(values[0]) - } else if pos >= float64(size) { - scores[i] = float64(values[size-1]) - } else { - lower := float64(values[int(pos)-1]) - upper := float64(values[int(pos)]) - scores[i] = lower + (pos-math.Floor(pos))*(upper-lower) - } + if size == 0 { + return scores + } + slices.Sort(values) + for i, p := range ps { + pos := p * float64(size+1) + + if pos < 1.0 { + scores[i] = float64(values[0]) + } else if pos >= float64(size) { + scores[i] = float64(values[size-1]) + } else { + lower := float64(values[int(pos)-1]) + upper := float64(values[int(pos)]) + scores[i] = lower + (pos-math.Floor(pos))*(upper-lower) } } return scores @@ -267,7 +273,7 @@ func (s *sampleSnapshot) Percentile(p float64) float64 { // Percentiles returns a slice of arbitrary percentiles of values at the time // the snapshot was taken. func (s *sampleSnapshot) Percentiles(ps []float64) []float64 { - return SamplePercentiles(s.values, ps) + return CalculatePercentiles(s.values, ps) } // Size returns the size of the sample at the time the snapshot was taken. diff --git a/metrics/sample_test.go b/metrics/sample_test.go index a57aba78681d..e15ffed15b42 100644 --- a/metrics/sample_test.go +++ b/metrics/sample_test.go @@ -367,3 +367,17 @@ func TestUniformSampleConcurrentUpdateCount(t *testing.T) { } quit <- struct{}{} } + +func BenchmarkCalculatePercentiles(b *testing.B) { + pss := []float64{0.5, 0.75, 0.95, 0.99, 0.999, 0.9999} + var vals []int64 + for i := 0; i < 1000; i++ { + vals = append(vals, int64(rand.Int31())) + } + v := make([]int64, len(vals)) + b.ResetTimer() + for i := 0; i < b.N; i++ { + copy(v, vals) + _ = CalculatePercentiles(v, pss) + } +} From f4d0f945904cf81b177faa9909da64417c62c326 Mon Sep 17 00:00:00 2001 From: Martin Holst Swende Date: Thu, 31 Aug 2023 20:19:19 +0200 Subject: [PATCH 12/26] metrics: split gaugeinfo into read/write --- metrics/exp/exp.go | 4 +- metrics/gauge_info.go | 88 ++++++--------------------------- metrics/gauge_info_test.go | 50 +++---------------- metrics/graphite.go | 2 +- metrics/log.go | 2 +- metrics/opentsdb.go | 2 +- metrics/prometheus/collector.go | 2 +- metrics/syslog.go | 2 +- metrics/writer.go | 2 +- 9 files changed, 28 insertions(+), 126 deletions(-) diff --git a/metrics/exp/exp.go b/metrics/exp/exp.go index b273ee03c7a6..7e3f82a075fc 100644 --- a/metrics/exp/exp.go +++ b/metrics/exp/exp.go @@ -127,7 +127,7 @@ func (exp *exp) publishGaugeFloat64(name string, metric metrics.GaugeFloat64Snap exp.getFloat(name).Set(metric.Value()) } -func (exp *exp) publishGaugeInfo(name string, metric metrics.GaugeInfo) { +func (exp *exp) publishGaugeInfo(name string, metric metrics.GaugeInfoSnapshot) { exp.getInfo(name).Set(metric.Value().String()) } @@ -197,7 +197,7 @@ func (exp *exp) syncToExpvar() { case metrics.GaugeFloat64: exp.publishGaugeFloat64(name, i.Snapshot()) case metrics.GaugeInfo: - exp.publishGaugeInfo(name, i) + exp.publishGaugeInfo(name, i.Snapshot()) case metrics.Histogram: exp.publishHistogram(name, i) case metrics.Meter: diff --git a/metrics/gauge_info.go b/metrics/gauge_info.go index f1b2075939e7..c44b2d85f3ad 100644 --- a/metrics/gauge_info.go +++ b/metrics/gauge_info.go @@ -5,14 +5,17 @@ import ( "sync" ) +type GaugeInfoSnapshot interface { + Value() GaugeInfoValue +} + // GaugeInfos hold a GaugeInfoValue value that can be set arbitrarily. type GaugeInfo interface { - Snapshot() GaugeInfo Update(GaugeInfoValue) - Value() GaugeInfoValue + Snapshot() GaugeInfoSnapshot } -// GaugeInfoValue is a mappng of (string) keys to (string) values +// GaugeInfoValue is a mapping of keys to values type GaugeInfoValue map[string]string func (val GaugeInfoValue) String() string { @@ -49,49 +52,17 @@ func NewRegisteredGaugeInfo(name string, r Registry) GaugeInfo { return c } -// NewFunctionalGauge constructs a new FunctionalGauge. -func NewFunctionalGaugeInfo(f func() GaugeInfoValue) GaugeInfo { - if !Enabled { - return NilGaugeInfo{} - } - return &FunctionalGaugeInfo{value: f} -} - -// NewRegisteredFunctionalGauge constructs and registers a new StandardGauge. -func NewRegisteredFunctionalGaugeInfo(name string, r Registry, f func() GaugeInfoValue) GaugeInfo { - c := NewFunctionalGaugeInfo(f) - if nil == r { - r = DefaultRegistry - } - r.Register(name, c) - return c -} - -// GaugeInfoSnapshot is a read-only copy of another GaugeInfo. -type GaugeInfoSnapshot GaugeInfoValue - -// Snapshot returns the snapshot. -func (g GaugeInfoSnapshot) Snapshot() GaugeInfo { return g } - -// Update panics. -func (GaugeInfoSnapshot) Update(GaugeInfoValue) { - panic("Update called on a GaugeInfoSnapshot") -} +// gaugeInfoSnapshot is a read-only copy of another GaugeInfo. +type gaugeInfoSnapshot GaugeInfoValue // Value returns the value at the time the snapshot was taken. -func (g GaugeInfoSnapshot) Value() GaugeInfoValue { return GaugeInfoValue(g) } +func (g gaugeInfoSnapshot) Value() GaugeInfoValue { return GaugeInfoValue(g) } -// NilGauge is a no-op Gauge. type NilGaugeInfo struct{} -// Snapshot is a no-op. -func (NilGaugeInfo) Snapshot() GaugeInfo { return NilGaugeInfo{} } - -// Update is a no-op. -func (NilGaugeInfo) Update(v GaugeInfoValue) {} - -// Value is a no-op. -func (NilGaugeInfo) Value() GaugeInfoValue { return GaugeInfoValue{} } +func (NilGaugeInfo) Snapshot() GaugeInfoSnapshot { return NilGaugeInfo{} } +func (NilGaugeInfo) Update(v GaugeInfoValue) {} +func (NilGaugeInfo) Value() GaugeInfoValue { return GaugeInfoValue{} } // StandardGaugeInfo is the standard implementation of a GaugeInfo and uses // sync.Mutex to manage a single string value. @@ -101,8 +72,8 @@ type StandardGaugeInfo struct { } // Snapshot returns a read-only copy of the gauge. -func (g *StandardGaugeInfo) Snapshot() GaugeInfo { - return GaugeInfoSnapshot(g.Value()) +func (g *StandardGaugeInfo) Snapshot() GaugeInfoSnapshot { + return gaugeInfoSnapshot(g.value) } // Update updates the gauge's value. @@ -111,34 +82,3 @@ func (g *StandardGaugeInfo) Update(v GaugeInfoValue) { defer g.mutex.Unlock() g.value = v } - -// Value returns the gauge's current value. -func (g *StandardGaugeInfo) Value() GaugeInfoValue { - g.mutex.Lock() - defer g.mutex.Unlock() - return g.value -} - -// FunctionalGaugeInfo returns value from given function -type FunctionalGaugeInfo struct { - value func() GaugeInfoValue -} - -// Value returns the gauge's current value. -func (g FunctionalGaugeInfo) Value() GaugeInfoValue { - return g.value() -} - -// Value returns the gauge's current value in JSON string format -func (g FunctionalGaugeInfo) ValueJsonString() string { - data, _ := json.Marshal(g.value()) - return string(data) -} - -// Snapshot returns the snapshot. -func (g FunctionalGaugeInfo) Snapshot() GaugeInfo { return GaugeInfoSnapshot(g.Value()) } - -// Update panics. -func (FunctionalGaugeInfo) Update(GaugeInfoValue) { - panic("Update called on a FunctionalGaugeInfo") -} diff --git a/metrics/gauge_info_test.go b/metrics/gauge_info_test.go index 4227a6a85fab..e1197a8e9e65 100644 --- a/metrics/gauge_info_test.go +++ b/metrics/gauge_info_test.go @@ -1,7 +1,6 @@ package metrics import ( - "strconv" "testing" ) @@ -14,62 +13,25 @@ func TestGaugeInfoJsonString(t *testing.T) { }, ) want := `{"anotherKey":"any_string_value","chain_id":"5","third_key":"anything"}` - if have := g.Value().String(); have != want { - t.Errorf("\nhave: %v\nwant: %v\n", have, want) - } -} -func TestGaugeInfoSnapshot(t *testing.T) { - g := NewGaugeInfo() - g.Update(GaugeInfoValue{"value": "original"}) - snapshot := g.Snapshot() // Snapshot @chainid 5 + original := g.Snapshot() g.Update(GaugeInfoValue{"value": "updated"}) - // The 'g' should be updated - if have, want := g.Value().String(), `{"value":"updated"}`; have != want { + + if have := original.Value().String(); have != want { t.Errorf("\nhave: %v\nwant: %v\n", have, want) } - // Snapshot should be unupdated - if have, want := snapshot.Value().String(), `{"value":"original"}`; have != want { + if have, want := g.Snapshot().Value().String(), `{"value":"updated"}`; have != want { t.Errorf("\nhave: %v\nwant: %v\n", have, want) } + } func TestGetOrRegisterGaugeInfo(t *testing.T) { r := NewRegistry() NewRegisteredGaugeInfo("foo", r).Update( GaugeInfoValue{"chain_id": "5"}) - g := GetOrRegisterGaugeInfo("foo", r) + g := GetOrRegisterGaugeInfo("foo", r).Snapshot() if have, want := g.Value().String(), `{"chain_id":"5"}`; have != want { t.Errorf("have\n%v\nwant\n%v\n", have, want) } } - -func TestFunctionalGaugeInfo(t *testing.T) { - info := GaugeInfoValue{"chain_id": "0"} - counter := 1 - // A "functional" gauge invokes the method to obtain the value - fg := NewFunctionalGaugeInfo(func() GaugeInfoValue { - info["chain_id"] = strconv.Itoa(counter) - counter++ - return info - }) - fg.Value() - fg.Value() - if have, want := info["chain_id"], "2"; have != want { - t.Errorf("have %v want %v", have, want) - } -} - -func TestGetOrRegisterFunctionalGaugeInfo(t *testing.T) { - r := NewRegistry() - NewRegisteredFunctionalGaugeInfo("foo", r, func() GaugeInfoValue { - return GaugeInfoValue{ - "chain_id": "5", - } - }) - want := `{"chain_id":"5"}` - have := GetOrRegisterGaugeInfo("foo", r).Value().String() - if have != want { - t.Errorf("have\n%v\nwant\n%v\n", have, want) - } -} diff --git a/metrics/graphite.go b/metrics/graphite.go index 61a73e217f39..aba752e0ed5e 100644 --- a/metrics/graphite.go +++ b/metrics/graphite.go @@ -74,7 +74,7 @@ func graphite(c *GraphiteConfig) error { case GaugeFloat64: fmt.Fprintf(w, "%s.%s.value %f %d\n", c.Prefix, name, metric.Snapshot().Value(), now) case GaugeInfo: - fmt.Fprintf(w, "%s.%s.value %s %d\n", c.Prefix, name, metric.Value().String(), now) + fmt.Fprintf(w, "%s.%s.value %s %d\n", c.Prefix, name, metric.Snapshot().Value().String(), now) case Histogram: h := metric.Snapshot() ps := h.Percentiles(c.Percentiles) diff --git a/metrics/log.go b/metrics/log.go index 008903da940d..3b9773faa728 100644 --- a/metrics/log.go +++ b/metrics/log.go @@ -35,7 +35,7 @@ func LogScaled(r Registry, freq time.Duration, scale time.Duration, l Logger) { l.Printf(" value: %f\n", metric.Snapshot().Value()) case GaugeInfo: l.Printf("gauge %s\n", name) - l.Printf(" value: %s\n", metric.Value()) + l.Printf(" value: %s\n", metric.Snapshot().Value()) case Healthcheck: metric.Check() l.Printf("healthcheck %s\n", name) diff --git a/metrics/opentsdb.go b/metrics/opentsdb.go index 67534ff677a0..e81690f94340 100644 --- a/metrics/opentsdb.go +++ b/metrics/opentsdb.go @@ -73,7 +73,7 @@ func (c *OpenTSDBConfig) writeRegistry(w io.Writer, now int64, shortHostname str case GaugeFloat64: fmt.Fprintf(w, "put %s.%s.value %d %f host=%s\n", c.Prefix, name, now, metric.Snapshot().Value(), shortHostname) case GaugeInfo: - fmt.Fprintf(w, "put %s.%s.value %d %s host=%s\n", c.Prefix, name, now, metric.Value().String(), shortHostname) + fmt.Fprintf(w, "put %s.%s.value %d %s host=%s\n", c.Prefix, name, now, metric.Snapshot().Value().String(), shortHostname) case Histogram: h := metric.Snapshot() ps := h.Percentiles([]float64{0.5, 0.75, 0.95, 0.99, 0.999}) diff --git a/metrics/prometheus/collector.go b/metrics/prometheus/collector.go index d6a0b4200c18..25b258d56ab1 100644 --- a/metrics/prometheus/collector.go +++ b/metrics/prometheus/collector.go @@ -91,7 +91,7 @@ func (c *collector) addGaugeFloat64(name string, m metrics.GaugeFloat64Snapshot) c.writeGaugeCounter(name, m.Value()) } -func (c *collector) addGaugeInfo(name string, m metrics.GaugeInfo) { +func (c *collector) addGaugeInfo(name string, m metrics.GaugeInfoSnapshot) { c.writeGaugeInfo(name, m.Value()) } diff --git a/metrics/syslog.go b/metrics/syslog.go index 9fd89c5e2043..fd856d697316 100644 --- a/metrics/syslog.go +++ b/metrics/syslog.go @@ -24,7 +24,7 @@ func Syslog(r Registry, d time.Duration, w *syslog.Writer) { case GaugeFloat64: w.Info(fmt.Sprintf("gauge %s: value: %f", name, metric.Snapshot().Value())) case GaugeInfo: - w.Info(fmt.Sprintf("gauge %s: value: %s", name, metric.Value())) + w.Info(fmt.Sprintf("gauge %s: value: %s", name, metric.Snapshot().Value())) case Healthcheck: metric.Check() w.Info(fmt.Sprintf("healthcheck %s: error: %v", name, metric.Error())) diff --git a/metrics/writer.go b/metrics/writer.go index a36918b3b736..098da45c27b2 100644 --- a/metrics/writer.go +++ b/metrics/writer.go @@ -41,7 +41,7 @@ func WriteOnce(r Registry, w io.Writer) { fmt.Fprintf(w, " value: %f\n", metric.Snapshot().Value()) case GaugeInfo: fmt.Fprintf(w, "gauge %s\n", namedMetric.name) - fmt.Fprintf(w, " value: %s\n", metric.Value().String()) + fmt.Fprintf(w, " value: %s\n", metric.Snapshot().Value().String()) case Healthcheck: metric.Check() fmt.Fprintf(w, "healthcheck %s\n", namedMetric.name) From 4ae74271cac5645d840364d344571fc32fb715e9 Mon Sep 17 00:00:00 2001 From: Martin Holst Swende Date: Fri, 1 Sep 2023 08:37:07 +0200 Subject: [PATCH 13/26] metrics/infuxdb, metrics/prometheus: fix fallout from resetting-timer percentiles type change --- metrics/influxdb/influxdb_test.go | 2 +- metrics/influxdb/testdata/influxdbv1.want | 2 +- metrics/influxdb/testdata/influxdbv2.want | 2 +- metrics/internal/sampledata.go | 10 +++++++++- metrics/librato/librato.go | 4 ++-- metrics/prometheus/collector_test.go | 4 ++-- metrics/prometheus/testdata/prometheus.want | 6 +++--- 7 files changed, 19 insertions(+), 11 deletions(-) diff --git a/metrics/influxdb/influxdb_test.go b/metrics/influxdb/influxdb_test.go index beeb36a53156..c6f2eeac6277 100644 --- a/metrics/influxdb/influxdb_test.go +++ b/metrics/influxdb/influxdb_test.go @@ -96,7 +96,7 @@ func TestExampleV2(t *testing.T) { } if have != want { t.Errorf("\nhave:\n%v\nwant:\n%v\n", have, want) - t.Logf("have vs want:\n %v", findFirstDiffPos(have, want)) + t.Logf("have vs want:\n%v", findFirstDiffPos(have, want)) } } diff --git a/metrics/influxdb/testdata/influxdbv1.want b/metrics/influxdb/testdata/influxdbv1.want index 5efffb959504..3bf30737b906 100644 --- a/metrics/influxdb/testdata/influxdbv1.want +++ b/metrics/influxdb/testdata/influxdbv1.want @@ -5,5 +5,5 @@ goth.test/gauge_float64.gauge value=34567.89 978307200000000000 goth.test/gauge_info.gauge value="{\"arch\":\"amd64\",\"commit\":\"7caa2d8163ae3132c1c2d6978c76610caee2d949\",\"os\":\"linux\",\"protocol_versions\":\"64 65 66\",\"version\":\"1.10.18-unstable\"}" 978307200000000000 goth.test/histogram.histogram count=3i,max=3i,mean=2,min=1i,p25=1,p50=2,p75=3,p95=3,p99=3,p999=3,p9999=3,stddev=0.816496580927726,variance=0.6666666666666666 978307200000000000 goth.test/meter.meter count=0i,m1=0,m15=0,m5=0,mean=0 978307200000000000 -goth.test/resetting_timer.span count=6i,max=120000000i,mean=30000000,min=10000000i,p50=12000000i,p95=120000000i,p99=120000000i 978307200000000000 +goth.test/resetting_timer.span count=6i,max=120000000i,mean=30000000,min=10000000i,p50=12500000,p95=120000000,p99=120000000 978307200000000000 goth.test/timer.timer count=6i,m1=0,m15=0,m5=0,max=120000000i,mean=38333333.333333336,meanrate=0,min=20000000i,p50=22500000,p75=48000000,p95=120000000,p99=120000000,p999=120000000,p9999=120000000,stddev=36545253.529775314,variance=1335555555555555.2 978307200000000000 diff --git a/metrics/influxdb/testdata/influxdbv2.want b/metrics/influxdb/testdata/influxdbv2.want index 5efffb959504..3bf30737b906 100644 --- a/metrics/influxdb/testdata/influxdbv2.want +++ b/metrics/influxdb/testdata/influxdbv2.want @@ -5,5 +5,5 @@ goth.test/gauge_float64.gauge value=34567.89 978307200000000000 goth.test/gauge_info.gauge value="{\"arch\":\"amd64\",\"commit\":\"7caa2d8163ae3132c1c2d6978c76610caee2d949\",\"os\":\"linux\",\"protocol_versions\":\"64 65 66\",\"version\":\"1.10.18-unstable\"}" 978307200000000000 goth.test/histogram.histogram count=3i,max=3i,mean=2,min=1i,p25=1,p50=2,p75=3,p95=3,p99=3,p999=3,p9999=3,stddev=0.816496580927726,variance=0.6666666666666666 978307200000000000 goth.test/meter.meter count=0i,m1=0,m15=0,m5=0,mean=0 978307200000000000 -goth.test/resetting_timer.span count=6i,max=120000000i,mean=30000000,min=10000000i,p50=12000000i,p95=120000000i,p99=120000000i 978307200000000000 +goth.test/resetting_timer.span count=6i,max=120000000i,mean=30000000,min=10000000i,p50=12500000,p95=120000000,p99=120000000 978307200000000000 goth.test/timer.timer count=6i,m1=0,m15=0,m5=0,max=120000000i,mean=38333333.333333336,meanrate=0,min=20000000i,p50=22500000,p75=48000000,p95=120000000,p99=120000000,p999=120000000,p9999=120000000,stddev=36545253.529775314,variance=1335555555555555.2 978307200000000000 diff --git a/metrics/internal/sampledata.go b/metrics/internal/sampledata.go index 9ace06957695..fa5314241016 100644 --- a/metrics/internal/sampledata.go +++ b/metrics/internal/sampledata.go @@ -38,7 +38,15 @@ func ExampleMetrics() metrics.Registry { "commit": "7caa2d8163ae3132c1c2d6978c76610caee2d949", "protocol_versions": "64 65 66", }) - metrics.NewRegisteredHistogram("test/histogram", registry, metrics.NewSampleSnapshot(3, []int64{1, 2, 3})) + + { + s := metrics.NewUniformSample(3) + s.Update(1) + s.Update(2) + s.Update(3) + //metrics.NewRegisteredHistogram("test/histogram", registry, metrics.NewSampleSnapshot(3, []int64{1, 2, 3})) + metrics.NewRegisteredHistogram("test/histogram", registry, s) + } registry.Register("test/meter", metrics.NewInactiveMeter()) { timer := metrics.NewRegisteredResettingTimer("test/resetting_timer", registry) diff --git a/metrics/librato/librato.go b/metrics/librato/librato.go index e7385264e9b9..98303cfb5c7f 100644 --- a/metrics/librato/librato.go +++ b/metrics/librato/librato.go @@ -98,7 +98,7 @@ func (rep *Reporter) BuildRequest(now time.Time, r metrics.Registry) (snapshot B switch m := metric.(type) { case metrics.Counter: ms := m.Snapshot() - if m.Count() > 0 { + if ms.Count() > 0 { measurement[Name] = fmt.Sprintf("%s.%s", name, "count") measurement[Value] = float64(ms.Count()) measurement[Attributes] = map[string]interface{}{ @@ -129,7 +129,7 @@ func (rep *Reporter) BuildRequest(now time.Time, r metrics.Registry) (snapshot B snapshot.Gauges = append(snapshot.Gauges, measurement) case metrics.GaugeInfo: measurement[Name] = name - measurement[Value] = m.Value() + measurement[Value] = m.Snapshot().Value() snapshot.Gauges = append(snapshot.Gauges, measurement) case metrics.Histogram: ms := m.Snapshot() diff --git a/metrics/prometheus/collector_test.go b/metrics/prometheus/collector_test.go index 3d7903d4adfd..ea17aac4585f 100644 --- a/metrics/prometheus/collector_test.go +++ b/metrics/prometheus/collector_test.go @@ -55,10 +55,10 @@ func findFirstDiffPos(a, b string) string { yy := strings.Split(b, "\n") for i, x := range strings.Split(a, "\n") { if i >= len(yy) { - return fmt.Sprintf("a:%d: %s\nb:%d: ", i, x, i) + return fmt.Sprintf("have:%d: %s\nwant:%d: ", i, x, i) } if y := yy[i]; x != y { - return fmt.Sprintf("a:%d: %s\nb:%d: %s", i, x, i, y) + return fmt.Sprintf("have:%d: %s\nwant:%d: %s", i, x, i, y) } } return "" diff --git a/metrics/prometheus/testdata/prometheus.want b/metrics/prometheus/testdata/prometheus.want index f35496e61d31..fb03f862905b 100644 --- a/metrics/prometheus/testdata/prometheus.want +++ b/metrics/prometheus/testdata/prometheus.want @@ -31,9 +31,9 @@ test_meter 0 test_resetting_timer_count 6 # TYPE test_resetting_timer summary -test_resetting_timer {quantile="0.50"} 12000000 -test_resetting_timer {quantile="0.95"} 120000000 -test_resetting_timer {quantile="0.99"} 120000000 +test_resetting_timer {quantile="0.50"} 1.25e+07 +test_resetting_timer {quantile="0.95"} 1.2e+08 +test_resetting_timer {quantile="0.99"} 1.2e+08 # TYPE test_timer_count counter test_timer_count 6 From 9723df673d7d5a747f3cac7167d5e623eec13f20 Mon Sep 17 00:00:00 2001 From: Martin Holst Swende Date: Fri, 1 Sep 2023 11:13:41 +0200 Subject: [PATCH 14/26] metrics: optimize expdecaysample, avoid iteration of values --- metrics/sample.go | 54 +++++++++++++++++++++++++++++++++-------------- 1 file changed, 38 insertions(+), 16 deletions(-) diff --git a/metrics/sample.go b/metrics/sample.go index 20a7997478ba..ff0346bd9fa4 100644 --- a/metrics/sample.go +++ b/metrics/sample.go @@ -83,13 +83,26 @@ func (s *ExpDecaySample) Clear() { // Snapshot returns a read-only copy of the sample. func (s *ExpDecaySample) Snapshot() SampleSnapshot { s.mutex.Lock() + defer s.mutex.Unlock() vals := s.values.Values() values := make([]int64, len(vals)) - for i, v := range vals { - values[i] = v.v + var ( + max int64 = math.MinInt64 + min int64 = math.MaxInt64 + sum int64 + ) + for i, item := range vals { + v := item.v + values[i] = v + sum += v + if v > max { + max = v + } + if v < min { + min = v + } } - s.mutex.Unlock() - return newSampleSnapshot(s.count, values) + return newSampleSnapshotPrecalculated(s.count, values, min, max, sum) } // Update samples a new value. @@ -221,16 +234,29 @@ type sampleSnapshot struct { sum int64 } -// newSampleSnapshot creates a read-only sampleSnapShot, and calculates some -// numbers. -func newSampleSnapshot(count int64, values []int64) *sampleSnapshot { - s := &sampleSnapshot{ +// newSampleSnapshotPrecalculated creates a read-only sampleSnapShot, using +// precalculated sums to avoid iterating the values +func newSampleSnapshotPrecalculated(count int64, values []int64, + min, max, sum int64) *sampleSnapshot { + if len(values) == 0 { + return &sampleSnapshot{ + count: count, + values: values, + } + } + return &sampleSnapshot{ count: count, values: values, + max: max, + min: min, + mean: float64(sum) / float64(len(values)), + sum: sum, } - if len(values) == 0 { - return s - } +} + +// newSampleSnapshot creates a read-only sampleSnapShot, and calculates some +// numbers. +func newSampleSnapshot(count int64, values []int64) *sampleSnapshot { var ( max int64 = math.MinInt64 min int64 = math.MaxInt64 @@ -245,11 +271,7 @@ func newSampleSnapshot(count int64, values []int64) *sampleSnapshot { min = v } } - s.min = min - s.max = max - s.mean = float64(sum) / float64(len(values)) - s.sum = sum - return s + return newSampleSnapshotPrecalculated(count, values, min, max, sum) } // Count returns the count of inputs at the time the snapshot was taken. From 17765d22bbb8ac29b672225f0c6e22fae76f389b Mon Sep 17 00:00:00 2001 From: Martin Holst Swende Date: Fri, 1 Sep 2023 11:15:56 +0200 Subject: [PATCH 15/26] metrics: linter nitpicks --- metrics/gauge_info_test.go | 1 - metrics/sample_test.go | 1 - 2 files changed, 2 deletions(-) diff --git a/metrics/gauge_info_test.go b/metrics/gauge_info_test.go index e1197a8e9e65..319afbf92e8f 100644 --- a/metrics/gauge_info_test.go +++ b/metrics/gauge_info_test.go @@ -23,7 +23,6 @@ func TestGaugeInfoJsonString(t *testing.T) { if have, want := g.Snapshot().Value().String(), `{"value":"updated"}`; have != want { t.Errorf("\nhave: %v\nwant: %v\n", have, want) } - } func TestGetOrRegisterGaugeInfo(t *testing.T) { diff --git a/metrics/sample_test.go b/metrics/sample_test.go index e15ffed15b42..e735a42819ff 100644 --- a/metrics/sample_test.go +++ b/metrics/sample_test.go @@ -31,7 +31,6 @@ func BenchmarkCompute1000000(b *testing.B) { for i := 0; i < len(s); i++ { s[i] = int64(i) sum += int64(i) - } mean := float64(sum) / float64(len(s)) b.ResetTimer() From 40f291b0439a3e0498d20f328e29a4d98d3607e2 Mon Sep 17 00:00:00 2001 From: Martin Holst Swende Date: Fri, 1 Sep 2023 18:58:59 +0200 Subject: [PATCH 16/26] metrics: optimize resetting timer, make influx resetting-timer span type use integer format --- metrics/influxdb/influxdb.go | 6 +++--- metrics/influxdb/testdata/influxdbv1.want | 2 +- metrics/influxdb/testdata/influxdbv2.want | 2 +- metrics/resetting_timer.go | 25 ++++++++++------------- 4 files changed, 16 insertions(+), 19 deletions(-) diff --git a/metrics/influxdb/influxdb.go b/metrics/influxdb/influxdb.go index 227c2bd57292..bbc4fc024b34 100644 --- a/metrics/influxdb/influxdb.go +++ b/metrics/influxdb/influxdb.go @@ -109,9 +109,9 @@ func readMeter(namespace, name string, i interface{}) (string, map[string]interf "max": t.Max(), "mean": t.Mean(), "min": t.Min(), - "p50": ps[0], - "p95": ps[1], - "p99": ps[2], + "p50": int(ps[0]), + "p95": int(ps[1]), + "p99": int(ps[2]), } return measurement, fields } diff --git a/metrics/influxdb/testdata/influxdbv1.want b/metrics/influxdb/testdata/influxdbv1.want index 3bf30737b906..83ab88af67b4 100644 --- a/metrics/influxdb/testdata/influxdbv1.want +++ b/metrics/influxdb/testdata/influxdbv1.want @@ -5,5 +5,5 @@ goth.test/gauge_float64.gauge value=34567.89 978307200000000000 goth.test/gauge_info.gauge value="{\"arch\":\"amd64\",\"commit\":\"7caa2d8163ae3132c1c2d6978c76610caee2d949\",\"os\":\"linux\",\"protocol_versions\":\"64 65 66\",\"version\":\"1.10.18-unstable\"}" 978307200000000000 goth.test/histogram.histogram count=3i,max=3i,mean=2,min=1i,p25=1,p50=2,p75=3,p95=3,p99=3,p999=3,p9999=3,stddev=0.816496580927726,variance=0.6666666666666666 978307200000000000 goth.test/meter.meter count=0i,m1=0,m15=0,m5=0,mean=0 978307200000000000 -goth.test/resetting_timer.span count=6i,max=120000000i,mean=30000000,min=10000000i,p50=12500000,p95=120000000,p99=120000000 978307200000000000 +goth.test/resetting_timer.span count=6i,max=120000000i,mean=30000000,min=10000000i,p50=12500000i,p95=120000000i,p99=120000000i 978307200000000000 goth.test/timer.timer count=6i,m1=0,m15=0,m5=0,max=120000000i,mean=38333333.333333336,meanrate=0,min=20000000i,p50=22500000,p75=48000000,p95=120000000,p99=120000000,p999=120000000,p9999=120000000,stddev=36545253.529775314,variance=1335555555555555.2 978307200000000000 diff --git a/metrics/influxdb/testdata/influxdbv2.want b/metrics/influxdb/testdata/influxdbv2.want index 3bf30737b906..83ab88af67b4 100644 --- a/metrics/influxdb/testdata/influxdbv2.want +++ b/metrics/influxdb/testdata/influxdbv2.want @@ -5,5 +5,5 @@ goth.test/gauge_float64.gauge value=34567.89 978307200000000000 goth.test/gauge_info.gauge value="{\"arch\":\"amd64\",\"commit\":\"7caa2d8163ae3132c1c2d6978c76610caee2d949\",\"os\":\"linux\",\"protocol_versions\":\"64 65 66\",\"version\":\"1.10.18-unstable\"}" 978307200000000000 goth.test/histogram.histogram count=3i,max=3i,mean=2,min=1i,p25=1,p50=2,p75=3,p95=3,p99=3,p999=3,p9999=3,stddev=0.816496580927726,variance=0.6666666666666666 978307200000000000 goth.test/meter.meter count=0i,m1=0,m15=0,m5=0,mean=0 978307200000000000 -goth.test/resetting_timer.span count=6i,max=120000000i,mean=30000000,min=10000000i,p50=12500000,p95=120000000,p99=120000000 978307200000000000 +goth.test/resetting_timer.span count=6i,max=120000000i,mean=30000000,min=10000000i,p50=12500000i,p95=120000000i,p99=120000000i 978307200000000000 goth.test/timer.timer count=6i,m1=0,m15=0,m5=0,max=120000000i,mean=38333333.333333336,meanrate=0,min=20000000i,p50=22500000,p75=48000000,p95=120000000,p99=120000000,p999=120000000,p9999=120000000,stddev=36545253.529775314,variance=1335555555555555.2 978307200000000000 diff --git a/metrics/resetting_timer.go b/metrics/resetting_timer.go index 8fb27c8a5c7c..ef1162fed82f 100644 --- a/metrics/resetting_timer.go +++ b/metrics/resetting_timer.go @@ -71,19 +71,22 @@ func (NilResettingTimer) Count() int { return 0 } // and Meter. type StandardResettingTimer struct { values []int64 - mutex sync.Mutex + sum int64 // sum is a running count of the total sum, used later to calculate mean + + mutex sync.Mutex } // Snapshot resets the timer and returns a read-only copy of its contents. func (t *StandardResettingTimer) Snapshot() ResettingTimerSnapshot { t.mutex.Lock() defer t.mutex.Unlock() - currentValues := t.values - t.values = make([]int64, 0, InitialResettingTimerSliceCap) - - return &resettingTimerSnapshot{ - values: currentValues, + snapshot := &resettingTimerSnapshot{ + values: t.values, + mean: float64(t.sum) / float64(len(t.values)), } + t.values = make([]int64, 0, InitialResettingTimerSliceCap) + t.sum = 0 + return snapshot } // Record the duration of the execution of the given function. @@ -98,13 +101,12 @@ func (t *StandardResettingTimer) Update(d time.Duration) { t.mutex.Lock() defer t.mutex.Unlock() t.values = append(t.values, int64(d)) + t.sum += int64(d) } // Record the duration of an event that started at a time and ends now. func (t *StandardResettingTimer) UpdateSince(ts time.Time) { - t.mutex.Lock() - defer t.mutex.Unlock() - t.values = append(t.values, int64(time.Since(ts))) + t.Update(time.Since(ts)) } // resettingTimerSnapshot is a point-in-time copy of another ResettingTimer. @@ -165,9 +167,4 @@ func (t *resettingTimerSnapshot) calc(percentiles []float64) { } t.min = t.values[0] t.max = t.values[len(t.values)-1] - var sum int64 - for _, v := range t.values { - sum += v - } - t.mean = float64(sum) / float64(len(t.values)) } From 2ed1f05ff5471d395b1ea4aa96fb79ec40f7d957 Mon Sep 17 00:00:00 2001 From: Martin Holst Swende Date: Sat, 2 Sep 2023 13:43:08 +0200 Subject: [PATCH 17/26] metrics: split ewma, create dedicated nil snapshot type --- metrics/counter.go | 17 ++------- metrics/doc.go | 4 -- metrics/ewma.go | 50 +++++++------------------ metrics/ewma_test.go | 2 + metrics/gauge.go | 21 +++-------- metrics/histogram.go | 47 ++--------------------- metrics/inactive.go | 48 ++++++++++++++++++++++++ metrics/librato/librato.go | 19 +++++----- metrics/meter.go | 33 ++++------------ metrics/runtimehistogram.go | 4 -- metrics/sample.go | 46 ++--------------------- metrics/sample_test.go | 2 + metrics/timer.go | 75 ++++--------------------------------- 13 files changed, 104 insertions(+), 264 deletions(-) delete mode 100644 metrics/doc.go create mode 100644 metrics/inactive.go diff --git a/metrics/counter.go b/metrics/counter.go index ada642eeec24..05ec419585ca 100644 --- a/metrics/counter.go +++ b/metrics/counter.go @@ -83,19 +83,10 @@ func (c counterSnapshot) Count() int64 { return int64(c) } type NilCounter struct{} // Clear is a no-op. -func (NilCounter) Clear() {} - -// Count is a no-op. -func (NilCounter) Count() int64 { return 0 } - -// Dec is a no-op. -func (NilCounter) Dec(i int64) {} - -// Inc is a no-op. -func (NilCounter) Inc(i int64) {} - -// Snapshot is a no-op. -func (NilCounter) Snapshot() CounterSnapshot { return NilCounter{} } +func (NilCounter) Clear() {} +func (NilCounter) Dec(i int64) {} +func (NilCounter) Inc(i int64) {} +func (NilCounter) Snapshot() CounterSnapshot { return (*emptySnapshot)(nil) } // StandardCounter is the standard implementation of a Counter and uses the // sync/atomic package to manage a single int64 value. diff --git a/metrics/doc.go b/metrics/doc.go deleted file mode 100644 index 13f429c1689d..000000000000 --- a/metrics/doc.go +++ /dev/null @@ -1,4 +0,0 @@ -package metrics - -const epsilon = 0.0000000000000001 -const epsilonPercentile = .00000000001 diff --git a/metrics/ewma.go b/metrics/ewma.go index ffe4fe83a54d..e695c48d8256 100644 --- a/metrics/ewma.go +++ b/metrics/ewma.go @@ -7,11 +7,14 @@ import ( "time" ) +type EWMASnapshot interface { + Rate() float64 +} + // EWMAs continuously calculate an exponentially-weighted moving average // based on an outside source of clock ticks. type EWMA interface { - Rate() float64 - Snapshot() EWMA + Snapshot() EWMASnapshot Tick() Update(int64) } @@ -36,40 +39,19 @@ func NewEWMA15() EWMA { return NewEWMA(1 - math.Exp(-5.0/60.0/15)) } -// EWMASnapshot is a read-only copy of another EWMA. -type EWMASnapshot float64 +// eWMASnapshot is a read-only copy of another EWMA. +type eWMASnapshot float64 // Rate returns the rate of events per second at the time the snapshot was // taken. -func (a EWMASnapshot) Rate() float64 { return float64(a) } - -// Snapshot returns the snapshot. -func (a EWMASnapshot) Snapshot() EWMA { return a } - -// Tick panics. -func (EWMASnapshot) Tick() { - panic("Tick called on an EWMASnapshot") -} - -// Update panics. -func (EWMASnapshot) Update(int64) { - panic("Update called on an EWMASnapshot") -} +func (a eWMASnapshot) Rate() float64 { return float64(a) } // NilEWMA is a no-op EWMA. type NilEWMA struct{} -// Rate is a no-op. -func (NilEWMA) Rate() float64 { return 0.0 } - -// Snapshot is a no-op. -func (NilEWMA) Snapshot() EWMA { return NilEWMA{} } - -// Tick is a no-op. -func (NilEWMA) Tick() {} - -// Update is a no-op. -func (NilEWMA) Update(n int64) {} +func (NilEWMA) Snapshot() EWMASnapshot { return (*emptySnapshot)(nil) } +func (NilEWMA) Tick() {} +func (NilEWMA) Update(n int64) {} // StandardEWMA is the standard implementation of an EWMA and tracks the number // of uncounted events and processes them on each tick. It uses the @@ -82,14 +64,10 @@ type StandardEWMA struct { mutex sync.Mutex } -// Rate returns the moving average rate of events per second. -func (a *StandardEWMA) Rate() float64 { - return math.Float64frombits(a.rate.Load()) * float64(time.Second) -} - // Snapshot returns a read-only copy of the EWMA. -func (a *StandardEWMA) Snapshot() EWMA { - return EWMASnapshot(a.Rate()) +func (a *StandardEWMA) Snapshot() EWMASnapshot { + r := math.Float64frombits(a.rate.Load()) * float64(time.Second) + return eWMASnapshot(r) } // Tick ticks the clock to update the moving average. It assumes it is called diff --git a/metrics/ewma_test.go b/metrics/ewma_test.go index 73e77f852f76..10c50509872e 100644 --- a/metrics/ewma_test.go +++ b/metrics/ewma_test.go @@ -5,6 +5,8 @@ import ( "testing" ) +const epsilon = 0.0000000000000001 + func BenchmarkEWMA(b *testing.B) { a := NewEWMA1() b.ResetTimer() diff --git a/metrics/gauge.go b/metrics/gauge.go index c4aef478a5f2..d1a56bbddaf8 100644 --- a/metrics/gauge.go +++ b/metrics/gauge.go @@ -52,22 +52,11 @@ func (g gaugeSnapshot) Value() int64 { return int64(g) } // NilGauge is a no-op Gauge. type NilGauge struct{} -// Snapshot is a no-op. -func (NilGauge) Snapshot() GaugeSnapshot { return NilGauge{} } - -// Update is a no-op. -func (NilGauge) Update(v int64) {} - -func (NilGauge) UpdateIfGt(v int64) {} - -// Dec is a no-op. -func (NilGauge) Dec(i int64) {} - -// Inc is a no-op. -func (NilGauge) Inc(i int64) {} - -// Value is a no-op. -func (NilGauge) Value() int64 { return 0 } +func (NilGauge) Snapshot() GaugeSnapshot { return (*emptySnapshot)(nil) } +func (NilGauge) Update(v int64) {} +func (NilGauge) UpdateIfGt(v int64) {} +func (NilGauge) Dec(i int64) {} +func (NilGauge) Inc(i int64) {} // StandardGauge is the standard implementation of a Gauge and uses the // sync/atomic package to manage a single int64 value. diff --git a/metrics/histogram.go b/metrics/histogram.go index 5f7e2996247a..ef146d8f4de4 100644 --- a/metrics/histogram.go +++ b/metrics/histogram.go @@ -7,7 +7,6 @@ type HistogramSnapshot interface { Min() int64 Percentile(float64) float64 Percentiles([]float64) []float64 - Sample() SampleSnapshot StdDev() float64 Sum() int64 Variance() float64 @@ -90,9 +89,6 @@ func (h *histogramSnapshot) Percentiles(ps []float64) []float64 { return h.sample.Percentiles(ps) } -// Sample returns the Sample underlying the histogram. -func (h *histogramSnapshot) Sample() SampleSnapshot { return h.sample } - // Snapshot returns the snapshot. func (h *histogramSnapshot) Snapshot() HistogramSnapshot { return h } @@ -109,46 +105,9 @@ func (h *histogramSnapshot) Variance() float64 { return h.sample.Variance() } // NilHistogram is a no-op Histogram. type NilHistogram struct{} -// Clear is a no-op. -func (NilHistogram) Clear() {} - -// Count is a no-op. -func (NilHistogram) Count() int64 { return 0 } - -// Max is a no-op. -func (NilHistogram) Max() int64 { return 0 } - -// Mean is a no-op. -func (NilHistogram) Mean() float64 { return 0.0 } - -// Min is a no-op. -func (NilHistogram) Min() int64 { return 0 } - -// Percentile is a no-op. -func (NilHistogram) Percentile(p float64) float64 { return 0.0 } - -// Percentiles is a no-op. -func (NilHistogram) Percentiles(ps []float64) []float64 { - return make([]float64, len(ps)) -} - -// Sample is a no-op. -func (NilHistogram) Sample() SampleSnapshot { return NilSample{} } - -// Snapshot is a no-op. -func (NilHistogram) Snapshot() HistogramSnapshot { return NilHistogram{} } - -// StdDev is a no-op. -func (NilHistogram) StdDev() float64 { return 0.0 } - -// Sum is a no-op. -func (NilHistogram) Sum() int64 { return 0 } - -// Update is a no-op. -func (NilHistogram) Update(v int64) {} - -// Variance is a no-op. -func (NilHistogram) Variance() float64 { return 0.0 } +func (NilHistogram) Clear() {} +func (NilHistogram) Snapshot() HistogramSnapshot { return (*emptySnapshot)(nil) } +func (NilHistogram) Update(v int64) {} // StandardHistogram is the standard implementation of a Histogram and uses a // Sample to bound its memory use. diff --git a/metrics/inactive.go b/metrics/inactive.go new file mode 100644 index 000000000000..1f47f0210af3 --- /dev/null +++ b/metrics/inactive.go @@ -0,0 +1,48 @@ +// Copyright 2023 The go-ethereum Authors +// This file is part of the go-ethereum library. +// +// The go-ethereum library is free software: you can redistribute it and/or modify +// it under the terms of the GNU Lesser General Public License as published by +// the Free Software Foundation, either version 3 of the License, or +// (at your option) any later version. +// +// The go-ethereum library is distributed in the hope that it will be useful, +// but WITHOUT ANY WARRANTY; without even the implied warranty of +// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +// GNU Lesser General Public License for more details. +// +// You should have received a copy of the GNU Lesser General Public License +// along with the go-ethereum library. If not, see . + +package metrics + +// compile-time checks that interfaces are implemented. +var ( + _ SampleSnapshot = (*emptySnapshot)(nil) + _ HistogramSnapshot = (*emptySnapshot)(nil) + _ CounterSnapshot = (*emptySnapshot)(nil) + _ GaugeSnapshot = (*emptySnapshot)(nil) + _ MeterSnapshot = (*emptySnapshot)(nil) + _ EWMASnapshot = (*emptySnapshot)(nil) + _ TimerSnapshot = (*emptySnapshot)(nil) +) + +type emptySnapshot struct{} + +func (*emptySnapshot) Count() int64 { return 0 } +func (*emptySnapshot) Max() int64 { return 0 } +func (*emptySnapshot) Mean() float64 { return 0.0 } +func (*emptySnapshot) Min() int64 { return 0 } +func (*emptySnapshot) Percentile(p float64) float64 { return 0.0 } +func (*emptySnapshot) Percentiles(ps []float64) []float64 { return make([]float64, len(ps)) } +func (*emptySnapshot) Size() int { return 0 } +func (*emptySnapshot) StdDev() float64 { return 0.0 } +func (*emptySnapshot) Sum() int64 { return 0 } +func (*emptySnapshot) Values() []int64 { return []int64{} } +func (*emptySnapshot) Variance() float64 { return 0.0 } +func (*emptySnapshot) Value() int64 { return 0 } +func (*emptySnapshot) Rate() float64 { return 0.0 } +func (*emptySnapshot) Rate1() float64 { return 0.0 } +func (*emptySnapshot) Rate5() float64 { return 0.0 } +func (*emptySnapshot) Rate15() float64 { return 0.0 } +func (*emptySnapshot) RateMean() float64 { return 0.0 } diff --git a/metrics/librato/librato.go b/metrics/librato/librato.go index 98303cfb5c7f..cd3e00e28d2e 100644 --- a/metrics/librato/librato.go +++ b/metrics/librato/librato.go @@ -61,10 +61,10 @@ func (rep *Reporter) Run() { // calculate sum of squares from data provided by metrics.Histogram // see http://en.wikipedia.org/wiki/Standard_deviation#Rapid_calculation_methods -func sumSquares(s metrics.SampleSnapshot) float64 { - count := float64(s.Count()) - sumSquared := math.Pow(count*s.Mean(), 2) - sumSquares := math.Pow(count*s.StdDev(), 2) + sumSquared/count +func sumSquares(icount, mean, stDev) float64 { + count := float64(icount) + sumSquared := math.Pow(count*mean, 2) + sumSquares := math.Pow(count*stDev, 2) + sumSquared/count if math.IsNaN(sumSquares) { return 0.0 } @@ -135,13 +135,12 @@ func (rep *Reporter) BuildRequest(now time.Time, r metrics.Registry) (snapshot B ms := m.Snapshot() if ms.Count() > 0 { gauges := make([]Measurement, histogramGaugeCount) - s := ms.Sample() measurement[Name] = fmt.Sprintf("%s.%s", name, "hist") - measurement[Count] = uint64(s.Count()) - measurement[Max] = float64(s.Max()) - measurement[Min] = float64(s.Min()) - measurement[Sum] = float64(s.Sum()) - measurement[SumSquares] = sumSquares(s) + measurement[Count] = uint64(ms.Count()) + measurement[Max] = float64(ms.Max()) + measurement[Min] = float64(ms.Min()) + measurement[Sum] = float64(ms.Sum()) + measurement[SumSquares] = sumSquares(ms.Count(), ms.Mean(), ms.StdDev()) gauges[0] = measurement for i, p := range rep.Percentiles { gauges[i+1] = Measurement{ diff --git a/metrics/meter.go b/metrics/meter.go index 28c86f35852c..22475ef6ebee 100644 --- a/metrics/meter.go +++ b/metrics/meter.go @@ -97,29 +97,10 @@ func (m *meterSnapshot) RateMean() float64 { return m.rateMean } // NilMeter is a no-op Meter. type NilMeter struct{} -// Count is a no-op. -func (NilMeter) Count() int64 { return 0 } - -// Mark is a no-op. -func (NilMeter) Mark(n int64) {} - -// Rate1 is a no-op. -func (NilMeter) Rate1() float64 { return 0.0 } - -// Rate5 is a no-op. -func (NilMeter) Rate5() float64 { return 0.0 } - -// Rate15 is a no-op. -func (NilMeter) Rate15() float64 { return 0.0 } - -// RateMean is a no-op. -func (NilMeter) RateMean() float64 { return 0.0 } - -// Snapshot is a no-op. -func (NilMeter) Snapshot() MeterSnapshot { return NilMeter{} } - -// Stop is a no-op. -func (NilMeter) Stop() {} +func (NilMeter) Count() int64 { return 0 } +func (NilMeter) Mark(n int64) {} +func (NilMeter) Snapshot() MeterSnapshot { return (*emptySnapshot)(nil) } +func (NilMeter) Stop() {} // StandardMeter is the standard implementation of a Meter. type StandardMeter struct { @@ -159,9 +140,9 @@ func (m *StandardMeter) Mark(n int64) { func (m *StandardMeter) Snapshot() MeterSnapshot { return &meterSnapshot{ count: m.count.Load() + m.uncounted.Load(), - rate1: m.a1.Rate(), - rate5: m.a5.Rate(), - rate15: m.a15.Rate(), + rate1: m.a1.Snapshot().Rate(), + rate5: m.a5.Snapshot().Rate(), + rate15: m.a15.Snapshot().Rate(), rateMean: math.Float64frombits(m.rateMean.Load()), } } diff --git a/metrics/runtimehistogram.go b/metrics/runtimehistogram.go index 6abdb67d59d9..a074b88e0d06 100644 --- a/metrics/runtimehistogram.go +++ b/metrics/runtimehistogram.go @@ -65,10 +65,6 @@ func (h *runtimeHistogram) Snapshot() HistogramSnapshot { type runtimeHistogramSnapshot metrics.Float64Histogram -func (h *runtimeHistogramSnapshot) Sample() SampleSnapshot { - return NilSample{} -} - func (h *runtimeHistogramSnapshot) Snapshot() HistogramSnapshot { return h } diff --git a/metrics/sample.go b/metrics/sample.go index ff0346bd9fa4..a5713edacbe0 100644 --- a/metrics/sample.go +++ b/metrics/sample.go @@ -145,49 +145,9 @@ func (s *ExpDecaySample) update(t time.Time, v int64) { // NilSample is a no-op Sample. type NilSample struct{} -// Clear is a no-op. -func (NilSample) Clear() {} - -// Count is a no-op. -func (NilSample) Count() int64 { return 0 } - -// Max is a no-op. -func (NilSample) Max() int64 { return 0 } - -// Mean is a no-op. -func (NilSample) Mean() float64 { return 0.0 } - -// Min is a no-op. -func (NilSample) Min() int64 { return 0 } - -// Percentile is a no-op. -func (NilSample) Percentile(p float64) float64 { return 0.0 } - -// Percentiles is a no-op. -func (NilSample) Percentiles(ps []float64) []float64 { - return make([]float64, len(ps)) -} - -// Size is a no-op. -func (NilSample) Size() int { return 0 } - -// Sample is a no-op. -func (NilSample) Snapshot() SampleSnapshot { return NilSample{} } - -// StdDev is a no-op. -func (NilSample) StdDev() float64 { return 0.0 } - -// Sum is a no-op. -func (NilSample) Sum() int64 { return 0 } - -// Update is a no-op. -func (NilSample) Update(v int64) {} - -// Values is a no-op. -func (NilSample) Values() []int64 { return []int64{} } - -// Variance is a no-op. -func (NilSample) Variance() float64 { return 0.0 } +func (NilSample) Clear() {} +func (NilSample) Snapshot() SampleSnapshot { return (*emptySnapshot)(nil) } +func (NilSample) Update(v int64) {} // CalculatePercentiles returns an arbitrary percentile of the slice of int64. func SamplePercentile(values []int64, p float64) float64 { diff --git a/metrics/sample_test.go b/metrics/sample_test.go index e735a42819ff..f0edfbcae7f4 100644 --- a/metrics/sample_test.go +++ b/metrics/sample_test.go @@ -8,6 +8,8 @@ import ( "time" ) +const epsilonPercentile = .00000000001 + // Benchmark{Compute,Copy}{1000,1000000} demonstrate that, even for relatively // expensive computations like Variance, the cost of copying the Sample, as // approximated by a make and copy, is much greater than the cost of the diff --git a/metrics/timer.go b/metrics/timer.go index c35bbc01b4e3..26d1bb81d2da 100644 --- a/metrics/timer.go +++ b/metrics/timer.go @@ -6,19 +6,8 @@ import ( ) type TimerSnapshot interface { - Count() int64 - Max() int64 - Mean() float64 - Min() int64 - Percentile(float64) float64 - Percentiles([]float64) []float64 - Rate1() float64 - Rate5() float64 - Rate15() float64 - RateMean() float64 - Sum() int64 - Variance() float64 - StdDev() float64 + HistogramSnapshot + MeterSnapshot } // Timers capture the duration and rate of events. @@ -81,61 +70,11 @@ func NewTimer() Timer { // NilTimer is a no-op Timer. type NilTimer struct{} -// Count is a no-op. -func (NilTimer) Count() int64 { return 0 } - -// Max is a no-op. -func (NilTimer) Max() int64 { return 0 } - -// Mean is a no-op. -func (NilTimer) Mean() float64 { return 0.0 } - -// Min is a no-op. -func (NilTimer) Min() int64 { return 0 } - -// Percentile is a no-op. -func (NilTimer) Percentile(p float64) float64 { return 0.0 } - -// Percentiles is a no-op. -func (NilTimer) Percentiles(ps []float64) []float64 { - return make([]float64, len(ps)) -} - -// Rate1 is a no-op. -func (NilTimer) Rate1() float64 { return 0.0 } - -// Rate5 is a no-op. -func (NilTimer) Rate5() float64 { return 0.0 } - -// Rate15 is a no-op. -func (NilTimer) Rate15() float64 { return 0.0 } - -// RateMean is a no-op. -func (NilTimer) RateMean() float64 { return 0.0 } - -// Snapshot is a no-op. -func (NilTimer) Snapshot() TimerSnapshot { return NilTimer{} } - -// StdDev is a no-op. -func (NilTimer) StdDev() float64 { return 0.0 } - -// Stop is a no-op. -func (NilTimer) Stop() {} - -// Sum is a no-op. -func (NilTimer) Sum() int64 { return 0 } - -// Time is a no-op. -func (NilTimer) Time(f func()) { f() } - -// Update is a no-op. -func (NilTimer) Update(time.Duration) {} - -// UpdateSince is a no-op. -func (NilTimer) UpdateSince(time.Time) {} - -// Variance is a no-op. -func (NilTimer) Variance() float64 { return 0.0 } +func (NilTimer) Snapshot() TimerSnapshot { return (*emptySnapshot)(nil) } +func (NilTimer) Stop() {} +func (NilTimer) Time(f func()) { f() } +func (NilTimer) Update(time.Duration) {} +func (NilTimer) UpdateSince(time.Time) {} // StandardTimer is the standard implementation of a Timer and uses a Histogram // and Meter. From abefcdc76de333a75e971794dd68c6137adacadc Mon Sep 17 00:00:00 2001 From: Martin Holst Swende Date: Sun, 3 Sep 2023 20:30:50 +0200 Subject: [PATCH 18/26] metrics: small leftover fixes --- metrics/ewma_test.go | 219 ++++++------------------------------- metrics/librato/librato.go | 4 +- 2 files changed, 35 insertions(+), 188 deletions(-) diff --git a/metrics/ewma_test.go b/metrics/ewma_test.go index 10c50509872e..9a91b43db81a 100644 --- a/metrics/ewma_test.go +++ b/metrics/ewma_test.go @@ -32,68 +32,17 @@ func TestEWMA1(t *testing.T) { a := NewEWMA1() a.Update(3) a.Tick() - if rate := a.Rate(); math.Abs(0.6-rate) > epsilon { - t.Errorf("initial a.Rate(): 0.6 != %v\n", rate) - } - elapseMinute(a) - if rate := a.Rate(); math.Abs(0.22072766470286553-rate) > epsilon { - t.Errorf("1 minute a.Rate(): 0.22072766470286553 != %v\n", rate) - } - elapseMinute(a) - if rate := a.Rate(); math.Abs(0.08120116994196772-rate) > epsilon { - t.Errorf("2 minute a.Rate(): 0.08120116994196772 != %v\n", rate) - } - elapseMinute(a) - if rate := a.Rate(); math.Abs(0.029872241020718428-rate) > epsilon { - t.Errorf("3 minute a.Rate(): 0.029872241020718428 != %v\n", rate) - } - elapseMinute(a) - if rate := a.Rate(); math.Abs(0.01098938333324054-rate) > epsilon { - t.Errorf("4 minute a.Rate(): 0.01098938333324054 != %v\n", rate) - } - elapseMinute(a) - if rate := a.Rate(); math.Abs(0.004042768199451294-rate) > epsilon { - t.Errorf("5 minute a.Rate(): 0.004042768199451294 != %v\n", rate) - } - elapseMinute(a) - if rate := a.Rate(); math.Abs(0.0014872513059998212-rate) > epsilon { - t.Errorf("6 minute a.Rate(): 0.0014872513059998212 != %v\n", rate) - } - elapseMinute(a) - if rate := a.Rate(); math.Abs(0.0005471291793327122-rate) > epsilon { - t.Errorf("7 minute a.Rate(): 0.0005471291793327122 != %v\n", rate) - } - elapseMinute(a) - if rate := a.Rate(); math.Abs(0.00020127757674150815-rate) > epsilon { - t.Errorf("8 minute a.Rate(): 0.00020127757674150815 != %v\n", rate) - } - elapseMinute(a) - if rate := a.Rate(); math.Abs(7.404588245200814e-05-rate) > epsilon { - t.Errorf("9 minute a.Rate(): 7.404588245200814e-05 != %v\n", rate) - } - elapseMinute(a) - if rate := a.Rate(); math.Abs(2.7239957857491083e-05-rate) > epsilon { - t.Errorf("10 minute a.Rate(): 2.7239957857491083e-05 != %v\n", rate) - } - elapseMinute(a) - if rate := a.Rate(); math.Abs(1.0021020474147462e-05-rate) > epsilon { - t.Errorf("11 minute a.Rate(): 1.0021020474147462e-05 != %v\n", rate) - } - elapseMinute(a) - if rate := a.Rate(); math.Abs(3.6865274119969525e-06-rate) > epsilon { - t.Errorf("12 minute a.Rate(): 3.6865274119969525e-06 != %v\n", rate) - } - elapseMinute(a) - if rate := a.Rate(); math.Abs(1.3561976441886433e-06-rate) > epsilon { - t.Errorf("13 minute a.Rate(): 1.3561976441886433e-06 != %v\n", rate) - } - elapseMinute(a) - if rate := a.Rate(); math.Abs(4.989172314621449e-07-rate) > epsilon { - t.Errorf("14 minute a.Rate(): 4.989172314621449e-07 != %v\n", rate) - } - elapseMinute(a) - if rate := a.Rate(); math.Abs(1.8354139230109722e-07-rate) > epsilon { - t.Errorf("15 minute a.Rate(): 1.8354139230109722e-07 != %v\n", rate) + for i, want := range []float64{0.6, + 0.22072766470286553, 0.08120116994196772, 0.029872241020718428, + 0.01098938333324054, 0.004042768199451294, 0.0014872513059998212, + 0.0005471291793327122, 0.00020127757674150815, 7.404588245200814e-05, + 2.7239957857491083e-05, 1.0021020474147462e-05, 3.6865274119969525e-06, + 1.3561976441886433e-06, 4.989172314621449e-07, 1.8354139230109722e-07, + } { + if rate := a.Snapshot().Rate(); math.Abs(want-rate) > epsilon { + t.Errorf("%d minute a.Snapshot().Rate(): %f != %v\n", i, want, rate) + } + elapseMinute(a) } } @@ -101,68 +50,17 @@ func TestEWMA5(t *testing.T) { a := NewEWMA5() a.Update(3) a.Tick() - if rate := a.Rate(); math.Abs(0.6-rate) > epsilon { - t.Errorf("initial a.Rate(): 0.6 != %v\n", rate) - } - elapseMinute(a) - if rate := a.Rate(); math.Abs(0.49123845184678905-rate) > epsilon { - t.Errorf("1 minute a.Rate(): 0.49123845184678905 != %v\n", rate) - } - elapseMinute(a) - if rate := a.Rate(); math.Abs(0.4021920276213837-rate) > epsilon { - t.Errorf("2 minute a.Rate(): 0.4021920276213837 != %v\n", rate) - } - elapseMinute(a) - if rate := a.Rate(); math.Abs(0.32928698165641596-rate) > epsilon { - t.Errorf("3 minute a.Rate(): 0.32928698165641596 != %v\n", rate) - } - elapseMinute(a) - if rate := a.Rate(); math.Abs(0.269597378470333-rate) > epsilon { - t.Errorf("4 minute a.Rate(): 0.269597378470333 != %v\n", rate) - } - elapseMinute(a) - if rate := a.Rate(); math.Abs(0.2207276647028654-rate) > epsilon { - t.Errorf("5 minute a.Rate(): 0.2207276647028654 != %v\n", rate) - } - elapseMinute(a) - if rate := a.Rate(); math.Abs(0.18071652714732128-rate) > epsilon { - t.Errorf("6 minute a.Rate(): 0.18071652714732128 != %v\n", rate) - } - elapseMinute(a) - if rate := a.Rate(); math.Abs(0.14795817836496392-rate) > epsilon { - t.Errorf("7 minute a.Rate(): 0.14795817836496392 != %v\n", rate) - } - elapseMinute(a) - if rate := a.Rate(); math.Abs(0.12113791079679326-rate) > epsilon { - t.Errorf("8 minute a.Rate(): 0.12113791079679326 != %v\n", rate) - } - elapseMinute(a) - if rate := a.Rate(); math.Abs(0.09917933293295193-rate) > epsilon { - t.Errorf("9 minute a.Rate(): 0.09917933293295193 != %v\n", rate) - } - elapseMinute(a) - if rate := a.Rate(); math.Abs(0.08120116994196763-rate) > epsilon { - t.Errorf("10 minute a.Rate(): 0.08120116994196763 != %v\n", rate) - } - elapseMinute(a) - if rate := a.Rate(); math.Abs(0.06648189501740036-rate) > epsilon { - t.Errorf("11 minute a.Rate(): 0.06648189501740036 != %v\n", rate) - } - elapseMinute(a) - if rate := a.Rate(); math.Abs(0.05443077197364752-rate) > epsilon { - t.Errorf("12 minute a.Rate(): 0.05443077197364752 != %v\n", rate) - } - elapseMinute(a) - if rate := a.Rate(); math.Abs(0.04456414692860035-rate) > epsilon { - t.Errorf("13 minute a.Rate(): 0.04456414692860035 != %v\n", rate) - } - elapseMinute(a) - if rate := a.Rate(); math.Abs(0.03648603757513079-rate) > epsilon { - t.Errorf("14 minute a.Rate(): 0.03648603757513079 != %v\n", rate) - } - elapseMinute(a) - if rate := a.Rate(); math.Abs(0.0298722410207183831020718428-rate) > epsilon { - t.Errorf("15 minute a.Rate(): 0.0298722410207183831020718428 != %v\n", rate) + for i, want := range []float64{ + 0.6, 0.49123845184678905, 0.4021920276213837, 0.32928698165641596, + 0.269597378470333, 0.2207276647028654, 0.18071652714732128, + 0.14795817836496392, 0.12113791079679326, 0.09917933293295193, + 0.08120116994196763, 0.06648189501740036, 0.05443077197364752, + 0.04456414692860035, 0.03648603757513079, 0.0298722410207183831020718428, + } { + if rate := a.Snapshot().Rate(); math.Abs(want-rate) > epsilon { + t.Errorf("%d minute a.Snapshot().Rate(): %f != %v\n", i, want, rate) + } + elapseMinute(a) } } @@ -170,68 +68,17 @@ func TestEWMA15(t *testing.T) { a := NewEWMA15() a.Update(3) a.Tick() - if rate := a.Rate(); math.Abs(0.6-rate) > epsilon { - t.Errorf("initial a.Rate(): 0.6 != %v\n", rate) - } - elapseMinute(a) - if rate := a.Rate(); math.Abs(0.5613041910189706-rate) > epsilon { - t.Errorf("1 minute a.Rate(): 0.5613041910189706 != %v\n", rate) - } - elapseMinute(a) - if rate := a.Rate(); math.Abs(0.5251039914257684-rate) > epsilon { - t.Errorf("2 minute a.Rate(): 0.5251039914257684 != %v\n", rate) - } - elapseMinute(a) - if rate := a.Rate(); math.Abs(0.4912384518467888184678905-rate) > epsilon { - t.Errorf("3 minute a.Rate(): 0.4912384518467888184678905 != %v\n", rate) - } - elapseMinute(a) - if rate := a.Rate(); math.Abs(0.459557003018789-rate) > epsilon { - t.Errorf("4 minute a.Rate(): 0.459557003018789 != %v\n", rate) - } - elapseMinute(a) - if rate := a.Rate(); math.Abs(0.4299187863442732-rate) > epsilon { - t.Errorf("5 minute a.Rate(): 0.4299187863442732 != %v\n", rate) - } - elapseMinute(a) - if rate := a.Rate(); math.Abs(0.4021920276213831-rate) > epsilon { - t.Errorf("6 minute a.Rate(): 0.4021920276213831 != %v\n", rate) - } - elapseMinute(a) - if rate := a.Rate(); math.Abs(0.37625345116383313-rate) > epsilon { - t.Errorf("7 minute a.Rate(): 0.37625345116383313 != %v\n", rate) - } - elapseMinute(a) - if rate := a.Rate(); math.Abs(0.3519877317060185-rate) > epsilon { - t.Errorf("8 minute a.Rate(): 0.3519877317060185 != %v\n", rate) - } - elapseMinute(a) - if rate := a.Rate(); math.Abs(0.3292869816564153165641596-rate) > epsilon { - t.Errorf("9 minute a.Rate(): 0.3292869816564153165641596 != %v\n", rate) - } - elapseMinute(a) - if rate := a.Rate(); math.Abs(0.3080502714195546-rate) > epsilon { - t.Errorf("10 minute a.Rate(): 0.3080502714195546 != %v\n", rate) - } - elapseMinute(a) - if rate := a.Rate(); math.Abs(0.2881831806538789-rate) > epsilon { - t.Errorf("11 minute a.Rate(): 0.2881831806538789 != %v\n", rate) - } - elapseMinute(a) - if rate := a.Rate(); math.Abs(0.26959737847033216-rate) > epsilon { - t.Errorf("12 minute a.Rate(): 0.26959737847033216 != %v\n", rate) - } - elapseMinute(a) - if rate := a.Rate(); math.Abs(0.2522102307052083-rate) > epsilon { - t.Errorf("13 minute a.Rate(): 0.2522102307052083 != %v\n", rate) - } - elapseMinute(a) - if rate := a.Rate(); math.Abs(0.23594443252115815-rate) > epsilon { - t.Errorf("14 minute a.Rate(): 0.23594443252115815 != %v\n", rate) - } - elapseMinute(a) - if rate := a.Rate(); math.Abs(0.2207276647028646247028654470286553-rate) > epsilon { - t.Errorf("15 minute a.Rate(): 0.2207276647028646247028654470286553 != %v\n", rate) + for i, want := range []float64{ + 0.6, 0.5613041910189706, 0.5251039914257684, 0.4912384518467888184678905, + 0.459557003018789, 0.4299187863442732, 0.4021920276213831, + 0.37625345116383313, 0.3519877317060185, 0.3292869816564153165641596, + 0.3080502714195546, 0.2881831806538789, 0.26959737847033216, + 0.2522102307052083, 0.23594443252115815, 0.2207276647028646247028654470286553, + } { + if rate := a.Snapshot().Rate(); math.Abs(want-rate) > epsilon { + t.Errorf("%d minute a.Snapshot().Rate(): %f != %v\n", i, want, rate) + } + elapseMinute(a) } } diff --git a/metrics/librato/librato.go b/metrics/librato/librato.go index cd3e00e28d2e..a86f75863786 100644 --- a/metrics/librato/librato.go +++ b/metrics/librato/librato.go @@ -61,7 +61,7 @@ func (rep *Reporter) Run() { // calculate sum of squares from data provided by metrics.Histogram // see http://en.wikipedia.org/wiki/Standard_deviation#Rapid_calculation_methods -func sumSquares(icount, mean, stDev) float64 { +func sumSquares(icount int64, mean, stDev float64) float64 { count := float64(icount) sumSquared := math.Pow(count*mean, 2) sumSquares := math.Pow(count*stDev, 2) + sumSquared/count @@ -145,7 +145,7 @@ func (rep *Reporter) BuildRequest(now time.Time, r metrics.Registry) (snapshot B for i, p := range rep.Percentiles { gauges[i+1] = Measurement{ Name: fmt.Sprintf("%s.%.2f", measurement[Name], p), - Value: s.Percentile(p), + Value: ms.Percentile(p), Period: measurement[Period], } } From 55c3396aacc83f11419bf6376050a21403f64af7 Mon Sep 17 00:00:00 2001 From: Martin Holst Swende Date: Sun, 3 Sep 2023 20:37:32 +0200 Subject: [PATCH 19/26] metrics: fix flaw re empty timer mean --- metrics/resetting_timer.go | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/metrics/resetting_timer.go b/metrics/resetting_timer.go index ef1162fed82f..6802e3fcea98 100644 --- a/metrics/resetting_timer.go +++ b/metrics/resetting_timer.go @@ -80,11 +80,12 @@ type StandardResettingTimer struct { func (t *StandardResettingTimer) Snapshot() ResettingTimerSnapshot { t.mutex.Lock() defer t.mutex.Unlock() - snapshot := &resettingTimerSnapshot{ - values: t.values, - mean: float64(t.sum) / float64(len(t.values)), + snapshot := &resettingTimerSnapshot{} + if len(t.values) > 0 { + snapshot.mean = float64(t.sum) / float64(len(t.values)) + snapshot.values = t.values + t.values = make([]int64, 0, InitialResettingTimerSliceCap) } - t.values = make([]int64, 0, InitialResettingTimerSliceCap) t.sum = 0 return snapshot } From 0295992985affd46a48134c9fc55b10f4381cb1b Mon Sep 17 00:00:00 2001 From: Martin Holst Swende Date: Mon, 4 Sep 2023 12:33:26 +0200 Subject: [PATCH 20/26] metrics: simplify interfaces, convert tests to table-driven --- metrics/histogram.go | 58 +-------------------- metrics/runtimehistogram.go | 5 ++ metrics/sample.go | 1 - metrics/sample_test.go | 100 ++++++++++++++---------------------- metrics/timer.go | 11 ++-- 5 files changed, 52 insertions(+), 123 deletions(-) diff --git a/metrics/histogram.go b/metrics/histogram.go index ef146d8f4de4..44de588bc1dc 100644 --- a/metrics/histogram.go +++ b/metrics/histogram.go @@ -1,15 +1,7 @@ package metrics type HistogramSnapshot interface { - Count() int64 - Max() int64 - Mean() float64 - Min() int64 - Percentile(float64) float64 - Percentiles([]float64) []float64 - StdDev() float64 - Sum() int64 - Variance() float64 + SampleSnapshot } // Histograms calculate distribution statistics from a series of int64 values. @@ -56,52 +48,6 @@ func NewRegisteredHistogram(name string, r Registry, s Sample) Histogram { return c } -// histogramSnapshot is a read-only copy of another Histogram. -type histogramSnapshot struct { - sample *sampleSnapshot -} - -// Count returns the number of samples recorded at the time the snapshot was -// taken. -func (h *histogramSnapshot) Count() int64 { return h.sample.Count() } - -// Max returns the maximum value in the sample at the time the snapshot was -// taken. -func (h *histogramSnapshot) Max() int64 { return h.sample.Max() } - -// Mean returns the mean of the values in the sample at the time the snapshot -// was taken. -func (h *histogramSnapshot) Mean() float64 { return h.sample.Mean() } - -// Min returns the minimum value in the sample at the time the snapshot was -// taken. -func (h *histogramSnapshot) Min() int64 { return h.sample.Min() } - -// Percentile returns an arbitrary percentile of values in the sample at the -// time the snapshot was taken. -func (h *histogramSnapshot) Percentile(p float64) float64 { - return h.sample.Percentile(p) -} - -// Percentiles returns a slice of arbitrary percentiles of values in the sample -// at the time the snapshot was taken. -func (h *histogramSnapshot) Percentiles(ps []float64) []float64 { - return h.sample.Percentiles(ps) -} - -// Snapshot returns the snapshot. -func (h *histogramSnapshot) Snapshot() HistogramSnapshot { return h } - -// StdDev returns the standard deviation of the values in the sample at the -// time the snapshot was taken. -func (h *histogramSnapshot) StdDev() float64 { return h.sample.StdDev() } - -// Sum returns the sum in the sample at the time the snapshot was taken. -func (h *histogramSnapshot) Sum() int64 { return h.sample.Sum() } - -// Variance returns the variance of inputs at the time the snapshot was taken. -func (h *histogramSnapshot) Variance() float64 { return h.sample.Variance() } - // NilHistogram is a no-op Histogram. type NilHistogram struct{} @@ -120,7 +66,7 @@ func (h *StandardHistogram) Clear() { h.sample.Clear() } // Snapshot returns a read-only copy of the histogram. func (h *StandardHistogram) Snapshot() HistogramSnapshot { - return &histogramSnapshot{sample: h.sample.Snapshot().(*sampleSnapshot)} + return h.sample.Snapshot() } // Update samples a new value. diff --git a/metrics/runtimehistogram.go b/metrics/runtimehistogram.go index a074b88e0d06..0a72f8ad62da 100644 --- a/metrics/runtimehistogram.go +++ b/metrics/runtimehistogram.go @@ -78,6 +78,11 @@ func (h *runtimeHistogramSnapshot) Count() int64 { return count } +// Size returns the size of the sample at the time the snapshot was taken. +func (h *runtimeHistogramSnapshot) Size() int { + return len(h.Counts) +} + // Mean returns an approximation of the mean. func (h *runtimeHistogramSnapshot) Mean() float64 { if len(h.Counts) == 0 { diff --git a/metrics/sample.go b/metrics/sample.go index a5713edacbe0..96598aec710e 100644 --- a/metrics/sample.go +++ b/metrics/sample.go @@ -21,7 +21,6 @@ type SampleSnapshot interface { Size() int StdDev() float64 Sum() int64 - Values() []int64 Variance() float64 } diff --git a/metrics/sample_test.go b/metrics/sample_test.go index f0edfbcae7f4..79673570554c 100644 --- a/metrics/sample_test.go +++ b/metrics/sample_test.go @@ -87,68 +87,42 @@ func BenchmarkUniformSample1028(b *testing.B) { benchmarkSample(b, NewUniformSample(1028)) } -func TestExpDecaySample10(t *testing.T) { - sw := NewExpDecaySample(100, 0.99) - for i := 0; i < 10; i++ { - sw.Update(int64(i)) - } - s := sw.Snapshot() - if size := s.Count(); size != 10 { - t.Errorf("s.Count(): 10 != %v\n", size) - } - if size := s.Size(); size != 10 { - t.Errorf("s.Size(): 10 != %v\n", size) - } - if l := len(s.Values()); l != 10 { - t.Errorf("len(s.Values()): 10 != %v\n", l) - } - for _, v := range s.Values() { - if v > 10 || v < 0 { - t.Errorf("out of range [0, 10): %v\n", v) - } +func min(a, b int) int { + if a < b { + return a } + return b } -func TestExpDecaySample100(t *testing.T) { - sw := NewExpDecaySample(1000, 0.01) - for i := 0; i < 100; i++ { - sw.Update(int64(i)) - } - s := sw.Snapshot() - if size := s.Count(); size != 100 { - t.Errorf("s.Count(): 100 != %v\n", size) - } - if size := s.Size(); size != 100 { - t.Errorf("s.Size(): 100 != %v\n", size) - } - if l := len(s.Values()); l != 100 { - t.Errorf("len(s.Values()): 100 != %v\n", l) - } - for _, v := range s.Values() { - if v > 100 || v < 0 { - t.Errorf("out of range [0, 100): %v\n", v) +func TestExpDecaySample(t *testing.T) { + for _, tc := range []struct { + reservoirSize int + alpha float64 + updates int + }{ + {100, 0.99, 10}, + {1000, 0.01, 100}, + {100, 0.99, 1000}, + } { + sample := NewExpDecaySample(tc.reservoirSize, tc.alpha) + for i := 0; i < tc.updates; i++ { + sample.Update(int64(i)) } - } -} - -func TestExpDecaySample1000(t *testing.T) { - sw := NewExpDecaySample(100, 0.99) - for i := 0; i < 1000; i++ { - sw.Update(int64(i)) - } - s := sw.Snapshot() - if size := s.Count(); size != 1000 { - t.Errorf("s.Count(): 1000 != %v\n", size) - } - if size := s.Size(); size != 100 { - t.Errorf("s.Size(): 100 != %v\n", size) - } - if l := len(s.Values()); l != 100 { - t.Errorf("len(s.Values()): 100 != %v\n", l) - } - for _, v := range s.Values() { - if v > 1000 || v < 0 { - t.Errorf("out of range [0, 1000): %v\n", v) + snap := sample.Snapshot() + if have, want := int(snap.Count()), tc.updates; have != want { + t.Errorf("have %d want %d", have, want) + } + if have, want := snap.Size(), min(tc.updates, tc.reservoirSize); have != want { + t.Errorf("have %d want %d", have, want) + } + values := snap.(*sampleSnapshot).values + if have, want := len(values), min(tc.updates, tc.reservoirSize); have != want { + t.Errorf("have %d want %d", have, want) + } + for _, v := range values { + if v > int64(tc.updates) || v < 0 { + t.Errorf("out of range [0, %d): %v", tc.updates, v) + } } } } @@ -167,7 +141,7 @@ func TestExpDecaySampleNanosecondRegression(t *testing.T) { sw.Update(20) } s := sw.Snapshot() - v := s.Values() + v := s.(*sampleSnapshot).values avg := float64(0) for i := 0; i < len(v); i++ { avg += float64(v[i]) @@ -221,10 +195,12 @@ func TestUniformSample(t *testing.T) { if size := s.Size(); size != 100 { t.Errorf("s.Size(): 100 != %v\n", size) } - if l := len(s.Values()); l != 100 { + values := s.(*sampleSnapshot).values + + if l := len(values); l != 100 { t.Errorf("len(s.Values()): 100 != %v\n", l) } - for _, v := range s.Values() { + for _, v := range values { if v > 1000 || v < 0 { t.Errorf("out of range [0, 100): %v\n", v) } @@ -238,7 +214,7 @@ func TestUniformSampleIncludesTail(t *testing.T) { sw.Update(int64(i)) } s := sw.Snapshot() - v := s.Values() + v := s.(*sampleSnapshot).values sum := 0 exp := (max - 1) * max / 2 for i := 0; i < len(v); i++ { diff --git a/metrics/timer.go b/metrics/timer.go index 26d1bb81d2da..576ad8aa3e63 100644 --- a/metrics/timer.go +++ b/metrics/timer.go @@ -89,8 +89,8 @@ func (t *StandardTimer) Snapshot() TimerSnapshot { t.mutex.Lock() defer t.mutex.Unlock() return &timerSnapshot{ - histogram: t.histogram.Snapshot().(*histogramSnapshot), - meter: t.meter.Snapshot().(*meterSnapshot), + histogram: t.histogram.Snapshot(), + meter: t.meter.Snapshot(), } } @@ -124,8 +124,8 @@ func (t *StandardTimer) UpdateSince(ts time.Time) { // timerSnapshot is a read-only copy of another Timer. type timerSnapshot struct { - histogram *histogramSnapshot - meter *meterSnapshot + histogram HistogramSnapshot + meter MeterSnapshot } // Count returns the number of events recorded at the time the snapshot was @@ -135,6 +135,9 @@ func (t *timerSnapshot) Count() int64 { return t.histogram.Count() } // Max returns the maximum value at the time the snapshot was taken. func (t *timerSnapshot) Max() int64 { return t.histogram.Max() } +// Size returns the size of the sample at the time the snapshot was taken. +func (t *timerSnapshot) Size() int { return t.histogram.Size() } + // Mean returns the mean value at the time the snapshot was taken. func (t *timerSnapshot) Mean() float64 { return t.histogram.Mean() } From 90be12cab2674419b0a3f691fb5f57bdba6180e1 Mon Sep 17 00:00:00 2001 From: Martin Holst Swende Date: Mon, 4 Sep 2023 14:24:15 +0200 Subject: [PATCH 21/26] metrics: sample data for runtime metrics metrics: influx/prometheus testdata update --- metrics/influxdb/testdata/influxdbv1.want | 2 ++ metrics/influxdb/testdata/influxdbv2.want | 2 ++ metrics/internal/sampledata.go | 23 ++++++++++++++++++ metrics/internal/sampledata_test.go | 27 +++++++++++++++++++++ metrics/metrics.go | 6 +++++ metrics/prometheus/testdata/prometheus.want | 22 +++++++++++++++++ metrics/runtimehistogram.go | 6 +++++ 7 files changed, 88 insertions(+) create mode 100644 metrics/internal/sampledata_test.go diff --git a/metrics/influxdb/testdata/influxdbv1.want b/metrics/influxdb/testdata/influxdbv1.want index 83ab88af67b4..9443faedc5a2 100644 --- a/metrics/influxdb/testdata/influxdbv1.want +++ b/metrics/influxdb/testdata/influxdbv1.want @@ -1,3 +1,5 @@ +goth.system/cpu/schedlatency.histogram count=5645i,max=41943040i,mean=1819544.0410983171,min=0i,p25=0,p50=0,p75=7168,p95=16777216,p99=29360128,p999=33554432,p9999=33554432,stddev=6393570.217198883,variance=40877740122252.57 978307200000000000 +goth.system/memory/pauses.histogram count=14i,max=229376i,mean=50066.28571428572,min=5120i,p25=10240,p50=32768,p75=57344,p95=196608,p99=196608,p999=196608,p9999=196608,stddev=54726.062410783874,variance=2994941906.9890113 978307200000000000 goth.test/counter.count value=12345 978307200000000000 goth.test/counter_float64.count value=54321.98 978307200000000000 goth.test/gauge.gauge value=23456i 978307200000000000 diff --git a/metrics/influxdb/testdata/influxdbv2.want b/metrics/influxdb/testdata/influxdbv2.want index 83ab88af67b4..9443faedc5a2 100644 --- a/metrics/influxdb/testdata/influxdbv2.want +++ b/metrics/influxdb/testdata/influxdbv2.want @@ -1,3 +1,5 @@ +goth.system/cpu/schedlatency.histogram count=5645i,max=41943040i,mean=1819544.0410983171,min=0i,p25=0,p50=0,p75=7168,p95=16777216,p99=29360128,p999=33554432,p9999=33554432,stddev=6393570.217198883,variance=40877740122252.57 978307200000000000 +goth.system/memory/pauses.histogram count=14i,max=229376i,mean=50066.28571428572,min=5120i,p25=10240,p50=32768,p75=57344,p95=196608,p99=196608,p999=196608,p9999=196608,stddev=54726.062410783874,variance=2994941906.9890113 978307200000000000 goth.test/counter.count value=12345 978307200000000000 goth.test/counter_float64.count value=54321.98 978307200000000000 goth.test/gauge.gauge value=23456i 978307200000000000 diff --git a/metrics/internal/sampledata.go b/metrics/internal/sampledata.go index fa5314241016..de9b207b6d4a 100644 --- a/metrics/internal/sampledata.go +++ b/metrics/internal/sampledata.go @@ -17,6 +17,9 @@ package internal import ( + "bytes" + "encoding/gob" + metrics2 "runtime/metrics" "time" "github.com/ethereum/go-ethereum/metrics" @@ -68,5 +71,25 @@ func ExampleMetrics() metrics.Registry { timer.Stop() } registry.Register("test/empty_resetting_timer", metrics.NewResettingTimer().Snapshot()) + + { // go runtime metrics + var sLatency = "7\xff\x81\x03\x01\x01\x10Float64Histogram\x01\xff\x82\x00\x01\x02\x01\x06Counts\x01\xff\x84\x00\x01\aBuckets\x01\xff\x86\x00\x00\x00\x16\xff\x83\x02\x01\x01\b[]uint64\x01\xff\x84\x00\x01\x06\x00\x00\x17\xff\x85\x02\x01\x01\t[]float64\x01\xff\x86\x00\x01\b\x00\x00\xfe\x06T\xff\x82\x01\xff\xa2\x00\xfe\r\xef\x00\x01\x02\x02\x04\x05\x04\b\x15\x17 B?6.L;$!2) \x1a? \x190aH7FY6#\x190\x1d\x14\x10\x1b\r\t\x04\x03\x01\x01\x00\x03\x02\x00\x03\x05\x05\x02\x02\x06\x04\v\x06\n\x15\x18\x13'&.\x12=H/L&\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x01\xff\xa3\xfe\xf0\xff\x00\xf8\x95\xd6&\xe8\v.q>\xf8\x95\xd6&\xe8\v.\x81>\xf8\xdfA:\xdc\x11ʼn>\xf8\x95\xd6&\xe8\v.\x91>\xf8:\x8c0\xe2\x8ey\x95>\xf8\xdfA:\xdc\x11ř>\xf8\x84\xf7C֔\x10\x9e>\xf8\x95\xd6&\xe8\v.\xa1>\xf8:\x8c0\xe2\x8ey\xa5>\xf8\xdfA:\xdc\x11ũ>\xf8\x84\xf7C֔\x10\xae>\xf8\x95\xd6&\xe8\v.\xb1>\xf8:\x8c0\xe2\x8ey\xb5>\xf8\xdfA:\xdc\x11Ź>\xf8\x84\xf7C֔\x10\xbe>\xf8\x95\xd6&\xe8\v.\xc1>\xf8:\x8c0\xe2\x8ey\xc5>\xf8\xdfA:\xdc\x11\xc5\xc9>\xf8\x84\xf7C֔\x10\xce>\xf8\x95\xd6&\xe8\v.\xd1>\xf8:\x8c0\xe2\x8ey\xd5>\xf8\xdfA:\xdc\x11\xc5\xd9>\xf8\x84\xf7C֔\x10\xde>\xf8\x95\xd6&\xe8\v.\xe1>\xf8:\x8c0\xe2\x8ey\xe5>\xf8\xdfA:\xdc\x11\xc5\xe9>\xf8\x84\xf7C֔\x10\xee>\xf8\x95\xd6&\xe8\v.\xf1>\xf8:\x8c0\xe2\x8ey\xf5>\xf8\xdfA:\xdc\x11\xc5\xf9>\xf8\x84\xf7C֔\x10\xfe>\xf8\x95\xd6&\xe8\v.\x01?\xf8:\x8c0\xe2\x8ey\x05?\xf8\xdfA:\xdc\x11\xc5\t?\xf8\x84\xf7C֔\x10\x0e?\xf8\x95\xd6&\xe8\v.\x11?\xf8:\x8c0\xe2\x8ey\x15?\xf8\xdfA:\xdc\x11\xc5\x19?\xf8\x84\xf7C֔\x10\x1e?\xf8\x95\xd6&\xe8\v.!?\xf8:\x8c0\xe2\x8ey%?\xf8\xdfA:\xdc\x11\xc5)?\xf8\x84\xf7C֔\x10.?\xf8\x95\xd6&\xe8\v.1?\xf8:\x8c0\xe2\x8ey5?\xf8\xdfA:\xdc\x11\xc59?\xf8\x84\xf7C֔\x10>?\xf8\x95\xd6&\xe8\v.A?\xf8:\x8c0\xe2\x8eyE?\xf8\xdfA:\xdc\x11\xc5I?\xf8\x84\xf7C֔\x10N?\xf8\x95\xd6&\xe8\v.Q?\xf8:\x8c0\xe2\x8eyU?\xf8\xdfA:\xdc\x11\xc5Y?\xf8\x84\xf7C֔\x10^?\xf8\x95\xd6&\xe8\v.a?\xf8:\x8c0\xe2\x8eye?\xf8\xdfA:\xdc\x11\xc5i?\xf8\x84\xf7C֔\x10n?\xf8\x95\xd6&\xe8\v.q?\xf8:\x8c0\xe2\x8eyu?\xf8\xdfA:\xdc\x11\xc5y?\xf8\x84\xf7C֔\x10~?\xf8\x95\xd6&\xe8\v.\x81?\xf8:\x8c0\xe2\x8ey\x85?\xf8\xdfA:\xdc\x11ʼn?\xf8\x84\xf7C֔\x10\x8e?\xf8\x95\xd6&\xe8\v.\x91?\xf8:\x8c0\xe2\x8ey\x95?\xf8\xdfA:\xdc\x11ř?\xf8\x84\xf7C֔\x10\x9e?\xf8\x95\xd6&\xe8\v.\xa1?\xf8:\x8c0\xe2\x8ey\xa5?\xf8\xdfA:\xdc\x11ũ?\xf8\x84\xf7C֔\x10\xae?\xf8\x95\xd6&\xe8\v.\xb1?\xf8:\x8c0\xe2\x8ey\xb5?\xf8\xdfA:\xdc\x11Ź?\xf8\x84\xf7C֔\x10\xbe?\xf8\x95\xd6&\xe8\v.\xc1?\xf8:\x8c0\xe2\x8ey\xc5?\xf8\xdfA:\xdc\x11\xc5\xc9?\xf8\x84\xf7C֔\x10\xce?\xf8\x95\xd6&\xe8\v.\xd1?\xf8:\x8c0\xe2\x8ey\xd5?\xf8\xdfA:\xdc\x11\xc5\xd9?\xf8\x84\xf7C֔\x10\xde?\xf8\x95\xd6&\xe8\v.\xe1?\xf8:\x8c0\xe2\x8ey\xe5?\xf8\xdfA:\xdc\x11\xc5\xe9?\xf8\x84\xf7C֔\x10\xee?\xf8\x95\xd6&\xe8\v.\xf1?\xf8:\x8c0\xe2\x8ey\xf5?\xf8\xdfA:\xdc\x11\xc5\xf9?\xf8\x84\xf7C֔\x10\xfe?\xf8\x95\xd6&\xe8\v.\x01@\xf8:\x8c0\xe2\x8ey\x05@\xf8\xdfA:\xdc\x11\xc5\t@\xf8\x84\xf7C֔\x10\x0e@\xf8\x95\xd6&\xe8\v.\x11@\xf8:\x8c0\xe2\x8ey\x15@\xf8\xdfA:\xdc\x11\xc5\x19@\xf8\x84\xf7C֔\x10\x1e@\xf8\x95\xd6&\xe8\v.!@\xf8:\x8c0\xe2\x8ey%@\xf8\xdfA:\xdc\x11\xc5)@\xf8\x84\xf7C֔\x10.@\xf8\x95\xd6&\xe8\v.1@\xf8:\x8c0\xe2\x8ey5@\xf8\xdfA:\xdc\x11\xc59@\xf8\x84\xf7C֔\x10>@\xf8\x95\xd6&\xe8\v.A@\xf8:\x8c0\xe2\x8eyE@\xf8\xdfA:\xdc\x11\xc5I@\xf8\x84\xf7C֔\x10N@\xf8\x95\xd6&\xe8\v.Q@\xf8:\x8c0\xe2\x8eyU@\xf8\xdfA:\xdc\x11\xc5Y@\xf8\x84\xf7C֔\x10^@\xf8\x95\xd6&\xe8\v.a@\xf8:\x8c0\xe2\x8eye@\xf8\xdfA:\xdc\x11\xc5i@\xf8\x84\xf7C֔\x10n@\xf8\x95\xd6&\xe8\v.q@\xf8:\x8c0\xe2\x8eyu@\xf8\xdfA:\xdc\x11\xc5y@\xf8\x84\xf7C֔\x10~@\xf8\x95\xd6&\xe8\v.\x81@\xf8:\x8c0\xe2\x8ey\x85@\xf8\xdfA:\xdc\x11ʼn@\xf8\x84\xf7C֔\x10\x8e@\xf8\x95\xd6&\xe8\v.\x91@\xf8:\x8c0\xe2\x8ey\x95@\xf8\xdfA:\xdc\x11ř@\xf8\x84\xf7C֔\x10\x9e@\xf8\x95\xd6&\xe8\v.\xa1@\xf8:\x8c0\xe2\x8ey\xa5@\xf8\xdfA:\xdc\x11ũ@\xf8\x84\xf7C֔\x10\xae@\xf8\x95\xd6&\xe8\v.\xb1@\xf8:\x8c0\xe2\x8ey\xb5@\xf8\xdfA:\xdc\x11Ź@\xf8\x84\xf7C֔\x10\xbe@\xf8\x95\xd6&\xe8\v.\xc1@\xf8:\x8c0\xe2\x8ey\xc5@\xf8\xdfA:\xdc\x11\xc5\xc9@\xf8\x84\xf7C֔\x10\xce@\xf8\x95\xd6&\xe8\v.\xd1@\xf8:\x8c0\xe2\x8ey\xd5@\xf8\xdfA:\xdc\x11\xc5\xd9@\xf8\x84\xf7C֔\x10\xde@\xf8\x95\xd6&\xe8\v.\xe1@\xf8:\x8c0\xe2\x8ey\xe5@\xf8\xdfA:\xdc\x11\xc5\xe9@\xf8\x84\xf7C֔\x10\xee@\xf8\x95\xd6&\xe8\v.\xf1@\xf8:\x8c0\xe2\x8ey\xf5@\xf8\xdfA:\xdc\x11\xc5\xf9@\xf8\x84\xf7C֔\x10\xfe@\xf8\x95\xd6&\xe8\v.\x01A\xfe\xf0\x7f\x00" + var gcPauses = "7\xff\x81\x03\x01\x01\x10Float64Histogram\x01\xff\x82\x00\x01\x02\x01\x06Counts\x01\xff\x84\x00\x01\aBuckets\x01\xff\x86\x00\x00\x00\x16\xff\x83\x02\x01\x01\b[]uint64\x01\xff\x84\x00\x01\x06\x00\x00\x17\xff\x85\x02\x01\x01\t[]float64\x01\xff\x86\x00\x01\b\x00\x00\xfe\x06R\xff\x82\x01\xff\xa2\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x01\x01\x01\x00\x01\x00\x00\x00\x01\x01\x01\x01\x01\x01\x01\x00\x02\x00\x00\x00\x00\x01\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x01\xff\xa3\xfe\xf0\xff\x00\xf8\x95\xd6&\xe8\v.q>\xf8\x95\xd6&\xe8\v.\x81>\xf8\xdfA:\xdc\x11ʼn>\xf8\x95\xd6&\xe8\v.\x91>\xf8:\x8c0\xe2\x8ey\x95>\xf8\xdfA:\xdc\x11ř>\xf8\x84\xf7C֔\x10\x9e>\xf8\x95\xd6&\xe8\v.\xa1>\xf8:\x8c0\xe2\x8ey\xa5>\xf8\xdfA:\xdc\x11ũ>\xf8\x84\xf7C֔\x10\xae>\xf8\x95\xd6&\xe8\v.\xb1>\xf8:\x8c0\xe2\x8ey\xb5>\xf8\xdfA:\xdc\x11Ź>\xf8\x84\xf7C֔\x10\xbe>\xf8\x95\xd6&\xe8\v.\xc1>\xf8:\x8c0\xe2\x8ey\xc5>\xf8\xdfA:\xdc\x11\xc5\xc9>\xf8\x84\xf7C֔\x10\xce>\xf8\x95\xd6&\xe8\v.\xd1>\xf8:\x8c0\xe2\x8ey\xd5>\xf8\xdfA:\xdc\x11\xc5\xd9>\xf8\x84\xf7C֔\x10\xde>\xf8\x95\xd6&\xe8\v.\xe1>\xf8:\x8c0\xe2\x8ey\xe5>\xf8\xdfA:\xdc\x11\xc5\xe9>\xf8\x84\xf7C֔\x10\xee>\xf8\x95\xd6&\xe8\v.\xf1>\xf8:\x8c0\xe2\x8ey\xf5>\xf8\xdfA:\xdc\x11\xc5\xf9>\xf8\x84\xf7C֔\x10\xfe>\xf8\x95\xd6&\xe8\v.\x01?\xf8:\x8c0\xe2\x8ey\x05?\xf8\xdfA:\xdc\x11\xc5\t?\xf8\x84\xf7C֔\x10\x0e?\xf8\x95\xd6&\xe8\v.\x11?\xf8:\x8c0\xe2\x8ey\x15?\xf8\xdfA:\xdc\x11\xc5\x19?\xf8\x84\xf7C֔\x10\x1e?\xf8\x95\xd6&\xe8\v.!?\xf8:\x8c0\xe2\x8ey%?\xf8\xdfA:\xdc\x11\xc5)?\xf8\x84\xf7C֔\x10.?\xf8\x95\xd6&\xe8\v.1?\xf8:\x8c0\xe2\x8ey5?\xf8\xdfA:\xdc\x11\xc59?\xf8\x84\xf7C֔\x10>?\xf8\x95\xd6&\xe8\v.A?\xf8:\x8c0\xe2\x8eyE?\xf8\xdfA:\xdc\x11\xc5I?\xf8\x84\xf7C֔\x10N?\xf8\x95\xd6&\xe8\v.Q?\xf8:\x8c0\xe2\x8eyU?\xf8\xdfA:\xdc\x11\xc5Y?\xf8\x84\xf7C֔\x10^?\xf8\x95\xd6&\xe8\v.a?\xf8:\x8c0\xe2\x8eye?\xf8\xdfA:\xdc\x11\xc5i?\xf8\x84\xf7C֔\x10n?\xf8\x95\xd6&\xe8\v.q?\xf8:\x8c0\xe2\x8eyu?\xf8\xdfA:\xdc\x11\xc5y?\xf8\x84\xf7C֔\x10~?\xf8\x95\xd6&\xe8\v.\x81?\xf8:\x8c0\xe2\x8ey\x85?\xf8\xdfA:\xdc\x11ʼn?\xf8\x84\xf7C֔\x10\x8e?\xf8\x95\xd6&\xe8\v.\x91?\xf8:\x8c0\xe2\x8ey\x95?\xf8\xdfA:\xdc\x11ř?\xf8\x84\xf7C֔\x10\x9e?\xf8\x95\xd6&\xe8\v.\xa1?\xf8:\x8c0\xe2\x8ey\xa5?\xf8\xdfA:\xdc\x11ũ?\xf8\x84\xf7C֔\x10\xae?\xf8\x95\xd6&\xe8\v.\xb1?\xf8:\x8c0\xe2\x8ey\xb5?\xf8\xdfA:\xdc\x11Ź?\xf8\x84\xf7C֔\x10\xbe?\xf8\x95\xd6&\xe8\v.\xc1?\xf8:\x8c0\xe2\x8ey\xc5?\xf8\xdfA:\xdc\x11\xc5\xc9?\xf8\x84\xf7C֔\x10\xce?\xf8\x95\xd6&\xe8\v.\xd1?\xf8:\x8c0\xe2\x8ey\xd5?\xf8\xdfA:\xdc\x11\xc5\xd9?\xf8\x84\xf7C֔\x10\xde?\xf8\x95\xd6&\xe8\v.\xe1?\xf8:\x8c0\xe2\x8ey\xe5?\xf8\xdfA:\xdc\x11\xc5\xe9?\xf8\x84\xf7C֔\x10\xee?\xf8\x95\xd6&\xe8\v.\xf1?\xf8:\x8c0\xe2\x8ey\xf5?\xf8\xdfA:\xdc\x11\xc5\xf9?\xf8\x84\xf7C֔\x10\xfe?\xf8\x95\xd6&\xe8\v.\x01@\xf8:\x8c0\xe2\x8ey\x05@\xf8\xdfA:\xdc\x11\xc5\t@\xf8\x84\xf7C֔\x10\x0e@\xf8\x95\xd6&\xe8\v.\x11@\xf8:\x8c0\xe2\x8ey\x15@\xf8\xdfA:\xdc\x11\xc5\x19@\xf8\x84\xf7C֔\x10\x1e@\xf8\x95\xd6&\xe8\v.!@\xf8:\x8c0\xe2\x8ey%@\xf8\xdfA:\xdc\x11\xc5)@\xf8\x84\xf7C֔\x10.@\xf8\x95\xd6&\xe8\v.1@\xf8:\x8c0\xe2\x8ey5@\xf8\xdfA:\xdc\x11\xc59@\xf8\x84\xf7C֔\x10>@\xf8\x95\xd6&\xe8\v.A@\xf8:\x8c0\xe2\x8eyE@\xf8\xdfA:\xdc\x11\xc5I@\xf8\x84\xf7C֔\x10N@\xf8\x95\xd6&\xe8\v.Q@\xf8:\x8c0\xe2\x8eyU@\xf8\xdfA:\xdc\x11\xc5Y@\xf8\x84\xf7C֔\x10^@\xf8\x95\xd6&\xe8\v.a@\xf8:\x8c0\xe2\x8eye@\xf8\xdfA:\xdc\x11\xc5i@\xf8\x84\xf7C֔\x10n@\xf8\x95\xd6&\xe8\v.q@\xf8:\x8c0\xe2\x8eyu@\xf8\xdfA:\xdc\x11\xc5y@\xf8\x84\xf7C֔\x10~@\xf8\x95\xd6&\xe8\v.\x81@\xf8:\x8c0\xe2\x8ey\x85@\xf8\xdfA:\xdc\x11ʼn@\xf8\x84\xf7C֔\x10\x8e@\xf8\x95\xd6&\xe8\v.\x91@\xf8:\x8c0\xe2\x8ey\x95@\xf8\xdfA:\xdc\x11ř@\xf8\x84\xf7C֔\x10\x9e@\xf8\x95\xd6&\xe8\v.\xa1@\xf8:\x8c0\xe2\x8ey\xa5@\xf8\xdfA:\xdc\x11ũ@\xf8\x84\xf7C֔\x10\xae@\xf8\x95\xd6&\xe8\v.\xb1@\xf8:\x8c0\xe2\x8ey\xb5@\xf8\xdfA:\xdc\x11Ź@\xf8\x84\xf7C֔\x10\xbe@\xf8\x95\xd6&\xe8\v.\xc1@\xf8:\x8c0\xe2\x8ey\xc5@\xf8\xdfA:\xdc\x11\xc5\xc9@\xf8\x84\xf7C֔\x10\xce@\xf8\x95\xd6&\xe8\v.\xd1@\xf8:\x8c0\xe2\x8ey\xd5@\xf8\xdfA:\xdc\x11\xc5\xd9@\xf8\x84\xf7C֔\x10\xde@\xf8\x95\xd6&\xe8\v.\xe1@\xf8:\x8c0\xe2\x8ey\xe5@\xf8\xdfA:\xdc\x11\xc5\xe9@\xf8\x84\xf7C֔\x10\xee@\xf8\x95\xd6&\xe8\v.\xf1@\xf8:\x8c0\xe2\x8ey\xf5@\xf8\xdfA:\xdc\x11\xc5\xf9@\xf8\x84\xf7C֔\x10\xfe@\xf8\x95\xd6&\xe8\v.\x01A\xfe\xf0\x7f\x00" + + var secondsToNs = float64(time.Second) + + dserialize := func(data string) *metrics2.Float64Histogram { + var res metrics2.Float64Histogram + if err := gob.NewDecoder(bytes.NewReader([]byte(data))).Decode(&res); err != nil { + panic(err) + } + return &res + } + cpuSchedLatency := metrics.RuntimeHistogramFromData(secondsToNs, dserialize(sLatency)) + registry.Register("system/cpu/schedlatency", cpuSchedLatency) + + memPauses := metrics.RuntimeHistogramFromData(secondsToNs, dserialize(gcPauses)) + registry.Register("system/memory/pauses", memPauses) + } return registry } diff --git a/metrics/internal/sampledata_test.go b/metrics/internal/sampledata_test.go new file mode 100644 index 000000000000..00132994064e --- /dev/null +++ b/metrics/internal/sampledata_test.go @@ -0,0 +1,27 @@ +package internal + +import ( + "bytes" + "encoding/gob" + "fmt" + metrics2 "runtime/metrics" + "testing" + "time" + + "github.com/ethereum/go-ethereum/metrics" +) + +func TestCollectRuntimeMetrics(t *testing.T) { + t.Skip("Only used for generating testdata") + serialize := func(path string, histogram *metrics2.Float64Histogram) { + var f = new(bytes.Buffer) + if err := gob.NewEncoder(f).Encode(histogram); err != nil { + panic(err) + } + fmt.Printf("var %v = %q\n", path, f.Bytes()) + } + time.Sleep(2 * time.Second) + stats := metrics.ReadRuntimeStats() + serialize("schedlatency", stats.SchedLatency) + serialize("gcpauses", stats.GCPauses) +} diff --git a/metrics/metrics.go b/metrics/metrics.go index c206f1692407..97f03fa31db2 100644 --- a/metrics/metrics.go +++ b/metrics/metrics.go @@ -85,6 +85,12 @@ var runtimeSamples = []metrics.Sample{ {Name: "/sched/latencies:seconds"}, // histogram } +func ReadRuntimeStats() *runtimeStats { + r := new(runtimeStats) + readRuntimeStats(r) + return r +} + func readRuntimeStats(v *runtimeStats) { metrics.Read(runtimeSamples) for _, s := range runtimeSamples { diff --git a/metrics/prometheus/testdata/prometheus.want b/metrics/prometheus/testdata/prometheus.want index fb03f862905b..861c5f5cf087 100644 --- a/metrics/prometheus/testdata/prometheus.want +++ b/metrics/prometheus/testdata/prometheus.want @@ -1,3 +1,25 @@ +# TYPE system_cpu_schedlatency_count counter +system_cpu_schedlatency_count 5645 + +# TYPE system_cpu_schedlatency summary +system_cpu_schedlatency {quantile="0.5"} 0 +system_cpu_schedlatency {quantile="0.75"} 7168 +system_cpu_schedlatency {quantile="0.95"} 1.6777216e+07 +system_cpu_schedlatency {quantile="0.99"} 2.9360128e+07 +system_cpu_schedlatency {quantile="0.999"} 3.3554432e+07 +system_cpu_schedlatency {quantile="0.9999"} 3.3554432e+07 + +# TYPE system_memory_pauses_count counter +system_memory_pauses_count 14 + +# TYPE system_memory_pauses summary +system_memory_pauses {quantile="0.5"} 32768 +system_memory_pauses {quantile="0.75"} 57344 +system_memory_pauses {quantile="0.95"} 196608 +system_memory_pauses {quantile="0.99"} 196608 +system_memory_pauses {quantile="0.999"} 196608 +system_memory_pauses {quantile="0.9999"} 196608 + # TYPE test_counter gauge test_counter 12345 diff --git a/metrics/runtimehistogram.go b/metrics/runtimehistogram.go index 0a72f8ad62da..698cf97c1818 100644 --- a/metrics/runtimehistogram.go +++ b/metrics/runtimehistogram.go @@ -27,6 +27,12 @@ func newRuntimeHistogram(scale float64) *runtimeHistogram { return h } +func RuntimeHistogramFromData(scale float64, hist *metrics.Float64Histogram) *runtimeHistogram { + h := &runtimeHistogram{scaleFactor: scale} + h.update(hist) + return h +} + func (h *runtimeHistogram) update(mh *metrics.Float64Histogram) { if mh == nil { // The update value can be nil if the current Go version doesn't support a From c8e1767e335aea21c51d0d8cc8edd352cc597cc2 Mon Sep 17 00:00:00 2001 From: Martin Holst Swende Date: Mon, 4 Sep 2023 15:12:06 +0200 Subject: [PATCH 22/26] metrics: split read/write + improve runtime histogram speed --- metrics/runtimehistogram.go | 160 +++++++++++++++++++------------ metrics/runtimehistogram_test.go | 37 ++++++- 2 files changed, 130 insertions(+), 67 deletions(-) diff --git a/metrics/runtimehistogram.go b/metrics/runtimehistogram.go index 698cf97c1818..6b0c6a4c4095 100644 --- a/metrics/runtimehistogram.go +++ b/metrics/runtimehistogram.go @@ -17,13 +17,13 @@ func getOrRegisterRuntimeHistogram(name string, scale float64, r Registry) *runt // runtimeHistogram wraps a runtime/metrics histogram. type runtimeHistogram struct { - v atomic.Value + v atomic.Value // v is a pointer to a metrics.Float64Histogram scaleFactor float64 } func newRuntimeHistogram(scale float64) *runtimeHistogram { h := &runtimeHistogram{scaleFactor: scale} - h.update(&metrics.Float64Histogram{}) + h.update(new(metrics.Float64Histogram)) return h } @@ -41,22 +41,17 @@ func (h *runtimeHistogram) update(mh *metrics.Float64Histogram) { return } - s := runtimeHistogramSnapshot{ + s := metrics.Float64Histogram{ Counts: make([]uint64, len(mh.Counts)), Buckets: make([]float64, len(mh.Buckets)), } copy(s.Counts, mh.Counts) - copy(s.Buckets, mh.Buckets) - for i, b := range s.Buckets { + for i, b := range mh.Buckets { s.Buckets[i] = b * h.scaleFactor } h.v.Store(&s) } -func (h *runtimeHistogram) load() *runtimeHistogramSnapshot { - return h.v.Load().(*runtimeHistogramSnapshot) -} - func (h *runtimeHistogram) Clear() { panic("runtimeHistogram does not support Clear") } @@ -64,54 +59,94 @@ func (h *runtimeHistogram) Update(int64) { panic("runtimeHistogram does not support Update") } -// Snapshot returns a non-changing cop of the histogram. +// Snapshot returns a non-changing copy of the histogram. func (h *runtimeHistogram) Snapshot() HistogramSnapshot { - return h.load() + hist := h.v.Load().(*metrics.Float64Histogram) + return newRuntimeHistogramSnapshot(hist) } -type runtimeHistogramSnapshot metrics.Float64Histogram +type runtimeHistogramSnapshot struct { + internal *metrics.Float64Histogram + calculated bool + // The following fields are (lazily) calculated based on 'internal' + mean float64 + count int64 + min int64 // min is the lowest sample value. + max int64 // max is the highest sample value. + variance float64 +} -func (h *runtimeHistogramSnapshot) Snapshot() HistogramSnapshot { - return h +func newRuntimeHistogramSnapshot(h *metrics.Float64Histogram) *runtimeHistogramSnapshot { + return &runtimeHistogramSnapshot{ + internal: h, + } +} + +// calc calculates the values for the snapshot. This method is not threadsafe. +func (h *runtimeHistogramSnapshot) calc() { + h.calculated = true + var ( + count int64 // number of samples + sum float64 // approx sum of all sample values + minSet = false + min int64 + max float64 + ) + if len(h.internal.Counts) == 0 { + return + } + for i, c := range h.internal.Counts { + count += int64(c) + midpoint := h.midpoint(i) + sum += midpoint * float64(c) + if c == 0 { + continue + } + + if c > 0 && !minSet { + minSet = true + min = int64(math.Floor(h.internal.Buckets[i])) + } + if count > 0 { + edge := h.internal.Buckets[i+1] + if math.IsInf(edge, 1) { + edge = h.internal.Buckets[i] + } + if edge > max { + max = edge + } + } + } + h.min = min + h.max = int64(max) + h.mean = sum / float64(count) + h.count = count } // Count returns the sample count. func (h *runtimeHistogramSnapshot) Count() int64 { - var count int64 - for _, c := range h.Counts { - count += int64(c) + if !h.calculated { + h.calc() } - return count + return h.count } // Size returns the size of the sample at the time the snapshot was taken. func (h *runtimeHistogramSnapshot) Size() int { - return len(h.Counts) + return len(h.internal.Counts) } // Mean returns an approximation of the mean. func (h *runtimeHistogramSnapshot) Mean() float64 { - if len(h.Counts) == 0 { - return 0 - } - mean, _ := h.mean() - return mean -} - -// mean computes the mean and also the total sample count. -func (h *runtimeHistogramSnapshot) mean() (mean, totalCount float64) { - var sum float64 - for i, c := range h.Counts { - midpoint := h.midpoint(i) - sum += midpoint * float64(c) - totalCount += float64(c) + if !h.calculated { + h.calc() } - return sum / totalCount, totalCount + return h.mean } func (h *runtimeHistogramSnapshot) midpoint(bucket int) float64 { - high := h.Buckets[bucket+1] - low := h.Buckets[bucket] + high := h.internal.Buckets[bucket+1] + low := h.internal.Buckets[bucket] if math.IsInf(high, 1) { // The edge of the highest bucket can be +Inf, and it's supposed to mean that this // bucket contains all remaining samples > low. We can't get the middle of an @@ -133,23 +168,31 @@ func (h *runtimeHistogramSnapshot) StdDev() float64 { // Variance approximates the variance of the histogram. func (h *runtimeHistogramSnapshot) Variance() float64 { - if len(h.Counts) == 0 { + if len(h.internal.Counts) == 0 { return 0 } - - mean, totalCount := h.mean() - if totalCount <= 1 { + if !h.calculated { + h.calc() + } + if h.count <= 1 { // There is no variance when there are zero or one items. return 0 } - + // Variance is not calculated in 'calc', because it requires a second iteration. + // Therefore we calculate it lazily in this method, triggered either by + // a direct call to Variance or via StdDev. + if h.variance != 0.0 { + return h.variance + } var sum float64 - for i, c := range h.Counts { + + for i, c := range h.internal.Counts { midpoint := h.midpoint(i) - d := midpoint - mean + d := midpoint - h.mean sum += float64(c) * (d * d) } - return sum / (totalCount - 1) + h.variance = sum / float64(h.count-1) + return h.variance } // Percentile computes the p'th percentile value. @@ -184,11 +227,11 @@ func (h *runtimeHistogramSnapshot) Percentiles(ps []float64) []float64 { func (h *runtimeHistogramSnapshot) computePercentiles(thresh []float64) { var totalCount float64 - for i, count := range h.Counts { + for i, count := range h.internal.Counts { totalCount += float64(count) for len(thresh) > 0 && thresh[0] < totalCount { - thresh[0] = h.Buckets[i] + thresh[0] = h.internal.Buckets[i] thresh = thresh[1:] } if len(thresh) == 0 { @@ -203,34 +246,25 @@ func (h *runtimeHistogramSnapshot) computePercentiles(thresh []float64) { // Max returns the highest sample value. func (h *runtimeHistogramSnapshot) Max() int64 { - for i := len(h.Counts) - 1; i >= 0; i-- { - count := h.Counts[i] - if count > 0 { - edge := h.Buckets[i+1] - if math.IsInf(edge, 1) { - edge = h.Buckets[i] - } - return int64(math.Ceil(edge)) - } + if !h.calculated { + h.calc() } - return 0 + return h.max } // Min returns the lowest sample value. func (h *runtimeHistogramSnapshot) Min() int64 { - for i, count := range h.Counts { - if count > 0 { - return int64(math.Floor(h.Buckets[i])) - } + if !h.calculated { + h.calc() } - return 0 + return h.min } // Sum returns the sum of all sample values. func (h *runtimeHistogramSnapshot) Sum() int64 { var sum float64 - for i := range h.Counts { - sum += h.Buckets[i] * float64(h.Counts[i]) + for i := range h.internal.Counts { + sum += h.internal.Buckets[i] * float64(h.internal.Counts[i]) } return int64(math.Ceil(sum)) } diff --git a/metrics/runtimehistogram_test.go b/metrics/runtimehistogram_test.go index d53a01438311..cf7e36420ae9 100644 --- a/metrics/runtimehistogram_test.go +++ b/metrics/runtimehistogram_test.go @@ -1,11 +1,14 @@ package metrics import ( + "bytes" + "encoding/gob" "fmt" "math" "reflect" "runtime/metrics" "testing" + "time" ) var _ Histogram = (*runtimeHistogram)(nil) @@ -74,7 +77,7 @@ func TestRuntimeHistogramStats(t *testing.T) { for i, test := range tests { t.Run(fmt.Sprint(i), func(t *testing.T) { - s := runtimeHistogramSnapshot(test.h) + s := RuntimeHistogramFromData(1.0, &test.h).Snapshot() if v := s.Count(); v != test.Count { t.Errorf("Count() = %v, want %v", v, test.Count) @@ -121,13 +124,39 @@ func approxEqual(x, y, ε float64) bool { // This test verifies that requesting Percentiles in unsorted order // returns them in the requested order. func TestRuntimeHistogramStatsPercentileOrder(t *testing.T) { - p := runtimeHistogramSnapshot{ + s := RuntimeHistogramFromData(1.0, &metrics.Float64Histogram{ Counts: []uint64{1, 1, 1, 1, 1, 1, 1, 1, 1, 1}, Buckets: []float64{0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10}, - } - result := p.Percentiles([]float64{1, 0.2, 0.5, 0.1, 0.2}) + }).Snapshot() + result := s.Percentiles([]float64{1, 0.2, 0.5, 0.1, 0.2}) expected := []float64{10, 2, 5, 1, 2} if !reflect.DeepEqual(result, expected) { t.Fatal("wrong result:", result) } } + +func BenchmarkRuntimeHistogramSnapshotRead(b *testing.B) { + var sLatency = "7\xff\x81\x03\x01\x01\x10Float64Histogram\x01\xff\x82\x00\x01\x02\x01\x06Counts\x01\xff\x84\x00\x01\aBuckets\x01\xff\x86\x00\x00\x00\x16\xff\x83\x02\x01\x01\b[]uint64\x01\xff\x84\x00\x01\x06\x00\x00\x17\xff\x85\x02\x01\x01\t[]float64\x01\xff\x86\x00\x01\b\x00\x00\xfe\x06T\xff\x82\x01\xff\xa2\x00\xfe\r\xef\x00\x01\x02\x02\x04\x05\x04\b\x15\x17 B?6.L;$!2) \x1a? \x190aH7FY6#\x190\x1d\x14\x10\x1b\r\t\x04\x03\x01\x01\x00\x03\x02\x00\x03\x05\x05\x02\x02\x06\x04\v\x06\n\x15\x18\x13'&.\x12=H/L&\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x01\xff\xa3\xfe\xf0\xff\x00\xf8\x95\xd6&\xe8\v.q>\xf8\x95\xd6&\xe8\v.\x81>\xf8\xdfA:\xdc\x11ʼn>\xf8\x95\xd6&\xe8\v.\x91>\xf8:\x8c0\xe2\x8ey\x95>\xf8\xdfA:\xdc\x11ř>\xf8\x84\xf7C֔\x10\x9e>\xf8\x95\xd6&\xe8\v.\xa1>\xf8:\x8c0\xe2\x8ey\xa5>\xf8\xdfA:\xdc\x11ũ>\xf8\x84\xf7C֔\x10\xae>\xf8\x95\xd6&\xe8\v.\xb1>\xf8:\x8c0\xe2\x8ey\xb5>\xf8\xdfA:\xdc\x11Ź>\xf8\x84\xf7C֔\x10\xbe>\xf8\x95\xd6&\xe8\v.\xc1>\xf8:\x8c0\xe2\x8ey\xc5>\xf8\xdfA:\xdc\x11\xc5\xc9>\xf8\x84\xf7C֔\x10\xce>\xf8\x95\xd6&\xe8\v.\xd1>\xf8:\x8c0\xe2\x8ey\xd5>\xf8\xdfA:\xdc\x11\xc5\xd9>\xf8\x84\xf7C֔\x10\xde>\xf8\x95\xd6&\xe8\v.\xe1>\xf8:\x8c0\xe2\x8ey\xe5>\xf8\xdfA:\xdc\x11\xc5\xe9>\xf8\x84\xf7C֔\x10\xee>\xf8\x95\xd6&\xe8\v.\xf1>\xf8:\x8c0\xe2\x8ey\xf5>\xf8\xdfA:\xdc\x11\xc5\xf9>\xf8\x84\xf7C֔\x10\xfe>\xf8\x95\xd6&\xe8\v.\x01?\xf8:\x8c0\xe2\x8ey\x05?\xf8\xdfA:\xdc\x11\xc5\t?\xf8\x84\xf7C֔\x10\x0e?\xf8\x95\xd6&\xe8\v.\x11?\xf8:\x8c0\xe2\x8ey\x15?\xf8\xdfA:\xdc\x11\xc5\x19?\xf8\x84\xf7C֔\x10\x1e?\xf8\x95\xd6&\xe8\v.!?\xf8:\x8c0\xe2\x8ey%?\xf8\xdfA:\xdc\x11\xc5)?\xf8\x84\xf7C֔\x10.?\xf8\x95\xd6&\xe8\v.1?\xf8:\x8c0\xe2\x8ey5?\xf8\xdfA:\xdc\x11\xc59?\xf8\x84\xf7C֔\x10>?\xf8\x95\xd6&\xe8\v.A?\xf8:\x8c0\xe2\x8eyE?\xf8\xdfA:\xdc\x11\xc5I?\xf8\x84\xf7C֔\x10N?\xf8\x95\xd6&\xe8\v.Q?\xf8:\x8c0\xe2\x8eyU?\xf8\xdfA:\xdc\x11\xc5Y?\xf8\x84\xf7C֔\x10^?\xf8\x95\xd6&\xe8\v.a?\xf8:\x8c0\xe2\x8eye?\xf8\xdfA:\xdc\x11\xc5i?\xf8\x84\xf7C֔\x10n?\xf8\x95\xd6&\xe8\v.q?\xf8:\x8c0\xe2\x8eyu?\xf8\xdfA:\xdc\x11\xc5y?\xf8\x84\xf7C֔\x10~?\xf8\x95\xd6&\xe8\v.\x81?\xf8:\x8c0\xe2\x8ey\x85?\xf8\xdfA:\xdc\x11ʼn?\xf8\x84\xf7C֔\x10\x8e?\xf8\x95\xd6&\xe8\v.\x91?\xf8:\x8c0\xe2\x8ey\x95?\xf8\xdfA:\xdc\x11ř?\xf8\x84\xf7C֔\x10\x9e?\xf8\x95\xd6&\xe8\v.\xa1?\xf8:\x8c0\xe2\x8ey\xa5?\xf8\xdfA:\xdc\x11ũ?\xf8\x84\xf7C֔\x10\xae?\xf8\x95\xd6&\xe8\v.\xb1?\xf8:\x8c0\xe2\x8ey\xb5?\xf8\xdfA:\xdc\x11Ź?\xf8\x84\xf7C֔\x10\xbe?\xf8\x95\xd6&\xe8\v.\xc1?\xf8:\x8c0\xe2\x8ey\xc5?\xf8\xdfA:\xdc\x11\xc5\xc9?\xf8\x84\xf7C֔\x10\xce?\xf8\x95\xd6&\xe8\v.\xd1?\xf8:\x8c0\xe2\x8ey\xd5?\xf8\xdfA:\xdc\x11\xc5\xd9?\xf8\x84\xf7C֔\x10\xde?\xf8\x95\xd6&\xe8\v.\xe1?\xf8:\x8c0\xe2\x8ey\xe5?\xf8\xdfA:\xdc\x11\xc5\xe9?\xf8\x84\xf7C֔\x10\xee?\xf8\x95\xd6&\xe8\v.\xf1?\xf8:\x8c0\xe2\x8ey\xf5?\xf8\xdfA:\xdc\x11\xc5\xf9?\xf8\x84\xf7C֔\x10\xfe?\xf8\x95\xd6&\xe8\v.\x01@\xf8:\x8c0\xe2\x8ey\x05@\xf8\xdfA:\xdc\x11\xc5\t@\xf8\x84\xf7C֔\x10\x0e@\xf8\x95\xd6&\xe8\v.\x11@\xf8:\x8c0\xe2\x8ey\x15@\xf8\xdfA:\xdc\x11\xc5\x19@\xf8\x84\xf7C֔\x10\x1e@\xf8\x95\xd6&\xe8\v.!@\xf8:\x8c0\xe2\x8ey%@\xf8\xdfA:\xdc\x11\xc5)@\xf8\x84\xf7C֔\x10.@\xf8\x95\xd6&\xe8\v.1@\xf8:\x8c0\xe2\x8ey5@\xf8\xdfA:\xdc\x11\xc59@\xf8\x84\xf7C֔\x10>@\xf8\x95\xd6&\xe8\v.A@\xf8:\x8c0\xe2\x8eyE@\xf8\xdfA:\xdc\x11\xc5I@\xf8\x84\xf7C֔\x10N@\xf8\x95\xd6&\xe8\v.Q@\xf8:\x8c0\xe2\x8eyU@\xf8\xdfA:\xdc\x11\xc5Y@\xf8\x84\xf7C֔\x10^@\xf8\x95\xd6&\xe8\v.a@\xf8:\x8c0\xe2\x8eye@\xf8\xdfA:\xdc\x11\xc5i@\xf8\x84\xf7C֔\x10n@\xf8\x95\xd6&\xe8\v.q@\xf8:\x8c0\xe2\x8eyu@\xf8\xdfA:\xdc\x11\xc5y@\xf8\x84\xf7C֔\x10~@\xf8\x95\xd6&\xe8\v.\x81@\xf8:\x8c0\xe2\x8ey\x85@\xf8\xdfA:\xdc\x11ʼn@\xf8\x84\xf7C֔\x10\x8e@\xf8\x95\xd6&\xe8\v.\x91@\xf8:\x8c0\xe2\x8ey\x95@\xf8\xdfA:\xdc\x11ř@\xf8\x84\xf7C֔\x10\x9e@\xf8\x95\xd6&\xe8\v.\xa1@\xf8:\x8c0\xe2\x8ey\xa5@\xf8\xdfA:\xdc\x11ũ@\xf8\x84\xf7C֔\x10\xae@\xf8\x95\xd6&\xe8\v.\xb1@\xf8:\x8c0\xe2\x8ey\xb5@\xf8\xdfA:\xdc\x11Ź@\xf8\x84\xf7C֔\x10\xbe@\xf8\x95\xd6&\xe8\v.\xc1@\xf8:\x8c0\xe2\x8ey\xc5@\xf8\xdfA:\xdc\x11\xc5\xc9@\xf8\x84\xf7C֔\x10\xce@\xf8\x95\xd6&\xe8\v.\xd1@\xf8:\x8c0\xe2\x8ey\xd5@\xf8\xdfA:\xdc\x11\xc5\xd9@\xf8\x84\xf7C֔\x10\xde@\xf8\x95\xd6&\xe8\v.\xe1@\xf8:\x8c0\xe2\x8ey\xe5@\xf8\xdfA:\xdc\x11\xc5\xe9@\xf8\x84\xf7C֔\x10\xee@\xf8\x95\xd6&\xe8\v.\xf1@\xf8:\x8c0\xe2\x8ey\xf5@\xf8\xdfA:\xdc\x11\xc5\xf9@\xf8\x84\xf7C֔\x10\xfe@\xf8\x95\xd6&\xe8\v.\x01A\xfe\xf0\x7f\x00" + + dserialize := func(data string) *metrics.Float64Histogram { + var res metrics.Float64Histogram + if err := gob.NewDecoder(bytes.NewReader([]byte(data))).Decode(&res); err != nil { + panic(err) + } + return &res + } + latency := RuntimeHistogramFromData(float64(time.Second), dserialize(sLatency)) + b.ResetTimer() + b.ReportAllocs() + for i := 0; i < b.N; i++ { + snap := latency.Snapshot() + // These are the fields that influxdb accesses + _ = snap.Count() + _ = snap.Max() + _ = snap.Mean() + _ = snap.Min() + _ = snap.StdDev() + _ = snap.Variance() + _ = snap.Percentiles([]float64{0.25, 0.5, 0.75, 0.95, 0.99, 0.999, 0.9999}) + } +} From 267656019bac1e797d14ae34ae1d1878f0b0373d Mon Sep 17 00:00:00 2001 From: Martin Holst Swende Date: Mon, 11 Sep 2023 10:18:07 -0400 Subject: [PATCH 23/26] metrics: apply suggestions from review --- metrics/counter.go | 1 - metrics/ewma.go | 8 ++++---- metrics/gauge.go | 10 ++++++++-- metrics/gauge_float64.go | 9 ++------- metrics/sample.go | 2 +- 5 files changed, 15 insertions(+), 15 deletions(-) diff --git a/metrics/counter.go b/metrics/counter.go index 05ec419585ca..cb81599c215a 100644 --- a/metrics/counter.go +++ b/metrics/counter.go @@ -82,7 +82,6 @@ func (c counterSnapshot) Count() int64 { return int64(c) } // NilCounter is a no-op Counter. type NilCounter struct{} -// Clear is a no-op. func (NilCounter) Clear() {} func (NilCounter) Dec(i int64) {} func (NilCounter) Inc(i int64) {} diff --git a/metrics/ewma.go b/metrics/ewma.go index e695c48d8256..1d7a4f00cf45 100644 --- a/metrics/ewma.go +++ b/metrics/ewma.go @@ -39,12 +39,12 @@ func NewEWMA15() EWMA { return NewEWMA(1 - math.Exp(-5.0/60.0/15)) } -// eWMASnapshot is a read-only copy of another EWMA. -type eWMASnapshot float64 +// ewmaSnapshot is a read-only copy of another EWMA. +type ewmaSnapshot float64 // Rate returns the rate of events per second at the time the snapshot was // taken. -func (a eWMASnapshot) Rate() float64 { return float64(a) } +func (a ewmaSnapshot) Rate() float64 { return float64(a) } // NilEWMA is a no-op EWMA. type NilEWMA struct{} @@ -67,7 +67,7 @@ type StandardEWMA struct { // Snapshot returns a read-only copy of the EWMA. func (a *StandardEWMA) Snapshot() EWMASnapshot { r := math.Float64frombits(a.rate.Load()) * float64(time.Second) - return eWMASnapshot(r) + return ewmaSnapshot(r) } // Tick ticks the clock to update the moving average. It assumes it is called diff --git a/metrics/gauge.go b/metrics/gauge.go index d1a56bbddaf8..68f8f11abcd7 100644 --- a/metrics/gauge.go +++ b/metrics/gauge.go @@ -76,8 +76,14 @@ func (g *StandardGauge) Update(v int64) { // Update updates the gauge's value if v is larger then the current valie. func (g *StandardGauge) UpdateIfGt(v int64) { - if g.value.Load() < v { - g.value.Store(v) + for { + exist := g.value.Load() + if exist >= v { + break + } + if g.value.CompareAndSwap(exist, v) { + break + } } } diff --git a/metrics/gauge_float64.go b/metrics/gauge_float64.go index 2f509b1cec8c..967f2bc60e5c 100644 --- a/metrics/gauge_float64.go +++ b/metrics/gauge_float64.go @@ -51,14 +51,9 @@ func (g gaugeFloat64Snapshot) Value() float64 { return float64(g) } // NilGauge is a no-op Gauge. type NilGaugeFloat64 struct{} -// Snapshot is a no-op. func (NilGaugeFloat64) Snapshot() GaugeFloat64Snapshot { return NilGaugeFloat64{} } - -// Update is a no-op. -func (NilGaugeFloat64) Update(v float64) {} - -// Value is a no-op. -func (NilGaugeFloat64) Value() float64 { return 0.0 } +func (NilGaugeFloat64) Update(v float64) {} +func (NilGaugeFloat64) Value() float64 { return 0.0 } // StandardGaugeFloat64 is the standard implementation of a GaugeFloat64 and uses // atomic to manage a single float64 value. diff --git a/metrics/sample.go b/metrics/sample.go index 96598aec710e..d204024f6ba6 100644 --- a/metrics/sample.go +++ b/metrics/sample.go @@ -148,7 +148,7 @@ func (NilSample) Clear() {} func (NilSample) Snapshot() SampleSnapshot { return (*emptySnapshot)(nil) } func (NilSample) Update(v int64) {} -// CalculatePercentiles returns an arbitrary percentile of the slice of int64. +// SamplePercentiles returns an arbitrary percentile of the slice of int64. func SamplePercentile(values []int64, p float64) float64 { return CalculatePercentiles(values, []float64{p})[0] } From 8038c0fb01a541a0285c5ff1e9ad3f10bd328544 Mon Sep 17 00:00:00 2001 From: Martin Holst Swende Date: Mon, 11 Sep 2023 20:30:12 +0200 Subject: [PATCH 24/26] metrics: cache sample variance, rm ineffectual clause rthistogram --- metrics/runtimehistogram.go | 3 +-- metrics/sample.go | 26 ++++++++++++++++---------- 2 files changed, 17 insertions(+), 12 deletions(-) diff --git a/metrics/runtimehistogram.go b/metrics/runtimehistogram.go index 6b0c6a4c4095..2d10c04c031e 100644 --- a/metrics/runtimehistogram.go +++ b/metrics/runtimehistogram.go @@ -102,8 +102,7 @@ func (h *runtimeHistogramSnapshot) calc() { if c == 0 { continue } - - if c > 0 && !minSet { + if !minSet { minSet = true min = int64(math.Floor(h.internal.Buckets[i])) } diff --git a/metrics/sample.go b/metrics/sample.go index d204024f6ba6..152ad0de7bda 100644 --- a/metrics/sample.go +++ b/metrics/sample.go @@ -187,10 +187,11 @@ type sampleSnapshot struct { count int64 values []int64 - max int64 - min int64 - mean float64 - sum int64 + max int64 + min int64 + mean float64 + sum int64 + variance float64 } // newSampleSnapshotPrecalculated creates a read-only sampleSnapShot, using @@ -265,7 +266,12 @@ func (s *sampleSnapshot) Snapshot() SampleSnapshot { return s } // StdDev returns the standard deviation of values at the time the snapshot was // taken. -func (s *sampleSnapshot) StdDev() float64 { return SampleStdDev(s.mean, s.values) } +func (s *sampleSnapshot) StdDev() float64 { + if s.variance == 0.0 { + s.variance = SampleVariance(s.mean, s.values) + } + return math.Sqrt(s.variance) +} // Sum returns the sum of values at the time the snapshot was taken. func (s *sampleSnapshot) Sum() int64 { return s.sum } @@ -278,11 +284,11 @@ func (s *sampleSnapshot) Values() []int64 { } // Variance returns the variance of values at the time the snapshot was taken. -func (s *sampleSnapshot) Variance() float64 { return SampleVariance(s.mean, s.values) } - -// SampleStdDev returns the standard deviation of the slice of int64. -func SampleStdDev(mean float64, values []int64) float64 { - return math.Sqrt(SampleVariance(mean, values)) +func (s *sampleSnapshot) Variance() float64 { + if s.variance == 0.0 { + s.variance = SampleVariance(s.mean, s.values) + } + return s.variance } // SampleVariance returns the variance of the slice of int64. From 878680ee23342d71a06709ab836b131650d7d1b7 Mon Sep 17 00:00:00 2001 From: Martin Holst Swende Date: Mon, 11 Sep 2023 20:35:25 +0200 Subject: [PATCH 25/26] metrics: minor improvements to runtimehistogram loop --- metrics/runtimehistogram.go | 32 ++++++++++++++------------------ 1 file changed, 14 insertions(+), 18 deletions(-) diff --git a/metrics/runtimehistogram.go b/metrics/runtimehistogram.go index 2d10c04c031e..92fcbcc2814c 100644 --- a/metrics/runtimehistogram.go +++ b/metrics/runtimehistogram.go @@ -86,34 +86,30 @@ func newRuntimeHistogramSnapshot(h *metrics.Float64Histogram) *runtimeHistogramS func (h *runtimeHistogramSnapshot) calc() { h.calculated = true var ( - count int64 // number of samples - sum float64 // approx sum of all sample values - minSet = false - min int64 - max float64 + count int64 // number of samples + sum float64 // approx sum of all sample values + min int64 + max float64 ) if len(h.internal.Counts) == 0 { return } for i, c := range h.internal.Counts { - count += int64(c) - midpoint := h.midpoint(i) - sum += midpoint * float64(c) if c == 0 { continue } - if !minSet { - minSet = true + if count == 0 { // Set min only first loop iteration min = int64(math.Floor(h.internal.Buckets[i])) } - if count > 0 { - edge := h.internal.Buckets[i+1] - if math.IsInf(edge, 1) { - edge = h.internal.Buckets[i] - } - if edge > max { - max = edge - } + count += int64(c) + sum += h.midpoint(i) * float64(c) + // Set max on every iteration + edge := h.internal.Buckets[i+1] + if math.IsInf(edge, 1) { + edge = h.internal.Buckets[i] + } + if edge > max { + max = edge } } h.min = min From 7d352f8acc4307a89c39dc31617a321bd7a8f426 Mon Sep 17 00:00:00 2001 From: Martin Holst Swende Date: Wed, 13 Sep 2023 07:39:36 +0200 Subject: [PATCH 26/26] metrics: oblige nitpicks from review --- metrics/sample.go | 15 +++++++-------- 1 file changed, 7 insertions(+), 8 deletions(-) diff --git a/metrics/sample.go b/metrics/sample.go index 152ad0de7bda..5398dd42d5de 100644 --- a/metrics/sample.go +++ b/metrics/sample.go @@ -83,14 +83,14 @@ func (s *ExpDecaySample) Clear() { func (s *ExpDecaySample) Snapshot() SampleSnapshot { s.mutex.Lock() defer s.mutex.Unlock() - vals := s.values.Values() - values := make([]int64, len(vals)) var ( - max int64 = math.MinInt64 - min int64 = math.MaxInt64 - sum int64 + samples = s.values.Values() + values = make([]int64, len(samples)) + max int64 = math.MinInt64 + min int64 = math.MaxInt64 + sum int64 ) - for i, item := range vals { + for i, item := range samples { v := item.v values[i] = v sum += v @@ -196,8 +196,7 @@ type sampleSnapshot struct { // newSampleSnapshotPrecalculated creates a read-only sampleSnapShot, using // precalculated sums to avoid iterating the values -func newSampleSnapshotPrecalculated(count int64, values []int64, - min, max, sum int64) *sampleSnapshot { +func newSampleSnapshotPrecalculated(count int64, values []int64, min, max, sum int64) *sampleSnapshot { if len(values) == 0 { return &sampleSnapshot{ count: count,