Fix `or` filter not being stringified correctly (#11337)

**What this PR does / why we need it**:

This PR adds a missing `String()` call for the `or` statement in line
filters. It also fixes a bug where `or` statements were not applied in
conjunction with an IPLineFilter expression.

**Which issue(s) this PR fixes**:
Fixes #<issue number>

**Special notes for your reviewer**:

**Checklist**
- [ ] Reviewed the
[`CONTRIBUTING.md`](https://github.com/grafana/loki/blob/main/CONTRIBUTING.md)
guide (**required**)
- [ ] Documentation added
- [ ] Tests updated
- [ ] `CHANGELOG.md` updated
- [ ] If the change is worth mentioning in the release notes, add
`add-to-release-notes` label
- [ ] Changes that require user attention or interaction to upgrade are
documented in `docs/sources/setup/upgrade/_index.md`
- [ ] For Helm chart changes bump the Helm chart version in
`production/helm/loki/Chart.yaml` and update
`production/helm/loki/CHANGELOG.md` and
`production/helm/loki/README.md`. [Example
PR](d10549e3ec)
- [ ] If the change is deprecating or removing a configuration option,
update the `deprecated-config.yaml` and `deleted-config.yaml` files
respectively in the `tools/deprecated-config-checker` directory.
[Example
PR](0d4416a4b0)

---------

Co-authored-by: Travis Patterson <travis.patterson@grafana.com>
pull/11279/head
Sven Grossmann 1 year ago committed by GitHub
parent 5270d10a7e
commit 1496eb4752
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
  1. 86
      pkg/logql/syntax/ast.go
  2. 50
      pkg/logql/syntax/ast_test.go

@ -307,11 +307,12 @@ func (e *PipelineExpr) HasFilter() bool {
}
type LineFilterExpr struct {
Left *LineFilterExpr
Or *LineFilterExpr
Ty labels.MatchType
Match string
Op string
Left *LineFilterExpr
Or *LineFilterExpr
IsOrChild bool
Ty labels.MatchType
Match string
Op string
implicit
}
@ -328,6 +329,7 @@ func newOrLineFilter(left, right *LineFilterExpr) *LineFilterExpr {
if left.Ty == labels.MatchEqual || left.Ty == labels.MatchRegexp {
left.Or = right
right.IsOrChild = true
return left
}
@ -380,52 +382,66 @@ func (e *LineFilterExpr) String() string {
sb.WriteString(e.Left.String())
sb.WriteString(" ")
}
switch e.Ty {
case labels.MatchRegexp:
sb.WriteString("|~")
case labels.MatchNotRegexp:
sb.WriteString("!~")
case labels.MatchEqual:
sb.WriteString("|=")
case labels.MatchNotEqual:
sb.WriteString("!=")
if !e.IsOrChild { // Only write the type when we're not chaining "or" filters
switch e.Ty {
case labels.MatchRegexp:
sb.WriteString("|~")
case labels.MatchNotRegexp:
sb.WriteString("!~")
case labels.MatchEqual:
sb.WriteString("|=")
case labels.MatchNotEqual:
sb.WriteString("!=")
}
sb.WriteString(" ")
}
sb.WriteString(" ")
if e.Op == "" {
sb.WriteString(strconv.Quote(e.Match))
return sb.String()
} else {
sb.WriteString(e.Op)
sb.WriteString("(")
sb.WriteString(strconv.Quote(e.Match))
sb.WriteString(")")
}
sb.WriteString(e.Op)
sb.WriteString("(")
sb.WriteString(strconv.Quote(e.Match))
sb.WriteString(")")
if e.Or != nil {
sb.WriteString(" or ")
// This is dirty but removes the leading MatchType from the or expression.
sb.WriteString(e.Or.String())
}
return sb.String()
}
func (e *LineFilterExpr) Filter() (log.Filterer, error) {
acc := make([]log.Filterer, 0)
for curr := e; curr != nil; curr = curr.Left {
switch curr.Op {
case OpFilterIP:
var err error
next, err := log.NewIPLineFilter(curr.Match, curr.Ty)
var next log.Filterer
var err error
if curr.Or != nil {
next, err = newOrFilter(curr)
if err != nil {
return nil, err
}
acc = append(acc, next)
default:
var next log.Filterer
var err error
if curr.Or != nil {
next, err = newOrFilter(curr)
} else {
} else {
switch curr.Op {
case OpFilterIP:
next, err := log.NewIPLineFilter(curr.Match, curr.Ty)
if err != nil {
return nil, err
}
acc = append(acc, next)
default:
next, err = log.NewFilter(curr.Match, curr.Ty)
}
if err != nil {
return nil, err
}
if err != nil {
return nil, err
}
acc = append(acc, next)
acc = append(acc, next)
}
}
}

@ -404,6 +404,20 @@ func Test_FilterMatcher(t *testing.T) {
},
[]linecheck{{"foo", false}, {"bar", false}, {"none", true}},
},
{
`{app="foo"} |= ip("127.0.0.1") or "foo"`,
[]*labels.Matcher{
mustNewMatcher(labels.MatchEqual, "app", "foo"),
},
[]linecheck{{"foo", true}, {"bar", false}, {"127.0.0.2", false}, {"127.0.0.1", true}},
},
{
`{app="foo"} != ip("127.0.0.1") or "foo"`,
[]*labels.Matcher{
mustNewMatcher(labels.MatchEqual, "app", "foo"),
},
[]linecheck{{"foo", false}, {"bar", true}, {"127.0.0.2", true}, {"127.0.0.1", false}},
},
} {
tt := tt
t.Run(tt.q, func(t *testing.T) {
@ -474,6 +488,42 @@ func TestStringer(t *testing.T) {
in: `0 > count_over_time({foo="bar"}[1m])`,
out: `(0 > count_over_time({foo="bar"}[1m]))`,
},
{
in: `{app="foo"} |= "foo" or "bar"`,
out: `{app="foo"} |= "foo" or "bar"`,
},
{
in: `{app="foo"} |~ "foo" or "bar" or "baz"`,
out: `{app="foo"} |~ "foo" or "bar" or "baz"`,
},
{
in: `{app="foo"} |= ip("127.0.0.1") or "foo"`,
out: `{app="foo"} |= ip("127.0.0.1") or "foo"`,
},
{
in: `{app="foo"} |= "foo" or ip("127.0.0.1")`,
out: `{app="foo"} |= "foo" or ip("127.0.0.1")`,
},
{
in: `{app="foo"} |~ ip("127.0.0.1") or "foo"`,
out: `{app="foo"} |~ ip("127.0.0.1") or "foo"`,
},
{ // !(A || B) == !A && !B
in: `{app="foo"} != "foo" or "bar"`,
out: `{app="foo"} != "foo" != "bar"`,
},
{
in: `{app="foo"} !~ "foo" or "bar"`,
out: `{app="foo"} !~ "foo" !~ "bar"`,
},
{
in: `{app="foo"} != ip("127.0.0.1") or "foo"`,
out: `{app="foo"} != ip("127.0.0.1") != "foo"`,
},
{
in: `{app="foo"} !~ ip("127.0.0.1") or "foo"`,
out: `{app="foo"} !~ ip("127.0.0.1") !~ "foo"`,
},
} {
t.Run(tc.in, func(t *testing.T) {
expr, err := ParseExpr(tc.in)

Loading…
Cancel
Save