From c79f1b0060949267181bb6ee4ffc7eb5bd63518a Mon Sep 17 00:00:00 2001 From: Tyler Helmuth <12352919+TylerHelmuth@users.noreply.github.com> Date: Wed, 25 Jan 2023 14:55:22 -0700 Subject: [PATCH 1/8] Experiment with StringGetter --- pkg/ottl/expression.go | 23 +++++ pkg/ottl/functions.go | 6 ++ pkg/ottl/functions_test.go | 24 +++++- pkg/ottl/ottlfuncs/func_is_match.go | 41 +-------- pkg/ottl/ottlfuncs/func_is_match_test.go | 103 ++++++++++++----------- 5 files changed, 105 insertions(+), 92 deletions(-) diff --git a/pkg/ottl/expression.go b/pkg/ottl/expression.go index fa79b7c04271..2b482bde53b7 100644 --- a/pkg/ottl/expression.go +++ b/pkg/ottl/expression.go @@ -42,6 +42,10 @@ type GetSetter[K any] interface { Setter[K] } +type StringGetter[K any] interface { + Get(ctx context.Context, tCtx K) (*string, error) +} + type StandardGetSetter[K any] struct { Getter func(ctx context.Context, tCx K) (interface{}, error) Setter func(ctx context.Context, tCx K, val interface{}) error @@ -89,6 +93,25 @@ func (l *listGetter[K]) Get(ctx context.Context, tCtx K) (interface{}, error) { return evaluated, nil } +type StandardStringGetter[K any] struct { + Getter func(ctx context.Context, tCx K) (interface{}, error) +} + +func (g StandardStringGetter[K]) Get(ctx context.Context, tCtx K) (*string, error) { + val, err := g.Getter(ctx, tCtx) + if err != nil { + return nil, err + } + if val == nil { + return nil, nil + } + strVal, ok := val.(string) + if !ok { + return nil, fmt.Errorf("value was not a string") + } + return &strVal, nil +} + func (p *Parser[K]) newGetter(val value) (Getter[K], error) { if val.IsNil != nil && *val.IsNil { return &literal[K]{value: nil}, nil diff --git a/pkg/ottl/functions.go b/pkg/ottl/functions.go index 615d1f64237c..cb66357d76cc 100644 --- a/pkg/ottl/functions.go +++ b/pkg/ottl/functions.go @@ -151,6 +151,12 @@ func (p *Parser[K]) buildArg(argVal value, argType reflect.Type) (any, error) { return nil, err } return arg, nil + case strings.HasPrefix(name, "StringGetter"): + arg, err := p.newGetter(argVal) + if err != nil { + return nil, err + } + return StandardStringGetter[K]{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 ba37e37cdfc9..e0f2b21ec8dc 100644 --- a/pkg/ottl/functions_test.go +++ b/pkg/ottl/functions_test.go @@ -462,7 +462,7 @@ func Test_NewFunctionCall(t *testing.T) { { Literal: &mathExprLiteral{ Converter: &converter{ - Function: "Testing_getter", + Function: "testing_getter", Arguments: []value{ { Literal: &mathExprLiteral{ @@ -599,7 +599,7 @@ func Test_NewFunctionCall(t *testing.T) { { Literal: &mathExprLiteral{ Converter: &converter{ - Function: "Testing_getter", + Function: "testing_getter", Arguments: []value{ { Literal: &mathExprLiteral{ @@ -623,6 +623,18 @@ func Test_NewFunctionCall(t *testing.T) { }, want: nil, }, + { + name: "stringgetter arg", + inv: invocation{ + Function: "testing_stringgetter", + Arguments: []value{ + { + String: ottltest.Strp("test"), + }, + }, + }, + want: nil, + }, { name: "string arg", inv: invocation{ @@ -872,6 +884,12 @@ func functionWithGetter(Getter[interface{}]) (ExprFunc[interface{}], error) { }, nil } +func functionWithStringGetter(StringGetter[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 @@ -943,7 +961,7 @@ func defaultFunctionsForTests() map[string]interface{} { functions["testing_setter"] = functionWithSetter functions["testing_getsetter"] = functionWithGetSetter functions["testing_getter"] = functionWithGetter - functions["Testing_getter"] = functionWithGetter + functions["testing_stringgetter"] = functionWithStringGetter functions["testing_string"] = functionWithString functions["testing_float"] = functionWithFloat functions["testing_int"] = functionWithInt diff --git a/pkg/ottl/ottlfuncs/func_is_match.go b/pkg/ottl/ottlfuncs/func_is_match.go index c46bd3084779..7681f3e23338 100644 --- a/pkg/ottl/ottlfuncs/func_is_match.go +++ b/pkg/ottl/ottlfuncs/func_is_match.go @@ -16,19 +16,12 @@ package ottlfuncs // import "github.com/open-telemetry/opentelemetry-collector-c import ( "context" - "encoding/base64" - "errors" "fmt" - "regexp" - "strconv" - - jsoniter "github.com/json-iterator/go" - "go.opentelemetry.io/collector/pdata/pcommon" - "github.com/open-telemetry/opentelemetry-collector-contrib/pkg/ottl" + "regexp" ) -func IsMatch[K any](target ottl.Getter[K], pattern string) (ottl.ExprFunc[K], error) { +func IsMatch[K any](target ottl.StringGetter[K], pattern string) (ottl.ExprFunc[K], error) { compiledPattern, err := regexp.Compile(pattern) if err != nil { return nil, fmt.Errorf("the pattern supplied to IsMatch is not a valid regexp pattern: %w", err) @@ -41,34 +34,6 @@ func IsMatch[K any](target ottl.Getter[K], pattern string) (ottl.ExprFunc[K], er if val == nil { return false, nil } - - switch v := val.(type) { - case string: - return compiledPattern.MatchString(v), nil - case bool: - return compiledPattern.MatchString(strconv.FormatBool(v)), nil - case int64: - return compiledPattern.MatchString(strconv.FormatInt(v, 10)), nil - case float64: - return compiledPattern.MatchString(strconv.FormatFloat(v, 'f', -1, 64)), nil - case []byte: - return compiledPattern.MatchString(base64.StdEncoding.EncodeToString(v)), nil - case pcommon.Map: - result, err := jsoniter.MarshalToString(v.AsRaw()) - if err != nil { - return nil, err - } - return compiledPattern.MatchString(result), nil - case pcommon.Slice: - result, err := jsoniter.MarshalToString(v.AsRaw()) - if err != nil { - return nil, err - } - return compiledPattern.MatchString(result), nil - case pcommon.Value: - return compiledPattern.MatchString(v.AsString()), nil - default: - return nil, errors.New("unsupported type") - } + return compiledPattern.MatchString(*val), nil }, nil } diff --git a/pkg/ottl/ottlfuncs/func_is_match_test.go b/pkg/ottl/ottlfuncs/func_is_match_test.go index 8c18512a94af..f1ae88be4457 100644 --- a/pkg/ottl/ottlfuncs/func_is_match_test.go +++ b/pkg/ottl/ottlfuncs/func_is_match_test.go @@ -28,13 +28,13 @@ import ( func Test_isMatch(t *testing.T) { tests := []struct { name string - target ottl.Getter[interface{}] + target ottl.StringGetter[interface{}] pattern string expected bool }{ { name: "replace match true", - target: &ottl.StandardGetSetter[interface{}]{ + target: &ottl.StandardStringGetter[interface{}]{ Getter: func(ctx context.Context, tCtx interface{}) (interface{}, error) { return "hello world", nil }, @@ -44,7 +44,7 @@ func Test_isMatch(t *testing.T) { }, { name: "replace match false", - target: &ottl.StandardGetSetter[interface{}]{ + target: &ottl.StandardStringGetter[interface{}]{ Getter: func(ctx context.Context, tCtx interface{}) (interface{}, error) { return "goodbye world", nil }, @@ -54,7 +54,7 @@ func Test_isMatch(t *testing.T) { }, { name: "replace match complex", - target: &ottl.StandardGetSetter[interface{}]{ + target: &ottl.StandardStringGetter[interface{}]{ Getter: func(ctx context.Context, tCtx interface{}) (interface{}, error) { return "-12.001", nil }, @@ -62,89 +62,90 @@ func Test_isMatch(t *testing.T) { pattern: "[-+]?\\d*\\.\\d+([eE][-+]?\\d+)?", expected: true, }, + { + name: "nil target", + target: &ottl.StandardStringGetter[interface{}]{ + Getter: func(ctx context.Context, tCtx interface{}) (interface{}, error) { + return nil, nil + }, + }, + pattern: "impossible to match", + expected: false, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + exprFunc, err := IsMatch(tt.target, tt.pattern) + assert.NoError(t, err) + result, err := exprFunc(context.Background(), nil) + assert.NoError(t, err) + assert.Equal(t, tt.expected, result) + }) + } +} + +func Test_isMatch_validation(t *testing.T) { + target := &ottl.StandardStringGetter[interface{}]{ + Getter: func(ctx context.Context, tCtx interface{}) (interface{}, error) { + return "anything", nil + }, + } + _, err := IsMatch[interface{}](target, "\\K") + require.Error(t, err) +} + +func Test_isMatch_error(t *testing.T) { + tests := []struct { + name string + target ottl.StringGetter[interface{}] + pattern string + }{ { name: "target bool", - target: &ottl.StandardGetSetter[interface{}]{ + target: &ottl.StandardStringGetter[interface{}]{ Getter: func(ctx context.Context, tCtx interface{}) (interface{}, error) { return true, nil }, }, - pattern: "true", - expected: true, + pattern: "true", }, { name: "target int", - target: &ottl.StandardGetSetter[interface{}]{ + target: &ottl.StandardStringGetter[interface{}]{ Getter: func(ctx context.Context, tCtx interface{}) (interface{}, error) { return int64(1), nil }, }, - pattern: `\d`, - expected: true, + pattern: `\d`, }, { name: "target float", - target: &ottl.StandardGetSetter[interface{}]{ + target: &ottl.StandardStringGetter[interface{}]{ Getter: func(ctx context.Context, tCtx interface{}) (interface{}, error) { return 1.1, nil }, }, - pattern: `\d\.\d`, - expected: true, + pattern: `\d\.\d`, }, { name: "target pcommon.Value", - target: &ottl.StandardGetSetter[interface{}]{ + target: &ottl.StandardStringGetter[interface{}]{ Getter: func(ctx context.Context, tCtx interface{}) (interface{}, error) { v := pcommon.NewValueEmpty() v.SetStr("test") return v, nil }, }, - pattern: `test`, - expected: true, - }, - { - name: "nil target", - target: &ottl.StandardGetSetter[interface{}]{ - Getter: func(ctx context.Context, tCtx interface{}) (interface{}, error) { - return nil, nil - }, - }, - pattern: "impossible to match", - expected: false, + pattern: `test`, }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { exprFunc, err := IsMatch(tt.target, tt.pattern) assert.NoError(t, err) - result, err := exprFunc(context.Background(), nil) - assert.NoError(t, err) - assert.Equal(t, tt.expected, result) + _, err = exprFunc(context.Background(), nil) + require.Error(t, err) }) } -} - -func Test_isMatch_validation(t *testing.T) { - target := &ottl.StandardGetSetter[interface{}]{ - Getter: func(ctx context.Context, tCtx interface{}) (interface{}, error) { - return "anything", nil - }, - } - _, err := IsMatch[interface{}](target, "\\K") - require.Error(t, err) -} -func Test_isMatch_error(t *testing.T) { - target := &ottl.StandardGetSetter[interface{}]{ - Getter: func(ctx context.Context, tCtx interface{}) (interface{}, error) { - v := ottl.Path{} - return v, nil - }, - } - exprFunc, err := IsMatch[interface{}](target, "test") - assert.NoError(t, err) - _, err = exprFunc(context.Background(), nil) - require.Error(t, err) } From 563be8cfac4f4f35424bd950f3dd051886c85b5b Mon Sep 17 00:00:00 2001 From: Tyler Helmuth <12352919+TylerHelmuth@users.noreply.github.com> Date: Tue, 31 Jan 2023 09:07:09 -0700 Subject: [PATCH 2/8] Add changelog entry --- .chloggen/ottl-type-specific-getter.yaml | 16 ++++ pkg/ottl/ottlfuncs/func_is_match.go | 41 ++++++++- pkg/ottl/ottlfuncs/func_is_match_test.go | 103 +++++++++++------------ 3 files changed, 105 insertions(+), 55 deletions(-) create mode 100755 .chloggen/ottl-type-specific-getter.yaml diff --git a/.chloggen/ottl-type-specific-getter.yaml b/.chloggen/ottl-type-specific-getter.yaml new file mode 100755 index 000000000000..8101ffd7e7fb --- /dev/null +++ b/.chloggen/ottl-type-specific-getter.yaml @@ -0,0 +1,16 @@ +# 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: Introduce the concept of type-specific Getters that help simplify type assertions in functions. + +# One or more tracking issues related to the change +issues: [18042] + +# (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: diff --git a/pkg/ottl/ottlfuncs/func_is_match.go b/pkg/ottl/ottlfuncs/func_is_match.go index 7681f3e23338..c46bd3084779 100644 --- a/pkg/ottl/ottlfuncs/func_is_match.go +++ b/pkg/ottl/ottlfuncs/func_is_match.go @@ -16,12 +16,19 @@ package ottlfuncs // import "github.com/open-telemetry/opentelemetry-collector-c import ( "context" + "encoding/base64" + "errors" "fmt" - "github.com/open-telemetry/opentelemetry-collector-contrib/pkg/ottl" "regexp" + "strconv" + + jsoniter "github.com/json-iterator/go" + "go.opentelemetry.io/collector/pdata/pcommon" + + "github.com/open-telemetry/opentelemetry-collector-contrib/pkg/ottl" ) -func IsMatch[K any](target ottl.StringGetter[K], pattern string) (ottl.ExprFunc[K], error) { +func IsMatch[K any](target ottl.Getter[K], pattern string) (ottl.ExprFunc[K], error) { compiledPattern, err := regexp.Compile(pattern) if err != nil { return nil, fmt.Errorf("the pattern supplied to IsMatch is not a valid regexp pattern: %w", err) @@ -34,6 +41,34 @@ func IsMatch[K any](target ottl.StringGetter[K], pattern string) (ottl.ExprFunc[ if val == nil { return false, nil } - return compiledPattern.MatchString(*val), nil + + switch v := val.(type) { + case string: + return compiledPattern.MatchString(v), nil + case bool: + return compiledPattern.MatchString(strconv.FormatBool(v)), nil + case int64: + return compiledPattern.MatchString(strconv.FormatInt(v, 10)), nil + case float64: + return compiledPattern.MatchString(strconv.FormatFloat(v, 'f', -1, 64)), nil + case []byte: + return compiledPattern.MatchString(base64.StdEncoding.EncodeToString(v)), nil + case pcommon.Map: + result, err := jsoniter.MarshalToString(v.AsRaw()) + if err != nil { + return nil, err + } + return compiledPattern.MatchString(result), nil + case pcommon.Slice: + result, err := jsoniter.MarshalToString(v.AsRaw()) + if err != nil { + return nil, err + } + return compiledPattern.MatchString(result), nil + case pcommon.Value: + return compiledPattern.MatchString(v.AsString()), nil + default: + return nil, errors.New("unsupported type") + } }, nil } diff --git a/pkg/ottl/ottlfuncs/func_is_match_test.go b/pkg/ottl/ottlfuncs/func_is_match_test.go index f1ae88be4457..8c18512a94af 100644 --- a/pkg/ottl/ottlfuncs/func_is_match_test.go +++ b/pkg/ottl/ottlfuncs/func_is_match_test.go @@ -28,13 +28,13 @@ import ( func Test_isMatch(t *testing.T) { tests := []struct { name string - target ottl.StringGetter[interface{}] + target ottl.Getter[interface{}] pattern string expected bool }{ { name: "replace match true", - target: &ottl.StandardStringGetter[interface{}]{ + target: &ottl.StandardGetSetter[interface{}]{ Getter: func(ctx context.Context, tCtx interface{}) (interface{}, error) { return "hello world", nil }, @@ -44,7 +44,7 @@ func Test_isMatch(t *testing.T) { }, { name: "replace match false", - target: &ottl.StandardStringGetter[interface{}]{ + target: &ottl.StandardGetSetter[interface{}]{ Getter: func(ctx context.Context, tCtx interface{}) (interface{}, error) { return "goodbye world", nil }, @@ -54,7 +54,7 @@ func Test_isMatch(t *testing.T) { }, { name: "replace match complex", - target: &ottl.StandardStringGetter[interface{}]{ + target: &ottl.StandardGetSetter[interface{}]{ Getter: func(ctx context.Context, tCtx interface{}) (interface{}, error) { return "-12.001", nil }, @@ -62,90 +62,89 @@ func Test_isMatch(t *testing.T) { pattern: "[-+]?\\d*\\.\\d+([eE][-+]?\\d+)?", expected: true, }, - { - name: "nil target", - target: &ottl.StandardStringGetter[interface{}]{ - Getter: func(ctx context.Context, tCtx interface{}) (interface{}, error) { - return nil, nil - }, - }, - pattern: "impossible to match", - expected: false, - }, - } - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - exprFunc, err := IsMatch(tt.target, tt.pattern) - assert.NoError(t, err) - result, err := exprFunc(context.Background(), nil) - assert.NoError(t, err) - assert.Equal(t, tt.expected, result) - }) - } -} - -func Test_isMatch_validation(t *testing.T) { - target := &ottl.StandardStringGetter[interface{}]{ - Getter: func(ctx context.Context, tCtx interface{}) (interface{}, error) { - return "anything", nil - }, - } - _, err := IsMatch[interface{}](target, "\\K") - require.Error(t, err) -} - -func Test_isMatch_error(t *testing.T) { - tests := []struct { - name string - target ottl.StringGetter[interface{}] - pattern string - }{ { name: "target bool", - target: &ottl.StandardStringGetter[interface{}]{ + target: &ottl.StandardGetSetter[interface{}]{ Getter: func(ctx context.Context, tCtx interface{}) (interface{}, error) { return true, nil }, }, - pattern: "true", + pattern: "true", + expected: true, }, { name: "target int", - target: &ottl.StandardStringGetter[interface{}]{ + target: &ottl.StandardGetSetter[interface{}]{ Getter: func(ctx context.Context, tCtx interface{}) (interface{}, error) { return int64(1), nil }, }, - pattern: `\d`, + pattern: `\d`, + expected: true, }, { name: "target float", - target: &ottl.StandardStringGetter[interface{}]{ + target: &ottl.StandardGetSetter[interface{}]{ Getter: func(ctx context.Context, tCtx interface{}) (interface{}, error) { return 1.1, nil }, }, - pattern: `\d\.\d`, + pattern: `\d\.\d`, + expected: true, }, { name: "target pcommon.Value", - target: &ottl.StandardStringGetter[interface{}]{ + target: &ottl.StandardGetSetter[interface{}]{ Getter: func(ctx context.Context, tCtx interface{}) (interface{}, error) { v := pcommon.NewValueEmpty() v.SetStr("test") return v, nil }, }, - pattern: `test`, + pattern: `test`, + expected: true, + }, + { + name: "nil target", + target: &ottl.StandardGetSetter[interface{}]{ + Getter: func(ctx context.Context, tCtx interface{}) (interface{}, error) { + return nil, nil + }, + }, + pattern: "impossible to match", + expected: false, }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { exprFunc, err := IsMatch(tt.target, tt.pattern) assert.NoError(t, err) - _, err = exprFunc(context.Background(), nil) - require.Error(t, err) + result, err := exprFunc(context.Background(), nil) + assert.NoError(t, err) + assert.Equal(t, tt.expected, result) }) } +} + +func Test_isMatch_validation(t *testing.T) { + target := &ottl.StandardGetSetter[interface{}]{ + Getter: func(ctx context.Context, tCtx interface{}) (interface{}, error) { + return "anything", nil + }, + } + _, err := IsMatch[interface{}](target, "\\K") + require.Error(t, err) +} +func Test_isMatch_error(t *testing.T) { + target := &ottl.StandardGetSetter[interface{}]{ + Getter: func(ctx context.Context, tCtx interface{}) (interface{}, error) { + v := ottl.Path{} + return v, nil + }, + } + exprFunc, err := IsMatch[interface{}](target, "test") + assert.NoError(t, err) + _, err = exprFunc(context.Background(), nil) + require.Error(t, err) } From b3266c33d1efbf54ee9e49a0ef4a51edfaba43ba Mon Sep 17 00:00:00 2001 From: Tyler Helmuth <12352919+TylerHelmuth@users.noreply.github.com> Date: Tue, 31 Jan 2023 15:39:18 -0700 Subject: [PATCH 3/8] make generic --- pkg/ottl/expression.go | 22 +++++++++++++--------- pkg/ottl/functions.go | 8 +++++++- pkg/ottl/functions_test.go | 21 +++++++++++++++++++++ 3 files changed, 41 insertions(+), 10 deletions(-) diff --git a/pkg/ottl/expression.go b/pkg/ottl/expression.go index 2b482bde53b7..01acc6965827 100644 --- a/pkg/ottl/expression.go +++ b/pkg/ottl/expression.go @@ -42,10 +42,6 @@ type GetSetter[K any] interface { Setter[K] } -type StringGetter[K any] interface { - Get(ctx context.Context, tCtx K) (*string, error) -} - type StandardGetSetter[K any] struct { Getter func(ctx context.Context, tCx K) (interface{}, error) Setter func(ctx context.Context, tCx K, val interface{}) error @@ -93,11 +89,19 @@ func (l *listGetter[K]) Get(ctx context.Context, tCtx K) (interface{}, error) { return evaluated, nil } -type StandardStringGetter[K any] struct { +type StringGetter[K any] interface { + Get(ctx context.Context, tCtx K) (*string, error) +} + +type IntGetter[K any] interface { + Get(ctx context.Context, tCtx K) (*int64, error) +} + +type StandardTypeGetter[K any, T any] struct { Getter func(ctx context.Context, tCx K) (interface{}, error) } -func (g StandardStringGetter[K]) Get(ctx context.Context, tCtx K) (*string, error) { +func (g StandardTypeGetter[K, T]) Get(ctx context.Context, tCtx K) (*T, error) { val, err := g.Getter(ctx, tCtx) if err != nil { return nil, err @@ -105,11 +109,11 @@ func (g StandardStringGetter[K]) Get(ctx context.Context, tCtx K) (*string, erro if val == nil { return nil, nil } - strVal, ok := val.(string) + v, ok := val.(T) if !ok { - return nil, fmt.Errorf("value was not a string") + return nil, fmt.Errorf("expected %T but got %T", v, v) } - return &strVal, nil + return &v, nil } func (p *Parser[K]) newGetter(val value) (Getter[K], error) { diff --git a/pkg/ottl/functions.go b/pkg/ottl/functions.go index cb66357d76cc..53cbdbc7f768 100644 --- a/pkg/ottl/functions.go +++ b/pkg/ottl/functions.go @@ -156,7 +156,13 @@ func (p *Parser[K]) buildArg(argVal value, argType reflect.Type) (any, error) { if err != nil { return nil, err } - return StandardStringGetter[K]{Getter: arg.Get}, nil + return StandardTypeGetter[K, string]{Getter: arg.Get}, nil + case strings.HasPrefix(name, "IntGetter"): + arg, err := p.newGetter(argVal) + if err != nil { + return nil, err + } + return StandardTypeGetter[K, int64]{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 e0f2b21ec8dc..bd820be332aa 100644 --- a/pkg/ottl/functions_test.go +++ b/pkg/ottl/functions_test.go @@ -635,6 +635,20 @@ func Test_NewFunctionCall(t *testing.T) { }, want: nil, }, + { + name: "intgetter arg", + inv: invocation{ + Function: "testing_intgetter", + Arguments: []value{ + { + Literal: &mathExprLiteral{ + Int: ottltest.Intp(1), + }, + }, + }, + }, + want: nil, + }, { name: "string arg", inv: invocation{ @@ -890,6 +904,12 @@ func functionWithStringGetter(StringGetter[interface{}]) (ExprFunc[interface{}], }, nil } +func functionWithIntGetter(IntGetter[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 @@ -962,6 +982,7 @@ func defaultFunctionsForTests() map[string]interface{} { functions["testing_getsetter"] = functionWithGetSetter functions["testing_getter"] = functionWithGetter functions["testing_stringgetter"] = functionWithStringGetter + functions["testing_intgetter"] = functionWithIntGetter functions["testing_string"] = functionWithString functions["testing_float"] = functionWithFloat functions["testing_int"] = functionWithInt From 792ac159f9c2078b881c8c3ae06598295d9ebd6c Mon Sep 17 00:00:00 2001 From: Tyler Helmuth <12352919+TylerHelmuth@users.noreply.github.com> Date: Tue, 31 Jan 2023 16:03:32 -0700 Subject: [PATCH 4/8] Add unit tests --- pkg/ottl/expression.go | 2 +- pkg/ottl/expression_test.go | 105 ++++++++++++++++++++++++++++++++++++ 2 files changed, 106 insertions(+), 1 deletion(-) diff --git a/pkg/ottl/expression.go b/pkg/ottl/expression.go index 01acc6965827..d6d104859e7e 100644 --- a/pkg/ottl/expression.go +++ b/pkg/ottl/expression.go @@ -111,7 +111,7 @@ func (g StandardTypeGetter[K, T]) Get(ctx context.Context, tCtx K) (*T, error) { } v, ok := val.(T) if !ok { - return nil, fmt.Errorf("expected %T but got %T", v, v) + return nil, fmt.Errorf("expected %T but got %T", v, val) } return &v, nil } diff --git a/pkg/ottl/expression_test.go b/pkg/ottl/expression_test.go index b6faff33ee2d..7e5675b54ffe 100644 --- a/pkg/ottl/expression_test.go +++ b/pkg/ottl/expression_test.go @@ -16,6 +16,7 @@ package ottl import ( "context" + "go.opentelemetry.io/collector/component/componenttest" "testing" "github.com/stretchr/testify/assert" @@ -318,3 +319,107 @@ func Test_newGetter(t *testing.T) { assert.Error(t, err) }) } + +func Test_StringGetter(t *testing.T) { + tests := []struct { + name string + val value + want interface{} + valid bool + expectedErrorMsg string + }{ + { + name: "Correct type", + val: value{ + String: ottltest.Strp("str"), + }, + want: "str", + valid: true, + }, + { + name: "Incorrect type", + val: value{ + Bool: (*boolean)(ottltest.Boolp(true)), + }, + valid: false, + expectedErrorMsg: "expected string but got bool", + }, + } + + p := NewParser[any]( + defaultFunctionsForTests(), + testParsePath, + componenttest.NewNopTelemetrySettings(), + WithEnumParser[any](testParseEnum), + ) + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + g, err := p.newGetter(tt.val) + assert.NoError(t, err) + + stg := StandardTypeGetter[interface{}, string]{Getter: g.Get} + val, err := stg.Get(context.Background(), nil) + + if tt.valid { + assert.NoError(t, err) + assert.Equal(t, tt.want, *val) + } else { + assert.EqualError(t, err, tt.expectedErrorMsg) + } + }) + } +} + +func Test_IntGetter(t *testing.T) { + tests := []struct { + name string + val value + want interface{} + valid bool + expectedErrorMsg string + }{ + { + name: "Correct type", + val: value{ + Literal: &mathExprLiteral{ + Int: ottltest.Intp(1), + }, + }, + want: int64(1), + valid: true, + }, + { + name: "Incorrect type", + val: value{ + String: ottltest.Strp("1"), + }, + valid: false, + expectedErrorMsg: "expected int64 but got string", + }, + } + + p := NewParser[any]( + defaultFunctionsForTests(), + testParsePath, + componenttest.NewNopTelemetrySettings(), + WithEnumParser[any](testParseEnum), + ) + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + g, err := p.newGetter(tt.val) + assert.NoError(t, err) + + stg := StandardTypeGetter[interface{}, int64]{Getter: g.Get} + val, err := stg.Get(context.Background(), nil) + + if tt.valid { + assert.NoError(t, err) + assert.Equal(t, tt.want, *val) + } else { + assert.EqualError(t, err, tt.expectedErrorMsg) + } + }) + } +} From 2b6ae1b286e6f8ab5c92cbca13443664770249e3 Mon Sep 17 00:00:00 2001 From: Tyler Helmuth <12352919+TylerHelmuth@users.noreply.github.com> Date: Wed, 1 Feb 2023 09:25:44 -0700 Subject: [PATCH 5/8] Fix lint --- pkg/ottl/expression_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/ottl/expression_test.go b/pkg/ottl/expression_test.go index 7e5675b54ffe..115ad41460ed 100644 --- a/pkg/ottl/expression_test.go +++ b/pkg/ottl/expression_test.go @@ -16,11 +16,11 @@ package ottl import ( "context" - "go.opentelemetry.io/collector/component/componenttest" "testing" "github.com/stretchr/testify/assert" "go.opentelemetry.io/collector/component" + "go.opentelemetry.io/collector/component/componenttest" "github.com/open-telemetry/opentelemetry-collector-contrib/pkg/ottl/ottltest" ) From 60b69671f75a83e160ffa520c7e45382e08db13e Mon Sep 17 00:00:00 2001 From: Tyler Helmuth <12352919+TylerHelmuth@users.noreply.github.com> Date: Wed, 1 Feb 2023 09:47:10 -0700 Subject: [PATCH 6/8] Simplify tests --- pkg/ottl/expression_test.go | 87 +++++-------------------------------- 1 file changed, 12 insertions(+), 75 deletions(-) diff --git a/pkg/ottl/expression_test.go b/pkg/ottl/expression_test.go index 115ad41460ed..1cd44c0a232c 100644 --- a/pkg/ottl/expression_test.go +++ b/pkg/ottl/expression_test.go @@ -18,11 +18,9 @@ import ( "context" "testing" + "github.com/open-telemetry/opentelemetry-collector-contrib/pkg/ottl/ottltest" "github.com/stretchr/testify/assert" "go.opentelemetry.io/collector/component" - "go.opentelemetry.io/collector/component/componenttest" - - "github.com/open-telemetry/opentelemetry-collector-contrib/pkg/ottl/ottltest" ) func hello[K any]() (ExprFunc[K], error) { @@ -320,100 +318,39 @@ func Test_newGetter(t *testing.T) { }) } -func Test_StringGetter(t *testing.T) { +func Test_StandardTypeGetter(t *testing.T) { tests := []struct { name string - val value + getter StandardTypeGetter[interface{}, string] want interface{} valid bool expectedErrorMsg string }{ { name: "Correct type", - val: value{ - String: ottltest.Strp("str"), + getter: StandardTypeGetter[interface{}, string]{ + Getter: func(ctx context.Context, tCx interface{}) (interface{}, error) { + return "str", nil + }, }, want: "str", valid: true, }, { name: "Incorrect type", - val: value{ - Bool: (*boolean)(ottltest.Boolp(true)), - }, - valid: false, - expectedErrorMsg: "expected string but got bool", - }, - } - - p := NewParser[any]( - defaultFunctionsForTests(), - testParsePath, - componenttest.NewNopTelemetrySettings(), - WithEnumParser[any](testParseEnum), - ) - - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - g, err := p.newGetter(tt.val) - assert.NoError(t, err) - - stg := StandardTypeGetter[interface{}, string]{Getter: g.Get} - val, err := stg.Get(context.Background(), nil) - - if tt.valid { - assert.NoError(t, err) - assert.Equal(t, tt.want, *val) - } else { - assert.EqualError(t, err, tt.expectedErrorMsg) - } - }) - } -} - -func Test_IntGetter(t *testing.T) { - tests := []struct { - name string - val value - want interface{} - valid bool - expectedErrorMsg string - }{ - { - name: "Correct type", - val: value{ - Literal: &mathExprLiteral{ - Int: ottltest.Intp(1), + getter: StandardTypeGetter[interface{}, string]{ + Getter: func(ctx context.Context, tCx interface{}) (interface{}, error) { + return true, nil }, }, - want: int64(1), - valid: true, - }, - { - name: "Incorrect type", - val: value{ - String: ottltest.Strp("1"), - }, valid: false, - expectedErrorMsg: "expected int64 but got string", + expectedErrorMsg: "expected string but got bool", }, } - p := NewParser[any]( - defaultFunctionsForTests(), - testParsePath, - componenttest.NewNopTelemetrySettings(), - WithEnumParser[any](testParseEnum), - ) - for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - g, err := p.newGetter(tt.val) - assert.NoError(t, err) - - stg := StandardTypeGetter[interface{}, int64]{Getter: g.Get} - val, err := stg.Get(context.Background(), nil) - + val, err := tt.getter.Get(context.Background(), nil) if tt.valid { assert.NoError(t, err) assert.Equal(t, tt.want, *val) From 4f121c88afbbfb0c4b6c61e9b260853436107e3e Mon Sep 17 00:00:00 2001 From: Tyler Helmuth <12352919+TylerHelmuth@users.noreply.github.com> Date: Wed, 1 Feb 2023 10:32:09 -0700 Subject: [PATCH 7/8] fix lint --- pkg/ottl/expression_test.go | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/pkg/ottl/expression_test.go b/pkg/ottl/expression_test.go index 1cd44c0a232c..bbf60814bb79 100644 --- a/pkg/ottl/expression_test.go +++ b/pkg/ottl/expression_test.go @@ -18,9 +18,10 @@ import ( "context" "testing" - "github.com/open-telemetry/opentelemetry-collector-contrib/pkg/ottl/ottltest" "github.com/stretchr/testify/assert" "go.opentelemetry.io/collector/component" + + "github.com/open-telemetry/opentelemetry-collector-contrib/pkg/ottl/ottltest" ) func hello[K any]() (ExprFunc[K], error) { From 1204f3ba827ad5ceeb2ec673aea117171303e623 Mon Sep 17 00:00:00 2001 From: Tyler Helmuth <12352919+TylerHelmuth@users.noreply.github.com> Date: Wed, 1 Feb 2023 14:03:07 -0700 Subject: [PATCH 8/8] cleanup var name --- pkg/ottl/expression.go | 14 +++++++------- pkg/ottl/expression_test.go | 4 ++-- 2 files changed, 9 insertions(+), 9 deletions(-) diff --git a/pkg/ottl/expression.go b/pkg/ottl/expression.go index d6d104859e7e..6ede35dc3ad7 100644 --- a/pkg/ottl/expression.go +++ b/pkg/ottl/expression.go @@ -43,16 +43,16 @@ type GetSetter[K any] interface { } type StandardGetSetter[K any] struct { - Getter func(ctx context.Context, tCx K) (interface{}, error) - Setter func(ctx context.Context, tCx K, val interface{}) error + Getter func(ctx context.Context, tCtx K) (interface{}, error) + Setter func(ctx context.Context, tCtx K, val interface{}) error } -func (path StandardGetSetter[K]) Get(ctx context.Context, tCx K) (interface{}, error) { - return path.Getter(ctx, tCx) +func (path StandardGetSetter[K]) Get(ctx context.Context, tCtx K) (interface{}, error) { + return path.Getter(ctx, tCtx) } -func (path StandardGetSetter[K]) Set(ctx context.Context, tCx K, val interface{}) error { - return path.Setter(ctx, tCx, val) +func (path StandardGetSetter[K]) Set(ctx context.Context, tCtx K, val interface{}) error { + return path.Setter(ctx, tCtx, val) } type literal[K any] struct { @@ -98,7 +98,7 @@ type IntGetter[K any] interface { } type StandardTypeGetter[K any, T any] struct { - Getter func(ctx context.Context, tCx K) (interface{}, error) + Getter func(ctx context.Context, tCtx K) (interface{}, error) } func (g StandardTypeGetter[K, T]) Get(ctx context.Context, tCtx K) (*T, error) { diff --git a/pkg/ottl/expression_test.go b/pkg/ottl/expression_test.go index bbf60814bb79..df64a103cf06 100644 --- a/pkg/ottl/expression_test.go +++ b/pkg/ottl/expression_test.go @@ -330,7 +330,7 @@ func Test_StandardTypeGetter(t *testing.T) { { name: "Correct type", getter: StandardTypeGetter[interface{}, string]{ - Getter: func(ctx context.Context, tCx interface{}) (interface{}, error) { + Getter: func(ctx context.Context, tCtx interface{}) (interface{}, error) { return "str", nil }, }, @@ -340,7 +340,7 @@ func Test_StandardTypeGetter(t *testing.T) { { name: "Incorrect type", getter: StandardTypeGetter[interface{}, string]{ - Getter: func(ctx context.Context, tCx interface{}) (interface{}, error) { + Getter: func(ctx context.Context, tCtx interface{}) (interface{}, error) { return true, nil }, },