From 86c71856e83254ef21635c5c203449bbc6024cf0 Mon Sep 17 00:00:00 2001 From: Ganesh Vernekar <15064823+codesome@users.noreply.github.com> Date: Tue, 9 Feb 2021 21:33:16 +0530 Subject: [PATCH] Add start() and end() pre-processors for @ modifier (#8425) * Add start() and end() pre-processors for @ modifier Signed-off-by: Ganesh Vernekar * Fix reviews Signed-off-by: Ganesh Vernekar * Fix review comments Signed-off-by: Ganesh Vernekar * Fix review comments Signed-off-by: Ganesh Vernekar --- docs/disabled_features.md | 5 +- docs/querying/basics.md | 17 +- promql/engine.go | 62 +- promql/engine_test.go | 180 +++++- promql/parser/ast.go | 8 +- promql/parser/generated_parser.y | 17 +- promql/parser/generated_parser.y.go | 880 ++++++++++++++-------------- promql/parser/lex.go | 4 + promql/parser/lex_test.go | 16 + promql/parser/parse.go | 74 ++- promql/parser/parse_test.go | 106 ++++ promql/parser/printer.go | 17 +- promql/parser/printer_test.go | 20 + promql/test.go | 6 - web/api/v1/api.go | 6 + 15 files changed, 917 insertions(+), 501 deletions(-) diff --git a/docs/disabled_features.md b/docs/disabled_features.md index b579b38342..7c1c6e09e8 100644 --- a/docs/disabled_features.md +++ b/docs/disabled_features.md @@ -15,11 +15,12 @@ They may be enabled by default in future versions. `--enable-feature=promql-at-modifier` -This feature lets you specify the evaluation time for instant vector selectors, -range vector selectors, and subqueries. More details can be found [here](querying/basics.md#@-modifier). +The `@` modifier lets you specify the evaluation time for instant vector selectors, +range vector selectors, and subqueries. More details can be found [here](querying/basics.md#-modifier). ## Remote Write Receiver `--enable-feature=remote-write-receiver` The remote write receiver allows Prometheus to accept remote write requests from other Prometheus servers. More details can be found [here](storage.md#overview). + diff --git a/docs/querying/basics.md b/docs/querying/basics.md index 6c76a83b0a..7229e8e1aa 100644 --- a/docs/querying/basics.md +++ b/docs/querying/basics.md @@ -207,7 +207,7 @@ The same works for range vectors. This returns the 5-minute rate that ### @ modifier The `@` modifier allows changing the evaluation time for individual instant -and range vectors in a query. The time supplied to `@` modifier +and range vectors in a query. The time supplied to the `@` modifier is a unix timestamp and described with a float literal. For example, the following expression returns the value of @@ -229,9 +229,9 @@ The same works for range vectors. This returns the 5-minute rate that rate(http_requests_total[5m] @ 1609746000) -`@` modifier supports all representation of float literals described +The `@` modifier supports all representation of float literals described above within the limits of `int64`. It can also be used along -with `offset` modifier where the offset is applied relative to the `@` +with the `offset` modifier where the offset is applied relative to the `@` modifier time irrespective of which modifier is written first. These 2 queries will produce the same result. @@ -242,7 +242,16 @@ These 2 queries will produce the same result. This modifier is disabled by default since it breaks the invariant that PromQL does not look ahead of the evaluation time for samples. It can be enabled by setting -`--enable-feature=promql-at-modifier` flag. It will be enabled by default in the future. +`--enable-feature=promql-at-modifier` flag. See [disabled features](../disabled_features.md) for more details about this flag. + +Additionally, `start()` and `end()` can also be used as values for the `@` modifier as special values. + +For a range query, they resolve to the start and end of the range query respectively and remain the same for all steps. + +For an instant query, `start()` and `end()` both resolve to the evaluation time. + + http_requests_total @ start() + rate(http_requests_total[5m] @ end()) ## Subquery diff --git a/promql/engine.go b/promql/engine.go index dfd21a6db9..2ed1014419 100644 --- a/promql/engine.go +++ b/promql/engine.go @@ -373,7 +373,7 @@ func (ng *Engine) newQuery(q storage.Queryable, expr parser.Expr, start, end tim } es := &parser.EvalStmt{ - Expr: WrapWithStepInvariantExpr(expr), + Expr: PreprocessExpr(expr, start, end), Start: start, End: end, Interval: interval, @@ -387,6 +387,8 @@ func (ng *Engine) newQuery(q storage.Queryable, expr parser.Expr, start, end tim return qry, nil } +var ErrValidationAtModifierDisabled = errors.New("@ modifier is disabled") + func (ng *Engine) validateOpts(expr parser.Expr) error { if ng.enableAtModifier { return nil @@ -396,20 +398,21 @@ func (ng *Engine) validateOpts(expr parser.Expr) error { parser.Inspect(expr, func(node parser.Node, path []parser.Node) error { switch n := node.(type) { case *parser.VectorSelector: - if n.Timestamp != nil { - validationErr = errors.New("@ modifier is disabled") + if n.Timestamp != nil || n.StartOrEnd == parser.START || n.StartOrEnd == parser.END { + validationErr = ErrValidationAtModifierDisabled return validationErr } case *parser.MatrixSelector: - if n.VectorSelector.(*parser.VectorSelector).Timestamp != nil { - validationErr = errors.New("@ modifier is disabled") + vs := n.VectorSelector.(*parser.VectorSelector) + if vs.Timestamp != nil || vs.StartOrEnd == parser.START || vs.StartOrEnd == parser.END { + validationErr = ErrValidationAtModifierDisabled return validationErr } case *parser.SubqueryExpr: - if n.Timestamp != nil { - validationErr = errors.New("@ modifier is disabled") + if n.Timestamp != nil || n.StartOrEnd == parser.START || n.StartOrEnd == parser.END { + validationErr = ErrValidationAtModifierDisabled return validationErr } } @@ -2311,29 +2314,35 @@ func unwrapStepInvariantExpr(e parser.Expr) parser.Expr { return e } -// WrapWithStepInvariantExpr wraps all possible parts of the given -// expression with StepInvariantExpr wherever valid. -func WrapWithStepInvariantExpr(expr parser.Expr) parser.Expr { - isStepInvariant := wrapWithStepInvariantExprHelper(expr) +// PreprocessExpr wraps all possible step invariant parts of the given expression with +// StepInvariantExpr. It also resolves the preprocessors. +func PreprocessExpr(expr parser.Expr, start, end time.Time) parser.Expr { + isStepInvariant := preprocessExprHelper(expr, start, end) if isStepInvariant { return newStepInvariantExpr(expr) } return expr } -// wrapWithStepInvariantExprHelper wraps the child nodes of the expression -// with a StepInvariantExpr wherever valid. The returned boolean is true if the +// preprocessExprHelper wraps the child nodes of the expression +// with a StepInvariantExpr wherever it's step invariant. The returned boolean is true if the // passed expression qualifies to be wrapped by StepInvariantExpr. -func wrapWithStepInvariantExprHelper(expr parser.Expr) bool { +// It also resolves the preprocessors. +func preprocessExprHelper(expr parser.Expr, start, end time.Time) bool { switch n := expr.(type) { case *parser.VectorSelector: + if n.StartOrEnd == parser.START { + n.Timestamp = makeInt64Pointer(timestamp.FromTime(start)) + } else if n.StartOrEnd == parser.END { + n.Timestamp = makeInt64Pointer(timestamp.FromTime(end)) + } return n.Timestamp != nil case *parser.AggregateExpr: - return wrapWithStepInvariantExprHelper(n.Expr) + return preprocessExprHelper(n.Expr, start, end) case *parser.BinaryExpr: - isInvariant1, isInvariant2 := wrapWithStepInvariantExprHelper(n.LHS), wrapWithStepInvariantExprHelper(n.RHS) + isInvariant1, isInvariant2 := preprocessExprHelper(n.LHS, start, end), preprocessExprHelper(n.RHS, start, end) if isInvariant1 && isInvariant2 { return true } @@ -2352,7 +2361,7 @@ func wrapWithStepInvariantExprHelper(expr parser.Expr) bool { isStepInvariant := !ok isStepInvariantSlice := make([]bool, len(n.Args)) for i := range n.Args { - isStepInvariantSlice[i] = wrapWithStepInvariantExprHelper(n.Args[i]) + isStepInvariantSlice[i] = preprocessExprHelper(n.Args[i], start, end) isStepInvariant = isStepInvariant && isStepInvariantSlice[i] } @@ -2370,7 +2379,7 @@ func wrapWithStepInvariantExprHelper(expr parser.Expr) bool { return false case *parser.MatrixSelector: - return n.VectorSelector.(*parser.VectorSelector).Timestamp != nil + return preprocessExprHelper(n.VectorSelector, start, end) case *parser.SubqueryExpr: // Since we adjust offset for the @ modifier evaluation, @@ -2378,17 +2387,22 @@ func wrapWithStepInvariantExprHelper(expr parser.Expr) bool { // Hence we wrap the inside of subquery irrespective of // @ on subquery (given it is also step invariant) so that // it is evaluated only once w.r.t. the start time of subquery. - isInvariant := wrapWithStepInvariantExprHelper(n.Expr) + isInvariant := preprocessExprHelper(n.Expr, start, end) if isInvariant { n.Expr = newStepInvariantExpr(n.Expr) } + if n.StartOrEnd == parser.START { + n.Timestamp = makeInt64Pointer(timestamp.FromTime(start)) + } else if n.StartOrEnd == parser.END { + n.Timestamp = makeInt64Pointer(timestamp.FromTime(end)) + } return n.Timestamp != nil case *parser.ParenExpr: - return wrapWithStepInvariantExprHelper(n.Expr) + return preprocessExprHelper(n.Expr, start, end) case *parser.UnaryExpr: - return wrapWithStepInvariantExprHelper(n.Expr) + return preprocessExprHelper(n.Expr, start, end) case *parser.StringLiteral, *parser.NumberLiteral: return true @@ -2441,3 +2455,9 @@ func setOffsetForAtModifier(evalTime int64, expr parser.Expr) { return nil }) } + +func makeInt64Pointer(val int64) *int64 { + valp := new(int64) + *valp = val + return valp +} diff --git a/promql/engine_test.go b/promql/engine_test.go index 3881e65049..c49d926238 100644 --- a/promql/engine_test.go +++ b/promql/engine_test.go @@ -1038,6 +1038,24 @@ load 1ms Metric: lblstopk2, }, }, + }, { + query: `metric_topk and topk(1, sum_over_time(metric_topk[50s] @ end()))`, + start: 70, end: 100, interval: 10, + result: Matrix{ + Series{ + Points: []Point{{V: 993, T: 70000}, {V: 992, T: 80000}, {V: 991, T: 90000}, {V: 990, T: 100000}}, + Metric: lblstopk3, + }, + }, + }, { + query: `metric_topk and topk(1, sum_over_time(metric_topk[50s] @ start()))`, + start: 100, end: 130, interval: 10, + result: Matrix{ + Series{ + Points: []Point{{V: 990, T: 100000}, {V: 989, T: 110000}, {V: 988, T: 120000}, {V: 987, T: 130000}}, + Metric: lblstopk3, + }, + }, }, { // Tests for https://github.com/prometheus/prometheus/issues/8433. // The trick here is that the query range should be > lookback delta. @@ -1517,7 +1535,9 @@ func TestQueryLogger_error(t *testing.T) { } } -func TestWrapWithStepInvariantExpr(t *testing.T) { +func TestPreprocessAndWrapWithStepInvariantExpr(t *testing.T) { + startTime := time.Unix(1000, 0) + endTime := time.Unix(9999, 0) var testCases = []struct { input string // The input to be parsed. expected parser.Expr // The expected expression AST. @@ -2106,6 +2126,120 @@ func TestWrapWithStepInvariantExpr(t *testing.T) { }, }, }, + }, { + input: `foo @ start()`, + expected: &parser.StepInvariantExpr{ + Expr: &parser.VectorSelector{ + Name: "foo", + LabelMatchers: []*labels.Matcher{ + parser.MustLabelMatcher(labels.MatchEqual, "__name__", "foo"), + }, + PosRange: parser.PositionRange{ + Start: 0, + End: 13, + }, + Timestamp: makeInt64Pointer(timestamp.FromTime(startTime)), + StartOrEnd: parser.START, + }, + }, + }, { + input: `foo @ end()`, + expected: &parser.StepInvariantExpr{ + Expr: &parser.VectorSelector{ + Name: "foo", + LabelMatchers: []*labels.Matcher{ + parser.MustLabelMatcher(labels.MatchEqual, "__name__", "foo"), + }, + PosRange: parser.PositionRange{ + Start: 0, + End: 11, + }, + Timestamp: makeInt64Pointer(timestamp.FromTime(endTime)), + StartOrEnd: parser.END, + }, + }, + }, { + input: `test[5y] @ start()`, + expected: &parser.StepInvariantExpr{ + Expr: &parser.MatrixSelector{ + VectorSelector: &parser.VectorSelector{ + Name: "test", + Timestamp: makeInt64Pointer(timestamp.FromTime(startTime)), + StartOrEnd: parser.START, + LabelMatchers: []*labels.Matcher{ + parser.MustLabelMatcher(labels.MatchEqual, "__name__", "test"), + }, + PosRange: parser.PositionRange{ + Start: 0, + End: 4, + }, + }, + Range: 5 * 365 * 24 * time.Hour, + EndPos: 18, + }, + }, + }, { + input: `test[5y] @ end()`, + expected: &parser.StepInvariantExpr{ + Expr: &parser.MatrixSelector{ + VectorSelector: &parser.VectorSelector{ + Name: "test", + Timestamp: makeInt64Pointer(timestamp.FromTime(endTime)), + StartOrEnd: parser.END, + LabelMatchers: []*labels.Matcher{ + parser.MustLabelMatcher(labels.MatchEqual, "__name__", "test"), + }, + PosRange: parser.PositionRange{ + Start: 0, + End: 4, + }, + }, + Range: 5 * 365 * 24 * time.Hour, + EndPos: 16, + }, + }, + }, { + input: `some_metric[10m:5s] @ start()`, + expected: &parser.StepInvariantExpr{ + Expr: &parser.SubqueryExpr{ + Expr: &parser.VectorSelector{ + Name: "some_metric", + LabelMatchers: []*labels.Matcher{ + parser.MustLabelMatcher(labels.MatchEqual, "__name__", "some_metric"), + }, + PosRange: parser.PositionRange{ + Start: 0, + End: 11, + }, + }, + Timestamp: makeInt64Pointer(timestamp.FromTime(startTime)), + StartOrEnd: parser.START, + Range: 10 * time.Minute, + Step: 5 * time.Second, + EndPos: 29, + }, + }, + }, { + input: `some_metric[10m:5s] @ end()`, + expected: &parser.StepInvariantExpr{ + Expr: &parser.SubqueryExpr{ + Expr: &parser.VectorSelector{ + Name: "some_metric", + LabelMatchers: []*labels.Matcher{ + parser.MustLabelMatcher(labels.MatchEqual, "__name__", "some_metric"), + }, + PosRange: parser.PositionRange{ + Start: 0, + End: 11, + }, + }, + Timestamp: makeInt64Pointer(timestamp.FromTime(endTime)), + StartOrEnd: parser.END, + Range: 10 * time.Minute, + Step: 5 * time.Second, + EndPos: 27, + }, + }, }, } @@ -2113,7 +2247,7 @@ func TestWrapWithStepInvariantExpr(t *testing.T) { t.Run(test.input, func(t *testing.T) { expr, err := parser.ParseExpr(test.input) require.NoError(t, err) - expr = WrapWithStepInvariantExpr(expr) + expr = PreprocessExpr(expr, startTime, endTime) require.Equal(t, test.expected, expr, "error on input '%s'", test.input) }) } @@ -2124,16 +2258,44 @@ func TestEngineOptsValidation(t *testing.T) { opts EngineOpts query string fail bool - expError string + expError error }{ { - opts: EngineOpts{EnableAtModifier: false}, - query: "metric @ 100", - fail: true, - expError: "@ modifier is disabled", + opts: EngineOpts{EnableAtModifier: false}, + query: "metric @ 100", fail: true, expError: ErrValidationAtModifierDisabled, + }, { + opts: EngineOpts{EnableAtModifier: false}, + query: "rate(metric[1m] @ 100)", fail: true, expError: ErrValidationAtModifierDisabled, + }, { + opts: EngineOpts{EnableAtModifier: false}, + query: "rate(metric[1h:1m] @ 100)", fail: true, expError: ErrValidationAtModifierDisabled, + }, { + opts: EngineOpts{EnableAtModifier: false}, + query: "metric @ start()", fail: true, expError: ErrValidationAtModifierDisabled, + }, { + opts: EngineOpts{EnableAtModifier: false}, + query: "rate(metric[1m] @ start())", fail: true, expError: ErrValidationAtModifierDisabled, + }, { + opts: EngineOpts{EnableAtModifier: false}, + query: "rate(metric[1h:1m] @ start())", fail: true, expError: ErrValidationAtModifierDisabled, + }, { + opts: EngineOpts{EnableAtModifier: false}, + query: "metric @ end()", fail: true, expError: ErrValidationAtModifierDisabled, + }, { + opts: EngineOpts{EnableAtModifier: false}, + query: "rate(metric[1m] @ end())", fail: true, expError: ErrValidationAtModifierDisabled, + }, { + opts: EngineOpts{EnableAtModifier: false}, + query: "rate(metric[1h:1m] @ end())", fail: true, expError: ErrValidationAtModifierDisabled, }, { opts: EngineOpts{EnableAtModifier: true}, query: "metric @ 100", + }, { + opts: EngineOpts{EnableAtModifier: true}, + query: "rate(metric[1m] @ start())", + }, { + opts: EngineOpts{EnableAtModifier: true}, + query: "rate(metric[1h:1m] @ end())", }, } @@ -2142,8 +2304,8 @@ func TestEngineOptsValidation(t *testing.T) { _, err1 := eng.NewInstantQuery(nil, c.query, time.Unix(10, 0)) _, err2 := eng.NewRangeQuery(nil, c.query, time.Unix(0, 0), time.Unix(10, 0), time.Second) if c.fail { - require.Equal(t, c.expError, err1.Error()) - require.Equal(t, c.expError, err2.Error()) + require.Equal(t, c.expError, err1) + require.Equal(t, c.expError, err2) } else { require.Nil(t, err1) require.Nil(t, err2) diff --git a/promql/parser/ast.go b/promql/parser/ast.go index 459f1f47f5..a8754f2405 100644 --- a/promql/parser/ast.go +++ b/promql/parser/ast.go @@ -133,9 +133,10 @@ type SubqueryExpr struct { // Offset is the offset used during the query execution // which is calculated using the original offset, at modifier time, // eval time, and subquery offsets in the AST tree. - Offset time.Duration - Timestamp *int64 - Step time.Duration + Offset time.Duration + Timestamp *int64 + StartOrEnd ItemType // Set when @ is used with start() or end() + Step time.Duration EndPos Pos } @@ -191,6 +192,7 @@ type VectorSelector struct { // eval time, and subquery offsets in the AST tree. Offset time.Duration Timestamp *int64 + StartOrEnd ItemType // Set when @ is used with start() or end() LabelMatchers []*labels.Matcher // The unexpanded seriesSet populated at query preparation time. diff --git a/promql/parser/generated_parser.y b/promql/parser/generated_parser.y index 7964bf35f8..6f398addfd 100644 --- a/promql/parser/generated_parser.y +++ b/promql/parser/generated_parser.y @@ -116,6 +116,13 @@ ON WITHOUT %token keywordsEnd +// Preprocessors. +%token preprocessorStart +%token +START +END +%token preprocessorEnd + // Start symbols for the generated parser. %token startSymbolsStart @@ -131,7 +138,7 @@ START_METRIC_SELECTOR %type label_match_list %type label_matcher -%type aggregate_op grouping_label match_op maybe_label metric_identifier unary_op +%type aggregate_op grouping_label match_op maybe_label metric_identifier unary_op at_modifier_preprocessors %type label_set label_set_list metric %type