use custom error for histogramQuantileForcedMonotonicityErr

Signed-off-by: Jeanette Tan <jeanette.tan@grafana.com>

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 {

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.

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)

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},
 	}
 }
 

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)`,
 	})
 }