From 587b910623bcb50fe0c0023341263766682223ec Mon Sep 17 00:00:00 2001 From: Peter Deng Date: Fri, 18 Aug 2023 10:41:16 +0800 Subject: [PATCH 1/4] use IntGetter argument for Substring function --- .chloggen/ottl-substring-intgetter.yaml | 27 ++++++ pkg/ottl/ottlfuncs/func_substring.go | 27 ++++-- pkg/ottl/ottlfuncs/func_substring_test.go | 113 +++++++++++++++++----- 3 files changed, 134 insertions(+), 33 deletions(-) create mode 100644 .chloggen/ottl-substring-intgetter.yaml diff --git a/.chloggen/ottl-substring-intgetter.yaml b/.chloggen/ottl-substring-intgetter.yaml new file mode 100644 index 000000000000..38ebaca18336 --- /dev/null +++ b/.chloggen/ottl-substring-intgetter.yaml @@ -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: use IntGetter argument for Substring function + +# Mandatory: One or more tracking issues related to the change. You can use the PR number here if no issue exists. +issues: [25852] + +# (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: [] diff --git a/pkg/ottl/ottlfuncs/func_substring.go b/pkg/ottl/ottlfuncs/func_substring.go index 43eedf9b2048..fb12e95f2b78 100644 --- a/pkg/ottl/ottlfuncs/func_substring.go +++ b/pkg/ottl/ottlfuncs/func_substring.go @@ -12,8 +12,8 @@ import ( type SubstringArguments[K any] struct { Target ottl.StringGetter[K] `ottlarg:"0"` - Start int64 `ottlarg:"1"` - Length int64 `ottlarg:"2"` + Start ottl.IntGetter[K] `ottlarg:"1"` + Length ottl.IntGetter[K] `ottlarg:"2"` } func NewSubstringFactory[K any]() ottl.Factory[K] { @@ -30,15 +30,22 @@ func createSubstringFunction[K any](_ ottl.FunctionContext, oArgs ottl.Arguments return substring(args.Target, args.Start, args.Length) } -func substring[K any](target ottl.StringGetter[K], start int64, length int64) (ottl.ExprFunc[K], error) { - if start < 0 { - return nil, fmt.Errorf("invalid start for substring function, %d cannot be negative", start) - } - if length <= 0 { - return nil, fmt.Errorf("invalid length for substring function, %d cannot be negative or zero", length) - } - +func substring[K any](target ottl.StringGetter[K], startGetter ottl.IntGetter[K], lengthGetter ottl.IntGetter[K]) (ottl.ExprFunc[K], error) { return func(ctx context.Context, tCtx K) (interface{}, error) { + start, err := startGetter.Get(ctx, tCtx) + if err != nil { + return nil, err + } + if start < 0 { + return nil, fmt.Errorf("invalid start for substring function, %d cannot be negative", start) + } + length, err := lengthGetter.Get(ctx, tCtx) + if err != nil { + return nil, err + } + if length <= 0 { + return nil, fmt.Errorf("invalid length for substring function, %d cannot be negative or zero", length) + } val, err := target.Get(ctx, tCtx) if err != nil { return nil, err diff --git a/pkg/ottl/ottlfuncs/func_substring_test.go b/pkg/ottl/ottlfuncs/func_substring_test.go index 9e1580b20e83..bc4a35750cf8 100644 --- a/pkg/ottl/ottlfuncs/func_substring_test.go +++ b/pkg/ottl/ottlfuncs/func_substring_test.go @@ -16,8 +16,8 @@ func Test_substring(t *testing.T) { tests := []struct { name string target ottl.StringGetter[interface{}] - start int64 - length int64 + start ottl.IntGetter[interface{}] + length ottl.IntGetter[interface{}] expected interface{} }{ { @@ -27,8 +27,16 @@ func Test_substring(t *testing.T) { return "123456789", nil }, }, - start: 3, - length: 3, + start: &ottl.StandardIntGetter[interface{}]{ + Getter: func(context.Context, interface{}) (interface{}, error) { + return int64(3), nil + }, + }, + length: &ottl.StandardIntGetter[interface{}]{ + Getter: func(context.Context, interface{}) (interface{}, error) { + return int64(3), nil + }, + }, expected: "456", }, { @@ -38,8 +46,16 @@ func Test_substring(t *testing.T) { return "123456789", nil }, }, - start: 0, - length: 9, + start: &ottl.StandardIntGetter[interface{}]{ + Getter: func(context.Context, interface{}) (interface{}, error) { + return int64(0), nil + }, + }, + length: &ottl.StandardIntGetter[interface{}]{ + Getter: func(context.Context, interface{}) (interface{}, error) { + return int64(9), nil + }, + }, expected: "123456789", }, } @@ -58,8 +74,8 @@ func Test_substring_validation(t *testing.T) { tests := []struct { name string target ottl.StringGetter[interface{}] - start int64 - length int64 + start ottl.IntGetter[interface{}] + length ottl.IntGetter[interface{}] }{ { name: "substring with result of empty string", @@ -68,8 +84,16 @@ func Test_substring_validation(t *testing.T) { return "123456789", nil }, }, - start: 0, - length: 0, + start: &ottl.StandardIntGetter[interface{}]{ + Getter: func(context.Context, interface{}) (interface{}, error) { + return int64(0), nil + }, + }, + length: &ottl.StandardIntGetter[interface{}]{ + Getter: func(context.Context, interface{}) (interface{}, error) { + return int64(0), nil + }, + }, }, { name: "substring with invalid start index", @@ -78,14 +102,25 @@ func Test_substring_validation(t *testing.T) { return "123456789", nil }, }, - start: -1, - length: 6, + start: &ottl.StandardIntGetter[interface{}]{ + Getter: func(context.Context, interface{}) (interface{}, error) { + return int64(-1), nil + }, + }, + length: &ottl.StandardIntGetter[interface{}]{ + Getter: func(context.Context, interface{}) (interface{}, error) { + return int64(6), nil + }, + }, }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - _, err := substring(tt.target, tt.start, tt.length) + exprFunc, err := substring(tt.target, tt.start, tt.length) + assert.NoError(t, err) + result, err := exprFunc(nil, nil) assert.Error(t, err) + assert.Nil(t, result) }) } } @@ -94,8 +129,8 @@ func Test_substring_error(t *testing.T) { tests := []struct { name string target ottl.StringGetter[interface{}] - start int64 - length int64 + start ottl.IntGetter[interface{}] + length ottl.IntGetter[interface{}] }{ { name: "substring empty string", @@ -104,8 +139,16 @@ func Test_substring_error(t *testing.T) { return "", nil }, }, - start: 3, - length: 6, + start: &ottl.StandardIntGetter[interface{}]{ + Getter: func(context.Context, interface{}) (interface{}, error) { + return int64(3), nil + }, + }, + length: &ottl.StandardIntGetter[interface{}]{ + Getter: func(context.Context, interface{}) (interface{}, error) { + return int64(6), nil + }, + }, }, { name: "substring with invalid length index", @@ -114,8 +157,16 @@ func Test_substring_error(t *testing.T) { return "123456789", nil }, }, - start: 3, - length: 20, + start: &ottl.StandardIntGetter[interface{}]{ + Getter: func(context.Context, interface{}) (interface{}, error) { + return int64(3), nil + }, + }, + length: &ottl.StandardIntGetter[interface{}]{ + Getter: func(context.Context, interface{}) (interface{}, error) { + return int64(20), nil + }, + }, }, { name: "substring non-string", @@ -124,8 +175,16 @@ func Test_substring_error(t *testing.T) { return 123456789, nil }, }, - start: 3, - length: 6, + start: &ottl.StandardIntGetter[interface{}]{ + Getter: func(context.Context, interface{}) (interface{}, error) { + return int64(3), nil + }, + }, + length: &ottl.StandardIntGetter[interface{}]{ + Getter: func(context.Context, interface{}) (interface{}, error) { + return int64(6), nil + }, + }, }, { name: "substring nil string", @@ -134,8 +193,16 @@ func Test_substring_error(t *testing.T) { return nil, nil }, }, - start: 3, - length: 6, + start: &ottl.StandardIntGetter[interface{}]{ + Getter: func(context.Context, interface{}) (interface{}, error) { + return int64(3), nil + }, + }, + length: &ottl.StandardIntGetter[interface{}]{ + Getter: func(context.Context, interface{}) (interface{}, error) { + return int64(6), nil + }, + }, }, } for _, tt := range tests { From d9c6480bfd1827dcaf24e9a46137a6fe261a7839 Mon Sep 17 00:00:00 2001 From: Peter Deng Date: Fri, 18 Aug 2023 11:09:11 +0800 Subject: [PATCH 2/4] change function signature to fix lint issue --- pkg/ottl/ottlfuncs/func_substring.go | 6 +++--- pkg/ottl/ottlfuncs/func_substring_test.go | 9 +++------ 2 files changed, 6 insertions(+), 9 deletions(-) diff --git a/pkg/ottl/ottlfuncs/func_substring.go b/pkg/ottl/ottlfuncs/func_substring.go index fb12e95f2b78..b1658a8dc608 100644 --- a/pkg/ottl/ottlfuncs/func_substring.go +++ b/pkg/ottl/ottlfuncs/func_substring.go @@ -27,10 +27,10 @@ func createSubstringFunction[K any](_ ottl.FunctionContext, oArgs ottl.Arguments return nil, fmt.Errorf("SubstringFactory args must be of type *SubstringArguments[K]") } - return substring(args.Target, args.Start, args.Length) + return substring(args.Target, args.Start, args.Length), nil } -func substring[K any](target ottl.StringGetter[K], startGetter ottl.IntGetter[K], lengthGetter ottl.IntGetter[K]) (ottl.ExprFunc[K], error) { +func substring[K any](target ottl.StringGetter[K], startGetter ottl.IntGetter[K], lengthGetter ottl.IntGetter[K]) ottl.ExprFunc[K] { return func(ctx context.Context, tCtx K) (interface{}, error) { start, err := startGetter.Get(ctx, tCtx) if err != nil { @@ -54,5 +54,5 @@ func substring[K any](target ottl.StringGetter[K], startGetter ottl.IntGetter[K] return nil, fmt.Errorf("invalid range for substring function, %d cannot be greater than the length of target string(%d)", start+length, len(val)) } return val[start : start+length], nil - }, nil + } } diff --git a/pkg/ottl/ottlfuncs/func_substring_test.go b/pkg/ottl/ottlfuncs/func_substring_test.go index bc4a35750cf8..1b659e9b6d76 100644 --- a/pkg/ottl/ottlfuncs/func_substring_test.go +++ b/pkg/ottl/ottlfuncs/func_substring_test.go @@ -61,8 +61,7 @@ func Test_substring(t *testing.T) { } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - exprFunc, err := substring(tt.target, tt.start, tt.length) - assert.NoError(t, err) + exprFunc := substring(tt.target, tt.start, tt.length) result, err := exprFunc(nil, nil) assert.NoError(t, err) assert.Equal(t, tt.expected, result) @@ -116,8 +115,7 @@ func Test_substring_validation(t *testing.T) { } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - exprFunc, err := substring(tt.target, tt.start, tt.length) - assert.NoError(t, err) + exprFunc := substring(tt.target, tt.start, tt.length) result, err := exprFunc(nil, nil) assert.Error(t, err) assert.Nil(t, result) @@ -207,8 +205,7 @@ func Test_substring_error(t *testing.T) { } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - exprFunc, err := substring(tt.target, tt.start, tt.length) - assert.NoError(t, err) + exprFunc := substring(tt.target, tt.start, tt.length) result, err := exprFunc(nil, nil) assert.Error(t, err) assert.Equal(t, nil, result) From 1f1a2e8dbb475c3fefa48c7ca053711e511ca9a7 Mon Sep 17 00:00:00 2001 From: Peter Deng Date: Fri, 18 Aug 2023 12:26:35 +0800 Subject: [PATCH 3/4] Update .chloggen/ottl-substring-intgetter.yaml Co-authored-by: Antoine Toulme --- .chloggen/ottl-substring-intgetter.yaml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.chloggen/ottl-substring-intgetter.yaml b/.chloggen/ottl-substring-intgetter.yaml index 38ebaca18336..7b8b099a5ffe 100644 --- a/.chloggen/ottl-substring-intgetter.yaml +++ b/.chloggen/ottl-substring-intgetter.yaml @@ -24,4 +24,4 @@ subtext: # 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: [] +change_logs: [api] From 03427c6e012c4ce7b045cad17369718ec155bd78 Mon Sep 17 00:00:00 2001 From: Peter Deng Date: Fri, 18 Aug 2023 12:30:52 +0800 Subject: [PATCH 4/4] Update ottl-substring-intgetter.yaml --- .chloggen/ottl-substring-intgetter.yaml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.chloggen/ottl-substring-intgetter.yaml b/.chloggen/ottl-substring-intgetter.yaml index 7b8b099a5ffe..09803104151c 100644 --- a/.chloggen/ottl-substring-intgetter.yaml +++ b/.chloggen/ottl-substring-intgetter.yaml @@ -1,7 +1,7 @@ # Use this changelog template to create an entry for release notes. # One of 'breaking', 'deprecation', 'new_component', 'enhancement', 'bug_fix' -change_type: enhancement +change_type: breaking # The name of the component, or a single word describing the area of concern, (e.g. filelogreceiver) component: pkg/ottl