diff --git a/.chloggen/ottl-use-mapgetter.yaml b/.chloggen/ottl-use-mapgetter.yaml new file mode 100755 index 000000000000..9c8110fdf204 --- /dev/null +++ b/.chloggen/ottl-use-mapgetter.yaml @@ -0,0 +1,16 @@ +# One of 'breaking', 'deprecation', 'new_component', 'enhancement', 'bug_fix' +change_type: breaking + +# 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: Add typed getter for `pcommon.map` and update related functions to use it. + +# One or more tracking issues related to the change +issues: [19781] + +# (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: Using the impacted functions, such as `keep_keys` or `replace_all_patterns`, without a `pcommon.map` will now result in an error. Only pass these function paths that result in a `pcommon.map` or set `ErrorMode` to `IgnoreError`. diff --git a/pkg/ottl/expression.go b/pkg/ottl/expression.go index a65c013cbda6..0f1c0d50e2f1 100644 --- a/pkg/ottl/expression.go +++ b/pkg/ottl/expression.go @@ -17,6 +17,8 @@ package ottl // import "github.com/open-telemetry/opentelemetry-collector-contri import ( "context" "fmt" + + "go.opentelemetry.io/collector/pdata/pcommon" ) type ExprFunc[K any] func(ctx context.Context, tCtx K) (interface{}, error) @@ -97,6 +99,10 @@ type IntGetter[K any] interface { Get(ctx context.Context, tCtx K) (int64, error) } +type PMapGetter[K any] interface { + Get(ctx context.Context, tCtx K) (pcommon.Map, error) +} + type StandardTypeGetter[K any, T any] struct { Getter func(ctx context.Context, tCtx K) (interface{}, error) } diff --git a/pkg/ottl/functions.go b/pkg/ottl/functions.go index ed40c9bd794f..5f79b76a6dc6 100644 --- a/pkg/ottl/functions.go +++ b/pkg/ottl/functions.go @@ -19,6 +19,8 @@ import ( "fmt" "reflect" "strings" + + "go.opentelemetry.io/collector/pdata/pcommon" ) type PathExpressionParser[K any] func(*Path) (GetSetter[K], error) @@ -125,6 +127,12 @@ func (p *Parser[K]) buildSliceArg(argVal value, argType reflect.Type) (any, erro return nil, err } return arg, nil + case strings.HasPrefix(name, "PMapGetter"): + arg, err := buildSlice[PMapGetter[K]](argVal, argType, p.buildArg, name) + if err != nil { + return nil, err + } + return arg, nil case strings.HasPrefix(name, "StringGetter"): arg, err := buildSlice[StringGetter[K]](argVal, argType, p.buildArg, name) if err != nil { @@ -169,6 +177,12 @@ func (p *Parser[K]) buildArg(argVal value, argType reflect.Type) (any, error) { return nil, err } return StandardTypeGetter[K, int64]{Getter: arg.Get}, nil + case strings.HasPrefix(name, "PMapGetter"): + arg, err := p.newGetter(argVal) + if err != nil { + return nil, err + } + return StandardTypeGetter[K, pcommon.Map]{Getter: arg.Get}, nil case name == "Enum": arg, err := p.enumParser(argVal.Enum) if err != nil { diff --git a/pkg/ottl/functions_test.go b/pkg/ottl/functions_test.go index 573150d9547c..090e57e5e97d 100644 --- a/pkg/ottl/functions_test.go +++ b/pkg/ottl/functions_test.go @@ -508,6 +508,43 @@ func Test_NewFunctionCall(t *testing.T) { }, want: 2, }, + { + name: "pmapgetter slice arg", + inv: invocation{ + Function: "testing_pmapgetter_slice", + Arguments: []value{ + { + List: &list{ + Values: []value{ + { + Literal: &mathExprLiteral{ + Path: &Path{ + Fields: []Field{ + { + Name: "name", + }, + }, + }, + }, + }, + { + Literal: &mathExprLiteral{ + Path: &Path{ + Fields: []Field{ + { + Name: "name", + }, + }, + }, + }, + }, + }, + }, + }, + }, + }, + want: 2, + }, { name: "setter arg", inv: invocation{ @@ -671,6 +708,26 @@ func Test_NewFunctionCall(t *testing.T) { }, want: nil, }, + { + name: "pmapgetter arg", + inv: invocation{ + Function: "testing_pmapgetter", + Arguments: []value{ + { + Literal: &mathExprLiteral{ + Path: &Path{ + Fields: []Field{ + { + Name: "name", + }, + }, + }, + }, + }, + }, + }, + want: nil, + }, { name: "string arg", inv: invocation{ @@ -908,6 +965,12 @@ func functionWithStringGetterSlice(getters []Getter[interface{}]) (ExprFunc[inte }, nil } +func functionWithPMapGetterSlice(getters []PMapGetter[interface{}]) (ExprFunc[interface{}], error) { + return func(context.Context, interface{}) (interface{}, error) { + return len(getters), nil + }, nil +} + func functionWithSetter(Setter[interface{}]) (ExprFunc[interface{}], error) { return func(context.Context, interface{}) (interface{}, error) { return "anything", nil @@ -938,6 +1001,12 @@ func functionWithIntGetter(IntGetter[interface{}]) (ExprFunc[interface{}], error }, nil } +func functionWithPMapGetter(PMapGetter[interface{}]) (ExprFunc[interface{}], error) { + return func(context.Context, interface{}) (interface{}, error) { + return "anything", nil + }, nil +} + func functionWithString(string) (ExprFunc[interface{}], error) { return func(context.Context, interface{}) (interface{}, error) { return "anything", nil @@ -1007,11 +1076,13 @@ func defaultFunctionsForTests() map[string]interface{} { functions["testing_byte_slice"] = functionWithByteSlice functions["testing_getter_slice"] = functionWithGetterSlice functions["testing_stringgetter_slice"] = functionWithStringGetterSlice + functions["testing_pmapgetter_slice"] = functionWithPMapGetterSlice functions["testing_setter"] = functionWithSetter functions["testing_getsetter"] = functionWithGetSetter functions["testing_getter"] = functionWithGetter functions["testing_stringgetter"] = functionWithStringGetter functions["testing_intgetter"] = functionWithIntGetter + functions["testing_pmapgetter"] = functionWithPMapGetter functions["testing_string"] = functionWithString functions["testing_float"] = functionWithFloat functions["testing_int"] = functionWithInt diff --git a/pkg/ottl/ottlfuncs/func_delete_key.go b/pkg/ottl/ottlfuncs/func_delete_key.go index 8013c9a910e4..1ed51675b002 100644 --- a/pkg/ottl/ottlfuncs/func_delete_key.go +++ b/pkg/ottl/ottlfuncs/func_delete_key.go @@ -17,24 +17,16 @@ package ottlfuncs // import "github.com/open-telemetry/opentelemetry-collector-c import ( "context" - "go.opentelemetry.io/collector/pdata/pcommon" - "github.com/open-telemetry/opentelemetry-collector-contrib/pkg/ottl" ) -func DeleteKey[K any](target ottl.Getter[K], key string) (ottl.ExprFunc[K], error) { +func DeleteKey[K any](target ottl.PMapGetter[K], key string) (ottl.ExprFunc[K], error) { return func(ctx context.Context, tCtx K) (interface{}, error) { val, err := target.Get(ctx, tCtx) if err != nil { return nil, err } - if val == nil { - return nil, nil - } - - if attrs, ok := val.(pcommon.Map); ok { - attrs.Remove(key) - } + val.Remove(key) return nil, nil }, nil } diff --git a/pkg/ottl/ottlfuncs/func_delete_key_test.go b/pkg/ottl/ottlfuncs/func_delete_key_test.go index 535ab0490a79..ad20e9a1994f 100644 --- a/pkg/ottl/ottlfuncs/func_delete_key_test.go +++ b/pkg/ottl/ottlfuncs/func_delete_key_test.go @@ -30,7 +30,7 @@ func Test_deleteKey(t *testing.T) { input.PutInt("test2", 3) input.PutBool("test3", true) - target := &ottl.StandardGetSetter[pcommon.Map]{ + target := &ottl.StandardTypeGetter[pcommon.Map, pcommon.Map]{ Getter: func(ctx context.Context, tCtx pcommon.Map) (interface{}, error) { return tCtx, nil }, @@ -38,7 +38,7 @@ func Test_deleteKey(t *testing.T) { tests := []struct { name string - target ottl.Getter[pcommon.Map] + target ottl.PMapGetter[pcommon.Map] key string want func(pcommon.Map) }{ @@ -92,42 +92,31 @@ func Test_deleteKey(t *testing.T) { func Test_deleteKey_bad_input(t *testing.T) { input := pcommon.NewValueStr("not a map") - target := &ottl.StandardGetSetter[interface{}]{ + target := &ottl.StandardTypeGetter[interface{}, pcommon.Map]{ Getter: func(ctx context.Context, tCtx interface{}) (interface{}, error) { return tCtx, nil }, - Setter: func(ctx context.Context, tCtx interface{}, val interface{}) error { - t.Errorf("nothing should be set in this scenario") - return nil - }, } key := "anything" exprFunc, err := DeleteKey[interface{}](target, key) assert.NoError(t, err) - result, err := exprFunc(nil, input) - assert.NoError(t, err) - assert.Nil(t, result) - assert.Equal(t, pcommon.NewValueStr("not a map"), input) + _, err = exprFunc(nil, input) + assert.Error(t, err) } func Test_deleteKey_get_nil(t *testing.T) { - target := &ottl.StandardGetSetter[interface{}]{ + target := &ottl.StandardTypeGetter[interface{}, pcommon.Map]{ Getter: func(ctx context.Context, tCtx interface{}) (interface{}, error) { return tCtx, nil }, - Setter: func(ctx context.Context, tCtx interface{}, val interface{}) error { - t.Errorf("nothing should be set in this scenario") - return nil - }, } key := "anything" exprFunc, err := DeleteKey[interface{}](target, key) assert.NoError(t, err) - result, err := exprFunc(nil, nil) - assert.NoError(t, err) - assert.Nil(t, result) + _, err = exprFunc(nil, nil) + assert.Error(t, err) } diff --git a/pkg/ottl/ottlfuncs/func_delete_matching_keys.go b/pkg/ottl/ottlfuncs/func_delete_matching_keys.go index 0f0b84b2cde1..a6f16f178817 100644 --- a/pkg/ottl/ottlfuncs/func_delete_matching_keys.go +++ b/pkg/ottl/ottlfuncs/func_delete_matching_keys.go @@ -24,7 +24,7 @@ import ( "github.com/open-telemetry/opentelemetry-collector-contrib/pkg/ottl" ) -func DeleteMatchingKeys[K any](target ottl.Getter[K], pattern string) (ottl.ExprFunc[K], error) { +func DeleteMatchingKeys[K any](target ottl.PMapGetter[K], pattern string) (ottl.ExprFunc[K], error) { compiledPattern, err := regexp.Compile(pattern) if err != nil { return nil, fmt.Errorf("the regex pattern supplied to delete_matching_keys is not a valid pattern: %w", err) @@ -34,15 +34,9 @@ func DeleteMatchingKeys[K any](target ottl.Getter[K], pattern string) (ottl.Expr if err != nil { return nil, err } - if val == nil { - return nil, nil - } - - if attrs, ok := val.(pcommon.Map); ok { - attrs.RemoveIf(func(key string, _ pcommon.Value) bool { - return compiledPattern.MatchString(key) - }) - } + val.RemoveIf(func(key string, _ pcommon.Value) bool { + return compiledPattern.MatchString(key) + }) return nil, nil }, nil } diff --git a/pkg/ottl/ottlfuncs/func_delete_matching_keys_test.go b/pkg/ottl/ottlfuncs/func_delete_matching_keys_test.go index 2ac251f2a8d2..45041e5e5084 100644 --- a/pkg/ottl/ottlfuncs/func_delete_matching_keys_test.go +++ b/pkg/ottl/ottlfuncs/func_delete_matching_keys_test.go @@ -31,7 +31,7 @@ func Test_deleteMatchingKeys(t *testing.T) { input.PutInt("test2", 3) input.PutBool("test3", true) - target := &ottl.StandardGetSetter[pcommon.Map]{ + target := &ottl.StandardTypeGetter[pcommon.Map, pcommon.Map]{ Getter: func(ctx context.Context, tCtx pcommon.Map) (interface{}, error) { return tCtx, nil }, @@ -39,7 +39,7 @@ func Test_deleteMatchingKeys(t *testing.T) { tests := []struct { name string - target ottl.Getter[pcommon.Map] + target ottl.PMapGetter[pcommon.Map] pattern string want func(pcommon.Map) }{ @@ -91,7 +91,7 @@ func Test_deleteMatchingKeys(t *testing.T) { func Test_deleteMatchingKeys_bad_input(t *testing.T) { input := pcommon.NewValueInt(1) - target := &ottl.StandardGetSetter[interface{}]{ + target := &ottl.StandardTypeGetter[interface{}, pcommon.Map]{ Getter: func(ctx context.Context, tCtx interface{}) (interface{}, error) { return tCtx, nil }, @@ -101,13 +101,11 @@ func Test_deleteMatchingKeys_bad_input(t *testing.T) { assert.NoError(t, err) _, err = exprFunc(nil, input) - assert.Nil(t, err) - - assert.Equal(t, pcommon.NewValueInt(1), input) + assert.Error(t, err) } func Test_deleteMatchingKeys_get_nil(t *testing.T) { - target := &ottl.StandardGetSetter[interface{}]{ + target := &ottl.StandardTypeGetter[interface{}, pcommon.Map]{ Getter: func(ctx context.Context, tCtx interface{}) (interface{}, error) { return tCtx, nil }, @@ -115,13 +113,12 @@ func Test_deleteMatchingKeys_get_nil(t *testing.T) { exprFunc, err := DeleteMatchingKeys[interface{}](target, "anything") assert.NoError(t, err) - result, err := exprFunc(nil, nil) - assert.NoError(t, err) - assert.Nil(t, result) + _, err = exprFunc(nil, nil) + assert.Error(t, err) } func Test_deleteMatchingKeys_invalid_pattern(t *testing.T) { - target := &ottl.StandardGetSetter[interface{}]{ + target := &ottl.StandardTypeGetter[interface{}, pcommon.Map]{ Getter: func(ctx context.Context, tCtx interface{}) (interface{}, error) { t.Errorf("nothing should be received in this scenario") return nil, nil diff --git a/pkg/ottl/ottlfuncs/func_keep_keys.go b/pkg/ottl/ottlfuncs/func_keep_keys.go index 1ade013ae02a..be46c87df516 100644 --- a/pkg/ottl/ottlfuncs/func_keep_keys.go +++ b/pkg/ottl/ottlfuncs/func_keep_keys.go @@ -22,7 +22,7 @@ import ( "github.com/open-telemetry/opentelemetry-collector-contrib/pkg/ottl" ) -func KeepKeys[K any](target ottl.GetSetter[K], keys []string) (ottl.ExprFunc[K], error) { +func KeepKeys[K any](target ottl.PMapGetter[K], keys []string) (ottl.ExprFunc[K], error) { keySet := make(map[string]struct{}, len(keys)) for _, key := range keys { keySet[key] = struct{}{} @@ -33,18 +33,12 @@ func KeepKeys[K any](target ottl.GetSetter[K], keys []string) (ottl.ExprFunc[K], if err != nil { return nil, err } - if val == nil { - return nil, nil - } - - if attrs, ok := val.(pcommon.Map); ok { - attrs.RemoveIf(func(key string, value pcommon.Value) bool { - _, ok := keySet[key] - return !ok - }) - if attrs.Len() == 0 { - attrs.Clear() - } + val.RemoveIf(func(key string, value pcommon.Value) bool { + _, ok := keySet[key] + return !ok + }) + if val.Len() == 0 { + val.Clear() } return nil, nil }, nil diff --git a/pkg/ottl/ottlfuncs/func_keep_keys_test.go b/pkg/ottl/ottlfuncs/func_keep_keys_test.go index ce848a64cbb1..87dea8d8041d 100644 --- a/pkg/ottl/ottlfuncs/func_keep_keys_test.go +++ b/pkg/ottl/ottlfuncs/func_keep_keys_test.go @@ -30,19 +30,15 @@ func Test_keepKeys(t *testing.T) { input.PutInt("test2", 3) input.PutBool("test3", true) - target := &ottl.StandardGetSetter[pcommon.Map]{ + target := &ottl.StandardTypeGetter[pcommon.Map, pcommon.Map]{ Getter: func(ctx context.Context, tCtx pcommon.Map) (interface{}, error) { return tCtx, nil }, - Setter: func(ctx context.Context, tCtx pcommon.Map, val interface{}) error { - val.(pcommon.Map).CopyTo(tCtx) - return nil - }, } tests := []struct { name string - target ottl.GetSetter[pcommon.Map] + target ottl.PMapGetter[pcommon.Map] keys []string want func(pcommon.Map) }{ @@ -75,12 +71,6 @@ func Test_keepKeys(t *testing.T) { keys: []string{"no match"}, want: func(expectedMap pcommon.Map) {}, }, - { - name: "input is not a pcommon.Map", - target: target, - keys: []string{"no match"}, - want: func(expectedMap pcommon.Map) {}, - }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { @@ -103,14 +93,10 @@ func Test_keepKeys(t *testing.T) { func Test_keepKeys_bad_input(t *testing.T) { input := pcommon.NewValueStr("not a map") - target := &ottl.StandardGetSetter[interface{}]{ + target := &ottl.StandardTypeGetter[interface{}, pcommon.Map]{ Getter: func(ctx context.Context, tCtx interface{}) (interface{}, error) { return tCtx, nil }, - Setter: func(ctx context.Context, tCtx interface{}, val interface{}) error { - t.Errorf("nothing should be set in this scenario") - return nil - }, } keys := []string{"anything"} @@ -119,27 +105,20 @@ func Test_keepKeys_bad_input(t *testing.T) { assert.NoError(t, err) _, err = exprFunc(nil, input) - assert.Nil(t, err) - - assert.Equal(t, pcommon.NewValueStr("not a map"), input) + assert.Error(t, err) } func Test_keepKeys_get_nil(t *testing.T) { - target := &ottl.StandardGetSetter[interface{}]{ + target := &ottl.StandardTypeGetter[interface{}, pcommon.Map]{ Getter: func(ctx context.Context, tCtx interface{}) (interface{}, error) { return tCtx, nil }, - Setter: func(ctx context.Context, tCtx interface{}, val interface{}) error { - t.Errorf("nothing should be set in this scenario") - return nil - }, } keys := []string{"anything"} exprFunc, err := KeepKeys[interface{}](target, keys) assert.NoError(t, err) - result, err := exprFunc(nil, nil) - assert.NoError(t, err) - assert.Nil(t, result) + _, err = exprFunc(nil, nil) + assert.Error(t, err) } diff --git a/pkg/ottl/ottlfuncs/func_limit.go b/pkg/ottl/ottlfuncs/func_limit.go index baa2478f020b..e34ed23d4d4e 100644 --- a/pkg/ottl/ottlfuncs/func_limit.go +++ b/pkg/ottl/ottlfuncs/func_limit.go @@ -23,7 +23,7 @@ import ( "github.com/open-telemetry/opentelemetry-collector-contrib/pkg/ottl" ) -func Limit[K any](target ottl.GetSetter[K], limit int64, priorityKeys []string) (ottl.ExprFunc[K], error) { +func Limit[K any](target ottl.PMapGetter[K], limit int64, priorityKeys []string) (ottl.ExprFunc[K], error) { if limit < 0 { return nil, fmt.Errorf("invalid limit for limit function, %d cannot be negative", limit) } @@ -43,27 +43,19 @@ func Limit[K any](target ottl.GetSetter[K], limit int64, priorityKeys []string) if err != nil { return nil, err } - if val == nil { - return nil, nil - } - - attrs, ok := val.(pcommon.Map) - if !ok { - return nil, nil - } - if int64(attrs.Len()) <= limit { + if int64(val.Len()) <= limit { return nil, nil } count := int64(0) for _, key := range priorityKeys { - if _, ok := attrs.Get(key); ok { + if _, ok := val.Get(key); ok { count++ } } - attrs.RemoveIf(func(key string, value pcommon.Value) bool { + val.RemoveIf(func(key string, value pcommon.Value) bool { if _, ok := keep[key]; ok { return false } diff --git a/pkg/ottl/ottlfuncs/func_limit_test.go b/pkg/ottl/ottlfuncs/func_limit_test.go index 3627a98f061d..0cb14726a415 100644 --- a/pkg/ottl/ottlfuncs/func_limit_test.go +++ b/pkg/ottl/ottlfuncs/func_limit_test.go @@ -30,19 +30,15 @@ func Test_limit(t *testing.T) { input.PutInt("test2", 3) input.PutBool("test3", true) - target := &ottl.StandardGetSetter[pcommon.Map]{ + target := &ottl.StandardTypeGetter[pcommon.Map, pcommon.Map]{ Getter: func(ctx context.Context, tCtx pcommon.Map) (interface{}, error) { return tCtx, nil }, - Setter: func(ctx context.Context, tCtx pcommon.Map, val interface{}) error { - val.(pcommon.Map).CopyTo(tCtx) - return nil - }, } tests := []struct { name string - target ottl.GetSetter[pcommon.Map] + target ottl.PMapGetter[pcommon.Map] limit int64 keep []string want func(pcommon.Map) @@ -146,18 +142,18 @@ func Test_limit(t *testing.T) { func Test_limit_validation(t *testing.T) { tests := []struct { name string - target ottl.GetSetter[interface{}] + target ottl.PMapGetter[interface{}] keep []string limit int64 }{ { name: "limit less than zero", - target: &ottl.StandardGetSetter[interface{}]{}, + target: &ottl.StandardTypeGetter[interface{}, pcommon.Map]{}, limit: int64(-1), }, { name: "limit less than # of keep attrs", - target: &ottl.StandardGetSetter[interface{}]{}, + target: &ottl.StandardTypeGetter[interface{}, pcommon.Map]{}, keep: []string{"test", "test"}, limit: int64(1), }, @@ -172,39 +168,27 @@ func Test_limit_validation(t *testing.T) { func Test_limit_bad_input(t *testing.T) { input := pcommon.NewValueStr("not a map") - target := &ottl.StandardGetSetter[interface{}]{ + target := &ottl.StandardTypeGetter[interface{}, pcommon.Map]{ Getter: func(ctx context.Context, tCtx interface{}) (interface{}, error) { return tCtx, nil }, - Setter: func(ctx context.Context, tCtx interface{}, val interface{}) error { - t.Errorf("nothing should be set in this scenario") - return nil - }, } exprFunc, err := Limit[interface{}](target, 1, []string{}) assert.NoError(t, err) - result, err := exprFunc(nil, input) - assert.NoError(t, err) - assert.Nil(t, result) - assert.Equal(t, pcommon.NewValueStr("not a map"), input) + _, err = exprFunc(nil, input) + assert.Error(t, err) } func Test_limit_get_nil(t *testing.T) { - target := &ottl.StandardGetSetter[interface{}]{ + target := &ottl.StandardTypeGetter[interface{}, pcommon.Map]{ Getter: func(ctx context.Context, tCtx interface{}) (interface{}, error) { return tCtx, nil }, - Setter: func(ctx context.Context, tCtx interface{}, val interface{}) error { - t.Errorf("nothing should be set in this scenario") - return nil - }, } exprFunc, err := Limit[interface{}](target, 1, []string{}) assert.NoError(t, err) - result, err := exprFunc(nil, nil) - assert.NoError(t, err) - assert.Nil(t, result) - assert.Nil(t, result) + _, err = exprFunc(nil, nil) + assert.Error(t, err) } diff --git a/pkg/ottl/ottlfuncs/func_merge_maps.go b/pkg/ottl/ottlfuncs/func_merge_maps.go index 85b5eb788234..9e99b7a8b566 100644 --- a/pkg/ottl/ottlfuncs/func_merge_maps.go +++ b/pkg/ottl/ottlfuncs/func_merge_maps.go @@ -35,50 +35,44 @@ const ( // insert: Insert the value from `source` into `target` where the key does not already exist. // update: Update the entry in `target` with the value from `source` where the key does exist // upsert: Performs insert or update. Insert the value from `source` into `target` where the key does not already exist and update the entry in `target` with the value from `source` where the key does exist. -func MergeMaps[K any](target ottl.Getter[K], source ottl.Getter[K], strategy string) (ottl.ExprFunc[K], error) { +func MergeMaps[K any](target ottl.PMapGetter[K], source ottl.PMapGetter[K], strategy string) (ottl.ExprFunc[K], error) { if strategy != INSERT && strategy != UPDATE && strategy != UPSERT { return nil, fmt.Errorf("invalid value for strategy, %v, must be 'insert', 'update' or 'upsert'", strategy) } return func(ctx context.Context, tCtx K) (interface{}, error) { - targetVal, err := target.Get(ctx, tCtx) + targetMap, err := target.Get(ctx, tCtx) if err != nil { return nil, err } - - if targetMap, ok := targetVal.(pcommon.Map); ok { - val, err := source.Get(ctx, tCtx) - if err != nil { - return nil, err - } - - if valueMap, ok := val.(pcommon.Map); ok { - switch strategy { - case INSERT: - valueMap.Range(func(k string, v pcommon.Value) bool { - if _, ok := targetMap.Get(k); !ok { - tv := targetMap.PutEmpty(k) - v.CopyTo(tv) - } - return true - }) - case UPDATE: - valueMap.Range(func(k string, v pcommon.Value) bool { - if tv, ok := targetMap.Get(k); ok { - v.CopyTo(tv) - } - return true - }) - case UPSERT: - valueMap.Range(func(k string, v pcommon.Value) bool { - tv := targetMap.PutEmpty(k) - v.CopyTo(tv) - return true - }) - default: - return nil, fmt.Errorf("unknown strategy, %v", strategy) + valueMap, err := source.Get(ctx, tCtx) + if err != nil { + return nil, err + } + switch strategy { + case INSERT: + valueMap.Range(func(k string, v pcommon.Value) bool { + if _, ok := targetMap.Get(k); !ok { + tv := targetMap.PutEmpty(k) + v.CopyTo(tv) + } + return true + }) + case UPDATE: + valueMap.Range(func(k string, v pcommon.Value) bool { + if tv, ok := targetMap.Get(k); ok { + v.CopyTo(tv) } - } + return true + }) + case UPSERT: + valueMap.Range(func(k string, v pcommon.Value) bool { + tv := targetMap.PutEmpty(k) + v.CopyTo(tv) + return true + }) + default: + return nil, fmt.Errorf("unknown strategy, %v", strategy) } return nil, nil }, nil diff --git a/pkg/ottl/ottlfuncs/func_merge_maps_test.go b/pkg/ottl/ottlfuncs/func_merge_maps_test.go index 89be07e81eab..d405dc551637 100644 --- a/pkg/ottl/ottlfuncs/func_merge_maps_test.go +++ b/pkg/ottl/ottlfuncs/func_merge_maps_test.go @@ -29,7 +29,7 @@ func Test_MergeMaps(t *testing.T) { input := pcommon.NewMap() input.PutStr("attr1", "value1") - targetGetter := &ottl.StandardGetSetter[pcommon.Map]{ + targetGetter := &ottl.StandardTypeGetter[pcommon.Map, pcommon.Map]{ Getter: func(ctx context.Context, tCtx pcommon.Map) (interface{}, error) { return tCtx, nil }, @@ -37,13 +37,13 @@ func Test_MergeMaps(t *testing.T) { tests := []struct { name string - source ottl.Getter[pcommon.Map] + source ottl.PMapGetter[pcommon.Map] strategy string want func(pcommon.Map) }{ { name: "Upsert no conflicting keys", - source: ottl.StandardGetSetter[pcommon.Map]{ + source: ottl.StandardTypeGetter[pcommon.Map, pcommon.Map]{ Getter: func(ctx context.Context, _ pcommon.Map) (interface{}, error) { m := pcommon.NewMap() m.PutStr("attr2", "value2") @@ -58,7 +58,7 @@ func Test_MergeMaps(t *testing.T) { }, { name: "Upsert conflicting key", - source: ottl.StandardGetSetter[pcommon.Map]{ + source: ottl.StandardTypeGetter[pcommon.Map, pcommon.Map]{ Getter: func(ctx context.Context, _ pcommon.Map) (interface{}, error) { m := pcommon.NewMap() m.PutStr("attr1", "value3") @@ -74,7 +74,7 @@ func Test_MergeMaps(t *testing.T) { }, { name: "Insert no conflicting keys", - source: ottl.StandardGetSetter[pcommon.Map]{ + source: ottl.StandardTypeGetter[pcommon.Map, pcommon.Map]{ Getter: func(ctx context.Context, _ pcommon.Map) (interface{}, error) { m := pcommon.NewMap() m.PutStr("attr2", "value2") @@ -89,7 +89,7 @@ func Test_MergeMaps(t *testing.T) { }, { name: "Insert conflicting key", - source: ottl.StandardGetSetter[pcommon.Map]{ + source: ottl.StandardTypeGetter[pcommon.Map, pcommon.Map]{ Getter: func(ctx context.Context, _ pcommon.Map) (interface{}, error) { m := pcommon.NewMap() m.PutStr("attr1", "value3") @@ -105,7 +105,7 @@ func Test_MergeMaps(t *testing.T) { }, { name: "Update no conflicting keys", - source: ottl.StandardGetSetter[pcommon.Map]{ + source: ottl.StandardTypeGetter[pcommon.Map, pcommon.Map]{ Getter: func(ctx context.Context, _ pcommon.Map) (interface{}, error) { m := pcommon.NewMap() m.PutStr("attr2", "value2") @@ -119,7 +119,7 @@ func Test_MergeMaps(t *testing.T) { }, { name: "Update conflicting key", - source: ottl.StandardGetSetter[pcommon.Map]{ + source: ottl.StandardTypeGetter[pcommon.Map, pcommon.Map]{ Getter: func(ctx context.Context, _ pcommon.Map) (interface{}, error) { m := pcommon.NewMap() m.PutStr("attr1", "value3") @@ -131,18 +131,6 @@ func Test_MergeMaps(t *testing.T) { expectedValue.PutStr("attr1", "value3") }, }, - { - name: "non-map value leaves target unchanged", - source: ottl.StandardGetSetter[pcommon.Map]{ - Getter: func(ctx context.Context, _ pcommon.Map) (interface{}, error) { - return nil, nil - }, - }, - strategy: UPSERT, - want: func(expectedValue pcommon.Map) { - expectedValue.PutStr("attr1", "value1") - }, - }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { @@ -163,3 +151,39 @@ func Test_MergeMaps(t *testing.T) { }) } } + +func Test_MergeMaps_bad_target(t *testing.T) { + input := &ottl.StandardTypeGetter[interface{}, pcommon.Map]{ + Getter: func(ctx context.Context, tCtx interface{}) (interface{}, error) { + return tCtx, nil + }, + } + target := &ottl.StandardTypeGetter[interface{}, pcommon.Map]{ + Getter: func(ctx context.Context, tCtx interface{}) (interface{}, error) { + return 1, nil + }, + } + + exprFunc, err := MergeMaps[interface{}](target, input, "insert") + assert.NoError(t, err) + _, err = exprFunc(nil, input) + assert.Error(t, err) +} + +func Test_MergeMaps_bad_input(t *testing.T) { + input := &ottl.StandardTypeGetter[interface{}, pcommon.Map]{ + Getter: func(ctx context.Context, tCtx interface{}) (interface{}, error) { + return 1, nil + }, + } + target := &ottl.StandardTypeGetter[interface{}, pcommon.Map]{ + Getter: func(ctx context.Context, tCtx interface{}) (interface{}, error) { + return tCtx, nil + }, + } + + exprFunc, err := MergeMaps[interface{}](target, input, "insert") + assert.NoError(t, err) + _, err = exprFunc(nil, input) + assert.Error(t, err) +} diff --git a/pkg/ottl/ottlfuncs/func_replace_all_matches.go b/pkg/ottl/ottlfuncs/func_replace_all_matches.go index 141794c662c2..78535ab65cf9 100644 --- a/pkg/ottl/ottlfuncs/func_replace_all_matches.go +++ b/pkg/ottl/ottlfuncs/func_replace_all_matches.go @@ -24,7 +24,7 @@ import ( "github.com/open-telemetry/opentelemetry-collector-contrib/pkg/ottl" ) -func ReplaceAllMatches[K any](target ottl.GetSetter[K], pattern string, replacement string) (ottl.ExprFunc[K], error) { +func ReplaceAllMatches[K any](target ottl.PMapGetter[K], pattern string, replacement string) (ottl.ExprFunc[K], error) { glob, err := glob.Compile(pattern) if err != nil { return nil, fmt.Errorf("the pattern supplied to replace_match is not a valid pattern: %w", err) @@ -34,25 +34,12 @@ func ReplaceAllMatches[K any](target ottl.GetSetter[K], pattern string, replacem if err != nil { return nil, err } - if val == nil { - return nil, nil - } - attrs, ok := val.(pcommon.Map) - if !ok { - return nil, nil - } - updated := pcommon.NewMap() - attrs.CopyTo(updated) - updated.Range(func(key string, value pcommon.Value) bool { + val.Range(func(key string, value pcommon.Value) bool { if glob.Match(value.Str()) { value.SetStr(replacement) } return true }) - err = target.Set(ctx, tCtx, updated) - if err != nil { - return nil, err - } return nil, nil }, nil } diff --git a/pkg/ottl/ottlfuncs/func_replace_all_matches_test.go b/pkg/ottl/ottlfuncs/func_replace_all_matches_test.go index 99aa63edd283..3e4d1ef45eb0 100644 --- a/pkg/ottl/ottlfuncs/func_replace_all_matches_test.go +++ b/pkg/ottl/ottlfuncs/func_replace_all_matches_test.go @@ -30,19 +30,15 @@ func Test_replaceAllMatches(t *testing.T) { input.PutStr("test2", "hello") input.PutStr("test3", "goodbye") - target := &ottl.StandardGetSetter[pcommon.Map]{ + target := &ottl.StandardTypeGetter[pcommon.Map, pcommon.Map]{ Getter: func(ctx context.Context, tCtx pcommon.Map) (interface{}, error) { return tCtx, nil }, - Setter: func(ctx context.Context, tCtx pcommon.Map, val interface{}) error { - val.(pcommon.Map).CopyTo(tCtx) - return nil - }, } tests := []struct { name string - target ottl.GetSetter[pcommon.Map] + target ottl.PMapGetter[pcommon.Map] pattern string replacement string want func(pcommon.Map) @@ -92,38 +88,27 @@ func Test_replaceAllMatches(t *testing.T) { func Test_replaceAllMatches_bad_input(t *testing.T) { input := pcommon.NewValueStr("not a map") - target := &ottl.StandardGetSetter[interface{}]{ + target := &ottl.StandardTypeGetter[interface{}, pcommon.Map]{ Getter: func(ctx context.Context, tCtx interface{}) (interface{}, error) { return tCtx, nil }, - Setter: func(ctx context.Context, tCtx interface{}, val interface{}) error { - t.Errorf("nothing should be set in this scenario") - return nil - }, } exprFunc, err := ReplaceAllMatches[interface{}](target, "*", "{replacement}") assert.NoError(t, err) - result, err := exprFunc(nil, input) - assert.NoError(t, err) - assert.Nil(t, result) - assert.Equal(t, pcommon.NewValueStr("not a map"), input) + _, err = exprFunc(nil, input) + assert.Error(t, err) } func Test_replaceAllMatches_get_nil(t *testing.T) { - target := &ottl.StandardGetSetter[interface{}]{ + target := &ottl.StandardTypeGetter[interface{}, pcommon.Map]{ Getter: func(ctx context.Context, tCtx interface{}) (interface{}, error) { return tCtx, nil }, - Setter: func(ctx context.Context, tCtx interface{}, val interface{}) error { - t.Errorf("nothing should be set in this scenario") - return nil - }, } exprFunc, err := ReplaceAllMatches[interface{}](target, "*", "{anything}") assert.NoError(t, err) - result, err := exprFunc(nil, nil) - assert.NoError(t, err) - assert.Nil(t, result) + _, err = exprFunc(nil, nil) + assert.Error(t, err) } diff --git a/pkg/ottl/ottlfuncs/func_replace_all_patterns.go b/pkg/ottl/ottlfuncs/func_replace_all_patterns.go index d52053f33c2d..26ac2c47cc3f 100644 --- a/pkg/ottl/ottlfuncs/func_replace_all_patterns.go +++ b/pkg/ottl/ottlfuncs/func_replace_all_patterns.go @@ -29,7 +29,7 @@ const ( modeValue = "value" ) -func ReplaceAllPatterns[K any](target ottl.GetSetter[K], mode string, regexPattern string, replacement string) (ottl.ExprFunc[K], error) { +func ReplaceAllPatterns[K any](target ottl.PMapGetter[K], mode string, regexPattern string, replacement string) (ottl.ExprFunc[K], error) { compiledPattern, err := regexp.Compile(regexPattern) if err != nil { return nil, fmt.Errorf("the regex pattern supplied to replace_all_patterns is not a valid pattern: %w", err) @@ -43,16 +43,9 @@ func ReplaceAllPatterns[K any](target ottl.GetSetter[K], mode string, regexPatte if err != nil { return nil, err } - if val == nil { - return nil, nil - } - attrs, ok := val.(pcommon.Map) - if !ok { - return nil, nil - } updated := pcommon.NewMap() - updated.EnsureCapacity(attrs.Len()) - attrs.Range(func(key string, originalValue pcommon.Value) bool { + updated.EnsureCapacity(val.Len()) + val.Range(func(key string, originalValue pcommon.Value) bool { switch mode { case modeValue: if compiledPattern.MatchString(originalValue.Str()) { @@ -71,11 +64,7 @@ func ReplaceAllPatterns[K any](target ottl.GetSetter[K], mode string, regexPatte } return true }) - err = target.Set(ctx, tCtx, updated) - if err != nil { - return nil, err - } - + updated.CopyTo(val) return nil, nil }, nil } diff --git a/pkg/ottl/ottlfuncs/func_replace_all_patterns_test.go b/pkg/ottl/ottlfuncs/func_replace_all_patterns_test.go index 30f9828792a9..60fcaabde2f5 100644 --- a/pkg/ottl/ottlfuncs/func_replace_all_patterns_test.go +++ b/pkg/ottl/ottlfuncs/func_replace_all_patterns_test.go @@ -31,19 +31,15 @@ func Test_replaceAllPatterns(t *testing.T) { input.PutStr("test2", "hello") input.PutStr("test3", "goodbye world1 and world2") - target := &ottl.StandardGetSetter[pcommon.Map]{ + target := &ottl.StandardTypeGetter[pcommon.Map, pcommon.Map]{ Getter: func(ctx context.Context, tCtx pcommon.Map) (interface{}, error) { return tCtx, nil }, - Setter: func(ctx context.Context, tCtx pcommon.Map, val interface{}) error { - val.(pcommon.Map).CopyTo(tCtx) - return nil - }, } tests := []struct { name string - target ottl.GetSetter[pcommon.Map] + target ottl.PMapGetter[pcommon.Map] mode string pattern string replacement string @@ -173,53 +169,39 @@ func Test_replaceAllPatterns(t *testing.T) { func Test_replaceAllPatterns_bad_input(t *testing.T) { input := pcommon.NewValueStr("not a map") - target := &ottl.StandardGetSetter[interface{}]{ + target := &ottl.StandardTypeGetter[interface{}, pcommon.Map]{ Getter: func(ctx context.Context, tCtx interface{}) (interface{}, error) { return tCtx, nil }, - Setter: func(ctx context.Context, tCtx interface{}, val interface{}) error { - t.Errorf("nothing should be set in this scenario") - return nil - }, } exprFunc, err := ReplaceAllPatterns[interface{}](target, modeValue, "regexpattern", "{replacement}") assert.Nil(t, err) _, err = exprFunc(nil, input) - assert.Nil(t, err) - - assert.Equal(t, pcommon.NewValueStr("not a map"), input) + assert.Error(t, err) } func Test_replaceAllPatterns_get_nil(t *testing.T) { - target := &ottl.StandardGetSetter[interface{}]{ + target := &ottl.StandardTypeGetter[interface{}, pcommon.Map]{ Getter: func(ctx context.Context, tCtx interface{}) (interface{}, error) { return tCtx, nil }, - Setter: func(ctx context.Context, tCtx interface{}, val interface{}) error { - t.Errorf("nothing should be set in this scenario") - return nil - }, } exprFunc, err := ReplaceAllPatterns[interface{}](target, modeValue, "regexp", "{anything}") assert.NoError(t, err) _, err = exprFunc(nil, nil) - assert.Nil(t, err) + assert.Error(t, err) } func Test_replaceAllPatterns_invalid_pattern(t *testing.T) { - target := &ottl.StandardGetSetter[interface{}]{ + target := &ottl.StandardTypeGetter[interface{}, pcommon.Map]{ Getter: func(ctx context.Context, tCtx interface{}) (interface{}, error) { t.Errorf("nothing should be received in this scenario") return nil, nil }, - Setter: func(ctx context.Context, tCtx interface{}, val interface{}) error { - t.Errorf("nothing should be set in this scenario") - return nil - }, } invalidRegexPattern := "*" @@ -230,15 +212,11 @@ func Test_replaceAllPatterns_invalid_pattern(t *testing.T) { } func Test_replaceAllPatterns_invalid_model(t *testing.T) { - target := &ottl.StandardGetSetter[interface{}]{ + target := &ottl.StandardTypeGetter[interface{}, pcommon.Map]{ Getter: func(ctx context.Context, tCtx interface{}) (interface{}, error) { t.Errorf("nothing should be received in this scenario") return nil, nil }, - Setter: func(ctx context.Context, tCtx interface{}, val interface{}) error { - t.Errorf("nothing should be set in this scenario") - return nil - }, } invalidMode := "invalid" diff --git a/pkg/ottl/ottlfuncs/func_truncate_all.go b/pkg/ottl/ottlfuncs/func_truncate_all.go index 6ef7f246bf0d..894c256b44f4 100644 --- a/pkg/ottl/ottlfuncs/func_truncate_all.go +++ b/pkg/ottl/ottlfuncs/func_truncate_all.go @@ -23,7 +23,7 @@ import ( "github.com/open-telemetry/opentelemetry-collector-contrib/pkg/ottl" ) -func TruncateAll[K any](target ottl.GetSetter[K], limit int64) (ottl.ExprFunc[K], error) { +func TruncateAll[K any](target ottl.PMapGetter[K], limit int64) (ottl.ExprFunc[K], error) { if limit < 0 { return nil, fmt.Errorf("invalid limit for truncate_all function, %d cannot be negative", limit) } @@ -36,28 +36,13 @@ func TruncateAll[K any](target ottl.GetSetter[K], limit int64) (ottl.ExprFunc[K] if err != nil { return nil, err } - if val == nil { - return nil, nil - } - - attrs, ok := val.(pcommon.Map) - if !ok { - return nil, nil - } - - updated := pcommon.NewMap() - attrs.CopyTo(updated) - updated.Range(func(key string, value pcommon.Value) bool { + val.Range(func(key string, value pcommon.Value) bool { stringVal := value.Str() if int64(len(stringVal)) > limit { value.SetStr(stringVal[:limit]) } return true }) - err = target.Set(ctx, tCtx, updated) - if err != nil { - return nil, err - } // TODO: Write log when truncation is performed // https://github.com/open-telemetry/opentelemetry-collector-contrib/issues/9730 return nil, nil diff --git a/pkg/ottl/ottlfuncs/func_truncate_all_test.go b/pkg/ottl/ottlfuncs/func_truncate_all_test.go index 47444718fbcf..4241ad5833ce 100644 --- a/pkg/ottl/ottlfuncs/func_truncate_all_test.go +++ b/pkg/ottl/ottlfuncs/func_truncate_all_test.go @@ -31,19 +31,15 @@ func Test_truncateAll(t *testing.T) { input.PutInt("test2", 3) input.PutBool("test3", true) - target := &ottl.StandardGetSetter[pcommon.Map]{ + target := &ottl.StandardTypeGetter[pcommon.Map, pcommon.Map]{ Getter: func(ctx context.Context, tCtx pcommon.Map) (interface{}, error) { return tCtx, nil }, - Setter: func(ctx context.Context, tCtx pcommon.Map, val interface{}) error { - val.(pcommon.Map).CopyTo(tCtx) - return nil - }, } tests := []struct { name string - target ottl.GetSetter[pcommon.Map] + target ottl.PMapGetter[pcommon.Map] limit int64 want func(pcommon.Map) }{ @@ -109,47 +105,36 @@ func Test_truncateAll(t *testing.T) { } func Test_truncateAll_validation(t *testing.T) { - _, err := TruncateAll[interface{}](&ottl.StandardGetSetter[interface{}]{}, -1) + _, err := TruncateAll[interface{}](&ottl.StandardTypeGetter[interface{}, pcommon.Map]{}, -1) require.Error(t, err) assert.ErrorContains(t, err, "invalid limit for truncate_all function, -1 cannot be negative") } func Test_truncateAll_bad_input(t *testing.T) { input := pcommon.NewValueStr("not a map") - target := &ottl.StandardGetSetter[interface{}]{ + target := &ottl.StandardTypeGetter[interface{}, pcommon.Map]{ Getter: func(ctx context.Context, tCtx interface{}) (interface{}, error) { return tCtx, nil }, - Setter: func(ctx context.Context, tCtx interface{}, val interface{}) error { - t.Errorf("nothing should be set in this scenario") - return nil - }, } exprFunc, err := TruncateAll[interface{}](target, 1) assert.NoError(t, err) - result, err := exprFunc(nil, input) - assert.NoError(t, err) - assert.Nil(t, result) - assert.Equal(t, pcommon.NewValueStr("not a map"), input) + _, err = exprFunc(nil, input) + assert.Error(t, err) } func Test_truncateAll_get_nil(t *testing.T) { - target := &ottl.StandardGetSetter[interface{}]{ + target := &ottl.StandardTypeGetter[interface{}, pcommon.Map]{ Getter: func(ctx context.Context, tCtx interface{}) (interface{}, error) { return tCtx, nil }, - Setter: func(ctx context.Context, tCtx interface{}, val interface{}) error { - t.Errorf("nothing should be set in this scenario") - return nil - }, } exprFunc, err := TruncateAll[interface{}](target, 1) assert.NoError(t, err) - result, err := exprFunc(nil, nil) - assert.NoError(t, err) - assert.Nil(t, result) + _, err = exprFunc(nil, nil) + assert.Error(t, err) }