From 69ccd1bce306a133c194941fd99ac78599da2d73 Mon Sep 17 00:00:00 2001 From: Raj Nishtala Date: Mon, 20 Nov 2023 15:49:52 -0500 Subject: [PATCH 01/10] (pkg/ottl) Fix issue with the hash value of a match group in replace_pattern* --- .chloggen/fix-hash-value.yaml | 27 ++++++ .../ottlfuncs/func_replace_all_patterns.go | 83 ++++++++++++++----- .../func_replace_all_patterns_test.go | 2 +- pkg/ottl/ottlfuncs/func_replace_pattern.go | 54 +++++++----- .../ottlfuncs/func_replace_pattern_test.go | 8 +- 5 files changed, 129 insertions(+), 45 deletions(-) create mode 100755 .chloggen/fix-hash-value.yaml diff --git a/.chloggen/fix-hash-value.yaml b/.chloggen/fix-hash-value.yaml new file mode 100755 index 000000000000..3d6c5ba6fb52 --- /dev/null +++ b/.chloggen/fix-hash-value.yaml @@ -0,0 +1,27 @@ +# Use this changelog template to create an entry for release notes. + +# One of 'breaking', 'deprecation', 'new_component', 'enhancement', 'bug_fix' +change_type: 'bug_fix' + +# The name of the component, or a single word describing the area of concern, (e.g. filelogreceiver) +component: pkg/ottl + +# A brief description of the change. Surround your text with quotes ("") if it needs to start with a backtick (`). +note: Fix issue with the hash value of a match subgroup in replace_pattern functions. + +# Mandatory: One or more tracking issues related to the change. You can use the PR number here if no issue exists. +issues: [29409] + +# (Optional) One or more lines of additional information to render under the primary note. +# These lines will be padded with 2 spaces and then inserted directly into the document. +# Use pipe (|) for multiline entries. +subtext: + +# If your change doesn't affect end users or the exported elements of any package, +# you should instead start your pull request title with [chore] or use the "Skip Changelog" label. +# Optional: The change log or logs in which this entry should be included. +# e.g. '[user]' or '[user, api]' +# Include 'user' if the change is relevant to end users. +# Include 'api' if there is a change to a library API. +# Default: '[user]' +change_logs: [] diff --git a/pkg/ottl/ottlfuncs/func_replace_all_patterns.go b/pkg/ottl/ottlfuncs/func_replace_all_patterns.go index 2c7ee3ef3386..ab5eeeea5781 100644 --- a/pkg/ottl/ottlfuncs/func_replace_all_patterns.go +++ b/pkg/ottl/ottlfuncs/func_replace_all_patterns.go @@ -7,6 +7,8 @@ import ( "context" "fmt" "regexp" + "strconv" + "strings" "go.opentelemetry.io/collector/pdata/pcommon" @@ -59,26 +61,9 @@ func replaceAllPatterns[K any](target ottl.PMapGetter[K], mode string, regexPatt if err != nil { return nil, err } - if fn.IsEmpty() { - replacementVal, err = replacement.Get(ctx, tCtx) - if err != nil { - return nil, err - } - } else { - fnVal := fn.Get() - replacementExpr, errNew := fnVal.Get(&replaceAllPatternFuncArgs[K]{Input: replacement}) - if errNew != nil { - return nil, errNew - } - replacementValRaw, errNew := replacementExpr.Eval(ctx, tCtx) - if errNew != nil { - return nil, errNew - } - replacementValStr, ok := replacementValRaw.(string) - if !ok { - return nil, fmt.Errorf("replacement value is not a string") - } - replacementVal = replacementValStr + replacementVal, err = replacement.Get(ctx, tCtx) + if err != nil { + return nil, err } updated := pcommon.NewMap() updated.EnsureCapacity(val.Len()) @@ -86,6 +71,35 @@ func replaceAllPatterns[K any](target ottl.PMapGetter[K], mode string, regexPatt switch mode { case modeValue: if compiledPattern.MatchString(originalValue.Str()) { + if !fn.IsEmpty() { + match := compiledPattern.FindStringSubmatch(originalValue.Str()) + if match != nil { + if strings.Contains(replacementVal, "$") { + groupNum := strings.ReplaceAll(replacementVal, "$", "") + num, _ := strconv.Atoi(groupNum) + replacementVal = match[num] + } + } + fnVal := fn.Get() + replaceValGetter := ottl.StandardStringGetter[K]{ + Getter: func(context.Context, K) (any, error) { + return replacementVal, nil + }, + } + replacementExpr, errNew := fnVal.Get(&replaceAllPatternFuncArgs[K]{Input: replaceValGetter}) + if errNew != nil { + return false + } + replacementValRaw, errNew := replacementExpr.Eval(ctx, tCtx) + if errNew != nil { + return false + } + replacementValStr, ok := replacementValRaw.(string) + if !ok { + return false + } + replacementVal = replacementValStr + } updatedString := compiledPattern.ReplaceAllString(originalValue.Str(), replacementVal) updated.PutStr(key, updatedString) } else { @@ -93,6 +107,35 @@ func replaceAllPatterns[K any](target ottl.PMapGetter[K], mode string, regexPatt } case modeKey: if compiledPattern.MatchString(key) { + if !fn.IsEmpty() { + match := compiledPattern.FindStringSubmatch(originalValue.Str()) + if match != nil { + if strings.Contains(replacementVal, "$") { + groupNum := strings.ReplaceAll(replacementVal, "$", "") + num, _ := strconv.Atoi(groupNum) + replacementVal = match[num] + } + } + fnVal := fn.Get() + replaceValGetter := ottl.StandardStringGetter[K]{ + Getter: func(context.Context, K) (any, error) { + return replacementVal, nil + }, + } + replacementExpr, errNew := fnVal.Get(&replaceAllPatternFuncArgs[K]{Input: replaceValGetter}) + if errNew != nil { + return false + } + replacementValRaw, errNew := replacementExpr.Eval(ctx, tCtx) + if errNew != nil { + return false + } + replacementValStr, ok := replacementValRaw.(string) + if !ok { + return false + } + replacementVal = replacementValStr + } updatedKey := compiledPattern.ReplaceAllString(key, replacementVal) originalValue.CopyTo(updated.PutEmpty(updatedKey)) } else { diff --git a/pkg/ottl/ottlfuncs/func_replace_all_patterns_test.go b/pkg/ottl/ottlfuncs/func_replace_all_patterns_test.go index c171b40e9cea..ca996b421f45 100644 --- a/pkg/ottl/ottlfuncs/func_replace_all_patterns_test.go +++ b/pkg/ottl/ottlfuncs/func_replace_all_patterns_test.go @@ -60,7 +60,7 @@ func Test_replaceAllPatterns(t *testing.T) { function: optionalArg, want: func(expectedMap pcommon.Map) { expectedMap.PutStr("test", "4804d6b7f03268e33f78c484977f3d81771220df07cc6aac4ad4868102141fad world") - expectedMap.PutStr("test2", "4804d6b7f03268e33f78c484977f3d81771220df07cc6aac4ad4868102141fad") + expectedMap.PutStr("test2", "09648f12e7a3940f539bb65d32321c2f96aa94ef698d87816e94d822c6f9d7c4") expectedMap.PutStr("test3", "goodbye world1 and world2") expectedMap.PutInt("test4", 1234) expectedMap.PutDouble("test5", 1234) diff --git a/pkg/ottl/ottlfuncs/func_replace_pattern.go b/pkg/ottl/ottlfuncs/func_replace_pattern.go index aed32367ddc6..1f5361eedc26 100644 --- a/pkg/ottl/ottlfuncs/func_replace_pattern.go +++ b/pkg/ottl/ottlfuncs/func_replace_pattern.go @@ -7,6 +7,8 @@ import ( "context" "fmt" "regexp" + "strconv" + "strings" "github.com/open-telemetry/opentelemetry-collector-contrib/pkg/ottl" ) @@ -47,32 +49,44 @@ func replacePattern[K any](target ottl.GetSetter[K], regexPattern string, replac if err != nil { return nil, err } - if fn.IsEmpty() { - replacementVal, err = replacement.Get(ctx, tCtx) - if err != nil { - return nil, err - } - } else { - fnVal := fn.Get() - replacementExpr, errNew := fnVal.Get(&replacePatternFuncArgs[K]{Input: replacement}) - if errNew != nil { - return nil, errNew - } - replacementValRaw, errNew := replacementExpr.Eval(ctx, tCtx) - if errNew != nil { - return nil, errNew - } - replacementValStr, ok := replacementValRaw.(string) - if !ok { - return nil, fmt.Errorf("replacement value is not a string") - } - replacementVal = replacementValStr + replacementVal, err = replacement.Get(ctx, tCtx) + if err != nil { + return nil, err } if originalVal == nil { return nil, nil } if originalValStr, ok := originalVal.(string); ok { if compiledPattern.MatchString(originalValStr) { + if !fn.IsEmpty() { + match := compiledPattern.FindStringSubmatch(originalValStr) + if match != nil { + if strings.Contains(replacementVal, "$") { + groupNum := strings.ReplaceAll(replacementVal, "$", "") + num, _ := strconv.Atoi(groupNum) + replacementVal = match[num] + } + } + fnVal := fn.Get() + replaceValGetter := ottl.StandardStringGetter[K]{ + Getter: func(context.Context, K) (any, error) { + return replacementVal, nil + }, + } + replacementExpr, errNew := fnVal.Get(&replacePatternFuncArgs[K]{Input: replaceValGetter}) + if errNew != nil { + return nil, errNew + } + replacementValRaw, errNew := replacementExpr.Eval(ctx, tCtx) + if errNew != nil { + return nil, errNew + } + replacementValStr, ok := replacementValRaw.(string) + if !ok { + return nil, fmt.Errorf("replacement value is not a string") + } + replacementVal = replacementValStr + } updatedStr := compiledPattern.ReplaceAllString(originalValStr, replacementVal) err = target.Set(ctx, tCtx, updatedStr) if err != nil { diff --git a/pkg/ottl/ottlfuncs/func_replace_pattern_test.go b/pkg/ottl/ottlfuncs/func_replace_pattern_test.go index 66edf7d25177..c38f2843ea5c 100644 --- a/pkg/ottl/ottlfuncs/func_replace_pattern_test.go +++ b/pkg/ottl/ottlfuncs/func_replace_pattern_test.go @@ -45,15 +45,15 @@ func Test_replacePattern(t *testing.T) { { name: "replace regex match (with hash function)", target: target, - pattern: `passwd\=[^\s]*(\s?)`, + pattern: `passwd\=([^\s]*)(\s?)`, replacement: ottl.StandardStringGetter[pcommon.Value]{ Getter: func(context.Context, pcommon.Value) (any, error) { - return "passwd=*** ", nil + return "$1", nil }, }, function: optionalArg, want: func(expectedValue pcommon.Value) { - expectedValue.SetStr("application 0f2407f2d83337b1f757eb1754a7643ce0e8fba620bc605c54566cd6dfd838beotherarg=notsensitive key1 key2") + expectedValue.SetStr("application 148b08b1ec2b1e41bca4c63ec80de7ea13d594a1b2583f0cb6833449f40c5ceeotherarg=notsensitive key1 key2") }, }, { @@ -228,7 +228,7 @@ func Test_replacePattern_bad_function_result(t *testing.T) { result, err := exprFunc(nil, input) require.Error(t, err) - assert.ErrorContains(t, err, "replacement value is not a string") + assert.ErrorContains(t, err, "expected string but got nil") assert.Nil(t, result) } From feb04b6f0674ede078e4f98968fb07359cbcbc1a Mon Sep 17 00:00:00 2001 From: Raj Nishtala Date: Wed, 22 Nov 2023 15:43:52 -0500 Subject: [PATCH 02/10] Use ExpandString to resolve submatches --- .../ottlfuncs/func_replace_all_patterns.go | 26 ++++++------------- pkg/ottl/ottlfuncs/func_replace_pattern.go | 20 +++++--------- 2 files changed, 15 insertions(+), 31 deletions(-) diff --git a/pkg/ottl/ottlfuncs/func_replace_all_patterns.go b/pkg/ottl/ottlfuncs/func_replace_all_patterns.go index ab5eeeea5781..a05785663a67 100644 --- a/pkg/ottl/ottlfuncs/func_replace_all_patterns.go +++ b/pkg/ottl/ottlfuncs/func_replace_all_patterns.go @@ -7,8 +7,6 @@ import ( "context" "fmt" "regexp" - "strconv" - "strings" "go.opentelemetry.io/collector/pdata/pcommon" @@ -72,14 +70,10 @@ func replaceAllPatterns[K any](target ottl.PMapGetter[K], mode string, regexPatt case modeValue: if compiledPattern.MatchString(originalValue.Str()) { if !fn.IsEmpty() { - match := compiledPattern.FindStringSubmatch(originalValue.Str()) - if match != nil { - if strings.Contains(replacementVal, "$") { - groupNum := strings.ReplaceAll(replacementVal, "$", "") - num, _ := strconv.Atoi(groupNum) - replacementVal = match[num] - } - } + result := []byte{} + submatches := compiledPattern.FindStringSubmatchIndex(originalValue.Str()) + result = compiledPattern.ExpandString(result, replacementVal, originalValue.Str(), submatches) + replacementVal = string(result) fnVal := fn.Get() replaceValGetter := ottl.StandardStringGetter[K]{ Getter: func(context.Context, K) (any, error) { @@ -108,14 +102,10 @@ func replaceAllPatterns[K any](target ottl.PMapGetter[K], mode string, regexPatt case modeKey: if compiledPattern.MatchString(key) { if !fn.IsEmpty() { - match := compiledPattern.FindStringSubmatch(originalValue.Str()) - if match != nil { - if strings.Contains(replacementVal, "$") { - groupNum := strings.ReplaceAll(replacementVal, "$", "") - num, _ := strconv.Atoi(groupNum) - replacementVal = match[num] - } - } + result := []byte{} + submatches := compiledPattern.FindStringSubmatchIndex(originalValue.Str()) + result = compiledPattern.ExpandString(result, replacementVal, originalValue.Str(), submatches) + replacementVal = string(result) fnVal := fn.Get() replaceValGetter := ottl.StandardStringGetter[K]{ Getter: func(context.Context, K) (any, error) { diff --git a/pkg/ottl/ottlfuncs/func_replace_pattern.go b/pkg/ottl/ottlfuncs/func_replace_pattern.go index 1f5361eedc26..81d43f0cf2cf 100644 --- a/pkg/ottl/ottlfuncs/func_replace_pattern.go +++ b/pkg/ottl/ottlfuncs/func_replace_pattern.go @@ -7,8 +7,6 @@ import ( "context" "fmt" "regexp" - "strconv" - "strings" "github.com/open-telemetry/opentelemetry-collector-contrib/pkg/ottl" ) @@ -49,24 +47,20 @@ func replacePattern[K any](target ottl.GetSetter[K], regexPattern string, replac if err != nil { return nil, err } + if originalVal == nil { + return nil, nil + } replacementVal, err = replacement.Get(ctx, tCtx) if err != nil { return nil, err } - if originalVal == nil { - return nil, nil - } if originalValStr, ok := originalVal.(string); ok { if compiledPattern.MatchString(originalValStr) { if !fn.IsEmpty() { - match := compiledPattern.FindStringSubmatch(originalValStr) - if match != nil { - if strings.Contains(replacementVal, "$") { - groupNum := strings.ReplaceAll(replacementVal, "$", "") - num, _ := strconv.Atoi(groupNum) - replacementVal = match[num] - } - } + result := []byte{} + submatches := compiledPattern.FindStringSubmatchIndex(originalValStr) + result = compiledPattern.ExpandString(result, replacementVal, originalValStr, submatches) + replacementVal = string(result) fnVal := fn.Get() replaceValGetter := ottl.StandardStringGetter[K]{ Getter: func(context.Context, K) (any, error) { From 26c13ee72c05d9c1715f90d5bf70046777bd68e7 Mon Sep 17 00:00:00 2001 From: Raj Nishtala Date: Tue, 28 Nov 2023 19:07:40 -0500 Subject: [PATCH 03/10] Additional tests for the replace_pattern editor --- .../func_replace_all_patterns_test.go | 60 +++++++++++++++++++ .../ottlfuncs/func_replace_pattern_test.go | 42 +++++++++++++ 2 files changed, 102 insertions(+) diff --git a/pkg/ottl/ottlfuncs/func_replace_all_patterns_test.go b/pkg/ottl/ottlfuncs/func_replace_all_patterns_test.go index ca996b421f45..199dcc34e200 100644 --- a/pkg/ottl/ottlfuncs/func_replace_all_patterns_test.go +++ b/pkg/ottl/ottlfuncs/func_replace_all_patterns_test.go @@ -67,6 +67,66 @@ func Test_replaceAllPatterns(t *testing.T) { expectedMap.PutBool("test6", true) }, }, + { + name: "replace only matches (with capture group and hash function)", + target: target, + mode: modeValue, + pattern: "(hello)", + replacement: ottl.StandardStringGetter[pcommon.Map]{ + Getter: func(context.Context, pcommon.Map) (any, error) { + return "$1", nil + }, + }, + function: optionalArg, + want: func(expectedMap pcommon.Map) { + expectedMap.PutStr("test", "2cf24dba5fb0a30e26e83b2ac5b9e29e1b161e5c1fa7425e73043362938b9824 world") + expectedMap.PutStr("test2", "d7914fe546b684688bb95f4f888a92dfc680603a75f23eb823658031fff766d9") + expectedMap.PutStr("test3", "goodbye world1 and world2") + expectedMap.PutInt("test4", 1234) + expectedMap.PutDouble("test5", 1234) + expectedMap.PutBool("test6", true) + }, + }, + { + name: "replace only matches (no capture group and with hash function)", + target: target, + mode: modeValue, + pattern: "hello", + replacement: ottl.StandardStringGetter[pcommon.Map]{ + Getter: func(context.Context, pcommon.Map) (any, error) { + return "$1", nil + }, + }, + function: optionalArg, + want: func(expectedMap pcommon.Map) { + expectedMap.PutStr("test", "e3b0c44298fc1c149afbf4c8996fb92427ae41e4649b934ca495991b7852b855 world") + expectedMap.PutStr("test2", "cd372fb85148700fa88095e3492d3f9f5beb43e555e5ff26d95f5a6adc36f8e6") + expectedMap.PutStr("test3", "goodbye world1 and world2") + expectedMap.PutInt("test4", 1234) + expectedMap.PutDouble("test5", 1234) + expectedMap.PutBool("test6", true) + }, + }, + { + name: "replace only matches (no capture group or hash function)", + target: target, + mode: modeValue, + pattern: "hello", + replacement: ottl.StandardStringGetter[pcommon.Map]{ + Getter: func(context.Context, pcommon.Map) (any, error) { + return "$1", nil + }, + }, + function: ottl.Optional[ottl.FunctionGetter[pcommon.Map]]{}, + want: func(expectedMap pcommon.Map) { + expectedMap.PutStr("test", " world") + expectedMap.PutStr("test2", "") + expectedMap.PutStr("test3", "goodbye world1 and world2") + expectedMap.PutInt("test4", 1234) + expectedMap.PutDouble("test5", 1234) + expectedMap.PutBool("test6", true) + }, + }, { name: "replace only matches", target: target, diff --git a/pkg/ottl/ottlfuncs/func_replace_pattern_test.go b/pkg/ottl/ottlfuncs/func_replace_pattern_test.go index c38f2843ea5c..5a042f21c751 100644 --- a/pkg/ottl/ottlfuncs/func_replace_pattern_test.go +++ b/pkg/ottl/ottlfuncs/func_replace_pattern_test.go @@ -56,6 +56,48 @@ func Test_replacePattern(t *testing.T) { expectedValue.SetStr("application 148b08b1ec2b1e41bca4c63ec80de7ea13d594a1b2583f0cb6833449f40c5ceeotherarg=notsensitive key1 key2") }, }, + { + name: "replace regex match (capture group without $1)", + target: target, + pattern: `passwd\=([^\s]*)(\s?)`, + replacement: ottl.StandardStringGetter[pcommon.Value]{ + Getter: func(context.Context, pcommon.Value) (any, error) { + return "test", nil + }, + }, + function: optionalArg, + want: func(expectedValue pcommon.Value) { + expectedValue.SetStr("application 9f86d081884c7d659a2feaa0c55ad015a3bf4f1b2b0b822cd15d6c15b0f00a08otherarg=notsensitive key1 key2") + }, + }, + { + name: "replace regex match (no capture group with $1 and hash function)", + target: target, + pattern: `passwd\=[^\s]*(\s?)`, + replacement: ottl.StandardStringGetter[pcommon.Value]{ + Getter: func(context.Context, pcommon.Value) (any, error) { + return "$1", nil + }, + }, + function: optionalArg, + want: func(expectedValue pcommon.Value) { + expectedValue.SetStr("application 36a9e7f1c95b82ffb99743e0c5c4ce95d83c9a430aac59f84ef3cbfab6145068otherarg=notsensitive key1 key2") + }, + }, + { + name: "replace regex match (no capture group or hash function with $1)", + target: target, + pattern: `passwd\=[^\s]*(\s?)`, + replacement: ottl.StandardStringGetter[pcommon.Value]{ + Getter: func(context.Context, pcommon.Value) (any, error) { + return "$1", nil + }, + }, + function: ottl.Optional[ottl.FunctionGetter[pcommon.Value]]{}, + want: func(expectedValue pcommon.Value) { + expectedValue.SetStr("application otherarg=notsensitive key1 key2") + }, + }, { name: "replace regex match", target: target, From e4a4da16b4d93d9adc5f0d6f3a1d5938ddc1c790 Mon Sep 17 00:00:00 2001 From: Raj Nishtala Date: Fri, 1 Dec 2023 11:37:58 -0500 Subject: [PATCH 04/10] Add test for multiple capture groups --- .../func_replace_all_patterns_test.go | 40 +++++++++++++++++++ 1 file changed, 40 insertions(+) diff --git a/pkg/ottl/ottlfuncs/func_replace_all_patterns_test.go b/pkg/ottl/ottlfuncs/func_replace_all_patterns_test.go index 199dcc34e200..9b615cff46cd 100644 --- a/pkg/ottl/ottlfuncs/func_replace_all_patterns_test.go +++ b/pkg/ottl/ottlfuncs/func_replace_all_patterns_test.go @@ -187,6 +187,46 @@ func Test_replaceAllPatterns(t *testing.T) { expectedMap.PutBool("test6", true) }, }, + { + name: "regex match (with multiple capture groups)", + target: target, + mode: modeValue, + pattern: `(world1) and (world2)`, + replacement: ottl.StandardStringGetter[pcommon.Map]{ + Getter: func(context.Context, pcommon.Map) (any, error) { + return "blue-$1 and blue-$2", nil + }, + }, + function: ottl.Optional[ottl.FunctionGetter[pcommon.Map]]{}, + want: func(expectedMap pcommon.Map) { + expectedMap.PutStr("test", "hello world") + expectedMap.PutStr("test2", "hello") + expectedMap.PutStr("test3", "goodbye blue-world1 and blue-world2") + expectedMap.PutInt("test4", 1234) + expectedMap.PutDouble("test5", 1234) + expectedMap.PutBool("test6", true) + }, + }, + { + name: "regex match (with multiple capture groups and hash function)", + target: target, + mode: modeValue, + pattern: `(world1) and (world2)`, + replacement: ottl.StandardStringGetter[pcommon.Map]{ + Getter: func(context.Context, pcommon.Map) (any, error) { + return "$1", nil + }, + }, + function: optionalArg, + want: func(expectedMap pcommon.Map) { + expectedMap.PutStr("test", "hello world") + expectedMap.PutStr("test2", "hello") + expectedMap.PutStr("test3", "goodbye da4c6d4adf93f13551bbad14a82b024befcf61e4b3b9cd9494668b018a3a148a") + expectedMap.PutInt("test4", 1234) + expectedMap.PutDouble("test5", 1234) + expectedMap.PutBool("test6", true) + }, + }, { name: "replace only matches", target: target, From bb4b392a90dbe69ca882d575ea0f79bfdeab0739 Mon Sep 17 00:00:00 2001 From: Raj Nishtala Date: Fri, 1 Dec 2023 15:06:14 -0500 Subject: [PATCH 05/10] Add a test replacement function to validate the results --- .../func_replace_all_matches_test.go | 6 +-- .../func_replace_all_patterns_test.go | 16 +++---- pkg/ottl/ottlfuncs/func_replace_match_test.go | 4 +- .../ottlfuncs/func_replace_pattern_test.go | 45 ++++++++++++++++--- 4 files changed, 51 insertions(+), 20 deletions(-) diff --git a/pkg/ottl/ottlfuncs/func_replace_all_matches_test.go b/pkg/ottl/ottlfuncs/func_replace_all_matches_test.go index 88d1d7fe019e..9ed7857926e8 100644 --- a/pkg/ottl/ottlfuncs/func_replace_all_matches_test.go +++ b/pkg/ottl/ottlfuncs/func_replace_all_matches_test.go @@ -25,7 +25,7 @@ func Test_replaceAllMatches(t *testing.T) { FCtx: ottl.FunctionContext{ Set: componenttest.NewNopTelemetrySettings(), }, - Fact: StandardConverters[pcommon.Map]()["SHA256"], + Fact: NewTestFactory[pcommon.Map](), } optionalArg := ottl.NewTestingOptional[ottl.FunctionGetter[pcommon.Map]](ottlValue) @@ -54,8 +54,8 @@ func Test_replaceAllMatches(t *testing.T) { }, function: optionalArg, want: func(expectedMap pcommon.Map) { - expectedMap.PutStr("test", "4804d6b7f03268e33f78c484977f3d81771220df07cc6aac4ad4868102141fad") - expectedMap.PutStr("test2", "4804d6b7f03268e33f78c484977f3d81771220df07cc6aac4ad4868102141fad") + expectedMap.PutStr("test", "hash(hello {universe})") + expectedMap.PutStr("test2", "hash(hello {universe})") expectedMap.PutStr("test3", "goodbye") }, }, diff --git a/pkg/ottl/ottlfuncs/func_replace_all_patterns_test.go b/pkg/ottl/ottlfuncs/func_replace_all_patterns_test.go index 9b615cff46cd..1d07a19ef0e7 100644 --- a/pkg/ottl/ottlfuncs/func_replace_all_patterns_test.go +++ b/pkg/ottl/ottlfuncs/func_replace_all_patterns_test.go @@ -28,7 +28,7 @@ func Test_replaceAllPatterns(t *testing.T) { FCtx: ottl.FunctionContext{ Set: componenttest.NewNopTelemetrySettings(), }, - Fact: StandardConverters[pcommon.Map]()["SHA256"], + Fact: NewTestFactory[pcommon.Map](), } optionalArg := ottl.NewTestingOptional[ottl.FunctionGetter[pcommon.Map]](ottlValue) @@ -59,8 +59,8 @@ func Test_replaceAllPatterns(t *testing.T) { }, function: optionalArg, want: func(expectedMap pcommon.Map) { - expectedMap.PutStr("test", "4804d6b7f03268e33f78c484977f3d81771220df07cc6aac4ad4868102141fad world") - expectedMap.PutStr("test2", "09648f12e7a3940f539bb65d32321c2f96aa94ef698d87816e94d822c6f9d7c4") + expectedMap.PutStr("test", "hash(hello {universe}) world") + expectedMap.PutStr("test2", "hash(hash(hello {universe}))") // hash applied twice because of replace_all_patterns expectedMap.PutStr("test3", "goodbye world1 and world2") expectedMap.PutInt("test4", 1234) expectedMap.PutDouble("test5", 1234) @@ -79,8 +79,8 @@ func Test_replaceAllPatterns(t *testing.T) { }, function: optionalArg, want: func(expectedMap pcommon.Map) { - expectedMap.PutStr("test", "2cf24dba5fb0a30e26e83b2ac5b9e29e1b161e5c1fa7425e73043362938b9824 world") - expectedMap.PutStr("test2", "d7914fe546b684688bb95f4f888a92dfc680603a75f23eb823658031fff766d9") + expectedMap.PutStr("test", "hash(hello) world") + expectedMap.PutStr("test2", "hash(hash(hello))") // hash applied twice because of replace_all_patterns expectedMap.PutStr("test3", "goodbye world1 and world2") expectedMap.PutInt("test4", 1234) expectedMap.PutDouble("test5", 1234) @@ -99,8 +99,8 @@ func Test_replaceAllPatterns(t *testing.T) { }, function: optionalArg, want: func(expectedMap pcommon.Map) { - expectedMap.PutStr("test", "e3b0c44298fc1c149afbf4c8996fb92427ae41e4649b934ca495991b7852b855 world") - expectedMap.PutStr("test2", "cd372fb85148700fa88095e3492d3f9f5beb43e555e5ff26d95f5a6adc36f8e6") + expectedMap.PutStr("test", "hash() world") + expectedMap.PutStr("test2", "hash(hash())") expectedMap.PutStr("test3", "goodbye world1 and world2") expectedMap.PutInt("test4", 1234) expectedMap.PutDouble("test5", 1234) @@ -221,7 +221,7 @@ func Test_replaceAllPatterns(t *testing.T) { want: func(expectedMap pcommon.Map) { expectedMap.PutStr("test", "hello world") expectedMap.PutStr("test2", "hello") - expectedMap.PutStr("test3", "goodbye da4c6d4adf93f13551bbad14a82b024befcf61e4b3b9cd9494668b018a3a148a") + expectedMap.PutStr("test3", "goodbye hash(world1)") expectedMap.PutInt("test4", 1234) expectedMap.PutDouble("test5", 1234) expectedMap.PutBool("test6", true) diff --git a/pkg/ottl/ottlfuncs/func_replace_match_test.go b/pkg/ottl/ottlfuncs/func_replace_match_test.go index 5de8512f1c1e..089ab141102c 100644 --- a/pkg/ottl/ottlfuncs/func_replace_match_test.go +++ b/pkg/ottl/ottlfuncs/func_replace_match_test.go @@ -21,7 +21,7 @@ func Test_replaceMatch(t *testing.T) { FCtx: ottl.FunctionContext{ Set: componenttest.NewNopTelemetrySettings(), }, - Fact: StandardConverters[pcommon.Value]()["SHA256"], + Fact: NewTestFactory[pcommon.Value](), } optionalArg := ottl.NewTestingOptional[ottl.FunctionGetter[pcommon.Value]](ottlValue) target := &ottl.StandardGetSetter[pcommon.Value]{ @@ -53,7 +53,7 @@ func Test_replaceMatch(t *testing.T) { }, function: optionalArg, want: func(expectedValue pcommon.Value) { - expectedValue.SetStr("4804d6b7f03268e33f78c484977f3d81771220df07cc6aac4ad4868102141fad") + expectedValue.SetStr("hash(hello {universe})") }, }, { diff --git a/pkg/ottl/ottlfuncs/func_replace_pattern_test.go b/pkg/ottl/ottlfuncs/func_replace_pattern_test.go index 5a042f21c751..bd6e95cfc734 100644 --- a/pkg/ottl/ottlfuncs/func_replace_pattern_test.go +++ b/pkg/ottl/ottlfuncs/func_replace_pattern_test.go @@ -5,6 +5,7 @@ package ottlfuncs import ( "context" + "fmt" "testing" "github.com/stretchr/testify/assert" @@ -15,13 +16,43 @@ import ( "github.com/open-telemetry/opentelemetry-collector-contrib/pkg/ottl" ) +type TestArguments[K any] struct { + Target ottl.StringGetter[K] +} + +// For testing Hash values. +func NewTestFactory[K any]() ottl.Factory[K] { + return ottl.NewFactory("Test", &TestArguments[K]{}, createTestFunction[K]) +} + +func createTestFunction[K any](_ ottl.FunctionContext, oArgs ottl.Arguments) (ottl.ExprFunc[K], error) { + args, ok := oArgs.(*TestArguments[K]) + + if !ok { + return nil, fmt.Errorf("TestFactory args must be of type *TestArguments[K]") + } + + return HashString(args.Target) +} + +func HashString[K any](target ottl.StringGetter[K]) (ottl.ExprFunc[K], error) { + + return func(ctx context.Context, tCtx K) (any, error) { + val, err := target.Get(ctx, tCtx) + if err != nil { + return nil, err + } + return fmt.Sprintf("hash(%s)", val), nil + }, nil +} + func Test_replacePattern(t *testing.T) { input := pcommon.NewValueStr("application passwd=sensitivedtata otherarg=notsensitive key1 key2") ottlValue := ottl.StandardFunctionGetter[pcommon.Value]{ FCtx: ottl.FunctionContext{ Set: componenttest.NewNopTelemetrySettings(), }, - Fact: StandardConverters[pcommon.Value]()["SHA256"], + Fact: NewTestFactory[pcommon.Value](), } optionalArg := ottl.NewTestingOptional[ottl.FunctionGetter[pcommon.Value]](ottlValue) target := &ottl.StandardGetSetter[pcommon.Value]{ @@ -53,7 +84,7 @@ func Test_replacePattern(t *testing.T) { }, function: optionalArg, want: func(expectedValue pcommon.Value) { - expectedValue.SetStr("application 148b08b1ec2b1e41bca4c63ec80de7ea13d594a1b2583f0cb6833449f40c5ceeotherarg=notsensitive key1 key2") + expectedValue.SetStr("application hash(sensitivedtata)otherarg=notsensitive key1 key2") }, }, { @@ -67,13 +98,13 @@ func Test_replacePattern(t *testing.T) { }, function: optionalArg, want: func(expectedValue pcommon.Value) { - expectedValue.SetStr("application 9f86d081884c7d659a2feaa0c55ad015a3bf4f1b2b0b822cd15d6c15b0f00a08otherarg=notsensitive key1 key2") + expectedValue.SetStr("application hash(test)otherarg=notsensitive key1 key2") }, }, { name: "replace regex match (no capture group with $1 and hash function)", target: target, - pattern: `passwd\=[^\s]*(\s?)`, + pattern: `passwd\=[^\s]*\s?`, replacement: ottl.StandardStringGetter[pcommon.Value]{ Getter: func(context.Context, pcommon.Value) (any, error) { return "$1", nil @@ -81,13 +112,13 @@ func Test_replacePattern(t *testing.T) { }, function: optionalArg, want: func(expectedValue pcommon.Value) { - expectedValue.SetStr("application 36a9e7f1c95b82ffb99743e0c5c4ce95d83c9a430aac59f84ef3cbfab6145068otherarg=notsensitive key1 key2") + expectedValue.SetStr("application hash()otherarg=notsensitive key1 key2") }, }, { name: "replace regex match (no capture group or hash function with $1)", target: target, - pattern: `passwd\=[^\s]*(\s?)`, + pattern: `passwd\=[^\s]*\s?`, replacement: ottl.StandardStringGetter[pcommon.Value]{ Getter: func(context.Context, pcommon.Value) (any, error) { return "$1", nil @@ -95,7 +126,7 @@ func Test_replacePattern(t *testing.T) { }, function: ottl.Optional[ottl.FunctionGetter[pcommon.Value]]{}, want: func(expectedValue pcommon.Value) { - expectedValue.SetStr("application otherarg=notsensitive key1 key2") + expectedValue.SetStr("application otherarg=notsensitive key1 key2") }, }, { From 2a9b23a3c679d2450ae2bb402e151dbed42e2a1d Mon Sep 17 00:00:00 2001 From: Raj Nishtala Date: Thu, 7 Dec 2023 15:21:39 -0500 Subject: [PATCH 06/10] Tests for multiple matches from one capture group --- .../func_replace_all_patterns_test.go | 40 +++++++++++++++++++ 1 file changed, 40 insertions(+) diff --git a/pkg/ottl/ottlfuncs/func_replace_all_patterns_test.go b/pkg/ottl/ottlfuncs/func_replace_all_patterns_test.go index 1d07a19ef0e7..1cf6f8bb2dda 100644 --- a/pkg/ottl/ottlfuncs/func_replace_all_patterns_test.go +++ b/pkg/ottl/ottlfuncs/func_replace_all_patterns_test.go @@ -207,6 +207,26 @@ func Test_replaceAllPatterns(t *testing.T) { expectedMap.PutBool("test6", true) }, }, + { + name: "regex match (with multiple matches from one capture group)", + target: target, + mode: modeValue, + pattern: `(world\d)`, + replacement: ottl.StandardStringGetter[pcommon.Map]{ + Getter: func(context.Context, pcommon.Map) (any, error) { + return "blue-$1", nil + }, + }, + function: ottl.Optional[ottl.FunctionGetter[pcommon.Map]]{}, + want: func(expectedMap pcommon.Map) { + expectedMap.PutStr("test", "hello world") + expectedMap.PutStr("test2", "hello") + expectedMap.PutStr("test3", "goodbye blue-world1 and blue-world2") + expectedMap.PutInt("test4", 1234) + expectedMap.PutDouble("test5", 1234) + expectedMap.PutBool("test6", true) + }, + }, { name: "regex match (with multiple capture groups and hash function)", target: target, @@ -227,6 +247,26 @@ func Test_replaceAllPatterns(t *testing.T) { expectedMap.PutBool("test6", true) }, }, + { + name: "regex match (with multiple matches from one capture group and hash function)", + target: target, + mode: modeValue, + pattern: `(world\d)`, + replacement: ottl.StandardStringGetter[pcommon.Map]{ + Getter: func(context.Context, pcommon.Map) (any, error) { + return "$1", nil + }, + }, + function: optionalArg, + want: func(expectedMap pcommon.Map) { + expectedMap.PutStr("test", "hello world") + expectedMap.PutStr("test2", "hello") + expectedMap.PutStr("test3", "goodbye hash(world1) and hash(world1)") + expectedMap.PutInt("test4", 1234) + expectedMap.PutDouble("test5", 1234) + expectedMap.PutBool("test6", true) + }, + }, { name: "replace only matches", target: target, From a56127ae5020bb7a73f8c484dc4aa357e4b75370 Mon Sep 17 00:00:00 2001 From: Raj Nishtala Date: Fri, 8 Dec 2023 16:24:37 -0500 Subject: [PATCH 07/10] Finding and hashing all submatches from the same capture group --- .../ottlfuncs/func_replace_all_patterns.go | 119 +++++++++++------- .../func_replace_all_patterns_test.go | 16 +-- pkg/ottl/ottlfuncs/func_replace_pattern.go | 69 ++++++---- .../ottlfuncs/func_replace_pattern_test.go | 11 +- 4 files changed, 128 insertions(+), 87 deletions(-) diff --git a/pkg/ottl/ottlfuncs/func_replace_all_patterns.go b/pkg/ottl/ottlfuncs/func_replace_all_patterns.go index a05785663a67..07e2f6eaf8e2 100644 --- a/pkg/ottl/ottlfuncs/func_replace_all_patterns.go +++ b/pkg/ottl/ottlfuncs/func_replace_all_patterns.go @@ -7,6 +7,7 @@ import ( "context" "fmt" "regexp" + "strings" "go.opentelemetry.io/collector/pdata/pcommon" @@ -70,64 +71,86 @@ func replaceAllPatterns[K any](target ottl.PMapGetter[K], mode string, regexPatt case modeValue: if compiledPattern.MatchString(originalValue.Str()) { if !fn.IsEmpty() { - result := []byte{} - submatches := compiledPattern.FindStringSubmatchIndex(originalValue.Str()) - result = compiledPattern.ExpandString(result, replacementVal, originalValue.Str(), submatches) - replacementVal = string(result) - fnVal := fn.Get() - replaceValGetter := ottl.StandardStringGetter[K]{ - Getter: func(context.Context, K) (any, error) { - return replacementVal, nil - }, + var updatedString string + foundMatch := false + updatedString = originalValue.Str() + submatches := compiledPattern.FindAllStringSubmatchIndex(updatedString, -1) + for _, submatch := range submatches { + for i := 2; i < len(submatch); i += 2 { + fnVal := fn.Get() + old := originalValue.Str()[submatch[i]:submatch[i+1]] + replaceValGetter := ottl.StandardStringGetter[K]{ + Getter: func(context.Context, K) (any, error) { + return old, nil + }, + } + replacementExpr, errNew := fnVal.Get(&replaceAllPatternFuncArgs[K]{Input: replaceValGetter}) + if errNew != nil { + return false + } + replacementValRaw, errNew := replacementExpr.Eval(ctx, tCtx) + if errNew != nil { + return false + } + replacementValStr, ok := replacementValRaw.(string) + if !ok { + return false + } + updatedString = strings.ReplaceAll(updatedString, old, replacementValStr) + foundMatch = true + updated.PutStr(key, updatedString) + } + if !foundMatch { + originalValue.CopyTo(updated.PutEmpty(key)) + } } - replacementExpr, errNew := fnVal.Get(&replaceAllPatternFuncArgs[K]{Input: replaceValGetter}) - if errNew != nil { - return false - } - replacementValRaw, errNew := replacementExpr.Eval(ctx, tCtx) - if errNew != nil { - return false - } - replacementValStr, ok := replacementValRaw.(string) - if !ok { - return false - } - replacementVal = replacementValStr + } else { + updatedString := compiledPattern.ReplaceAllString(originalValue.Str(), replacementVal) + updated.PutStr(key, updatedString) } - updatedString := compiledPattern.ReplaceAllString(originalValue.Str(), replacementVal) - updated.PutStr(key, updatedString) } else { originalValue.CopyTo(updated.PutEmpty(key)) } case modeKey: if compiledPattern.MatchString(key) { if !fn.IsEmpty() { - result := []byte{} - submatches := compiledPattern.FindStringSubmatchIndex(originalValue.Str()) - result = compiledPattern.ExpandString(result, replacementVal, originalValue.Str(), submatches) - replacementVal = string(result) - fnVal := fn.Get() - replaceValGetter := ottl.StandardStringGetter[K]{ - Getter: func(context.Context, K) (any, error) { - return replacementVal, nil - }, - } - replacementExpr, errNew := fnVal.Get(&replaceAllPatternFuncArgs[K]{Input: replaceValGetter}) - if errNew != nil { - return false - } - replacementValRaw, errNew := replacementExpr.Eval(ctx, tCtx) - if errNew != nil { - return false - } - replacementValStr, ok := replacementValRaw.(string) - if !ok { - return false + foundMatch := false + var updatedString string + updatedString = key + submatches := compiledPattern.FindAllStringSubmatchIndex(updatedString, -1) + for _, submatch := range submatches { + for i := 2; i < len(submatch); i += 2 { + fnVal := fn.Get() + old := originalValue.Str()[submatch[i]:submatch[i+1]] + replaceValGetter := ottl.StandardStringGetter[K]{ + Getter: func(context.Context, K) (any, error) { + return old, nil + }, + } + replacementExpr, errNew := fnVal.Get(&replaceAllPatternFuncArgs[K]{Input: replaceValGetter}) + if errNew != nil { + return false + } + replacementValRaw, errNew := replacementExpr.Eval(ctx, tCtx) + if errNew != nil { + return false + } + replacementValStr, ok := replacementValRaw.(string) + if !ok { + return false + } + updatedString = strings.ReplaceAll(updatedString, old, replacementValStr) + foundMatch = true + updated.PutStr(key, updatedString) + } + if !foundMatch { + originalValue.CopyTo(updated.PutEmpty(key)) + } } - replacementVal = replacementValStr + } else { + updatedKey := compiledPattern.ReplaceAllString(key, replacementVal) + originalValue.CopyTo(updated.PutEmpty(updatedKey)) } - updatedKey := compiledPattern.ReplaceAllString(key, replacementVal) - originalValue.CopyTo(updated.PutEmpty(updatedKey)) } else { originalValue.CopyTo(updated.PutEmpty(key)) } diff --git a/pkg/ottl/ottlfuncs/func_replace_all_patterns_test.go b/pkg/ottl/ottlfuncs/func_replace_all_patterns_test.go index 1cf6f8bb2dda..607ed8e22f42 100644 --- a/pkg/ottl/ottlfuncs/func_replace_all_patterns_test.go +++ b/pkg/ottl/ottlfuncs/func_replace_all_patterns_test.go @@ -59,8 +59,8 @@ func Test_replaceAllPatterns(t *testing.T) { }, function: optionalArg, want: func(expectedMap pcommon.Map) { - expectedMap.PutStr("test", "hash(hello {universe}) world") - expectedMap.PutStr("test2", "hash(hash(hello {universe}))") // hash applied twice because of replace_all_patterns + expectedMap.PutStr("test", "hello world") + expectedMap.PutStr("test2", "hello") expectedMap.PutStr("test3", "goodbye world1 and world2") expectedMap.PutInt("test4", 1234) expectedMap.PutDouble("test5", 1234) @@ -80,7 +80,7 @@ func Test_replaceAllPatterns(t *testing.T) { function: optionalArg, want: func(expectedMap pcommon.Map) { expectedMap.PutStr("test", "hash(hello) world") - expectedMap.PutStr("test2", "hash(hash(hello))") // hash applied twice because of replace_all_patterns + expectedMap.PutStr("test2", "hash(hello)") expectedMap.PutStr("test3", "goodbye world1 and world2") expectedMap.PutInt("test4", 1234) expectedMap.PutDouble("test5", 1234) @@ -99,8 +99,8 @@ func Test_replaceAllPatterns(t *testing.T) { }, function: optionalArg, want: func(expectedMap pcommon.Map) { - expectedMap.PutStr("test", "hash() world") - expectedMap.PutStr("test2", "hash(hash())") + expectedMap.PutStr("test", "hello world") + expectedMap.PutStr("test2", "hello") expectedMap.PutStr("test3", "goodbye world1 and world2") expectedMap.PutInt("test4", 1234) expectedMap.PutDouble("test5", 1234) @@ -241,7 +241,7 @@ func Test_replaceAllPatterns(t *testing.T) { want: func(expectedMap pcommon.Map) { expectedMap.PutStr("test", "hello world") expectedMap.PutStr("test2", "hello") - expectedMap.PutStr("test3", "goodbye hash(world1)") + expectedMap.PutStr("test3", "goodbye hash(world1) and hash(world2)") expectedMap.PutInt("test4", 1234) expectedMap.PutDouble("test5", 1234) expectedMap.PutBool("test6", true) @@ -251,7 +251,7 @@ func Test_replaceAllPatterns(t *testing.T) { name: "regex match (with multiple matches from one capture group and hash function)", target: target, mode: modeValue, - pattern: `(world\d)`, + pattern: `(world[\d^)])`, replacement: ottl.StandardStringGetter[pcommon.Map]{ Getter: func(context.Context, pcommon.Map) (any, error) { return "$1", nil @@ -261,7 +261,7 @@ func Test_replaceAllPatterns(t *testing.T) { want: func(expectedMap pcommon.Map) { expectedMap.PutStr("test", "hello world") expectedMap.PutStr("test2", "hello") - expectedMap.PutStr("test3", "goodbye hash(world1) and hash(world1)") + expectedMap.PutStr("test3", "goodbye hash(world1) and hash(world2)") expectedMap.PutInt("test4", 1234) expectedMap.PutDouble("test5", 1234) expectedMap.PutBool("test6", true) diff --git a/pkg/ottl/ottlfuncs/func_replace_pattern.go b/pkg/ottl/ottlfuncs/func_replace_pattern.go index 81d43f0cf2cf..04c9e2ae1606 100644 --- a/pkg/ottl/ottlfuncs/func_replace_pattern.go +++ b/pkg/ottl/ottlfuncs/func_replace_pattern.go @@ -57,34 +57,51 @@ func replacePattern[K any](target ottl.GetSetter[K], regexPattern string, replac if originalValStr, ok := originalVal.(string); ok { if compiledPattern.MatchString(originalValStr) { if !fn.IsEmpty() { - result := []byte{} - submatches := compiledPattern.FindStringSubmatchIndex(originalValStr) - result = compiledPattern.ExpandString(result, replacementVal, originalValStr, submatches) - replacementVal = string(result) - fnVal := fn.Get() - replaceValGetter := ottl.StandardStringGetter[K]{ - Getter: func(context.Context, K) (any, error) { - return replacementVal, nil - }, + var updatedString string + foundMatch := false + updatedString = originalValStr + submatches := compiledPattern.FindAllStringSubmatchIndex(updatedString, -1) + for _, submatch := range submatches { + for i := 2; i < len(submatch); i += 2 { + fnVal := fn.Get() + old := originalValStr[submatch[i]:submatch[i+1]] + replaceValGetter := ottl.StandardStringGetter[K]{ + Getter: func(context.Context, K) (any, error) { + return old, nil + }, + } + replacementExpr, errNew := fnVal.Get(&replacePatternFuncArgs[K]{Input: replaceValGetter}) + if errNew != nil { + return nil, errNew + } + replacementValRaw, errNew := replacementExpr.Eval(ctx, tCtx) + if errNew != nil { + return nil, errNew + } + replacementValStr, ok := replacementValRaw.(string) + if !ok { + return nil, fmt.Errorf("replacement value is not a string") + } + updatedString = compiledPattern.ReplaceAllString(updatedString, replacementValStr) + foundMatch = true + err = target.Set(ctx, tCtx, updatedString) + if err != nil { + return nil, err + } + } + if !foundMatch { + err = target.Set(ctx, tCtx, updatedString) + if err != nil { + return nil, err + } + } } - replacementExpr, errNew := fnVal.Get(&replacePatternFuncArgs[K]{Input: replaceValGetter}) - if errNew != nil { - return nil, errNew + } else { + updatedStr := compiledPattern.ReplaceAllString(originalValStr, replacementVal) + err = target.Set(ctx, tCtx, updatedStr) + if err != nil { + return nil, err } - replacementValRaw, errNew := replacementExpr.Eval(ctx, tCtx) - if errNew != nil { - return nil, errNew - } - replacementValStr, ok := replacementValRaw.(string) - if !ok { - return nil, fmt.Errorf("replacement value is not a string") - } - replacementVal = replacementValStr - } - updatedStr := compiledPattern.ReplaceAllString(originalValStr, replacementVal) - err = target.Set(ctx, tCtx, updatedStr) - if err != nil { - return nil, err } } } diff --git a/pkg/ottl/ottlfuncs/func_replace_pattern_test.go b/pkg/ottl/ottlfuncs/func_replace_pattern_test.go index bd6e95cfc734..c00db3fdc9c7 100644 --- a/pkg/ottl/ottlfuncs/func_replace_pattern_test.go +++ b/pkg/ottl/ottlfuncs/func_replace_pattern_test.go @@ -76,7 +76,7 @@ func Test_replacePattern(t *testing.T) { { name: "replace regex match (with hash function)", target: target, - pattern: `passwd\=([^\s]*)(\s?)`, + pattern: `passwd\=([^\s]*)\s?`, replacement: ottl.StandardStringGetter[pcommon.Value]{ Getter: func(context.Context, pcommon.Value) (any, error) { return "$1", nil @@ -90,15 +90,16 @@ func Test_replacePattern(t *testing.T) { { name: "replace regex match (capture group without $1)", target: target, - pattern: `passwd\=([^\s]*)(\s?)`, + pattern: `passwd\=([^\s]*)\s?`, replacement: ottl.StandardStringGetter[pcommon.Value]{ Getter: func(context.Context, pcommon.Value) (any, error) { - return "test", nil + return "passwd=$1", nil }, }, function: optionalArg, want: func(expectedValue pcommon.Value) { - expectedValue.SetStr("application hash(test)otherarg=notsensitive key1 key2") + // TODO: We should expect "application passwd=hash(sensitivedtata) otherarg=notsensitive key1 key2" + expectedValue.SetStr("application hash(sensitivedtata)otherarg=notsensitive key1 key2") // since we only hash capture groups }, }, { @@ -112,7 +113,7 @@ func Test_replacePattern(t *testing.T) { }, function: optionalArg, want: func(expectedValue pcommon.Value) { - expectedValue.SetStr("application hash()otherarg=notsensitive key1 key2") + expectedValue.SetStr("application passwd=sensitivedtata otherarg=notsensitive key1 key2") }, }, { From 6e182a2ce4a561f1807ec5c340a4476f6a223b22 Mon Sep 17 00:00:00 2001 From: Raj Nishtala Date: Wed, 13 Dec 2023 15:56:53 -0500 Subject: [PATCH 08/10] Only hash the capture group specified in the replacement string --- .../ottlfuncs/func_replace_all_patterns.go | 38 +++++++++++++++++-- .../func_replace_all_patterns_test.go | 6 +-- pkg/ottl/ottlfuncs/func_replace_pattern.go | 31 ++++++++++++++- .../ottlfuncs/func_replace_pattern_test.go | 4 +- 4 files changed, 68 insertions(+), 11 deletions(-) diff --git a/pkg/ottl/ottlfuncs/func_replace_all_patterns.go b/pkg/ottl/ottlfuncs/func_replace_all_patterns.go index 07e2f6eaf8e2..0cf1afc7a5d3 100644 --- a/pkg/ottl/ottlfuncs/func_replace_all_patterns.go +++ b/pkg/ottl/ottlfuncs/func_replace_all_patterns.go @@ -7,6 +7,7 @@ import ( "context" "fmt" "regexp" + "strconv" "strings" "go.opentelemetry.io/collector/pdata/pcommon" @@ -72,13 +73,28 @@ func replaceAllPatterns[K any](target ottl.PMapGetter[K], mode string, regexPatt if compiledPattern.MatchString(originalValue.Str()) { if !fn.IsEmpty() { var updatedString string + var groupNum int foundMatch := false updatedString = originalValue.Str() submatches := compiledPattern.FindAllStringSubmatchIndex(updatedString, -1) + // check if the string is indeed a group number string + if !IsGroupNumString(replacementVal) { + updatedString = compiledPattern.ReplaceAllString(originalValue.Str(), replacementVal) + updated.PutStr(key, updatedString) + return true + } + // parse the group number string to get actual group number + captureGroup := strings.TrimPrefix(replacementVal, "$") + groupNum, err = strconv.Atoi(captureGroup) + if err != nil { + return false + } for _, submatch := range submatches { - for i := 2; i < len(submatch); i += 2 { + groupStart := 2 * groupNum + groupEnd := 2*groupStart - 1 + if len(submatch) > groupEnd { fnVal := fn.Get() - old := originalValue.Str()[submatch[i]:submatch[i+1]] + old := originalValue.Str()[submatch[groupStart]:submatch[groupEnd]] replaceValGetter := ottl.StandardStringGetter[K]{ Getter: func(context.Context, K) (any, error) { return old, nil @@ -118,10 +134,24 @@ func replaceAllPatterns[K any](target ottl.PMapGetter[K], mode string, regexPatt var updatedString string updatedString = key submatches := compiledPattern.FindAllStringSubmatchIndex(updatedString, -1) + // check if the string is indeed a group number string or if there is no capture group + if !IsGroupNumString(replacementVal) { + updatedString = compiledPattern.ReplaceAllString(originalValue.Str(), replacementVal) + updated.PutStr(key, updatedString) + return true + } + // parse the group number string to get actual group number + captureGroup := strings.TrimPrefix(replacementVal, "$") + groupNum, err := strconv.Atoi(captureGroup) + if err != nil { + return false + } for _, submatch := range submatches { - for i := 2; i < len(submatch); i += 2 { + groupStart := 2 * groupNum + groupEnd := 2*groupStart - 1 + if len(submatch) > groupEnd { fnVal := fn.Get() - old := originalValue.Str()[submatch[i]:submatch[i+1]] + old := originalValue.Str()[submatch[groupStart]:submatch[groupEnd]] replaceValGetter := ottl.StandardStringGetter[K]{ Getter: func(context.Context, K) (any, error) { return old, nil diff --git a/pkg/ottl/ottlfuncs/func_replace_all_patterns_test.go b/pkg/ottl/ottlfuncs/func_replace_all_patterns_test.go index 607ed8e22f42..39f8acd92e1e 100644 --- a/pkg/ottl/ottlfuncs/func_replace_all_patterns_test.go +++ b/pkg/ottl/ottlfuncs/func_replace_all_patterns_test.go @@ -59,8 +59,8 @@ func Test_replaceAllPatterns(t *testing.T) { }, function: optionalArg, want: func(expectedMap pcommon.Map) { - expectedMap.PutStr("test", "hello world") - expectedMap.PutStr("test2", "hello") + expectedMap.PutStr("test", "hello {universe} world") + expectedMap.PutStr("test2", "hello {universe}") expectedMap.PutStr("test3", "goodbye world1 and world2") expectedMap.PutInt("test4", 1234) expectedMap.PutDouble("test5", 1234) @@ -241,7 +241,7 @@ func Test_replaceAllPatterns(t *testing.T) { want: func(expectedMap pcommon.Map) { expectedMap.PutStr("test", "hello world") expectedMap.PutStr("test2", "hello") - expectedMap.PutStr("test3", "goodbye hash(world1) and hash(world2)") + expectedMap.PutStr("test3", "goodbye hash(world1) and world2") expectedMap.PutInt("test4", 1234) expectedMap.PutDouble("test5", 1234) expectedMap.PutBool("test6", true) diff --git a/pkg/ottl/ottlfuncs/func_replace_pattern.go b/pkg/ottl/ottlfuncs/func_replace_pattern.go index 04c9e2ae1606..b0f253d32073 100644 --- a/pkg/ottl/ottlfuncs/func_replace_pattern.go +++ b/pkg/ottl/ottlfuncs/func_replace_pattern.go @@ -7,6 +7,8 @@ import ( "context" "fmt" "regexp" + "strconv" + "strings" "github.com/open-telemetry/opentelemetry-collector-contrib/pkg/ottl" ) @@ -36,6 +38,12 @@ func createReplacePatternFunction[K any](_ ottl.FunctionContext, oArgs ottl.Argu return replacePattern(args.Target, args.RegexPattern, args.Replacement, args.Function) } +// function to check if a string is a group number string like "$1", "$2", ... +func IsGroupNumString(s string) bool { + match, _ := regexp.MatchString(`^\$\d+$`, s) + return match +} + func replacePattern[K any](target ottl.GetSetter[K], regexPattern string, replacement ottl.StringGetter[K], fn ottl.Optional[ottl.FunctionGetter[K]]) (ottl.ExprFunc[K], error) { compiledPattern, err := regexp.Compile(regexPattern) if err != nil { @@ -58,13 +66,32 @@ func replacePattern[K any](target ottl.GetSetter[K], regexPattern string, replac if compiledPattern.MatchString(originalValStr) { if !fn.IsEmpty() { var updatedString string + var groupNum int foundMatch := false updatedString = originalValStr + // check if the string is indeed a group number string + if !IsGroupNumString(replacementVal) { + updatedStr := compiledPattern.ReplaceAllString(originalValStr, replacementVal) + err = target.Set(ctx, tCtx, updatedStr) + if err != nil { + return nil, err + } + return nil, nil + } + // parse the group number string to get actual group number + captureGroup := strings.TrimPrefix(replacementVal, "$") + groupNum, err = strconv.Atoi(captureGroup) + if err != nil { + return nil, err + } submatches := compiledPattern.FindAllStringSubmatchIndex(updatedString, -1) for _, submatch := range submatches { - for i := 2; i < len(submatch); i += 2 { + groupStart := 2 * groupNum + groupEnd := 2*groupStart - 1 + if len(submatch) > groupEnd { + fmt.Println("submatch: ", submatch) fnVal := fn.Get() - old := originalValStr[submatch[i]:submatch[i+1]] + old := originalValStr[submatch[groupStart]:submatch[groupEnd]] replaceValGetter := ottl.StandardStringGetter[K]{ Getter: func(context.Context, K) (any, error) { return old, nil diff --git a/pkg/ottl/ottlfuncs/func_replace_pattern_test.go b/pkg/ottl/ottlfuncs/func_replace_pattern_test.go index c00db3fdc9c7..46405a3bbf6f 100644 --- a/pkg/ottl/ottlfuncs/func_replace_pattern_test.go +++ b/pkg/ottl/ottlfuncs/func_replace_pattern_test.go @@ -93,13 +93,13 @@ func Test_replacePattern(t *testing.T) { pattern: `passwd\=([^\s]*)\s?`, replacement: ottl.StandardStringGetter[pcommon.Value]{ Getter: func(context.Context, pcommon.Value) (any, error) { - return "passwd=$1", nil + return "passwd=$1 ", nil }, }, function: optionalArg, want: func(expectedValue pcommon.Value) { // TODO: We should expect "application passwd=hash(sensitivedtata) otherarg=notsensitive key1 key2" - expectedValue.SetStr("application hash(sensitivedtata)otherarg=notsensitive key1 key2") // since we only hash capture groups + expectedValue.SetStr("application passwd=sensitivedtata otherarg=notsensitive key1 key2") // since we only hash capture groups }, }, { From 7a03a7ef44cbc0afce3c0d7dc41f29583ac05ac7 Mon Sep 17 00:00:00 2001 From: Raj Nishtala Date: Fri, 15 Dec 2023 16:52:20 -0500 Subject: [PATCH 09/10] Use ExpandString instead of manually resolving submatches --- .../ottlfuncs/func_replace_all_patterns.go | 99 +---------------- .../func_replace_all_patterns_test.go | 24 ++++- pkg/ottl/ottlfuncs/func_replace_pattern.go | 101 ++++++++---------- .../ottlfuncs/func_replace_pattern_test.go | 6 +- 4 files changed, 76 insertions(+), 154 deletions(-) diff --git a/pkg/ottl/ottlfuncs/func_replace_all_patterns.go b/pkg/ottl/ottlfuncs/func_replace_all_patterns.go index 0cf1afc7a5d3..e36759acc417 100644 --- a/pkg/ottl/ottlfuncs/func_replace_all_patterns.go +++ b/pkg/ottl/ottlfuncs/func_replace_all_patterns.go @@ -7,8 +7,6 @@ import ( "context" "fmt" "regexp" - "strconv" - "strings" "go.opentelemetry.io/collector/pdata/pcommon" @@ -28,10 +26,6 @@ type ReplaceAllPatternsArguments[K any] struct { Function ottl.Optional[ottl.FunctionGetter[K]] } -type replaceAllPatternFuncArgs[K any] struct { - Input ottl.StringGetter[K] -} - func NewReplaceAllPatternsFactory[K any]() ottl.Factory[K] { return ottl.NewFactory("replace_all_patterns", &ReplaceAllPatternsArguments[K]{}, createReplaceAllPatternsFunction[K]) } @@ -72,54 +66,11 @@ func replaceAllPatterns[K any](target ottl.PMapGetter[K], mode string, regexPatt case modeValue: if compiledPattern.MatchString(originalValue.Str()) { if !fn.IsEmpty() { - var updatedString string - var groupNum int - foundMatch := false - updatedString = originalValue.Str() - submatches := compiledPattern.FindAllStringSubmatchIndex(updatedString, -1) - // check if the string is indeed a group number string - if !IsGroupNumString(replacementVal) { - updatedString = compiledPattern.ReplaceAllString(originalValue.Str(), replacementVal) - updated.PutStr(key, updatedString) - return true - } - // parse the group number string to get actual group number - captureGroup := strings.TrimPrefix(replacementVal, "$") - groupNum, err = strconv.Atoi(captureGroup) + updatedString, err := ApplyReplaceFunction(ctx, tCtx, compiledPattern, fn, originalValue.Str(), replacementVal) if err != nil { return false } - for _, submatch := range submatches { - groupStart := 2 * groupNum - groupEnd := 2*groupStart - 1 - if len(submatch) > groupEnd { - fnVal := fn.Get() - old := originalValue.Str()[submatch[groupStart]:submatch[groupEnd]] - replaceValGetter := ottl.StandardStringGetter[K]{ - Getter: func(context.Context, K) (any, error) { - return old, nil - }, - } - replacementExpr, errNew := fnVal.Get(&replaceAllPatternFuncArgs[K]{Input: replaceValGetter}) - if errNew != nil { - return false - } - replacementValRaw, errNew := replacementExpr.Eval(ctx, tCtx) - if errNew != nil { - return false - } - replacementValStr, ok := replacementValRaw.(string) - if !ok { - return false - } - updatedString = strings.ReplaceAll(updatedString, old, replacementValStr) - foundMatch = true - updated.PutStr(key, updatedString) - } - if !foundMatch { - originalValue.CopyTo(updated.PutEmpty(key)) - } - } + updated.PutStr(key, updatedString) } else { updatedString := compiledPattern.ReplaceAllString(originalValue.Str(), replacementVal) updated.PutStr(key, updatedString) @@ -130,53 +81,11 @@ func replaceAllPatterns[K any](target ottl.PMapGetter[K], mode string, regexPatt case modeKey: if compiledPattern.MatchString(key) { if !fn.IsEmpty() { - foundMatch := false - var updatedString string - updatedString = key - submatches := compiledPattern.FindAllStringSubmatchIndex(updatedString, -1) - // check if the string is indeed a group number string or if there is no capture group - if !IsGroupNumString(replacementVal) { - updatedString = compiledPattern.ReplaceAllString(originalValue.Str(), replacementVal) - updated.PutStr(key, updatedString) - return true - } - // parse the group number string to get actual group number - captureGroup := strings.TrimPrefix(replacementVal, "$") - groupNum, err := strconv.Atoi(captureGroup) + updatedString, err := ApplyReplaceFunction(ctx, tCtx, compiledPattern, fn, key, replacementVal) if err != nil { return false } - for _, submatch := range submatches { - groupStart := 2 * groupNum - groupEnd := 2*groupStart - 1 - if len(submatch) > groupEnd { - fnVal := fn.Get() - old := originalValue.Str()[submatch[groupStart]:submatch[groupEnd]] - replaceValGetter := ottl.StandardStringGetter[K]{ - Getter: func(context.Context, K) (any, error) { - return old, nil - }, - } - replacementExpr, errNew := fnVal.Get(&replaceAllPatternFuncArgs[K]{Input: replaceValGetter}) - if errNew != nil { - return false - } - replacementValRaw, errNew := replacementExpr.Eval(ctx, tCtx) - if errNew != nil { - return false - } - replacementValStr, ok := replacementValRaw.(string) - if !ok { - return false - } - updatedString = strings.ReplaceAll(updatedString, old, replacementValStr) - foundMatch = true - updated.PutStr(key, updatedString) - } - if !foundMatch { - originalValue.CopyTo(updated.PutEmpty(key)) - } - } + updated.PutStr(key, updatedString) } else { updatedKey := compiledPattern.ReplaceAllString(key, replacementVal) originalValue.CopyTo(updated.PutEmpty(updatedKey)) diff --git a/pkg/ottl/ottlfuncs/func_replace_all_patterns_test.go b/pkg/ottl/ottlfuncs/func_replace_all_patterns_test.go index 39f8acd92e1e..d05e597dd0c9 100644 --- a/pkg/ottl/ottlfuncs/func_replace_all_patterns_test.go +++ b/pkg/ottl/ottlfuncs/func_replace_all_patterns_test.go @@ -241,7 +241,27 @@ func Test_replaceAllPatterns(t *testing.T) { want: func(expectedMap pcommon.Map) { expectedMap.PutStr("test", "hello world") expectedMap.PutStr("test2", "hello") - expectedMap.PutStr("test3", "goodbye hash(world1) and world2") + expectedMap.PutStr("test3", "goodbye hash(world1)") + expectedMap.PutInt("test4", 1234) + expectedMap.PutDouble("test5", 1234) + expectedMap.PutBool("test6", true) + }, + }, + { + name: "regex match (with multiple capture groups and hash function)", + target: target, + mode: modeValue, + pattern: `(world1) and (world2)`, + replacement: ottl.StandardStringGetter[pcommon.Map]{ + Getter: func(context.Context, pcommon.Map) (any, error) { + return "$2", nil + }, + }, + function: optionalArg, + want: func(expectedMap pcommon.Map) { + expectedMap.PutStr("test", "hello world") + expectedMap.PutStr("test2", "hello") + expectedMap.PutStr("test3", "goodbye hash(world2)") expectedMap.PutInt("test4", 1234) expectedMap.PutDouble("test5", 1234) expectedMap.PutBool("test6", true) @@ -251,7 +271,7 @@ func Test_replaceAllPatterns(t *testing.T) { name: "regex match (with multiple matches from one capture group and hash function)", target: target, mode: modeValue, - pattern: `(world[\d^)])`, + pattern: `(world\d)`, replacement: ottl.StandardStringGetter[pcommon.Map]{ Getter: func(context.Context, pcommon.Map) (any, error) { return "$1", nil diff --git a/pkg/ottl/ottlfuncs/func_replace_pattern.go b/pkg/ottl/ottlfuncs/func_replace_pattern.go index b0f253d32073..bdc50fe08c53 100644 --- a/pkg/ottl/ottlfuncs/func_replace_pattern.go +++ b/pkg/ottl/ottlfuncs/func_replace_pattern.go @@ -7,7 +7,6 @@ import ( "context" "fmt" "regexp" - "strconv" "strings" "github.com/open-telemetry/opentelemetry-collector-contrib/pkg/ottl" @@ -44,6 +43,43 @@ func IsGroupNumString(s string) bool { return match } +func ApplyReplaceFunction[K any](ctx context.Context, tCtx K, compiledPattern *regexp.Regexp, fn ottl.Optional[ottl.FunctionGetter[K]], originalValStr string, replacementVal string) (string, error) { + var updatedString string + updatedString = originalValStr + submatches := compiledPattern.FindAllStringSubmatchIndex(updatedString, -1) + for _, submatch := range submatches { + fullMatch := originalValStr[submatch[0]:submatch[1]] + result := compiledPattern.ExpandString([]byte{}, replacementVal, originalValStr, submatch) + fnVal := fn.Get() + replaceValGetter := ottl.StandardStringGetter[K]{ + Getter: func(context.Context, K) (any, error) { + return string(result), nil + }, + } + replacementExpr, errNew := fnVal.Get(&replacePatternFuncArgs[K]{Input: replaceValGetter}) + if errNew != nil { + return "", errNew + } + replacementValRaw, errNew := replacementExpr.Eval(ctx, tCtx) + if errNew != nil { + return "", errNew + } + replacementValStr, ok := replacementValRaw.(string) + if !ok { + return "", fmt.Errorf("the replacement value must be a string") + } + // if there are submatches then use the resolved capture group value + if len(submatch) > 2 { + updatedString = strings.ReplaceAll(updatedString, fullMatch, replacementValStr) + } + // if there are no submatches and the replacement string isn't a capture group then use the replacement value as is + if !IsGroupNumString(replacementVal) { + updatedString = strings.ReplaceAll(updatedString, fullMatch, replacementVal) + } + } + return updatedString, nil +} + func replacePattern[K any](target ottl.GetSetter[K], regexPattern string, replacement ottl.StringGetter[K], fn ottl.Optional[ottl.FunctionGetter[K]]) (ottl.ExprFunc[K], error) { compiledPattern, err := regexp.Compile(regexPattern) if err != nil { @@ -66,62 +102,13 @@ func replacePattern[K any](target ottl.GetSetter[K], regexPattern string, replac if compiledPattern.MatchString(originalValStr) { if !fn.IsEmpty() { var updatedString string - var groupNum int - foundMatch := false - updatedString = originalValStr - // check if the string is indeed a group number string - if !IsGroupNumString(replacementVal) { - updatedStr := compiledPattern.ReplaceAllString(originalValStr, replacementVal) - err = target.Set(ctx, tCtx, updatedStr) - if err != nil { - return nil, err - } - return nil, nil - } - // parse the group number string to get actual group number - captureGroup := strings.TrimPrefix(replacementVal, "$") - groupNum, err = strconv.Atoi(captureGroup) + updatedString, err = ApplyReplaceFunction[K](ctx, tCtx, compiledPattern, fn, originalValStr, replacementVal) if err != nil { return nil, err } - submatches := compiledPattern.FindAllStringSubmatchIndex(updatedString, -1) - for _, submatch := range submatches { - groupStart := 2 * groupNum - groupEnd := 2*groupStart - 1 - if len(submatch) > groupEnd { - fmt.Println("submatch: ", submatch) - fnVal := fn.Get() - old := originalValStr[submatch[groupStart]:submatch[groupEnd]] - replaceValGetter := ottl.StandardStringGetter[K]{ - Getter: func(context.Context, K) (any, error) { - return old, nil - }, - } - replacementExpr, errNew := fnVal.Get(&replacePatternFuncArgs[K]{Input: replaceValGetter}) - if errNew != nil { - return nil, errNew - } - replacementValRaw, errNew := replacementExpr.Eval(ctx, tCtx) - if errNew != nil { - return nil, errNew - } - replacementValStr, ok := replacementValRaw.(string) - if !ok { - return nil, fmt.Errorf("replacement value is not a string") - } - updatedString = compiledPattern.ReplaceAllString(updatedString, replacementValStr) - foundMatch = true - err = target.Set(ctx, tCtx, updatedString) - if err != nil { - return nil, err - } - } - if !foundMatch { - err = target.Set(ctx, tCtx, updatedString) - if err != nil { - return nil, err - } - } + err = target.Set(ctx, tCtx, updatedString) + if err != nil { + return nil, err } } else { updatedStr := compiledPattern.ReplaceAllString(originalValStr, replacementVal) @@ -130,6 +117,12 @@ func replacePattern[K any](target ottl.GetSetter[K], regexPattern string, replac return nil, err } } + } else { + updatedStr := compiledPattern.ReplaceAllString(originalValStr, replacementVal) + err = target.Set(ctx, tCtx, updatedStr) + if err != nil { + return nil, err + } } } return nil, nil diff --git a/pkg/ottl/ottlfuncs/func_replace_pattern_test.go b/pkg/ottl/ottlfuncs/func_replace_pattern_test.go index 46405a3bbf6f..f92f881c7523 100644 --- a/pkg/ottl/ottlfuncs/func_replace_pattern_test.go +++ b/pkg/ottl/ottlfuncs/func_replace_pattern_test.go @@ -90,16 +90,16 @@ func Test_replacePattern(t *testing.T) { { name: "replace regex match (capture group without $1)", target: target, - pattern: `passwd\=([^\s]*)\s?`, + pattern: `passwd\=([^\s]*)`, replacement: ottl.StandardStringGetter[pcommon.Value]{ Getter: func(context.Context, pcommon.Value) (any, error) { - return "passwd=$1 ", nil + return "passwd=$1", nil }, }, function: optionalArg, want: func(expectedValue pcommon.Value) { // TODO: We should expect "application passwd=hash(sensitivedtata) otherarg=notsensitive key1 key2" - expectedValue.SetStr("application passwd=sensitivedtata otherarg=notsensitive key1 key2") // since we only hash capture groups + expectedValue.SetStr("application hash(passwd=$1) otherarg=notsensitive key1 key2") // since we only hash capture groups }, }, { From 04faa3d010b88ec23bf373253023475b91a34f3d Mon Sep 17 00:00:00 2001 From: Raj Nishtala Date: Tue, 19 Dec 2023 13:42:57 -0500 Subject: [PATCH 10/10] Always apply the optional function if provided --- .../func_replace_all_matches_test.go | 2 +- .../ottlfuncs/func_replace_all_patterns.go | 4 +-- .../func_replace_all_patterns_test.go | 10 +++---- pkg/ottl/ottlfuncs/func_replace_match_test.go | 2 +- pkg/ottl/ottlfuncs/func_replace_pattern.go | 25 ++--------------- .../ottlfuncs/func_replace_pattern_test.go | 28 +++++++++---------- 6 files changed, 25 insertions(+), 46 deletions(-) diff --git a/pkg/ottl/ottlfuncs/func_replace_all_matches_test.go b/pkg/ottl/ottlfuncs/func_replace_all_matches_test.go index 9ed7857926e8..3494aa0d4866 100644 --- a/pkg/ottl/ottlfuncs/func_replace_all_matches_test.go +++ b/pkg/ottl/ottlfuncs/func_replace_all_matches_test.go @@ -25,7 +25,7 @@ func Test_replaceAllMatches(t *testing.T) { FCtx: ottl.FunctionContext{ Set: componenttest.NewNopTelemetrySettings(), }, - Fact: NewTestFactory[pcommon.Map](), + Fact: optionalFnTestFactory[pcommon.Map](), } optionalArg := ottl.NewTestingOptional[ottl.FunctionGetter[pcommon.Map]](ottlValue) diff --git a/pkg/ottl/ottlfuncs/func_replace_all_patterns.go b/pkg/ottl/ottlfuncs/func_replace_all_patterns.go index e36759acc417..461741b90b11 100644 --- a/pkg/ottl/ottlfuncs/func_replace_all_patterns.go +++ b/pkg/ottl/ottlfuncs/func_replace_all_patterns.go @@ -66,7 +66,7 @@ func replaceAllPatterns[K any](target ottl.PMapGetter[K], mode string, regexPatt case modeValue: if compiledPattern.MatchString(originalValue.Str()) { if !fn.IsEmpty() { - updatedString, err := ApplyReplaceFunction(ctx, tCtx, compiledPattern, fn, originalValue.Str(), replacementVal) + updatedString, err := applyOptReplaceFunction(ctx, tCtx, compiledPattern, fn, originalValue.Str(), replacementVal) if err != nil { return false } @@ -81,7 +81,7 @@ func replaceAllPatterns[K any](target ottl.PMapGetter[K], mode string, regexPatt case modeKey: if compiledPattern.MatchString(key) { if !fn.IsEmpty() { - updatedString, err := ApplyReplaceFunction(ctx, tCtx, compiledPattern, fn, key, replacementVal) + updatedString, err := applyOptReplaceFunction(ctx, tCtx, compiledPattern, fn, key, replacementVal) if err != nil { return false } diff --git a/pkg/ottl/ottlfuncs/func_replace_all_patterns_test.go b/pkg/ottl/ottlfuncs/func_replace_all_patterns_test.go index d05e597dd0c9..b02e84297199 100644 --- a/pkg/ottl/ottlfuncs/func_replace_all_patterns_test.go +++ b/pkg/ottl/ottlfuncs/func_replace_all_patterns_test.go @@ -28,7 +28,7 @@ func Test_replaceAllPatterns(t *testing.T) { FCtx: ottl.FunctionContext{ Set: componenttest.NewNopTelemetrySettings(), }, - Fact: NewTestFactory[pcommon.Map](), + Fact: optionalFnTestFactory[pcommon.Map](), } optionalArg := ottl.NewTestingOptional[ottl.FunctionGetter[pcommon.Map]](ottlValue) @@ -59,8 +59,8 @@ func Test_replaceAllPatterns(t *testing.T) { }, function: optionalArg, want: func(expectedMap pcommon.Map) { - expectedMap.PutStr("test", "hello {universe} world") - expectedMap.PutStr("test2", "hello {universe}") + expectedMap.PutStr("test", "hash(hello {universe}) world") + expectedMap.PutStr("test2", "hash(hello {universe})") expectedMap.PutStr("test3", "goodbye world1 and world2") expectedMap.PutInt("test4", 1234) expectedMap.PutDouble("test5", 1234) @@ -99,8 +99,8 @@ func Test_replaceAllPatterns(t *testing.T) { }, function: optionalArg, want: func(expectedMap pcommon.Map) { - expectedMap.PutStr("test", "hello world") - expectedMap.PutStr("test2", "hello") + expectedMap.PutStr("test", "hash() world") + expectedMap.PutStr("test2", "hash()") expectedMap.PutStr("test3", "goodbye world1 and world2") expectedMap.PutInt("test4", 1234) expectedMap.PutDouble("test5", 1234) diff --git a/pkg/ottl/ottlfuncs/func_replace_match_test.go b/pkg/ottl/ottlfuncs/func_replace_match_test.go index 089ab141102c..a61e28560f90 100644 --- a/pkg/ottl/ottlfuncs/func_replace_match_test.go +++ b/pkg/ottl/ottlfuncs/func_replace_match_test.go @@ -21,7 +21,7 @@ func Test_replaceMatch(t *testing.T) { FCtx: ottl.FunctionContext{ Set: componenttest.NewNopTelemetrySettings(), }, - Fact: NewTestFactory[pcommon.Value](), + Fact: optionalFnTestFactory[pcommon.Value](), } optionalArg := ottl.NewTestingOptional[ottl.FunctionGetter[pcommon.Value]](ottlValue) target := &ottl.StandardGetSetter[pcommon.Value]{ diff --git a/pkg/ottl/ottlfuncs/func_replace_pattern.go b/pkg/ottl/ottlfuncs/func_replace_pattern.go index bdc50fe08c53..7b02b3896ea1 100644 --- a/pkg/ottl/ottlfuncs/func_replace_pattern.go +++ b/pkg/ottl/ottlfuncs/func_replace_pattern.go @@ -37,13 +37,7 @@ func createReplacePatternFunction[K any](_ ottl.FunctionContext, oArgs ottl.Argu return replacePattern(args.Target, args.RegexPattern, args.Replacement, args.Function) } -// function to check if a string is a group number string like "$1", "$2", ... -func IsGroupNumString(s string) bool { - match, _ := regexp.MatchString(`^\$\d+$`, s) - return match -} - -func ApplyReplaceFunction[K any](ctx context.Context, tCtx K, compiledPattern *regexp.Regexp, fn ottl.Optional[ottl.FunctionGetter[K]], originalValStr string, replacementVal string) (string, error) { +func applyOptReplaceFunction[K any](ctx context.Context, tCtx K, compiledPattern *regexp.Regexp, fn ottl.Optional[ottl.FunctionGetter[K]], originalValStr string, replacementVal string) (string, error) { var updatedString string updatedString = originalValStr submatches := compiledPattern.FindAllStringSubmatchIndex(updatedString, -1) @@ -68,14 +62,7 @@ func ApplyReplaceFunction[K any](ctx context.Context, tCtx K, compiledPattern *r if !ok { return "", fmt.Errorf("the replacement value must be a string") } - // if there are submatches then use the resolved capture group value - if len(submatch) > 2 { - updatedString = strings.ReplaceAll(updatedString, fullMatch, replacementValStr) - } - // if there are no submatches and the replacement string isn't a capture group then use the replacement value as is - if !IsGroupNumString(replacementVal) { - updatedString = strings.ReplaceAll(updatedString, fullMatch, replacementVal) - } + updatedString = strings.ReplaceAll(updatedString, fullMatch, replacementValStr) } return updatedString, nil } @@ -102,7 +89,7 @@ func replacePattern[K any](target ottl.GetSetter[K], regexPattern string, replac if compiledPattern.MatchString(originalValStr) { if !fn.IsEmpty() { var updatedString string - updatedString, err = ApplyReplaceFunction[K](ctx, tCtx, compiledPattern, fn, originalValStr, replacementVal) + updatedString, err = applyOptReplaceFunction[K](ctx, tCtx, compiledPattern, fn, originalValStr, replacementVal) if err != nil { return nil, err } @@ -117,12 +104,6 @@ func replacePattern[K any](target ottl.GetSetter[K], regexPattern string, replac return nil, err } } - } else { - updatedStr := compiledPattern.ReplaceAllString(originalValStr, replacementVal) - err = target.Set(ctx, tCtx, updatedStr) - if err != nil { - return nil, err - } } } return nil, nil diff --git a/pkg/ottl/ottlfuncs/func_replace_pattern_test.go b/pkg/ottl/ottlfuncs/func_replace_pattern_test.go index f92f881c7523..e14306b57890 100644 --- a/pkg/ottl/ottlfuncs/func_replace_pattern_test.go +++ b/pkg/ottl/ottlfuncs/func_replace_pattern_test.go @@ -16,26 +16,25 @@ import ( "github.com/open-telemetry/opentelemetry-collector-contrib/pkg/ottl" ) -type TestArguments[K any] struct { +type optionalFnTestArgs[K any] struct { Target ottl.StringGetter[K] } -// For testing Hash values. -func NewTestFactory[K any]() ottl.Factory[K] { - return ottl.NewFactory("Test", &TestArguments[K]{}, createTestFunction[K]) +func optionalFnTestFactory[K any]() ottl.Factory[K] { + return ottl.NewFactory("Test", &optionalFnTestArgs[K]{}, createTestFunction[K]) } func createTestFunction[K any](_ ottl.FunctionContext, oArgs ottl.Arguments) (ottl.ExprFunc[K], error) { - args, ok := oArgs.(*TestArguments[K]) + args, ok := oArgs.(*optionalFnTestArgs[K]) if !ok { - return nil, fmt.Errorf("TestFactory args must be of type *TestArguments[K]") + return nil, fmt.Errorf("TestFactory args must be of type *optionalFnTestArgs[K]") } - return HashString(args.Target) + return hashString(args.Target), nil } -func HashString[K any](target ottl.StringGetter[K]) (ottl.ExprFunc[K], error) { +func hashString[K any](target ottl.StringGetter[K]) ottl.ExprFunc[K] { return func(ctx context.Context, tCtx K) (any, error) { val, err := target.Get(ctx, tCtx) @@ -43,7 +42,7 @@ func HashString[K any](target ottl.StringGetter[K]) (ottl.ExprFunc[K], error) { return nil, err } return fmt.Sprintf("hash(%s)", val), nil - }, nil + } } func Test_replacePattern(t *testing.T) { @@ -52,7 +51,7 @@ func Test_replacePattern(t *testing.T) { FCtx: ottl.FunctionContext{ Set: componenttest.NewNopTelemetrySettings(), }, - Fact: NewTestFactory[pcommon.Value](), + Fact: optionalFnTestFactory[pcommon.Value](), } optionalArg := ottl.NewTestingOptional[ottl.FunctionGetter[pcommon.Value]](ottlValue) target := &ottl.StandardGetSetter[pcommon.Value]{ @@ -88,18 +87,17 @@ func Test_replacePattern(t *testing.T) { }, }, { - name: "replace regex match (capture group without $1)", + name: "replace regex match (static text)", target: target, pattern: `passwd\=([^\s]*)`, replacement: ottl.StandardStringGetter[pcommon.Value]{ Getter: func(context.Context, pcommon.Value) (any, error) { - return "passwd=$1", nil + return "passwd", nil }, }, function: optionalArg, want: func(expectedValue pcommon.Value) { - // TODO: We should expect "application passwd=hash(sensitivedtata) otherarg=notsensitive key1 key2" - expectedValue.SetStr("application hash(passwd=$1) otherarg=notsensitive key1 key2") // since we only hash capture groups + expectedValue.SetStr("application hash(passwd) otherarg=notsensitive key1 key2") }, }, { @@ -113,7 +111,7 @@ func Test_replacePattern(t *testing.T) { }, function: optionalArg, want: func(expectedValue pcommon.Value) { - expectedValue.SetStr("application passwd=sensitivedtata otherarg=notsensitive key1 key2") + expectedValue.SetStr("application hash()otherarg=notsensitive key1 key2") }, }, {