diff --git a/promql/engine.go b/promql/engine.go index deb778a1c3..11a53065e8 100644 --- a/promql/engine.go +++ b/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 diff --git a/promql/engine_test.go b/promql/engine_test.go index b6d9c3f1a4..536f4cac62 100644 --- a/promql/engine_test.go +++ b/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) } diff --git a/promql/functions.go b/promql/functions.go index 34a43e3ef5..7250cdb2bd 100644 --- a/promql/functions.go +++ b/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 { diff --git a/promql/promqltest/testdata/native_histograms.test b/promql/promqltest/testdata/native_histograms.test index 78224c0346..52ee853802 100644 --- a/promql/promqltest/testdata/native_histograms.test +++ b/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" diff --git a/util/annotations/annotations.go b/util/annotations/annotations.go index f8070ff343..ec92b06d06 100644 --- a/util/annotations/annotations.go +++ b/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) - NativeHistogramQuantileNaNSkewInfo = fmt.Errorf("%w: input to histogram_quantile has NaN observations, result is skewed higher for metric name", PromQLInfo) - NativeHistogramFractionNaNsInfo = fmt.Errorf("%w: input to histogram_fraction has NaN observations, which are excluded from all fractions 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", 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), } }