chore(promHistogram): remove Merge implementation and add LinearBuckets

Author: clementnuss

prometheus_histogram.go

@@ -7,7 +7,7 @@ import (
 	"sync"
 )
 
-var defaultUpperBounds = []float64{.005, .01, .025, .05, .1, .25, .5, 1, 2.5, 5, 10}
+var PrometheusHistogramDefaultBuckets = []float64{.005, .01, .025, .05, .1, .25, .5, 1, 2.5, 5, 10}
 
 // Prometheus Histogram is a histogram for non-negative values with pre-defined buckets
 //
@@ -79,38 +79,6 @@ func (h *PrometheusHistogram) Update(v float64) {
 	h.buckets[bucketIdx]++
 }
 
-// Merge merges src to h
-func (h *PrometheusHistogram) Merge(src *PrometheusHistogram) {
-	// first we must compare if the upper bounds are identical
-	valid := true
-	if len(h.upperBounds) != len(src.upperBounds) {
-		valid = false
-	} else {
-		for i := range h.upperBounds {
-			if h.upperBounds[i] != src.upperBounds[i] {
-				valid = false
-			}
-		}
-	}
-
-	if !valid {
-		panic("impossible to merge buckets with different bucket upper bounds")
-	}
-
-	h.mu.Lock()
-	defer h.mu.Unlock()
-
-	src.mu.Lock()
-	defer src.mu.Unlock()
-
-	h.sum += src.sum
-	h.count += src.count
-
-	for i := range h.buckets {
-		h.buckets[i] += src.buckets[i]
-	}
-}
-
 // NewPrometheusHistogram creates and returns new prometheus histogram with the given name.
 //
 // name must be valid Prometheus-compatible metric with possible labels.
