Skip to content
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.

Sign up for GitHub

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

[pkg/ottl] Support dynamic indexing #36719

Open
wants to merge 20 commits into
base: main
Choose a base branch
from
Open

[pkg/ottl] Support dynamic indexing #36719

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
20 commits
Select commit Hold shift + click to select a range
d965519
dollar version
odubajDT Dec 12, 2024
02b72ec
different approach in parser
odubajDT Dec 13, 2024
fda69bc
working version
odubajDT Dec 16, 2024
97b369c
polish
odubajDT Dec 16, 2024
bc1dc8e
remove dead code
odubajDT Dec 16, 2024
28ffc31
revert tests
odubajDT Dec 16, 2024
ee9878d
revert tests
odubajDT Dec 16, 2024
db9ec8b
polish
odubajDT Dec 17, 2024
09a6ab2
support more use cases
odubajDT Dec 17, 2024
b3bc495
fix lint
odubajDT Dec 17, 2024
2a2e8c4
fix lint
odubajDT Dec 17, 2024
31c45d6
working tests
odubajDT Dec 18, 2024
63b2fe7
refactor
odubajDT Dec 18, 2024
0cb42da
fix typo
odubajDT Dec 18, 2024
d8af6b1
make generate
odubajDT Dec 18, 2024
dde8935
support expression as attribute key in parser
odubajDT Dec 18, 2024
51c0d1f
support converters as attribute keys
odubajDT Dec 19, 2024
b8c4622
add chlog
odubajDT Dec 19, 2024
b62391b
polishing of implementation
odubajDT Dec 19, 2024
5e92cc6
add more e2e tests
odubajDT Dec 19, 2024
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/indexing-pkg-ottl.yaml
View file
Open in desktop
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: enhancement

# 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: "Support dynamic indexing of maps and slices."

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

# (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: []
28 changes: 26 additions & 2 deletions pkg/ottl/contexts/internal/map.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,11 @@ func GetMapValue[K any](ctx context.Context, tCtx K, m pcommon.Map, keys []ottl.
return nil, err
}
if s == nil {
return nil, fmt.Errorf("non-string indexing is not supported")
resString, err := FetchValueFromExpression[K, string](ctx, tCtx, keys[0])
if err != nil {
return nil, fmt.Errorf("non-string indexing is not supported")
}
s = resString
}

val, ok := m.Get(*s)
Expand All @@ -43,7 +47,11 @@ func SetMapValue[K any](ctx context.Context, tCtx K, m pcommon.Map, keys []ottl.
return err
}
if s == nil {
return fmt.Errorf("non-string indexing is not supported")
resString, err := FetchValueFromExpression[K, string](ctx, tCtx, keys[0])
if err != nil {
return fmt.Errorf("non-string indexing is not supported")
}
s = resString
}

currentValue, ok := m.Get(*s)
Expand All @@ -52,3 +60,19 @@ func SetMapValue[K any](ctx context.Context, tCtx K, m pcommon.Map, keys []ottl.
}
return setIndexableValue[K](ctx, tCtx, currentValue, val, keys[1:])
}

func FetchValueFromExpression[K any, T int64 | string](ctx context.Context, tCtx K, key ottl.Key[K]) (*T, error) {
p, err := key.ExpressionGetter(ctx, tCtx)
if err != nil {
return nil, err
}
res, err := p.Get(ctx, tCtx)
if err != nil {
return nil, err
}
resVal, ok := res.(T)
if !ok {
return nil, fmt.Errorf("casting not successful")
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
return nil, fmt.Errorf("casting not successful")
return nil, fmt.Errorf("could not resolve key for map/slice, expecting `int64` or `string` but got %T", res)

It would be nice to make this more explicit what's happening here, if a user ever hits this error we should try to give them all the relevant information we have. If possible, it would also be nice to print the type of T here so we can make it clear what type of key was expected.

}
return &resVal, nil
}
38 changes: 38 additions & 0 deletions pkg/ottl/contexts/internal/map_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,14 @@ import (
)

