Adding tests that show the change in the expected result without the …

…optional hash function argument
open-telemetry · Oct 2, 2023 · 5189734 · 5189734
1 parent d2fa6bf
commit 5189734
Show file tree

Hide file tree

Showing 10 changed files with 162 additions and 155 deletions.
diff --git a/.chloggen/ottl-replace-pattern.yaml b/.chloggen/ottl-replace-pattern.yaml
@@ -16,8 +16,11 @@ issues: [27235]
 # These lines will be padded with 2 spaces and then inserted directly into the document.
 # Use pipe (|) for multiline entries.
 subtext: |
-  The `ottl.Optional` type can now be used in a converter's `Arguments` struct
-  to indicate that a parameter is optional. Hashing functions are passed as optional parameters to converters.
+  Hashing functions are passed as optional arguments to the following converters:
+  - `replace_pattern`
+  - `replace_all_patterns`
+  - `replace_match`
+  - `replace_all_matches`
 
 # 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.

diff --git a/pkg/ottl/functions.go b/pkg/ottl/functions.go
@@ -381,25 +381,25 @@ type optionalManager interface {
 }
 
 type Optional[T any] struct {
-	Val      T
-	HasValue bool
+	val      T
+	hasValue bool
 }
 
 // This is called only by reflection.
 // nolint:unused
 func (o Optional[T]) set(val any) reflect.Value {
 	return reflect.ValueOf(Optional[T]{
-		Val:      val.(T),
-		HasValue: true,
+		val:      val.(T),
+		hasValue: true,
 	})
 }
 
 func (o Optional[T]) IsEmpty() bool {
-	return !o.HasValue
+	return !o.hasValue
 }
 
 func (o Optional[T]) Get() T {
-	return o.Val
+	return o.val
 }
 
 func (o Optional[T]) get() reflect.Value {
@@ -414,7 +414,7 @@ func (o Optional[T]) get() reflect.Value {
 // OTTL functions.
 func NewTestingOptional[T any](val T) Optional[T] {
 	return Optional[T]{
-		Val:      val,
-		HasValue: true,
+		val:      val,
+		hasValue: true,
 	}
 }
diff --git a/pkg/ottl/ottlfuncs/func_replace_all_matches.go b/pkg/ottl/ottlfuncs/func_replace_all_matches.go
@@ -37,8 +37,6 @@ func createReplaceAllMatchesFunction[K any](_ ottl.FunctionContext, oArgs ottl.A
 func replaceAllMatches[K any](target ottl.PMapGetter[K], pattern string, replacement ottl.StringGetter[K], fn ottl.Optional[ottl.FunctionGetter[K]]) (ottl.ExprFunc[K], error) {
 	glob, err := glob.Compile(pattern)
 	var replacementVal string
-	var replacementExpr ottl.Expr[K]
-	var replacementValRaw interface{}
 	if err != nil {
 		return nil, fmt.Errorf("the pattern supplied to replace_match is not a valid pattern: %w", err)
 	}
@@ -54,13 +52,13 @@ func replaceAllMatches[K any](target ottl.PMapGetter[K], pattern string, replace
 			}
 		} else {
 			fnVal := fn.Get()
-			replacementExpr, err = fnVal.Get(&FuncArgs[K]{Input: replacement})
-			if err != nil {
-				return nil, err
+			replacementExpr, errNew := fnVal.Get(&FuncArgs[K]{Input: replacement})
+			if errNew != nil {
+				return nil, errNew
 			}
-			replacementValRaw, err = replacementExpr.Eval(ctx, tCtx)
-			if err != nil {
-				return nil, err
+			replacementValRaw, errNew := replacementExpr.Eval(ctx, tCtx)
+			if errNew != nil {
+				return nil, errNew
 			}
 			replacementVal = replacementValRaw.(string)
 		}

diff --git a/pkg/ottl/ottlfuncs/func_replace_all_matches_test.go b/pkg/ottl/ottlfuncs/func_replace_all_matches_test.go
@@ -20,6 +20,14 @@ func Test_replaceAllMatches(t *testing.T) {
 	input.PutStr("test2", "hello")
 	input.PutStr("test3", "goodbye")
 
+	ottlValue := ottl.StandardFunctionGetter[pcommon.Map]{
+		FCtx: ottl.FunctionContext{
+			Set: componenttest.NewNopTelemetrySettings(),
+		},
+		Fact: StandardConverters[pcommon.Map]()["SHA256"],
+	}
+	optionalArg := ottl.NewTestingOptional[ottl.FunctionGetter[pcommon.Map]](ottlValue)
+
 	target := &ottl.StandardPMapGetter[pcommon.Map]{
 		Getter: func(ctx context.Context, tCtx pcommon.Map) (interface{}, error) {
 			return tCtx, nil
@@ -35,29 +43,37 @@ func Test_replaceAllMatches(t *testing.T) {
 		want        func(pcommon.Map)
 	}{
 		{
-			name:    "replace only matches",
+			name:    "replace only matches (with hash function)",
 			target:  target,
 			pattern: "hello*",
 			replacement: ottl.StandardStringGetter[pcommon.Map]{
 				Getter: func(context.Context, pcommon.Map) (interface{}, error) {
 					return "hello {universe}", nil
 				},
 			},
-			function: ottl.Optional[ottl.FunctionGetter[pcommon.Map]]{
-				Val: ottl.StandardFunctionGetter[pcommon.Map]{
-					FCtx: ottl.FunctionContext{
-						Set: componenttest.NewNopTelemetrySettings(),
-					},
-					Fact: StandardConverters[pcommon.Map]()["SHA256"],
-				},
-				HasValue: true,
-			},
+			function: optionalArg,
 			want: func(expectedMap pcommon.Map) {
 				expectedMap.PutStr("test", "4804d6b7f03268e33f78c484977f3d81771220df07cc6aac4ad4868102141fad")
 				expectedMap.PutStr("test2", "4804d6b7f03268e33f78c484977f3d81771220df07cc6aac4ad4868102141fad")
 				expectedMap.PutStr("test3", "goodbye")
 			},
 		},
+		{
+			name:    "replace only matches",
+			target:  target,
+			pattern: "hello*",
+			replacement: ottl.StandardStringGetter[pcommon.Map]{
+				Getter: func(context.Context, pcommon.Map) (interface{}, error) {
+					return "hello {universe}", nil
+				},
+			},
+			function: ottl.Optional[ottl.FunctionGetter[pcommon.Map]]{},
+			want: func(expectedMap pcommon.Map) {
+				expectedMap.PutStr("test", "hello {universe}")
+				expectedMap.PutStr("test2", "hello {universe}")
+				expectedMap.PutStr("test3", "goodbye")
+			},
+		},
 		{
 			name:    "no matches",
 			target:  target,
@@ -67,9 +83,7 @@ func Test_replaceAllMatches(t *testing.T) {
 					return "nothing {matches}", nil
 				},
 			},
-			function: ottl.Optional[ottl.FunctionGetter[pcommon.Map]]{
-				HasValue: false,
-			},
+			function: ottl.Optional[ottl.FunctionGetter[pcommon.Map]]{},
 			want: func(expectedMap pcommon.Map) {
 				expectedMap.PutStr("test", "hello world")
 				expectedMap.PutStr("test2", "hello")
@@ -109,9 +123,7 @@ func Test_replaceAllMatches_bad_input(t *testing.T) {
 			return "{replacement}", nil
 		},
 	}
-	function := ottl.Optional[ottl.FunctionGetter[interface{}]]{
-		HasValue: false,
-	}
+	function := ottl.Optional[ottl.FunctionGetter[interface{}]]{}
 
 	exprFunc, err := replaceAllMatches[interface{}](target, "*", replacement, function)
 	assert.NoError(t, err)
@@ -130,9 +142,7 @@ func Test_replaceAllMatches_get_nil(t *testing.T) {
 			return "{anything}", nil
 		},
 	}
-	function := ottl.Optional[ottl.FunctionGetter[interface{}]]{
-		HasValue: false,
-	}
+	function := ottl.Optional[ottl.FunctionGetter[interface{}]]{}
 
 	exprFunc, err := replaceAllMatches[interface{}](target, "*", replacement, function)
 	assert.NoError(t, err)

diff --git a/pkg/ottl/ottlfuncs/func_replace_all_patterns.go b/pkg/ottl/ottlfuncs/func_replace_all_patterns.go
@@ -43,8 +43,6 @@ func createReplaceAllPatternsFunction[K any](_ ottl.FunctionContext, oArgs ottl.
 func replaceAllPatterns[K any](target ottl.PMapGetter[K], mode string, regexPattern string, replacement ottl.StringGetter[K], fn ottl.Optional[ottl.FunctionGetter[K]]) (ottl.ExprFunc[K], error) {
 	compiledPattern, err := regexp.Compile(regexPattern)
 	var replacementVal string
-	var replacementExpr ottl.Expr[K]
-	var replacementValRaw interface{}
 	if err != nil {
 		return nil, fmt.Errorf("the regex pattern supplied to replace_all_patterns is not a valid pattern: %w", err)
 	}
@@ -64,13 +62,13 @@ func replaceAllPatterns[K any](target ottl.PMapGetter[K], mode string, regexPatt
 			}
 		} else {
 			fnVal := fn.Get()
-			replacementExpr, err = fnVal.Get(&FuncArgs[K]{Input: replacement})
-			if err != nil {
-				return nil, err
+			replacementExpr, errNew := fnVal.Get(&FuncArgs[K]{Input: replacement})
+			if errNew != nil {
+				return nil, errNew
 			}
-			replacementValRaw, err = replacementExpr.Eval(ctx, tCtx)
-			if err != nil {
-				return nil, err
+			replacementValRaw, errNew := replacementExpr.Eval(ctx, tCtx)
+			if errNew != nil {
+				return nil, errNew
 			}
 			replacementVal = replacementValRaw.(string)
 		}