Skip to content

Commit

Permalink
[component] Print path to error in ValidateConfig (#12108)
Browse files Browse the repository at this point in the history
<!--Ex. Fixing a bug - Describe the bug and how this fixes the issue.
Ex. Adding a feature - Explain what this achieves.-->
#### Description

Follow up to
#12102.

This prints the full path to where the error occurred in a config object
using available reflection metadata. Currently some paths will be
duplicated until we shift otelcol to call `component.ValidateConfig` on
`otelcol.Config` and remove manual calls to `Validate`, but no
information will be missing. If this is a concern we can do both steps
at once.

Example error messages:

```
collector server run finished with error: invalid configuration: receivers::otlp: must specify at least one protocol when using the OTLP receiver
service::telemetry: collector telemetry metrics reader should exist when metric level is not None, level is Normal
```

I'll clean this PR up once
#12102 is
merged, but you can see the latest commit for the changes relevant to
only this PR.

---------

Co-authored-by: Evan Bradley <[email protected]>
Co-authored-by: Pablo Baeyens <[email protected]>
Co-authored-by: Pablo Baeyens <[email protected]>
  • Loading branch information
4 people authored Jan 31, 2025
1 parent a737a48 commit 349b84b
Show file tree
Hide file tree
Showing 5 changed files with 273 additions and 41 deletions.
25 changes: 25 additions & 0 deletions .chloggen/validateconfig-print-path.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
# 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. otlpreceiver)
component: component

# A brief description of the change. Surround your text with quotes ("") if it needs to start with a backtick (`).
note: Show path to invalid config in errors returned from `component.ValidateConfig`

# One or more tracking issues or pull requests related to the change
issues: [12108]

# (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:

# 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: [api]
141 changes: 126 additions & 15 deletions component/config.go
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,119 @@ 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

for _, validationErr := range validate(reflect.ValueOf(cfg)) {
err = errors.Join(err, validationErr)
}

return err
}

type pathError struct {
err error
path []string
}

func (pe pathError) Error() string {
if len(pe.path) > 0 {
var path string
sb := strings.Builder{}

_, _ = sb.WriteString(pe.path[len(pe.path)-1])
for i := len(pe.path) - 2; i >= 0; i-- {
_, _ = sb.WriteString("::")
_, _ = sb.WriteString(pe.path[i])
}
path = sb.String()

return fmt.Sprintf("%s: %s", path, pe.err)
}

return pe.err.Error()
}

func (pe pathError) Unwrap() error {
return pe.err
}

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))
err := callValidateIfPossible(v)
if err != nil {
errs = append(errs, pathError{err: err})
}

// 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)))
field := v.Type().Field(i)
path := fieldName(field)

subpathErrs := validate(v.Field(i))
for _, err := range subpathErrs {
errs = append(errs, pathError{
err: err.err,
path: append(err.path, path),
})
}
}
return errs
case reflect.Slice, reflect.Array:
var errs error
errs = multierr.Append(errs, callValidateIfPossible(v))
err := callValidateIfPossible(v)
if err != nil {
errs = append(errs, pathError{err: err})
}

// 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 {
errs = append(errs, pathError{
err: err.err,
path: append(err.path, strconv.Itoa(i)),
})
}
}
return errs
case reflect.Map:
var errs error
errs = multierr.Append(errs, callValidateIfPossible(v))
err := callValidateIfPossible(v)
if err != nil {
errs = append(errs, pathError{err: err})
}

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())
key := stringifyMapKey(iter.Key())

for _, err := range keyErrs {
errs = append(errs, pathError{err: err.err, path: append(err.path, key)})
}

for _, err := range valueErrs {
errs = append(errs, pathError{err: err.err, path: append(err.path, key)})
}
}
return errs
default:
return callValidateIfPossible(v)
err := callValidateIfPossible(v)
if err != nil {
return []pathError{{err: err}}
}

return nil
}
}

Expand All @@ -93,3 +168,39 @@ func callValidateIfPossible(v reflect.Value) error {

return nil
}

func fieldName(field reflect.StructField) string {
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)
}

return fieldName
}

func stringifyMapKey(val reflect.Value) string {
var key string

if str, ok := val.Interface().(string); ok {
key = str
} else if stringer, ok := val.Interface().(fmt.Stringer); ok {
key = stringer.String()
} else {
switch val.Kind() {
case reflect.Ptr, reflect.Interface, reflect.Struct, reflect.Slice, reflect.Array, reflect.Map:
key = fmt.Sprintf("[%T key]", val.Interface())
default:
key = fmt.Sprintf("%v", val.Interface())
}
}

return key
}
Loading

0 comments on commit 349b84b

Please sign in to comment.