From 0613fb6e61a6d4eeed402212cd7a7293b1bb44a7 Mon Sep 17 00:00:00 2001 From: Raj Nishtala <113392743+rnishtala-sumo@users.noreply.github.com> Date: Mon, 8 Jan 2024 17:10:12 -0500 Subject: [PATCH] (pkg/ottl) Fix issue with the hash value of a match subgroup in the replace_pattern editors (#29408) **Description:** Fix issue with the hash value of a match group in replace_pattern* **Link to tracking Issue:** https://github.com/open-telemetry/opentelemetry-collector-contrib/issues/29409 --------- Co-authored-by: Tyler Helmuth <12352919+TylerHelmuth@users.noreply.github.com> --- .chloggen/fix-hash-value.yaml | 27 +++ .../func_replace_all_matches_test.go | 6 +- .../ottlfuncs/func_replace_all_patterns.go | 51 +++--- .../func_replace_all_patterns_test.go | 166 +++++++++++++++++- pkg/ottl/ottlfuncs/func_replace_match_test.go | 4 +- pkg/ottl/ottlfuncs/func_replace_pattern.go | 76 +++++--- .../ottlfuncs/func_replace_pattern_test.go | 82 ++++++++- 7 files changed, 346 insertions(+), 66 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 0000000000000..3d6c5ba6fb525 --- /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_matches_test.go b/pkg/ottl/ottlfuncs/func_replace_all_matches_test.go index 88d1d7fe019eb..3494aa0d48665 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: optionalFnTestFactory[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.go b/pkg/ottl/ottlfuncs/func_replace_all_patterns.go index 2c7ee3ef33869..461741b90b118 100644 --- a/pkg/ottl/ottlfuncs/func_replace_all_patterns.go +++ b/pkg/ottl/ottlfuncs/func_replace_all_patterns.go @@ -26,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]) } @@ -59,26 +55,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,15 +65,31 @@ func replaceAllPatterns[K any](target ottl.PMapGetter[K], mode string, regexPatt switch mode { case modeValue: if compiledPattern.MatchString(originalValue.Str()) { - updatedString := compiledPattern.ReplaceAllString(originalValue.Str(), replacementVal) - updated.PutStr(key, updatedString) + if !fn.IsEmpty() { + updatedString, err := applyOptReplaceFunction(ctx, tCtx, compiledPattern, fn, originalValue.Str(), replacementVal) + if err != nil { + return false + } + updated.PutStr(key, updatedString) + } else { + updatedString := compiledPattern.ReplaceAllString(originalValue.Str(), replacementVal) + updated.PutStr(key, updatedString) + } } else { originalValue.CopyTo(updated.PutEmpty(key)) } case modeKey: if compiledPattern.MatchString(key) { - updatedKey := compiledPattern.ReplaceAllString(key, replacementVal) - originalValue.CopyTo(updated.PutEmpty(updatedKey)) + if !fn.IsEmpty() { + updatedString, err := applyOptReplaceFunction(ctx, tCtx, compiledPattern, fn, key, replacementVal) + if err != nil { + return false + } + updated.PutStr(key, updatedString) + } else { + 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 c171b40e9ceae..b02e84297199d 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: optionalFnTestFactory[pcommon.Map](), } optionalArg := ottl.NewTestingOptional[ottl.FunctionGetter[pcommon.Map]](ottlValue) @@ -59,8 +59,68 @@ func Test_replaceAllPatterns(t *testing.T) { }, function: optionalArg, want: func(expectedMap pcommon.Map) { - expectedMap.PutStr("test", "4804d6b7f03268e33f78c484977f3d81771220df07cc6aac4ad4868102141fad world") - expectedMap.PutStr("test2", "4804d6b7f03268e33f78c484977f3d81771220df07cc6aac4ad4868102141fad") + 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) + 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", "hash(hello) world") + expectedMap.PutStr("test2", "hash(hello)") + 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", "hash() world") + expectedMap.PutStr("test2", "hash()") + 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) @@ -127,6 +187,106 @@ 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 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, + 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 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) + }, + }, + { + 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(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_match_test.go b/pkg/ottl/ottlfuncs/func_replace_match_test.go index 5de8512f1c1ef..a61e28560f90a 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: optionalFnTestFactory[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.go b/pkg/ottl/ottlfuncs/func_replace_pattern.go index aed32367ddc64..7b02b3896ea1b 100644 --- a/pkg/ottl/ottlfuncs/func_replace_pattern.go +++ b/pkg/ottl/ottlfuncs/func_replace_pattern.go @@ -7,6 +7,7 @@ import ( "context" "fmt" "regexp" + "strings" "github.com/open-telemetry/opentelemetry-collector-contrib/pkg/ottl" ) @@ -36,6 +37,36 @@ func createReplacePatternFunction[K any](_ ottl.FunctionContext, oArgs ottl.Argu return replacePattern(args.Target, args.RegexPattern, args.Replacement, args.Function) } +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) + 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") + } + updatedString = strings.ReplaceAll(updatedString, fullMatch, replacementValStr) + } + 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 { @@ -47,36 +78,31 @@ 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 - } if originalVal == nil { return nil, nil } + replacementVal, err = replacement.Get(ctx, tCtx) + if err != nil { + return nil, err + } if originalValStr, ok := originalVal.(string); ok { if compiledPattern.MatchString(originalValStr) { - updatedStr := compiledPattern.ReplaceAllString(originalValStr, replacementVal) - err = target.Set(ctx, tCtx, updatedStr) - if err != nil { - return nil, err + if !fn.IsEmpty() { + var updatedString string + updatedString, err = applyOptReplaceFunction[K](ctx, tCtx, compiledPattern, fn, originalValStr, replacementVal) + if err != nil { + return nil, err + } + err = target.Set(ctx, tCtx, updatedString) + if err != nil { + return nil, err + } + } else { + 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 66edf7d251774..e14306b578903 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,42 @@ import ( "github.com/open-telemetry/opentelemetry-collector-contrib/pkg/ottl" ) +type optionalFnTestArgs[K any] struct { + Target ottl.StringGetter[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.(*optionalFnTestArgs[K]) + + if !ok { + return nil, fmt.Errorf("TestFactory args must be of type *optionalFnTestArgs[K]") + } + + return hashString(args.Target), nil +} + +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) + if err != nil { + return nil, err + } + return fmt.Sprintf("hash(%s)", val), 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: optionalFnTestFactory[pcommon.Value](), } optionalArg := ottl.NewTestingOptional[ottl.FunctionGetter[pcommon.Value]](ottlValue) target := &ottl.StandardGetSetter[pcommon.Value]{ @@ -45,15 +75,57 @@ 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 hash(sensitivedtata)otherarg=notsensitive key1 key2") + }, + }, + { + 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", nil + }, + }, + function: optionalArg, + want: func(expectedValue pcommon.Value) { + expectedValue.SetStr("application hash(passwd) otherarg=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 0f2407f2d83337b1f757eb1754a7643ce0e8fba620bc605c54566cd6dfd838beotherarg=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?`, + 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") }, }, { @@ -228,7 +300,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) }