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

Simplify unmarshal logic by adding more supported hooks #4237

Merged
merged 3 commits into from
Oct 22, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
14 changes: 7 additions & 7 deletions config/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -159,7 +159,7 @@ type Service struct {

// ServiceTelemetry defines the configurable settings for service telemetry.
type ServiceTelemetry struct {
Logs ServiceTelemetryLogs
Logs ServiceTelemetryLogs `mapstructure:"logs"`
}

func (srvT *ServiceTelemetry) validate() error {
Expand All @@ -171,15 +171,15 @@ func (srvT *ServiceTelemetry) validate() error {
// the collector uses mapstructure and not yaml tags.
type ServiceTelemetryLogs struct {
// Level is the minimum enabled logging level.
Level zapcore.Level
Level zapcore.Level `mapstructure:"level"`

// Development puts the logger in development mode, which changes the
// behavior of DPanicLevel and takes stacktraces more liberally.
Development bool
Development bool `mapstructure:"development"`

// Encoding sets the logger's encoding.
// Valid values are "json" and "console".
Encoding string
Encoding string `mapstructure:"encoding"`
}

func (srvTL *ServiceTelemetryLogs) validate() error {
Expand Down Expand Up @@ -226,9 +226,9 @@ const (
type Pipeline struct {
Name string
InputType DataType
Receivers []ComponentID
Processors []ComponentID
Exporters []ComponentID
Receivers []ComponentID `mapstructure:"receivers"`
Processors []ComponentID `mapstructure:"processors"`
Exporters []ComponentID `mapstructure:"exporters"`
}

// Pipelines is a map of names to Pipelines.
Expand Down
2 changes: 1 addition & 1 deletion config/configgrpc/configgrpc_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -209,7 +209,7 @@ func TestGRPCClientSettingsError(t *testing.T) {
},
},
{
err: "idStr must have non empty type",
err: "id must not be empty",
settings: GRPCClientSettings{
Endpoint: "localhost:1234",
Auth: &configauth.Authentication{},
Expand Down
2 changes: 1 addition & 1 deletion config/confighttp/confighttp_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -135,7 +135,7 @@ func TestHTTPClientSettingsError(t *testing.T) {
},
},
{
err: "idStr must have non empty type",
err: "id must not be empty",
settings: HTTPClientSettings{
Endpoint: "https://localhost:1234/v1/traces",
Auth: &configauth.Authentication{AuthenticatorName: ""},
Expand Down
81 changes: 58 additions & 23 deletions config/configmap.go
Original file line number Diff line number Diff line change
Expand Up @@ -96,8 +96,8 @@ func (l *Map) Unmarshal(rawVal interface{}) error {
}

// UnmarshalExact unmarshalls the config into a struct, erroring if a field is nonexistent.
func (l *Map) UnmarshalExact(intoCfg interface{}) error {
dc := decoderConfig(intoCfg)
func (l *Map) UnmarshalExact(rawVal interface{}) error {
dc := decoderConfig(rawVal)
dc.ErrorUnused = true
decoder, err := mapstructure.NewDecoder(dc)
if err != nil {
Expand Down Expand Up @@ -164,13 +164,23 @@ func decoderConfig(result interface{}) *mapstructure.DecoderConfig {
TagName: "mapstructure",
WeaklyTypedInput: true,
DecodeHook: mapstructure.ComposeDecodeHookFunc(
expandNilStructPointers(),
mapstructure.StringToTimeDurationHookFunc(),
mapstructure.StringToSliceHookFunc(","),
expandNilStructPointersFunc,
stringToSliceHookFunc,
mapStringToMapComponentIDHookFunc,
stringToTimeDurationHookFunc,
textUnmarshallerHookFunc,
),
}
}

var (
stringToSliceHookFunc = mapstructure.StringToSliceHookFunc(",")
stringToTimeDurationHookFunc = mapstructure.StringToTimeDurationHookFunc()
textUnmarshallerHookFunc = mapstructure.TextUnmarshallerHookFunc()

componentIDType = reflect.TypeOf(NewComponentID("foo"))
)

// In cases where a config has a mapping of something to a struct pointers
// we want nil values to resolve to a pointer to the zero value of the
// underlying struct just as we want nil values of a mapping of something
Expand All @@ -185,26 +195,51 @@ func decoderConfig(result interface{}) *mapstructure.DecoderConfig {
//
// we want an unmarshaled Config to be equivalent to
// Config{Thing: &SomeStruct{}} instead of Config{Thing: nil}
func expandNilStructPointers() mapstructure.DecodeHookFunc {
return func(from reflect.Value, to reflect.Value) (interface{}, error) {
// ensure we are dealing with map to map comparison
if from.Kind() == reflect.Map && to.Kind() == reflect.Map {
toElem := to.Type().Elem()
// ensure that map values are pointers to a struct
// (that may be nil and require manual setting w/ zero value)
if toElem.Kind() == reflect.Ptr && toElem.Elem().Kind() == reflect.Struct {
fromRange := from.MapRange()
for fromRange.Next() {
fromKey := fromRange.Key()
fromValue := fromRange.Value()
// ensure that we've run into a nil pointer instance
if fromValue.IsNil() {
newFromValue := reflect.New(toElem.Elem())
from.SetMapIndex(fromKey, newFromValue)
}
var expandNilStructPointersFunc = func(from reflect.Value, to reflect.Value) (interface{}, error) {
// ensure we are dealing with map to map comparison
if from.Kind() == reflect.Map && to.Kind() == reflect.Map {
toElem := to.Type().Elem()
// ensure that map values are pointers to a struct
// (that may be nil and require manual setting w/ zero value)
if toElem.Kind() == reflect.Ptr && toElem.Elem().Kind() == reflect.Struct {
fromRange := from.MapRange()
for fromRange.Next() {
fromKey := fromRange.Key()
fromValue := fromRange.Value()
// ensure that we've run into a nil pointer instance
if fromValue.IsNil() {
newFromValue := reflect.New(toElem.Elem())
from.SetMapIndex(fromKey, newFromValue)
}
}
}
return from.Interface(), nil
}
return from.Interface(), nil
}

// mapStringToMapComponentIDHookFunc returns a DecodeHookFunc that converts a map[string]interface{} to
// map[ComponentID]interface{}.
// This is needed in combination since the ComponentID.UnmarshalText may produce equal IDs for different strings,
// and an error needs to be returned in that case, otherwise the last equivalent ID overwrites the previous one.
var mapStringToMapComponentIDHookFunc = func(f reflect.Type, t reflect.Type, data interface{}) (interface{}, error) {
if f.Kind() != reflect.Map || f.Key().Kind() != reflect.String {
return data, nil
}

if t.Kind() != reflect.Map || t.Key() != componentIDType {
return data, nil
}

m := make(map[ComponentID]interface{})
for k, v := range data.(map[string]interface{}) {
id, err := NewComponentIDFromString(k)
if err != nil {
return nil, err
}
if _, ok := m[id]; ok {
return nil, fmt.Errorf("duplicate name %q after trimming spaces %v", k, id)
}
m[id] = v
}
return m, nil
}