[ottl/pkg] Remove the ottlarg struct tag for the arguments and rely o…

…n field ordering (open-telemetry#25705) Remove the ottlarg struct tag and rely on field ordering **Link to tracking Issue:** open-telemetry#24874
jorgeancal · Sep 18, 2023 · 3e1f3f7 · 3e1f3f7
1 parent 9f3e52c
commit 3e1f3f7
Show file tree

Hide file tree

Showing 33 changed files with 57 additions and 202 deletions.
diff --git a/pkg/ottl/expression.go b/pkg/ottl/expression.go
@@ -280,15 +280,7 @@ func (g StandardFunctionGetter[K]) Get(args Arguments) (Expr[K], error) {
  }
  for i := 0; i < fArgsVal.NumField(); i++ {
  field := argsVal.Field(i)
- argIndex, err := getArgumentIndex(i, argsVal)
- if err != nil {
- return Expr[K]{}, err
- }
- fArgIndex, err := getArgumentIndex(argIndex, fArgsVal)
- if err != nil {
- return Expr[K]{}, err
- }
- fArgsVal.Field(fArgIndex).Set(field)
+ fArgsVal.Field(i).Set(field)
  }
  fn, err := g.fact.CreateFunction(g.fCtx, fArgs)
  if err != nil {

diff --git a/pkg/ottl/expression_test.go b/pkg/ottl/expression_test.go
@@ -690,11 +690,6 @@ func Test_FunctionGetter(t *testing.T) {
  &multipleArgsArguments{},
  functionWithStringGetter,
  ),
- createFactory[any](
- "no_struct_tag",
- &noStructTagFunctionArguments{},
- functionWithStringGetter,
- ),
  NewFactory(
  "cannot_create_function",
  &stringGetterArguments{},
@@ -753,18 +748,6 @@ func Test_FunctionGetter(t *testing.T) {
  valid: false,
  expectedErrorMsg: "incorrect number of arguments. Expected: 4 Received: 1",
  },
- {
- name: "Invalid Arguments struct tag",
- getter: StandardStringGetter[interface{}]{
- Getter: func(ctx context.Context, tCtx interface{}) (interface{}, error) {
- return nil, nil
- },
- },
- function: StandardFunctionGetter[any]{fCtx: FunctionContext{Set: componenttest.NewNopTelemetrySettings()}, fact: functions["no_struct_tag"]},
- want: "anything",
- valid: false,
- expectedErrorMsg: "no `ottlarg` struct tag on Arguments field \"StringArg\"",
- },
  {
  name: "Cannot create function",
  getter: StandardStringGetter[interface{}]{

diff --git a/pkg/ottl/functions.go b/pkg/ottl/functions.go
@@ -7,7 +7,6 @@ import (
  "errors"
  "fmt"
  "reflect"
- "strconv"
  "strings"
 )
 
@@ -47,22 +46,6 @@ func (p *Parser[K]) newFunctionCall(ed editor) (Expr[K], error) {
  return Expr[K]{exprFunc: fn}, err
 }
 
-func getArgumentIndex(index int, args reflect.Value) (int, error) {
- argsType := args.Type()
- fieldTag, ok := argsType.Field(index).Tag.Lookup("ottlarg")
- if !ok {
- return 0, fmt.Errorf("no `ottlarg` struct tag on Arguments field %q", argsType.Field(index).Name)
- }
- argNum, err := strconv.Atoi(fieldTag)
- if err != nil {
- return 0, fmt.Errorf("ottlarg struct tag on field %q is not a valid integer: %w", argsType.Field(index).Name, err)
- }
- if argNum < 0 || argNum >= args.NumField() {
- return 0, fmt.Errorf("ottlarg struct tag on field %q has value %d, but must be between 0 and %d", argsType.Field(index).Name, argNum, args.NumField())
- }
- return argNum, nil
-}
-
 func (p *Parser[K]) buildArgs(ed editor, argsVal reflect.Value) error {
  if len(ed.Arguments) != argsVal.NumField() {
  return fmt.Errorf("incorrect number of arguments. Expected: %d Received: %d", argsVal.NumField(), len(ed.Arguments))
@@ -71,12 +54,9 @@ func (p *Parser[K]) buildArgs(ed editor, argsVal reflect.Value) error {
  for i := 0; i < argsVal.NumField(); i++ {
  field := argsVal.Field(i)
  fieldType := field.Type()
- argNum, err := getArgumentIndex(i, argsVal)
- if err != nil {
- return err
- }
- argVal := ed.Arguments[argNum]
+ argVal := ed.Arguments[i]
  var val any
+ var err error
  switch {
  case strings.HasPrefix(fieldType.Name(), "FunctionGetter"):
  var name string

diff --git a/pkg/ottl/functions_test.go b/pkg/ottl/functions_test.go
@@ -63,31 +63,6 @@ func Test_NewFunctionCall_invalid(t *testing.T) {
  errorFunctionArguments{},
  functionThatHasAnError,
  ),
- createFactory(
- "no_struct_tag",
- &noStructTagFunctionArguments{},
- functionThatHasAnError,
- ),
- createFactory(
- "wrong_struct_tag",
- &wrongTagFunctionArguments{},
- functionThatHasAnError,
- ),
- createFactory(
- "bad_struct_tag",
- &badStructTagFunctionArguments{},
- functionThatHasAnError,
- ),
- createFactory(
- "negative_struct_tag",
- &negativeStructTagFunctionArguments{},
- functionThatHasAnError,
- ),
- createFactory(
- "out_of_bounds_struct_tag",
- &outOfBoundsStructTagFunctionArguments{},
- functionThatHasAnError,
- ),
  createFactory(
  "testing_unknown_function",
  &functionGetterArguments{},
@@ -349,61 +324,6 @@ func Test_NewFunctionCall_invalid(t *testing.T) {
  Function: "non_pointer",
  },
  },
- {
- name: "no struct tags",
- inv: editor{
- Function: "no_struct_tag",
- Arguments: []value{
- {
- String: ottltest.Strp("str"),
- },
- },
- },
- },
- {
- name: "using the wrong struct tag",
- inv: editor{
- Function: "wrong_struct_tag",
- Arguments: []value{
- {
- String: ottltest.Strp("str"),
- },
- },
- },
- },
- {
- name: "non-integer struct tags",
- inv: editor{
- Function: "bad_struct_tag",
- Arguments: []value{
- {
- String: ottltest.Strp("str"),
- },
- },
- },
- },
- {
- name: "struct tag index too low",
- inv: editor{
- Function: "negative_struct_tag",
- Arguments: []value{
- {
- String: ottltest.Strp("str"),
- },
- },
- },
- },
- {
- name: "struct tag index too high",
- inv: editor{
- Function: "out_of_bounds_struct_tag",
- Arguments: []value{
- {
- String: ottltest.Strp("str"),
- },
- },
- },
- },
  }
 
  for _, tt := range tests {
@@ -1612,26 +1532,6 @@ func functionWithEnum(Enum) (ExprFunc[interface{}], error) {
  }, nil
 }
 
-type noStructTagFunctionArguments struct {
- StringArg string
-}
-
-type badStructTagFunctionArguments struct {
- StringArg string `ottlarg:"a"`
-}
-
-type negativeStructTagFunctionArguments struct {
- StringArg string `ottlarg:"-1"`
-}
-
-type outOfBoundsStructTagFunctionArguments struct {
- StringArg string `ottlarg:"1"`
-}
-
-type wrongTagFunctionArguments struct {
- StringArg string `argument:"1"`
-}
-
 func createFactory[A any](name string, args A, fn any) Factory[any] {
  createFunction := func(fCtx FunctionContext, oArgs Arguments) (ExprFunc[any], error) {
  fArgs, ok := oArgs.(A)

diff --git a/pkg/ottl/ottlfuncs/func_concat.go b/pkg/ottl/ottlfuncs/func_concat.go
@@ -12,8 +12,8 @@ import (
 )
 
 type ConcatArguments[K any] struct {
- Vals []ottl.StringLikeGetter[K] `ottlarg:"0"`
- Delimiter string `ottlarg:"1"`
+ Vals []ottl.StringLikeGetter[K]
+ Delimiter string
 }
 
 func NewConcatFactory[K any]() ottl.Factory[K] {

diff --git a/pkg/ottl/ottlfuncs/func_convert_case.go b/pkg/ottl/ottlfuncs/func_convert_case.go
@@ -14,8 +14,8 @@ import (
 )
 
 type ConvertCaseArguments[K any] struct {
- Target ottl.StringGetter[K] `ottlarg:"0"`
- ToCase string `ottlarg:"1"`
+ Target ottl.StringGetter[K]
+ ToCase string
 }
 
 func NewConvertCaseFactory[K any]() ottl.Factory[K] {

diff --git a/pkg/ottl/ottlfuncs/func_delete_key.go b/pkg/ottl/ottlfuncs/func_delete_key.go
@@ -11,8 +11,8 @@ import (
 )
 
 type DeleteKeyArguments[K any] struct {
- Target ottl.PMapGetter[K] `ottlarg:"0"`
- Key string `ottlarg:"1"`
+ Target ottl.PMapGetter[K]
+ Key string
 }
 
 func NewDeleteKeyFactory[K any]() ottl.Factory[K] {

diff --git a/pkg/ottl/ottlfuncs/func_delete_matching_keys.go b/pkg/ottl/ottlfuncs/func_delete_matching_keys.go
@@ -14,8 +14,8 @@ import (
 )
 
 type DeleteMatchingKeysArguments[K any] struct {
- Target ottl.PMapGetter[K] `ottlarg:"0"`
- Pattern string `ottlarg:"1"`
+ Target ottl.PMapGetter[K]
+ Pattern string
 }
 
 func NewDeleteMatchingKeysFactory[K any]() ottl.Factory[K] {

diff --git a/pkg/ottl/ottlfuncs/func_duration.go b/pkg/ottl/ottlfuncs/func_duration.go
@@ -12,7 +12,7 @@ import (
 )
 
 type DurationArguments[K any] struct {
- Duration ottl.StringGetter[K] `ottlarg:"0"`
+ Duration ottl.StringGetter[K]
 }
 
 func NewDurationFactory[K any]() ottl.Factory[K] {

diff --git a/pkg/ottl/ottlfuncs/func_fnv.go b/pkg/ottl/ottlfuncs/func_fnv.go
@@ -12,7 +12,7 @@ import (
 )
 
 type FnvArguments[K any] struct {
- Target ottl.StringGetter[K] `ottlarg:"0"`
+ Target ottl.StringGetter[K]
 }
 
 func NewFnvFactory[K any]() ottl.Factory[K] {

diff --git a/pkg/ottl/ottlfuncs/func_int.go b/pkg/ottl/ottlfuncs/func_int.go
@@ -11,7 +11,7 @@ import (
 )
 
 type IntArguments[K any] struct {
- Target ottl.IntLikeGetter[K] `ottlarg:"0"`
+ Target ottl.IntLikeGetter[K]
 }
 
 func NewIntFactory[K any]() ottl.Factory[K] {

diff --git a/pkg/ottl/ottlfuncs/func_is_map.go b/pkg/ottl/ottlfuncs/func_is_map.go
@@ -11,7 +11,7 @@ import (
 )
 
 type IsMapArguments[K any] struct {
- Target ottl.PMapGetter[K] `ottlarg:"0"`
+ Target ottl.PMapGetter[K]
 }
 
 func NewIsMapFactory[K any]() ottl.Factory[K] {

diff --git a/pkg/ottl/ottlfuncs/func_is_match.go b/pkg/ottl/ottlfuncs/func_is_match.go
@@ -12,8 +12,8 @@ import (
 )
 
 type IsMatchArguments[K any] struct {
- Target ottl.StringLikeGetter[K] `ottlarg:"0"`
- Pattern string `ottlarg:"1"`
+ Target ottl.StringLikeGetter[K]
+ Pattern string
 }
 
 func NewIsMatchFactory[K any]() ottl.Factory[K] {

diff --git a/pkg/ottl/ottlfuncs/func_is_string.go b/pkg/ottl/ottlfuncs/func_is_string.go
@@ -11,7 +11,7 @@ import (
 )
 
 type IsStringArguments[K any] struct {
- Target ottl.StringGetter[K] `ottlarg:"0"`
+ Target ottl.StringGetter[K]
 }
 
 func NewIsStringFactory[K any]() ottl.Factory[K] {

diff --git a/pkg/ottl/ottlfuncs/func_keep_keys.go b/pkg/ottl/ottlfuncs/func_keep_keys.go
@@ -13,8 +13,8 @@ import (
 )
 
 type KeepKeysArguments[K any] struct {
- Target ottl.PMapGetter[K] `ottlarg:"0"`
- Keys []string `ottlarg:"1"`
+ Target ottl.PMapGetter[K]
+ Keys []string
 }
 
 func NewKeepKeysFactory[K any]() ottl.Factory[K] {

diff --git a/pkg/ottl/ottlfuncs/func_len.go b/pkg/ottl/ottlfuncs/func_len.go
@@ -21,7 +21,7 @@ const (
 )
 
 type LenArguments[K any] struct {
- Target ottl.Getter[K] `ottlarg:"0"`
+ Target ottl.Getter[K]
 }
 
 func NewLenFactory[K any]() ottl.Factory[K] {

diff --git a/pkg/ottl/ottlfuncs/func_limit.go b/pkg/ottl/ottlfuncs/func_limit.go
@@ -13,9 +13,9 @@ import (
 )
 
 type LimitArguments[K any] struct {
- Target ottl.PMapGetter[K] `ottlarg:"0"`
- Limit int64 `ottlarg:"1"`
- PriorityKeys []string `ottlarg:"2"`
+ Target ottl.PMapGetter[K]
+ Limit int64
+ PriorityKeys []string
 }
 
 func NewLimitFactory[K any]() ottl.Factory[K] {

diff --git a/pkg/ottl/ottlfuncs/func_log.go b/pkg/ottl/ottlfuncs/func_log.go
@@ -12,7 +12,7 @@ import (
 )
 
 type LogArguments[K any] struct {
- Target ottl.FloatLikeGetter[K] `ottlarg:"0"`
+ Target ottl.FloatLikeGetter[K]
 }
 
 func NewLogFactory[K any]() ottl.Factory[K] {

diff --git a/pkg/ottl/ottlfuncs/func_merge_maps.go b/pkg/ottl/ottlfuncs/func_merge_maps.go
@@ -19,9 +19,9 @@ const (
 )
 
 type MergeMapsArguments[K any] struct {
- Target ottl.PMapGetter[K] `ottlarg:"0"`
- Source ottl.PMapGetter[K] `ottlarg:"1"`
- Strategy string `ottlarg:"2"`
+ Target ottl.PMapGetter[K]
+ Source ottl.PMapGetter[K]
+ Strategy string
 }
 
 func NewMergeMapsFactory[K any]() ottl.Factory[K] {

diff --git a/pkg/ottl/ottlfuncs/func_parse_json.go b/pkg/ottl/ottlfuncs/func_parse_json.go
@@ -14,7 +14,7 @@ import (
 )
 
 type ParseJSONArguments[K any] struct {
- Target ottl.StringGetter[K] `ottlarg:"0"`
+ Target ottl.StringGetter[K]
 }
 
 func NewParseJSONFactory[K any]() ottl.Factory[K] {

diff --git a/pkg/ottl/ottlfuncs/func_replace_all_matches.go b/pkg/ottl/ottlfuncs/func_replace_all_matches.go
@@ -14,9 +14,9 @@ import (
 )
 
 type ReplaceAllMatchesArguments[K any] struct {
- Target ottl.PMapGetter[K] `ottlarg:"0"`
- Pattern string `ottlarg:"1"`
- Replacement ottl.StringGetter[K] `ottlarg:"2"`
+ Target ottl.PMapGetter[K]
+ Pattern string
+ Replacement ottl.StringGetter[K]
 }
 
 func NewReplaceAllMatchesFactory[K any]() ottl.Factory[K] {