Skip to content

Commit

Permalink
[pkg/ottl] Fix empty string matching (#37071)
Browse files Browse the repository at this point in the history
  • Loading branch information
TylerHelmuth authored Jan 8, 2025
1 parent 5c38c98 commit 570f619
Show file tree
Hide file tree
Showing 5 changed files with 99 additions and 2 deletions.
27 changes: 27 additions & 0 deletions .chloggen/ottl-empty-string-fix.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 bug with `replace_all_matches` and `replace_all_patterns` that caused non-string values to be changed to empty string when matching against empty string.

# Mandatory: One or more tracking issues related to the change. You can use the PR number here if no issue exists.
issues: [37071]

# (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: []
2 changes: 1 addition & 1 deletion pkg/ottl/ottlfuncs/func_replace_all_matches.go
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,7 @@ func replaceAllMatches[K any](target ottl.PMapGetter[K], pattern string, replace
}
}
val.Range(func(_ string, value pcommon.Value) bool {
if glob.Match(value.Str()) {
if value.Type() == pcommon.ValueTypeStr && glob.Match(value.Str()) {
value.SetStr(replacementVal)
}
return true
Expand Down
27 changes: 27 additions & 0 deletions pkg/ottl/ottlfuncs/func_replace_all_matches_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,8 @@ func Test_replaceAllMatches(t *testing.T) {
input.PutStr("test", "hello world")
input.PutStr("test2", "hello")
input.PutStr("test3", "goodbye")
input.PutStr("test4", "")
input.PutInt("test5", 123)

ottlValue := ottl.StandardFunctionGetter[pcommon.Map]{
FCtx: ottl.FunctionContext{
Expand Down Expand Up @@ -64,6 +66,8 @@ func Test_replaceAllMatches(t *testing.T) {
expectedMap.PutStr("test", "prefix=hash(hello {universe})")
expectedMap.PutStr("test2", "prefix=hash(hello {universe})")
expectedMap.PutStr("test3", "goodbye")
expectedMap.PutStr("test4", "")
expectedMap.PutInt("test5", 123)
},
},
{
Expand All @@ -81,6 +85,8 @@ func Test_replaceAllMatches(t *testing.T) {
expectedMap.PutStr("test", "hello {universe}")
expectedMap.PutStr("test2", "hello {universe}")
expectedMap.PutStr("test3", "goodbye")
expectedMap.PutStr("test4", "")
expectedMap.PutInt("test5", 123)
},
},
{
Expand All @@ -98,6 +104,27 @@ func Test_replaceAllMatches(t *testing.T) {
expectedMap.PutStr("test", "hello world")
expectedMap.PutStr("test2", "hello")
expectedMap.PutStr("test3", "goodbye")
expectedMap.PutStr("test4", "")
expectedMap.PutInt("test5", 123)
},
},
{
name: "replace empty string",
target: target,
pattern: "",
replacement: ottl.StandardStringGetter[pcommon.Map]{
Getter: func(context.Context, pcommon.Map) (any, error) {
return "empty_string_replacement", nil
},
},
function: ottl.Optional[ottl.FunctionGetter[pcommon.Map]]{},
replacementFormat: ottl.Optional[ottl.StringGetter[pcommon.Map]]{},
want: func(expectedMap pcommon.Map) {
expectedMap.PutStr("test", "hello world")
expectedMap.PutStr("test2", "hello")
expectedMap.PutStr("test3", "goodbye")
expectedMap.PutStr("test4", "empty_string_replacement")
expectedMap.PutInt("test5", 123)
},
},
}
Expand Down
2 changes: 1 addition & 1 deletion pkg/ottl/ottlfuncs/func_replace_all_patterns.go
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,7 @@ func replaceAllPatterns[K any](target ottl.PMapGetter[K], mode string, regexPatt
val.Range(func(key string, originalValue pcommon.Value) bool {
switch mode {
case modeValue:
if compiledPattern.MatchString(originalValue.Str()) {
if originalValue.Type() == pcommon.ValueTypeStr && compiledPattern.MatchString(originalValue.Str()) {
if !fn.IsEmpty() {
updatedString, err := applyOptReplaceFunction(ctx, tCtx, compiledPattern, fn, originalValue.Str(), replacementVal, replacementFormat)
if err != nil {
Expand Down
43 changes: 43 additions & 0 deletions pkg/ottl/ottlfuncs/func_replace_all_patterns_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ func Test_replaceAllPatterns(t *testing.T) {
input.PutInt("test4", 1234)
input.PutDouble("test5", 1234)
input.PutBool("test6", true)
input.PutStr("test7", "")

ottlValue := ottl.StandardFunctionGetter[pcommon.Map]{
FCtx: ottl.FunctionContext{
Expand Down Expand Up @@ -77,6 +78,7 @@ func Test_replaceAllPatterns(t *testing.T) {
expectedMap.PutInt("test4", 1234)
expectedMap.PutDouble("test5", 1234)
expectedMap.PutBool("test6", true)
expectedMap.PutStr("test7", "")
},
},
{
Expand All @@ -97,6 +99,7 @@ func Test_replaceAllPatterns(t *testing.T) {
expectedMap.PutInt("test4", 1234)
expectedMap.PutDouble("test5", 1234)
expectedMap.PutBool("test6", true)
expectedMap.PutStr("test7", "")
},
},
{
Expand All @@ -117,6 +120,7 @@ func Test_replaceAllPatterns(t *testing.T) {
expectedMap.PutInt("test4", 1234)
expectedMap.PutDouble("test5", 1234)
expectedMap.PutBool("test6", true)
expectedMap.PutStr("test7", "")
},
},
{
Expand All @@ -137,6 +141,7 @@ func Test_replaceAllPatterns(t *testing.T) {
expectedMap.PutInt("test4", 1234)
expectedMap.PutDouble("test5", 1234)
expectedMap.PutBool("test6", true)
expectedMap.PutStr("test7", "")
},
},
{
Expand All @@ -158,6 +163,7 @@ func Test_replaceAllPatterns(t *testing.T) {
expectedMap.PutInt("test4", 1234)
expectedMap.PutDouble("test5", 1234)
expectedMap.PutBool("test6", true)
expectedMap.PutStr("test7", "")
},
},
{
Expand Down Expand Up @@ -196,6 +202,7 @@ func Test_replaceAllPatterns(t *testing.T) {
expectedMap.PutInt("test4", 1234)
expectedMap.PutDouble("test5", 1234)
expectedMap.PutBool("test6", true)
expectedMap.PutStr("test7", "")
},
},
{
Expand All @@ -217,6 +224,7 @@ func Test_replaceAllPatterns(t *testing.T) {
expectedMap.PutInt("test4", 1234)
expectedMap.PutDouble("test5", 1234)
expectedMap.PutBool("test6", true)
expectedMap.PutStr("test7", "")
},
},
{
Expand All @@ -238,6 +246,7 @@ func Test_replaceAllPatterns(t *testing.T) {
expectedMap.PutInt("test4", 1234)
expectedMap.PutDouble("test5", 1234)
expectedMap.PutBool("test6", true)
expectedMap.PutStr("test7", "")
},
},
{
Expand All @@ -258,6 +267,7 @@ func Test_replaceAllPatterns(t *testing.T) {
expectedMap.PutInt("test4", 1234)
expectedMap.PutDouble("test5", 1234)
expectedMap.PutBool("test6", true)
expectedMap.PutStr("test7", "")
},
},
{
Expand All @@ -278,6 +288,7 @@ func Test_replaceAllPatterns(t *testing.T) {
expectedMap.PutInt("test4", 1234)
expectedMap.PutDouble("test5", 1234)
expectedMap.PutBool("test6", true)
expectedMap.PutStr("test7", "")
},
},
{
Expand All @@ -298,6 +309,7 @@ func Test_replaceAllPatterns(t *testing.T) {
expectedMap.PutInt("test4", 1234)
expectedMap.PutDouble("test5", 1234)
expectedMap.PutBool("test6", true)
expectedMap.PutStr("test7", "")
},
},
{
Expand All @@ -318,6 +330,7 @@ func Test_replaceAllPatterns(t *testing.T) {
expectedMap.PutInt("test4", 1234)
expectedMap.PutDouble("test5", 1234)
expectedMap.PutBool("test6", true)
expectedMap.PutStr("test7", "")
},
},
{
Expand All @@ -338,6 +351,7 @@ func Test_replaceAllPatterns(t *testing.T) {
expectedMap.PutInt("test4", 1234)
expectedMap.PutDouble("test5", 1234)
expectedMap.PutBool("test6", true)
expectedMap.PutStr("test7", "")
},
},
{
Expand All @@ -360,6 +374,7 @@ func Test_replaceAllPatterns(t *testing.T) {
expectedMap.PutInt("test4", 1234)
expectedMap.PutDouble("test5", 1234)
expectedMap.PutBool("test6", true)
expectedMap.PutStr("test7", "")
},
},
{
Expand All @@ -382,6 +397,7 @@ func Test_replaceAllPatterns(t *testing.T) {
expectedMap.PutInt("test4", 1234)
expectedMap.PutDouble("test5", 1234)
expectedMap.PutBool("test6", true)
expectedMap.PutStr("test7", "")
},
},
{
Expand All @@ -404,6 +420,7 @@ func Test_replaceAllPatterns(t *testing.T) {
expectedMap.PutInt("test.4", 1234)
expectedMap.PutDouble("test.5", 1234)
expectedMap.PutBool("test.6", true)
expectedMap.PutStr("test.7", "")
},
},
{
Expand All @@ -426,6 +443,7 @@ func Test_replaceAllPatterns(t *testing.T) {
expectedMap.PutInt("test4", 1234)
expectedMap.PutDouble("test5", 1234)
expectedMap.PutBool("test6", true)
expectedMap.PutStr("test7", "")
},
},
{
Expand All @@ -447,6 +465,7 @@ func Test_replaceAllPatterns(t *testing.T) {
expectedMap.PutInt("test-4", 1234)
expectedMap.PutDouble("test-5", 1234)
expectedMap.PutBool("test-6", true)
expectedMap.PutStr("test-7", "")
},
},
{
Expand All @@ -469,6 +488,30 @@ func Test_replaceAllPatterns(t *testing.T) {
expectedMap.PutInt("test4", 1234)
expectedMap.PutDouble("test5", 1234)
expectedMap.PutBool("test6", true)
expectedMap.PutStr("test7", "")
},
},
{
name: "replacement for empty string",
target: target,
mode: modeValue,
pattern: `^$`,
replacement: ottl.StandardStringGetter[pcommon.Map]{
Getter: func(context.Context, pcommon.Map) (any, error) {
return "empty_string_replacement", nil
},
},
replacementFormat: ottl.Optional[ottl.StringGetter[pcommon.Map]]{},
function: ottl.Optional[ottl.FunctionGetter[pcommon.Map]]{},
want: func(expectedMap pcommon.Map) {
expectedMap.Clear()
expectedMap.PutStr("test", "hello world")
expectedMap.PutStr("test2", "hello")
expectedMap.PutStr("test3", "goodbye world1 and world2")
expectedMap.PutInt("test4", 1234)
expectedMap.PutDouble("test5", 1234)
expectedMap.PutBool("test6", true)
expectedMap.PutStr("test7", "empty_string_replacement")
},
},
}
Expand Down

0 comments on commit 570f619

Please sign in to comment.