diff --git a/CHANGELOG.md b/CHANGELOG.md index cb3c177074..0c4aa60fae 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -69,6 +69,7 @@ * [ENHANCEMENT] Distributor: Initialize ha_tracker cache before ha_tracker and distributor reach running state and begin serving writes. #9826 #9976 * [ENHANCEMENT] Ingester: `-ingest-storage.kafka.max-buffered-bytes` to limit the memory for buffered records when using concurrent fetching. #9892 * [ENHANCEMENT] Querier: improve performance and memory consumption of queries that select many series. #9914 +* [ENHANCEMENT] Query-Frontend: prune ` and on() (vector(x)==y)` style queries. Triggered by https://github.com/prometheus/prometheus/pull/15245. #10026 * [BUGFIX] Fix issue where functions such as `rate()` over native histograms could return incorrect values if a float stale marker was present in the selected range. #9508 * [BUGFIX] Fix issue where negation of native histograms (eg. `-some_native_histogram_series`) did nothing. #9508 * [BUGFIX] Fix issue where `metric might not be a counter, name does not end in _total/_sum/_count/_bucket` annotation would be emitted even if `rate` or `increase` did not have enough samples to compute a result. #9508 diff --git a/pkg/frontend/querymiddleware/astmapper/pruning.go b/pkg/frontend/querymiddleware/astmapper/pruning.go index 3040e44f83..3b94438f2a 100644 --- a/pkg/frontend/querymiddleware/astmapper/pruning.go +++ b/pkg/frontend/querymiddleware/astmapper/pruning.go @@ -4,8 +4,6 @@ package astmapper import ( "context" - "math" - "strconv" "github.com/go-kit/log" "github.com/prometheus/prometheus/promql/parser" @@ -33,185 +31,89 @@ func (pruner *queryPruner) MapExpr(expr parser.Expr) (mapped parser.Expr, finish return nil, false, err } - switch e := expr.(type) { - case *parser.ParenExpr: - mapped, finished, err = pruner.MapExpr(e.Expr) - if err != nil { - return e, false, err - } - return &parser.ParenExpr{Expr: mapped, PosRange: e.PosRange}, finished, nil - case *parser.BinaryExpr: - return pruner.pruneBinOp(e) - default: - return e, false, nil + e, ok := expr.(*parser.BinaryExpr) + if !ok { + return expr, false, nil } -} -func (pruner *queryPruner) pruneBinOp(expr *parser.BinaryExpr) (mapped parser.Expr, finished bool, err error) { - switch expr.Op { - case parser.MUL: - return pruner.handleMultiplyOp(expr), false, nil - case parser.GTR, parser.LSS: - return pruner.handleCompOp(expr), false, nil - case parser.LOR: - return pruner.handleOrOp(expr), false, nil - case parser.LAND: - return pruner.handleAndOp(expr), false, nil - case parser.LUNLESS: - return pruner.handleUnlessOp(expr), false, nil - default: + if e.Op != parser.LAND || e.VectorMatching == nil || + !e.VectorMatching.On || len(e.VectorMatching.MatchingLabels) != 0 { + // Return if not " and on() " return expr, false, nil } -} -// The bool signifies if the number evaluates to infinity, and if it does -// we return the infinity of the correct sign. -func calcInf(isPositive bool, num string) (*parser.NumberLiteral, bool) { - coeff, err := strconv.Atoi(num) - if err != nil || coeff == 0 { - return nil, false + isConst, isEmpty := pruner.isConst(e.RHS) + if !isConst { + return expr, false, nil } - switch { - case isPositive && coeff > 0: - return &parser.NumberLiteral{Val: math.Inf(1)}, true - case isPositive && coeff < 0: - return &parser.NumberLiteral{Val: math.Inf(-1)}, true - case !isPositive && coeff > 0: - return &parser.NumberLiteral{Val: math.Inf(-1)}, true - case !isPositive && coeff < 0: - return &parser.NumberLiteral{Val: math.Inf(1)}, true - default: - return nil, false + if isEmpty { + // The right hand side is empty, so the whole expression is empty due to + // "and on()", return the right hand side. + return e.RHS, false, nil } + // The right hand side is const no empty, so the whole expression is just the + // left side. + return e.LHS, false, nil } -func (pruner *queryPruner) handleMultiplyOp(expr *parser.BinaryExpr) parser.Expr { - isInfR, signR := pruner.isInfinite(expr.RHS) - if isInfR { - newExpr, ok := calcInf(signR, expr.LHS.String()) - if ok { - return newExpr - } - } - isInfL, signL := pruner.isInfinite(expr.LHS) - if isInfL { - newExpr, ok := calcInf(signL, expr.RHS.String()) - if ok { - return newExpr +func (pruner *queryPruner) isConst(expr parser.Expr) (isConst, isEmpty bool) { + var lhs, rhs parser.Expr + switch e := expr.(type) { + case *parser.ParenExpr: + return pruner.isConst(e.Expr) + case *parser.BinaryExpr: + if e.Op != parser.EQLC || e.ReturnBool { + return false, false } - } - return expr -} - -func (pruner *queryPruner) handleCompOp(expr *parser.BinaryExpr) parser.Expr { - var refNeg, refPos parser.Expr - switch expr.Op { - case parser.LSS: - refNeg = expr.RHS - refPos = expr.LHS - case parser.GTR: - refNeg = expr.LHS - refPos = expr.RHS + lhs = e.LHS + rhs = e.RHS default: - return expr + return false, false } - // foo < -Inf or -Inf > foo => vector(0) < -Inf - isInf, sign := pruner.isInfinite(refNeg) - if isInf && !sign { - return &parser.BinaryExpr{ - LHS: &parser.Call{ - Func: parser.Functions["vector"], - Args: []parser.Expr{&parser.NumberLiteral{Val: 0}}, - }, - Op: parser.LSS, - RHS: &parser.NumberLiteral{Val: math.Inf(-1)}, - ReturnBool: false, + lIsVector, lValue := pruner.isConstVector(lhs) + if lIsVector { + rIsConst, rValue := pruner.isNumber(rhs) + if rIsConst { + return true, rValue != lValue } + return false, false } - - // foo > +Inf or +Inf < foo => vector(0) > +Inf => vector(0) < -Inf - isInf, sign = pruner.isInfinite(refPos) - if isInf && sign { - return &parser.BinaryExpr{ - LHS: &parser.Call{ - Func: parser.Functions["vector"], - Args: []parser.Expr{&parser.NumberLiteral{Val: 0}}, - }, - Op: parser.LSS, - RHS: &parser.NumberLiteral{Val: math.Inf(-1)}, - ReturnBool: false, - } + var lIsConst bool + lIsConst, lValue = pruner.isNumber(lhs) + if !lIsConst { + return false, false } - - return expr + rIsVector, rValue := pruner.isConstVector(rhs) + if !rIsVector { + return false, false + } + return true, lValue != rValue } -// 1st bool is true if the number is infinite. -// 2nd bool is true if the number is positive infinity. -func (pruner *queryPruner) isInfinite(expr parser.Expr) (bool, bool) { - mapped, _, err := pruner.MapExpr(expr) - if err == nil { - expr = mapped - } +func (pruner *queryPruner) isConstVector(expr parser.Expr) (isVector bool, value float64) { switch e := expr.(type) { case *parser.ParenExpr: - return pruner.isInfinite(e.Expr) - case *parser.NumberLiteral: - if math.IsInf(e.Val, 1) { - return true, true + return pruner.isConstVector(e.Expr) + case *parser.Call: + if e.Func.Name != "vector" || len(e.Args) != 1 { + return false, 0 } - if math.IsInf(e.Val, -1) { - return true, false + lit, ok := e.Args[0].(*parser.NumberLiteral) + if !ok { + return false, 0 } - return false, false - default: - return false, false - } -} - -func (pruner *queryPruner) handleOrOp(expr *parser.BinaryExpr) parser.Expr { - switch { - case pruner.isEmpty(expr.LHS): - return expr.RHS - case pruner.isEmpty(expr.RHS): - return expr.LHS - } - return expr -} - -func (pruner *queryPruner) handleAndOp(expr *parser.BinaryExpr) parser.Expr { - switch { - case pruner.isEmpty(expr.LHS): - return expr.LHS - case pruner.isEmpty(expr.RHS): - return expr.RHS + return true, lit.Val } - return expr + return false, 0 } -func (pruner *queryPruner) handleUnlessOp(expr *parser.BinaryExpr) parser.Expr { - switch { - case pruner.isEmpty(expr.LHS): - return expr.LHS - case pruner.isEmpty(expr.RHS): - return expr.LHS - } - return expr -} - -func (pruner *queryPruner) isEmpty(expr parser.Expr) bool { - mapped, _, err := pruner.MapExpr(expr) - if err == nil { - expr = mapped - } +func (pruner *queryPruner) isNumber(expr parser.Expr) (isNumber bool, value float64) { switch e := expr.(type) { case *parser.ParenExpr: - return pruner.isEmpty(e.Expr) - default: - if e.String() == `vector(0) < -Inf` { - return true - } - return false + return pruner.isNumber(e.Expr) + case *parser.NumberLiteral: + return true, e.Val } + return false, 0 } diff --git a/pkg/frontend/querymiddleware/astmapper/pruning_deprecated.go b/pkg/frontend/querymiddleware/astmapper/pruning_deprecated.go new file mode 100644 index 0000000000..322e1264a9 --- /dev/null +++ b/pkg/frontend/querymiddleware/astmapper/pruning_deprecated.go @@ -0,0 +1,217 @@ +// SPDX-License-Identifier: AGPL-3.0-only + +package astmapper + +import ( + "context" + "math" + "strconv" + + "github.com/go-kit/log" + "github.com/prometheus/prometheus/promql/parser" +) + +func NewQueryPrunerDeprecated(ctx context.Context, logger log.Logger) ASTMapper { + pruner := newQueryPrunerDeprecated(ctx, logger) + return NewASTExprMapper(pruner) +} + +type queryPrunerDeprecated struct { + ctx context.Context + logger log.Logger +} + +func newQueryPrunerDeprecated(ctx context.Context, logger log.Logger) *queryPrunerDeprecated { + return &queryPrunerDeprecated{ + ctx: ctx, + logger: logger, + } +} + +func (pruner *queryPrunerDeprecated) MapExpr(expr parser.Expr) (mapped parser.Expr, finished bool, err error) { + if err := pruner.ctx.Err(); err != nil { + return nil, false, err + } + + switch e := expr.(type) { + case *parser.ParenExpr: + mapped, finished, err = pruner.MapExpr(e.Expr) + if err != nil { + return e, false, err + } + return &parser.ParenExpr{Expr: mapped, PosRange: e.PosRange}, finished, nil + case *parser.BinaryExpr: + return pruner.pruneBinOp(e) + default: + return e, false, nil + } +} + +func (pruner *queryPrunerDeprecated) pruneBinOp(expr *parser.BinaryExpr) (mapped parser.Expr, finished bool, err error) { + switch expr.Op { + case parser.MUL: + return pruner.handleMultiplyOp(expr), false, nil + case parser.GTR, parser.LSS: + return pruner.handleCompOp(expr), false, nil + case parser.LOR: + return pruner.handleOrOp(expr), false, nil + case parser.LAND: + return pruner.handleAndOp(expr), false, nil + case parser.LUNLESS: + return pruner.handleUnlessOp(expr), false, nil + default: + return expr, false, nil + } +} + +// The bool signifies if the number evaluates to infinity, and if it does +// we return the infinity of the correct sign. +func calcInf(isPositive bool, num string) (*parser.NumberLiteral, bool) { + coeff, err := strconv.Atoi(num) + if err != nil || coeff == 0 { + return nil, false + } + switch { + case isPositive && coeff > 0: + return &parser.NumberLiteral{Val: math.Inf(1)}, true + case isPositive && coeff < 0: + return &parser.NumberLiteral{Val: math.Inf(-1)}, true + case !isPositive && coeff > 0: + return &parser.NumberLiteral{Val: math.Inf(-1)}, true + case !isPositive && coeff < 0: + return &parser.NumberLiteral{Val: math.Inf(1)}, true + default: + return nil, false + } +} + +func (pruner *queryPrunerDeprecated) handleMultiplyOp(expr *parser.BinaryExpr) parser.Expr { + isInfR, signR := pruner.isInfinite(expr.RHS) + if isInfR { + newExpr, ok := calcInf(signR, expr.LHS.String()) + if ok { + return newExpr + } + } + isInfL, signL := pruner.isInfinite(expr.LHS) + if isInfL { + newExpr, ok := calcInf(signL, expr.RHS.String()) + if ok { + return newExpr + } + } + return expr +} + +func (pruner *queryPrunerDeprecated) handleCompOp(expr *parser.BinaryExpr) parser.Expr { + var refNeg, refPos parser.Expr + switch expr.Op { + case parser.LSS: + refNeg = expr.RHS + refPos = expr.LHS + case parser.GTR: + refNeg = expr.LHS + refPos = expr.RHS + default: + return expr + } + + // foo < -Inf or -Inf > foo => vector(0) < -Inf + isInf, sign := pruner.isInfinite(refNeg) + if isInf && !sign { + return &parser.BinaryExpr{ + LHS: &parser.Call{ + Func: parser.Functions["vector"], + Args: []parser.Expr{&parser.NumberLiteral{Val: 0}}, + }, + Op: parser.LSS, + RHS: &parser.NumberLiteral{Val: math.Inf(-1)}, + ReturnBool: false, + } + } + + // foo > +Inf or +Inf < foo => vector(0) > +Inf => vector(0) < -Inf + isInf, sign = pruner.isInfinite(refPos) + if isInf && sign { + return &parser.BinaryExpr{ + LHS: &parser.Call{ + Func: parser.Functions["vector"], + Args: []parser.Expr{&parser.NumberLiteral{Val: 0}}, + }, + Op: parser.LSS, + RHS: &parser.NumberLiteral{Val: math.Inf(-1)}, + ReturnBool: false, + } + } + + return expr +} + +// 1st bool is true if the number is infinite. +// 2nd bool is true if the number is positive infinity. +func (pruner *queryPrunerDeprecated) isInfinite(expr parser.Expr) (bool, bool) { + mapped, _, err := pruner.MapExpr(expr) + if err == nil { + expr = mapped + } + switch e := expr.(type) { + case *parser.ParenExpr: + return pruner.isInfinite(e.Expr) + case *parser.NumberLiteral: + if math.IsInf(e.Val, 1) { + return true, true + } + if math.IsInf(e.Val, -1) { + return true, false + } + return false, false + default: + return false, false + } +} + +func (pruner *queryPrunerDeprecated) handleOrOp(expr *parser.BinaryExpr) parser.Expr { + switch { + case pruner.isEmpty(expr.LHS): + return expr.RHS + case pruner.isEmpty(expr.RHS): + return expr.LHS + } + return expr +} + +func (pruner *queryPrunerDeprecated) handleAndOp(expr *parser.BinaryExpr) parser.Expr { + switch { + case pruner.isEmpty(expr.LHS): + return expr.LHS + case pruner.isEmpty(expr.RHS): + return expr.RHS + } + return expr +} + +func (pruner *queryPrunerDeprecated) handleUnlessOp(expr *parser.BinaryExpr) parser.Expr { + switch { + case pruner.isEmpty(expr.LHS): + return expr.LHS + case pruner.isEmpty(expr.RHS): + return expr.LHS + } + return expr +} + +func (pruner *queryPrunerDeprecated) isEmpty(expr parser.Expr) bool { + mapped, _, err := pruner.MapExpr(expr) + if err == nil { + expr = mapped + } + switch e := expr.(type) { + case *parser.ParenExpr: + return pruner.isEmpty(e.Expr) + default: + if e.String() == `vector(0) < -Inf` { + return true + } + return false + } +} diff --git a/pkg/frontend/querymiddleware/astmapper/pruning_deprecated_test.go b/pkg/frontend/querymiddleware/astmapper/pruning_deprecated_test.go new file mode 100644 index 0000000000..52005ea28f --- /dev/null +++ b/pkg/frontend/querymiddleware/astmapper/pruning_deprecated_test.go @@ -0,0 +1,207 @@ +// SPDX-License-Identifier: AGPL-3.0-only + +package astmapper + +import ( + "context" + "testing" + + "github.com/go-kit/log" + "github.com/prometheus/prometheus/promql/parser" + "github.com/stretchr/testify/require" +) + +func TestQueryPrunerDeprecated(t *testing.T) { + pruner := NewQueryPrunerDeprecated(context.Background(), log.NewNopLogger()) + + for _, tt := range []struct { + in string + out string + }{ + { + `quantile(0.9,foo)`, + `quantile(0.9,foo)`, + }, + { + `histogram_quantile(0.5, rate(bar1{baz="blip"}[30s]))`, + `histogram_quantile(0.5, rate(bar1{baz="blip"}[30s]))`, + }, + { + `count(up)`, + `count(up)`, + }, + { + `avg(rate(foo[1m]))`, + `avg(rate(foo[1m]))`, + }, + { + `vector(0)`, + `vector(0)`, + }, + { + `up < -Inf`, + `vector(0) < -Inf`, + }, + { + `-Inf > up`, + `vector(0) < -Inf`, + }, + { + `up > +Inf`, + `vector(0) < -Inf`, + }, + { + `+Inf < up`, + `vector(0) < -Inf`, + }, + { + `up < +Inf`, + `up < +Inf`, + }, + { + `+Inf > up`, + `+Inf > up`, + }, + { + `up > -Inf`, + `up > -Inf`, + }, + { + `-Inf < up`, + `-Inf < up`, + }, + { + `avg(rate(foo[1m])) < (-Inf)`, + `vector(0) < -Inf`, + }, + { + `Inf * -1`, + `-Inf`, + }, + { + `+1 * -Inf`, + `-Inf`, + }, + { + `1 * +Inf`, + `Inf`, + }, + { + `-Inf * -1`, + `+Inf`, + }, + { + `avg(rate(foo[1m])) < (-1 * +Inf)`, + `vector(0) < -Inf`, + }, + { + `avg(rate(foo[1m])) < (+1 * +Inf)`, + `avg(rate(foo[1m])) < (+Inf)`, + }, + { + `avg(rate(foo[1m])) < (-1 * -Inf)`, + `avg(rate(foo[1m])) < (+Inf)`, + }, + { + `avg(rate(foo[1m])) < (+1 * -Inf)`, + `vector(0) < -Inf`, + }, + { + `(-1 * -Inf) < avg(rate(foo[1m]))`, + `vector(0) < -Inf`, + }, + { + `vector(0) < -Inf or avg(rate(foo[1m]))`, + `avg(rate(foo[1m]))`, + }, + { + `avg(rate(foo[1m])) or vector(0) < -Inf`, + `avg(rate(foo[1m]))`, + }, + { + `avg(rate(foo[1m])) or (vector(0) < -Inf)`, + `avg(rate(foo[1m]))`, + }, + { + `((-1 * -Inf) < avg(rate(foo[2m]))) or avg(rate(foo[1m]))`, + `avg(rate(foo[1m]))`, + }, + { + `((-1 * -Inf) <= avg(rate(foo[2m]))) or avg(rate(foo[1m]))`, + `((+Inf) <= avg(rate(foo[2m]))) or avg(rate(foo[1m]))`, + }, + { + `((-1 * -Inf) >= avg(rate(foo[2m]))) or avg(rate(foo[1m]))`, + `((+Inf) >= avg(rate(foo[2m]))) or avg(rate(foo[1m]))`, + }, + { + `((-1 * -Inf) > avg(rate(foo[2m]))) or avg(rate(foo[1m]))`, + `((+Inf) > avg(rate(foo[2m]))) or avg(rate(foo[1m]))`, + }, + { + `((-1 * +Inf) > avg(rate(foo[2m]))) or avg(rate(foo[1m]))`, + `avg(rate(foo[1m]))`, + }, + { + `((+1 * -Inf) > avg(rate(foo[2m]))) or avg(rate(foo[1m]))`, + `avg(rate(foo[1m]))`, + }, + { + `(avg(rate(foo[2m])) < (+1 * -Inf)) or avg(rate(foo[1m]))`, + `avg(rate(foo[1m]))`, + }, + { + `((-1 * -Inf) == avg(rate(foo[2m]))) or avg(rate(foo[1m]))`, + `((+Inf) == avg(rate(foo[2m]))) or avg(rate(foo[1m]))`, + }, + { + `((-1 * -Inf) != avg(rate(foo[2m]))) or avg(rate(foo[1m]))`, + `((+Inf) != avg(rate(foo[2m]))) or avg(rate(foo[1m]))`, + }, + { + `((-1 * -Inf) < avg(rate(foo[2m]))) and avg(rate(foo[1m]))`, + `(vector(0) < -Inf)`, + }, + { + `((-1 * -Inf) < avg(rate(foo[2m]))) unless avg(rate(foo[1m]))`, + `(vector(0) < -Inf)`, + }, + { + `avg(rate(foo[1m])) unless ((-1 * -Inf) < avg(rate(foo[2m])))`, + `avg(rate(foo[1m]))`, + }, + { + `((2 * +Inf) < avg(rate(foo[2m]))) or avg(rate(foo[1m]))`, + `avg(rate(foo[1m]))`, + }, + { + `(((-1 * -Inf) < avg(rate(foo[3m]))) unless avg(rate(foo[2m]))) or avg(rate(foo[1m]))`, + `avg(rate(foo[1m]))`, + }, + { + `((((-1 * -Inf) < avg(rate(foo[4m]))) unless avg(rate(foo[3m]))) and avg(rate(foo[2m]))) or avg(rate(foo[1m]))`, + `avg(rate(foo[1m]))`, + }, + { + `(((-1 * -Inf) < avg(rate(foo[4m]))) unless (avg(rate(foo[3m])) and avg(rate(foo[2m])))) or avg(rate(foo[1m]))`, + `avg(rate(foo[1m]))`, + }, + { + `(((-1 * -Inf) < avg(rate(foo[4m]))) unless avg(rate(foo[3m])) and avg(rate(foo[2m]))) or avg(rate(foo[1m]))`, + `avg(rate(foo[1m]))`, + }, + } { + tt := tt + + t.Run(tt.in, func(t *testing.T) { + expr, err := parser.ParseExpr(tt.in) + require.NoError(t, err) + out, err := parser.ParseExpr(tt.out) + require.NoError(t, err) + + mapped, err := pruner.Map(expr) + require.NoError(t, err) + require.Equal(t, out.String(), mapped.String()) + }) + } +} diff --git a/pkg/frontend/querymiddleware/astmapper/pruning_test.go b/pkg/frontend/querymiddleware/astmapper/pruning_test.go index 963c97e29d..791d75d1da 100644 --- a/pkg/frontend/querymiddleware/astmapper/pruning_test.go +++ b/pkg/frontend/querymiddleware/astmapper/pruning_test.go @@ -18,6 +18,23 @@ func TestQueryPruner(t *testing.T) { in string out string }{ + // Non-prunable expressions. + { + `foo`, + `foo`, + }, + { + `foo[1m]`, + `foo[1m]`, + }, + { + `foo{bar="baz"}[1m]`, + `foo{bar="baz"}[1m]`, + }, + { + `foo{bar="baz"}`, + `foo{bar="baz"}`, + }, { `quantile(0.9,foo)`, `quantile(0.9,foo)`, @@ -40,155 +57,110 @@ func TestQueryPruner(t *testing.T) { }, { `up < -Inf`, - `vector(0) < -Inf`, - }, - { - `-Inf > up`, - `vector(0) < -Inf`, - }, - { - `up > +Inf`, - `vector(0) < -Inf`, - }, - { - `+Inf < up`, - `vector(0) < -Inf`, - }, - { - `up < +Inf`, - `up < +Inf`, - }, - { - `+Inf > up`, - `+Inf > up`, - }, - { - `up > -Inf`, - `up > -Inf`, - }, - { - `-Inf < up`, - `-Inf < up`, + `up < -Inf`, }, { `avg(rate(foo[1m])) < (-Inf)`, - `vector(0) < -Inf`, + `avg(rate(foo[1m])) < (-Inf)`, }, { `Inf * -1`, - `-Inf`, - }, - { - `+1 * -Inf`, - `-Inf`, - }, - { - `1 * +Inf`, - `Inf`, - }, - { - `-Inf * -1`, - `+Inf`, + `Inf * -1`, }, { `avg(rate(foo[1m])) < (-1 * +Inf)`, - `vector(0) < -Inf`, - }, - { - `avg(rate(foo[1m])) < (+1 * +Inf)`, - `avg(rate(foo[1m])) < (+Inf)`, - }, - { - `avg(rate(foo[1m])) < (-1 * -Inf)`, - `avg(rate(foo[1m])) < (+Inf)`, - }, - { - `avg(rate(foo[1m])) < (+1 * -Inf)`, - `vector(0) < -Inf`, + `avg(rate(foo[1m])) < (-1 * +Inf)`, }, { `(-1 * -Inf) < avg(rate(foo[1m]))`, - `vector(0) < -Inf`, + `(-1 * -Inf) < avg(rate(foo[1m]))`, }, { `vector(0) < -Inf or avg(rate(foo[1m]))`, - `avg(rate(foo[1m]))`, + `vector(0) < -Inf or avg(rate(foo[1m]))`, }, { `avg(rate(foo[1m])) or vector(0) < -Inf`, - `avg(rate(foo[1m]))`, + `avg(rate(foo[1m])) or vector(0) < -Inf`, }, { `avg(rate(foo[1m])) or (vector(0) < -Inf)`, - `avg(rate(foo[1m]))`, + `avg(rate(foo[1m])) or (vector(0) < -Inf)`, }, { `((-1 * -Inf) < avg(rate(foo[2m]))) or avg(rate(foo[1m]))`, - `avg(rate(foo[1m]))`, + `((-1 * -Inf) < avg(rate(foo[2m]))) or avg(rate(foo[1m]))`, }, { - `((-1 * -Inf) <= avg(rate(foo[2m]))) or avg(rate(foo[1m]))`, - `((+Inf) <= avg(rate(foo[2m]))) or avg(rate(foo[1m]))`, + `(avg(rate(foo[2m])) < (+1 * -Inf)) or avg(rate(foo[1m]))`, + `(avg(rate(foo[2m])) < (+1 * -Inf)) or avg(rate(foo[1m]))`, }, { - `((-1 * -Inf) >= avg(rate(foo[2m]))) or avg(rate(foo[1m]))`, - `((+Inf) >= avg(rate(foo[2m]))) or avg(rate(foo[1m]))`, + `((-1 * -Inf) == avg(rate(foo[2m]))) or avg(rate(foo[1m]))`, + `((-1 * -Inf) == avg(rate(foo[2m]))) or avg(rate(foo[1m]))`, }, { - `((-1 * -Inf) > avg(rate(foo[2m]))) or avg(rate(foo[1m]))`, - `((+Inf) > avg(rate(foo[2m]))) or avg(rate(foo[1m]))`, + `avg(rate(foo[1m])) unless ((-1 * -Inf) < avg(rate(foo[2m])))`, + `avg(rate(foo[1m])) unless ((-1 * -Inf) < avg(rate(foo[2m])))`, }, { - `((-1 * +Inf) > avg(rate(foo[2m]))) or avg(rate(foo[1m]))`, - `avg(rate(foo[1m]))`, + `((2 * +Inf) < avg(rate(foo[2m]))) or avg(rate(foo[1m]))`, + `((2 * +Inf) < avg(rate(foo[2m]))) or avg(rate(foo[1m]))`, }, { - `((+1 * -Inf) > avg(rate(foo[2m]))) or avg(rate(foo[1m]))`, - `avg(rate(foo[1m]))`, + `(((-1 * -Inf) < avg(rate(foo[3m]))) unless avg(rate(foo[2m]))) or avg(rate(foo[1m]))`, + `(((-1 * -Inf) < avg(rate(foo[3m]))) unless avg(rate(foo[2m]))) or avg(rate(foo[1m]))`, }, { - `(avg(rate(foo[2m])) < (+1 * -Inf)) or avg(rate(foo[1m]))`, - `avg(rate(foo[1m]))`, + `((((-1 * -Inf) < avg(rate(foo[4m]))) unless avg(rate(foo[3m]))) and avg(rate(foo[2m]))) or avg(rate(foo[1m]))`, + `((((-1 * -Inf) < avg(rate(foo[4m]))) unless avg(rate(foo[3m]))) and avg(rate(foo[2m]))) or avg(rate(foo[1m]))`, }, { - `((-1 * -Inf) == avg(rate(foo[2m]))) or avg(rate(foo[1m]))`, - `((+Inf) == avg(rate(foo[2m]))) or avg(rate(foo[1m]))`, + `avg(rate(foo[1m])) or avg(rate(bar[1m]))`, + `avg(rate(foo[1m])) or avg(rate(bar[1m]))`, }, - { - `((-1 * -Inf) != avg(rate(foo[2m]))) or avg(rate(foo[1m]))`, - `((+Inf) != avg(rate(foo[2m]))) or avg(rate(foo[1m]))`, + { // The const expression is on the wrong side. + `(vector(0) == 1) and on() (avg(rate(foo[1m])))`, + `(vector(0) == 1) and on() (avg(rate(foo[1m])))`, }, - { - `((-1 * -Inf) < avg(rate(foo[2m]))) and avg(rate(foo[1m]))`, - `(vector(0) < -Inf)`, + { // Matching on labels. + `(avg(rate(foo[1m]))) and on(id) (vector(0) == 1)`, + `(avg(rate(foo[1m]))) and on(id) (vector(0) == 1)`, }, + { // Not "on" expression. + `(avg(rate(foo[1m]))) and ignoring() (vector(0) == 1)`, + `(avg(rate(foo[1m]))) and ignoring() (vector(0) == 1)`, + }, + // Pruned expressions. { - `((-1 * -Inf) < avg(rate(foo[2m]))) unless avg(rate(foo[1m]))`, - `(vector(0) < -Inf)`, + `(avg(rate(foo[1m]))) and on() (vector(0) == 1)`, + `(vector(0) == 1)`, }, { - `avg(rate(foo[1m])) unless ((-1 * -Inf) < avg(rate(foo[2m])))`, - `avg(rate(foo[1m]))`, + `(avg(rate(foo[1m]))) and on() (vector(1) == 1)`, + `(avg(rate(foo[1m])))`, }, { - `((2 * +Inf) < avg(rate(foo[2m]))) or avg(rate(foo[1m]))`, - `avg(rate(foo[1m]))`, + `(avg(rate(foo[1m]))) and on() (vector(3) == 4.5)`, + `(vector(3) == 4.5)`, }, { - `(((-1 * -Inf) < avg(rate(foo[3m]))) unless avg(rate(foo[2m]))) or avg(rate(foo[1m]))`, - `avg(rate(foo[1m]))`, + `(avg(rate(foo[1m]))) and on() (vector(5.5) == 5.5)`, + `(avg(rate(foo[1m])))`, }, { - `((((-1 * -Inf) < avg(rate(foo[4m]))) unless avg(rate(foo[3m]))) and avg(rate(foo[2m]))) or avg(rate(foo[1m]))`, - `avg(rate(foo[1m]))`, + // "and on()" is not on top level, "or" has lower precedence. + `(avg(rate(foo[1m]))) and on() (vector(0) == 1) or avg(rate(bar[1m]))`, + `(vector(0) == 1) or avg(rate(bar[1m]))`, }, { - `(((-1 * -Inf) < avg(rate(foo[4m]))) unless (avg(rate(foo[3m])) and avg(rate(foo[2m])))) or avg(rate(foo[1m]))`, - `avg(rate(foo[1m]))`, + // "and on()" is not on top level, due to left-right associativity. + `(avg(rate(foo[1m]))) and on() (vector(0) == 1) and avg(rate(bar[1m]))`, + `(vector(0) == 1) and avg(rate(bar[1m]))`, }, { - `(((-1 * -Inf) < avg(rate(foo[4m]))) unless avg(rate(foo[3m])) and avg(rate(foo[2m]))) or avg(rate(foo[1m]))`, - `avg(rate(foo[1m]))`, + `(avg(rate(foo[1m]))) and on() (vector(0) == 1) or avg(rate(bar[1m])) and on() (vector(1) == 1)`, + `(vector(0) == 1) or avg(rate(bar[1m]))`, }, } { tt := tt diff --git a/pkg/frontend/querymiddleware/prune_deprecated.go b/pkg/frontend/querymiddleware/prune_deprecated.go new file mode 100644 index 0000000000..c1300b6267 --- /dev/null +++ b/pkg/frontend/querymiddleware/prune_deprecated.go @@ -0,0 +1,74 @@ +// SPDX-License-Identifier: AGPL-3.0-only + +package querymiddleware + +import ( + "context" + + "github.com/go-kit/log" + "github.com/go-kit/log/level" + "github.com/prometheus/prometheus/promql/parser" + + apierror "github.com/grafana/mimir/pkg/api/error" + "github.com/grafana/mimir/pkg/frontend/querymiddleware/astmapper" + "github.com/grafana/mimir/pkg/util/spanlogger" +) + +type pruneMiddlewareDeprecated struct { + next MetricsQueryHandler + logger log.Logger +} + +// newPruneMiddleware creates a middleware that optimises queries by rewriting them to prune away +// unnecessary computations. +func newPruneMiddlewareDeprecated(logger log.Logger) MetricsQueryMiddleware { + return MetricsQueryMiddlewareFunc(func(next MetricsQueryHandler) MetricsQueryHandler { + return &pruneMiddlewareDeprecated{ + next: next, + logger: logger, + } + }) +} + +func (p *pruneMiddlewareDeprecated) Do(ctx context.Context, r MetricsQueryRequest) (Response, error) { + log := spanlogger.FromContext(ctx, p.logger) + + prunedQuery, success, err := p.pruneQuery(ctx, r.GetQuery()) + if err != nil { + level.Warn(log).Log("msg", "failed to prune the input query, falling back to the original query", "query", r.GetQuery(), "err", err) + + return p.next.Do(ctx, r) + } + + if !success { + level.Debug(log).Log("msg", "query pruning had no effect", "query", r.GetQuery()) + return p.next.Do(ctx, r) + } + + level.Debug(log).Log("msg", "query has been rewritten by pruning", "original", r.GetQuery(), "rewritten", prunedQuery) + + updatedReq, err := r.WithQuery(prunedQuery) + if err != nil { + return nil, err + } + + return p.next.Do(ctx, updatedReq) +} + +func (p *pruneMiddlewareDeprecated) pruneQuery(ctx context.Context, query string) (string, bool, error) { + // Parse the query. + expr, err := parser.ParseExpr(query) + if err != nil { + return "", false, apierror.New(apierror.TypeBadData, DecorateWithParamName(err, "query").Error()) + } + origQueryString := expr.String() + + mapper := astmapper.NewQueryPruner(ctx, p.logger) + prunedQuery, err := mapper.Map(expr) + if err != nil { + return "", false, err + } + prunedQueryString := prunedQuery.String() + + return prunedQueryString, origQueryString != prunedQueryString, nil +} diff --git a/pkg/frontend/querymiddleware/prune_deprecated_test.go b/pkg/frontend/querymiddleware/prune_deprecated_test.go new file mode 100644 index 0000000000..9ffb5fcffd --- /dev/null +++ b/pkg/frontend/querymiddleware/prune_deprecated_test.go @@ -0,0 +1,98 @@ +// SPDX-License-Identifier: AGPL-3.0-only + +package querymiddleware + +import ( + "context" + "fmt" + "strings" + "testing" + "time" + + "github.com/go-kit/log" + "github.com/grafana/dskit/user" + "github.com/prometheus/prometheus/promql/promqltest" + "github.com/stretchr/testify/require" +) + +func TestQueryPruningDeprecated(t *testing.T) { + numSamples := 100 + seriesName := `test_float` + replacer := strings.NewReplacer("", seriesName, "", fmt.Sprintf("%d", numSamples)) + data := replacer.Replace(` + load 1m + {series="1"} 0+1x + {series="2"} 0+2x + {series="3"} 0+3x + {series="4"} 0+4x + {series="5"} 0+5x + `) + queryable := promqltest.LoadedStorage(t, data) + + const step = 20 * time.Second + + engine := newEngine() + pruningware := newPruneMiddlewareDeprecated( + log.NewNopLogger(), + ) + downstream := &downstreamHandler{ + engine: engine, + queryable: queryable, + } + + type template struct { + query string + IsEmpty bool + } + + templates := []template{ + {`avg(rate(%s[1m])) < (-1 * +Inf)`, true}, + {`avg(rate(%s[1m])) < (+1 * +Inf)`, false}, + {`avg(rate(%s[1m])) < (-1 * -Inf)`, false}, + {`avg(rate(%s[1m])) < (+1 * -Inf)`, true}, + {`(-1 * -Inf) < avg(rate(%s[1m]))`, true}, + {`((-1 * -Inf) < avg(rate(foo[2m]))) or avg(rate(%s[1m]))`, false}, + {`((-1 * -Inf) < avg(rate(foo[2m]))) and avg(rate(%s[1m]))`, true}, + {`((-1 * -Inf) < avg(rate(foo[2m]))) unless avg(rate(%s[1m]))`, true}, + {`avg(rate(%s[1m])) unless ((-1 * -Inf) < avg(rate(foo[2m])))`, false}, + {`(((-1 * -Inf) < avg(rate(foo[3m]))) unless avg(rate(foo[2m]))) or avg(rate(%s[1m]))`, true}, + {`((((-1 * -Inf) < avg(rate(foo[4m]))) unless avg(rate(foo[3m]))) and avg(rate(foo[2m]))) or avg(rate(%s[1m]))`, true}, + } + for _, template := range templates { + t.Run(template.query, func(t *testing.T) { + query := fmt.Sprintf(template.query, seriesName) + req := &PrometheusRangeQueryRequest{ + path: "/query_range", + start: 0, + end: int64(numSamples) * time.Minute.Milliseconds(), + step: step.Milliseconds(), + queryExpr: parseQuery(t, query), + } + + injectedContext := user.InjectOrgID(context.Background(), "test") + + // Run the query without pruning. + expectedRes, err := downstream.Do(injectedContext, req) + require.Nil(t, err) + + if !template.IsEmpty { + // Ensure the query produces some results. + require.NotEmpty(t, expectedRes.(*PrometheusResponse).Data.Result) + requireValidSamples(t, expectedRes.(*PrometheusResponse).Data.Result) + } + + // Run the query with pruning. + prunedRes, err := pruningware.Wrap(downstream).Do(injectedContext, req) + require.Nil(t, err) + + if !template.IsEmpty { + // Ensure the query produces some results. + require.NotEmpty(t, prunedRes.(*PrometheusResponse).Data.Result) + requireValidSamples(t, prunedRes.(*PrometheusResponse).Data.Result) + } + + // Ensure the results are approximately equal. + approximatelyEqualsSamples(t, expectedRes.(*PrometheusResponse), prunedRes.(*PrometheusResponse)) + }) + } +} diff --git a/pkg/frontend/querymiddleware/prune_test.go b/pkg/frontend/querymiddleware/prune_test.go index d0ea3cdda5..35e06b6911 100644 --- a/pkg/frontend/querymiddleware/prune_test.go +++ b/pkg/frontend/querymiddleware/prune_test.go @@ -46,17 +46,9 @@ func TestQueryPruning(t *testing.T) { } templates := []template{ - {`avg(rate(%s[1m])) < (-1 * +Inf)`, true}, - {`avg(rate(%s[1m])) < (+1 * +Inf)`, false}, - {`avg(rate(%s[1m])) < (-1 * -Inf)`, false}, - {`avg(rate(%s[1m])) < (+1 * -Inf)`, true}, - {`(-1 * -Inf) < avg(rate(%s[1m]))`, true}, - {`((-1 * -Inf) < avg(rate(foo[2m]))) or avg(rate(%s[1m]))`, false}, - {`((-1 * -Inf) < avg(rate(foo[2m]))) and avg(rate(%s[1m]))`, true}, - {`((-1 * -Inf) < avg(rate(foo[2m]))) unless avg(rate(%s[1m]))`, true}, - {`avg(rate(%s[1m])) unless ((-1 * -Inf) < avg(rate(foo[2m])))`, false}, - {`(((-1 * -Inf) < avg(rate(foo[3m]))) unless avg(rate(foo[2m]))) or avg(rate(%s[1m]))`, true}, - {`((((-1 * -Inf) < avg(rate(foo[4m]))) unless avg(rate(foo[3m]))) and avg(rate(foo[2m]))) or avg(rate(%s[1m]))`, true}, + {`(avg(rate(%s[1m]))) and on() (vector(0) == 1)`, true}, + {`(avg(rate(%s[1m]))) and on() (vector(1) == 1)`, false}, + {`avg(rate(%s[1m])) or (avg(rate(test_float[1m]))) and on() (vector(0) == 1)`, false}, } for _, template := range templates { t.Run(template.query, func(t *testing.T) { diff --git a/pkg/frontend/querymiddleware/roundtrip.go b/pkg/frontend/querymiddleware/roundtrip.go index 2df612e699..048b374640 100644 --- a/pkg/frontend/querymiddleware/roundtrip.go +++ b/pkg/frontend/querymiddleware/roundtrip.go @@ -437,15 +437,18 @@ func newQueryMiddlewares( if cfg.PrunedQueries { pruneMiddleware := newPruneMiddleware(log) + pruneMiddlewareDeprecated := newPruneMiddlewareDeprecated(log) queryRangeMiddleware = append( queryRangeMiddleware, newInstrumentMiddleware("pruning", metrics), pruneMiddleware, + pruneMiddlewareDeprecated, ) queryInstantMiddleware = append( queryInstantMiddleware, newInstrumentMiddleware("pruning", metrics), pruneMiddleware, + pruneMiddlewareDeprecated, ) } diff --git a/pkg/frontend/querymiddleware/roundtrip_test.go b/pkg/frontend/querymiddleware/roundtrip_test.go index 65f5216ffc..59faae27a1 100644 --- a/pkg/frontend/querymiddleware/roundtrip_test.go +++ b/pkg/frontend/querymiddleware/roundtrip_test.go @@ -587,6 +587,7 @@ func TestMiddlewaresConsistency(t *testing.T) { "splitInstantQueryByIntervalMiddleware", // Not applicable because specific to instant queries. "stepAlignMiddleware", // Not applicable because remote read requests don't take step in account when running in Mimir. "pruneMiddleware", // No query pruning support. + "pruneMiddlewareDeprecated", // No query pruning support. "experimentalFunctionsMiddleware", // No blocking for PromQL experimental functions as it is executed remotely. }, },