Skip to content

Commit

Permalink
chore: refactor line filter MatchType (#12388)
Browse files Browse the repository at this point in the history
  • Loading branch information
kolesnikovae authored Mar 28, 2024
1 parent 5b4bfa5 commit cc941fe
Show file tree
Hide file tree
Showing 16 changed files with 215 additions and 202 deletions.
4 changes: 2 additions & 2 deletions pkg/loghttp/params.go
Original file line number Diff line number Diff line change
Expand Up @@ -11,9 +11,9 @@ import (
"github.com/c2h5oh/datasize"
"github.com/pkg/errors"
"github.com/prometheus/common/model"
"github.com/prometheus/prometheus/model/labels"

"github.com/grafana/loki/pkg/logproto"
"github.com/grafana/loki/pkg/logql/log"
"github.com/grafana/loki/pkg/logql/syntax"
)

Expand Down Expand Up @@ -193,7 +193,7 @@ func parseRegexQuery(httpRequest *http.Request) (string, error) {
if err != nil {
return "", err
}
newExpr, err := syntax.AddFilterExpr(expr, labels.MatchRegexp, "", regexp)
newExpr, err := syntax.AddFilterExpr(expr, log.LineMatchRegexp, "", regexp)
if err != nil {
return "", err
}
Expand Down
36 changes: 31 additions & 5 deletions pkg/logql/log/filter.go
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,32 @@ import (
"github.com/grafana/loki/pkg/util"
)

// LineMatchType is an enum for line matching types.
type LineMatchType int

// Possible LineMatchTypes.
const (
LineMatchEqual LineMatchType = iota
LineMatchNotEqual
LineMatchRegexp
LineMatchNotRegexp
)

func (t LineMatchType) String() string {
switch t {
case LineMatchEqual:
return "|="
case LineMatchNotEqual:
return "!="
case LineMatchRegexp:
return "|~"
case LineMatchNotRegexp:
return "!~"
default:
return ""
}
}

// Checker is an interface that matches against the input line or regexp.
type Checker interface {
Test(line []byte, caseInsensitive bool, equal bool) bool
Expand Down Expand Up @@ -517,15 +543,15 @@ func (f containsAllFilter) Matches(test Checker) bool {
}

// NewFilter creates a new line filter from a match string and type.
func NewFilter(match string, mt labels.MatchType) (Filterer, error) {
func NewFilter(match string, mt LineMatchType) (Filterer, error) {
switch mt {
case labels.MatchRegexp:
case LineMatchRegexp:
return parseRegexpFilter(match, true, false)
case labels.MatchNotRegexp:
case LineMatchNotRegexp:
return parseRegexpFilter(match, false, false)
case labels.MatchEqual:
case LineMatchEqual:
return newContainsFilter([]byte(match), false), nil
case labels.MatchNotEqual:
case LineMatchNotEqual:
return NewNotFilter(newContainsFilter([]byte(match), false)), nil
default:
return nil, fmt.Errorf("unknown matcher: %v", match)
Expand Down
11 changes: 5 additions & 6 deletions pkg/logql/log/ip.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@ import (
"net/netip"
"unicode"

"github.com/prometheus/prometheus/model/labels"
"go4.org/netipx"
)

Expand All @@ -27,14 +26,14 @@ type IPMatcher interface{}

type IPLineFilter struct {
ip *ipFilter
ty labels.MatchType
ty LineMatchType
}

// NewIPLineFilter is used to construct ip filter as a `LineFilter`
func NewIPLineFilter(pattern string, ty labels.MatchType) (*IPLineFilter, error) {
func NewIPLineFilter(pattern string, ty LineMatchType) (*IPLineFilter, error) {
// check if `ty` supported in ip matcher.
switch ty {
case labels.MatchEqual, labels.MatchNotEqual:
case LineMatchEqual, LineMatchNotEqual:
default:
return nil, ErrIPFilterInvalidOperation
}
Expand Down Expand Up @@ -69,8 +68,8 @@ func (f *IPLineFilter) RequiredLabelNames() []string {
return []string{} // empty for line filter
}

func (f *IPLineFilter) filterTy(line []byte, ty labels.MatchType) bool {
if ty == labels.MatchNotEqual {
func (f *IPLineFilter) filterTy(line []byte, ty LineMatchType) bool {
if ty == LineMatchNotEqual {
return !f.ip.filter(line)
}
return f.ip.filter(line)
Expand Down
10 changes: 5 additions & 5 deletions pkg/logql/log/ip_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -189,7 +189,7 @@ func Test_IPLineFilterTy(t *testing.T) {
cases := []struct {
name string
pat string
ty labels.MatchType
ty LineMatchType
line []byte
expectedMatch bool

Expand All @@ -199,29 +199,29 @@ func Test_IPLineFilterTy(t *testing.T) {
{
name: "equal operator",
pat: "192.168.0.1",
ty: labels.MatchEqual,
ty: LineMatchEqual,
line: []byte("192.168.0.1"),
expectedMatch: true,
},
{
name: "not equal operator",
pat: "192.168.0.2",
ty: labels.MatchNotEqual,
ty: LineMatchNotEqual,
line: []byte("192.168.0.1"), // match because !=ip("192.168.0.2")
expectedMatch: true,
},
{
name: "regex not equal",
pat: "192.168.0.2",
ty: labels.MatchNotRegexp, // not supported
ty: LineMatchNotRegexp, // not supported
line: []byte("192.168.0.1"),
fail: true,
err: ErrIPFilterInvalidOperation,
},
{
name: "regex equal",
pat: "192.168.0.2",
ty: labels.MatchRegexp, // not supported
ty: LineMatchRegexp, // not supported
line: []byte("192.168.0.1"),
fail: true,
err: ErrIPFilterInvalidOperation,
Expand Down
4 changes: 2 additions & 2 deletions pkg/logql/log/metrics_extraction_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -346,7 +346,7 @@ func TestNewLineSampleExtractor(t *testing.T) {
require.Equal(t, 1., f)
assertLabelResult(t, lbs, l)

stage := mustFilter(NewFilter("foo", labels.MatchEqual)).ToStage()
stage := mustFilter(NewFilter("foo", LineMatchEqual)).ToStage()
se, err = NewLineSampleExtractor(BytesExtractor, []Stage{stage}, []string{"namespace"}, false, false)
require.NoError(t, err)

Expand Down Expand Up @@ -404,7 +404,7 @@ func TestNewLineSampleExtractorWithStructuredMetadata(t *testing.T) {
se, err = NewLineSampleExtractor(BytesExtractor, []Stage{
NewStringLabelFilter(labels.MustNewMatcher(labels.MatchEqual, "foo", "bar")),
NewStringLabelFilter(labels.MustNewMatcher(labels.MatchEqual, "user", "bob")),
mustFilter(NewFilter("foo", labels.MatchEqual)).ToStage(),
mustFilter(NewFilter("foo", LineMatchEqual)).ToStage(),
}, []string{"foo"}, false, false)
require.NoError(t, err)

Expand Down
10 changes: 5 additions & 5 deletions pkg/logql/log/pipeline_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -240,7 +240,7 @@ func newPipelineFilter(start, end int64, lbls, structuredMetadata labels.Labels,
stages = append(stages, s)
})

stages = append(stages, mustFilter(NewFilter(filter, labels.MatchEqual)).ToStage())
stages = append(stages, mustFilter(NewFilter(filter, LineMatchEqual)).ToStage())

return PipelineFilter{start, end, matchers, NewPipeline(stages)}
}
Expand Down Expand Up @@ -527,7 +527,7 @@ func Benchmark_Pipeline(b *testing.B) {
b.ReportAllocs()

stages := []Stage{
mustFilter(NewFilter("metrics.go", labels.MatchEqual)).ToStage(),
mustFilter(NewFilter("metrics.go", LineMatchEqual)).ToStage(),
NewLogfmtParser(false, false),
NewAndLabelFilter(
NewDurationLabelFilter(LabelFilterGreaterThan, "duration", 10*time.Millisecond),
Expand Down Expand Up @@ -611,7 +611,7 @@ func jsonBenchmark(b *testing.B, parser Stage) {
b.ReportAllocs()

p := NewPipeline([]Stage{
mustFilter(NewFilter("metrics.go", labels.MatchEqual)).ToStage(),
mustFilter(NewFilter("metrics.go", LineMatchEqual)).ToStage(),
parser,
})
line := []byte(`{"ts":"2020-12-27T09:15:54.333026285Z","error":"action could not be completed", "context":{"file": "metrics.go"}}`)
Expand Down Expand Up @@ -643,7 +643,7 @@ func invalidJSONBenchmark(b *testing.B, parser Stage) {
b.ReportAllocs()

p := NewPipeline([]Stage{
mustFilter(NewFilter("invalid json", labels.MatchEqual)).ToStage(),
mustFilter(NewFilter("invalid json", LineMatchEqual)).ToStage(),
parser,
})
line := []byte(`invalid json`)
Expand Down Expand Up @@ -696,7 +696,7 @@ func logfmtBenchmark(b *testing.B, parser Stage) {
b.ReportAllocs()

p := NewPipeline([]Stage{
mustFilter(NewFilter("ts", labels.MatchEqual)).ToStage(),
mustFilter(NewFilter("ts", LineMatchEqual)).ToStage(),
parser,
})

Expand Down
5 changes: 3 additions & 2 deletions pkg/logql/shardmapper_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import (
"github.com/prometheus/prometheus/model/labels"
"github.com/stretchr/testify/require"

"github.com/grafana/loki/pkg/logql/log"
"github.com/grafana/loki/pkg/logql/syntax"
"github.com/grafana/loki/pkg/logqlmodel"
"github.com/grafana/loki/pkg/querier/astmapper"
Expand Down Expand Up @@ -529,7 +530,7 @@ func TestMapping(t *testing.T) {
MultiStages: syntax.MultiStageExpr{
&syntax.LineFilterExpr{
LineFilter: syntax.LineFilter{
Ty: labels.MatchEqual,
Ty: log.LineMatchEqual,
Match: "error",
Op: "",
},
Expand All @@ -550,7 +551,7 @@ func TestMapping(t *testing.T) {
MultiStages: syntax.MultiStageExpr{
&syntax.LineFilterExpr{
LineFilter: syntax.LineFilter{
Ty: labels.MatchEqual,
Ty: log.LineMatchEqual,
Match: "error",
Op: "",
},
Expand Down
19 changes: 5 additions & 14 deletions pkg/logql/syntax/ast.go
Original file line number Diff line number Diff line change
Expand Up @@ -329,7 +329,7 @@ func (e *PipelineExpr) HasFilter() bool {
}

type LineFilter struct {
Ty labels.MatchType
Ty log.LineMatchType
Match string
Op string
}
Expand All @@ -342,7 +342,7 @@ type LineFilterExpr struct {
implicit
}

func newLineFilterExpr(ty labels.MatchType, op, match string) *LineFilterExpr {
func newLineFilterExpr(ty log.LineMatchType, op, match string) *LineFilterExpr {
return &LineFilterExpr{
LineFilter: LineFilter{
Ty: ty,
Expand All @@ -355,7 +355,7 @@ func newLineFilterExpr(ty labels.MatchType, op, match string) *LineFilterExpr {
func newOrLineFilter(left, right *LineFilterExpr) *LineFilterExpr {
right.Ty = left.Ty

if left.Ty == labels.MatchEqual || left.Ty == labels.MatchRegexp {
if left.Ty == log.LineMatchEqual || left.Ty == log.LineMatchRegexp {
left.Or = right
right.IsOrChild = true
return left
Expand Down Expand Up @@ -389,7 +389,7 @@ func (e *LineFilterExpr) Accept(v RootVisitor) {
}

// AddFilterExpr adds a filter expression to a logselector expression.
func AddFilterExpr(expr LogSelectorExpr, ty labels.MatchType, op, match string) (LogSelectorExpr, error) {
func AddFilterExpr(expr LogSelectorExpr, ty log.LineMatchType, op, match string) (LogSelectorExpr, error) {
filter := newLineFilterExpr(ty, op, match)
switch e := expr.(type) {
case *MatchersExpr:
Expand All @@ -412,16 +412,7 @@ func (e *LineFilterExpr) String() string {
}

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(e.Ty.String())
sb.WriteString(" ")
}

Expand Down
12 changes: 6 additions & 6 deletions pkg/logql/syntax/ast_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -449,16 +449,16 @@ func Test_FilterMatcher(t *testing.T) {

func TestOrLineFilterTypes(t *testing.T) {
for _, tt := range []struct {
ty labels.MatchType
ty log.LineMatchType
}{
{labels.MatchEqual},
{labels.MatchNotEqual},
{labels.MatchRegexp},
{labels.MatchNotRegexp},
{log.LineMatchEqual},
{log.LineMatchNotEqual},
{log.LineMatchRegexp},
{log.LineMatchNotRegexp},
} {
t.Run("right inherits left's type", func(t *testing.T) {
left := &LineFilterExpr{LineFilter: LineFilter{Ty: tt.ty, Match: "something"}}
right := &LineFilterExpr{LineFilter: LineFilter{Ty: labels.MatchEqual, Match: "something"}}
right := &LineFilterExpr{LineFilter: LineFilter{Ty: log.LineMatchEqual, Match: "something"}}

_ = newOrLineFilter(left, right)
require.Equal(t, tt.ty, right.Ty)
Expand Down
16 changes: 8 additions & 8 deletions pkg/logql/syntax/expr.y
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ import (

%union{
Expr Expr
Filter labels.MatchType
Filter log.LineMatchType
Grouping *Grouping
Labels []string
LogExpr LogSelectorExpr
Expand Down Expand Up @@ -239,10 +239,10 @@ labelReplaceExpr:
;

filter:
PIPE_MATCH { $$ = labels.MatchRegexp }
| PIPE_EXACT { $$ = labels.MatchEqual }
| NRE { $$ = labels.MatchNotRegexp }
| NEQ { $$ = labels.MatchNotEqual }
PIPE_MATCH { $$ = log.LineMatchRegexp }
| PIPE_EXACT { $$ = log.LineMatchEqual }
| NRE { $$ = log.LineMatchNotRegexp }
| NEQ { $$ = log.LineMatchNotEqual }
;

selector:
Expand Down Expand Up @@ -287,9 +287,9 @@ filterOp:
;

orFilter:
STRING { $$ = newLineFilterExpr(labels.MatchEqual, "", $1) }
| filterOp OPEN_PARENTHESIS STRING CLOSE_PARENTHESIS { $$ = newLineFilterExpr(labels.MatchEqual, $1, $3) }
| STRING OR orFilter { $$ = newOrLineFilter(newLineFilterExpr(labels.MatchEqual, "", $1), $3) }
STRING { $$ = newLineFilterExpr(log.LineMatchEqual, "", $1) }
| filterOp OPEN_PARENTHESIS STRING CLOSE_PARENTHESIS { $$ = newLineFilterExpr(log.LineMatchEqual, $1, $3) }
| STRING OR orFilter { $$ = newOrLineFilter(newLineFilterExpr(log.LineMatchEqual, "", $1), $3) }
;

lineFilter:
Expand Down
Loading

0 comments on commit cc941fe

Please sign in to comment.