metrics: Support Trend.Max with negative values

grafana · Apr 20, 2023 · b9e7b8c · b9e7b8c
1 parent d946fdb
commit b9e7b8c
Show file tree

Hide file tree

Showing 2 changed files with 60 additions and 9 deletions.
diff --git a/metrics/sink.go b/metrics/sink.go
@@ -80,18 +80,22 @@ type TrendSink struct {
 func (t *TrendSink) IsEmpty() bool { return t.Count == 0 }
 
 func (t *TrendSink) Add(s Sample) {
+	if t.Count == 0 {
+		t.Max, t.Min = s.Value, s.Value
+	} else {
+		if s.Value > t.Max {
+			t.Max = s.Value
+		}
+		if s.Value < t.Min {
+			t.Min = s.Value
+		}
+	}
+
 	t.Values = append(t.Values, s.Value)
 	t.sorted = false
 	t.Count += 1
 	t.Sum += s.Value
 	t.Avg = t.Sum / float64(t.Count)
-
-	if s.Value > t.Max {
-		t.Max = s.Value
-	}
-	if s.Value < t.Min || t.Count == 1 {
-		t.Min = s.Value
-	}
 }
 
 // P calculates the given percentile from sink values.

diff --git a/metrics/sink_test.go b/metrics/sink_test.go
@@ -1,6 +1,7 @@
 package metrics
 
 import (
+	"math"
 	"testing"
 	"time"
 
@@ -70,47 +71,89 @@ func TestGaugeSink(t *testing.T) {
 }
 
 func TestTrendSink(t *testing.T) {
+	t.Parallel()
+
 	unsortedSamples10 := []float64{0.0, 100.0, 30.0, 80.0, 70.0, 60.0, 50.0, 40.0, 90.0, 20.0}
 
 	t.Run("add", func(t *testing.T) {
 		t.Run("one value", func(t *testing.T) {
+			t.Parallel()
+
 			sink := TrendSink{}
 			sink.Add(Sample{TimeSeries: TimeSeries{Metric: &Metric{}}, Value: 7.0})
 			assert.Equal(t, uint64(1), sink.Count)
-			assert.Equal(t, false, sink.sorted)
 			assert.Equal(t, 7.0, sink.Min)
 			assert.Equal(t, 7.0, sink.Max)
 			assert.Equal(t, 7.0, sink.Avg)
+			assert.Equal(t, 7.0, sink.Sum)
+			assert.Equal(t, []float64{7.0}, sink.Values)
 		})
 		t.Run("values", func(t *testing.T) {
+			t.Parallel()
+
 			sink := TrendSink{}
 			for _, s := range unsortedSamples10 {
 				sink.Add(Sample{TimeSeries: TimeSeries{Metric: &Metric{}}, Value: s})
 			}
 			assert.Equal(t, uint64(len(unsortedSamples10)), sink.Count)
-			assert.Equal(t, false, sink.sorted)
 			assert.Equal(t, 0.0, sink.Min)
 			assert.Equal(t, 100.0, sink.Max)
 			assert.Equal(t, 54.0, sink.Avg)
+			assert.Equal(t, 540.0, sink.Sum)
+			assert.Equal(t, unsortedSamples10, sink.Values)
+		})
+		t.Run("negative", func(t *testing.T) {
+			t.Parallel()
+
+			sink := TrendSink{}
+			for _, s := range []float64{-10, -20} {
+				sink.Add(Sample{TimeSeries: TimeSeries{Metric: &Metric{}}, Value: s})
+			}
+			assert.Equal(t, uint64(2), sink.Count)
+			assert.Equal(t, -20.0, sink.Min)
+			assert.Equal(t, -10.0, sink.Max)
+			assert.Equal(t, -15.0, sink.Avg)
+			assert.Equal(t, -30.0, sink.Sum)
+			assert.Equal(t, []float64{-10, -20}, sink.Values)
+		})
+		t.Run("mixed", func(t *testing.T) {
+			t.Parallel()
+
+			sink := TrendSink{}
+			for _, s := range []float64{1.4, 0, -1.2} {
+				sink.Add(Sample{TimeSeries: TimeSeries{Metric: &Metric{}}, Value: s})
+			}
+			assert.Equal(t, uint64(3), sink.Count)
+			assert.Equal(t, -1.2, sink.Min)
+			assert.Equal(t, 1.4, sink.Max)
+			assert.Equal(t, 0.067, math.Round(sink.Avg*1000)/1000)
+			assert.Equal(t, 0.199, math.Floor(sink.Sum*1000)/1000)
+			assert.Equal(t, []float64{1.4, 0, -1.2}, sink.Values)
 		})
 	})
 
 	tolerance := 0.000001
 	t.Run("percentile", func(t *testing.T) {
 		t.Run("no values", func(t *testing.T) {
+			t.Parallel()
+
 			sink := TrendSink{}
 			for i := 1; i <= 100; i++ {
 				assert.Equal(t, 0.0, sink.P(float64(i)/100.0))
 			}
 		})
 		t.Run("one value", func(t *testing.T) {
+			t.Parallel()
+
 			sink := TrendSink{}
 			sink.Add(Sample{TimeSeries: TimeSeries{Metric: &Metric{}}, Value: 10.0})
 			for i := 1; i <= 100; i++ {
 				assert.Equal(t, 10.0, sink.P(float64(i)/100.0))
 			}
 		})
 		t.Run("two values", func(t *testing.T) {
+			t.Parallel()
+
 			sink := TrendSink{}
 			sink.Add(Sample{TimeSeries: TimeSeries{Metric: &Metric{}}, Value: 5.0})
 			sink.Add(Sample{TimeSeries: TimeSeries{Metric: &Metric{}}, Value: 10.0})
@@ -123,6 +166,8 @@ func TestTrendSink(t *testing.T) {
 			assert.Equal(t, true, sink.sorted)
 		})
 		t.Run("more than 2", func(t *testing.T) {
+			t.Parallel()
+
 			sink := TrendSink{}
 			for _, s := range unsortedSamples10 {
 				sink.Add(Sample{TimeSeries: TimeSeries{Metric: &Metric{}}, Value: s})
@@ -136,6 +181,8 @@ func TestTrendSink(t *testing.T) {
 		})
 	})
 	t.Run("format", func(t *testing.T) {
+		t.Parallel()
+
 		sink := TrendSink{}
 		for _, s := range unsortedSamples10 {
 			sink.Add(Sample{TimeSeries: TimeSeries{Metric: &Metric{}}, Value: s})