Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[4189] Fix colliding filters by updating match method for MultiCharSequenceFilter to iterate through all patterns for the most complete match and not return on the first pattern found #4188

Merged
merged 9 commits into from
Feb 22, 2023
29 changes: 25 additions & 4 deletions src/metrics/filters/filter.go
Original file line number Diff line number Diff line change
Expand Up @@ -585,14 +585,35 @@ func (f *multiCharSequenceFilter) matches(val []byte) ([]byte, bool) {
return nil, false
}

var matchIndex int
var bestPattern []byte
for _, pattern := range f.patterns {
if f.backwards && bytes.HasSuffix(val, pattern) {
return val[:len(val)-len(pattern)], true
if len(pattern) > len(val) {
continue
}

if !f.backwards && bytes.HasPrefix(val, pattern) {
return val[len(pattern):], true
if f.backwards {
if bytes.HasSuffix(val, pattern) {
if bestPattern == nil || len(pattern) > len(bestPattern) {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit: seems like the bestPattern == nil isn't needed

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good catch, updated.

bestPattern = pattern
matchIndex = len(val) - len(pattern)
}
}
} else {
if bytes.HasPrefix(val, pattern) {
if bestPattern == nil || len(pattern) > len(bestPattern) {
bestPattern = pattern
matchIndex = len(pattern)
}
}
}
}

if bestPattern != nil {
if f.backwards {
return val[:matchIndex], true
}
return val[matchIndex:], true
}

return nil, false
Expand Down
5 changes: 5 additions & 0 deletions src/metrics/filters/filter_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -349,6 +349,11 @@ func TestMultiCharSequenceFilter(t *testing.T) {
validateLookup(t, f, "12test", true, "12")
validateLookup(t, f, "12test2", true, "12")
validateLookup(t, f, "123book", true, "123")

f, err = newMultiCharSequenceFilter([]byte("arachne_failures,arachne_failures_by_rack"), false)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Add a similar test case where backwards == true.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added

require.NoError(t, err)
validateLookup(t, f, "arachne_failures", true, "")
validateLookup(t, f, "arachne_failures_by_rack", true, "")
}

func validateLookup(t *testing.T, f chainFilter, val string, expectedMatch bool, expectedRemainder string) {
Expand Down