From f6e4b775e273c719acc4ff2d01c66934d66acea6 Mon Sep 17 00:00:00 2001 From: Charles Korn Date: Wed, 7 Aug 2024 14:25:46 +1000 Subject: [PATCH 1/2] Check for errors first Signed-off-by: Charles Korn --- promql/promqltest/test.go | 28 ++++++++++++++-------------- 1 file changed, 14 insertions(+), 14 deletions(-) diff --git a/promql/promqltest/test.go b/promql/promqltest/test.go index 83137e661..8b1ec381a 100644 --- a/promql/promqltest/test.go +++ b/promql/promqltest/test.go @@ -1003,13 +1003,6 @@ func (t *test) execRangeEval(cmd *evalCmd, engine promql.QueryEngine) error { return fmt.Errorf("error creating range query for %q (line %d): %w", cmd.expr, cmd.line, err) } res := q.Exec(t.context) - countWarnings, _ := res.Warnings.CountWarningsAndInfo() - if !cmd.warn && countWarnings > 0 { - return fmt.Errorf("unexpected warnings evaluating query %q (line %d): %v", cmd.expr, cmd.line, res.Warnings) - } - if cmd.warn && countWarnings == 0 { - return fmt.Errorf("expected warnings evaluating query %q (line %d) but got none", cmd.expr, cmd.line) - } if res.Err != nil { if cmd.fail { return cmd.checkExpectedFailure(res.Err) @@ -1020,6 +1013,13 @@ func (t *test) execRangeEval(cmd *evalCmd, engine promql.QueryEngine) error { if res.Err == nil && cmd.fail { return fmt.Errorf("expected error evaluating query %q (line %d) but got none", cmd.expr, cmd.line) } + countWarnings, _ := res.Warnings.CountWarningsAndInfo() + if !cmd.warn && countWarnings > 0 { + return fmt.Errorf("unexpected warnings evaluating query %q (line %d): %v", cmd.expr, cmd.line, res.Warnings) + } + if cmd.warn && countWarnings == 0 { + return fmt.Errorf("expected warnings evaluating query %q (line %d) but got none", cmd.expr, cmd.line) + } defer q.Close() if err := cmd.compareResult(res.Value); err != nil { @@ -1050,13 +1050,6 @@ func (t *test) runInstantQuery(iq atModifierTestCase, cmd *evalCmd, engine promq } defer q.Close() res := q.Exec(t.context) - countWarnings, _ := res.Warnings.CountWarningsAndInfo() - if !cmd.warn && countWarnings > 0 { - return fmt.Errorf("unexpected warnings evaluating query %q (line %d): %v", iq.expr, cmd.line, res.Warnings) - } - if cmd.warn && countWarnings == 0 { - return fmt.Errorf("expected warnings evaluating query %q (line %d) but got none", iq.expr, cmd.line) - } if res.Err != nil { if cmd.fail { if err := cmd.checkExpectedFailure(res.Err); err != nil { @@ -1070,6 +1063,13 @@ func (t *test) runInstantQuery(iq atModifierTestCase, cmd *evalCmd, engine promq if res.Err == nil && cmd.fail { return fmt.Errorf("expected error evaluating query %q (line %d) but got none", iq.expr, cmd.line) } + countWarnings, _ := res.Warnings.CountWarningsAndInfo() + if !cmd.warn && countWarnings > 0 { + return fmt.Errorf("unexpected warnings evaluating query %q (line %d): %v", iq.expr, cmd.line, res.Warnings) + } + if cmd.warn && countWarnings == 0 { + return fmt.Errorf("expected warnings evaluating query %q (line %d) but got none", iq.expr, cmd.line) + } err = cmd.compareResult(res.Value) if err != nil { return fmt.Errorf("error in %s %s (line %d): %w", cmd, iq.expr, cmd.line, err) From 424cefcf5e34f8b3266dbc77df933fa7b94961aa Mon Sep 17 00:00:00 2001 From: Charles Korn Date: Wed, 7 Aug 2024 14:39:50 +1000 Subject: [PATCH 2/2] Fix "cannot reduce resolution to custom buckets schema" panic in `rate` over native histograms with mix of custom and exponential buckets Signed-off-by: Charles Korn --- promql/functions.go | 9 ++++++++ .../testdata/native_histograms.test | 22 +++++++++++++++++++ 2 files changed, 31 insertions(+) diff --git a/promql/functions.go b/promql/functions.go index 35dbd2970..018023bf0 100644 --- a/promql/functions.go +++ b/promql/functions.go @@ -179,15 +179,21 @@ func extrapolatedRate(vals []parser.Value, args parser.Expressions, enh *EvalNod // Otherwise, it returns the calculated histogram and an empty annotation. func histogramRate(points []HPoint, isCounter bool, metricName string, pos posrange.PositionRange) (*histogram.FloatHistogram, annotations.Annotations) { prev := points[0].H + usingCustomBuckets := prev.UsesCustomBuckets() last := points[len(points)-1].H if last == nil { return nil, annotations.New().Add(annotations.NewMixedFloatsHistogramsWarning(metricName, pos)) } + minSchema := prev.Schema if last.Schema < minSchema { minSchema = last.Schema } + if last.UsesCustomBuckets() != usingCustomBuckets { + return nil, annotations.New().Add(annotations.NewMixedExponentialCustomHistogramsWarning(metricName, pos)) + } + var annos annotations.Annotations // We check for gauge type histograms in the loop below, but the loop below does not run on the first and last point, @@ -215,6 +221,9 @@ func histogramRate(points []HPoint, isCounter bool, metricName string, pos posra if curr.Schema < minSchema { minSchema = curr.Schema } + if curr.UsesCustomBuckets() != usingCustomBuckets { + return nil, annotations.New().Add(annotations.NewMixedExponentialCustomHistogramsWarning(metricName, pos)) + } } h := last.CopyToSchema(minSchema) diff --git a/promql/promqltest/testdata/native_histograms.test b/promql/promqltest/testdata/native_histograms.test index 034d73eb5..f91626c34 100644 --- a/promql/promqltest/testdata/native_histograms.test +++ b/promql/promqltest/testdata/native_histograms.test @@ -762,3 +762,25 @@ eval_warn instant at 30s rate(some_metric[30s]) # Test the case where we have more than two points for rate eval_warn instant at 1m rate(some_metric[1m]) {} {{count:0.03333333333333333 sum:0.03333333333333333 buckets:[0.03333333333333333]}} + +clear + +# Test rate() over mixed exponential and custom buckets. +load 30s + some_metric {{schema:0 sum:1 count:1 buckets:[1]}} {{schema:-53 sum:1 count:1 custom_values:[5 10] buckets:[1]}} {{schema:0 sum:5 count:4 buckets:[1 2 1]}} {{schema:-53 sum:1 count:1 custom_values:[5 10] buckets:[1]}} + +# Start and end with exponential, with custom in the middle. +eval_warn instant at 1m rate(some_metric[1m]) + # Should produce no results. + +# Start and end with custom, with exponential in the middle. +eval_warn instant at 1m30s rate(some_metric[1m]) + # Should produce no results. + +# Start with custom, end with exponential. +eval_warn instant at 1m rate(some_metric[30s]) + # Should produce no results. + +# Start with exponential, end with custom. +eval_warn instant at 30s rate(some_metric[30s]) + # Should produce no results.