func Test_GetMapValue_Invalid(t *testing.T) {
getSetter := &ottl.StandardGetSetter[any]{
Getter: func(_ context.Context, tCtx any) (any, error) {
return nil, nil
},
Setter: func(_ context.Context, tCtx any, val any) error {
return nil
},
}
tests := []struct {
name string
keys []ottl.Key[any]
Expand All @@ -26,6 +34,7 @@ func Test_GetMapValue_Invalid(t *testing.T) {
keys: []ottl.Key[any]{
&TestKey[any]{
I: ottltest.Intp(0),
P: getSetter,
},
},
err: fmt.Errorf("non-string indexing is not supported"),
Expand All @@ -35,9 +44,11 @@ func Test_GetMapValue_Invalid(t *testing.T) {
keys: []ottl.Key[any]{
&TestKey[any]{
S: ottltest.Strp("map"),
P: getSetter,
},
&TestKey[any]{
I: ottltest.Intp(0),
P: getSetter,
},
},
err: fmt.Errorf("map must be indexed by a string"),
Expand All @@ -47,9 +58,11 @@ func Test_GetMapValue_Invalid(t *testing.T) {
keys: []ottl.Key[any]{
&TestKey[any]{
S: ottltest.Strp("slice"),
P: getSetter,
},
&TestKey[any]{
S: ottltest.Strp("invalid"),
P: getSetter,
},
},
err: fmt.Errorf("slice must be indexed by an int"),
Expand All @@ -59,9 +72,11 @@ func Test_GetMapValue_Invalid(t *testing.T) {
keys: []ottl.Key[any]{
&TestKey[any]{
S: ottltest.Strp("slice"),
P: getSetter,
},
&TestKey[any]{
I: ottltest.Intp(1),
P: getSetter,
},
},
err: fmt.Errorf("index 1 out of bounds"),
Expand All @@ -71,9 +86,11 @@ func Test_GetMapValue_Invalid(t *testing.T) {
keys: []ottl.Key[any]{
&TestKey[any]{
S: ottltest.Strp("slice"),
P: getSetter,
},
&TestKey[any]{
I: ottltest.Intp(-1),
P: getSetter,
},
},
err: fmt.Errorf("index -1 out of bounds"),
Expand All @@ -83,9 +100,11 @@ func Test_GetMapValue_Invalid(t *testing.T) {
keys: []ottl.Key[any]{
&TestKey[any]{
S: ottltest.Strp("string"),
P: getSetter,
},
&TestKey[any]{
S: ottltest.Strp("string"),
P: getSetter,
},
},
err: fmt.Errorf("type Str does not support string indexing"),
Expand Down Expand Up @@ -129,6 +148,14 @@ func Test_GetMapValue_NilKey(t *testing.T) {
}

func Test_SetMapValue_Invalid(t *testing.T) {
getSetter := &ottl.StandardGetSetter[any]{
Getter: func(_ context.Context, tCtx any) (any, error) {
return nil, nil
},
Setter: func(_ context.Context, tCtx any, val any) error {
return nil
},
}
tests := []struct {
name string
keys []ottl.Key[any]
Expand All @@ -139,6 +166,7 @@ func Test_SetMapValue_Invalid(t *testing.T) {
keys: []ottl.Key[any]{
&TestKey[any]{
I: ottltest.Intp(0),
P: getSetter,
},
},
err: fmt.Errorf("non-string indexing is not supported"),
Expand All @@ -148,9 +176,11 @@ func Test_SetMapValue_Invalid(t *testing.T) {
keys: []ottl.Key[any]{
&TestKey[any]{
S: ottltest.Strp("map"),
P: getSetter,
},
&TestKey[any]{
I: ottltest.Intp(0),
P: getSetter,
},
},
err: fmt.Errorf("map must be indexed by a string"),
Expand All @@ -160,9 +190,11 @@ func Test_SetMapValue_Invalid(t *testing.T) {
keys: []ottl.Key[any]{
&TestKey[any]{
S: ottltest.Strp("slice"),
P: getSetter,
},
&TestKey[any]{
S: ottltest.Strp("map"),
P: getSetter,
},
},
err: fmt.Errorf("slice must be indexed by an int"),
Expand All @@ -172,9 +204,11 @@ func Test_SetMapValue_Invalid(t *testing.T) {
keys: []ottl.Key[any]{
&TestKey[any]{
S: ottltest.Strp("slice"),
P: getSetter,
},
&TestKey[any]{
I: ottltest.Intp(1),
P: getSetter,
},
},
err: fmt.Errorf("index 1 out of bounds"),
Expand All @@ -184,9 +218,11 @@ func Test_SetMapValue_Invalid(t *testing.T) {
keys: []ottl.Key[any]{
&TestKey[any]{
S: ottltest.Strp("slice"),
P: getSetter,
},
&TestKey[any]{
I: ottltest.Intp(-1),
P: getSetter,
},
},
err: fmt.Errorf("index -1 out of bounds"),
Expand All @@ -196,9 +232,11 @@ func Test_SetMapValue_Invalid(t *testing.T) {
keys: []ottl.Key[any]{
&TestKey[any]{
S: ottltest.Strp("string"),
P: getSetter,
},
&TestKey[any]{
S: ottltest.Strp("string"),
P: getSetter,
},
},
err: fmt.Errorf("type Str does not support string indexing"),
Expand Down
5 changes: 5 additions & 0 deletions pkg/ottl/contexts/internal/path.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,7 @@ var _ ottl.Key[any] = &TestKey[any]{}
type TestKey[K any] struct {
S *string
I *int64
P ottl.Getter[K]
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
P ottl.Getter[K]
G ottl.Getter[K]

Same here, I think a G for Getter makes the most sense.

}

func (k *TestKey[K]) String(_ context.Context, _ K) (*string, error) {
Expand All @@ -55,3 +56,7 @@ func (k *TestKey[K]) String(_ context.Context, _ K) (*string, error) {
func (k *TestKey[K]) Int(_ context.Context, _ K) (*int64, error) {
return k.I, nil
}

func (k *TestKey[K]) ExpressionGetter(_ context.Context, _ K) (ottl.Getter[K], error) {
return k.P, nil
}
12 changes: 10 additions & 2 deletions pkg/ottl/contexts/internal/slice.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,11 @@ func GetSliceValue[K any](ctx context.Context, tCtx K, s pcommon.Slice, keys []o
return nil, err
}
if i == nil {
return nil, fmt.Errorf("non-integer indexing is not supported")
resInt, err := FetchValueFromExpression[K, int64](ctx, tCtx, keys[0])
if err != nil {
return nil, fmt.Errorf("non-integer indexing is not supported")
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
return nil, fmt.Errorf("non-integer indexing is not supported")
return nil, fmt.Errorf("unable to resolve an integer index: %w", err)

I think at a minimum we should wrap the underlying error, but we should also consider rewording this message, since I'm not sure a failure in FetchValueFromExpression necessarily means the user tried to use a non-integer value to index a slice. For example, they could have tried to retrieve attributes["my_int"] and the attribute was nil.

}
i = resInt
}

idx := int(*i)
Expand All @@ -44,7 +48,11 @@ func SetSliceValue[K any](ctx context.Context, tCtx K, s pcommon.Slice, keys []o
return err
}
if i == nil {
return fmt.Errorf("non-integer indexing is not supported")
resInt, err := FetchValueFromExpression[K, int64](ctx, tCtx, keys[0])
if err != nil {
return fmt.Errorf("non-integer indexing is not supported")
}
i = resInt
}

idx := int(*i)
Expand Down
26 changes: 26 additions & 0 deletions pkg/ottl/contexts/internal/slice_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,14 @@ import (
)

func Test_GetSliceValue_Invalid(t *testing.T) {
getSetter := &ottl.StandardGetSetter[any]{
Getter: func(_ context.Context, tCtx any) (any, error) {
return nil, nil
},
Setter: func(_ context.Context, tCtx any, val any) error {
return nil
},
}
tests := []struct {
name string
keys []ottl.Key[any]
Expand All @@ -26,6 +34,7 @@ func Test_GetSliceValue_Invalid(t *testing.T) {
keys: []ottl.Key[any]{
&TestKey[any]{
S: ottltest.Strp("key"),
P: getSetter,
},
},
err: fmt.Errorf("non-integer indexing is not supported"),
Expand All @@ -35,6 +44,7 @@ func Test_GetSliceValue_Invalid(t *testing.T) {
keys: []ottl.Key[any]{
&TestKey[any]{
I: ottltest.Intp(1),
P: getSetter,
},
},
err: fmt.Errorf("index 1 out of bounds"),
Expand All @@ -44,6 +54,7 @@ func Test_GetSliceValue_Invalid(t *testing.T) {
keys: []ottl.Key[any]{
&TestKey[any]{
I: ottltest.Intp(-1),
P: getSetter,
},
},
err: fmt.Errorf("index -1 out of bounds"),
Expand All @@ -53,9 +64,11 @@ func Test_GetSliceValue_Invalid(t *testing.T) {
keys: []ottl.Key[any]{
&TestKey[any]{
I: ottltest.Intp(0),
P: getSetter,
},
&TestKey[any]{
S: ottltest.Strp("string"),
P: getSetter,
},
},
err: fmt.Errorf("type Str does not support string indexing"),
Expand All @@ -79,6 +92,14 @@ func Test_GetSliceValue_NilKey(t *testing.T) {
}

func Test_SetSliceValue_Invalid(t *testing.T) {
getSetter := &ottl.StandardGetSetter[any]{
Getter: func(_ context.Context, tCtx any) (any, error) {
return nil, nil
},
Setter: func(_ context.Context, tCtx any, val any) error {
return nil
},
}
tests := []struct {
name string
keys []ottl.Key[any]
Expand All @@ -89,6 +110,7 @@ func Test_SetSliceValue_Invalid(t *testing.T) {
keys: []ottl.Key[any]{
&TestKey[any]{
S: ottltest.Strp("key"),
P: getSetter,
},
},
err: fmt.Errorf("non-integer indexing is not supported"),
Expand All @@ -98,6 +120,7 @@ func Test_SetSliceValue_Invalid(t *testing.T) {
keys: []ottl.Key[any]{
&TestKey[any]{
I: ottltest.Intp(1),
P: getSetter,
},
},
err: fmt.Errorf("index 1 out of bounds"),
Expand All @@ -107,6 +130,7 @@ func Test_SetSliceValue_Invalid(t *testing.T) {
keys: []ottl.Key[any]{
&TestKey[any]{
I: ottltest.Intp(-1),
P: getSetter,
},
},
err: fmt.Errorf("index -1 out of bounds"),
Expand All @@ -116,9 +140,11 @@ func Test_SetSliceValue_Invalid(t *testing.T) {
keys: []ottl.Key[any]{
&TestKey[any]{
I: ottltest.Intp(0),
P: getSetter,
},
&TestKey[any]{
S: ottltest.Strp("string"),
P: getSetter,
},
},
err: fmt.Errorf("type Str does not support string indexing"),
Expand Down
Loading
Loading