From d903a9142d92bdb77b1f271e0203e28fbf1a47d8 Mon Sep 17 00:00:00 2001 From: DanKans Date: Fri, 21 Jul 2017 18:53:57 +0100 Subject: [PATCH] Fix filtering when both pass and drop match an item (#3036) Adjust logic in functions responsible for passing metrics in order to be able to process them correctly in case where pass and drop are defined together. --- docs/CONFIGURATION.md | 4 ++-- internal/models/filter.go | 41 ++++++++++++++++++++++++++-------- internal/models/filter_test.go | 40 +++++++++++++++++++++++++++++++++ 3 files changed, 74 insertions(+), 11 deletions(-) diff --git a/docs/CONFIGURATION.md b/docs/CONFIGURATION.md index 45ef09338..7d375e829 100644 --- a/docs/CONFIGURATION.md +++ b/docs/CONFIGURATION.md @@ -134,8 +134,8 @@ is tested on points after they have passed the `namepass` test. An array of glob pattern strings. Only fields whose field key matches a pattern in this list are emitted. Not available for outputs. * **fielddrop**: -The inverse of `fieldpass`. Fields with a field key matching one of the -patterns will be discarded from the point. Not available for outputs. +The inverse of `fieldpass`. Fields with a field key matching one of the +patterns will be discarded from the point. This is tested on points after they have passed the `fieldpass` test. Not available for outputs. * **tagpass**: A table mapping tag keys to arrays of glob pattern strings. Only points that contain a tag key in the table and a tag value matching one of its diff --git a/internal/models/filter.go b/internal/models/filter.go index 445ce3434..2848ccf09 100644 --- a/internal/models/filter.go +++ b/internal/models/filter.go @@ -132,6 +132,7 @@ func (f *Filter) Apply( return true } +// IsActive checking if filter is active func (f *Filter) IsActive() bool { return f.isActive } @@ -139,36 +140,58 @@ func (f *Filter) IsActive() bool { // shouldNamePass returns true if the metric should pass, false if should drop // based on the drop/pass filter parameters func (f *Filter) shouldNamePass(key string) bool { - if f.namePass != nil { + + pass := func(f *Filter) bool { if f.namePass.Match(key) { return true } return false } - if f.nameDrop != nil { + drop := func(f *Filter) bool { if f.nameDrop.Match(key) { return false } + return true } + + if f.namePass != nil && f.nameDrop != nil { + return pass(f) && drop(f) + } else if f.namePass != nil { + return pass(f) + } else if f.nameDrop != nil { + return drop(f) + } + return true } // shouldFieldPass returns true if the metric should pass, false if should drop // based on the drop/pass filter parameters func (f *Filter) shouldFieldPass(key string) bool { - if f.fieldPass != nil { + + pass := func(f *Filter) bool { if f.fieldPass.Match(key) { return true } return false } - if f.fieldDrop != nil { + drop := func(f *Filter) bool { if f.fieldDrop.Match(key) { return false } + return true } + + if f.fieldPass != nil && f.fieldDrop != nil { + return pass(f) && drop(f) + } else if f.fieldPass != nil { + return pass(f) + } else if f.fieldDrop != nil { + return drop(f) + } + return true } @@ -176,7 +199,7 @@ func (f *Filter) shouldFieldPass(key string) bool { // based on the tagdrop/tagpass filter parameters func (f *Filter) shouldTagsPass(tags map[string]string) bool { - tagPass := func(f *Filter) bool { + pass := func(f *Filter) bool { for _, pat := range f.TagPass { if pat.filter == nil { continue @@ -190,7 +213,7 @@ func (f *Filter) shouldTagsPass(tags map[string]string) bool { return false } - tagDrop := func(f *Filter) bool { + drop := func(f *Filter) bool { for _, pat := range f.TagDrop { if pat.filter == nil { continue @@ -209,11 +232,11 @@ func (f *Filter) shouldTagsPass(tags map[string]string) bool { if f.TagPass != nil && f.TagDrop != nil { // return true only in case when tag pass and won't be dropped (true, true). // in case when the same tag should be passed and dropped it will be dropped (true, false). - return tagPass(f) && tagDrop(f) + return pass(f) && drop(f) } else if f.TagPass != nil { - return tagPass(f) + return pass(f) } else if f.TagDrop != nil { - return tagDrop(f) + return drop(f) } return true diff --git a/internal/models/filter_test.go b/internal/models/filter_test.go index 5bbbc4a8f..46f16e835 100644 --- a/internal/models/filter_test.go +++ b/internal/models/filter_test.go @@ -358,6 +358,46 @@ func TestFilter_FilterTagsMatches(t *testing.T) { }, pretags) } +// TestFilter_FilterNamePassAndDrop used for check case when +// both parameters were defined +// see: https://github.com/influxdata/telegraf/issues/2860 +func TestFilter_FilterNamePassAndDrop(t *testing.T) { + + inputData := []string{"name1", "name2", "name3", "name4"} + expectedResult := []bool{false, true, false, false} + + f := Filter{ + NamePass: []string{"name1", "name2"}, + NameDrop: []string{"name1", "name3"}, + } + + require.NoError(t, f.Compile()) + + for i, name := range inputData { + assert.Equal(t, f.shouldNamePass(name), expectedResult[i]) + } +} + +// TestFilter_FilterFieldPassAndDrop used for check case when +// both parameters were defined +// see: https://github.com/influxdata/telegraf/issues/2860 +func TestFilter_FilterFieldPassAndDrop(t *testing.T) { + + inputData := []string{"field1", "field2", "field3", "field4"} + expectedResult := []bool{false, true, false, false} + + f := Filter{ + FieldPass: []string{"field1", "field2"}, + FieldDrop: []string{"field1", "field3"}, + } + + require.NoError(t, f.Compile()) + + for i, field := range inputData { + assert.Equal(t, f.shouldFieldPass(field), expectedResult[i]) + } +} + // TestFilter_FilterTagsPassAndDrop used for check case when // both parameters were defined // see: https://github.com/influxdata/telegraf/issues/2860