Fix histogram_quantile annotation in range query when delayed name removal is disabled (#16794)

Signed-off-by: Neeraj Gartia <neerajgartia211002@gmail.com>

promql/engine.go

@@ -1190,7 +1190,11 @@ func (enh *EvalNodeHelper) resetHistograms(inVec Vector, arg parser.Expr) annota
 			sample.Metric.Get(model.BucketLabel), 64,
 		)
 		if err != nil {
-			annos.Add(annotations.NewBadBucketLabelWarning(sample.Metric.Get(labels.MetricName), sample.Metric.Get(model.BucketLabel), arg.PositionRange()))
+			if enh.enableDelayedNameRemoval {
+				annos.Add(annotations.NewBadBucketLabelWarning(sample.Metric.Get(labels.MetricName), sample.Metric.Get(model.BucketLabel), arg.PositionRange()))
+			} else {
+				annos.Add(annotations.NewBadBucketLabelWarning("", sample.Metric.Get(model.BucketLabel), arg.PositionRange()))
+			}
 			continue
 		}
 		enh.lblBuf = sample.Metric.BytesWithoutLabels(enh.lblBuf, labels.BucketLabel)
@@ -1213,7 +1217,11 @@ func (enh *EvalNodeHelper) resetHistograms(inVec Vector, arg parser.Expr) annota
 			// At this data point, we have classic histogram
 			// buckets and a native histogram with the same name and
 			// labels. Do not evaluate anything.
-			annos.Add(annotations.NewMixedClassicNativeHistogramsWarning(sample.Metric.Get(labels.MetricName), arg.PositionRange()))
+			if enh.enableDelayedNameRemoval {
+				annos.Add(annotations.NewMixedClassicNativeHistogramsWarning(sample.Metric.Get(labels.MetricName), arg.PositionRange()))
+			} else {
+				annos.Add(annotations.NewMixedClassicNativeHistogramsWarning("", arg.PositionRange()))
+			}
 			delete(enh.signatureToMetricWithBuckets, string(enh.lblBuf))
 			enh.nativeHistogramSamples[idx].H = nil
 			continue

promql/engine_test.go

@@ -3662,81 +3662,6 @@ func TestRateAnnotations(t *testing.T) {
 	}
 }
 
