Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

(pkg/ottl) Fix issue with the hash value of a match subgroup in the replace_pattern editors #29408

Merged
merged 11 commits into from
Jan 8, 2024
Merged
27 changes: 27 additions & 0 deletions .chloggen/fix-hash-value.yaml
Original file line number Diff line number Diff line change
@@ -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: []
6 changes: 3 additions & 3 deletions pkg/ottl/ottlfuncs/func_replace_all_matches_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)

Expand Down Expand Up @@ -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")
},
},
Expand Down
51 changes: 23 additions & 28 deletions pkg/ottl/ottlfuncs/func_replace_all_patterns.go
Original file line number Diff line number Diff line change
Expand Up @@ -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])
}
Expand Down Expand Up @@ -59,42 +55,41 @@ 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())
val.Range(func(key string, originalValue pcommon.Value) bool {
switch mode {
case modeValue:
if compiledPattern.MatchString(originalValue.Str()) {
updatedString := compiledPattern.ReplaceAllString(originalValue.Str(), replacementVal)
updated.PutStr(key, updatedString)
if !fn.IsEmpty() {
rnishtala-sumo marked this conversation as resolved.
Show resolved Hide resolved
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))
}
Expand Down
166 changes: 163 additions & 3 deletions pkg/ottl/ottlfuncs/func_replace_all_patterns_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)

Expand Down Expand Up @@ -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")
rnishtala-sumo marked this conversation as resolved.
Show resolved Hide resolved
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)",
evan-bradley marked this conversation as resolved.
Show resolved Hide resolved
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")
evan-bradley marked this conversation as resolved.
Show resolved Hide resolved
expectedMap.PutStr("test2", "")
expectedMap.PutStr("test3", "goodbye world1 and world2")
expectedMap.PutInt("test4", 1234)
expectedMap.PutDouble("test5", 1234)
Expand Down Expand Up @@ -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,
Expand Down
4 changes: 2 additions & 2 deletions pkg/ottl/ottlfuncs/func_replace_match_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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]{
Expand Down Expand Up @@ -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})")
},
},
{
Expand Down
Loading
Loading