From 30948120dc46caf6d953ddc14c042bcca42499d3 Mon Sep 17 00:00:00 2001 From: beorn7 Date: Thu, 25 May 2023 13:58:22 +0200 Subject: [PATCH 1/2] histogram: expose bug in bucket key calculation The current code doesn't work fork negative schemas if the observed value should go into a bucket with a non-positive key. Signed-off-by: beorn7 --- prometheus/histogram_test.go | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/prometheus/histogram_test.go b/prometheus/histogram_test.go index 079866b..ddbe575 100644 --- a/prometheus/histogram_test.go +++ b/prometheus/histogram_test.go @@ -500,23 +500,26 @@ func TestNativeHistogram(t *testing.T) { { name: "factor 4 results in schema -1", observations: []float64{ + 0.0156251, 0.0625, // Bucket -2: (0.015625, 0.0625) + 0.1, 0.25, // Bucket -1: (0.0625, 0.25] 0.5, 1, // Bucket 0: (0.25, 1] 1.5, 2, 3, 3.5, // Bucket 1: (1, 4] 5, 6, 7, // Bucket 2: (4, 16] 33.33, // Bucket 3: (16, 64] }, factor: 4, - want: `sample_count:10 sample_sum:62.83 schema:-1 zero_threshold:2.938735877055719e-39 zero_count:0 positive_span: positive_delta:2 positive_delta:2 positive_delta:-1 positive_delta:-2 `, + want: `sample_count:14 sample_sum:63.2581251 schema:-1 zero_threshold:2.938735877055719e-39 zero_count:0 positive_span: positive_delta:2 positive_delta:0 positive_delta:0 positive_delta:2 positive_delta:-1 positive_delta:-2 `, }, { name: "factor 17 results in schema -2", observations: []float64{ + 0.0156251, 0.0625, 0.1, 0.25, // Bucket -1: (0.015625, 0.25] 0.5, 1, // Bucket 0: (0.0625, 1] 1.5, 2, 3, 3.5, 5, 6, 7, // Bucket 1: (1, 16] 33.33, // Bucket 2: (16, 256] }, factor: 17, - want: `sample_count:10 sample_sum:62.83 schema:-2 zero_threshold:2.938735877055719e-39 zero_count:0 positive_span: positive_delta:2 positive_delta:5 positive_delta:-6 `, + want: `sample_count:14 sample_sum:63.2581251 schema:-2 zero_threshold:2.938735877055719e-39 zero_count:0 positive_span: positive_delta:4 positive_delta:-2 positive_delta:5 positive_delta:-6 `, }, { name: "negative buckets", From 77e97da564bd65455499c35b20e23dfa144d6925 Mon Sep 17 00:00:00 2001 From: beorn7 Date: Thu, 25 May 2023 19:03:43 +0200 Subject: [PATCH 2/2] histogram: Fix bug in bucket key calculation The current code doesn't work fork negative schemas if the observed value should go into a bucket with a non-positive key. Signed-off-by: beorn7 --- prometheus/histogram.go | 4 ++-- prometheus/histogram_test.go | 6 +++--- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/prometheus/histogram.go b/prometheus/histogram.go index 5b69965..8a41226 100644 --- a/prometheus/histogram.go +++ b/prometheus/histogram.go @@ -639,8 +639,8 @@ func (hc *histogramCounts) observe(v float64, bucket int, doSparse bool) { if frac == 0.5 { key-- } - div := 1 << -schema - key = (key + div - 1) / div + offset := (1 << -schema) - 1 + key = (key + offset) >> -schema } if isInf { key++ diff --git a/prometheus/histogram_test.go b/prometheus/histogram_test.go index ddbe575..6357c23 100644 --- a/prometheus/histogram_test.go +++ b/prometheus/histogram_test.go @@ -513,13 +513,13 @@ func TestNativeHistogram(t *testing.T) { { name: "factor 17 results in schema -2", observations: []float64{ - 0.0156251, 0.0625, 0.1, 0.25, // Bucket -1: (0.015625, 0.25] - 0.5, 1, // Bucket 0: (0.0625, 1] + 0.0156251, 0.0625, // Bucket -1: (0.015625, 0.0625] + 0.1, 0.25, 0.5, 1, // Bucket 0: (0.0625, 1] 1.5, 2, 3, 3.5, 5, 6, 7, // Bucket 1: (1, 16] 33.33, // Bucket 2: (16, 256] }, factor: 17, - want: `sample_count:14 sample_sum:63.2581251 schema:-2 zero_threshold:2.938735877055719e-39 zero_count:0 positive_span: positive_delta:4 positive_delta:-2 positive_delta:5 positive_delta:-6 `, + want: `sample_count:14 sample_sum:63.2581251 schema:-2 zero_threshold:2.938735877055719e-39 zero_count:0 positive_span: positive_delta:2 positive_delta:2 positive_delta:3 positive_delta:-6 `, }, { name: "negative buckets",