@@ -176,7 +144,7 @@ func GetOrCreatePrometheusHistogramExt(name string, upperBounds []float64) *Prom
 }
 
 func newPrometheusHistogram(upperBounds []float64) *PrometheusHistogram {
-	validateBuckets(upperBounds)
+	mustValidateBuckets(upperBounds)
 	last := len(upperBounds) - 1
 	if math.IsInf(upperBounds[last], +1) {
 		upperBounds = upperBounds[:last] // ignore +Inf bucket as it is covered anyways
@@ -189,15 +157,34 @@ func newPrometheusHistogram(upperBounds []float64) *PrometheusHistogram {
 	return &h
 }
 
-func validateBuckets(upperBounds []float64) {
+func mustValidateBuckets(upperBounds []float64) {
+	if err := ValidateBuckets(upperBounds); err != nil {
+		panic(err)
+	}
+}
+
+func ValidateBuckets(upperBounds []float64) error {
 	if len(upperBounds) == 0 {
-		panic("no upper bounds were given for the buckets")
+		return fmt.Errorf("no upper bounds were given for the buckets")
 	}
 	for i := 0; i < len(upperBounds)-1; i++ {
 		if upperBounds[i] >= upperBounds[i+1] {
-			panic("upper bounds for the buckets must be strictly increasing")
+			return fmt.Errorf("upper bounds for the buckets must be strictly increasing")
 		}
 	}
+	return nil
+}
+
+func LinearBuckets(start, width float64, count int) []float64 {
+	if count < 1 {
+		panic("LinearBuckets needs a positive count")
+	}
+	upperBounds := make([]float64, count)
+	for i := range upperBounds {
+		upperBounds[i] = start
+		start += width
+	}
+	return upperBounds
 }
 
 func (h *PrometheusHistogram) marshalTo(prefix string, w io.Writer) {

prometheus_histogram_test.go

@@ -44,7 +44,7 @@ prefix_count 405
 	testMarshalTo(t, h, "prefix", expected)
 
 	// make sure that if the +Inf bucket is manually specified it gets ignored and we have the same resutls at the end
-	h2 := NewPrometheusHistogramExt("TestPrometheusHistogram2", append(defaultUpperBounds, math.Inf(+1)))
+	h2 := NewPrometheusHistogramExt("TestPrometheusHistogram2", append(PrometheusHistogramDefaultBuckets, math.Inf(+1)))
 
 	h.Reset()
 
@@ -63,46 +63,41 @@ prefix_count 405
 	testMarshalTo(t, h2, "prefix", expected)
 }
 
-func TestPrometheusHistogramMerge(t *testing.T) {
-	name := `TestPrometheusHistogramMerge`
-	h := NewPrometheusHistogram(name)
-	// Write data to histogram
-	for i := 0; i <= 10_100; i += 25 { // from 0 to 10'100 ms in 25ms steps
-		h.Update(float64(i) * 1e-3)
-	}
+func TestPrometheusHistogramLinearBuckets(t *testing.T) {
+	const expected string = `prefix_bucket{le="0.5"} 1
+prefix_bucket{le="1.5"} 2
+prefix_bucket{le="2.5"} 3
+prefix_bucket{le="3.5"} 4
+prefix_bucket{le="4.5"} 5
+prefix_bucket{le="5.5"} 6
+prefix_bucket{le="6.5"} 7
+prefix_bucket{le="7.5"} 8
+prefix_bucket{le="8.5"} 9
+prefix_bucket{le="9.5"} 10
+prefix_bucket{le="+Inf"} 11
+prefix_sum 55
+prefix_count 11
+`
+	name := "TestPrometheusHistogramLinearBuckets"
+	upperBounds := LinearBuckets(0.5, 1.0, 10)
+	h := NewPrometheusHistogramExt(name, upperBounds)
 
-	b := NewPrometheusHistogram("test")
-	for i := 0; i <= 10_100; i += 25 { // from 0 to 10'100 ms in 25ms steps
-		h.Update(float64(i) * 1e-3)
+	// Write data to histogram
+	for i := 0; i <= 10; i++ { // from 0 to 10
+		h.Update(float64(i))
 	}
 
-	h.Merge(b)
-
 	// Make sure the histogram prints <prefix>_bucket on marshalTo call
-	testMarshalTo(t, h, "prefix", `prefix_bucket{le="0.005"} 2
-prefix_bucket{le="0.01"} 2
-prefix_bucket{le="0.025"} 4
-prefix_bucket{le="0.05"} 6
-prefix_bucket{le="0.1"} 10
-prefix_bucket{le="0.25"} 22
-prefix_bucket{le="0.5"} 42
-prefix_bucket{le="1"} 82
-prefix_bucket{le="2.5"} 202
-prefix_bucket{le="5"} 402
-prefix_bucket{le="10"} 802
-prefix_bucket{le="+Inf"} 810
-prefix_sum 4090.5
-prefix_count 810
-`)
+	testMarshalTo(t, h, "prefix", expected)
 
+	// Make sure we panic when the count of linear buckets is < 1
 	func() {
 		defer func() {
 			if r := recover(); r == nil {
-				t.Error("Merge did not panic with mimatched buckets.")
+				t.Errorf("LinearBuckets should've panicked with a count of 0.")
 			}
 		}()
-		h2 := NewPrometheusHistogramExt("TestPrometheusHistogramMerge2", []float64{14})
-		h.Merge(h2)
+		_ = LinearBuckets(.14, .22, 0)
 	}()
 }
 

set.go

@@ -140,7 +140,7 @@ func (s *Set) GetOrCreateHistogram(name string) *Histogram {
 //
 // The returned histogram is safe to use from concurrent goroutines.
 func (s *Set) NewPrometheusHistogram(name string) *PrometheusHistogram {
-	return s.NewPrometheusHistogramExt(name, defaultUpperBounds)
+	return s.NewPrometheusHistogramExt(name, PrometheusHistogramDefaultBuckets)
 }
 
 
@@ -176,7 +176,7 @@ func (s *Set) NewPrometheusHistogramExt(name string, upperBounds []float64) *Pro
 //
 // Performance tip: prefer NewPrometheusHistogram instead of GetOrCreatePrometheusHistogram.
 func (s *Set) GetOrCreatePrometheusHistogram(name string) *PrometheusHistogram {
-	return s.GetOrCreatePrometheusHistogramExt(name, defaultUpperBounds)
+	return s.GetOrCreatePrometheusHistogramExt(name, PrometheusHistogramDefaultBuckets)
 }
 
 // GetOrCreatePrometheusHistogramExt returns registered prometheus histogram in