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.

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

[component] Print path to error in ValidateConfig #12108

Merged
merged 11 commits into from
Jan 31, 2025
Next Next commit
[component] Print path to error in ValidateConfig
evan-bradley committed Jan 24, 2025
commit 38d4974de7657f81d2d9778a97b07a6c4b29051a
118 changes: 103 additions & 15 deletions component/config.go
Original file line number Diff line number Diff line change
@@ -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.
@@ -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: ""}}
}
}

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

import (
"errors"
"reflect"
"testing"

"github.com/stretchr/testify/assert"
@@ -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
@@ -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)
}
})
}
}