From 1b39bcfc5c1b1f1a767e26c094dc879bf903edb6 Mon Sep 17 00:00:00 2001 From: Rob Skillington Date: Thu, 23 Mar 2017 19:12:35 -0400 Subject: [PATCH 1/2] Replace strings with byte slices for filters --- filters/filter.go | 87 ++++++++++++++++---------------- filters/filter_benchmark_test.go | 72 ++++++++++++++------------ filters/filter_test.go | 30 +++++------ filters/mock_filter.go | 14 ++--- filters/tags_filter.go | 26 +++++----- filters/tags_filter_test.go | 6 +-- rules/rule.go | 8 +-- rules/rule_test.go | 6 +-- 8 files changed, 129 insertions(+), 120 deletions(-) diff --git a/filters/filter.go b/filters/filter.go index f588000..de2baeb 100644 --- a/filters/filter.go +++ b/filters/filter.go @@ -24,7 +24,6 @@ import ( "bytes" "errors" "fmt" - "strings" ) var ( @@ -65,13 +64,13 @@ type Filter interface { fmt.Stringer // Matches returns true if the conditions are met - Matches(val string) bool + Matches(val []byte) bool } // NewFilter supports startsWith, endsWith, contains and a single wildcard // along with negation and glob matching support // TODO(martinm): Add variable length glob ranges -func NewFilter(pattern string) (Filter, error) { +func NewFilter(pattern []byte) (Filter, error) { if len(pattern) == 0 { return newEqualityFilter(pattern), nil } @@ -95,8 +94,10 @@ func NewFilter(pattern string) (Filter, error) { // newWildcardFilter creates a filter that segments the pattern based // on wildcards, creating a rangeFilter for each segment -func newWildcardFilter(pattern string) (Filter, error) { - wIdx := strings.IndexRune(pattern, wildcardChar) +func newWildcardFilter(pattern []byte) (Filter, error) { + wIdx := bytes.IndexRune(pattern, wildcardChar) + + // TODO: use utf8 to take into account size of rune in byte slice if wIdx == -1 { // No wildcards @@ -113,7 +114,7 @@ func newWildcardFilter(pattern string) (Filter, error) { return newRangeFilter(pattern[:len(pattern)-1], false, start) } - secondWIdx := strings.IndexRune(pattern[wIdx+1:], wildcardChar) + secondWIdx := bytes.IndexRune(pattern[wIdx+1:], wildcardChar) if secondWIdx == -1 { if wIdx == 0 { // Single wildcard at start @@ -144,7 +145,7 @@ func newWildcardFilter(pattern string) (Filter, error) { // newRangeFilter creates a filter that checks for ranges (? or [] or {}) and segments // the pattern into a multiple chain filters based on ranges found -func newRangeFilter(pattern string, backwards bool, seg chainSegment) (Filter, error) { +func newRangeFilter(pattern []byte, backwards bool, seg chainSegment) (Filter, error) { var filters []chainFilter eqIdx := -1 for i := 0; i < len(pattern); i++ { @@ -156,7 +157,7 @@ func newRangeFilter(pattern string, backwards bool, seg chainSegment) (Filter, e eqIdx = -1 } - endIdx := strings.IndexRune(pattern[i+1:], singleRangeEndChar) + endIdx := bytes.IndexRune(pattern[i+1:], singleRangeEndChar) if endIdx == -1 { return nil, errInvalidFilterPattern } @@ -195,32 +196,32 @@ type allowFilter struct{} func newAllowFilter() Filter { return allowAllFilter } func (f allowFilter) String() string { return "All" } -func (f allowFilter) Matches(val string) bool { return true } +func (f allowFilter) Matches(val []byte) bool { return true } // equalityFilter is a filter that matches exact values type equalityFilter struct { - pattern string + pattern []byte } -func newEqualityFilter(pattern string) Filter { +func newEqualityFilter(pattern []byte) Filter { return &equalityFilter{pattern: pattern} } func (f *equalityFilter) String() string { - return "Equals(\"" + f.pattern + "\")" + return "Equals(\"" + string(f.pattern) + "\")" } -func (f *equalityFilter) Matches(val string) bool { - return f.pattern == val +func (f *equalityFilter) Matches(val []byte) bool { + return bytes.Equal(f.pattern, val) } // containsFilter is a filter that performs contains matches type containsFilter struct { - pattern string + pattern []byte } -func newContainsFilter(pattern string) (Filter, error) { - if strings.ContainsAny(pattern, invalidNestedChars) { +func newContainsFilter(pattern []byte) (Filter, error) { + if bytes.ContainsAny(pattern, invalidNestedChars) { return nil, errInvalidFilterPattern } @@ -228,11 +229,11 @@ func newContainsFilter(pattern string) (Filter, error) { } func (f *containsFilter) String() string { - return "Contains(\"" + f.pattern + "\")" + return "Contains(\"" + string(f.pattern) + "\")" } -func (f *containsFilter) Matches(val string) bool { - return strings.Contains(val, f.pattern) +func (f *containsFilter) Matches(val []byte) bool { + return bytes.Contains(val, f.pattern) } // negationFilter is a filter that matches the opposite of the provided filter @@ -248,7 +249,7 @@ func (f *negationFilter) String() string { return "Not(" + f.filter.String() + ")" } -func (f *negationFilter) Matches(val string) bool { +func (f *negationFilter) Matches(val []byte) bool { return !f.filter.Matches(val) } @@ -277,7 +278,7 @@ func (f *multiFilter) String() string { return buf.String() } -func (f *multiFilter) Matches(val string) bool { +func (f *multiFilter) Matches(val []byte) bool { if len(f.filters) == 0 { return true } @@ -301,34 +302,34 @@ func (f *multiFilter) Matches(val string) bool { type chainFilter interface { fmt.Stringer - matches(val string) (string, bool) + matches(val []byte) ([]byte, bool) } // equalityChainFilter is a filter that performs equality string matches // from either the front or back of the string type equalityChainFilter struct { - pattern string + pattern []byte backwards bool } -func newEqualityChainFilter(pattern string, backwards bool) chainFilter { +func newEqualityChainFilter(pattern []byte, backwards bool) chainFilter { return &equalityChainFilter{pattern: pattern, backwards: backwards} } func (f *equalityChainFilter) String() string { - return "Equals(\"" + f.pattern + "\")" + return "Equals(\"" + string(f.pattern) + "\")" } -func (f *equalityChainFilter) matches(val string) (string, bool) { - if f.backwards && strings.HasSuffix(val, f.pattern) { +func (f *equalityChainFilter) matches(val []byte) ([]byte, bool) { + if f.backwards && bytes.HasSuffix(val, f.pattern) { return val[:len(val)-len(f.pattern)], true } - if !f.backwards && strings.HasPrefix(val, f.pattern) { + if !f.backwards && bytes.HasPrefix(val, f.pattern) { return val[len(f.pattern):], true } - return "", false + return nil, false } // singleAnyCharFilter is a filter that allows any one char @@ -346,9 +347,9 @@ func newSingleAnyCharFilter(backwards bool) chainFilter { func (f *singleAnyCharFilter) String() string { return "AnyChar" } -func (f *singleAnyCharFilter) matches(val string) (string, bool) { +func (f *singleAnyCharFilter) matches(val []byte) ([]byte, bool) { if len(val) == 0 { - return "", false + return nil, false } if f.backwards { @@ -360,7 +361,7 @@ func (f *singleAnyCharFilter) matches(val string) (string, bool) { // newSingleRangeFilter creates a filter that performs range matching // on a single char -func newSingleRangeFilter(pattern string, backwards bool) (chainFilter, error) { +func newSingleRangeFilter(pattern []byte, backwards bool) (chainFilter, error) { if len(pattern) == 0 { return nil, errInvalidFilterPattern } @@ -382,20 +383,20 @@ func newSingleRangeFilter(pattern string, backwards bool) (chainFilter, error) { return &singleCharSetFilter{pattern: pattern, backwards: backwards, negate: negate}, nil } -func genSingleRangeFilterStr(pattern string, negate bool) string { +func genSingleRangeFilterStr(pattern []byte, negate bool) string { var negatePrefix, negateSuffix string if negate { negatePrefix = "Not(" negateSuffix = ")" } - return negatePrefix + "Range(\"" + pattern + "\")" + negateSuffix + return negatePrefix + "Range(\"" + string(pattern) + "\")" + negateSuffix } // singleRangeFilter is a filter that performs a single character match against // a range of chars given in a range format eg. [a-z] type singleRangeFilter struct { - pattern string + pattern []byte backwards bool negate bool } @@ -404,9 +405,9 @@ func (f *singleRangeFilter) String() string { return genSingleRangeFilterStr(f.pattern, f.negate) } -func (f *singleRangeFilter) matches(val string) (string, bool) { +func (f *singleRangeFilter) matches(val []byte) ([]byte, bool) { if len(val) == 0 { - return "", false + return nil, false } idx := 0 @@ -427,7 +428,7 @@ func (f *singleRangeFilter) matches(val string) (string, bool) { // singleCharSetFilter is a filter that performs a single character match against // a set of chars given explicity eg. [abcdefg] type singleCharSetFilter struct { - pattern string + pattern []byte backwards bool negate bool } @@ -436,9 +437,9 @@ func (f *singleCharSetFilter) String() string { return genSingleRangeFilterStr(f.pattern, f.negate) } -func (f *singleCharSetFilter) matches(val string) (string, bool) { +func (f *singleCharSetFilter) matches(val []byte) ([]byte, bool) { if len(val) == 0 { - return "", false + return nil, false } match := false @@ -503,7 +504,7 @@ func (f *multiChainFilter) String() string { return buf.String() } -func (f *multiChainFilter) Matches(val string) bool { +func (f *multiChainFilter) Matches(val []byte) bool { if len(f.filters) == 0 { return true } @@ -526,7 +527,7 @@ func (f *multiChainFilter) Matches(val string) bool { } } - if f.seg == middle && val != "" { + if f.seg == middle && len(val) != 0 { // chain was middle segment and some value was left over at end of chain return false } diff --git a/filters/filter_benchmark_test.go b/filters/filter_benchmark_test.go index 8560c1e..963dc5f 100644 --- a/filters/filter_benchmark_test.go +++ b/filters/filter_benchmark_test.go @@ -21,12 +21,13 @@ package filters import ( + "bytes" "fmt" "testing" ) var ( - testFlatID = "tagname1=tagvalue1,tagname3=tagvalue3,tagname4=tagvalue4,tagname2=tagvalue2,tagname6=tagvalue6,tagname5=tagvalue5,name=my.test.metric.name,tagname7=tagvalue7" + testFlatID = []byte("tagname1=tagvalue1,tagname3=tagvalue3,tagname4=tagvalue4,tagname2=tagvalue2,tagname6=tagvalue6,tagname5=tagvalue5,name=my.test.metric.name,tagname7=tagvalue7") testTagsFilterMapOne = map[string]string{ "tagname1": "tagvalue1", } @@ -40,20 +41,21 @@ var ( ) func BenchmarkEquityFilter(b *testing.B) { - f1 := newEqualityFilter("test") - f2 := newEqualityFilter("test2") + f1 := newEqualityFilter([]byte("test")) + f2 := newEqualityFilter([]byte("test2")) for n := 0; n < b.N; n++ { - testUnionFilter("test", []Filter{f1, f2}) + testUnionFilter([]byte("test"), []Filter{f1, f2}) } } func BenchmarkEquityFilterByValue(b *testing.B) { - f1 := newTestEqualityFilter("test") - f2 := newTestEqualityFilter("test2") + f1 := newTestEqualityFilter([]byte("test")) + f2 := newTestEqualityFilter([]byte("test2")) + test := []byte("test") for n := 0; n < b.N; n++ { - testUnionFilter("test", []Filter{f1, f2}) + testUnionFilter(test, []Filter{f1, f2}) } } @@ -76,46 +78,46 @@ func BenchmarkMapTagsFilterThree(b *testing.B) { } func BenchmarkRangeFilterStructsMatchRange(b *testing.B) { - benchRangeFilterStructs(b, "a-z", "p", true) + benchRangeFilterStructs(b, []byte("a-z"), []byte("p"), true) } func BenchmarkRangeFilterRangeMatchRange(b *testing.B) { - benchRangeFilterRange(b, "a-z", "p", true) + benchRangeFilterRange(b, []byte("a-z"), []byte("p"), true) } func BenchmarkRangeFilterStructsNotMatchRange(b *testing.B) { - benchRangeFilterStructs(b, "a-z", "P", false) + benchRangeFilterStructs(b, []byte("a-z"), []byte("P"), false) } func BenchmarkRangeFilterRangeNotMatchRange(b *testing.B) { - benchRangeFilterRange(b, "a-z", "P", false) + benchRangeFilterRange(b, []byte("a-z"), []byte("P"), false) } func BenchmarkRangeFilterStructsMatch(b *testing.B) { - benchRangeFilterStructs(b, "02468", "6", true) + benchRangeFilterStructs(b, []byte("02468"), []byte("6"), true) } func BenchmarkRangeFilterRangeMatch(b *testing.B) { - benchRangeFilterRange(b, "02468", "6", true) + benchRangeFilterRange(b, []byte("02468"), []byte("6"), true) } func BenchmarkRangeFilterStructsNotMatch(b *testing.B) { - benchRangeFilterStructs(b, "13579", "6", false) + benchRangeFilterStructs(b, []byte("13579"), []byte("6"), false) } func BenchmarkRangeFilterRangeNotMatch(b *testing.B) { - benchRangeFilterRange(b, "13579", "6", false) + benchRangeFilterRange(b, []byte("13579"), []byte("6"), false) } func BenchmarkRangeFilterStructsMatchNegation(b *testing.B) { - benchRangeFilterStructs(b, "!a-z", "p", false) + benchRangeFilterStructs(b, []byte("!a-z"), []byte("p"), false) } func BenchmarkRangeFilterRangeMatchNegation(b *testing.B) { - benchRangeFilterRange(b, "!a-z", "p", false) + benchRangeFilterRange(b, []byte("!a-z"), []byte("p"), false) } -func benchRangeFilterStructs(b *testing.B, pattern string, val string, expectedMatch bool) { +func benchRangeFilterStructs(b *testing.B, pattern []byte, val []byte, expectedMatch bool) { f, _ := newSingleRangeFilter(pattern, false) for n := 0; n < b.N; n++ { _, match := f.matches(val) @@ -125,7 +127,7 @@ func benchRangeFilterStructs(b *testing.B, pattern string, val string, expectedM } } -func benchRangeFilterRange(b *testing.B, pattern string, val string, expectedMatch bool) { +func benchRangeFilterRange(b *testing.B, pattern []byte, val []byte, expectedMatch bool) { for n := 0; n < b.N; n++ { match, _ := validateRangeByScan(pattern, val) if match != expectedMatch { @@ -134,13 +136,13 @@ func benchRangeFilterRange(b *testing.B, pattern string, val string, expectedMat } } -func benchTagsFilter(b *testing.B, id string, tagsFilter Filter) { +func benchTagsFilter(b *testing.B, id []byte, tagsFilter Filter) { for n := 0; n < b.N; n++ { tagsFilter.Matches(id) } } -func testUnionFilter(val string, filters []Filter) bool { +func testUnionFilter(val []byte, filters []Filter) bool { for _, filter := range filters { if !filter.Matches(val) { return false @@ -151,19 +153,19 @@ func testUnionFilter(val string, filters []Filter) bool { } type testEqualityFilter struct { - pattern string + pattern []byte } -func newTestEqualityFilter(pattern string) Filter { +func newTestEqualityFilter(pattern []byte) Filter { return testEqualityFilter{pattern: pattern} } func (f testEqualityFilter) String() string { - return fmt.Sprintf("Equals(%q)", f.pattern) + return fmt.Sprintf("Equals(%s)", string(f.pattern)) } -func (f testEqualityFilter) Matches(id string) bool { - return f.pattern == id +func (f testEqualityFilter) Matches(id []byte) bool { + return bytes.Equal(f.pattern, id) } type testMapTagsFilter struct { @@ -174,7 +176,7 @@ type testMapTagsFilter struct { func newTestMapTagsFilter(tagFilters map[string]string, iterFn NewSortedTagIteratorFn) Filter { filters := make(map[string]Filter, len(tagFilters)) for name, value := range tagFilters { - filter, _ := NewFilter(value) + filter, _ := NewFilter([]byte(value)) filters[name] = filter } @@ -188,7 +190,7 @@ func (f *testMapTagsFilter) String() string { return "" } -func (f *testMapTagsFilter) Matches(id string) bool { +func (f *testMapTagsFilter) Matches(id []byte) bool { if len(f.filters) == 0 { return true } @@ -200,13 +202,17 @@ func (f *testMapTagsFilter) Matches(id string) bool { for iter.Next() { name, value := iter.Current() - if filter, exists := f.filters[name]; exists { + for n, filter := range f.filters { + if !bytes.Equal([]byte(n), name) { + continue + } + if filter.Matches(value) { matches++ if matches == len(f.filters) { return true } - continue + break } return false @@ -216,13 +222,13 @@ func (f *testMapTagsFilter) Matches(id string) bool { return iter.Err() == nil && matches == len(f.filters) } -func validateRangeByScan(pattern string, val string) (bool, error) { +func validateRangeByScan(pattern []byte, val []byte) (bool, error) { if len(pattern) == 0 { return false, errInvalidFilterPattern } negate := false - if pattern[0] == negationChar { + if rune(pattern[0]) == negationChar { pattern = pattern[1:] if len(pattern) == 0 { return false, errInvalidFilterPattern @@ -230,7 +236,7 @@ func validateRangeByScan(pattern string, val string) (bool, error) { negate = true } - if len(pattern) == 3 && pattern[1] == rangeChar { + if len(pattern) == 3 && rune(pattern[1]) == rangeChar { if pattern[0] >= pattern[2] { return false, errInvalidFilterPattern } diff --git a/filters/filter_test.go b/filters/filter_test.go index 12b07ce..490b051 100644 --- a/filters/filter_test.go +++ b/filters/filter_test.go @@ -57,18 +57,18 @@ func TestEqualityFilter(t *testing.T) { {val: "fo", match: false}, {val: "foob", match: false}, } - f := newEqualityFilter("foo") + f := newEqualityFilter([]byte("foo")) for _, input := range inputs { - require.Equal(t, input.match, f.Matches(input.val)) + require.Equal(t, input.match, f.Matches([]byte(input.val))) } } func TestEmptyFilter(t *testing.T) { - f, err := NewFilter("") + f, err := NewFilter(nil) require.NoError(t, err) - require.True(t, f.Matches("")) - require.False(t, f.Matches(" ")) - require.False(t, f.Matches("foo")) + require.True(t, f.Matches([]byte(""))) + require.False(t, f.Matches([]byte(" "))) + require.False(t, f.Matches([]byte("foo"))) } func TestWildcardFilters(t *testing.T) { @@ -134,14 +134,14 @@ func TestRangeFilters(t *testing.T) { } func TestMultiFilter(t *testing.T) { - cf, _ := newContainsFilter("bar") + cf, _ := newContainsFilter([]byte("bar")) filters := []Filter{ NewMultiFilter([]Filter{}, Conjunction), NewMultiFilter([]Filter{}, Disjunction), - NewMultiFilter([]Filter{newEqualityFilter("foo")}, Conjunction), - NewMultiFilter([]Filter{newEqualityFilter("foo")}, Disjunction), - NewMultiFilter([]Filter{newEqualityFilter("foo"), cf}, Conjunction), - NewMultiFilter([]Filter{newEqualityFilter("foo"), cf}, Disjunction), + NewMultiFilter([]Filter{newEqualityFilter([]byte("foo"))}, Conjunction), + NewMultiFilter([]Filter{newEqualityFilter([]byte("foo"))}, Disjunction), + NewMultiFilter([]Filter{newEqualityFilter([]byte("foo")), cf}, Conjunction), + NewMultiFilter([]Filter{newEqualityFilter([]byte("foo")), cf}, Disjunction), } inputs := []testInput{ @@ -206,7 +206,7 @@ func TestBadPatterns(t *testing.T) { } for _, pattern := range patterns { - _, err := NewFilter(pattern) + _, err := NewFilter([]byte(pattern)) require.Error(t, err, fmt.Sprintf("pattern: %s", pattern)) } } @@ -217,19 +217,19 @@ type testPattern struct { } type testInput struct { - val string + val []byte matches []bool } func newTestInput(val string, matches ...bool) testInput { - return testInput{val: val, matches: matches} + return testInput{val: []byte(val), matches: matches} } func genAndValidateFilters(t *testing.T, patterns []testPattern) []Filter { var err error filters := make([]Filter, len(patterns)) for i, pattern := range patterns { - filters[i], err = NewFilter(pattern.pattern) + filters[i], err = NewFilter([]byte(pattern.pattern)) require.NoError(t, err, fmt.Sprintf("No error expected, but got: %v for pattern: %s", err, pattern.pattern)) require.Equal(t, pattern.expectedStr, filters[i].String()) } diff --git a/filters/mock_filter.go b/filters/mock_filter.go index 3253064..d7a46ce 100644 --- a/filters/mock_filter.go +++ b/filters/mock_filter.go @@ -35,8 +35,8 @@ type mockFilterData struct { } type mockTagPair struct { - name string - value string + name []byte + value []byte } type mockSortedTagIterator struct { @@ -45,18 +45,18 @@ type mockSortedTagIterator struct { pairs []mockTagPair } -func idToMockTagPairs(id string) []mockTagPair { - tagPairs := strings.Split(id, mockTagPairSeparator) +func idToMockTagPairs(id []byte) []mockTagPair { + tagPairs := strings.Split(string(id), mockTagPairSeparator) var pairs []mockTagPair for _, pair := range tagPairs { p := strings.Split(pair, mockTagValueSeparator) - pairs = append(pairs, mockTagPair{name: p[0], value: p[1]}) + pairs = append(pairs, mockTagPair{name: []byte(p[0]), value: []byte(p[1])}) } return pairs } // NewMockSortedTagIterator creates a mock SortedTagIterator based on given ID -func NewMockSortedTagIterator(id string) SortedTagIterator { +func NewMockSortedTagIterator(id []byte) SortedTagIterator { pairs := idToMockTagPairs(id) return &mockSortedTagIterator{idx: -1, pairs: pairs} } @@ -69,7 +69,7 @@ func (it *mockSortedTagIterator) Next() bool { return it.err == nil && it.idx < len(it.pairs) } -func (it *mockSortedTagIterator) Current() (string, string) { +func (it *mockSortedTagIterator) Current() ([]byte, []byte) { return it.pairs[it.idx].name, it.pairs[it.idx].value } diff --git a/filters/tags_filter.go b/filters/tags_filter.go index 8e46d55..e7c8e29 100644 --- a/filters/tags_filter.go +++ b/filters/tags_filter.go @@ -33,7 +33,7 @@ type SortedTagIterator interface { Next() bool // Current returns the current tag name and value - Current() (string, string) + Current() ([]byte, []byte) // Err returns any errors encountered Err() error @@ -43,23 +43,23 @@ type SortedTagIterator interface { } // NewSortedTagIteratorFn creates a tag iterator given an id -type NewSortedTagIteratorFn func(id string) SortedTagIterator +type NewSortedTagIteratorFn func(id []byte) SortedTagIterator // tagFilter is a filter associated with a given tag type tagFilter struct { - name string + name []byte valueFilter Filter } func (f *tagFilter) String() string { - return fmt.Sprintf("%s:%s", f.name, f.valueFilter.String()) + return fmt.Sprintf("%s:%s", string(f.name), f.valueFilter.String()) } type tagFiltersByNameAsc []tagFilter func (tn tagFiltersByNameAsc) Len() int { return len(tn) } func (tn tagFiltersByNameAsc) Swap(i, j int) { tn[i], tn[j] = tn[j], tn[i] } -func (tn tagFiltersByNameAsc) Less(i, j int) bool { return tn[i].name < tn[j].name } +func (tn tagFiltersByNameAsc) Less(i, j int) bool { return bytes.Compare(tn[i].name, tn[j].name) < 0 } // tagsFilter contains a list of tag filters. type tagsFilter struct { @@ -72,13 +72,13 @@ type tagsFilter struct { func NewTagsFilter(tagFilters map[string]string, iterFn NewSortedTagIteratorFn, op LogicalOp) (Filter, error) { filters := make([]tagFilter, 0, len(tagFilters)) for name, value := range tagFilters { - valFilter, err := NewFilter(value) + valFilter, err := NewFilter([]byte(value)) if err != nil { return nil, err } filters = append(filters, tagFilter{ - name: name, + name: []byte(name), valueFilter: valFilter, }) } @@ -103,7 +103,7 @@ func (f *tagsFilter) String() string { return buf.String() } -func (f *tagsFilter) Matches(id string) bool { +func (f *tagsFilter) Matches(id []byte) bool { if len(f.filters) == 0 { return true } @@ -113,11 +113,13 @@ func (f *tagsFilter) Matches(id string) bool { currIdx := 0 for iter.Next() && currIdx < len(f.filters) { name, value := iter.Current() - if name < f.filters[currIdx].name { + + comparison := bytes.Compare(name, f.filters[currIdx].name) + if comparison < 0 { continue } - if name > f.filters[currIdx].name { + if comparison > 0 { if f.op == Conjunction { // For AND, if the current filter tag doesn't exist, bail immediately return false @@ -125,7 +127,7 @@ func (f *tagsFilter) Matches(id string) bool { // Iterate filters for the OR case currIdx++ - for currIdx < len(f.filters) && name > f.filters[currIdx].name { + for currIdx < len(f.filters) && bytes.Compare(name, f.filters[currIdx].name) > 0 { currIdx++ } @@ -134,7 +136,7 @@ func (f *tagsFilter) Matches(id string) bool { return false } - if name < f.filters[currIdx].name { + if bytes.Compare(name, f.filters[currIdx].name) < 0 { continue } } diff --git a/filters/tags_filter_test.go b/filters/tags_filter_test.go index 6e3d1b7..6bd6875 100644 --- a/filters/tags_filter_test.go +++ b/filters/tags_filter_test.go @@ -29,7 +29,7 @@ import ( func TestEmptyTagsFilterMatches(t *testing.T) { f, err := NewTagsFilter(nil, NewMockSortedTagIterator, Conjunction) require.NoError(t, err) - require.True(t, f.Matches("foo")) + require.True(t, f.Matches([]byte("foo"))) } func TestTagsFilterMatches(t *testing.T) { @@ -47,7 +47,7 @@ func TestTagsFilterMatches(t *testing.T) { } require.NoError(t, err) for _, input := range inputs { - require.Equal(t, input.match, f.Matches(input.val)) + require.Equal(t, input.match, f.Matches([]byte(input.val))) } f, err = NewTagsFilter(filters, NewMockSortedTagIterator, Disjunction) @@ -64,7 +64,7 @@ func TestTagsFilterMatches(t *testing.T) { } require.NoError(t, err) for _, input := range inputs { - require.Equal(t, input.match, f.Matches(input.val), "val:", input.val) + require.Equal(t, input.match, f.Matches([]byte(input.val)), "val:", input.val) } } diff --git a/rules/rule.go b/rules/rule.go index fc02310..2038c2e 100644 --- a/rules/rule.go +++ b/rules/rule.go @@ -114,7 +114,7 @@ type RuleSet interface { // Match matches the set of rules against a metric id, returning // the applicable mapping policies and rollup policies - Match(id string) MatchResult + Match(id []byte) MatchResult } // mappingRule defines a rule such that if a metric matches the provided filters, @@ -216,7 +216,7 @@ func (rs *ruleSet) Namespace() string { return rs.namespace } func (rs *ruleSet) Version() int { return rs.version } func (rs *ruleSet) Cutover() time.Time { return rs.cutover } -func (rs *ruleSet) Match(id string) MatchResult { +func (rs *ruleSet) Match(id []byte) MatchResult { if rs.tombStoned { return defaultMatchResult } @@ -226,7 +226,7 @@ func (rs *ruleSet) Match(id string) MatchResult { } } -func (rs *ruleSet) mappingPolicies(id string) []policy.Policy { +func (rs *ruleSet) mappingPolicies(id []byte) []policy.Policy { var policies []policy.Policy for _, rule := range rs.mappingRules { if rule.filter.Matches(id) { @@ -236,7 +236,7 @@ func (rs *ruleSet) mappingPolicies(id string) []policy.Policy { return resolvePolicies(policies) } -func (rs *ruleSet) rollupTargets(id string) []RollupTarget { +func (rs *ruleSet) rollupTargets(id []byte) []RollupTarget { var rollups []RollupTarget for _, rule := range rs.rollupRules { if !rule.filter.Matches(id) { diff --git a/rules/rule_test.go b/rules/rule_test.go index 644326d..76607d6 100644 --- a/rules/rule_test.go +++ b/rules/rule_test.go @@ -363,7 +363,7 @@ func TestRuleSetMatchMappingRules(t *testing.T) { }, } for _, input := range inputs { - res := ruleSet.Match(input.id) + res := ruleSet.Match([]byte(input.id)) require.Equal(t, input.result, res.Mappings) } } @@ -411,7 +411,7 @@ func TestRuleSetMatchRollupRules(t *testing.T) { }, } for _, input := range inputs { - res := ruleSet.Match(input.id) + res := ruleSet.Match([]byte(input.id)) require.Equal(t, input.result, res.Rollups) } } @@ -426,5 +426,5 @@ func TestTombstonedRuleSetMatch(t *testing.T) { require.NoError(t, err) id := "rtagName1=rtagValue1" - require.Equal(t, defaultMatchResult, ruleSet.Match(id)) + require.Equal(t, defaultMatchResult, ruleSet.Match([]byte(id))) } From 5a8262cc12a8eab9da8cbfab9c021f2ddf448654 Mon Sep 17 00:00:00 2001 From: Rob Skillington Date: Tue, 28 Mar 2017 10:06:05 -0400 Subject: [PATCH 2/2] Remove matchedValue struct and add note about UTF8 support --- filters/filter.go | 8 ++++---- rules/rule.go | 6 ------ 2 files changed, 4 insertions(+), 10 deletions(-) diff --git a/filters/filter.go b/filters/filter.go index bbabfef..b4ab48d 100644 --- a/filters/filter.go +++ b/filters/filter.go @@ -74,9 +74,11 @@ type Filter interface { } // NewFilter supports startsWith, endsWith, contains and a single wildcard -// along with negation and glob matching support -// TODO(martinm): Provide more detailed error messages +// along with negation and glob matching support. +// NOTE: Currently only supports ASCII matching and has zero compatibility +// with UTF8 so you should make sure all matches are done against ASCII only. func NewFilter(pattern []byte) (Filter, error) { + // TODO(martinm): Provide more detailed error messages if len(pattern) == 0 { return newEqualityFilter(pattern), nil } @@ -103,8 +105,6 @@ func NewFilter(pattern []byte) (Filter, error) { func newWildcardFilter(pattern []byte) (Filter, error) { wIdx := bytes.IndexRune(pattern, wildcardChar) - // TODO(r): use utf8 to take into account size of rune in byte slice - if wIdx == -1 { // No wildcards return newRangeFilter(pattern, false, middle) diff --git a/rules/rule.go b/rules/rule.go index 3fb9eb7..eb6bba4 100644 --- a/rules/rule.go +++ b/rules/rule.go @@ -345,12 +345,6 @@ func (rs *ruleSet) rollupResults(id []byte) []RollupResult { return rs.toRollupResults(id, rollups) } -type matchedValue struct { - name []byte - value []byte - matched bool -} - // toRollupResults encodes rollup target name and values into ids for each rollup target func (rs *ruleSet) toRollupResults(id []byte, targets []RollupTarget) []RollupResult { // NB(r): This is n^2 however this should be quite fast still as