From 50ef3282b6e678a641ab40bb3b9f80e5b21dd152 Mon Sep 17 00:00:00 2001 From: Cameron Sparr Date: Mon, 5 Sep 2016 16:16:37 +0100 Subject: [PATCH] Refactor and code cleanup of filtering started working on this with the idea of fixing #1623, although I realized that this was actually just a documentation issue around a toml eccentricity. closes #1623 --- agent/accumulator.go | 43 +++------ agent/accumulator_test.go | 2 +- docs/CONFIGURATION.md | 8 ++ filter/filter.go | 6 +- filter/filter_test.go | 24 ++--- internal/config/config.go | 8 +- internal/config/config_test.go | 9 +- internal/models/filter.go | 89 +++++++++++++----- internal/models/filter_test.go | 120 ++++++++++++++++--------- internal/models/running_output.go | 14 ++- internal/models/running_output_test.go | 62 +++++-------- plugins/inputs/varnish/varnish.go | 4 +- 12 files changed, 216 insertions(+), 173 deletions(-) diff --git a/agent/accumulator.go b/agent/accumulator.go index ce3e22eeb..a0d0461a4 100644 --- a/agent/accumulator.go +++ b/agent/accumulator.go @@ -83,13 +83,8 @@ func (ac *accumulator) makeMetric( if len(fields) == 0 || len(measurement) == 0 { return nil } - - if !ac.inputConfig.Filter.ShouldNamePass(measurement) { - return nil - } - - if !ac.inputConfig.Filter.ShouldTagsPass(tags) { - return nil + if tags == nil { + tags = make(map[string]string) } // Override measurement name if set @@ -104,9 +99,6 @@ func (ac *accumulator) makeMetric( measurement = measurement + ac.inputConfig.MeasurementSuffix } - if tags == nil { - tags = make(map[string]string) - } // Apply plugin-wide tags if set for k, v := range ac.inputConfig.Tags { if _, ok := tags[k]; !ok { @@ -119,25 +111,21 @@ func (ac *accumulator) makeMetric( tags[k] = v } } - ac.inputConfig.Filter.FilterTags(tags) - result := make(map[string]interface{}) + // Apply the metric filter(s) + if ok := ac.inputConfig.Filter.Apply(measurement, fields, tags); !ok { + return nil + } + for k, v := range fields { - // Filter out any filtered fields - if ac.inputConfig != nil { - if !ac.inputConfig.Filter.ShouldFieldsPass(k) { - continue - } - } - // Validate uint64 and float64 fields switch val := v.(type) { case uint64: // InfluxDB does not support writing uint64 if val < uint64(9223372036854775808) { - result[k] = int64(val) + fields[k] = int64(val) } else { - result[k] = int64(9223372036854775807) + fields[k] = int64(9223372036854775807) } continue case float64: @@ -148,15 +136,12 @@ func (ac *accumulator) makeMetric( "field, skipping", measurement, k) } + delete(fields, k) continue } } - result[k] = v - } - fields = nil - if len(result) == 0 { - return nil + fields[k] = v } var timestamp time.Time @@ -171,11 +156,11 @@ func (ac *accumulator) makeMetric( var err error switch mType { case telegraf.Counter: - m, err = telegraf.NewCounterMetric(measurement, tags, result, timestamp) + m, err = telegraf.NewCounterMetric(measurement, tags, fields, timestamp) case telegraf.Gauge: - m, err = telegraf.NewGaugeMetric(measurement, tags, result, timestamp) + m, err = telegraf.NewGaugeMetric(measurement, tags, fields, timestamp) default: - m, err = telegraf.NewMetric(measurement, tags, result, timestamp) + m, err = telegraf.NewMetric(measurement, tags, fields, timestamp) } if err != nil { log.Printf("Error adding point [%s]: %s\n", measurement, err.Error()) diff --git a/agent/accumulator_test.go b/agent/accumulator_test.go index 7da8c96e2..ef5a34ec9 100644 --- a/agent/accumulator_test.go +++ b/agent/accumulator_test.go @@ -560,7 +560,7 @@ func TestAccFilterTags(t *testing.T) { filter := models.Filter{ TagExclude: []string{"acc"}, } - assert.NoError(t, filter.CompileFilter()) + assert.NoError(t, filter.Compile()) a.inputConfig = &models.InputConfig{} a.inputConfig.Filter = filter diff --git a/docs/CONFIGURATION.md b/docs/CONFIGURATION.md index a01178919..46f044ab7 100644 --- a/docs/CONFIGURATION.md +++ b/docs/CONFIGURATION.md @@ -86,6 +86,10 @@ as it is more efficient to filter out tags at the ingestion point. * **taginclude**: taginclude is the inverse of tagexclude. It will only include the tag keys in the final measurement. +**NOTE** `tagpass` and `tagdrop` parameters must be defined at the _end_ of +the plugin definition, otherwise subsequent plugin config options will be +interpreted as part of the tagpass/tagdrop map. + ## Input Configuration Some configuration options are configurable per input: @@ -129,6 +133,10 @@ fields which begin with `time_`. #### Input Config: tagpass and tagdrop +**NOTE** `tagpass` and `tagdrop` parameters must be defined at the _end_ of +the plugin definition, otherwise subsequent plugin config options will be +interpreted as part of the tagpass/tagdrop map. + ```toml [[inputs.cpu]] percpu = true diff --git a/filter/filter.go b/filter/filter.go index 85eed17ac..9a28c2627 100644 --- a/filter/filter.go +++ b/filter/filter.go @@ -10,16 +10,16 @@ type Filter interface { Match(string) bool } -// CompileFilter takes a list of string filters and returns a Filter interface +// Compile takes a list of string filters and returns a Filter interface // for matching a given string against the filter list. The filter list // supports glob matching too, ie: // -// f, _ := CompileFilter([]string{"cpu", "mem", "net*"}) +// f, _ := Compile([]string{"cpu", "mem", "net*"}) // f.Match("cpu") // true // f.Match("network") // true // f.Match("memory") // false // -func CompileFilter(filters []string) (Filter, error) { +func Compile(filters []string) (Filter, error) { // return if there is nothing to compile if len(filters) == 0 { return nil, nil diff --git a/filter/filter_test.go b/filter/filter_test.go index 85072e2ac..2f52e036a 100644 --- a/filter/filter_test.go +++ b/filter/filter_test.go @@ -6,30 +6,30 @@ import ( "github.com/stretchr/testify/assert" ) -func TestCompileFilter(t *testing.T) { - f, err := CompileFilter([]string{}) +func TestCompile(t *testing.T) { + f, err := Compile([]string{}) assert.NoError(t, err) assert.Nil(t, f) - f, err = CompileFilter([]string{"cpu"}) + f, err = Compile([]string{"cpu"}) assert.NoError(t, err) assert.True(t, f.Match("cpu")) assert.False(t, f.Match("cpu0")) assert.False(t, f.Match("mem")) - f, err = CompileFilter([]string{"cpu*"}) + f, err = Compile([]string{"cpu*"}) assert.NoError(t, err) assert.True(t, f.Match("cpu")) assert.True(t, f.Match("cpu0")) assert.False(t, f.Match("mem")) - f, err = CompileFilter([]string{"cpu", "mem"}) + f, err = Compile([]string{"cpu", "mem"}) assert.NoError(t, err) assert.True(t, f.Match("cpu")) assert.False(t, f.Match("cpu0")) assert.True(t, f.Match("mem")) - f, err = CompileFilter([]string{"cpu", "mem", "net*"}) + f, err = Compile([]string{"cpu", "mem", "net*"}) assert.NoError(t, err) assert.True(t, f.Match("cpu")) assert.False(t, f.Match("cpu0")) @@ -40,7 +40,7 @@ func TestCompileFilter(t *testing.T) { var benchbool bool func BenchmarkFilterSingleNoGlobFalse(b *testing.B) { - f, _ := CompileFilter([]string{"cpu"}) + f, _ := Compile([]string{"cpu"}) var tmp bool for n := 0; n < b.N; n++ { tmp = f.Match("network") @@ -49,7 +49,7 @@ func BenchmarkFilterSingleNoGlobFalse(b *testing.B) { } func BenchmarkFilterSingleNoGlobTrue(b *testing.B) { - f, _ := CompileFilter([]string{"cpu"}) + f, _ := Compile([]string{"cpu"}) var tmp bool for n := 0; n < b.N; n++ { tmp = f.Match("cpu") @@ -58,7 +58,7 @@ func BenchmarkFilterSingleNoGlobTrue(b *testing.B) { } func BenchmarkFilter(b *testing.B) { - f, _ := CompileFilter([]string{"cpu", "mem", "net*"}) + f, _ := Compile([]string{"cpu", "mem", "net*"}) var tmp bool for n := 0; n < b.N; n++ { tmp = f.Match("network") @@ -67,7 +67,7 @@ func BenchmarkFilter(b *testing.B) { } func BenchmarkFilterNoGlob(b *testing.B) { - f, _ := CompileFilter([]string{"cpu", "mem", "net"}) + f, _ := Compile([]string{"cpu", "mem", "net"}) var tmp bool for n := 0; n < b.N; n++ { tmp = f.Match("net") @@ -76,7 +76,7 @@ func BenchmarkFilterNoGlob(b *testing.B) { } func BenchmarkFilter2(b *testing.B) { - f, _ := CompileFilter([]string{"aa", "bb", "c", "ad", "ar", "at", "aq", + f, _ := Compile([]string{"aa", "bb", "c", "ad", "ar", "at", "aq", "aw", "az", "axxx", "ab", "cpu", "mem", "net*"}) var tmp bool for n := 0; n < b.N; n++ { @@ -86,7 +86,7 @@ func BenchmarkFilter2(b *testing.B) { } func BenchmarkFilter2NoGlob(b *testing.B) { - f, _ := CompileFilter([]string{"aa", "bb", "c", "ad", "ar", "at", "aq", + f, _ := Compile([]string{"aa", "bb", "c", "ad", "ar", "at", "aq", "aw", "az", "axxx", "ab", "cpu", "mem", "net"}) var tmp bool for n := 0; n < b.N; n++ { diff --git a/internal/config/config.go b/internal/config/config.go index 24c1af3fa..30e627890 100644 --- a/internal/config/config.go +++ b/internal/config/config.go @@ -665,7 +665,6 @@ func buildFilter(tbl *ast.Table) (models.Filter, error) { for _, elem := range ary.Value { if str, ok := elem.(*ast.String); ok { f.NamePass = append(f.NamePass, str.Value) - f.IsActive = true } } } @@ -678,7 +677,6 @@ func buildFilter(tbl *ast.Table) (models.Filter, error) { for _, elem := range ary.Value { if str, ok := elem.(*ast.String); ok { f.NameDrop = append(f.NameDrop, str.Value) - f.IsActive = true } } } @@ -693,7 +691,6 @@ func buildFilter(tbl *ast.Table) (models.Filter, error) { for _, elem := range ary.Value { if str, ok := elem.(*ast.String); ok { f.FieldPass = append(f.FieldPass, str.Value) - f.IsActive = true } } } @@ -709,7 +706,6 @@ func buildFilter(tbl *ast.Table) (models.Filter, error) { for _, elem := range ary.Value { if str, ok := elem.(*ast.String); ok { f.FieldDrop = append(f.FieldDrop, str.Value) - f.IsActive = true } } } @@ -730,7 +726,6 @@ func buildFilter(tbl *ast.Table) (models.Filter, error) { } } f.TagPass = append(f.TagPass, *tagfilter) - f.IsActive = true } } } @@ -749,7 +744,6 @@ func buildFilter(tbl *ast.Table) (models.Filter, error) { } } f.TagDrop = append(f.TagDrop, *tagfilter) - f.IsActive = true } } } @@ -778,7 +772,7 @@ func buildFilter(tbl *ast.Table) (models.Filter, error) { } } } - if err := f.CompileFilter(); err != nil { + if err := f.Compile(); err != nil { return f, err } diff --git a/internal/config/config_test.go b/internal/config/config_test.go index cb8c9192c..3498d815d 100644 --- a/internal/config/config_test.go +++ b/internal/config/config_test.go @@ -43,9 +43,8 @@ func TestConfig_LoadSingleInputWithEnvVars(t *testing.T) { Filter: []string{"mytag"}, }, }, - IsActive: true, } - assert.NoError(t, filter.CompileFilter()) + assert.NoError(t, filter.Compile()) mConfig := &models.InputConfig{ Name: "memcached", Filter: filter, @@ -83,9 +82,8 @@ func TestConfig_LoadSingleInput(t *testing.T) { Filter: []string{"mytag"}, }, }, - IsActive: true, } - assert.NoError(t, filter.CompileFilter()) + assert.NoError(t, filter.Compile()) mConfig := &models.InputConfig{ Name: "memcached", Filter: filter, @@ -130,9 +128,8 @@ func TestConfig_LoadDirectory(t *testing.T) { Filter: []string{"mytag"}, }, }, - IsActive: true, } - assert.NoError(t, filter.CompileFilter()) + assert.NoError(t, filter.Compile()) mConfig := &models.InputConfig{ Name: "memcached", Filter: filter, diff --git a/internal/models/filter.go b/internal/models/filter.go index 9ad4c0049..b87c59501 100644 --- a/internal/models/filter.go +++ b/internal/models/filter.go @@ -3,7 +3,6 @@ package models import ( "fmt" - "github.com/influxdata/telegraf" "github.com/influxdata/telegraf/filter" ) @@ -34,47 +33,59 @@ type Filter struct { TagInclude []string tagInclude filter.Filter - IsActive bool + isActive bool } // Compile all Filter lists into filter.Filter objects. -func (f *Filter) CompileFilter() error { +func (f *Filter) Compile() error { + if len(f.NameDrop) == 0 && + len(f.NamePass) == 0 && + len(f.FieldDrop) == 0 && + len(f.FieldPass) == 0 && + len(f.TagInclude) == 0 && + len(f.TagExclude) == 0 && + len(f.TagPass) == 0 && + len(f.TagDrop) == 0 { + return nil + } + + f.isActive = true var err error - f.nameDrop, err = filter.CompileFilter(f.NameDrop) + f.nameDrop, err = filter.Compile(f.NameDrop) if err != nil { return fmt.Errorf("Error compiling 'namedrop', %s", err) } - f.namePass, err = filter.CompileFilter(f.NamePass) + f.namePass, err = filter.Compile(f.NamePass) if err != nil { return fmt.Errorf("Error compiling 'namepass', %s", err) } - f.fieldDrop, err = filter.CompileFilter(f.FieldDrop) + f.fieldDrop, err = filter.Compile(f.FieldDrop) if err != nil { return fmt.Errorf("Error compiling 'fielddrop', %s", err) } - f.fieldPass, err = filter.CompileFilter(f.FieldPass) + f.fieldPass, err = filter.Compile(f.FieldPass) if err != nil { return fmt.Errorf("Error compiling 'fieldpass', %s", err) } - f.tagExclude, err = filter.CompileFilter(f.TagExclude) + f.tagExclude, err = filter.Compile(f.TagExclude) if err != nil { return fmt.Errorf("Error compiling 'tagexclude', %s", err) } - f.tagInclude, err = filter.CompileFilter(f.TagInclude) + f.tagInclude, err = filter.Compile(f.TagInclude) if err != nil { return fmt.Errorf("Error compiling 'taginclude', %s", err) } for i, _ := range f.TagDrop { - f.TagDrop[i].filter, err = filter.CompileFilter(f.TagDrop[i].Filter) + f.TagDrop[i].filter, err = filter.Compile(f.TagDrop[i].Filter) if err != nil { return fmt.Errorf("Error compiling 'tagdrop', %s", err) } } for i, _ := range f.TagPass { - f.TagPass[i].filter, err = filter.CompileFilter(f.TagPass[i].Filter) + f.TagPass[i].filter, err = filter.Compile(f.TagPass[i].Filter) if err != nil { return fmt.Errorf("Error compiling 'tagpass', %s", err) } @@ -82,16 +93,52 @@ func (f *Filter) CompileFilter() error { return nil } -func (f *Filter) ShouldMetricPass(metric telegraf.Metric) bool { - if f.ShouldNamePass(metric.Name()) && f.ShouldTagsPass(metric.Tags()) { +// Apply applies the filter to the given measurement name, fields map, and +// tags map. It will return false if the metric should be "filtered out", and +// true if the metric should "pass". +// It will modify tags in-place if they need to be deleted. +func (f *Filter) Apply( + measurement string, + fields map[string]interface{}, + tags map[string]string, +) bool { + if !f.isActive { return true } - return false + + // check if the measurement name should pass + if !f.shouldNamePass(measurement) { + return false + } + + // check if the tags should pass + if !f.shouldTagsPass(tags) { + return false + } + + // filter fields + for fieldkey, _ := range fields { + if !f.shouldFieldPass(fieldkey) { + delete(fields, fieldkey) + } + } + if len(fields) == 0 { + return false + } + + // filter tags + f.filterTags(tags) + + return true } -// ShouldFieldsPass returns true if the metric should pass, false if should drop +func (f *Filter) IsActive() bool { + return f.isActive +} + +// 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 { +func (f *Filter) shouldNamePass(key string) bool { if f.namePass != nil { if f.namePass.Match(key) { return true @@ -107,9 +154,9 @@ func (f *Filter) ShouldNamePass(key string) bool { return true } -// ShouldFieldsPass returns true if the metric should pass, false if should drop +// shouldFieldPass returns true if the metric should pass, false if should drop // based on the drop/pass filter parameters -func (f *Filter) ShouldFieldsPass(key string) bool { +func (f *Filter) shouldFieldPass(key string) bool { if f.fieldPass != nil { if f.fieldPass.Match(key) { return true @@ -125,9 +172,9 @@ func (f *Filter) ShouldFieldsPass(key string) bool { return true } -// ShouldTagsPass returns true if the metric should pass, false if should drop +// shouldTagsPass returns true if the metric should pass, false if should drop // based on the tagdrop/tagpass filter parameters -func (f *Filter) ShouldTagsPass(tags map[string]string) bool { +func (f *Filter) shouldTagsPass(tags map[string]string) bool { if f.TagPass != nil { for _, pat := range f.TagPass { if pat.filter == nil { @@ -161,7 +208,7 @@ func (f *Filter) ShouldTagsPass(tags map[string]string) bool { // Apply TagInclude and TagExclude filters. // modifies the tags map in-place. -func (f *Filter) FilterTags(tags map[string]string) { +func (f *Filter) filterTags(tags map[string]string) { if f.tagInclude != nil { for k, _ := range tags { if !f.tagInclude.Match(k) { diff --git a/internal/models/filter_test.go b/internal/models/filter_test.go index 497d08532..95b63e30a 100644 --- a/internal/models/filter_test.go +++ b/internal/models/filter_test.go @@ -3,12 +3,62 @@ package models import ( "testing" - "github.com/influxdata/telegraf/testutil" - "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" ) +func TestFilter_ApplyEmpty(t *testing.T) { + f := Filter{} + require.NoError(t, f.Compile()) + assert.False(t, f.IsActive()) + + assert.True(t, f.Apply("m", map[string]interface{}{"value": int64(1)}, map[string]string{})) +} + +func TestFilter_ApplyTagsDontPass(t *testing.T) { + filters := []TagFilter{ + TagFilter{ + Name: "cpu", + Filter: []string{"cpu-*"}, + }, + } + f := Filter{ + TagDrop: filters, + } + require.NoError(t, f.Compile()) + require.NoError(t, f.Compile()) + assert.True(t, f.IsActive()) + + assert.False(t, f.Apply("m", + map[string]interface{}{"value": int64(1)}, + map[string]string{"cpu": "cpu-total"})) +} + +func TestFilter_ApplyDeleteFields(t *testing.T) { + f := Filter{ + FieldDrop: []string{"value"}, + } + require.NoError(t, f.Compile()) + require.NoError(t, f.Compile()) + assert.True(t, f.IsActive()) + + fields := map[string]interface{}{"value": int64(1), "value2": int64(2)} + assert.True(t, f.Apply("m", fields, nil)) + assert.Equal(t, map[string]interface{}{"value2": int64(2)}, fields) +} + +func TestFilter_ApplyDeleteAllFields(t *testing.T) { + f := Filter{ + FieldDrop: []string{"value*"}, + } + require.NoError(t, f.Compile()) + require.NoError(t, f.Compile()) + assert.True(t, f.IsActive()) + + fields := map[string]interface{}{"value": int64(1), "value2": int64(2)} + assert.False(t, f.Apply("m", fields, nil)) +} + func TestFilter_Empty(t *testing.T) { f := Filter{} @@ -23,7 +73,7 @@ func TestFilter_Empty(t *testing.T) { } for _, measurement := range measurements { - if !f.ShouldFieldsPass(measurement) { + if !f.shouldFieldPass(measurement) { t.Errorf("Expected measurement %s to pass", measurement) } } @@ -33,7 +83,7 @@ func TestFilter_NamePass(t *testing.T) { f := Filter{ NamePass: []string{"foo*", "cpu_usage_idle"}, } - require.NoError(t, f.CompileFilter()) + require.NoError(t, f.Compile()) passes := []string{ "foo", @@ -51,13 +101,13 @@ func TestFilter_NamePass(t *testing.T) { } for _, measurement := range passes { - if !f.ShouldNamePass(measurement) { + if !f.shouldNamePass(measurement) { t.Errorf("Expected measurement %s to pass", measurement) } } for _, measurement := range drops { - if f.ShouldNamePass(measurement) { + if f.shouldNamePass(measurement) { t.Errorf("Expected measurement %s to drop", measurement) } } @@ -67,7 +117,7 @@ func TestFilter_NameDrop(t *testing.T) { f := Filter{ NameDrop: []string{"foo*", "cpu_usage_idle"}, } - require.NoError(t, f.CompileFilter()) + require.NoError(t, f.Compile()) drops := []string{ "foo", @@ -85,13 +135,13 @@ func TestFilter_NameDrop(t *testing.T) { } for _, measurement := range passes { - if !f.ShouldNamePass(measurement) { + if !f.shouldNamePass(measurement) { t.Errorf("Expected measurement %s to pass", measurement) } } for _, measurement := range drops { - if f.ShouldNamePass(measurement) { + if f.shouldNamePass(measurement) { t.Errorf("Expected measurement %s to drop", measurement) } } @@ -101,7 +151,7 @@ func TestFilter_FieldPass(t *testing.T) { f := Filter{ FieldPass: []string{"foo*", "cpu_usage_idle"}, } - require.NoError(t, f.CompileFilter()) + require.NoError(t, f.Compile()) passes := []string{ "foo", @@ -119,13 +169,13 @@ func TestFilter_FieldPass(t *testing.T) { } for _, measurement := range passes { - if !f.ShouldFieldsPass(measurement) { + if !f.shouldFieldPass(measurement) { t.Errorf("Expected measurement %s to pass", measurement) } } for _, measurement := range drops { - if f.ShouldFieldsPass(measurement) { + if f.shouldFieldPass(measurement) { t.Errorf("Expected measurement %s to drop", measurement) } } @@ -135,7 +185,7 @@ func TestFilter_FieldDrop(t *testing.T) { f := Filter{ FieldDrop: []string{"foo*", "cpu_usage_idle"}, } - require.NoError(t, f.CompileFilter()) + require.NoError(t, f.Compile()) drops := []string{ "foo", @@ -153,13 +203,13 @@ func TestFilter_FieldDrop(t *testing.T) { } for _, measurement := range passes { - if !f.ShouldFieldsPass(measurement) { + if !f.shouldFieldPass(measurement) { t.Errorf("Expected measurement %s to pass", measurement) } } for _, measurement := range drops { - if f.ShouldFieldsPass(measurement) { + if f.shouldFieldPass(measurement) { t.Errorf("Expected measurement %s to drop", measurement) } } @@ -178,7 +228,7 @@ func TestFilter_TagPass(t *testing.T) { f := Filter{ TagPass: filters, } - require.NoError(t, f.CompileFilter()) + require.NoError(t, f.Compile()) passes := []map[string]string{ {"cpu": "cpu-total"}, @@ -197,13 +247,13 @@ func TestFilter_TagPass(t *testing.T) { } for _, tags := range passes { - if !f.ShouldTagsPass(tags) { + if !f.shouldTagsPass(tags) { t.Errorf("Expected tags %v to pass", tags) } } for _, tags := range drops { - if f.ShouldTagsPass(tags) { + if f.shouldTagsPass(tags) { t.Errorf("Expected tags %v to drop", tags) } } @@ -222,7 +272,7 @@ func TestFilter_TagDrop(t *testing.T) { f := Filter{ TagDrop: filters, } - require.NoError(t, f.CompileFilter()) + require.NoError(t, f.Compile()) drops := []map[string]string{ {"cpu": "cpu-total"}, @@ -241,30 +291,18 @@ func TestFilter_TagDrop(t *testing.T) { } for _, tags := range passes { - if !f.ShouldTagsPass(tags) { + if !f.shouldTagsPass(tags) { t.Errorf("Expected tags %v to pass", tags) } } for _, tags := range drops { - if f.ShouldTagsPass(tags) { + if f.shouldTagsPass(tags) { t.Errorf("Expected tags %v to drop", tags) } } } -func TestFilter_ShouldMetricsPass(t *testing.T) { - m := testutil.TestMetric(1, "testmetric") - f := Filter{ - NameDrop: []string{"foobar"}, - } - require.NoError(t, f.CompileFilter()) - require.True(t, f.ShouldMetricPass(m)) - - m = testutil.TestMetric(1, "foobar") - require.False(t, f.ShouldMetricPass(m)) -} - func TestFilter_FilterTagsNoMatches(t *testing.T) { pretags := map[string]string{ "host": "localhost", @@ -273,9 +311,9 @@ func TestFilter_FilterTagsNoMatches(t *testing.T) { f := Filter{ TagExclude: []string{"nomatch"}, } - require.NoError(t, f.CompileFilter()) + require.NoError(t, f.Compile()) - f.FilterTags(pretags) + f.filterTags(pretags) assert.Equal(t, map[string]string{ "host": "localhost", "mytag": "foobar", @@ -284,9 +322,9 @@ func TestFilter_FilterTagsNoMatches(t *testing.T) { f = Filter{ TagInclude: []string{"nomatch"}, } - require.NoError(t, f.CompileFilter()) + require.NoError(t, f.Compile()) - f.FilterTags(pretags) + f.filterTags(pretags) assert.Equal(t, map[string]string{}, pretags) } @@ -298,9 +336,9 @@ func TestFilter_FilterTagsMatches(t *testing.T) { f := Filter{ TagExclude: []string{"ho*"}, } - require.NoError(t, f.CompileFilter()) + require.NoError(t, f.Compile()) - f.FilterTags(pretags) + f.filterTags(pretags) assert.Equal(t, map[string]string{ "mytag": "foobar", }, pretags) @@ -312,9 +350,9 @@ func TestFilter_FilterTagsMatches(t *testing.T) { f = Filter{ TagInclude: []string{"my*"}, } - require.NoError(t, f.CompileFilter()) + require.NoError(t, f.Compile()) - f.FilterTags(pretags) + f.filterTags(pretags) assert.Equal(t, map[string]string{ "mytag": "foobar", }, pretags) diff --git a/internal/models/running_output.go b/internal/models/running_output.go index 82a6885d5..c4de4afd9 100644 --- a/internal/models/running_output.go +++ b/internal/models/running_output.go @@ -57,21 +57,17 @@ func NewRunningOutput( // AddMetric adds a metric to the output. This function can also write cached // points if FlushBufferWhenFull is true. func (ro *RunningOutput) AddMetric(metric telegraf.Metric) { - if ro.Config.Filter.IsActive { - if !ro.Config.Filter.ShouldMetricPass(metric) { - return - } - } - // Filter any tagexclude/taginclude parameters before adding metric - if len(ro.Config.Filter.TagExclude) != 0 || len(ro.Config.Filter.TagInclude) != 0 { + if ro.Config.Filter.IsActive() { // In order to filter out tags, we need to create a new metric, since // metrics are immutable once created. + name := metric.Name() tags := metric.Tags() fields := metric.Fields() t := metric.Time() - name := metric.Name() - ro.Config.Filter.FilterTags(tags) + if ok := ro.Config.Filter.Apply(name, fields, tags); !ok { + return + } // error is not possible if creating from another metric, so ignore. metric, _ = telegraf.NewMetric(name, tags, fields, t) } diff --git a/internal/models/running_output_test.go b/internal/models/running_output_test.go index a552629e9..a42d6fc7e 100644 --- a/internal/models/running_output_test.go +++ b/internal/models/running_output_test.go @@ -31,9 +31,7 @@ var next5 = []telegraf.Metric{ // Benchmark adding metrics. func BenchmarkRunningOutputAddWrite(b *testing.B) { conf := &OutputConfig{ - Filter: Filter{ - IsActive: false, - }, + Filter: Filter{}, } m := &perfOutput{} @@ -49,9 +47,7 @@ func BenchmarkRunningOutputAddWrite(b *testing.B) { // Benchmark adding metrics. func BenchmarkRunningOutputAddWriteEvery100(b *testing.B) { conf := &OutputConfig{ - Filter: Filter{ - IsActive: false, - }, + Filter: Filter{}, } m := &perfOutput{} @@ -69,9 +65,7 @@ func BenchmarkRunningOutputAddWriteEvery100(b *testing.B) { // Benchmark adding metrics. func BenchmarkRunningOutputAddFailWrites(b *testing.B) { conf := &OutputConfig{ - Filter: Filter{ - IsActive: false, - }, + Filter: Filter{}, } m := &perfOutput{} @@ -88,11 +82,10 @@ func BenchmarkRunningOutputAddFailWrites(b *testing.B) { func TestRunningOutput_DropFilter(t *testing.T) { conf := &OutputConfig{ Filter: Filter{ - IsActive: true, NameDrop: []string{"metric1", "metric2"}, }, } - assert.NoError(t, conf.Filter.CompileFilter()) + assert.NoError(t, conf.Filter.Compile()) m := &mockOutput{} ro := NewRunningOutput("test", m, conf, 1000, 10000) @@ -114,11 +107,10 @@ func TestRunningOutput_DropFilter(t *testing.T) { func TestRunningOutput_PassFilter(t *testing.T) { conf := &OutputConfig{ Filter: Filter{ - IsActive: true, NameDrop: []string{"metric1000", "foo*"}, }, } - assert.NoError(t, conf.Filter.CompileFilter()) + assert.NoError(t, conf.Filter.Compile()) m := &mockOutput{} ro := NewRunningOutput("test", m, conf, 1000, 10000) @@ -140,11 +132,11 @@ func TestRunningOutput_PassFilter(t *testing.T) { func TestRunningOutput_TagIncludeNoMatch(t *testing.T) { conf := &OutputConfig{ Filter: Filter{ - IsActive: true, + TagInclude: []string{"nothing*"}, }, } - assert.NoError(t, conf.Filter.CompileFilter()) + assert.NoError(t, conf.Filter.Compile()) m := &mockOutput{} ro := NewRunningOutput("test", m, conf, 1000, 10000) @@ -162,11 +154,11 @@ func TestRunningOutput_TagIncludeNoMatch(t *testing.T) { func TestRunningOutput_TagExcludeMatch(t *testing.T) { conf := &OutputConfig{ Filter: Filter{ - IsActive: true, + TagExclude: []string{"tag*"}, }, } - assert.NoError(t, conf.Filter.CompileFilter()) + assert.NoError(t, conf.Filter.Compile()) m := &mockOutput{} ro := NewRunningOutput("test", m, conf, 1000, 10000) @@ -184,11 +176,11 @@ func TestRunningOutput_TagExcludeMatch(t *testing.T) { func TestRunningOutput_TagExcludeNoMatch(t *testing.T) { conf := &OutputConfig{ Filter: Filter{ - IsActive: true, + TagExclude: []string{"nothing*"}, }, } - assert.NoError(t, conf.Filter.CompileFilter()) + assert.NoError(t, conf.Filter.Compile()) m := &mockOutput{} ro := NewRunningOutput("test", m, conf, 1000, 10000) @@ -206,11 +198,11 @@ func TestRunningOutput_TagExcludeNoMatch(t *testing.T) { func TestRunningOutput_TagIncludeMatch(t *testing.T) { conf := &OutputConfig{ Filter: Filter{ - IsActive: true, + TagInclude: []string{"tag*"}, }, } - assert.NoError(t, conf.Filter.CompileFilter()) + assert.NoError(t, conf.Filter.Compile()) m := &mockOutput{} ro := NewRunningOutput("test", m, conf, 1000, 10000) @@ -227,9 +219,7 @@ func TestRunningOutput_TagIncludeMatch(t *testing.T) { // Test that we can write metrics with simple default setup. func TestRunningOutputDefault(t *testing.T) { conf := &OutputConfig{ - Filter: Filter{ - IsActive: false, - }, + Filter: Filter{}, } m := &mockOutput{} @@ -252,9 +242,7 @@ func TestRunningOutputDefault(t *testing.T) { // FlushBufferWhenFull is set. func TestRunningOutputFlushWhenFull(t *testing.T) { conf := &OutputConfig{ - Filter: Filter{ - IsActive: false, - }, + Filter: Filter{}, } m := &mockOutput{} @@ -283,9 +271,7 @@ func TestRunningOutputFlushWhenFull(t *testing.T) { // FlushBufferWhenFull is set, twice. func TestRunningOutputMultiFlushWhenFull(t *testing.T) { conf := &OutputConfig{ - Filter: Filter{ - IsActive: false, - }, + Filter: Filter{}, } m := &mockOutput{} @@ -304,9 +290,7 @@ func TestRunningOutputMultiFlushWhenFull(t *testing.T) { func TestRunningOutputWriteFail(t *testing.T) { conf := &OutputConfig{ - Filter: Filter{ - IsActive: false, - }, + Filter: Filter{}, } m := &mockOutput{} @@ -339,9 +323,7 @@ func TestRunningOutputWriteFail(t *testing.T) { // Verify that the order of points is preserved during a write failure. func TestRunningOutputWriteFailOrder(t *testing.T) { conf := &OutputConfig{ - Filter: Filter{ - IsActive: false, - }, + Filter: Filter{}, } m := &mockOutput{} @@ -379,9 +361,7 @@ func TestRunningOutputWriteFailOrder(t *testing.T) { // Verify that the order of points is preserved during many write failures. func TestRunningOutputWriteFailOrder2(t *testing.T) { conf := &OutputConfig{ - Filter: Filter{ - IsActive: false, - }, + Filter: Filter{}, } m := &mockOutput{} @@ -452,9 +432,7 @@ func TestRunningOutputWriteFailOrder2(t *testing.T) { // func TestRunningOutputWriteFailOrder3(t *testing.T) { conf := &OutputConfig{ - Filter: Filter{ - IsActive: false, - }, + Filter: Filter{}, } m := &mockOutput{} diff --git a/plugins/inputs/varnish/varnish.go b/plugins/inputs/varnish/varnish.go index 2b0e84514..229a5e8b0 100644 --- a/plugins/inputs/varnish/varnish.go +++ b/plugins/inputs/varnish/varnish.go @@ -77,13 +77,13 @@ func (s *Varnish) Gather(acc telegraf.Accumulator) error { if s.filter == nil { var err error if len(s.Stats) == 0 { - s.filter, err = filter.CompileFilter(defaultStats) + s.filter, err = filter.Compile(defaultStats) } else { // legacy support, change "all" -> "*": if s.Stats[0] == "all" { s.Stats[0] = "*" } - s.filter, err = filter.CompileFilter(s.Stats) + s.filter, err = filter.Compile(s.Stats) } if err != nil { return err