From 7687661453fbf9ba3e9cfeb1f16e5c4ac66b8fae Mon Sep 17 00:00:00 2001 From: beorn7 Date: Wed, 8 Jan 2025 12:55:27 +0100 Subject: [PATCH] promqltest: make eval_ordered ignore annotations Besides making eval_ordered ignore annotations, this does the following: - Adds a test to verify that eval_ordered indeed ignores an info annotations now, while eval complains about it, eval_info recognizes it and, eval_warn flags the missing of the warn annotation. - Refactors the annotation check into its own method. - Moves closing of the query to the appropriate place where it wasn't so far. Signed-off-by: beorn7 --- promql/promqltest/test.go | 47 ++++++++++++++++++---------------- promql/promqltest/test_test.go | 38 +++++++++++++++++++++++++++ 2 files changed, 63 insertions(+), 22 deletions(-) diff --git a/promql/promqltest/test.go b/promql/promqltest/test.go index efa2136f10..518164827a 100644 --- a/promql/promqltest/test.go +++ b/promql/promqltest/test.go @@ -39,6 +39,7 @@ import ( "github.com/prometheus/prometheus/promql/parser/posrange" "github.com/prometheus/prometheus/storage" "github.com/prometheus/prometheus/util/almost" + "github.com/prometheus/prometheus/util/annotations" "github.com/prometheus/prometheus/util/convertnhcb" "github.com/prometheus/prometheus/util/teststorage" "github.com/prometheus/prometheus/util/testutil" @@ -692,6 +693,24 @@ func (ev *evalCmd) expectMetric(pos int, m labels.Labels, vals ...parser.Sequenc ev.expected[h] = entry{pos: pos, vals: vals} } +// checkAnnotations asserts if the annotations match the expectations. +func (ev *evalCmd) checkAnnotations(expr string, annos annotations.Annotations) error { + countWarnings, countInfo := annos.CountWarningsAndInfo() + switch { + case ev.ordered: + // Ignore annotations if testing for order. + case !ev.warn && countWarnings > 0: + return fmt.Errorf("unexpected warnings evaluating query %q (line %d): %v", expr, ev.line, annos.AsErrors()) + case ev.warn && countWarnings == 0: + return fmt.Errorf("expected warnings evaluating query %q (line %d) but got none", expr, ev.line) + case !ev.info && countInfo > 0: + return fmt.Errorf("unexpected info annotations evaluating query %q (line %d): %v", expr, ev.line, annos.AsErrors()) + case ev.info && countInfo == 0: + return fmt.Errorf("expected info annotations evaluating query %q (line %d) but got none", expr, ev.line) + } + return nil +} + // compareResult compares the result value with the defined expectation. func (ev *evalCmd) compareResult(result parser.Value) error { switch val := result.(type) { @@ -1131,6 +1150,7 @@ func (t *test) execRangeEval(cmd *evalCmd, engine promql.QueryEngine) error { if err != nil { return fmt.Errorf("error creating range query for %q (line %d): %w", cmd.expr, cmd.line, err) } + defer q.Close() res := q.Exec(t.context) if res.Err != nil { if cmd.fail { @@ -1142,18 +1162,9 @@ 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, countInfo := res.Warnings.CountWarningsAndInfo() - switch { - case !cmd.warn && countWarnings > 0: - return fmt.Errorf("unexpected warnings evaluating query %q (line %d): %v", cmd.expr, cmd.line, res.Warnings) - case cmd.warn && countWarnings == 0: - return fmt.Errorf("expected warnings evaluating query %q (line %d) but got none", cmd.expr, cmd.line) - case !cmd.info && countInfo > 0: - return fmt.Errorf("unexpected info annotations evaluating query %q (line %d): %v", cmd.expr, cmd.line, res.Warnings) - case cmd.info && countInfo == 0: - return fmt.Errorf("expected info annotations evaluating query %q (line %d) but got none", cmd.expr, cmd.line) + if err := cmd.checkAnnotations(cmd.expr, res.Warnings); err != nil { + return err } - defer q.Close() if err := cmd.compareResult(res.Value); err != nil { return fmt.Errorf("error in %s %s (line %d): %w", cmd, cmd.expr, cmd.line, err) @@ -1196,16 +1207,8 @@ 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, countInfo := res.Warnings.CountWarningsAndInfo() - switch { - case !cmd.warn && countWarnings > 0: - return fmt.Errorf("unexpected warnings evaluating query %q (line %d): %v", iq.expr, cmd.line, res.Warnings) - case cmd.warn && countWarnings == 0: - return fmt.Errorf("expected warnings evaluating query %q (line %d) but got none", iq.expr, cmd.line) - case !cmd.info && countInfo > 0: - return fmt.Errorf("unexpected info annotations evaluating query %q (line %d): %v", iq.expr, cmd.line, res.Warnings) - case cmd.info && countInfo == 0: - return fmt.Errorf("expected info annotations evaluating query %q (line %d) but got none", iq.expr, cmd.line) + if err := cmd.checkAnnotations(iq.expr, res.Warnings); err != nil { + return err } err = cmd.compareResult(res.Value) if err != nil { @@ -1218,11 +1221,11 @@ func (t *test) runInstantQuery(iq atModifierTestCase, cmd *evalCmd, engine promq if err != nil { return fmt.Errorf("error creating range query for %q (line %d): %w", cmd.expr, cmd.line, err) } + defer q.Close() rangeRes := q.Exec(t.context) if rangeRes.Err != nil { return fmt.Errorf("error evaluating query %q (line %d) in range mode: %w", iq.expr, cmd.line, rangeRes.Err) } - defer q.Close() if cmd.ordered { // Range queries are always sorted by labels, so skip this test case that expects results in a particular order. return nil diff --git a/promql/promqltest/test_test.go b/promql/promqltest/test_test.go index 327dcd78fe..96499e869d 100644 --- a/promql/promqltest/test_test.go +++ b/promql/promqltest/test_test.go @@ -353,6 +353,44 @@ eval_ordered instant at 50m sort(http_requests) `, expectedError: `error in eval sort(http_requests) (line 10): unexpected metric {__name__="http_requests", group="canary", instance="1", job="api-server"} in result, has value 400`, }, + "instant query with results expected to match provided order, result is in expected order and info annotation is ignored": { + input: testData + ` +eval_ordered instant at 50m sort(rate(http_requests[10m])) + {group="production", instance="0", job="api-server"} 0.03333333333333333 + {group="production", instance="1", job="api-server"} 0.06666666666666667 + {group="canary", instance="0", job="api-server"} 0.1 + {group="canary", instance="1", job="api-server"} 0.13333333333333333 +`, + }, + "instant query with expected info annotation": { + input: testData + ` +eval_info instant at 50m sort(rate(http_requests[10m])) + {group="production", instance="0", job="api-server"} 0.03333333333333333 + {group="production", instance="1", job="api-server"} 0.06666666666666667 + {group="canary", instance="0", job="api-server"} 0.1 + {group="canary", instance="1", job="api-server"} 0.13333333333333333 +`, + }, + "instant query with unexpected info annotation": { + input: testData + ` +eval instant at 50m sort(rate(http_requests[10m])) + {group="production", instance="0", job="api-server"} 0.03333333333333333 + {group="production", instance="1", job="api-server"} 0.06666666666666667 + {group="canary", instance="0", job="api-server"} 0.1 + {group="canary", instance="1", job="api-server"} 0.13333333333333333 +`, + expectedError: `unexpected info annotations evaluating query "sort(rate(http_requests[10m]))" (line 10): [PromQL info: metric might not be a counter, name does not end in _total/_sum/_count/_bucket: "http_requests"]`, + }, + "instant query with unexpectedly missing warn annotation": { + input: testData + ` +eval_warn instant at 50m sort(rate(http_requests[10m])) + {group="production", instance="0", job="api-server"} 0.03333333333333333 + {group="production", instance="1", job="api-server"} 0.06666666666666667 + {group="canary", instance="0", job="api-server"} 0.1 + {group="canary", instance="1", job="api-server"} 0.13333333333333333 +`, + expectedError: `expected warnings evaluating query "sort(rate(http_requests[10m]))" (line 10) but got none`, + }, "instant query with invalid timestamp": { input: `eval instant at abc123 vector(0)`, expectedError: `error in eval vector(0) (line 1): invalid timestamp definition "abc123": not a valid duration string: "abc123"`,