From d4f2fae8113019ea37b10e005bdbb8d76e983896 Mon Sep 17 00:00:00 2001 From: Akash Gupta Date: Tue, 28 Nov 2023 10:26:52 +0530 Subject: [PATCH 1/7] feat: Allow adding separators for namepass and namedrop filters in the input config --- config/config.go | 21 ++++++++++++++++++++- filter/filter.go | 27 +++++++++++++++------------ models/filter.go | 14 ++++++++------ 3 files changed, 43 insertions(+), 19 deletions(-) diff --git a/config/config.go b/config/config.go index 97f8afc974497..d5ba28202285d 100644 --- a/config/config.go +++ b/config/config.go @@ -1331,7 +1331,9 @@ func (c *Config) buildFilter(tbl *ast.Table) (models.Filter, error) { f := models.Filter{} c.getFieldStringSlice(tbl, "namepass", &f.NamePass) + c.getFieldRuneSlice(tbl, "namepass_separator", &f.NamePassSeparators) c.getFieldStringSlice(tbl, "namedrop", &f.NameDrop) + c.getFieldRuneSlice(tbl, "namedrop_separator", &f.NameDropSeparators) c.getFieldStringSlice(tbl, "pass", &f.FieldPass) c.getFieldStringSlice(tbl, "fieldpass", &f.FieldPass) @@ -1446,7 +1448,7 @@ func (c *Config) missingTomlField(_ reflect.Type, key string) error { "interval", "lvm", // What is this used for? "metric_batch_size", "metric_buffer_limit", "metricpass", - "name_override", "name_prefix", "name_suffix", "namedrop", "namepass", + "name_override", "name_prefix", "name_suffix", "namedrop", "namedrop_separator", "namepass", "namepass_separator", "order", "pass", "period", "precision", "tagdrop", "tagexclude", "taginclude", "tagpass", "tags": @@ -1582,6 +1584,23 @@ func (c *Config) getFieldInt64(tbl *ast.Table, fieldName string, target *int64) } } +func (c *Config) getFieldRuneSlice(tbl *ast.Table, fieldName string, target *[]rune) { + if node, ok := tbl.Fields[fieldName]; ok { + if kv, ok := node.(*ast.KeyValue); ok { + ary, ok := kv.Value.(*ast.Array) + if !ok { + c.addError(tbl, fmt.Errorf("found unexpected format while parsing %q, expecting string slice format", fieldName)) + return + } + for _, elem := range ary.Value { + if str, ok := elem.(*ast.String); ok { + *target = append(*target, []rune(str.Value)...) + } + } + } + } +} + func (c *Config) getFieldStringSlice(tbl *ast.Table, fieldName string, target *[]string) { if node, ok := tbl.Fields[fieldName]; ok { if kv, ok := node.(*ast.KeyValue); ok { diff --git a/filter/filter.go b/filter/filter.go index 3e3798a47d2d8..495099670d016 100644 --- a/filter/filter.go +++ b/filter/filter.go @@ -18,18 +18,20 @@ type Filter interface { // f.Match("cpu") // true // f.Match("network") // true // f.Match("memory") // false -func Compile(filters []string) (Filter, error) { +func Compile(filters []string, separators ...rune) (Filter, error) { // return if there is nothing to compile if len(filters) == 0 { return nil, nil } // check if we can compile a non-glob filter - noGlob := true - for _, filter := range filters { - if hasMeta(filter) { - noGlob = false - break + noGlob := len(separators) == 0 + if noGlob { + for _, filter := range filters { + if hasMeta(filter) { + noGlob = false + break + } } } @@ -38,14 +40,14 @@ func Compile(filters []string) (Filter, error) { // return non-globbing filter if not needed. return compileFilterNoGlob(filters), nil case len(filters) == 1: - return glob.Compile(filters[0]) + return glob.Compile(filters[0], separators...) default: - return glob.Compile("{" + strings.Join(filters, ",") + "}") + return glob.Compile("{"+strings.Join(filters, ",")+"}", separators...) } } -func MustCompile(filters []string) Filter { - f, err := Compile(filters) +func MustCompile(filters []string, separators ...rune) Filter { + f, err := Compile(filters, separators...) if err != nil { panic(err) } @@ -104,13 +106,14 @@ func NewIncludeExcludeFilterDefaults( exclude []string, includeDefault bool, excludeDefault bool, + separators ...rune, ) (Filter, error) { - in, err := Compile(include) + in, err := Compile(include, separators...) if err != nil { return nil, err } - ex, err := Compile(exclude) + ex, err := Compile(exclude, separators...) if err != nil { return nil, err } diff --git a/models/filter.go b/models/filter.go index e595e5be4873d..1e152d006b297 100644 --- a/models/filter.go +++ b/models/filter.go @@ -33,10 +33,12 @@ func (tf *TagFilter) Compile() error { // Filter containing drop/pass and tagdrop/tagpass rules type Filter struct { - NameDrop []string - nameDropFilter filter.Filter - NamePass []string - namePassFilter filter.Filter + NameDrop []string + NameDropSeparators []rune + nameDropFilter filter.Filter + NamePass []string + NamePassSeparators []rune + namePassFilter filter.Filter FieldDrop []string fieldDropFilter filter.Filter @@ -78,11 +80,11 @@ func (f *Filter) Compile() error { if f.selectActive { var err error - f.nameDropFilter, err = filter.Compile(f.NameDrop) + f.nameDropFilter, err = filter.Compile(f.NameDrop, f.NameDropSeparators...) if err != nil { return fmt.Errorf("error compiling 'namedrop', %w", err) } - f.namePassFilter, err = filter.Compile(f.NamePass) + f.namePassFilter, err = filter.Compile(f.NamePass, f.NamePassSeparators...) if err != nil { return fmt.Errorf("error compiling 'namepass', %w", err) } From 646e4c74ed3dd85df796bd51885e154c717701af Mon Sep 17 00:00:00 2001 From: Akash Gupta Date: Tue, 28 Nov 2023 11:09:13 +0530 Subject: [PATCH 2/7] fix separator parser to take string of separators instead of array of characters --- config/config.go | 10 +++------- 1 file changed, 3 insertions(+), 7 deletions(-) diff --git a/config/config.go b/config/config.go index d5ba28202285d..e6517b72ccc6a 100644 --- a/config/config.go +++ b/config/config.go @@ -1587,16 +1587,12 @@ func (c *Config) getFieldInt64(tbl *ast.Table, fieldName string, target *int64) func (c *Config) getFieldRuneSlice(tbl *ast.Table, fieldName string, target *[]rune) { if node, ok := tbl.Fields[fieldName]; ok { if kv, ok := node.(*ast.KeyValue); ok { - ary, ok := kv.Value.(*ast.Array) + str, ok := kv.Value.(*ast.String) if !ok { - c.addError(tbl, fmt.Errorf("found unexpected format while parsing %q, expecting string slice format", fieldName)) + c.addError(tbl, fmt.Errorf("found unexpected format while parsing %q, expecting string format", fieldName)) return } - for _, elem := range ary.Value { - if str, ok := elem.(*ast.String); ok { - *target = append(*target, []rune(str.Value)...) - } - } + *target = []rune(str.Value) } } } From 09166bd8126120a938a35b396ad1eabd55b91435 Mon Sep 17 00:00:00 2001 From: Akash Gupta Date: Tue, 28 Nov 2023 12:45:06 +0530 Subject: [PATCH 3/7] Add tests for the changes and description comments in filter.go --- config/config_test.go | 40 +++++++----- config/testdata/single_plugin.toml | 2 + config/testdata/single_plugin_env_vars.toml | 2 + config/testdata/subconfig/memcached.conf | 2 + filter/filter.go | 16 +++-- filter/filter_test.go | 12 ++++ models/filter_test.go | 72 +++++++++++++++++++++ 7 files changed, 126 insertions(+), 20 deletions(-) diff --git a/config/config_test.go b/config/config_test.go index 870e7a2b8939e..41e7c68709653 100644 --- a/config/config_test.go +++ b/config/config_test.go @@ -76,10 +76,12 @@ func TestConfig_LoadSingleInputWithEnvVars(t *testing.T) { # is unique` filter := models.Filter{ - NameDrop: []string{"metricname2"}, - NamePass: []string{"metricname1", "ip_192.168.1.1_name"}, - FieldDrop: []string{"other", "stuff"}, - FieldPass: []string{"some", "strings"}, + NameDrop: []string{"metricname2"}, + NameDropSeparators: []rune{'.'}, + NamePass: []string{"metricname1", "ip_192.168.1.1_name"}, + NamePassSeparators: []rune{'.'}, + FieldDrop: []string{"other", "stuff"}, + FieldPass: []string{"some", "strings"}, TagDropFilters: []models.TagFilter{ { Name: "badtag", @@ -117,10 +119,12 @@ func TestConfig_LoadSingleInput(t *testing.T) { input.Servers = []string{"localhost"} filter := models.Filter{ - NameDrop: []string{"metricname2"}, - NamePass: []string{"metricname1"}, - FieldDrop: []string{"other", "stuff"}, - FieldPass: []string{"some", "strings"}, + NameDrop: []string{"metricname2"}, + NameDropSeparators: []rune{'.'}, + NamePass: []string{"metricname1"}, + NamePassSeparators: []rune{'.'}, + FieldDrop: []string{"other", "stuff"}, + FieldPass: []string{"some", "strings"}, TagDropFilters: []models.TagFilter{ { Name: "badtag", @@ -166,10 +170,12 @@ func TestConfig_LoadDirectory(t *testing.T) { expectedPlugins[0].Servers = []string{"localhost"} filterMockup := models.Filter{ - NameDrop: []string{"metricname2"}, - NamePass: []string{"metricname1"}, - FieldDrop: []string{"other", "stuff"}, - FieldPass: []string{"some", "strings"}, + NameDrop: []string{"metricname2"}, + NameDropSeparators: []rune{'.'}, + NamePass: []string{"metricname1"}, + NamePassSeparators: []rune{'.'}, + FieldDrop: []string{"other", "stuff"}, + FieldPass: []string{"some", "strings"}, TagDropFilters: []models.TagFilter{ { Name: "badtag", @@ -210,10 +216,12 @@ func TestConfig_LoadDirectory(t *testing.T) { expectedPlugins[2].Servers = []string{"192.168.1.1"} filterMemcached := models.Filter{ - NameDrop: []string{"metricname2"}, - NamePass: []string{"metricname1"}, - FieldDrop: []string{"other", "stuff"}, - FieldPass: []string{"some", "strings"}, + NameDrop: []string{"metricname2"}, + NameDropSeparators: []rune{'.'}, + NamePass: []string{"metricname1"}, + NamePassSeparators: []rune{'.'}, + FieldDrop: []string{"other", "stuff"}, + FieldPass: []string{"some", "strings"}, TagDropFilters: []models.TagFilter{ { Name: "badtag", diff --git a/config/testdata/single_plugin.toml b/config/testdata/single_plugin.toml index 664937b254353..acac08c1e7e47 100644 --- a/config/testdata/single_plugin.toml +++ b/config/testdata/single_plugin.toml @@ -1,7 +1,9 @@ [[inputs.memcached]] servers = ["localhost"] namepass = ["metricname1"] + namepass_separator = "." namedrop = ["metricname2"] + namedrop_separator = "." fieldpass = ["some", "strings"] fielddrop = ["other", "stuff"] interval = "5s" diff --git a/config/testdata/single_plugin_env_vars.toml b/config/testdata/single_plugin_env_vars.toml index 16059342afd33..8ff4866516d40 100644 --- a/config/testdata/single_plugin_env_vars.toml +++ b/config/testdata/single_plugin_env_vars.toml @@ -17,7 +17,9 @@ # this comment line will be ignored by the parser servers = ["$MY_TEST_SERVER"] namepass = ["metricname1", "ip_${MY_TEST_SERVER}_name"] # this comment will be ignored as well + namepass_separator = "." namedrop = ["metricname2"] + namedrop_separator = "." fieldpass = ["some", "strings"] fielddrop = ["other", "stuff"] interval = "$TEST_INTERVAL" diff --git a/config/testdata/subconfig/memcached.conf b/config/testdata/subconfig/memcached.conf index 2cd07d15d331d..5be1f4600bf1f 100644 --- a/config/testdata/subconfig/memcached.conf +++ b/config/testdata/subconfig/memcached.conf @@ -1,7 +1,9 @@ [[inputs.memcached]] servers = ["192.168.1.1"] namepass = ["metricname1"] + namepass_separator = "." namedrop = ["metricname2"] + namedrop_separator = "." pass = ["some", "strings"] drop = ["other", "stuff"] interval = "5s" diff --git a/filter/filter.go b/filter/filter.go index 495099670d016..bf22b48124e59 100644 --- a/filter/filter.go +++ b/filter/filter.go @@ -12,12 +12,21 @@ type 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: +// supports glob matching with separators too, ie: // // f, _ := Compile([]string{"cpu", "mem", "net*"}) // f.Match("cpu") // true // f.Match("network") // true // f.Match("memory") // false +// +// separators are only to be used for globbing filters, ie: +// +// f, _ := Compile([]string{"cpu.*.count"}, '.') +// f.Match("cpu.count") // false +// f.Match("cpu.measurement.count") // true +// f.Match("cpu.field.measurement.count") // false +// +// Compile will return nil if the filter list is empty. func Compile(filters []string, separators ...rune) (Filter, error) { // return if there is nothing to compile if len(filters) == 0 { @@ -106,14 +115,13 @@ func NewIncludeExcludeFilterDefaults( exclude []string, includeDefault bool, excludeDefault bool, - separators ...rune, ) (Filter, error) { - in, err := Compile(include, separators...) + in, err := Compile(include) if err != nil { return nil, err } - ex, err := Compile(exclude, separators...) + ex, err := Compile(exclude) if err != nil { return nil, err } diff --git a/filter/filter_test.go b/filter/filter_test.go index 78918948a8998..4268ccbbcae61 100644 --- a/filter/filter_test.go +++ b/filter/filter_test.go @@ -35,6 +35,18 @@ func TestCompile(t *testing.T) { require.False(t, f.Match("cpu0")) require.True(t, f.Match("mem")) require.True(t, f.Match("network")) + + f, err = Compile([]string{"cpu.*.count"}, '.') + require.NoError(t, err) + require.False(t, f.Match("cpu.count")) + require.True(t, f.Match("cpu.measurement.count")) + require.False(t, f.Match("cpu.field.measurement.count")) + + f, err = Compile([]string{"cpu.*.count"}, '.', ',') + require.NoError(t, err) + require.True(t, f.Match("cpu.measurement.count")) + require.False(t, f.Match("cpu.,.count")) // ',' is not considered under * as it is specified as a separator + require.False(t, f.Match("cpu.field,measurement.count")) } func TestIncludeExclude(t *testing.T) { diff --git a/models/filter_test.go b/models/filter_test.go index 80f976809f72b..2b90f64e33bb3 100644 --- a/models/filter_test.go +++ b/models/filter_test.go @@ -146,6 +146,42 @@ func TestFilter_NamePass(t *testing.T) { } } +func TestFilter_NamePass_WithSeparator(t *testing.T) { + f := Filter{ + NamePass: []string{"foo.*.bar", "foo.*.abc.*.bar"}, + NamePassSeparators: []rune{'.', ','}, + } + require.NoError(t, f.Compile()) + + passes := []string{ + "foo..bar", + "foo.abc.bar", + "foo..abc..bar", + "foo.xyz.abc.xyz-xyz.bar", + } + + drops := []string{ + "foo.bar", + "foo.abc,.bar", // "abc," is not considered under * as ',' is specified as a separator + "foo..abc.bar", // ".abc" shall not be matched under * as '.' is specified as a separator + "foo.abc.abc.bar", + "foo.xyz.abc.xyz.xyz.bar", + "foo.xyz.abc.xyz,xyz.bar", + } + + for _, measurement := range passes { + if !f.shouldNamePass(measurement) { + t.Errorf("Expected measurement %s to pass", measurement) + } + } + + for _, measurement := range drops { + if f.shouldNamePass(measurement) { + t.Errorf("Expected measurement %s to drop", measurement) + } + } +} + func TestFilter_NameDrop(t *testing.T) { f := Filter{ NameDrop: []string{"foo*", "cpu_usage_idle"}, @@ -180,6 +216,42 @@ func TestFilter_NameDrop(t *testing.T) { } } +func TestFilter_NameDrop_WithSeparator(t *testing.T) { + f := Filter{ + NameDrop: []string{"foo.*.bar", "foo.*.abc.*.bar"}, + NameDropSeparators: []rune{'.', ','}, + } + require.NoError(t, f.Compile()) + + drops := []string{ + "foo..bar", + "foo.abc.bar", + "foo..abc..bar", + "foo.xyz.abc.xyz-xyz.bar", + } + + passes := []string{ + "foo.bar", + "foo.abc,.bar", // "abc," is not considered under * as ',' is specified as a separator + "foo..abc.bar", // ".abc" shall not be matched under * as '.' is specified as a separator + "foo.abc.abc.bar", + "foo.xyz.abc.xyz.xyz.bar", + "foo.xyz.abc.xyz,xyz.bar", + } + + for _, measurement := range passes { + if !f.shouldNamePass(measurement) { + t.Errorf("Expected measurement %s to pass", measurement) + } + } + + for _, measurement := range drops { + if f.shouldNamePass(measurement) { + t.Errorf("Expected measurement %s to drop", measurement) + } + } +} + func TestFilter_FieldPass(t *testing.T) { f := Filter{ FieldPass: []string{"foo*", "cpu_usage_idle"}, From 0ea3f4b17c0808134d8930059c67bfe95132d62f Mon Sep 17 00:00:00 2001 From: Akash Gupta Date: Tue, 28 Nov 2023 13:04:32 +0530 Subject: [PATCH 4/7] Add documentation with example on how to use namepass/namedrop separators. --- docs/CONFIGURATION.md | 28 ++++++++++++++++++++++++++-- 1 file changed, 26 insertions(+), 2 deletions(-) diff --git a/docs/CONFIGURATION.md b/docs/CONFIGURATION.md index dc00fc8bfa726..47df42b8be0a6 100644 --- a/docs/CONFIGURATION.md +++ b/docs/CONFIGURATION.md @@ -626,11 +626,15 @@ sent onwards to the next stage of processing. - **namepass**: An array of [glob pattern][] strings. Only metrics whose measurement name -matches a pattern in this list are emitted. +matches a pattern in this list are emitted. Additionally, custom list of +separators can be specified using **namepass_separator**. These separators +are excluded from wildcard glob pattern matching. - **namedrop**: The inverse of `namepass`. If a match is found the metric is discarded. This -is tested on metrics after they have passed the `namepass` test. +is tested on metrics after they have passed the `namepass` test. Additionally, +custom list of separators can be specified using **namedrop_separator**. These +separators are excluded from wildcard glob pattern matching. - **tagpass**: A table mapping tag keys to arrays of [glob pattern][] strings. Only metrics @@ -770,6 +774,26 @@ tags and the agent `host` tag. namepass = ["rest_client_*"] ``` +#### Using namepass and namedrop with separators + +```toml +# Pass all metrics of type 'A.C.B' and drop all others like 'A.C.D.B' +[[inputs.socket_listener]] + data_format = "graphite" + templates = ["measurement*"] + + namepass = ["A.*.B"] + namepass_separator = "." + +# Drop all metrics of type 'A.C.B' and pass all others like 'A.C.D.B' +[[inputs.socket_listener]] + data_format = "graphite" + templates = ["measurement*"] + + namedrop = ["A.*.B"] + namedrop_separator = "." +``` + #### Using taginclude and tagexclude ```toml From ba933196d1328755ba96c3587321d92a53c37765 Mon Sep 17 00:00:00 2001 From: Akash Gupta Date: Tue, 28 Nov 2023 13:18:58 +0530 Subject: [PATCH 5/7] fix minor MD lint error --- docs/CONFIGURATION.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/CONFIGURATION.md b/docs/CONFIGURATION.md index 47df42b8be0a6..2e1832da73ce9 100644 --- a/docs/CONFIGURATION.md +++ b/docs/CONFIGURATION.md @@ -626,7 +626,7 @@ sent onwards to the next stage of processing. - **namepass**: An array of [glob pattern][] strings. Only metrics whose measurement name -matches a pattern in this list are emitted. Additionally, custom list of +matches a pattern in this list are emitted. Additionally, custom list of separators can be specified using **namepass_separator**. These separators are excluded from wildcard glob pattern matching. From f037f0c9fad2bebf13c5563ced477e030d47ba65 Mon Sep 17 00:00:00 2001 From: Akash Gupta Date: Wed, 29 Nov 2023 10:33:07 +0530 Subject: [PATCH 6/7] addresses PR review comments --- config/config.go | 17 ++--------------- config/config_test.go | 16 ++++++++-------- filter/filter.go | 10 ++++------ models/filter.go | 8 ++++---- models/filter_test.go | 4 ++-- 5 files changed, 20 insertions(+), 35 deletions(-) diff --git a/config/config.go b/config/config.go index e6517b72ccc6a..bab955e932616 100644 --- a/config/config.go +++ b/config/config.go @@ -1331,9 +1331,9 @@ func (c *Config) buildFilter(tbl *ast.Table) (models.Filter, error) { f := models.Filter{} c.getFieldStringSlice(tbl, "namepass", &f.NamePass) - c.getFieldRuneSlice(tbl, "namepass_separator", &f.NamePassSeparators) + c.getFieldString(tbl, "namepass_separator", &f.NamePassSeparators) c.getFieldStringSlice(tbl, "namedrop", &f.NameDrop) - c.getFieldRuneSlice(tbl, "namedrop_separator", &f.NameDropSeparators) + c.getFieldString(tbl, "namedrop_separator", &f.NameDropSeparators) c.getFieldStringSlice(tbl, "pass", &f.FieldPass) c.getFieldStringSlice(tbl, "fieldpass", &f.FieldPass) @@ -1584,19 +1584,6 @@ func (c *Config) getFieldInt64(tbl *ast.Table, fieldName string, target *int64) } } -func (c *Config) getFieldRuneSlice(tbl *ast.Table, fieldName string, target *[]rune) { - if node, ok := tbl.Fields[fieldName]; ok { - if kv, ok := node.(*ast.KeyValue); ok { - str, ok := kv.Value.(*ast.String) - if !ok { - c.addError(tbl, fmt.Errorf("found unexpected format while parsing %q, expecting string format", fieldName)) - return - } - *target = []rune(str.Value) - } - } -} - func (c *Config) getFieldStringSlice(tbl *ast.Table, fieldName string, target *[]string) { if node, ok := tbl.Fields[fieldName]; ok { if kv, ok := node.(*ast.KeyValue); ok { diff --git a/config/config_test.go b/config/config_test.go index 41e7c68709653..d99e11b62c9e1 100644 --- a/config/config_test.go +++ b/config/config_test.go @@ -77,9 +77,9 @@ func TestConfig_LoadSingleInputWithEnvVars(t *testing.T) { filter := models.Filter{ NameDrop: []string{"metricname2"}, - NameDropSeparators: []rune{'.'}, + NameDropSeparators: ".", NamePass: []string{"metricname1", "ip_192.168.1.1_name"}, - NamePassSeparators: []rune{'.'}, + NamePassSeparators: ".", FieldDrop: []string{"other", "stuff"}, FieldPass: []string{"some", "strings"}, TagDropFilters: []models.TagFilter{ @@ -120,9 +120,9 @@ func TestConfig_LoadSingleInput(t *testing.T) { filter := models.Filter{ NameDrop: []string{"metricname2"}, - NameDropSeparators: []rune{'.'}, + NameDropSeparators: ".", NamePass: []string{"metricname1"}, - NamePassSeparators: []rune{'.'}, + NamePassSeparators: ".", FieldDrop: []string{"other", "stuff"}, FieldPass: []string{"some", "strings"}, TagDropFilters: []models.TagFilter{ @@ -171,9 +171,9 @@ func TestConfig_LoadDirectory(t *testing.T) { filterMockup := models.Filter{ NameDrop: []string{"metricname2"}, - NameDropSeparators: []rune{'.'}, + NameDropSeparators: ".", NamePass: []string{"metricname1"}, - NamePassSeparators: []rune{'.'}, + NamePassSeparators: ".", FieldDrop: []string{"other", "stuff"}, FieldPass: []string{"some", "strings"}, TagDropFilters: []models.TagFilter{ @@ -217,9 +217,9 @@ func TestConfig_LoadDirectory(t *testing.T) { filterMemcached := models.Filter{ NameDrop: []string{"metricname2"}, - NameDropSeparators: []rune{'.'}, + NameDropSeparators: ".", NamePass: []string{"metricname1"}, - NamePassSeparators: []rune{'.'}, + NamePassSeparators: ".", FieldDrop: []string{"other", "stuff"}, FieldPass: []string{"some", "strings"}, TagDropFilters: []models.TagFilter{ diff --git a/filter/filter.go b/filter/filter.go index bf22b48124e59..88e946180ad44 100644 --- a/filter/filter.go +++ b/filter/filter.go @@ -35,12 +35,10 @@ func Compile(filters []string, separators ...rune) (Filter, error) { // check if we can compile a non-glob filter noGlob := len(separators) == 0 - if noGlob { - for _, filter := range filters { - if hasMeta(filter) { - noGlob = false - break - } + for _, filter := range filters { + if hasMeta(filter) { + noGlob = false + break } } diff --git a/models/filter.go b/models/filter.go index 1e152d006b297..5c55ae3224cbe 100644 --- a/models/filter.go +++ b/models/filter.go @@ -34,10 +34,10 @@ func (tf *TagFilter) Compile() error { // Filter containing drop/pass and tagdrop/tagpass rules type Filter struct { NameDrop []string - NameDropSeparators []rune + NameDropSeparators string nameDropFilter filter.Filter NamePass []string - NamePassSeparators []rune + NamePassSeparators string namePassFilter filter.Filter FieldDrop []string @@ -80,11 +80,11 @@ func (f *Filter) Compile() error { if f.selectActive { var err error - f.nameDropFilter, err = filter.Compile(f.NameDrop, f.NameDropSeparators...) + f.nameDropFilter, err = filter.Compile(f.NameDrop, []rune(f.NameDropSeparators)...) if err != nil { return fmt.Errorf("error compiling 'namedrop', %w", err) } - f.namePassFilter, err = filter.Compile(f.NamePass, f.NamePassSeparators...) + f.namePassFilter, err = filter.Compile(f.NamePass, []rune(f.NamePassSeparators)...) if err != nil { return fmt.Errorf("error compiling 'namepass', %w", err) } diff --git a/models/filter_test.go b/models/filter_test.go index 2b90f64e33bb3..a1586a5d6540d 100644 --- a/models/filter_test.go +++ b/models/filter_test.go @@ -149,7 +149,7 @@ func TestFilter_NamePass(t *testing.T) { func TestFilter_NamePass_WithSeparator(t *testing.T) { f := Filter{ NamePass: []string{"foo.*.bar", "foo.*.abc.*.bar"}, - NamePassSeparators: []rune{'.', ','}, + NamePassSeparators: ".,", } require.NoError(t, f.Compile()) @@ -219,7 +219,7 @@ func TestFilter_NameDrop(t *testing.T) { func TestFilter_NameDrop_WithSeparator(t *testing.T) { f := Filter{ NameDrop: []string{"foo.*.bar", "foo.*.abc.*.bar"}, - NameDropSeparators: []rune{'.', ','}, + NameDropSeparators: ".,", } require.NoError(t, f.Compile()) From c9e341e33487c79ad8c11f000e652830b774a7d6 Mon Sep 17 00:00:00 2001 From: Akash Gupta Date: Thu, 30 Nov 2023 16:44:48 +0530 Subject: [PATCH 7/7] add separate test instead of changing older ones to maintain backward compatibility --- config/config_test.go | 71 ++++++++++++++----- config/testdata/single_plugin.toml | 2 - config/testdata/single_plugin_env_vars.toml | 2 - .../single_plugin_with_separators.toml | 13 ++++ config/testdata/subconfig/memcached.conf | 2 - docs/CONFIGURATION.md | 4 +- 6 files changed, 68 insertions(+), 26 deletions(-) create mode 100644 config/testdata/single_plugin_with_separators.toml diff --git a/config/config_test.go b/config/config_test.go index d99e11b62c9e1..aba436f2d52bb 100644 --- a/config/config_test.go +++ b/config/config_test.go @@ -76,12 +76,10 @@ func TestConfig_LoadSingleInputWithEnvVars(t *testing.T) { # is unique` filter := models.Filter{ - NameDrop: []string{"metricname2"}, - NameDropSeparators: ".", - NamePass: []string{"metricname1", "ip_192.168.1.1_name"}, - NamePassSeparators: ".", - FieldDrop: []string{"other", "stuff"}, - FieldPass: []string{"some", "strings"}, + NameDrop: []string{"metricname2"}, + NamePass: []string{"metricname1", "ip_192.168.1.1_name"}, + FieldDrop: []string{"other", "stuff"}, + FieldPass: []string{"some", "strings"}, TagDropFilters: []models.TagFilter{ { Name: "badtag", @@ -118,6 +116,47 @@ func TestConfig_LoadSingleInput(t *testing.T) { input := inputs.Inputs["memcached"]().(*MockupInputPlugin) input.Servers = []string{"localhost"} + filter := models.Filter{ + NameDrop: []string{"metricname2"}, + NamePass: []string{"metricname1"}, + FieldDrop: []string{"other", "stuff"}, + FieldPass: []string{"some", "strings"}, + TagDropFilters: []models.TagFilter{ + { + Name: "badtag", + Values: []string{"othertag"}, + }, + }, + TagPassFilters: []models.TagFilter{ + { + Name: "goodtag", + Values: []string{"mytag"}, + }, + }, + } + require.NoError(t, filter.Compile()) + inputConfig := &models.InputConfig{ + Name: "memcached", + Filter: filter, + Interval: 5 * time.Second, + } + inputConfig.Tags = make(map[string]string) + + // Ignore Log, Parser and ID + c.Inputs[0].Input.(*MockupInputPlugin).Log = nil + c.Inputs[0].Input.(*MockupInputPlugin).parser = nil + c.Inputs[0].Config.ID = "" + require.Equal(t, input, c.Inputs[0].Input, "Testdata did not produce a correct memcached struct.") + require.Equal(t, inputConfig, c.Inputs[0].Config, "Testdata did not produce correct memcached metadata.") +} + +func TestConfig_LoadSingleInput_WithSeparators(t *testing.T) { + c := config.NewConfig() + require.NoError(t, c.LoadConfig("./testdata/single_plugin_with_separators.toml")) + + input := inputs.Inputs["memcached"]().(*MockupInputPlugin) + input.Servers = []string{"localhost"} + filter := models.Filter{ NameDrop: []string{"metricname2"}, NameDropSeparators: ".", @@ -170,12 +209,10 @@ func TestConfig_LoadDirectory(t *testing.T) { expectedPlugins[0].Servers = []string{"localhost"} filterMockup := models.Filter{ - NameDrop: []string{"metricname2"}, - NameDropSeparators: ".", - NamePass: []string{"metricname1"}, - NamePassSeparators: ".", - FieldDrop: []string{"other", "stuff"}, - FieldPass: []string{"some", "strings"}, + NameDrop: []string{"metricname2"}, + NamePass: []string{"metricname1"}, + FieldDrop: []string{"other", "stuff"}, + FieldPass: []string{"some", "strings"}, TagDropFilters: []models.TagFilter{ { Name: "badtag", @@ -216,12 +253,10 @@ func TestConfig_LoadDirectory(t *testing.T) { expectedPlugins[2].Servers = []string{"192.168.1.1"} filterMemcached := models.Filter{ - NameDrop: []string{"metricname2"}, - NameDropSeparators: ".", - NamePass: []string{"metricname1"}, - NamePassSeparators: ".", - FieldDrop: []string{"other", "stuff"}, - FieldPass: []string{"some", "strings"}, + NameDrop: []string{"metricname2"}, + NamePass: []string{"metricname1"}, + FieldDrop: []string{"other", "stuff"}, + FieldPass: []string{"some", "strings"}, TagDropFilters: []models.TagFilter{ { Name: "badtag", diff --git a/config/testdata/single_plugin.toml b/config/testdata/single_plugin.toml index acac08c1e7e47..664937b254353 100644 --- a/config/testdata/single_plugin.toml +++ b/config/testdata/single_plugin.toml @@ -1,9 +1,7 @@ [[inputs.memcached]] servers = ["localhost"] namepass = ["metricname1"] - namepass_separator = "." namedrop = ["metricname2"] - namedrop_separator = "." fieldpass = ["some", "strings"] fielddrop = ["other", "stuff"] interval = "5s" diff --git a/config/testdata/single_plugin_env_vars.toml b/config/testdata/single_plugin_env_vars.toml index 8ff4866516d40..16059342afd33 100644 --- a/config/testdata/single_plugin_env_vars.toml +++ b/config/testdata/single_plugin_env_vars.toml @@ -17,9 +17,7 @@ # this comment line will be ignored by the parser servers = ["$MY_TEST_SERVER"] namepass = ["metricname1", "ip_${MY_TEST_SERVER}_name"] # this comment will be ignored as well - namepass_separator = "." namedrop = ["metricname2"] - namedrop_separator = "." fieldpass = ["some", "strings"] fielddrop = ["other", "stuff"] interval = "$TEST_INTERVAL" diff --git a/config/testdata/single_plugin_with_separators.toml b/config/testdata/single_plugin_with_separators.toml new file mode 100644 index 0000000000000..acac08c1e7e47 --- /dev/null +++ b/config/testdata/single_plugin_with_separators.toml @@ -0,0 +1,13 @@ +[[inputs.memcached]] + servers = ["localhost"] + namepass = ["metricname1"] + namepass_separator = "." + namedrop = ["metricname2"] + namedrop_separator = "." + fieldpass = ["some", "strings"] + fielddrop = ["other", "stuff"] + interval = "5s" + [inputs.memcached.tagpass] + goodtag = ["mytag"] + [inputs.memcached.tagdrop] + badtag = ["othertag"] diff --git a/config/testdata/subconfig/memcached.conf b/config/testdata/subconfig/memcached.conf index 5be1f4600bf1f..2cd07d15d331d 100644 --- a/config/testdata/subconfig/memcached.conf +++ b/config/testdata/subconfig/memcached.conf @@ -1,9 +1,7 @@ [[inputs.memcached]] servers = ["192.168.1.1"] namepass = ["metricname1"] - namepass_separator = "." namedrop = ["metricname2"] - namedrop_separator = "." pass = ["some", "strings"] drop = ["other", "stuff"] interval = "5s" diff --git a/docs/CONFIGURATION.md b/docs/CONFIGURATION.md index 2e1832da73ce9..36cd80fc80865 100644 --- a/docs/CONFIGURATION.md +++ b/docs/CONFIGURATION.md @@ -627,13 +627,13 @@ sent onwards to the next stage of processing. - **namepass**: An array of [glob pattern][] strings. Only metrics whose measurement name matches a pattern in this list are emitted. Additionally, custom list of -separators can be specified using **namepass_separator**. These separators +separators can be specified using `namepass_separator`. These separators are excluded from wildcard glob pattern matching. - **namedrop**: The inverse of `namepass`. If a match is found the metric is discarded. This is tested on metrics after they have passed the `namepass` test. Additionally, -custom list of separators can be specified using **namedrop_separator**. These +custom list of separators can be specified using `namedrop_separator`. These separators are excluded from wildcard glob pattern matching. - **tagpass**: