Skip to content

[pkg/ottl] Fix OTTL functions by using setters #39416

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

Open
wants to merge 14 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
27 changes: 27 additions & 0 deletions .chloggen/fix-ottl-functions.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 OTTL functions by using setters.

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

# (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: [user]
6 changes: 3 additions & 3 deletions pkg/ottl/ottlfuncs/func_delete_key.go
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ import (
)

type DeleteKeyArguments[K any] struct {
Target ottl.PMapGetter[K]
Target ottl.PMapGetSetter[K]
Key string
}

Expand All @@ -29,13 +29,13 @@ func createDeleteKeyFunction[K any](_ ottl.FunctionContext, oArgs ottl.Arguments
return deleteKey(args.Target, args.Key), nil
}

func deleteKey[K any](target ottl.PMapGetter[K], key string) ottl.ExprFunc[K] {
func deleteKey[K any](target ottl.PMapGetSetter[K], key string) ottl.ExprFunc[K] {
return func(ctx context.Context, tCtx K) (any, error) {
val, err := target.Get(ctx, tCtx)
if err != nil {
return nil, err
}
val.Remove(key)
return nil, nil
return nil, target.Set(ctx, tCtx, val)
}
}
65 changes: 39 additions & 26 deletions pkg/ottl/ottlfuncs/func_delete_key_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ package ottlfuncs

import (
"context"
"errors"
"testing"

"github.com/stretchr/testify/assert"
Expand All @@ -19,40 +20,30 @@ func Test_deleteKey(t *testing.T) {
input.PutInt("test2", 3)
input.PutBool("test3", true)

target := &ottl.StandardPMapGetter[pcommon.Map]{
Getter: func(_ context.Context, tCtx pcommon.Map) (any, error) {
return tCtx, nil
},
}

tests := []struct {
name string
target ottl.PMapGetter[pcommon.Map]
key string
want func(pcommon.Map)
name string
key string
want func(pcommon.Map)
}{
{
name: "delete test",
target: target,
key: "test",
name: "delete test",
key: "test",
want: func(expectedMap pcommon.Map) {
expectedMap.PutBool("test3", true)
expectedMap.PutInt("test2", 3)
},
},
{
name: "delete test2",
target: target,
key: "test2",
name: "delete test2",
key: "test2",
want: func(expectedMap pcommon.Map) {
expectedMap.PutStr("test", "hello world")
expectedMap.PutBool("test3", true)
},
},
{
name: "delete nothing",
target: target,
key: "not a valid key",
name: "delete nothing",
key: "not a valid key",
want: func(expectedMap pcommon.Map) {
expectedMap.PutStr("test", "hello world")
expectedMap.PutInt("test2", 3)
Expand All @@ -65,10 +56,26 @@ func Test_deleteKey(t *testing.T) {
scenarioMap := pcommon.NewMap()
input.CopyTo(scenarioMap)

exprFunc := deleteKey(tt.target, tt.key)
setterWasCalled := false
target := &ottl.StandardPMapGetSetter[pcommon.Map]{
Getter: func(_ context.Context, tCtx pcommon.Map) (pcommon.Map, error) {
return tCtx, nil
},
Setter: func(_ context.Context, tCtx pcommon.Map, val any) error {
setterWasCalled = true
if v, ok := val.(pcommon.Map); ok {
v.CopyTo(tCtx)
return nil
}
return errors.New("expected pcommon.Map")
},
}

exprFunc := deleteKey(target, tt.key)

_, err := exprFunc(nil, scenarioMap)
assert.NoError(t, err)
assert.True(t, setterWasCalled)

expected := pcommon.NewMap()
tt.want(expected)
Expand All @@ -80,9 +87,12 @@ func Test_deleteKey(t *testing.T) {

func Test_deleteKey_bad_input(t *testing.T) {
input := pcommon.NewValueStr("not a map")
target := &ottl.StandardPMapGetter[any]{
Getter: func(_ context.Context, tCtx any) (any, error) {
return tCtx, nil
target := &ottl.StandardPMapGetSetter[any]{
Getter: func(_ context.Context, tCtx any) (pcommon.Map, error) {
if v, ok := tCtx.(pcommon.Map); ok {
return v, nil
}
return pcommon.Map{}, errors.New("expected pcommon.Map")
},
}

Expand All @@ -94,9 +104,12 @@ func Test_deleteKey_bad_input(t *testing.T) {
}

func Test_deleteKey_get_nil(t *testing.T) {
target := &ottl.StandardPMapGetter[any]{
Getter: func(_ context.Context, tCtx any) (any, error) {
return tCtx, nil
target := &ottl.StandardPMapGetSetter[any]{
Getter: func(_ context.Context, tCtx any) (pcommon.Map, error) {
if v, ok := tCtx.(pcommon.Map); ok {
return v, nil
}
return pcommon.Map{}, errors.New("expected pcommon.Map")
},
}

Expand Down
6 changes: 3 additions & 3 deletions pkg/ottl/ottlfuncs/func_delete_matching_keys.go
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ import (
)

type DeleteMatchingKeysArguments[K any] struct {
Target ottl.PMapGetter[K]
Target ottl.PMapGetSetter[K]
Pattern string
}

Expand All @@ -33,7 +33,7 @@ func createDeleteMatchingKeysFunction[K any](_ ottl.FunctionContext, oArgs ottl.
return deleteMatchingKeys(args.Target, args.Pattern)
}

func deleteMatchingKeys[K any](target ottl.PMapGetter[K], pattern string) (ottl.ExprFunc[K], error) {
func deleteMatchingKeys[K any](target ottl.PMapGetSetter[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)
Expand All @@ -46,6 +46,6 @@ func deleteMatchingKeys[K any](target ottl.PMapGetter[K], pattern string) (ottl.
val.RemoveIf(func(key string, _ pcommon.Value) bool {
return compiledPattern.MatchString(key)
})
return nil, nil
return nil, target.Set(ctx, tCtx, val)
}, nil
}
53 changes: 33 additions & 20 deletions pkg/ottl/ottlfuncs/func_delete_matching_keys_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ package ottlfuncs

import (
"context"
"errors"
"testing"

"github.com/stretchr/testify/assert"
Expand All @@ -20,37 +21,27 @@ func Test_deleteMatchingKeys(t *testing.T) {
input.PutInt("test2", 3)
input.PutBool("test3", true)

target := &ottl.StandardPMapGetter[pcommon.Map]{
Getter: func(_ context.Context, tCtx pcommon.Map) (any, error) {
return tCtx, nil
},
}

tests := []struct {
name string
target ottl.PMapGetter[pcommon.Map]
pattern string
want func(pcommon.Map)
}{
{
name: "delete everything",
target: target,
pattern: "test.*",
want: func(expectedMap pcommon.Map) {
expectedMap.EnsureCapacity(3)
},
},
{
name: "delete attributes that end in a number",
target: target,
pattern: "\\d$",
want: func(expectedMap pcommon.Map) {
expectedMap.PutStr("test", "hello world")
},
},
{
name: "delete nothing",
target: target,
pattern: "not a matching pattern",
want: func(expectedMap pcommon.Map) {
expectedMap.PutStr("test", "hello world")
Expand All @@ -64,11 +55,27 @@ func Test_deleteMatchingKeys(t *testing.T) {
scenarioMap := pcommon.NewMap()
input.CopyTo(scenarioMap)

exprFunc, err := deleteMatchingKeys(tt.target, tt.pattern)
setterWasCalled := false
target := &ottl.StandardPMapGetSetter[pcommon.Map]{
Getter: func(_ context.Context, tCtx pcommon.Map) (pcommon.Map, error) {
return tCtx, nil
},
Setter: func(_ context.Context, tCtx pcommon.Map, m any) error {
setterWasCalled = true
if v, ok := m.(pcommon.Map); ok {
v.CopyTo(tCtx)
return nil
}
return errors.New("expected pcommon.Map")
},
}

exprFunc, err := deleteMatchingKeys(target, tt.pattern)
assert.NoError(t, err)

_, err = exprFunc(nil, scenarioMap)
assert.NoError(t, err)
assert.True(t, setterWasCalled)

expected := pcommon.NewMap()
tt.want(expected)
Expand All @@ -80,9 +87,12 @@ func Test_deleteMatchingKeys(t *testing.T) {

func Test_deleteMatchingKeys_bad_input(t *testing.T) {
input := pcommon.NewValueInt(1)
target := &ottl.StandardPMapGetter[any]{
Getter: func(_ context.Context, tCtx any) (any, error) {
return tCtx, nil
target := &ottl.StandardPMapGetSetter[any]{
Getter: func(_ context.Context, tCtx any) (pcommon.Map, error) {
if v, ok := tCtx.(pcommon.Map); ok {
return v, nil
}
return pcommon.Map{}, errors.New("expected pcommon.Map")
},
}

Expand All @@ -94,9 +104,12 @@ func Test_deleteMatchingKeys_bad_input(t *testing.T) {
}

func Test_deleteMatchingKeys_get_nil(t *testing.T) {
target := &ottl.StandardPMapGetter[any]{
Getter: func(_ context.Context, tCtx any) (any, error) {
return tCtx, nil
target := &ottl.StandardPMapGetSetter[any]{
Getter: func(_ context.Context, tCtx any) (pcommon.Map, error) {
if v, ok := tCtx.(pcommon.Map); ok {
return v, nil
}
return pcommon.Map{}, errors.New("expected pcommon.Map")
},
}

Expand All @@ -107,10 +120,10 @@ func Test_deleteMatchingKeys_get_nil(t *testing.T) {
}

func Test_deleteMatchingKeys_invalid_pattern(t *testing.T) {
target := &ottl.StandardPMapGetter[any]{
Getter: func(_ context.Context, _ any) (any, error) {
target := &ottl.StandardPMapGetSetter[any]{
Getter: func(_ context.Context, _ any) (pcommon.Map, error) {
t.Errorf("nothing should be received in this scenario")
return nil, nil
return pcommon.Map{}, nil
},
}

Expand Down
8 changes: 3 additions & 5 deletions pkg/ottl/ottlfuncs/func_flatten.go
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ import (
)

type FlattenArguments[K any] struct {
Target ottl.PMapGetter[K]
Target ottl.PMapGetSetter[K]
Prefix ottl.Optional[string]
Depth ottl.Optional[int64]
ResolveConflicts ottl.Optional[bool]
Expand All @@ -43,7 +43,7 @@ func createFlattenFunction[K any](_ ottl.FunctionContext, oArgs ottl.Arguments)
return flatten(args.Target, args.Prefix, args.Depth, args.ResolveConflicts)
}

func flatten[K any](target ottl.PMapGetter[K], p ottl.Optional[string], d ottl.Optional[int64], c ottl.Optional[bool]) (ottl.ExprFunc[K], error) {
func flatten[K any](target ottl.PMapGetSetter[K], p ottl.Optional[string], d ottl.Optional[int64], c ottl.Optional[bool]) (ottl.ExprFunc[K], error) {
depth := int64(math.MaxInt64)
if !d.IsEmpty() {
depth = d.Get()
Expand All @@ -70,9 +70,7 @@ func flatten[K any](target ottl.PMapGetter[K], p ottl.Optional[string], d ottl.O

flattenData := initFlattenData(resolveConflict, depth)
flattenData.flattenMap(m, prefix, 0)
flattenData.result.MoveTo(m)

return nil, nil
return nil, target.Set(ctx, tCtx, flattenData.result)
}, nil
}

Expand Down
Loading