From eff4ff1231c8f25092b965ae5b239651c81d5117 Mon Sep 17 00:00:00 2001 From: Cyril Tovena Date: Wed, 31 Mar 2021 08:26:44 -0400 Subject: [PATCH] Improve matchers validations. (#3564) 1- It currently 500. 2- Happens too late, at ingester level and querier. (better at the frontend) 3- Improve API surface level, we don't need to expose this validation for anything else than ParseLogSelector. see #3216 Signed-off-by: Cyril Tovena --- pkg/chunkenc/memchunk_test.go | 2 +- pkg/logql/ast.go | 4 +- pkg/logql/functions_test.go | 2 +- pkg/logql/log/parser_hints_test.go | 2 +- pkg/logql/optimize.go | 2 +- pkg/logql/optimize_test.go | 5 +- pkg/logql/parser.go | 63 ++++++++---- pkg/logql/parser_test.go | 150 +++++++++++++++-------------- 8 files changed, 128 insertions(+), 102 deletions(-) diff --git a/pkg/chunkenc/memchunk_test.go b/pkg/chunkenc/memchunk_test.go index a1b19da8bc07..f3f23748325e 100644 --- a/pkg/chunkenc/memchunk_test.go +++ b/pkg/chunkenc/memchunk_test.go @@ -999,7 +999,7 @@ func BenchmarkBufferedIteratorLabels(b *testing.B) { } { b.Run(test, func(b *testing.B) { b.ReportAllocs() - expr, err := logql.ParseSampleExpr(test, true) + expr, err := logql.ParseSampleExpr(test) if err != nil { b.Fatal(err) } diff --git a/pkg/logql/ast.go b/pkg/logql/ast.go index b2e7138ca14b..4e258ef08723 100644 --- a/pkg/logql/ast.go +++ b/pkg/logql/ast.go @@ -54,13 +54,13 @@ type SelectSampleParams struct { // Expr returns the SampleExpr from the SelectSampleParams. // The `LogSelectorExpr` can then returns all matchers and filters to use for that request. func (s SelectSampleParams) Expr() (SampleExpr, error) { - return ParseSampleExpr(s.Selector, true) + return ParseSampleExpr(s.Selector) } // LogSelector returns the LogSelectorExpr from the SelectParams. // The `LogSelectorExpr` can then returns all matchers and filters to use for that request. func (s SelectSampleParams) LogSelector() (LogSelectorExpr, error) { - expr, err := ParseSampleExpr(s.Selector, true) + expr, err := ParseSampleExpr(s.Selector) if err != nil { return nil, err } diff --git a/pkg/logql/functions_test.go b/pkg/logql/functions_test.go index 03eb33933f8e..31af9b9ec82e 100644 --- a/pkg/logql/functions_test.go +++ b/pkg/logql/functions_test.go @@ -96,7 +96,7 @@ func Test_Extractor(t *testing.T) { `, } { t.Run(tc, func(t *testing.T) { - expr, err := ParseSampleExpr(tc, true) + expr, err := ParseSampleExpr(tc) require.Nil(t, err) _, err = expr.Extractor() require.Nil(t, err) diff --git a/pkg/logql/log/parser_hints_test.go b/pkg/logql/log/parser_hints_test.go index fc64884feb57..fc639461ca82 100644 --- a/pkg/logql/log/parser_hints_test.go +++ b/pkg/logql/log/parser_hints_test.go @@ -209,7 +209,7 @@ func Test_ParserHints(t *testing.T) { tt := tt t.Run(tt.expr, func(t *testing.T) { t.Parallel() - expr, err := logql.ParseSampleExpr(tt.expr, true) + expr, err := logql.ParseSampleExpr(tt.expr) require.NoError(t, err) ex, err := expr.Extractor() diff --git a/pkg/logql/optimize.go b/pkg/logql/optimize.go index 6c157129043e..25745fcaeeac 100644 --- a/pkg/logql/optimize.go +++ b/pkg/logql/optimize.go @@ -15,7 +15,7 @@ func optimizeSampleExpr(expr SampleExpr) (SampleExpr, error) { } // clone the expr. q := expr.String() - expr, err := ParseSampleExpr(q, true) + expr, err := ParseSampleExpr(q) if err != nil { return nil, err } diff --git a/pkg/logql/optimize_test.go b/pkg/logql/optimize_test.go index 5f419f8a937d..8f12f3ffff59 100644 --- a/pkg/logql/optimize_test.go +++ b/pkg/logql/optimize_test.go @@ -7,11 +7,10 @@ import ( ) func Test_optimizeSampleExpr(t *testing.T) { - tests := []struct { in, expected string }{ - //noop + // noop {`1`, `1`}, {`1 + 1`, `2`}, {`topk(10,sum by(name)(rate({region="us-east1"}[5m])))`, `topk(10,sum by(name)(rate({region="us-east1"}[5m])))`}, @@ -28,7 +27,7 @@ func Test_optimizeSampleExpr(t *testing.T) { } for _, tt := range tests { t.Run(tt.in, func(t *testing.T) { - e, err := ParseSampleExpr(tt.in, true) + e, err := ParseSampleExpr(tt.in) require.NoError(t, err) got, err := optimizeSampleExpr(e) require.NoError(t, err) diff --git a/pkg/logql/parser.go b/pkg/logql/parser.go index 55b43affb224..f295e386ff2f 100644 --- a/pkg/logql/parser.go +++ b/pkg/logql/parser.go @@ -58,7 +58,18 @@ func (p *parser) Parse() (Expr, error) { } // ParseExpr parses a string and returns an Expr. -func ParseExpr(input string) (expr Expr, err error) { +func ParseExpr(input string) (Expr, error) { + expr, err := parseExprWithoutValidation(input) + if err != nil { + return nil, err + } + if err := validateExpr(expr); err != nil { + return nil, err + } + return expr, nil +} + +func parseExprWithoutValidation(input string) (expr Expr, err error) { if len(input) >= maxInputSize { return nil, newParseError(fmt.Sprintf("input size too long (%d > %d)", len(input), maxInputSize), 0, 0) } @@ -83,14 +94,32 @@ func ParseExpr(input string) (expr Expr, err error) { return p.Parse() } -// checkEqualityMatchers checks whether a query would touch all the streams in the query range or uses at least one matcher to select specific streams. -func checkEqualityMatchers(matchers []*labels.Matcher) error { +func validateExpr(expr Expr) error { + switch e := expr.(type) { + case SampleExpr: + err := validateSampleExpr(e) + if err != nil { + return err + } + case LogSelectorExpr: + err := validateMatchers(e.Matchers()) + if err != nil { + return err + } + default: + return newParseError(fmt.Sprintf("unexpected expression type: %v", e), 0, 0) + } + return nil +} + +// validateMatchers checks whether a query would touch all the streams in the query range or uses at least one matcher to select specific streams. +func validateMatchers(matchers []*labels.Matcher) error { if len(matchers) == 0 { return nil } _, matchers = util.SplitFiltersAndMatchers(matchers) if len(matchers) == 0 { - return errors.New(errAtleastOneEqualityMatcherRequired) + return newParseError(errAtleastOneEqualityMatcherRequired, 0, 0) } return nil @@ -111,7 +140,7 @@ func ParseMatchers(input string) ([]*labels.Matcher, error) { } // ParseSampleExpr parses a string and returns the sampleExpr -func ParseSampleExpr(input string, equalityMatcherRequired bool) (SampleExpr, error) { +func ParseSampleExpr(input string) (SampleExpr, error) { expr, err := ParseExpr(input) if err != nil { return nil, err @@ -121,33 +150,27 @@ func ParseSampleExpr(input string, equalityMatcherRequired bool) (SampleExpr, er return nil, errors.New("only sample expression supported") } - if equalityMatcherRequired { - err = sampleExprCheckEqualityMatchers(sampleExpr) - if err != nil { - return nil, err - } - } return sampleExpr, nil } -func sampleExprCheckEqualityMatchers(expr SampleExpr) error { +func validateSampleExpr(expr SampleExpr) error { switch e := expr.(type) { case *binOpExpr: - if err := sampleExprCheckEqualityMatchers(e.SampleExpr); err != nil { + if err := validateSampleExpr(e.SampleExpr); err != nil { return err } - return sampleExprCheckEqualityMatchers(e.RHS) + return validateSampleExpr(e.RHS) case *literalExpr: return nil default: - return checkEqualityMatchers(expr.Selector().Matchers()) + return validateMatchers(expr.Selector().Matchers()) } } // ParseLogSelector parses a log selector expression `{app="foo"} |= "filter"` -func ParseLogSelector(input string, equalityMatcherRequired bool) (LogSelectorExpr, error) { - expr, err := ParseExpr(input) +func ParseLogSelector(input string, validate bool) (LogSelectorExpr, error) { + expr, err := parseExprWithoutValidation(input) if err != nil { return nil, err } @@ -155,10 +178,8 @@ func ParseLogSelector(input string, equalityMatcherRequired bool) (LogSelectorEx if !ok { return nil, errors.New("only log selector is supported") } - - if equalityMatcherRequired { - err = checkEqualityMatchers(logSelector.Matchers()) - if err != nil { + if validate { + if err := validateExpr(expr); err != nil { return nil, err } } diff --git a/pkg/logql/parser_test.go b/pkg/logql/parser_test.go index b3533b47f4b0..68b59848423f 100644 --- a/pkg/logql/parser_test.go +++ b/pkg/logql/parser_test.go @@ -81,112 +81,118 @@ func TestParse(t *testing.T) { exp: &matchersExpr{matchers: []*labels.Matcher{mustNewMatcher(labels.MatchEqual, "foo", "bar")}}, }, { - in: `{ foo != "bar" }`, - exp: &matchersExpr{matchers: []*labels.Matcher{mustNewMatcher(labels.MatchNotEqual, "foo", "bar")}}, + in: `{ namespace="buzz", foo != "bar" }`, + exp: &matchersExpr{matchers: []*labels.Matcher{ + mustNewMatcher(labels.MatchEqual, "namespace", "buzz"), + mustNewMatcher(labels.MatchNotEqual, "foo", "bar"), + }}, }, { in: `{ foo =~ "bar" }`, exp: &matchersExpr{matchers: []*labels.Matcher{mustNewMatcher(labels.MatchRegexp, "foo", "bar")}}, }, { - in: `{ foo !~ "bar" }`, - exp: &matchersExpr{matchers: []*labels.Matcher{mustNewMatcher(labels.MatchNotRegexp, "foo", "bar")}}, + in: `{ namespace="buzz", foo !~ "bar" }`, + exp: &matchersExpr{matchers: []*labels.Matcher{ + mustNewMatcher(labels.MatchEqual, "namespace", "buzz"), + mustNewMatcher(labels.MatchNotRegexp, "foo", "bar"), + }}, }, { - in: `count_over_time({ foo !~ "bar" }[12m])`, + in: `count_over_time({ foo = "bar" }[12m])`, exp: &rangeAggregationExpr{ left: &logRange{ - left: &matchersExpr{matchers: []*labels.Matcher{mustNewMatcher(labels.MatchNotRegexp, "foo", "bar")}}, + left: &matchersExpr{matchers: []*labels.Matcher{mustNewMatcher(labels.MatchEqual, "foo", "bar")}}, interval: 12 * time.Minute, }, operation: "count_over_time", }, }, { - in: `bytes_over_time({ foo !~ "bar" }[12m])`, + in: `bytes_over_time({ foo = "bar" }[12m])`, exp: &rangeAggregationExpr{ left: &logRange{ - left: &matchersExpr{matchers: []*labels.Matcher{mustNewMatcher(labels.MatchNotRegexp, "foo", "bar")}}, + left: &matchersExpr{matchers: []*labels.Matcher{mustNewMatcher(labels.MatchEqual, "foo", "bar")}}, interval: 12 * time.Minute, }, operation: OpRangeTypeBytes, }, }, { - in: `bytes_rate({ foo !~ "bar" }[12m])`, + in: `bytes_rate({ foo = "bar" }[12m])`, exp: &rangeAggregationExpr{ left: &logRange{ - left: &matchersExpr{matchers: []*labels.Matcher{mustNewMatcher(labels.MatchNotRegexp, "foo", "bar")}}, + left: &matchersExpr{matchers: []*labels.Matcher{mustNewMatcher(labels.MatchEqual, "foo", "bar")}}, interval: 12 * time.Minute, }, operation: OpRangeTypeBytesRate, }, }, { - in: `rate({ foo !~ "bar" }[5h])`, + in: `rate({ foo = "bar" }[5h])`, exp: &rangeAggregationExpr{ left: &logRange{ - left: &matchersExpr{matchers: []*labels.Matcher{mustNewMatcher(labels.MatchNotRegexp, "foo", "bar")}}, + left: &matchersExpr{matchers: []*labels.Matcher{mustNewMatcher(labels.MatchEqual, "foo", "bar")}}, interval: 5 * time.Hour, }, operation: "rate", }, }, { - in: `rate({ foo !~ "bar" }[5d])`, + in: `rate({ foo = "bar" }[5d])`, exp: &rangeAggregationExpr{ left: &logRange{ - left: &matchersExpr{matchers: []*labels.Matcher{mustNewMatcher(labels.MatchNotRegexp, "foo", "bar")}}, + left: &matchersExpr{matchers: []*labels.Matcher{mustNewMatcher(labels.MatchEqual, "foo", "bar")}}, interval: 5 * 24 * time.Hour, }, operation: "rate", }, }, { - in: `count_over_time({ foo !~ "bar" }[1w])`, + in: `count_over_time({ foo = "bar" }[1w])`, exp: &rangeAggregationExpr{ left: &logRange{ - left: &matchersExpr{matchers: []*labels.Matcher{mustNewMatcher(labels.MatchNotRegexp, "foo", "bar")}}, + left: &matchersExpr{matchers: []*labels.Matcher{mustNewMatcher(labels.MatchEqual, "foo", "bar")}}, interval: 7 * 24 * time.Hour, }, operation: "count_over_time", }, }, { - in: `absent_over_time({ foo !~ "bar" }[1w])`, + in: `absent_over_time({ foo = "bar" }[1w])`, exp: &rangeAggregationExpr{ left: &logRange{ - left: &matchersExpr{matchers: []*labels.Matcher{mustNewMatcher(labels.MatchNotRegexp, "foo", "bar")}}, + left: &matchersExpr{matchers: []*labels.Matcher{mustNewMatcher(labels.MatchEqual, "foo", "bar")}}, interval: 7 * 24 * time.Hour, }, operation: OpRangeTypeAbsent, }, }, { - in: `sum(rate({ foo !~ "bar" }[5h]))`, + in: `sum(rate({ foo = "bar" }[5h]))`, exp: mustNewVectorAggregationExpr(&rangeAggregationExpr{ left: &logRange{ - left: &matchersExpr{matchers: []*labels.Matcher{mustNewMatcher(labels.MatchNotRegexp, "foo", "bar")}}, + left: &matchersExpr{matchers: []*labels.Matcher{mustNewMatcher(labels.MatchEqual, "foo", "bar")}}, interval: 5 * time.Hour, }, operation: "rate", }, "sum", nil, nil), }, { - in: `sum(rate({ foo !~ "bar" }[1y]))`, + in: `sum(rate({ foo ="bar" }[1y]))`, exp: mustNewVectorAggregationExpr(&rangeAggregationExpr{ left: &logRange{ - left: &matchersExpr{matchers: []*labels.Matcher{mustNewMatcher(labels.MatchNotRegexp, "foo", "bar")}}, + left: &matchersExpr{matchers: []*labels.Matcher{mustNewMatcher(labels.MatchEqual, "foo", "bar")}}, interval: 365 * 24 * time.Hour, }, operation: "rate", }, "sum", nil, nil), }, { - in: `avg(count_over_time({ foo !~ "bar" }[5h])) by (bar,foo)`, + in: `avg(count_over_time({ foo = "bar" }[5h])) by (bar,foo)`, exp: mustNewVectorAggregationExpr(&rangeAggregationExpr{ left: &logRange{ - left: &matchersExpr{matchers: []*labels.Matcher{mustNewMatcher(labels.MatchNotRegexp, "foo", "bar")}}, + left: &matchersExpr{matchers: []*labels.Matcher{mustNewMatcher(labels.MatchEqual, "foo", "bar")}}, interval: 5 * time.Hour, }, operation: "count_over_time", @@ -198,7 +204,7 @@ func TestParse(t *testing.T) { { in: `avg( label_replace( - count_over_time({ foo !~ "bar" }[5h]), + count_over_time({ foo = "bar" }[5h]), "bar", "$1$2", "foo", @@ -209,7 +215,7 @@ func TestParse(t *testing.T) { mustNewLabelReplaceExpr( &rangeAggregationExpr{ left: &logRange{ - left: &matchersExpr{matchers: []*labels.Matcher{mustNewMatcher(labels.MatchNotRegexp, "foo", "bar")}}, + left: &matchersExpr{matchers: []*labels.Matcher{mustNewMatcher(labels.MatchEqual, "foo", "bar")}}, interval: 5 * time.Hour, }, operation: "count_over_time", @@ -222,10 +228,10 @@ func TestParse(t *testing.T) { }, nil), }, { - in: `avg(count_over_time({ foo !~ "bar" }[5h])) by ()`, + in: `avg(count_over_time({ foo = "bar" }[5h])) by ()`, exp: mustNewVectorAggregationExpr(&rangeAggregationExpr{ left: &logRange{ - left: &matchersExpr{matchers: []*labels.Matcher{mustNewMatcher(labels.MatchNotRegexp, "foo", "bar")}}, + left: &matchersExpr{matchers: []*labels.Matcher{mustNewMatcher(labels.MatchEqual, "foo", "bar")}}, interval: 5 * time.Hour, }, operation: "count_over_time", @@ -235,10 +241,10 @@ func TestParse(t *testing.T) { }, nil), }, { - in: `max without (bar) (count_over_time({ foo !~ "bar" }[5h]))`, + in: `max without (bar) (count_over_time({ foo = "bar" }[5h]))`, exp: mustNewVectorAggregationExpr(&rangeAggregationExpr{ left: &logRange{ - left: &matchersExpr{matchers: []*labels.Matcher{mustNewMatcher(labels.MatchNotRegexp, "foo", "bar")}}, + left: &matchersExpr{matchers: []*labels.Matcher{mustNewMatcher(labels.MatchEqual, "foo", "bar")}}, interval: 5 * time.Hour, }, operation: "count_over_time", @@ -248,10 +254,10 @@ func TestParse(t *testing.T) { }, nil), }, { - in: `max without () (count_over_time({ foo !~ "bar" }[5h]))`, + in: `max without () (count_over_time({ foo = "bar" }[5h]))`, exp: mustNewVectorAggregationExpr(&rangeAggregationExpr{ left: &logRange{ - left: &matchersExpr{matchers: []*labels.Matcher{mustNewMatcher(labels.MatchNotRegexp, "foo", "bar")}}, + left: &matchersExpr{matchers: []*labels.Matcher{mustNewMatcher(labels.MatchEqual, "foo", "bar")}}, interval: 5 * time.Hour, }, operation: "count_over_time", @@ -261,10 +267,10 @@ func TestParse(t *testing.T) { }, nil), }, { - in: `topk(10,count_over_time({ foo !~ "bar" }[5h])) without (bar)`, + in: `topk(10,count_over_time({ foo = "bar" }[5h])) without (bar)`, exp: mustNewVectorAggregationExpr(&rangeAggregationExpr{ left: &logRange{ - left: &matchersExpr{matchers: []*labels.Matcher{mustNewMatcher(labels.MatchNotRegexp, "foo", "bar")}}, + left: &matchersExpr{matchers: []*labels.Matcher{mustNewMatcher(labels.MatchEqual, "foo", "bar")}}, interval: 5 * time.Hour, }, operation: "count_over_time", @@ -274,10 +280,10 @@ func TestParse(t *testing.T) { }, NewStringLabelFilter("10")), }, { - in: `bottomk(30 ,sum(rate({ foo !~ "bar" }[5h])) by (foo))`, + in: `bottomk(30 ,sum(rate({ foo = "bar" }[5h])) by (foo))`, exp: mustNewVectorAggregationExpr(mustNewVectorAggregationExpr(&rangeAggregationExpr{ left: &logRange{ - left: &matchersExpr{matchers: []*labels.Matcher{mustNewMatcher(labels.MatchNotRegexp, "foo", "bar")}}, + left: &matchersExpr{matchers: []*labels.Matcher{mustNewMatcher(labels.MatchEqual, "foo", "bar")}}, interval: 5 * time.Hour, }, operation: "rate", @@ -288,10 +294,10 @@ func TestParse(t *testing.T) { NewStringLabelFilter("30")), }, { - in: `max( sum(count_over_time({ foo !~ "bar" }[5h])) without (foo,bar) ) by (foo)`, + in: `max( sum(count_over_time({ foo = "bar" }[5h])) without (foo,bar) ) by (foo)`, exp: mustNewVectorAggregationExpr(mustNewVectorAggregationExpr(&rangeAggregationExpr{ left: &logRange{ - left: &matchersExpr{matchers: []*labels.Matcher{mustNewMatcher(labels.MatchNotRegexp, "foo", "bar")}}, + left: &matchersExpr{matchers: []*labels.Matcher{mustNewMatcher(labels.MatchEqual, "foo", "bar")}}, interval: 5 * time.Hour, }, operation: "count_over_time", @@ -304,7 +310,7 @@ func TestParse(t *testing.T) { }, nil), }, { - in: `unk({ foo !~ "bar" }[5m])`, + in: `unk({ foo = "bar" }[5m])`, err: ParseError{ msg: "syntax error: unexpected IDENTIFIER", line: 1, @@ -312,7 +318,7 @@ func TestParse(t *testing.T) { }, }, { - in: `absent_over_time({ foo !~ "bar" }[5h]) by (foo)`, + in: `absent_over_time({ foo = "bar" }[5h]) by (foo)`, err: ParseError{ msg: "grouping not allowed for absent_over_time aggregation", line: 0, @@ -320,23 +326,23 @@ func TestParse(t *testing.T) { }, }, { - in: `rate({ foo !~ "bar" }[5minutes])`, + in: `rate({ foo = "bar" }[5minutes])`, err: ParseError{ msg: `not a valid duration string: "5minutes"`, line: 0, - col: 22, + col: 21, }, }, { - in: `label_replace(rate({ foo !~ "bar" }[5m]),"")`, + in: `label_replace(rate({ foo = "bar" }[5m]),"")`, err: ParseError{ msg: `syntax error: unexpected ), expecting ,`, line: 1, - col: 44, + col: 43, }, }, { - in: `label_replace(rate({ foo !~ "bar" }[5m]),"foo","$1","bar","^^^^x43\\q")`, + in: `label_replace(rate({ foo = "bar" }[5m]),"foo","$1","bar","^^^^x43\\q")`, err: ParseError{ msg: "invalid regex in label_replace: error parsing regexp: invalid escape sequence: `\\q`", line: 0, @@ -344,23 +350,23 @@ func TestParse(t *testing.T) { }, }, { - in: `rate({ foo !~ "bar" }[5)`, + in: `rate({ foo = "bar" }[5)`, err: ParseError{ msg: "missing closing ']' in duration", line: 0, - col: 22, + col: 21, }, }, { - in: `min({ foo !~ "bar" }[5m])`, + in: `min({ foo = "bar" }[5m])`, err: ParseError{ msg: "syntax error: unexpected RANGE", line: 0, - col: 21, + col: 20, }, }, { - in: `sum(3 ,count_over_time({ foo !~ "bar" }[5h]))`, + in: `sum(3 ,count_over_time({ foo = "bar" }[5h]))`, err: ParseError{ msg: "unsupported parameter for operation sum(3,", line: 0, @@ -368,7 +374,7 @@ func TestParse(t *testing.T) { }, }, { - in: `topk(count_over_time({ foo !~ "bar" }[5h]))`, + in: `topk(count_over_time({ foo = "bar" }[5h]))`, err: ParseError{ msg: "parameter required for operation topk", line: 0, @@ -376,7 +382,7 @@ func TestParse(t *testing.T) { }, }, { - in: `bottomk(he,count_over_time({ foo !~ "bar" }[5h]))`, + in: `bottomk(he,count_over_time({ foo = "bar" }[5h]))`, err: ParseError{ msg: "syntax error: unexpected IDENTIFIER", line: 1, @@ -384,7 +390,7 @@ func TestParse(t *testing.T) { }, }, { - in: `bottomk(1.2,count_over_time({ foo !~ "bar" }[5h]))`, + in: `bottomk(1.2,count_over_time({ foo = "bar" }[5h]))`, err: ParseError{ msg: "invalid parameter bottomk(1.2,", line: 0, @@ -392,11 +398,11 @@ func TestParse(t *testing.T) { }, }, { - in: `stddev({ foo !~ "bar" })`, + in: `stddev({ foo = "bar" })`, err: ParseError{ msg: "syntax error: unexpected )", line: 1, - col: 24, + col: 23, }, }, { @@ -2171,12 +2177,12 @@ func TestParse(t *testing.T) { }, }, { - in: `count_over_time({ foo != "bar" }[12m]) > count_over_time({ foo = "bar" }[12m])`, + in: `count_over_time({ foo ="bar" }[12m]) > count_over_time({ foo = "bar" }[12m])`, exp: &binOpExpr{ op: OpTypeGT, SampleExpr: &rangeAggregationExpr{ left: &logRange{ - left: &matchersExpr{matchers: []*labels.Matcher{mustNewMatcher(labels.MatchNotEqual, "foo", "bar")}}, + left: &matchersExpr{matchers: []*labels.Matcher{mustNewMatcher(labels.MatchEqual, "foo", "bar")}}, interval: 12 * time.Minute, }, operation: "count_over_time", @@ -2191,12 +2197,12 @@ func TestParse(t *testing.T) { }, }, { - in: `count_over_time({ foo != "bar" }[12m]) > 1`, + in: `count_over_time({ foo = "bar" }[12m]) > 1`, exp: &binOpExpr{ op: OpTypeGT, SampleExpr: &rangeAggregationExpr{ left: &logRange{ - left: &matchersExpr{matchers: []*labels.Matcher{mustNewMatcher(labels.MatchNotEqual, "foo", "bar")}}, + left: &matchersExpr{matchers: []*labels.Matcher{mustNewMatcher(labels.MatchEqual, "foo", "bar")}}, interval: 12 * time.Minute, }, operation: "count_over_time", @@ -2206,18 +2212,18 @@ func TestParse(t *testing.T) { }, { // cannot compare metric & log queries - in: `count_over_time({ foo != "bar" }[12m]) > { foo = "bar" }`, + in: `count_over_time({ foo = "bar" }[12m]) > { foo = "bar" }`, err: ParseError{ msg: "unexpected type for right leg of binary operation (>): *logql.matchersExpr", }, }, { - in: `count_over_time({ foo != "bar" }[12m]) or count_over_time({ foo = "bar" }[12m]) > 1`, + in: `count_over_time({ foo = "bar" }[12m]) or count_over_time({ foo = "bar" }[12m]) > 1`, exp: &binOpExpr{ op: OpTypeOr, SampleExpr: &rangeAggregationExpr{ left: &logRange{ - left: &matchersExpr{matchers: []*labels.Matcher{mustNewMatcher(labels.MatchNotEqual, "foo", "bar")}}, + left: &matchersExpr{matchers: []*labels.Matcher{mustNewMatcher(labels.MatchEqual, "foo", "bar")}}, interval: 12 * time.Minute, }, operation: "count_over_time", @@ -2516,7 +2522,7 @@ func TestParseSampleExpr_equalityMatcher(t *testing.T) { }, { in: `count_over_time({foo!="bar"}[5m])`, - err: errors.New(errAtleastOneEqualityMatcherRequired), + err: newParseError(errAtleastOneEqualityMatcherRequired, 0, 0), }, { in: `count_over_time({app="baz", foo!="bar"}[5m])`, @@ -2526,36 +2532,36 @@ func TestParseSampleExpr_equalityMatcher(t *testing.T) { }, { in: `count_over_time({app=~".*"}[5m])`, - err: errors.New(errAtleastOneEqualityMatcherRequired), + err: newParseError(errAtleastOneEqualityMatcherRequired, 0, 0), }, { in: `count_over_time({app=~"bar|baz"}[5m])`, }, { in: `count_over_time({app!~"bar|baz"}[5m])`, - err: errors.New(errAtleastOneEqualityMatcherRequired), + err: newParseError(errAtleastOneEqualityMatcherRequired, 0, 0), }, { in: `1 + count_over_time({app=~".*"}[5m])`, - err: errors.New(errAtleastOneEqualityMatcherRequired), + err: newParseError(errAtleastOneEqualityMatcherRequired, 0, 0), }, { in: `1 + count_over_time({app=~".+"}[5m]) + count_over_time({app=~".*"}[5m])`, - err: errors.New(errAtleastOneEqualityMatcherRequired), + err: newParseError(errAtleastOneEqualityMatcherRequired, 0, 0), }, { in: `1 + count_over_time({app=~".+"}[5m]) + count_over_time({app=~".+"}[5m])`, }, { in: `1 + count_over_time({app=~".+"}[5m]) + count_over_time({app=~".*"}[5m]) + 1`, - err: errors.New(errAtleastOneEqualityMatcherRequired), + err: newParseError(errAtleastOneEqualityMatcherRequired, 0, 0), }, { in: `1 + count_over_time({app=~".+"}[5m]) + count_over_time({app=~".+"}[5m]) + 1`, }, } { t.Run(tc.in, func(t *testing.T) { - _, err := ParseSampleExpr(tc.in, true) + _, err := ParseSampleExpr(tc.in) require.Equal(t, tc.err, err) }) } @@ -2571,7 +2577,7 @@ func TestParseLogSelectorExpr_equalityMatcher(t *testing.T) { }, { in: `{foo!="bar"}`, - err: errors.New(errAtleastOneEqualityMatcherRequired), + err: newParseError(errAtleastOneEqualityMatcherRequired, 0, 0), }, { in: `{app="baz", foo!="bar"}`, @@ -2581,14 +2587,14 @@ func TestParseLogSelectorExpr_equalityMatcher(t *testing.T) { }, { in: `{app=~".*"}`, - err: errors.New(errAtleastOneEqualityMatcherRequired), + err: newParseError(errAtleastOneEqualityMatcherRequired, 0, 0), }, { in: `{foo=~"bar|baz"}`, }, { in: `{foo!~"bar|baz"}`, - err: errors.New(errAtleastOneEqualityMatcherRequired), + err: newParseError(errAtleastOneEqualityMatcherRequired, 0, 0), }, } { t.Run(tc.in, func(t *testing.T) {