fix(promql): histogram_fraction first bucket heuristic consistent with histogram_quantile#18474
Draft
fix(promql): histogram_fraction first bucket heuristic consistent with histogram_quantile#18474
Conversation
…h histogram_quantile For classic histograms and NHCB (custom buckets) where the first bucket has a non-positive upper boundary (-Inf, x] with x<=0, both BucketQuantile and HistogramQuantile place all observations at the upper boundary x. BucketFraction and HistogramFraction were inconsistent: they treated observations as being at -Inf (contributing to every fraction query whose lower bound fell inside the bucket). Fix BucketFraction and HistogramFraction to use the same heuristic as their quantile counterparts: observations in the first bucket with a non-positive upper boundary are assumed to be at x (the upper boundary). Observations in the first NHCB bucket with a positive upper boundary retain the existing behaviour of assuming a lower boundary of 0. Add missing test cases for histogram_fraction on NHCB histograms with non-positive first bucket boundaries, and add cross-checks between histogram_quantile and histogram_fraction for exponential native histograms. Coded with Claude Sonnet 4.6. Signed-off-by: György Krajcsovits <[email protected]>
Member
Author
|
First version made with AI. I'm refining it and discussing the merits with Beorn. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
For classic histograms and NHCB (custom buckets) where the first bucket has a non-positive upper boundary
(-Inf, x]withx<=0, bothBucketQuantileandHistogramQuantileplace all observations at the upper boundaryx.BucketFractionandHistogramFractionwere inconsistent: they treated observations as being at-Inf(contributing to every fraction query whose lower bound fell inside the bucket).The upshot is that if there's a bucket
(-Inf, x]wherex<=0, for example(-Inf, -3]and there are 10 observations in it out of 100 observations total, then the 10% quantile is -3, since quantile assumes that observations are at the upper boundary. However without the fix the histogram_fraction treats them as being at -Inf, meaning that the fraction from for example -4 to -3 is zero. Which is a contradiction with quantile at -3.BucketFraction(classic histograms) to matchBucketQuantile: observations in the first bucket withupperBound <= 0are assumed to be at the upper boundary.HistogramFraction(native histograms) to matchHistogramQuantile: same heuristic for the first NHCB bucket withb.Upper <= 0.histogram_fractionon NHCB histograms with non-positive first bucket upper boundaries.histogram_quantileandhistogram_fractionfor exponential native histograms.Test plan
go test ./promql/...passeshistograms.testcover classic histogram first bucket with non-positive upper boundarynative_histograms.testcover NHCB first bucket with non-positive upper boundarysingle_histogram,negative_histogram)Coded with Claude Sonnet 4.6.
Signed-off-by: György Krajcsovits [email protected]