From c84cf3622f750596077de02be0a9c9283c7e5519 Mon Sep 17 00:00:00 2001 From: beorn7 Date: Thu, 4 Sep 2025 00:26:31 +0200 Subject: [PATCH 1/2] promql: Add a two-legged benchmark for HistogramStatsIterator Signed-off-by: beorn7 --- promql/bench_test.go | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/promql/bench_test.go b/promql/bench_test.go index d425565788..92831de346 100644 --- a/promql/bench_test.go +++ b/promql/bench_test.go @@ -350,6 +350,14 @@ func BenchmarkNativeHistograms(b *testing.B) { name: "histogram_count with long rate interval", query: "histogram_count(sum(rate(native_histogram_series[20m])))", }, + { + name: "two-legged histogram_count/sum with short rate interval", + query: "histogram_count(sum(rate(native_histogram_series[2m]))) + histogram_sum(sum(rate(native_histogram_series[2m])))", + }, + { + name: "two-legged histogram_count/sum with long rate interval", + query: "histogram_count(sum(rate(native_histogram_series[20m]))) + histogram_sum(sum(rate(native_histogram_series[20m])))", + }, } opts := promql.EngineOpts{ From 0fa70e0f6c129e121669ee9823f22d4e4993966d Mon Sep 17 00:00:00 2001 From: beorn7 Date: Wed, 3 Sep 2025 19:21:21 +0200 Subject: [PATCH 2/2] promql: Use `HistogramStatsIterator` more often The current code stops the walk after we have found the first relevant function. However, in expressions with multiple legs, we will then use the `HistogramStatsIterator` at most once. This change should make sure we explore all legs. The added tests make sure we are not using `HistogramStatsIterator` where we shouldn't (but the opposite can only be seen in a benchmark or with a more explicit test). Signed-off-by: beorn7 --- promql/engine.go | 2 +- .../promqltest/testdata/native_histograms.test | 17 +++++++++++++++++ 2 files changed, 18 insertions(+), 1 deletion(-) diff --git a/promql/engine.go b/promql/engine.go index 91257eae37..edafe4fd43 100644 --- a/promql/engine.go +++ b/promql/engine.go @@ -3920,7 +3920,7 @@ func detectHistogramStatsDecoding(expr parser.Expr) { break pathLoop } } - return errors.New("stop") + return nil }) } diff --git a/promql/promqltest/testdata/native_histograms.test b/promql/promqltest/testdata/native_histograms.test index 83933e1e7f..f0c510bd50 100644 --- a/promql/promqltest/testdata/native_histograms.test +++ b/promql/promqltest/testdata/native_histograms.test @@ -1660,3 +1660,20 @@ eval instant at 30s histogram_count(avg(reset)) eval instant at 5m histogram_count(rate(reset{timing="late"}[5m])) expect no_warn {timing="late"} 0.0175 + +clear + +# Test edge cases of HistogramStatsIterator detection. +# We access the same series multiple times within the same expression, +# once with and once without HistogramStatsIterator. The results here +# at least prove that we do not use HistogramStatsIterator where we +# should not. +load 1m + histogram {{schema:0 count:10 sum:50 counter_reset_hint:gauge buckets:[1 2 3 4]}} + +eval instant at 1m histogram_count(histogram unless histogram_quantile(0.5, histogram) < 3) + {} 10 + +eval instant at 1m histogram_quantile(0.5, histogram unless histogram_count(histogram) == 0) + {} 3.1748021039363987 +