Skip to content

Commit

Permalink
[component] Print path to error in ValidateConfig
Browse files Browse the repository at this point in the history
  • Loading branch information
@evan-bradley
evan-bradley committed Jan 24, 2025
1 parent df99547 commit 38d4974
Show file tree
Hide file tree
Showing 2 changed files with 193 additions and 37 deletions.
118 changes: 103 additions & 15 deletions component/config.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4,9 +4,11 @@
package component // import "go.opentelemetry.io/collector/component"

import (
"errors"
"fmt"
"reflect"

"go.uber.org/multierr"
"strconv"
"strings"
)

// Config defines the configuration for a component.Component.
Expand All @@ -31,46 +33,132 @@ type ConfigValidator interface {
// ValidateConfig validates a config, by doing this:
// - Call Validate on the config itself if the config implements ConfigValidator.
func ValidateConfig(cfg Config) error {
return validate(reflect.ValueOf(cfg))
var err error
errs := validate(reflect.ValueOf(cfg))

for _, suberr := range errs {
if suberr.err != nil {
if suberr.path != "" {
err = errors.Join(err, fmt.Errorf("%s: %w", suberr.path, suberr.err))
} else {
err = errors.Join(err, suberr.err)
}
}
}

return err
}

type pathError struct {
err error
path string
}

func validate(v reflect.Value) error {
func validate(v reflect.Value) []pathError {
errs := []pathError{}
// Validate the value itself.
switch v.Kind() {
case reflect.Invalid:
return nil
case reflect.Ptr, reflect.Interface:
return validate(v.Elem())
case reflect.Struct:
var errs error
errs = multierr.Append(errs, callValidateIfPossible(v))
errs = append(errs, pathError{err: callValidateIfPossible(v), path: ""})
// Reflect on the pointed data and check each of its fields.
for i := 0; i < v.NumField(); i++ {
if !v.Type().Field(i).IsExported() {
continue
}
errs = multierr.Append(errs, validate(v.Field(i)))
subpathErrs := validate(v.Field(i))
for _, err := range subpathErrs {
field := v.Type().Field(i)
var fieldName string
if tag, ok := field.Tag.Lookup("mapstructure"); ok {
tags := strings.Split(tag, ",")
if len(tags) > 0 {
fieldName = tags[0]
}
}
// Even if the mapstructure tag exists, the field name may not
// be available, so set it if it is still blank.
if len(fieldName) == 0 {
fieldName = strings.ToLower(field.Name)
}

var path string
if len(err.path) > 0 {
path = strings.Join([]string{fieldName, err.path}, "::")
} else {
path = fieldName
}
errs = append(errs, pathError{
err: err.err,
path: path,
})
}
}
return errs
case reflect.Slice, reflect.Array:
var errs error
errs = multierr.Append(errs, callValidateIfPossible(v))
errs = append(errs, pathError{err: callValidateIfPossible(v), path: ""})
// Reflect on the pointed data and check each of its fields.
for i := 0; i < v.Len(); i++ {
errs = multierr.Append(errs, validate(v.Index(i)))
subPathErrs := validate(v.Index(i))

for _, err := range subPathErrs {
var path string
if len(err.path) > 0 {
path = strings.Join([]string{strconv.Itoa(i), err.path}, "::")
} else {
path = strconv.Itoa(i)
}

errs = append(errs, pathError{
err: err.err,
path: path,
})
}

}
return errs
case reflect.Map:
var errs error
errs = multierr.Append(errs, callValidateIfPossible(v))
errs = append(errs, pathError{err: callValidateIfPossible(v), path: ""})
iter := v.MapRange()
for iter.Next() {
errs = multierr.Append(errs, validate(iter.Key()))
errs = multierr.Append(errs, validate(iter.Value()))
keyErrs := validate(iter.Key())
valueErrs := validate(iter.Value())
var key string

if str, ok := iter.Key().Interface().(string); ok {
key = str
} else if stringer, ok := iter.Key().Interface().(fmt.Stringer); ok {
key = stringer.String()
} else {
key = fmt.Sprintf("[%T key]", iter.Key().Interface())
}

for _, err := range keyErrs {
var path string
if len(err.path) > 0 {
path = strings.Join([]string{key, err.path}, "::")
} else {
path = key
}
errs = append(errs, pathError{err: err.err, path: path})
}

for _, err := range valueErrs {
var path string
if len(err.path) > 0 {
path = strings.Join([]string{key, err.path}, "::")
} else {
path = key
}
errs = append(errs, pathError{err: err.err, path: path})
}
}
return errs
default:
return callValidateIfPossible(v)
return []pathError{{err: callValidateIfPossible(v), path: ""}}
}
}

Expand Down
112 changes: 90 additions & 22 deletions component/config_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@ package component

import (
"errors"
"reflect"
"testing"

"github.com/stretchr/testify/assert"
Expand Down Expand Up @@ -68,11 +67,40 @@ func (e errMapType) Validate() error {
return errors.New(e["err"])
}

type structKey struct {
k string
e error
}

func (s structKey) String() string {
return s.k
}

func (s structKey) Validate() error {
return s.e
}

type configChildMapCustomKey struct {
Child map[structKey]errConfig
}

func newErrMapType() *errMapType {
et := errMapType(nil)
return &et
}

type configMapstructure struct {
Valid *errConfig `mapstructure:"validtag,omitempty"`
NoData *errConfig `mapstructure:""`
NoName *errConfig `mapstructure:",remain"`
}

type configDeeplyNested struct {
MapKeyChild map[configChildStruct]string
MapValueChild map[string]configChildStruct
SliceChild []configChildSlice
}

func TestValidateConfig(t *testing.T) {
tests := []struct {
name string
Expand Down Expand Up @@ -127,104 +155,144 @@ func TestValidateConfig(t *testing.T) {
{
name: "child struct",
cfg: configChildStruct{Child: errConfig{err: errors.New("child struct")}},
expected: errors.New("child struct"),
expected: errors.New("child: child struct"),
},
{
name: "pointer child struct",
cfg: &configChildStruct{Child: errConfig{err: errors.New("pointer child struct")}},
expected: errors.New("pointer child struct"),
expected: errors.New("child: pointer child struct"),
},
{
name: "child struct pointer",
cfg: &configChildStruct{ChildPtr: &errConfig{err: errors.New("child struct pointer")}},
expected: errors.New("child struct pointer"),
expected: errors.New("childptr: child struct pointer"),
},
{
name: "child interface",
cfg: configChildInterface{Child: errConfig{err: errors.New("child interface")}},
expected: errors.New("child interface"),
expected: errors.New("child: child interface"),
},
{
name: "pointer to child interface",
cfg: &configChildInterface{Child: errConfig{err: errors.New("pointer to child interface")}},
expected: errors.New("pointer to child interface"),
expected: errors.New("child: pointer to child interface"),
},
{
name: "child interface with pointer",
cfg: configChildInterface{Child: &errConfig{err: errors.New("child interface with pointer")}},
expected: errors.New("child interface with pointer"),
expected: errors.New("child: child interface with pointer"),
},
{
name: "pointer to child interface with pointer",
cfg: &configChildInterface{Child: &errConfig{err: errors.New("pointer to child interface with pointer")}},
expected: errors.New("pointer to child interface with pointer"),
expected: errors.New("child: pointer to child interface with pointer"),
},
{
name: "child slice",
cfg: configChildSlice{Child: []errConfig{{}, {err: errors.New("child slice")}}},
expected: errors.New("child slice"),
expected: errors.New("child::1: child slice"),
},
{
name: "pointer child slice",
cfg: &configChildSlice{Child: []errConfig{{}, {err: errors.New("pointer child slice")}}},
expected: errors.New("pointer child slice"),
expected: errors.New("child::1: pointer child slice"),
},
{
name: "child slice pointer",
cfg: &configChildSlice{ChildPtr: []*errConfig{{}, {err: errors.New("child slice pointer")}}},
expected: errors.New("child slice pointer"),
expected: errors.New("childptr::1: child slice pointer"),
},
{
name: "child map value",
cfg: configChildMapValue{Child: map[string]errConfig{"test": {err: errors.New("child map")}}},
expected: errors.New("child map"),
expected: errors.New("child::test: child map"),
},
{
name: "pointer child map value",
cfg: &configChildMapValue{Child: map[string]errConfig{"test": {err: errors.New("pointer child map")}}},
expected: errors.New("pointer child map"),
expected: errors.New("child::test: pointer child map"),
},
{
name: "child map value pointer",
cfg: &configChildMapValue{ChildPtr: map[string]*errConfig{"test": {err: errors.New("child map pointer")}}},
expected: errors.New("child map pointer"),
expected: errors.New("childptr::test: child map pointer"),
},
{
name: "child map key",
cfg: configChildMapKey{Child: map[errType]string{"child map key": ""}},
expected: errors.New("child map key"),
expected: errors.New("child::[component.errType key]: child map key"),
},
{
name: "pointer child map key",
cfg: &configChildMapKey{Child: map[errType]string{"pointer child map key": ""}},
expected: errors.New("pointer child map key"),
expected: errors.New("child::[component.errType key]: pointer child map key"),
},
{
name: "child map key pointer",
cfg: &configChildMapKey{ChildPtr: map[*errType]string{newErrType("child map key pointer"): ""}},
expected: errors.New("child map key pointer"),
expected: errors.New("childptr::[*component.errType key]: child map key pointer"),
},
{
name: "map with stringified non-string key type",
cfg: &configChildMapCustomKey{Child: map[structKey]errConfig{{k: "struct_key", e: errors.New("custom key error")}: {err: errors.New("value error")}}},
expected: errors.New("child::struct_key: custom key error\nchild::struct_key: value error"),
},
{
name: "child type",
cfg: configChildTypeDef{Child: "child type"},
expected: errors.New("child type"),
expected: errors.New("child: child type"),
},
{
name: "pointer child type",
cfg: &configChildTypeDef{Child: "pointer child type"},
expected: errors.New("pointer child type"),
expected: errors.New("child: pointer child type"),
},
{
name: "child type pointer",
cfg: &configChildTypeDef{ChildPtr: newErrType("child type pointer")},
expected: errors.New("child type pointer"),
expected: errors.New("childptr: child type pointer"),
},
{
name: "valid mapstructure tag",
cfg: configMapstructure{Valid: &errConfig{errors.New("test")}},
expected: errors.New("validtag: test"),
},
{
name: "zero-length mapstructure tag",
cfg: configMapstructure{NoData: &errConfig{errors.New("test")}},
expected: errors.New("nodata: test"),
},
{
name: "no field name in mapstructure tag",
cfg: configMapstructure{NoName: &errConfig{errors.New("test")}},
expected: errors.New("noname: test"),
},
{
name: "nested map key error",
cfg: configDeeplyNested{MapKeyChild: map[configChildStruct]string{{Child: errConfig{err: errors.New("child key error")}}: "val"}},
expected: errors.New("mapkeychild::[component.configChildStruct key]::child: child key error"),
},
{
name: "nested map value error",
cfg: configDeeplyNested{MapValueChild: map[string]configChildStruct{"key": {Child: errConfig{err: errors.New("child key error")}}}},
expected: errors.New("mapvaluechild::key::child: child key error"),
},
{
name: "nested slice value error",
cfg: configDeeplyNested{SliceChild: []configChildSlice{{Child: []errConfig{{err: errors.New("child key error")}}}}},
expected: errors.New("slicechild::0::child::0: child key error"),
},
}

for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
err := validate(reflect.ValueOf(tt.cfg))
assert.Equal(t, tt.expected, err)
err := ValidateConfig(tt.cfg)

if tt.expected != nil {
assert.EqualError(t, err, tt.expected.Error())
} else {
assert.Nil(t, err)
}
})
}
}

0 comments on commit 38d4974

Please sign in to comment.