From 807b1ee73c2079346d041019cffb933fa43aaf3d Mon Sep 17 00:00:00 2001 From: Arun Mahendra <89400134+arun-shopify@users.noreply.github.com> Date: Tue, 2 Aug 2022 04:48:18 -0400 Subject: [PATCH] explicitly add +inf bucket in withExemplarsMetric (#1094) * explicitly adding +inf bucket to withExemplarsMetric Signed-off-by: Arun Mahendra * Update prometheus/metric_test.go Co-authored-by: Bartlomiej Plotka * Update prometheus/metric.go Co-authored-by: Bartlomiej Plotka * updated comment and removed unnecessary test Co-authored-by: Bartlomiej Plotka --- prometheus/metric.go | 12 ++++++++++-- prometheus/metric_test.go | 14 ++++++++++++-- 2 files changed, 22 insertions(+), 4 deletions(-) diff --git a/prometheus/metric.go b/prometheus/metric.go index 48d4a5d50..63e1491e6 100644 --- a/prometheus/metric.go +++ b/prometheus/metric.go @@ -15,6 +15,7 @@ package prometheus import ( "errors" + "math" "sort" "strings" "time" @@ -184,8 +185,15 @@ func (m *withExemplarsMetric) Write(pb *dto.Metric) error { if i < len(pb.Histogram.Bucket) { pb.Histogram.Bucket[i].Exemplar = e } else { - // This is not possible as last bucket is Inf. - panic("no bucket was found for given exemplar value") + // The +Inf bucket should be explicitly added if there is an exemplar for it, similar to non-const histogram logic in https://github.com/prometheus/client_golang/blob/main/prometheus/histogram.go#L357-L365. + b := &dto.Bucket{ + CumulativeCount: proto.Uint64(pb.Histogram.Bucket[len(pb.Histogram.GetBucket())-1].GetCumulativeCount()), + UpperBound: proto.Float64(math.Inf(1)), + Exemplar: e, + } + pb.Histogram.Bucket = append(pb.Histogram.Bucket, b) + break + // Terminating the loop after creating the +Inf bucket and adding one exemplar, if there are other exemplars in the +Inf bucket range they will be ignored. } } default: diff --git a/prometheus/metric_test.go b/prometheus/metric_test.go index 61d807cdc..6e90d0b13 100644 --- a/prometheus/metric_test.go +++ b/prometheus/metric_test.go @@ -14,6 +14,7 @@ package prometheus import ( + "math" "testing" //nolint:staticcheck // Ignore SA1019. Need to keep deprecated package for compatibility. @@ -56,16 +57,19 @@ func TestWithExemplarsMetric(t *testing.T) { {Value: proto.Float64(89.0)}, {Value: proto.Float64(100.0)}, {Value: proto.Float64(157.0)}, + {Value: proto.Float64(500.0)}, + {Value: proto.Float64(2000.0)}, }} metric := dto.Metric{} if err := m.Write(&metric); err != nil { t.Fatal(err) } - if want, got := 4, len(metric.GetHistogram().Bucket); want != got { + if want, got := 5, len(metric.GetHistogram().Bucket); want != got { t.Errorf("want %v, got %v", want, got) } - expectedExemplarVals := []float64{24.0, 42.0, 100.0, 157.0} + // When there are more exemplars than there are buckets, a +Inf bucket will be created and the last exemplar value will be added. + expectedExemplarVals := []float64{24.0, 42.0, 100.0, 157.0, 500.0} for i, b := range metric.GetHistogram().Bucket { if b.Exemplar == nil { t.Errorf("Expected exemplar for bucket %v, got nil", i) @@ -74,5 +78,11 @@ func TestWithExemplarsMetric(t *testing.T) { t.Errorf("%v: want %v, got %v", i, want, got) } } + + infBucket := metric.GetHistogram().Bucket[len(metric.GetHistogram().Bucket)-1].GetUpperBound() + + if infBucket != math.Inf(1) { + t.Errorf("want %v, got %v", math.Inf(1), infBucket) + } }) }