-func TestHistogramQuantileAnnotations(t *testing.T) {
-	testCases := map[string]struct {
-		data                       string
-		expr                       string
-		expectedWarningAnnotations []string
-		expectedInfoAnnotations    []string
-	}{
-		"info annotation for nonmonotonic buckets": {
-			data: `
-				nonmonotonic_bucket{le="0.1"}   0+2x10
-				nonmonotonic_bucket{le="1"}     0+1x10
-				nonmonotonic_bucket{le="10"}    0+5x10
-				nonmonotonic_bucket{le="100"}   0+4x10
-				nonmonotonic_bucket{le="1000"}  0+9x10
-				nonmonotonic_bucket{le="+Inf"}  0+8x10
-			`,
-			expr:                       "histogram_quantile(0.5, nonmonotonic_bucket)",
-			expectedWarningAnnotations: []string{},
-			expectedInfoAnnotations: []string{
-				`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 "nonmonotonic_bucket" (1:25)`,
-			},
-		},
-		"warning annotation for missing le label": {
-			data: `
-				myHistogram{abe="0.1"}   0+2x10
-			`,
-			expr: "histogram_quantile(0.5, myHistogram)",
-			expectedWarningAnnotations: []string{
-				`PromQL warning: bucket label "le" is missing or has a malformed value of "" for metric name "myHistogram" (1:25)`,
-			},
-			expectedInfoAnnotations: []string{},
-		},
-		"warning annotation for malformed le label": {
-			data: `
-				myHistogram{le="Hello World"}   0+2x10
-			`,
-			expr: "histogram_quantile(0.5, myHistogram)",
-			expectedWarningAnnotations: []string{
-				`PromQL warning: bucket label "le" is missing or has a malformed value of "Hello World" for metric name "myHistogram" (1:25)`,
-			},
-			expectedInfoAnnotations: []string{},
-		},
-		"warning annotation for mixed histograms": {
-			data: `
-				mixedHistogram{le="0.1"}   0+2x10
-				mixedHistogram{le="1"}     0+3x10
-				mixedHistogram{}           {{schema:0 count:10 sum:50 buckets:[1 2 3]}}
-			`,
-			expr: "histogram_quantile(0.5, mixedHistogram)",
-			expectedWarningAnnotations: []string{
-				`PromQL warning: vector contains a mix of classic and native histograms for metric name "mixedHistogram" (1:25)`,
-			},
-			expectedInfoAnnotations: []string{},
-		},
-	}
-	for name, testCase := range testCases {
-		t.Run(name, func(t *testing.T) {
-			store := promqltest.LoadedStorage(t, "load 1m\n"+strings.TrimSpace(testCase.data))
-			t.Cleanup(func() { _ = store.Close() })
-
-			engine := newTestEngine(t)
-			query, err := engine.NewInstantQuery(context.Background(), store, nil, testCase.expr, timestamp.Time(0).Add(1*time.Minute))
-			require.NoError(t, err)
-			t.Cleanup(query.Close)
-
-			res := query.Exec(context.Background())
-			require.NoError(t, res.Err)
-
-			warnings, infos := res.Warnings.AsStrings(testCase.expr, 0, 0)
-			testutil.RequireEqual(t, testCase.expectedWarningAnnotations, warnings)
-			testutil.RequireEqual(t, testCase.expectedInfoAnnotations, infos)
-		})
-	}
-}
-
 func TestHistogramRateWithFloatStaleness(t *testing.T) {
 	// Make a chunk with two normal histograms of the same value.
 	h1 := histogram.Histogram{
@@ -3908,6 +3833,48 @@ eval instant at 10m last_over_time(metric_total{env="1"}[10m])
 # Drops name for other _over_time functions
 eval instant at 10m max_over_time(metric_total{env="1"}[10m])
 	{env="1"} 120
+
+clear
+
+load 1m
+	float{a="b"}									2x10
+	classic_histogram_invalid{abe="0.1"}			2x10
+	native_histogram{a="b", c="d"}					{{sum:0 count:0}}x10
+    histogram_nan{case="100% NaNs"} 				{{schema:0 count:0 sum:0}} {{schema:0 count:3 sum:NaN}}
+    histogram_nan{case="20% NaNs"} 					{{schema:0 count:0 sum:0}} {{schema:0 count:15 sum:NaN buckets:[12]}}
+	conflicting_histogram{le="0.1"}					2x10
+	conflicting_histogram							{{sum:0}}x10
+	nonmonotonic_bucket{le="0.1"}   				0+2x10
+	nonmonotonic_bucket{le="1"}     				0+1x10
+	nonmonotonic_bucket{le="10"}    				0+5x10
+	nonmonotonic_bucket{le="100"}   				0+4x10
+	nonmonotonic_bucket{le="1000"}  				0+9x10
+	nonmonotonic_bucket{le="+Inf"}  				0+8x10
+
+eval instant at 1m histogram_quantile(0.5, classic_histogram_invalid)
+	expect warn msg: PromQL warning: bucket label "le" is missing or has a malformed value of ""
+
+eval instant at 1m histogram_fraction(0.5, 1, conflicting_histogram)
+	expect warn msg: PromQL warning: vector contains a mix of classic and native histograms
+
+eval instant at 1m histogram_quantile(0.5, nonmonotonic_bucket)
+	expect info msg: PromQL info: input to histogram_quantile needed to be fixed for monotonicity (see https://prometheus.io/docs/prometheus/latest/querying/functions/#histogram_quantile)
+	{} 8.5
+
+eval instant at 1m histogram_quantile(1, histogram_nan)
+    expect info msg: PromQL info: input to histogram_quantile has NaN observations, result is NaN
+    {case="100% NaNs"} NaN
+    {case="20% NaNs"} NaN
+
+eval instant at 1m histogram_quantile(0.8, histogram_nan{case="20% NaNs"})
+    expect info msg: PromQL info: input to histogram_quantile has NaN observations, result is skewed higher
+    {case="20% NaNs"} 1
+
+eval instant at 1m histogram_fraction(-Inf, 0.7071067811865475, histogram_nan)
+    expect info msg: PromQL info: input to histogram_fraction has NaN observations, which are excluded from all fractions
+    {case="100% NaNs"} 0.0
+    {case="20% NaNs"} 0.4
+
 `, engine)
 }
 

promql/functions.go

@@ -1462,7 +1462,11 @@ func funcHistogramQuantile(vectorVals []Vector, _ Matrix, args parser.Expression
 		if len(mb.buckets) > 0 {
 			res, forcedMonotonicity, _ := BucketQuantile(q, mb.buckets)
 			if forcedMonotonicity {
-				annos.Add(annotations.NewHistogramQuantileForcedMonotonicityInfo(mb.metric.Get(labels.MetricName), args[1].PositionRange()))
+				if enh.enableDelayedNameRemoval {
+					annos.Add(annotations.NewHistogramQuantileForcedMonotonicityInfo(mb.metric.Get(labels.MetricName), args[1].PositionRange()))
+				} else {
+					annos.Add(annotations.NewHistogramQuantileForcedMonotonicityInfo("", args[1].PositionRange()))
+				}
 			}
 
 			if !enh.enableDelayedNameRemoval {

promql/promqltest/testdata/native_histograms.test

@@ -1494,7 +1494,7 @@ eval instant at 55m increase(metric[90m])
 eval instant at 54m30s increase(metric[90m])
     {type="histogram"} {{count:2512.9166666666665 sum:50.76599326599326 counter_reset_hint:gauge buckets:[10.153198653198652 2502.7634680134674]}}
     {type="counter"} 2512.9166666666665
-    
+
 # End of range coincides with sample. Zero point of count is reached outside of (i.e. before) the range.
 # This means no change of extrapolation is required for the histogram count (and neither for the float counter),
 # however, the 2nd bucket's extrapolation will reach zero within the range. The overestimation is visible
@@ -1552,3 +1552,32 @@ eval instant at 5m -metric
   expect no_info
   {id="1"} {{count:-4 sum:-4 counter_reset_hint:gauge buckets:[-1 -2 -1]}}
   {id="2"} {{count:-4 sum:-4 counter_reset_hint:gauge buckets:[-1 -2 -1]}}
+
+clear
+
+# Test histogram_quantile annotations.
+load 1m
+	nonmonotonic_bucket{le="0.1"}   0+2x10
+	nonmonotonic_bucket{le="1"}     0+1x10
+	nonmonotonic_bucket{le="10"}    0+5x10
+	nonmonotonic_bucket{le="100"}   0+4x10
+	nonmonotonic_bucket{le="1000"}  0+9x10
+	nonmonotonic_bucket{le="+Inf"}  0+8x10
+	myHistogram1{abe="0.1"}   0+2x10
+	myHistogram2{le="Hello World"}   0+2x10
+	mixedHistogram{le="0.1"}   0+2x10
+	mixedHistogram{le="1"}     0+3x10
+	mixedHistogram{}           {{schema:0 count:10 sum:50 buckets:[1 2 3]}}
+
+eval instant at 1m histogram_quantile(0.5, nonmonotonic_bucket)
+	expect info msg: 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 "nonmonotonic_bucket"
+	{} 8.5
+
+eval instant at 1m histogram_quantile(0.5, myHistogram1)
+	expect warn msg: PromQL warning: bucket label "le" is missing or has a malformed value of "" for metric name "myHistogram1"
+
+eval instant at 1m histogram_quantile(0.5, myHistogram2)
+	expect warn msg: PromQL warning: bucket label "le" is missing or has a malformed value of "Hello World" for metric name "myHistogram2"
+
+eval instant at 1m histogram_quantile(0.5, mixedHistogram)
+	expect warn msg: PromQL warning: vector contains a mix of classic and native histograms for metric name "mixedHistogram"

util/annotations/annotations.go

@@ -139,7 +139,7 @@ var (
 	InvalidQuantileWarning                     = fmt.Errorf("%w: quantile value should be between 0 and 1", PromQLWarning)
 	BadBucketLabelWarning                      = fmt.Errorf("%w: bucket label %q is missing or has a malformed value", PromQLWarning, model.BucketLabel)
 	MixedFloatsHistogramsWarning               = fmt.Errorf("%w: encountered a mix of histograms and floats for", PromQLWarning)
-	MixedClassicNativeHistogramsWarning        = fmt.Errorf("%w: vector contains a mix of classic and native histograms for metric name", PromQLWarning)
+	MixedClassicNativeHistogramsWarning        = fmt.Errorf("%w: vector contains a mix of classic and native histograms", PromQLWarning)
 	NativeHistogramNotCounterWarning           = fmt.Errorf("%w: this native histogram metric is not a counter:", PromQLWarning)
 	NativeHistogramNotGaugeWarning             = fmt.Errorf("%w: this native histogram metric is not a gauge:", PromQLWarning)
 	MixedExponentialCustomHistogramsWarning    = fmt.Errorf("%w: vector contains a mix of histograms with exponential and custom buckets schemas for metric name", PromQLWarning)
@@ -148,13 +148,13 @@ var (
 
 	PossibleNonCounterInfo                  = fmt.Errorf("%w: metric might not be a counter, name does not end in _total/_sum/_count/_bucket:", PromQLInfo)
 	PossibleNonCounterLabelInfo             = fmt.Errorf("%w: metric might not be a counter, __type__ label is not set to %q or %q", PromQLInfo, model.MetricTypeCounter, model.MetricTypeHistogram)
-	HistogramQuantileForcedMonotonicityInfo = fmt.Errorf("%w: input to histogram_quantile needed to be fixed for monotonicity (see https://prometheus.io/docs/prometheus/latest/querying/functions/#histogram_quantile) for metric name", PromQLInfo)
+	HistogramQuantileForcedMonotonicityInfo = fmt.Errorf("%w: input to histogram_quantile needed to be fixed for monotonicity (see https://prometheus.io/docs/prometheus/latest/querying/functions/#histogram_quantile)", PromQLInfo)
 	IncompatibleTypesInBinOpInfo            = fmt.Errorf("%w: incompatible sample types encountered for binary operator", PromQLInfo)
 	HistogramIgnoredInAggregationInfo       = fmt.Errorf("%w: ignored histogram in", PromQLInfo)
 	HistogramIgnoredInMixedRangeInfo        = fmt.Errorf("%w: ignored histograms in a range containing both floats and histograms for metric name", PromQLInfo)
-	NativeHistogramQuantileNaNResultInfo    = fmt.Errorf("%w: input to histogram_quantile has NaN observations, result is NaN for metric name", PromQLInfo)
+	NativeHistogramQuantileNaNResultInfo    = fmt.Errorf("%w: input to histogram_quantile has NaN observations, result is NaN", PromQLInfo)
-	NativeHistogramQuantileNaNSkewInfo      = fmt.Errorf("%w: input to histogram_quantile has NaN observations, result is skewed higher for metric name", PromQLInfo)
+	NativeHistogramQuantileNaNSkewInfo      = fmt.Errorf("%w: input to histogram_quantile has NaN observations, result is skewed higher", PromQLInfo)
-	NativeHistogramFractionNaNsInfo         = fmt.Errorf("%w: input to histogram_fraction has NaN observations, which are excluded from all fractions for metric name", PromQLInfo)
+	NativeHistogramFractionNaNsInfo         = fmt.Errorf("%w: input to histogram_fraction has NaN observations, which are excluded from all fractions", PromQLInfo)
 )
 
 type annoErr struct {
@@ -174,6 +174,13 @@ func (e annoErr) Unwrap() error {
 	return e.Err
 }
 
+func maybeAddMetricName(anno error, metricName string) error {
+	if metricName == "" {
+		return anno
+	}
+	return fmt.Errorf("%w for metric name %q", anno, metricName)
+}
+
 // NewInvalidQuantileWarning is used when the user specifies an invalid quantile
 // value, i.e. a float that is outside the range [0, 1] or NaN.
 func NewInvalidQuantileWarning(q float64, pos posrange.PositionRange) error {
@@ -195,9 +202,10 @@ func NewInvalidRatioWarning(q, to float64, pos posrange.PositionRange) error {
 // NewBadBucketLabelWarning is used when there is an error parsing the bucket label
 // of a classic histogram.
 func NewBadBucketLabelWarning(metricName, label string, pos posrange.PositionRange) error {
+	anno := maybeAddMetricName(fmt.Errorf("%w of %q", BadBucketLabelWarning, label), metricName)
 	return annoErr{
 		PositionRange: pos,
-		Err:           fmt.Errorf("%w of %q for metric name %q", BadBucketLabelWarning, label, metricName),
+		Err:           anno,
 	}
 }
 
@@ -225,7 +233,7 @@ func NewMixedFloatsHistogramsAggWarning(pos posrange.PositionRange) error {
 func NewMixedClassicNativeHistogramsWarning(metricName string, pos posrange.PositionRange) error {
 	return annoErr{
 		PositionRange: pos,
-		Err:           fmt.Errorf("%w %q", MixedClassicNativeHistogramsWarning, metricName),
+		Err:           maybeAddMetricName(MixedClassicNativeHistogramsWarning, metricName),
 	}
 }
 
@@ -288,7 +296,7 @@ func NewPossibleNonCounterLabelInfo(metricName, typeLabel string, pos posrange.P
 func NewHistogramQuantileForcedMonotonicityInfo(metricName string, pos posrange.PositionRange) error {
 	return annoErr{
 		PositionRange: pos,
-		Err:           fmt.Errorf("%w %q", HistogramQuantileForcedMonotonicityInfo, metricName),
+		Err:           maybeAddMetricName(HistogramQuantileForcedMonotonicityInfo, metricName),
 	}
 }
 
@@ -331,20 +339,20 @@ func NewIncompatibleBucketLayoutInBinOpWarning(operator string, pos posrange.Pos
 func NewNativeHistogramQuantileNaNResultInfo(metricName string, pos posrange.PositionRange) error {
 	return annoErr{
 		PositionRange: pos,
-		Err:           fmt.Errorf("%w %q", NativeHistogramQuantileNaNResultInfo, metricName),
+		Err:           maybeAddMetricName(NativeHistogramQuantileNaNResultInfo, metricName),
 	}
 }
 
 func NewNativeHistogramQuantileNaNSkewInfo(metricName string, pos posrange.PositionRange) error {
 	return annoErr{
 		PositionRange: pos,
-		Err:           fmt.Errorf("%w %q", NativeHistogramQuantileNaNSkewInfo, metricName),
+		Err:           maybeAddMetricName(NativeHistogramQuantileNaNSkewInfo, metricName),
 	}
 }
 
 func NewNativeHistogramFractionNaNsInfo(metricName string, pos posrange.PositionRange) error {
 	return annoErr{
 		PositionRange: pos,
-		Err:           fmt.Errorf("%w %q", NativeHistogramFractionNaNsInfo, metricName),
+		Err:           maybeAddMetricName(NativeHistogramFractionNaNsInfo, metricName),
 	}
 }