[monitoring] Add ClearOnVisitHistogram to adapter for go-metrics (#330)

Add ClearOnVisitHistogram to adapter for go-metrics

This type is the same as go-metrics Histogram except that it is cleared (counts reset) every time it is Visited (read for a report).

Author: leehinman

monitoring/adapter/go-metrics-wrapper.go

@@ -25,7 +25,7 @@ import (
 
 // go-metrics wrapper interface required to unpack the original metric
 type goMetricsWrapper interface {
-	wrapped() interface{}
+	wrapped() any
 }
 
 // go-metrics wrappers
@@ -40,13 +40,14 @@ type (
 		g metrics.FunctionalGaugeFloat64
 	}
 
-	goMetricsHistogram struct{ h metrics.Histogram }
+	goMetricsHistogram             struct{ h metrics.Histogram }
+	goMetricsClearOnVisitHistogram struct{ h *ClearOnVisitHistogram }
 
 	goMetricsMeter struct{ m metrics.Meter }
 )
 
 // goMetricsWrap tries to wrap a metric for use with monitoring package.
-func goMetricsWrap(metric interface{}) (monitoring.Var, bool) {
+func goMetricsWrap(metric any) (monitoring.Var, bool) {
 	switch v := metric.(type) {
 	case *metrics.StandardCounter:
 		return goMetricsCounter{v}, true
@@ -62,42 +63,44 @@ func goMetricsWrap(metric interface{}) (monitoring.Var, bool) {
 		return goMetricsHistogram{v}, true
 	case *metrics.StandardMeter:
 		return goMetricsMeter{v}, true
+	case *ClearOnVisitHistogram:
+		return goMetricsClearOnVisitHistogram{v}, true
 	}
 	return nil, false
 }
 
-func (w goMetricsCounter) wrapped() interface{} { return w.c }
-func (w goMetricsCounter) Get() int64           { return w.c.Count() }
+func (w goMetricsCounter) wrapped() any { return w.c }
+func (w goMetricsCounter) Get() int64   { return w.c.Count() }
 func (w goMetricsCounter) Visit(_ monitoring.Mode, vs monitoring.Visitor) {
 	vs.OnInt(w.Get())
 }
 
-func (w goMetricsGauge) wrapped() interface{} { return w.g }
-func (w goMetricsGauge) Get() int64           { return w.g.Value() }
+func (w goMetricsGauge) wrapped() any { return w.g }
+func (w goMetricsGauge) Get() int64   { return w.g.Value() }
 func (w goMetricsGauge) Visit(_ monitoring.Mode, vs monitoring.Visitor) {
 	vs.OnInt(w.Get())
 }
 
-func (w goMetricsGaugeFloat64) wrapped() interface{} { return w.g }
-func (w goMetricsGaugeFloat64) Get() float64         { return w.g.Value() }
+func (w goMetricsGaugeFloat64) wrapped() any { return w.g }
+func (w goMetricsGaugeFloat64) Get() float64 { return w.g.Value() }
 func (w goMetricsGaugeFloat64) Visit(_ monitoring.Mode, vs monitoring.Visitor) {
 	vs.OnFloat(w.Get())
 }
 
-func (w goMetricsFuncGauge) wrapped() interface{} { return w.g }
-func (w goMetricsFuncGauge) Get() int64           { return w.g.Value() }
+func (w goMetricsFuncGauge) wrapped() any { return w.g }
+func (w goMetricsFuncGauge) Get() int64   { return w.g.Value() }
 func (w goMetricsFuncGauge) Visit(_ monitoring.Mode, vs monitoring.Visitor) {
 	vs.OnInt(w.Get())
 }
 
-func (w goMetricsFuncGaugeFloat) wrapped() interface{} { return w.g }
-func (w goMetricsFuncGaugeFloat) Get() float64         { return w.g.Value() }
+func (w goMetricsFuncGaugeFloat) wrapped() any { return w.g }
+func (w goMetricsFuncGaugeFloat) Get() float64 { return w.g.Value() }
 func (w goMetricsFuncGaugeFloat) Visit(_ monitoring.Mode, vs monitoring.Visitor) {
 	vs.OnFloat(w.Get())
 }
 
-func (w goMetricsHistogram) wrapped() interface{} { return w.h }
-func (w goMetricsHistogram) Get() int64           { return w.h.Sum() }
+func (w goMetricsHistogram) wrapped() any { return w.h }
+func (w goMetricsHistogram) Get() int64   { return w.h.Sum() }
 func (w goMetricsHistogram) Visit(_ monitoring.Mode, vs monitoring.Visitor) {
 	vs.OnRegistryStart()
 	defer vs.OnRegistryFinished()
@@ -126,8 +129,30 @@ func (w goMetricsHistogram) Visit(_ monitoring.Mode, vs monitoring.Visitor) {
 	vs.OnFloat(ps[4])
 }
 
-func (w goMetricsMeter) wrapped() interface{} { return w.m }
-func (w goMetricsMeter) Get() int64           { return w.m.Count() }
+func (w goMetricsMeter) wrapped() any { return w.m }
+func (w goMetricsMeter) Get() int64   { return w.m.Count() }
 func (w goMetricsMeter) Visit(_ monitoring.Mode, vs monitoring.Visitor) {
 	vs.OnInt(w.Get())
 }
+
+func (w goMetricsClearOnVisitHistogram) wrapped() any { return w.h }
+func (w goMetricsClearOnVisitHistogram) Get() int64   { return w.h.Sum() }
+func (w goMetricsClearOnVisitHistogram) Visit(_ monitoring.Mode, vs monitoring.Visitor) {
+	vs.OnRegistryStart()
+	defer vs.OnRegistryFinished()
+
+	h := w.h.Snapshot()
+	w.h.Clear()
+	ps := h.Percentiles([]float64{0.5, 0.99})
+	vs.OnKey("count")
+	vs.OnInt(h.Count())
+	vs.OnKey("min")
+	vs.OnInt(h.Min())
+	vs.OnKey("max")
+	vs.OnInt(h.Max())
+	vs.OnKey("median")
+	vs.OnFloat(ps[0])
+	vs.OnKey("p99")
+	vs.OnFloat(ps[1])
+
+}

monitoring/adapter/go-metrics.go

@@ -36,9 +36,10 @@ import (
 // the go-metrics.Registry interface.
 //
 // Note: with the go-metrics using `interface{}`, there is no guarantee
-//       a variable satisfying any of go-metrics interfaces is returned.
-//       It's recommended to not mix go-metrics with other metrics types
-//       in the same namespace.
+//
+//	a variable satisfying any of go-metrics interfaces is returned.
+//	It's recommended to not mix go-metrics with other metrics types
+//	in the same namespace.
 type GoMetricsRegistry struct {
 	mutex sync.Mutex
 
@@ -54,14 +55,15 @@ type GoMetricsRegistry struct {
 // If the monitoring.Registry does not exist yet, a new one will be generated.
 //
 // Note: with users of go-metrics potentially removing any metric at runtime,
-//       it's recommended to have the underlying registry being generated with
-//       `monitoring.IgnorePublishExpvar`.
+//
+//	it's recommended to have the underlying registry being generated with
+//	`monitoring.IgnorePublishExpvar`.
 func GetGoMetrics(parent *monitoring.Registry, name string, filters ...MetricFilter) *GoMetricsRegistry {
 	v := parent.Get(name)
 	if v == nil {
 		return NewGoMetrics(parent, name, filters...)
 	}
-	return newGoMetrics(v.(*monitoring.Registry), filters...)
+	return newGoMetrics(v.(*monitoring.Registry), filters...) //nolint:errcheck //code depends on panic
 }
 
 // NewGoMetrics creates and registers a new GoMetricsRegistry with the parent
@@ -97,9 +99,10 @@ func (r *GoMetricsRegistry) find(name string) interface{} {
 // Get retrieves a registered metric by name. If the name is unknown, Get returns nil.
 //
 // Note: with the return values being `interface{}`, there is no guarantee
-//       a variable satisfying any of go-metrics interfaces is returned.
-//       It's recommended to not mix go-metrics with other metrics types in one
-//       namespace.
+//
+//	a variable satisfying any of go-metrics interfaces is returned.
+//	It's recommended to not mix go-metrics with other metrics types in one
+//	namespace.
 func (r *GoMetricsRegistry) Get(name string) interface{} {
 	r.mutex.Lock()
 	defer r.mutex.Unlock()

monitoring/adapter/go-metrics_test.go

@@ -58,15 +58,15 @@ func TestGoMetricsAdapter(t *testing.T) {
 	// register some metrics and check they're satisfying the go-metrics interface
 	// no matter if owned by monitoring or go-metrics
 	for name := range counters {
-		cnt, ok := reg.GetOrRegister(name, func() interface{} {
+		cnt, ok := reg.GetOrRegister(name, func() any {
 			return metrics.NewCounter()
 		}).(metrics.Counter)
 		require.True(t, ok)
 		cnt.Clear()
 	}
 
 	for name := range meters {
-		meter, ok := reg.GetOrRegister(name, func() interface{} {
+		meter, ok := reg.GetOrRegister(name, func() any {
 			return metrics.NewMeter()
 		}).(metrics.Meter)
 		require.True(t, ok)
@@ -100,14 +100,117 @@ func TestGoMetricsAdapter(t *testing.T) {
 	}
 
 	// check Each only returns metrics not registered with monitoring.Registry
-	reg.Each(func(name string, v interface{}) {
+	reg.Each(func(name string, v any) {
 		if strings.HasPrefix(name, "mon") {
 			t.Errorf("metric %v should not have been reported by each", name)
 		}
 	})
-	monReg.Do(monitoring.Full, func(name string, v interface{}) {
+	monReg.Do(monitoring.Full, func(name string, v any) {
 		if !strings.HasPrefix(name, "test.mon") {
 			t.Errorf("metric %v should not have been reported by each", name)
 		}
 	})
 }
+
+func TestGoMetricsHistogramClearOnVisit(t *testing.T) {
+	monReg := monitoring.NewRegistry()
+	histogramSample := metrics.NewUniformSample(10)
+	clearedHistogramSample := metrics.NewUniformSample(10)
+	_ = NewGoMetrics(monReg, "original", Accept).Register("histogram", metrics.NewHistogram(histogramSample))
+	_ = NewGoMetrics(monReg, "cleared", Accept).Register("histogram", NewClearOnVisitHistogram(clearedHistogramSample))
+	dataPoints := [...]int{2, 4, 8, 4, 2}
+	dataPointsMedian := 4.0
+	for _, i := range dataPoints {
+		histogramSample.Update(int64(i))
+		clearedHistogramSample.Update(int64(i))
+	}
+
+	preSnapshot := []struct {
+		expected any
+		actual   any
+		msg      string
+	}{
+		{
+			actual:   histogramSample.Count(),
+			expected: int64(len(dataPoints)),
+			msg:      "histogram sample count incorrect",
+		},
+		{
+			actual:   clearedHistogramSample.Count(),
+			expected: int64(len(dataPoints)),
+			msg:      "cleared histogram sample count incorrect",
+		},
+		{
+			actual:   histogramSample.Percentiles([]float64{0.5}),
+			expected: []float64{dataPointsMedian},
+			msg:      "histogram median incorrect",
+		},
+		{
+			actual:   clearedHistogramSample.Percentiles([]float64{0.5}),
+			expected: []float64{dataPointsMedian},
+			msg:      "cleared histogram median incorrect",
+		},
+	}
+
+	for _, tc := range preSnapshot {
+		require.Equal(t, tc.expected, tc.actual, tc.msg)
+	}
+
+	// collecting the snapshot triggers the visit of each histogram
+	flatSnapshot := monitoring.CollectFlatSnapshot(monReg, monitoring.Full, false)
+
+	// Check to make sure after the snapshot the samples in
+	// clearedHistogramSample have been reset to zero, but the
+	// snapshot reports the values before the clear
+
+	postSnapshot := []struct {
+		expected any
+		actual   any
+		msg      string
+	}{
+		{
+			actual:   histogramSample.Count(),
+			expected: int64(len(dataPoints)),
+			msg:      "histogram sample count incorrect",
+		},
+		{
+			actual:   clearedHistogramSample.Count(),
+			expected: int64(0),
+			msg:      "cleared histogram sample count incorrect",
+		},
+		{
+			actual:   histogramSample.Percentiles([]float64{0.5}),
+			expected: []float64{dataPointsMedian},
+			msg:      "histogram median incorrect",
+		},
+		{
+			actual:   clearedHistogramSample.Percentiles([]float64{0.5}),
+			expected: []float64{0},
+			msg:      "cleared histogram median incorrect",
+		},
+		{
+			actual:   flatSnapshot.Ints["original.histogram.count"],
+			expected: int64(len(dataPoints)),
+			msg:      "visited histogram count is wrong",
+		},
+		{
+			actual:   flatSnapshot.Ints["cleared.histogram.count"],
+			expected: int64(len(dataPoints)),
+			msg:      "visited cleared histogram count is wrong",
+		},
+		{
+			actual:   flatSnapshot.Floats["original.histogram.median"],
+			expected: float64(dataPointsMedian),
+			msg:      "visited histogram median is wrong",
+		},
+		{
+			actual:   flatSnapshot.Floats["cleared.histogram.median"],
+			expected: float64(dataPointsMedian),
+			msg:      "visited cleared histogram median is wrong",
+		},
+	}
+
+	for _, tc := range postSnapshot {
+		require.Equal(t, tc.expected, tc.actual, tc.msg)
+	}
+}