From 65f2ab6ea7081ff02f37bd84f52a942c94a3758e Mon Sep 17 00:00:00 2001 From: Balint Zsilavecz Date: Wed, 12 Oct 2022 15:07:31 +0100 Subject: [PATCH 1/4] Fix `CumulativeCount` value of `+Inf` bucket created from exemplar Signed-off-by: Balint Zsilavecz --- prometheus/metric.go | 2 +- prometheus/metric_test.go | 10 +++++++--- 2 files changed, 8 insertions(+), 4 deletions(-) diff --git a/prometheus/metric.go b/prometheus/metric.go index f0941f6f0..b5119c504 100644 --- a/prometheus/metric.go +++ b/prometheus/metric.go @@ -187,7 +187,7 @@ func (m *withExemplarsMetric) Write(pb *dto.Metric) error { } else { // 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()), + CumulativeCount: proto.Uint64(pb.Histogram.GetSampleCount()), UpperBound: proto.Float64(math.Inf(1)), Exemplar: e, } diff --git a/prometheus/metric_test.go b/prometheus/metric_test.go index 2d69b08f9..33da8513f 100644 --- a/prometheus/metric_test.go +++ b/prometheus/metric_test.go @@ -79,10 +79,14 @@ func TestWithExemplarsMetric(t *testing.T) { } } - infBucket := metric.GetHistogram().Bucket[len(metric.GetHistogram().Bucket)-1].GetUpperBound() + infBucket := metric.GetHistogram().Bucket[len(metric.GetHistogram().Bucket)-1] - if infBucket != math.Inf(1) { - t.Errorf("want %v, got %v", math.Inf(1), infBucket) + if upperBound := infBucket.GetUpperBound(); upperBound != math.Inf(1) { + t.Errorf("want %v, got %v", math.Inf(1), upperBound) + } + + if cumulativeCount := infBucket.GetCumulativeCount(); cumulativeCount != 4711 { + t.Errorf("want %v, got %v", 4711, cumulativeCount) } }) } From 80a1737729dab8374ec189fd67d5c073f2d4cb92 Mon Sep 17 00:00:00 2001 From: Balint Zsilavecz Date: Thu, 13 Oct 2022 11:28:38 +0100 Subject: [PATCH 2/4] Update prometheus/metric_test.go Co-authored-by: Bartlomiej Plotka Signed-off-by: Balint Zsilavecz --- prometheus/metric_test.go | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/prometheus/metric_test.go b/prometheus/metric_test.go index 33da8513f..e152d6211 100644 --- a/prometheus/metric_test.go +++ b/prometheus/metric_test.go @@ -81,12 +81,12 @@ func TestWithExemplarsMetric(t *testing.T) { infBucket := metric.GetHistogram().Bucket[len(metric.GetHistogram().Bucket)-1] - if upperBound := infBucket.GetUpperBound(); upperBound != math.Inf(1) { - t.Errorf("want %v, got %v", math.Inf(1), upperBound) + if want, got := infBucket.GetUpperBound(), math.Inf(1); want != got { + t.Errorf("want %v, got %v", want, got) } - if cumulativeCount := infBucket.GetCumulativeCount(); cumulativeCount != 4711 { - t.Errorf("want %v, got %v", 4711, cumulativeCount) + if want, got := infBucket.GetCumulativeCount(), uint64(4711); want != got { + t.Errorf("want %v, got %v", want, got) } }) } From 9c6dbbb70f44c75fefc9efefe26b3f9b7c99ebde Mon Sep 17 00:00:00 2001 From: Balint Zsilavecz Date: Thu, 13 Oct 2022 11:32:45 +0100 Subject: [PATCH 3/4] Clarify description of implicit `+Inf` bucket count Signed-off-by: Balint Zsilavecz --- prometheus/histogram.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/prometheus/histogram.go b/prometheus/histogram.go index 0d47fecdc..73e814a4d 100644 --- a/prometheus/histogram.go +++ b/prometheus/histogram.go @@ -613,7 +613,7 @@ func (h *constHistogram) Write(out *dto.Metric) error { // to send it to Prometheus in the Collect method. // // buckets is a map of upper bounds to cumulative counts, excluding the +Inf -// bucket. +// bucket. The +Inf bucket is implicit, and its value is equal to the provided count. // // NewConstHistogram returns an error if the length of labelValues is not // consistent with the variable labels in Desc or if Desc is invalid. From 0b02476d660c9e3e70865c42109d39428922d869 Mon Sep 17 00:00:00 2001 From: Balint Zsilavecz Date: Thu, 13 Oct 2022 11:37:28 +0100 Subject: [PATCH 4/4] Fix test variables Signed-off-by: Balint Zsilavecz --- prometheus/metric_test.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/prometheus/metric_test.go b/prometheus/metric_test.go index e152d6211..dd7d84301 100644 --- a/prometheus/metric_test.go +++ b/prometheus/metric_test.go @@ -81,11 +81,11 @@ func TestWithExemplarsMetric(t *testing.T) { infBucket := metric.GetHistogram().Bucket[len(metric.GetHistogram().Bucket)-1] - if want, got := infBucket.GetUpperBound(), math.Inf(1); want != got { + if want, got := math.Inf(1), infBucket.GetUpperBound(); want != got { t.Errorf("want %v, got %v", want, got) } - if want, got := infBucket.GetCumulativeCount(), uint64(4711); want != got { + if want, got := uint64(4711), infBucket.GetCumulativeCount(); want != got { t.Errorf("want %v, got %v", want, got) } })