Skip to content

Commit f2d2dcd

Browse files
rockdabootDylan-M
authored andcommitted
[pkg/ottl] Fix OTTL functions by using setters (open-telemetry#39416)
#### Description Some OTTL functions do not call the accessor.setter function, but instead change the variable directly. This doesn't work with the profiles signal where non-trivial processing is needed. For example, attributes are split into a lookup table and arrays of indices (the rationale is de-duplication). While the accessor.getter function returns a pcommon.Map for attributes, any changes made to that map require passing the map to the accessor.setter function. #### Link to tracking issue Fixes open-telemetry#39100 <!--Describe what testing was performed and which tests were added.--> #### Testing <!--Describe the documentation added.--> #### Documentation <!--Please delete paragraphs that you did not use before submitting.-->
1 parent 046d070 commit f2d2dcd

21 files changed

+479
-318
lines changed

.chloggen/fix-ottl-functions.yaml

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,27 @@
1+
# Use this changelog template to create an entry for release notes.
2+
3+
# One of 'breaking', 'deprecation', 'new_component', 'enhancement', 'bug_fix'
4+
change_type: bug_fix
5+
6+
# The name of the component, or a single word describing the area of concern, (e.g. filelogreceiver)
7+
component: pkg/ottl
8+
9+
# A brief description of the change. Surround your text with quotes ("") if it needs to start with a backtick (`).
10+
note: Fix OTTL functions by using setters.
11+
12+
# Mandatory: One or more tracking issues related to the change. You can use the PR number here if no issue exists.
13+
issues: [39100]
14+
15+
# (Optional) One or more lines of additional information to render under the primary note.
16+
# These lines will be padded with 2 spaces and then inserted directly into the document.
17+
# Use pipe (|) for multiline entries.
18+
subtext:
19+
20+
# If your change doesn't affect end users or the exported elements of any package,
21+
# you should instead start your pull request title with [chore] or use the "Skip Changelog" label.
22+
# Optional: The change log or logs in which this entry should be included.
23+
# e.g. '[user]' or '[user, api]'
24+
# Include 'user' if the change is relevant to end users.
25+
# Include 'api' if there is a change to a library API.
26+
# Default: '[user]'
27+
change_logs: [user]

pkg/ottl/ottlfuncs/func_delete_key.go

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,7 @@ import (
1111
)
1212

1313
type DeleteKeyArguments[K any] struct {
14-
Target ottl.PMapGetter[K]
14+
Target ottl.PMapGetSetter[K]
1515
Key string
1616
}
1717

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

32-
func deleteKey[K any](target ottl.PMapGetter[K], key string) ottl.ExprFunc[K] {
32+
func deleteKey[K any](target ottl.PMapGetSetter[K], key string) ottl.ExprFunc[K] {
3333
return func(ctx context.Context, tCtx K) (any, error) {
3434
val, err := target.Get(ctx, tCtx)
3535
if err != nil {
3636
return nil, err
3737
}
3838
val.Remove(key)
39-
return nil, nil
39+
return nil, target.Set(ctx, tCtx, val)
4040
}
4141
}

pkg/ottl/ottlfuncs/func_delete_key_test.go

Lines changed: 39 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@ package ottlfuncs
55

66
import (
77
"context"
8+
"errors"
89
"testing"
910

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

22-
target := &ottl.StandardPMapGetter[pcommon.Map]{
23-
Getter: func(_ context.Context, tCtx pcommon.Map) (any, error) {
24-
return tCtx, nil
25-
},
26-
}
27-
2823
tests := []struct {
29-
name string
30-
target ottl.PMapGetter[pcommon.Map]
31-
key string
32-
want func(pcommon.Map)
24+
name string
25+
key string
26+
want func(pcommon.Map)
3327
}{
3428
{
35-
name: "delete test",
36-
target: target,
37-
key: "test",
29+
name: "delete test",
30+
key: "test",
3831
want: func(expectedMap pcommon.Map) {
3932
expectedMap.PutBool("test3", true)
4033
expectedMap.PutInt("test2", 3)
4134
},
4235
},
4336
{
44-
name: "delete test2",
45-
target: target,
46-
key: "test2",
37+
name: "delete test2",
38+
key: "test2",
4739
want: func(expectedMap pcommon.Map) {
4840
expectedMap.PutStr("test", "hello world")
4941
expectedMap.PutBool("test3", true)
5042
},
5143
},
5244
{
53-
name: "delete nothing",
54-
target: target,
55-
key: "not a valid key",
45+
name: "delete nothing",
46+
key: "not a valid key",
5647
want: func(expectedMap pcommon.Map) {
5748
expectedMap.PutStr("test", "hello world")
5849
expectedMap.PutInt("test2", 3)
@@ -65,10 +56,26 @@ func Test_deleteKey(t *testing.T) {
6556
scenarioMap := pcommon.NewMap()
6657
input.CopyTo(scenarioMap)
6758

68-
exprFunc := deleteKey(tt.target, tt.key)
59+
setterWasCalled := false
60+
target := &ottl.StandardPMapGetSetter[pcommon.Map]{
61+
Getter: func(_ context.Context, tCtx pcommon.Map) (pcommon.Map, error) {
62+
return tCtx, nil
63+
},
64+
Setter: func(_ context.Context, tCtx pcommon.Map, val any) error {
65+
setterWasCalled = true
66+
if v, ok := val.(pcommon.Map); ok {
67+
v.CopyTo(tCtx)
68+
return nil
69+
}
70+
return errors.New("expected pcommon.Map")
71+
},
72+
}
73+
74+
exprFunc := deleteKey(target, tt.key)
6975

7076
_, err := exprFunc(nil, scenarioMap)
7177
assert.NoError(t, err)
78+
assert.True(t, setterWasCalled)
7279

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

8188
func Test_deleteKey_bad_input(t *testing.T) {
8289
input := pcommon.NewValueStr("not a map")
83-
target := &ottl.StandardPMapGetter[any]{
84-
Getter: func(_ context.Context, tCtx any) (any, error) {
85-
return tCtx, nil
90+
target := &ottl.StandardPMapGetSetter[any]{
91+
Getter: func(_ context.Context, tCtx any) (pcommon.Map, error) {
92+
if v, ok := tCtx.(pcommon.Map); ok {
93+
return v, nil
94+
}
95+
return pcommon.Map{}, errors.New("expected pcommon.Map")
8696
},
8797
}
8898

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

96106
func Test_deleteKey_get_nil(t *testing.T) {
97-
target := &ottl.StandardPMapGetter[any]{
98-
Getter: func(_ context.Context, tCtx any) (any, error) {
99-
return tCtx, nil
107+
target := &ottl.StandardPMapGetSetter[any]{
108+
Getter: func(_ context.Context, tCtx any) (pcommon.Map, error) {
109+
if v, ok := tCtx.(pcommon.Map); ok {
110+
return v, nil
111+
}
112+
return pcommon.Map{}, errors.New("expected pcommon.Map")
100113
},
101114
}
102115

pkg/ottl/ottlfuncs/func_delete_matching_keys.go

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,7 @@ import (
1515
)
1616

1717
type DeleteMatchingKeysArguments[K any] struct {
18-
Target ottl.PMapGetter[K]
18+
Target ottl.PMapGetSetter[K]
1919
Pattern string
2020
}
2121

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

36-
func deleteMatchingKeys[K any](target ottl.PMapGetter[K], pattern string) (ottl.ExprFunc[K], error) {
36+
func deleteMatchingKeys[K any](target ottl.PMapGetSetter[K], pattern string) (ottl.ExprFunc[K], error) {
3737
compiledPattern, err := regexp.Compile(pattern)
3838
if err != nil {
3939
return nil, fmt.Errorf("the regex pattern supplied to delete_matching_keys is not a valid pattern: %w", err)
@@ -46,6 +46,6 @@ func deleteMatchingKeys[K any](target ottl.PMapGetter[K], pattern string) (ottl.
4646
val.RemoveIf(func(key string, _ pcommon.Value) bool {
4747
return compiledPattern.MatchString(key)
4848
})
49-
return nil, nil
49+
return nil, target.Set(ctx, tCtx, val)
5050
}, nil
5151
}

pkg/ottl/ottlfuncs/func_delete_matching_keys_test.go

Lines changed: 33 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@ package ottlfuncs
55

66
import (
77
"context"
8+
"errors"
89
"testing"
910

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

23-
target := &ottl.StandardPMapGetter[pcommon.Map]{
24-
Getter: func(_ context.Context, tCtx pcommon.Map) (any, error) {
25-
return tCtx, nil
26-
},
27-
}
28-
2924
tests := []struct {
3025
name string
31-
target ottl.PMapGetter[pcommon.Map]
3226
pattern string
3327
want func(pcommon.Map)
3428
}{
3529
{
3630
name: "delete everything",
37-
target: target,
3831
pattern: "test.*",
3932
want: func(expectedMap pcommon.Map) {
4033
expectedMap.EnsureCapacity(3)
4134
},
4235
},
4336
{
4437
name: "delete attributes that end in a number",
45-
target: target,
4638
pattern: "\\d$",
4739
want: func(expectedMap pcommon.Map) {
4840
expectedMap.PutStr("test", "hello world")
4941
},
5042
},
5143
{
5244
name: "delete nothing",
53-
target: target,
5445
pattern: "not a matching pattern",
5546
want: func(expectedMap pcommon.Map) {
5647
expectedMap.PutStr("test", "hello world")
@@ -64,11 +55,27 @@ func Test_deleteMatchingKeys(t *testing.T) {
6455
scenarioMap := pcommon.NewMap()
6556
input.CopyTo(scenarioMap)
6657

67-
exprFunc, err := deleteMatchingKeys(tt.target, tt.pattern)
58+
setterWasCalled := false
59+
target := &ottl.StandardPMapGetSetter[pcommon.Map]{
60+
Getter: func(_ context.Context, tCtx pcommon.Map) (pcommon.Map, error) {
61+
return tCtx, nil
62+
},
63+
Setter: func(_ context.Context, tCtx pcommon.Map, m any) error {
64+
setterWasCalled = true
65+
if v, ok := m.(pcommon.Map); ok {
66+
v.CopyTo(tCtx)
67+
return nil
68+
}
69+
return errors.New("expected pcommon.Map")
70+
},
71+
}
72+
73+
exprFunc, err := deleteMatchingKeys(target, tt.pattern)
6874
assert.NoError(t, err)
6975

7076
_, err = exprFunc(nil, scenarioMap)
7177
assert.NoError(t, err)
78+
assert.True(t, setterWasCalled)
7279

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

8188
func Test_deleteMatchingKeys_bad_input(t *testing.T) {
8289
input := pcommon.NewValueInt(1)
83-
target := &ottl.StandardPMapGetter[any]{
84-
Getter: func(_ context.Context, tCtx any) (any, error) {
85-
return tCtx, nil
90+
target := &ottl.StandardPMapGetSetter[any]{
91+
Getter: func(_ context.Context, tCtx any) (pcommon.Map, error) {
92+
if v, ok := tCtx.(pcommon.Map); ok {
93+
return v, nil
94+
}
95+
return pcommon.Map{}, errors.New("expected pcommon.Map")
8696
},
8797
}
8898

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

96106
func Test_deleteMatchingKeys_get_nil(t *testing.T) {
97-
target := &ottl.StandardPMapGetter[any]{
98-
Getter: func(_ context.Context, tCtx any) (any, error) {
99-
return tCtx, nil
107+
target := &ottl.StandardPMapGetSetter[any]{
108+
Getter: func(_ context.Context, tCtx any) (pcommon.Map, error) {
109+
if v, ok := tCtx.(pcommon.Map); ok {
110+
return v, nil
111+
}
112+
return pcommon.Map{}, errors.New("expected pcommon.Map")
100113
},
101114
}
102115

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

109122
func Test_deleteMatchingKeys_invalid_pattern(t *testing.T) {
110-
target := &ottl.StandardPMapGetter[any]{
111-
Getter: func(_ context.Context, _ any) (any, error) {
123+
target := &ottl.StandardPMapGetSetter[any]{
124+
Getter: func(_ context.Context, _ any) (pcommon.Map, error) {
112125
t.Errorf("nothing should be received in this scenario")
113-
return nil, nil
126+
return pcommon.Map{}, nil
114127
},
115128
}
116129

pkg/ottl/ottlfuncs/func_flatten.go

Lines changed: 3 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,7 @@ import (
1616
)
1717

1818
type FlattenArguments[K any] struct {
19-
Target ottl.PMapGetter[K]
19+
Target ottl.PMapGetSetter[K]
2020
Prefix ottl.Optional[string]
2121
Depth ottl.Optional[int64]
2222
ResolveConflicts ottl.Optional[bool]
@@ -43,7 +43,7 @@ func createFlattenFunction[K any](_ ottl.FunctionContext, oArgs ottl.Arguments)
4343
return flatten(args.Target, args.Prefix, args.Depth, args.ResolveConflicts)
4444
}
4545

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

7171
flattenData := initFlattenData(resolveConflict, depth)
7272
flattenData.flattenMap(m, prefix, 0)
73-
flattenData.result.MoveTo(m)
74-
75-
return nil, nil
73+
return nil, target.Set(ctx, tCtx, flattenData.result)
7674
}, nil
7775
}
7876

0 commit comments

Comments
 (0)