diff --git a/promql/functions.go b/promql/functions.go index aad02370f8..99b35c5f71 100644 --- a/promql/functions.go +++ b/promql/functions.go @@ -1666,13 +1666,13 @@ func funcHistogramQuantile(vectorVals []Vector, _ Matrix, args parser.Expression // Deal with classic histograms that have already been filtered for conflicting native histograms. for _, mb := range enh.signatureToMetricWithBuckets { if len(mb.buckets) > 0 { - res, forcedMonotonicity, _ := BucketQuantile(q, mb.buckets) + res, forcedMonotonicMinBucket, forcedMonotonicMaxBucket, forcedMonotonicMaxDiff, forcedMonotonicity, _ := BucketQuantile(q, mb.buckets) if forcedMonotonicity { + metricName := "" if enh.enableDelayedNameRemoval { - annos.Add(annotations.NewHistogramQuantileForcedMonotonicityInfo(getMetricName(mb.metric), args[1].PositionRange())) - } else { - annos.Add(annotations.NewHistogramQuantileForcedMonotonicityInfo("", args[1].PositionRange())) + metricName = getMetricName(mb.metric) } + annos.Add(annotations.NewHistogramQuantileForcedMonotonicityInfo(metricName, args[1].PositionRange(), enh.Ts, forcedMonotonicMinBucket, forcedMonotonicMaxBucket, forcedMonotonicMaxDiff)) } if !enh.enableDelayedNameRemoval { diff --git a/promql/quantile.go b/promql/quantile.go index c44eb89e68..708f943882 100644 --- a/promql/quantile.go +++ b/promql/quantile.go @@ -94,10 +94,7 @@ type metricWithBuckets struct { // // If q>1, +Inf is returned. // -// We also return a bool to indicate if monotonicity needed to be forced, -// and another bool to indicate if small differences between buckets (that -// are likely artifacts of floating point precision issues) have been -// ignored. +// We also return extra info, see doc for ensureMonotonicAndIgnoreSmallDeltas. // // Generically speaking, BucketQuantile is for calculating the // histogram_quantile() of classic histograms. See also: HistogramQuantile @@ -105,15 +102,18 @@ type metricWithBuckets struct { // // BucketQuantile is exported as a useful quantile function over a set of // given buckets. It may be used by other PromQL engine implementations. -func BucketQuantile(q float64, buckets Buckets) (float64, bool, bool) { +func BucketQuantile(q float64, buckets Buckets) (res, forcedMonotonicMinBucket, forcedMonotonicMaxBucket, forcedMonotonicMaxDiff float64, forcedMonotonic, fixedPrecision bool) { if math.IsNaN(q) { - return math.NaN(), false, false + res = math.NaN() + return } if q < 0 { - return math.Inf(-1), false, false + res = math.Inf(-1) + return } if q > 1 { - return math.Inf(+1), false, false + res = math.Inf(+1) + return } slices.SortFunc(buckets, func(a, b Bucket) int { // We don't expect the bucket boundary to be a NaN. @@ -126,39 +126,44 @@ func BucketQuantile(q float64, buckets Buckets) (float64, bool, bool) { return 0 }) if !math.IsInf(buckets[len(buckets)-1].UpperBound, +1) { - return math.NaN(), false, false + res = math.NaN() + return } buckets = coalesceBuckets(buckets) - forcedMonotonic, fixedPrecision := ensureMonotonicAndIgnoreSmallDeltas(buckets, smallDeltaTolerance) + forcedMonotonicMinBucket, forcedMonotonicMaxBucket, forcedMonotonicMaxDiff, forcedMonotonic, fixedPrecision = ensureMonotonicAndIgnoreSmallDeltas(buckets, smallDeltaTolerance) if len(buckets) < 2 { - return math.NaN(), false, false + res = math.NaN() + return } observations := buckets[len(buckets)-1].Count if observations == 0 { - return math.NaN(), false, false + res = math.NaN() + return } rank := q * observations b := sort.Search(len(buckets)-1, func(i int) bool { return buckets[i].Count >= rank }) - if b == len(buckets)-1 { - return buckets[len(buckets)-2].UpperBound, forcedMonotonic, fixedPrecision + switch { + case b == len(buckets)-1: + res = buckets[len(buckets)-2].UpperBound + case b == 0 && buckets[0].UpperBound <= 0: + res = buckets[0].UpperBound + default: + var ( + bucketStart float64 + bucketEnd = buckets[b].UpperBound + count = buckets[b].Count + ) + if b > 0 { + bucketStart = buckets[b-1].UpperBound + count -= buckets[b-1].Count + rank -= buckets[b-1].Count + } + res = bucketStart + (bucketEnd-bucketStart)*(rank/count) } - if b == 0 && buckets[0].UpperBound <= 0 { - return buckets[0].UpperBound, forcedMonotonic, fixedPrecision - } - var ( - bucketStart float64 - bucketEnd = buckets[b].UpperBound - count = buckets[b].Count - ) - if b > 0 { - bucketStart = buckets[b-1].UpperBound - count -= buckets[b-1].Count - rank -= buckets[b-1].Count - } - return bucketStart + (bucketEnd-bucketStart)*(rank/count), forcedMonotonic, fixedPrecision + return } // HistogramQuantile calculates the quantile 'q' based on the given histogram. @@ -655,10 +660,17 @@ func coalesceBuckets(buckets Buckets) Buckets { // the histogram buckets, essentially removing any decreases in the count // between successive buckets. // -// We return a bool to indicate if this monotonicity was forced or not, and -// another bool to indicate if small deltas were ignored or not. -func ensureMonotonicAndIgnoreSmallDeltas(buckets Buckets, tolerance float64) (bool, bool) { - var forcedMonotonic, fixedPrecision bool +// We return: +// - a float to indicate the minimum bucket upper bound where monotonicity was forced, if applicable +// - a float to indicate the maximum bucket upper bound where monotonicity was forced, if applicable +// - a float to indicate the maximum difference between the count of two consecutive buckets +// where monotonicity was forced, if applicable +// - a bool to indicate if monotonicity needed to be forced +// - a bool to indicate if small differences between buckets (that are likely +// artifacts of floating point precision issues) have been ignored. +func ensureMonotonicAndIgnoreSmallDeltas(buckets Buckets, tolerance float64) (forcedMonotonicMinBucket, forcedMonotonicMaxBucket, forcedMonotonicMaxDiff float64, forcedMonotonic, fixedPrecision bool) { + forcedMonotonicMinBucket = math.Inf(+1) + forcedMonotonicMaxBucket = math.Inf(-1) prev := buckets[0].Count for i := 1; i < len(buckets); i++ { curr := buckets[i].Count // Assumed always positive. @@ -679,11 +691,20 @@ func ensureMonotonicAndIgnoreSmallDeltas(buckets Buckets, tolerance float64) (bo // Do not update the 'prev' value as we are ignoring the decrease. buckets[i].Count = prev forcedMonotonic = true + if buckets[i].UpperBound < forcedMonotonicMinBucket { + forcedMonotonicMinBucket = buckets[i].UpperBound + } + if buckets[i].UpperBound > forcedMonotonicMaxBucket { + forcedMonotonicMaxBucket = buckets[i].UpperBound + } + if diff := prev - curr; diff > forcedMonotonicMaxDiff { + forcedMonotonicMaxDiff = diff + } continue } prev = curr } - return forcedMonotonic, fixedPrecision + return } // quantile calculates the given quantile of a vector of samples. diff --git a/promql/quantile_test.go b/promql/quantile_test.go index c97ff7c3c4..45dd2504ce 100644 --- a/promql/quantile_test.go +++ b/promql/quantile_test.go @@ -308,7 +308,7 @@ func TestBucketQuantile_ForcedMonotonicity(t *testing.T) { } { t.Run(name, func(t *testing.T) { for q, v := range tc.expectedValues { - res, forced, fixed := BucketQuantile(q, tc.getInput()) + res, _, _, _, forced, fixed := BucketQuantile(q, tc.getInput()) require.Equal(t, tc.expectedForced, forced) require.Equal(t, tc.expectedFixed, fixed) require.InEpsilon(t, v, res, eps) diff --git a/util/annotations/annotations.go b/util/annotations/annotations.go index 581e4987d1..416ad8d282 100644 --- a/util/annotations/annotations.go +++ b/util/annotations/annotations.go @@ -16,6 +16,7 @@ package annotations import ( "errors" "fmt" + "time" "github.com/prometheus/common/model" @@ -319,12 +320,64 @@ func NewPossibleNonCounterLabelInfo(metricName, typeLabel string, pos posrange.P } } +type histogramQuantileForcedMonotonicityErr struct { + PositionRange posrange.PositionRange + Err error + Query string + Min []float64 + Max []float64 + Count int +} + +func (e *histogramQuantileForcedMonotonicityErr) Error() string { + if e.Query == "" { + return e.Err.Error() + } + startTime := time.Unix(int64(e.Min[0]/1000), 0).UTC().Format(time.RFC3339) + endTime := time.Unix(int64(e.Max[0]/1000), 0).UTC().Format(time.RFC3339) + return fmt.Sprintf("%s, from buckets %g to %g, with a max diff of %.2g, over %d samples from %s to %s (%s)", e.Err, e.Min[1], e.Max[1], e.Max[2], e.Count+1, startTime, endTime, e.PositionRange.StartPosInput(e.Query, 0)) +} + +func (e *histogramQuantileForcedMonotonicityErr) Unwrap() error { + return e.Err +} + +func (e *histogramQuantileForcedMonotonicityErr) SetQuery(query string) { + e.Query = query +} + +func (e *histogramQuantileForcedMonotonicityErr) Merge(other error) error { + o := &histogramQuantileForcedMonotonicityErr{} + ok := errors.As(other, &o) + if !ok { + return e + } + if e.Err.Error() != o.Err.Error() || len(e.Min) != len(o.Min) || len(e.Max) != len(o.Max) { + return e + } + for i, aMin := range e.Min { + if aMin < o.Min[i] { + o.Min[i] = aMin + } + } + for i, aMax := range e.Max { + if aMax > o.Max[i] { + o.Max[i] = aMax + } + } + o.Count += e.Count + 1 + return o +} + // NewHistogramQuantileForcedMonotonicityInfo is used when the input (classic histograms) to // histogram_quantile needs to be forced to be monotonic. -func NewHistogramQuantileForcedMonotonicityInfo(metricName string, pos posrange.PositionRange) error { - return &annoErr{ +func NewHistogramQuantileForcedMonotonicityInfo(metricName string, pos posrange.PositionRange, ts int64, forcedMonotonicMinBucket, forcedMonotonicMaxBucket, forcedMonotonicMaxDiff float64) error { + floatTs := float64(ts) + return &histogramQuantileForcedMonotonicityErr{ PositionRange: pos, Err: maybeAddMetricName(HistogramQuantileForcedMonotonicityInfo, metricName), + Min: []float64{floatTs, forcedMonotonicMinBucket}, + Max: []float64{floatTs, forcedMonotonicMaxBucket, forcedMonotonicMaxDiff}, } } diff --git a/util/annotations/annotations_test.go b/util/annotations/annotations_test.go index e3caaae7eb..39fb8e62f4 100644 --- a/util/annotations/annotations_test.go +++ b/util/annotations/annotations_test.go @@ -39,6 +39,10 @@ func TestAnnotations_AsStrings(t *testing.T) { annos.Add(NewHistogramIgnoredInAggregationInfo("sum", pos)) + annos.Add(NewHistogramQuantileForcedMonotonicityInfo("series_1", pos, 1735084800000, 5, 50, 5.5)) + annos.Add(NewHistogramQuantileForcedMonotonicityInfo("series_1", pos, 1703462400000, 10, 100, 10)) + annos.Add(NewHistogramQuantileForcedMonotonicityInfo("series_1", pos, 1733011200000, 2.5, 75, 7.5)) + warnings, infos := annos.AsStrings("lorem ipsum dolor sit amet", 0, 0) require.ElementsMatch(t, warnings, []string{ "this is a non-annotation error", @@ -48,6 +52,7 @@ func TestAnnotations_AsStrings(t *testing.T) { }) require.ElementsMatch(t, infos, []string{ "PromQL info: ignored histogram in sum aggregation (1:4)", + `PromQL info: input to histogram_quantile needed to be fixed for monotonicity (see https://prometheus.io/docs/prometheus/latest/querying/functions/#histogram_quantile) for metric name "series_1", from buckets 2.5 to 100, with a max diff of 10, over 3 samples from 2023-12-25T00:00:00Z to 2024-12-25T00:00:00Z (1:4)`, }) }