Fix CumulativeCount value of +Inf bucket created from exemplar (#1148)

balintzs · bwplotka · bwplotka · commit 1f93f6458077 · 2022-11-01T16:22:22.000Z
* Fix `CumulativeCount` value of `+Inf` bucket created from exemplar

Signed-off-by: Balint Zsilavecz &lt;balint.zsilavecz@skyscanner.net&gt;

* Update prometheus/metric_test.go

Co-authored-by: Bartlomiej Plotka &lt;bwplotka@gmail.com&gt;
Signed-off-by: Balint Zsilavecz &lt;balint.zsilavecz@skyscanner.net&gt;

* Clarify description of implicit `+Inf` bucket count

Signed-off-by: Balint Zsilavecz &lt;balint.zsilavecz@skyscanner.net&gt;

* Fix test variables

Signed-off-by: Balint Zsilavecz &lt;balint.zsilavecz@skyscanner.net&gt;

Signed-off-by: Balint Zsilavecz &lt;balint.zsilavecz@skyscanner.net&gt;
Co-authored-by: Bartlomiej Plotka &lt;bwplotka@gmail.com&gt;
diff --git 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.
diff --git 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
@@ -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 want, got := math.Inf(1), infBucket.GetUpperBound(); want != got {
+			t.Errorf("want %v, got %v", want, got)
+		}
+
+		if want, got := uint64(4711), infBucket.GetCumulativeCount(); want != got {
+			t.Errorf("want %v, got %v", want, got)
 		}
 	})
 }

Original file line number	Diff line number	Diff line change
`@@ -187,7 +187,7 @@ func (m withExemplarsMetric) Write(pb dto.Metric) error {`
`187`	`187`	`} else {`
`188`	`188`	`// 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.`
`189`	`189`	`b := &dto.Bucket{`
`190`		`- CumulativeCount: proto.Uint64(pb.Histogram.Bucket[len(pb.Histogram.GetBucket())-1].GetCumulativeCount()),`
	`190`	`+ CumulativeCount: proto.Uint64(pb.Histogram.GetSampleCount()),`
`191`	`191`	`UpperBound: proto.Float64(math.Inf(1)),`
`192`	`192`	`Exemplar: e,`
`193`	`193`	`}`
Original file line number	Diff line number	Diff line change
`@@ -79,10 +79,14 @@ func TestWithExemplarsMetric(t *testing.T) {`
`79`	`79`	`}`
`80`	`80`	`}`
`81`	`81`
`82`		`- infBucket := metric.GetHistogram().Bucket[len(metric.GetHistogram().Bucket)-1].GetUpperBound()`
	`82`	`+ infBucket := metric.GetHistogram().Bucket[len(metric.GetHistogram().Bucket)-1]`
`83`	`83`
`84`		`- if infBucket != math.Inf(1) {`
`85`		`- t.Errorf("want %v, got %v", math.Inf(1), infBucket)`
	`84`	`+ if want, got := math.Inf(1), infBucket.GetUpperBound(); want != got {`
	`85`	`+ t.Errorf("want %v, got %v", want, got)`
	`86`	`+ }`
	`87`	`+`
	`88`	`+ if want, got := uint64(4711), infBucket.GetCumulativeCount(); want != got {`
	`89`	`+ t.Errorf("want %v, got %v", want, got)`
`86`	`90`	`}`
`87`	`91`	`})`
`88`	`92`	`}`