From 347083140ff5eb6f7f43f9d1f0e69402bd8b2d4e Mon Sep 17 00:00:00 2001 From: Tigran Najaryan <4194920+tigrannajaryan@users.noreply.github.com> Date: Tue, 8 Feb 2022 15:06:19 -0500 Subject: [PATCH] Deprecate log_names setting from filter processor (#7552) The LogRecord Name field [is removed](https://github.com/open-telemetry/opentelemetry-specification/pull/2271) from the specification and is [going to be removed](https://github.com/open-telemetry/opentelemetry-proto/pull/357) from the OTLP. --- CHANGELOG.md | 2 + .../processor/filterconfig/config.go | 5 +- .../processor/filterlog/filterlog_test.go | 72 ++++++++----------- .../filtermatcher/attributematcher.go | 4 +- .../processor/filterset/config.go | 6 +- .../attributes_log_test.go | 36 +++++----- processor/attributesprocessor/factory.go | 7 +- processor/spanprocessor/span_test.go | 3 + 8 files changed, 70 insertions(+), 65 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 8a72e6c70070..5cef157da242 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,6 +8,8 @@ ## 🚩 Deprecations 🚩 +- Deprecated log_names setting from filter processor. (#7552) + ## 🧰 Bug fixes 🧰 - `tailsamplingprocessor`: "And" policy only works as a sub policy under a composite policy (#7590) diff --git a/internal/coreinternal/processor/filterconfig/config.go b/internal/coreinternal/processor/filterconfig/config.go index ebde3f9c688d..02729593e0b3 100644 --- a/internal/coreinternal/processor/filterconfig/config.go +++ b/internal/coreinternal/processor/filterconfig/config.go @@ -89,6 +89,7 @@ type MatchProperties struct { // LogNames is a list of strings that the LogRecord's name field must match // against. + // Deprecated: the Name field is removed from the log data model. LogNames []string `mapstructure:"log_names"` // Attributes specifies the list of attributes to match against. @@ -128,8 +129,8 @@ func (mp *MatchProperties) ValidateForLogs() error { return errors.New("neither services nor span_names should be specified for log records") } - if len(mp.LogNames) == 0 && len(mp.Attributes) == 0 && len(mp.Libraries) == 0 && len(mp.Resources) == 0 { - return errors.New(`at least one of "log_names", "attributes", "libraries" or "resources" field must be specified`) + if len(mp.Attributes) == 0 && len(mp.Libraries) == 0 && len(mp.Resources) == 0 { + return errors.New(`at least one of "attributes", "libraries" or "resources" field must be specified`) } return nil diff --git a/internal/coreinternal/processor/filterlog/filterlog_test.go b/internal/coreinternal/processor/filterlog/filterlog_test.go index 426f9cd0b61a..2c3ad55abf01 100644 --- a/internal/coreinternal/processor/filterlog/filterlog_test.go +++ b/internal/coreinternal/processor/filterlog/filterlog_test.go @@ -40,14 +40,14 @@ func TestLogRecord_validateMatchesConfiguration_InvalidConfig(t *testing.T) { { name: "empty_property", property: filterconfig.MatchProperties{}, - errorString: "at least one of \"log_names\", \"attributes\", \"libraries\" or \"resources\" field must be specified", + errorString: "at least one of \"attributes\", \"libraries\" or \"resources\" field must be specified", }, { name: "empty_log_names_and_attributes", property: filterconfig.MatchProperties{ LogNames: []string{}, }, - errorString: "at least one of \"log_names\", \"attributes\", \"libraries\" or \"resources\" field must be specified", + errorString: "at least one of \"attributes\", \"libraries\" or \"resources\" field must be specified", }, { name: "span_properties", @@ -59,33 +59,25 @@ func TestLogRecord_validateMatchesConfiguration_InvalidConfig(t *testing.T) { { name: "invalid_match_type", property: filterconfig.MatchProperties{ - Config: *createConfig("wrong_match_type"), - LogNames: []string{"abc"}, + Config: *createConfig("wrong_match_type"), + Attributes: []filterconfig.Attribute{{Key: "abc", Value: "def"}}, }, - errorString: "error creating log record name filters: unrecognized match_type: 'wrong_match_type', valid types are: [regexp strict]", + errorString: "error creating attribute filters: unrecognized match_type: 'wrong_match_type', valid types are: [regexp strict]", }, { name: "missing_match_type", property: filterconfig.MatchProperties{ - LogNames: []string{"abc"}, + Attributes: []filterconfig.Attribute{{Key: "abc", Value: "def"}}, }, - errorString: "error creating log record name filters: unrecognized match_type: '', valid types are: [regexp strict]", + errorString: "error creating attribute filters: unrecognized match_type: '', valid types are: [regexp strict]", }, { name: "invalid_regexp_pattern", property: filterconfig.MatchProperties{ - Config: *createConfig(filterset.Regexp), - LogNames: []string{"["}, - }, - errorString: "error creating log record name filters: error parsing regexp: missing closing ]: `[`", - }, - { - name: "invalid_regexp_pattern2", - property: filterconfig.MatchProperties{ - Config: *createConfig(filterset.Regexp), - LogNames: []string{"["}, + Config: *createConfig(filterset.Regexp), + Attributes: []filterconfig.Attribute{{Key: "abc", Value: "["}}, }, - errorString: "error creating log record name filters: error parsing regexp: missing closing ]: `[`", + errorString: "error creating attribute filters: error parsing regexp: missing closing ]: `[`", }, } for _, tc := range testcases { @@ -104,35 +96,32 @@ func TestLogRecord_Matching_False(t *testing.T) { properties *filterconfig.MatchProperties }{ { - name: "log_name_doesnt_match", + name: "attributes_dont_match", properties: &filterconfig.MatchProperties{ - Config: *createConfig(filterset.Regexp), - LogNames: []string{"logNo.*Name"}, - Attributes: []filterconfig.Attribute{}, + Config: *createConfig(filterset.Regexp), + Attributes: []filterconfig.Attribute{ + {Key: "abc", Value: "def"}, + }, }, }, { - name: "log_name_doesnt_match_any", + name: "attributes_dont_match_regex", properties: &filterconfig.MatchProperties{ Config: *createConfig(filterset.Regexp), - LogNames: []string{ - "logNo.*Name", - "non-matching?pattern", - "regular string", + Attributes: []filterconfig.Attribute{ + {Key: "ab.*c", Value: "def"}, }, - Attributes: []filterconfig.Attribute{}, }, }, } lr := pdata.NewLogRecord() - lr.SetName("logName") for _, tc := range testcases { t.Run(tc.name, func(t *testing.T) { matcher, err := NewMatcher(tc.properties) assert.Nil(t, err) - assert.NotNil(t, matcher) + require.NotNil(t, matcher) assert.False(t, matcher.MatchLogRecord(lr, pdata.Resource{}, pdata.InstrumentationLibrary{})) }) @@ -145,36 +134,31 @@ func TestLogRecord_Matching_True(t *testing.T) { properties *filterconfig.MatchProperties }{ { - name: "log_name_match", + name: "attribute_strict_match", properties: &filterconfig.MatchProperties{ - Config: *createConfig(filterset.Regexp), - LogNames: []string{"log.*"}, - Attributes: []filterconfig.Attribute{}, + Config: *createConfig(filterset.Strict), + Attributes: []filterconfig.Attribute{{Key: "abc", Value: "def"}}, }, }, { - name: "log_name_second_match", + name: "attribute_regex_match", properties: &filterconfig.MatchProperties{ Config: *createConfig(filterset.Regexp), - LogNames: []string{ - "wrong.*pattern", - "log.*", - "yet another?pattern", - "regularstring", + Attributes: []filterconfig.Attribute{ + {Key: "abc", Value: "d.f"}, }, - Attributes: []filterconfig.Attribute{}, }, }, } lr := pdata.NewLogRecord() - lr.SetName("logName") + lr.Attributes().InsertString("abc", "def") for _, tc := range testcases { t.Run(tc.name, func(t *testing.T) { mp, err := NewMatcher(tc.properties) - assert.Nil(t, err) - assert.NotNil(t, mp) + assert.NoError(t, err) + require.NotNil(t, mp) assert.NotNil(t, lr) assert.True(t, mp.MatchLogRecord(lr, pdata.Resource{}, pdata.InstrumentationLibrary{})) diff --git a/internal/coreinternal/processor/filtermatcher/attributematcher.go b/internal/coreinternal/processor/filtermatcher/attributematcher.go index 75c89f2298f3..76372f07f1f0 100644 --- a/internal/coreinternal/processor/filtermatcher/attributematcher.go +++ b/internal/coreinternal/processor/filtermatcher/attributematcher.go @@ -70,8 +70,10 @@ func NewAttributesMatcher(config filterset.Config, attributes []filterconfig.Att return nil, err } entry.StringFilter = filter - } else { + } else if config.MatchType == filterset.Strict { entry.AttributeValue = &val + } else { + return nil, filterset.NewUnrecognizedMatchTypeError(config.MatchType) } } diff --git a/internal/coreinternal/processor/filterset/config.go b/internal/coreinternal/processor/filterset/config.go index 669537d9d0aa..cc8cd26685fb 100644 --- a/internal/coreinternal/processor/filterset/config.go +++ b/internal/coreinternal/processor/filterset/config.go @@ -43,6 +43,10 @@ type Config struct { RegexpConfig *regexp.Config `mapstructure:"regexp"` } +func NewUnrecognizedMatchTypeError(matchType MatchType) error { + return fmt.Errorf("unrecognized %v: '%v', valid types are: %v", MatchTypeFieldName, matchType, validMatchTypes) +} + // CreateFilterSet creates a FilterSet from yaml config. func CreateFilterSet(filters []string, cfg *Config) (FilterSet, error) { switch cfg.MatchType { @@ -52,6 +56,6 @@ func CreateFilterSet(filters []string, cfg *Config) (FilterSet, error) { // Strict FilterSets do not have any extra configuration options, so call the constructor directly. return strict.NewFilterSet(filters), nil default: - return nil, fmt.Errorf("unrecognized %v: '%v', valid types are: %v", MatchTypeFieldName, cfg.MatchType, validMatchTypes) + return nil, NewUnrecognizedMatchTypeError(cfg.MatchType) } } diff --git a/processor/attributesprocessor/attributes_log_test.go b/processor/attributesprocessor/attributes_log_test.go index 6efc4ca24c8e..d11d49fe186f 100644 --- a/processor/attributesprocessor/attributes_log_test.go +++ b/processor/attributesprocessor/attributes_log_test.go @@ -49,10 +49,12 @@ func runIndividualLogTestCase(t *testing.T, tt logTestCase, tp component.LogsPro }) } -func generateLogData(logName string, attrs map[string]pdata.AttributeValue) pdata.Logs { +func generateLogData(resourceName string, attrs map[string]pdata.AttributeValue) pdata.Logs { td := pdata.NewLogs() - lr := td.ResourceLogs().AppendEmpty().InstrumentationLibraryLogs().AppendEmpty().LogRecords().AppendEmpty() - lr.SetName(logName) + res := td.ResourceLogs().AppendEmpty() + res.Resource().Attributes().InsertString("name", resourceName) + ill := res.InstrumentationLibraryLogs().AppendEmpty() + lr := ill.LogRecords().AppendEmpty() pdata.NewAttributeMapFromMap(attrs).CopyTo(lr.Attributes()) lr.Attributes().Sort() return td @@ -161,8 +163,9 @@ func TestAttributes_FilterLogs(t *testing.T) { {Key: "attribute1", Action: attraction.INSERT, Value: 123}, } oCfg.Include = &filterconfig.MatchProperties{ - LogNames: []string{"^[^i].*"}, - Config: *createConfig(filterset.Regexp), + Resources: []filterconfig.Attribute{{Key: "name", Value: "^[^i].*"}}, + //Libraries: []filterconfig.InstrumentationLibrary{{Name: "^[^i].*"}}, + Config: *createConfig(filterset.Regexp), } oCfg.Exclude = &filterconfig.MatchProperties{ Attributes: []filterconfig.Attribute{ @@ -171,7 +174,7 @@ func TestAttributes_FilterLogs(t *testing.T) { Config: *createConfig(filterset.Strict), } tp, err := factory.CreateLogsProcessor(context.Background(), componenttest.NewNopProcessorCreateSettings(), cfg, consumertest.NewNop()) - require.Nil(t, err) + require.NoError(t, err) require.NotNil(t, tp) for _, tt := range testCases { @@ -226,12 +229,12 @@ func TestAttributes_FilterLogsByNameStrict(t *testing.T) { {Key: "attribute1", Action: attraction.INSERT, Value: 123}, } oCfg.Include = &filterconfig.MatchProperties{ - LogNames: []string{"apply", "dont_apply"}, - Config: *createConfig(filterset.Strict), + Resources: []filterconfig.Attribute{{Key: "name", Value: "apply"}}, + Config: *createConfig(filterset.Strict), } oCfg.Exclude = &filterconfig.MatchProperties{ - LogNames: []string{"dont_apply"}, - Config: *createConfig(filterset.Strict), + Resources: []filterconfig.Attribute{{Key: "name", Value: "dont_apply"}}, + Config: *createConfig(filterset.Strict), } tp, err := factory.CreateLogsProcessor(context.Background(), componenttest.NewNopProcessorCreateSettings(), cfg, consumertest.NewNop()) require.Nil(t, err) @@ -289,12 +292,12 @@ func TestAttributes_FilterLogsByNameRegexp(t *testing.T) { {Key: "attribute1", Action: attraction.INSERT, Value: 123}, } oCfg.Include = &filterconfig.MatchProperties{ - LogNames: []string{"^apply.*"}, - Config: *createConfig(filterset.Regexp), + Resources: []filterconfig.Attribute{{Key: "name", Value: "^apply.*"}}, + Config: *createConfig(filterset.Regexp), } oCfg.Exclude = &filterconfig.MatchProperties{ - LogNames: []string{".*dont_apply$"}, - Config: *createConfig(filterset.Regexp), + Resources: []filterconfig.Attribute{{Key: "name", Value: ".*dont_apply$"}}, + Config: *createConfig(filterset.Regexp), } tp, err := factory.CreateLogsProcessor(context.Background(), componenttest.NewNopProcessorCreateSettings(), cfg, consumertest.NewNop()) require.Nil(t, err) @@ -397,10 +400,11 @@ func BenchmarkAttributes_FilterLogsByName(b *testing.B) { {Key: "attribute1", Action: attraction.INSERT, Value: 123}, } oCfg.Include = &filterconfig.MatchProperties{ - LogNames: []string{"^apply.*"}, + Config: *createConfig(filterset.Regexp), + Resources: []filterconfig.Attribute{{Key: "name", Value: "^apply.*"}}, } tp, err := factory.CreateLogsProcessor(context.Background(), componenttest.NewNopProcessorCreateSettings(), cfg, consumertest.NewNop()) - require.Nil(b, err) + require.NoError(b, err) require.NotNil(b, tp) for _, tt := range testCases { diff --git a/processor/attributesprocessor/factory.go b/processor/attributesprocessor/factory.go index c49eb74fce0d..ff127dd9fa74 100644 --- a/processor/attributesprocessor/factory.go +++ b/processor/attributesprocessor/factory.go @@ -83,7 +83,7 @@ func createTracesProcessor( func createLogProcessor( _ context.Context, - _ component.ProcessorCreateSettings, + set component.ProcessorCreateSettings, cfg config.Processor, nextConsumer consumer.Logs, ) (component.LogsProcessor, error) { @@ -95,6 +95,11 @@ func createLogProcessor( if err != nil { return nil, fmt.Errorf("error creating \"attributes\" processor: %w of processor %v", err, cfg.ID()) } + + if (oCfg.Include != nil && len(oCfg.Include.LogNames) > 0) || (oCfg.Exclude != nil && len(oCfg.Exclude.LogNames) > 0) { + set.Logger.Warn("log_names setting is deprecated and will be removed soon") + } + include, err := filterlog.NewMatcher(oCfg.Include) if err != nil { return nil, err diff --git a/processor/spanprocessor/span_test.go b/processor/spanprocessor/span_test.go index 24abe90a0b79..c0b5f3cefafe 100644 --- a/processor/spanprocessor/span_test.go +++ b/processor/spanprocessor/span_test.go @@ -635,6 +635,9 @@ func TestSpanProcessor_setStatusCodeConditionally(t *testing.T) { } // This test numer two include rule for applying rule only for status code 400 oCfg.Include = &filterconfig.MatchProperties{ + Config: filterset.Config{ + MatchType: filterset.Strict, + }, Attributes: []filterconfig.Attribute{ {Key: "http.status_code", Value: 400}, },