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

Create configload.Parser struct to abstract away from Viper #2728

Merged
merged 17 commits into from
Mar 25, 2021
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 5 additions & 7 deletions component/component.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,8 +17,6 @@ package component
import (
"context"

"github.com/spf13/viper"

"go.opentelemetry.io/collector/config/configmodels"
)

Expand Down Expand Up @@ -68,17 +66,17 @@ type Factory interface {
// the configuration loading system will use to unmarshal the config.
type ConfigUnmarshaler interface {
// Unmarshal is a function that un-marshals a viper data into a config struct in a custom way.
mx-psi marked this conversation as resolved.
Show resolved Hide resolved
// componentViperSection *viper.Viper
// componentSection map[string]interface{}
// The config for this specific component. May be nil or empty if no config available.
// intoCfg interface{}
// An empty interface wrapping a pointer to the config struct to unmarshal into.
Unmarshal(componentViperSection *viper.Viper, intoCfg interface{}) error
Unmarshal(componentSection map[string]interface{}, intoCfg interface{}) error
mx-psi marked this conversation as resolved.
Show resolved Hide resolved
}

// CustomUnmarshaler is a function that un-marshals a viper data into a config struct
// CustomUnmarshaler is a function that un-marshals map data into a config struct
// in a custom way.
// componentViperSection *viper.Viper
// componentViperSection map[string]interface{}
mx-psi marked this conversation as resolved.
Show resolved Hide resolved
// The config for this specific component. May be nil or empty if no config available.
// intoCfg interface{}
// An empty interface wrapping a pointer to the config struct to unmarshal into.
type CustomUnmarshaler func(componentViperSection *viper.Viper, intoCfg interface{}) error
type CustomUnmarshaler func(componentSection map[string]interface{}, intoCfg interface{}) error
76 changes: 71 additions & 5 deletions config/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -104,6 +104,71 @@ func NewViper() *viper.Viper {
return viper.NewWithOptions(viper.KeyDelimiter(ViperDelimiter))
}

// fromStringMap creates a Viper object from a map[string]interface{}
func fromStringMap(cfg map[string]interface{}) *viper.Viper {
viperCfg := NewViper()
viperCfg.MergeConfigMap(cfg)
return viperCfg
}

// deepSearch scans deep maps, following the key indexes listed in the
// sequence "path".
// The last value is expected to be another map, and is returned.
//
// In case intermediate keys do not exist, or map to a non-map value,
// a new map is created and inserted, and the search continues from there:
// the initial map "m" may be modified!
// This function comes from Viper code https://github.com/spf13/viper/blob/5253694/util.go#L201-L230
// It is used here because of https://github.com/spf13/viper/issues/819
func deepSearch(m map[string]interface{}, path []string) map[string]interface{} {
for _, k := range path {
m2, ok := m[k]
if !ok {
// intermediate key does not exist
// => create it and continue from there
m3 := make(map[string]interface{})
m[k] = m3
m = m3
continue
}
m3, ok := m2.(map[string]interface{})
if !ok {
// intermediate key is a value
// => replace with a new map
m3 = make(map[string]interface{})
m[k] = m3
}
// continue search from here
m = m3
}
return m
}

// toStringMap creates a map[string]interface{} from a Viper object
// It is equivalent to v.AllSettings() but it maps nil values
// It is used here because of https://github.com/spf13/viper/issues/819
func toStringMap(v *viper.Viper) map[string]interface{} {
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This can be simplified if/once spf13/viper#819 gets resolved (it seems like Bogdan already stumbled on this one 😄). The code is the same as the AllSettings method except that it does not skip over nil values.

Copy link
Member Author

@mx-psi mx-psi Mar 22, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not used anymore, cast.ToStringMap seems to overcome this issue

m := map[string]interface{}{}
// start from the list of keys, and construct the map one value at a time
for _, k := range v.AllKeys() {
value := v.Get(k)
path := strings.Split(k, ViperDelimiter)
lastKey := strings.ToLower(path[len(path)-1])
deepestMap := deepSearch(m, path[0:len(path)-1])
// set innermost value
deepestMap[lastKey] = value
}
return m
}

// ToCustomUnmarshaler creates a custom unmarshaler from a Viper unmarshaled.
// It should not be used by new code.
func ToCustomUnmarshaler(viperUnmarshaler func(*viper.Viper, interface{}) error) component.CustomUnmarshaler {
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wrote this wrapper for convenience to fix existing tests but I don't think it should be used by new components.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe not in this PR, but I would like to remove this before we declare this module stable. Maybe we can open an issue and decide how to get rid of this.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Created #2735 to track this

return func(componentSection map[string]interface{}, intoCfg interface{}) error {
return viperUnmarshaler(fromStringMap(componentSection), intoCfg)
}
}

// Load loads a Config from Viper.
// After loading the config, need to check if it is valid by calling `ValidateConfig`.
func Load(
Expand Down Expand Up @@ -262,7 +327,7 @@ func loadExtensions(exts map[string]interface{}, factories map[configmodels.Type
// Now that the default config struct is created we can Unmarshal into it
// and it will apply user-defined config on top of the default.
unm := unmarshaler(factory)
if err := unm(componentConfig, extensionCfg); err != nil {
if err := unm(toStringMap(componentConfig), extensionCfg); err != nil {
return nil, errorUnmarshalError(extensionsKeyName, fullName, err)
}

Expand Down Expand Up @@ -298,7 +363,7 @@ func LoadReceiver(componentConfig *viper.Viper, typeStr configmodels.Type, fullN
// Now that the default config struct is created we can Unmarshal into it
// and it will apply user-defined config on top of the default.
unm := unmarshaler(factory)
if err := unm(componentConfig, receiverCfg); err != nil {
if err := unm(toStringMap(componentConfig), receiverCfg); err != nil {
return nil, errorUnmarshalError(receiversKeyName, fullName, err)
}

Expand Down Expand Up @@ -371,7 +436,7 @@ func loadExporters(exps map[string]interface{}, factories map[configmodels.Type]
// Now that the default config struct is created we can Unmarshal into it
// and it will apply user-defined config on top of the default.
unm := unmarshaler(factory)
if err := unm(componentConfig, exporterCfg); err != nil {
if err := unm(toStringMap(componentConfig), exporterCfg); err != nil {
return nil, errorUnmarshalError(exportersKeyName, fullName, err)
}

Expand Down Expand Up @@ -414,7 +479,7 @@ func loadProcessors(procs map[string]interface{}, factories map[configmodels.Typ
// Now that the default config struct is created we can Unmarshal into it
// and it will apply user-defined config on top of the default.
unm := unmarshaler(factory)
if err := unm(componentConfig, processorCfg); err != nil {
if err := unm(toStringMap(componentConfig), processorCfg); err != nil {
return nil, errorUnmarshalError(processorsKeyName, fullName, err)
}

Expand Down Expand Up @@ -556,7 +621,8 @@ func unmarshaler(factory component.Factory) component.CustomUnmarshaler {
return defaultUnmarshaler
}

func defaultUnmarshaler(componentViperSection *viper.Viper, intoCfg interface{}) error {
func defaultUnmarshaler(componentSection map[string]interface{}, intoCfg interface{}) error {
componentViperSection := fromStringMap(componentSection)
return componentViperSection.UnmarshalExact(intoCfg)
}

Expand Down
6 changes: 2 additions & 4 deletions exporter/exporterhelper/factory.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,8 +17,6 @@ package exporterhelper
import (
"context"

"github.com/spf13/viper"

"go.opentelemetry.io/collector/component"
"go.opentelemetry.io/collector/config/configerror"
"go.opentelemetry.io/collector/config/configmodels"
Expand Down Expand Up @@ -148,6 +146,6 @@ type factoryWithUnmarshaler struct {
}

// Unmarshal un-marshals the config using the provided custom unmarshaler.
func (f *factoryWithUnmarshaler) Unmarshal(componentViperSection *viper.Viper, intoCfg interface{}) error {
return f.customUnmarshaler(componentViperSection, intoCfg)
func (f *factoryWithUnmarshaler) Unmarshal(componentSection map[string]interface{}, intoCfg interface{}) error {
return f.customUnmarshaler(componentSection, intoCfg)
}
3 changes: 1 addition & 2 deletions exporter/exporterhelper/factory_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,6 @@ import (
"errors"
"testing"

"github.com/spf13/viper"
"github.com/stretchr/testify/assert"
"go.uber.org/zap"

Expand Down Expand Up @@ -107,6 +106,6 @@ func createLogsExporter(context.Context, component.ExporterCreateParams, configm
return nopLogsExporter, nil
}

func customUnmarshaler(*viper.Viper, interface{}) error {
func customUnmarshaler(map[string]interface{}, interface{}) error {
return errors.New("my error")
}
6 changes: 2 additions & 4 deletions extension/extensionhelper/factory.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,8 +17,6 @@ package extensionhelper
import (
"context"

"github.com/spf13/viper"

"go.opentelemetry.io/collector/component"
"go.opentelemetry.io/collector/config/configmodels"
)
Expand Down Expand Up @@ -94,6 +92,6 @@ type factoryWithUnmarshaler struct {
}

// Unmarshal un-marshals the config using the provided custom unmarshaler.
func (f *factoryWithUnmarshaler) Unmarshal(componentViperSection *viper.Viper, intoCfg interface{}) error {
return f.customUnmarshaler(componentViperSection, intoCfg)
func (f *factoryWithUnmarshaler) Unmarshal(componentSection map[string]interface{}, intoCfg interface{}) error {
return f.customUnmarshaler(componentSection, intoCfg)
}
3 changes: 1 addition & 2 deletions extension/extensionhelper/factory_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,6 @@ import (
"errors"
"testing"

"github.com/spf13/viper"
"github.com/stretchr/testify/assert"

"go.opentelemetry.io/collector/component"
Expand Down Expand Up @@ -74,7 +73,7 @@ func createExtension(context.Context, component.ExtensionCreateParams, configmod
return nopExtensionInstance, nil
}

func customUnmarshaler(*viper.Viper, interface{}) error {
func customUnmarshaler(map[string]interface{}, interface{}) error {
return errors.New("my error")
}

Expand Down
6 changes: 2 additions & 4 deletions processor/processorhelper/factory.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,8 +17,6 @@ package processorhelper
import (
"context"

"github.com/spf13/viper"

"go.opentelemetry.io/collector/component"
"go.opentelemetry.io/collector/config/configerror"
"go.opentelemetry.io/collector/config/configmodels"
Expand Down Expand Up @@ -144,6 +142,6 @@ type factoryWithUnmarshaler struct {
}

// Unmarshal un-marshals the config using the provided custom unmarshaler.
func (f *factoryWithUnmarshaler) Unmarshal(componentViperSection *viper.Viper, intoCfg interface{}) error {
return f.customUnmarshaler(componentViperSection, intoCfg)
func (f *factoryWithUnmarshaler) Unmarshal(componentSection map[string]interface{}, intoCfg interface{}) error {
return f.customUnmarshaler(componentSection, intoCfg)
}
3 changes: 1 addition & 2 deletions processor/processorhelper/factory_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,6 @@ import (
"errors"
"testing"

"github.com/spf13/viper"
"github.com/stretchr/testify/assert"

"go.opentelemetry.io/collector/component"
Expand Down Expand Up @@ -91,6 +90,6 @@ func createLogsProcessor(context.Context, component.ProcessorCreateParams, confi
return nil, nil
}

func customUnmarshaler(*viper.Viper, interface{}) error {
func customUnmarshaler(map[string]interface{}, interface{}) error {
return errors.New("my error")
}
2 changes: 1 addition & 1 deletion receiver/hostmetricsreceiver/factory.go
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,7 @@ func NewFactory() component.ReceiverFactory {
typeStr,
createDefaultConfig,
receiverhelper.WithMetrics(createMetricsReceiver),
receiverhelper.WithCustomUnmarshaler(customUnmarshaler))
receiverhelper.WithCustomUnmarshaler(config.ToCustomUnmarshaler(customUnmarshaler)))
}

// customUnmarshaler returns custom unmarshaler for this config.
Expand Down
3 changes: 2 additions & 1 deletion receiver/jaegerreceiver/factory.go
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ import (
"github.com/spf13/viper"

"go.opentelemetry.io/collector/component"
"go.opentelemetry.io/collector/config"
"go.opentelemetry.io/collector/config/configgrpc"
"go.opentelemetry.io/collector/config/confighttp"
"go.opentelemetry.io/collector/config/configmodels"
Expand Down Expand Up @@ -56,7 +57,7 @@ func NewFactory() component.ReceiverFactory {
typeStr,
createDefaultConfig,
receiverhelper.WithTraces(createTraceReceiver),
receiverhelper.WithCustomUnmarshaler(customUnmarshaler))
receiverhelper.WithCustomUnmarshaler(config.ToCustomUnmarshaler(customUnmarshaler)))
}

// customUnmarshaler is used to add defaults for named but empty protocols
Expand Down
5 changes: 2 additions & 3 deletions receiver/jaegerreceiver/factory_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,6 @@ import (

"go.opentelemetry.io/collector/component"
"go.opentelemetry.io/collector/component/componenttest"
"go.opentelemetry.io/collector/config"
"go.opentelemetry.io/collector/config/configcheck"
"go.opentelemetry.io/collector/config/configerror"
"go.opentelemetry.io/collector/config/configgrpc"
Expand Down Expand Up @@ -344,9 +343,9 @@ func TestCustomUnmarshalErrors(t *testing.T) {
fu, ok := factory.(component.ConfigUnmarshaler)
assert.True(t, ok)

err := fu.Unmarshal(config.NewViper(), nil)
err := fu.Unmarshal(nil, nil)
assert.Error(t, err, "should not have been able to marshal to a nil config")

err = fu.Unmarshal(config.NewViper(), &RemoteSamplingConfig{})
err = fu.Unmarshal(nil, &RemoteSamplingConfig{})
assert.Error(t, err, "should not have been able to marshal to a non-jaegerreceiver config")
}
3 changes: 2 additions & 1 deletion receiver/otlpreceiver/factory.go
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ import (
"go.uber.org/zap"

"go.opentelemetry.io/collector/component"
"go.opentelemetry.io/collector/config"
"go.opentelemetry.io/collector/config/configgrpc"
"go.opentelemetry.io/collector/config/confighttp"
"go.opentelemetry.io/collector/config/configmodels"
Expand Down Expand Up @@ -51,7 +52,7 @@ func NewFactory() component.ReceiverFactory {
receiverhelper.WithTraces(createTraceReceiver),
receiverhelper.WithMetrics(createMetricsReceiver),
receiverhelper.WithLogs(createLogReceiver),
receiverhelper.WithCustomUnmarshaler(customUnmarshaler))
receiverhelper.WithCustomUnmarshaler(config.ToCustomUnmarshaler(customUnmarshaler)))
}

// createDefaultConfig creates the default configuration for receiver.
Expand Down
3 changes: 2 additions & 1 deletion receiver/prometheusreceiver/factory.go
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ import (
"gopkg.in/yaml.v2"

"go.opentelemetry.io/collector/component"
"go.opentelemetry.io/collector/config"
"go.opentelemetry.io/collector/config/configmodels"
"go.opentelemetry.io/collector/consumer"
"go.opentelemetry.io/collector/receiver/receiverhelper"
Expand All @@ -48,7 +49,7 @@ func NewFactory() component.ReceiverFactory {
typeStr,
createDefaultConfig,
receiverhelper.WithMetrics(createMetricsReceiver),
receiverhelper.WithCustomUnmarshaler(customUnmarshaler))
receiverhelper.WithCustomUnmarshaler(config.ToCustomUnmarshaler(customUnmarshaler)))
}

func customUnmarshaler(componentViperSection *viper.Viper, intoCfg interface{}) error {
Expand Down
6 changes: 2 additions & 4 deletions receiver/receiverhelper/factory.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,8 +17,6 @@ package receiverhelper
import (
"context"

"github.com/spf13/viper"

"go.opentelemetry.io/collector/component"
"go.opentelemetry.io/collector/config/configerror"
"go.opentelemetry.io/collector/config/configmodels"
Expand Down Expand Up @@ -152,6 +150,6 @@ type factoryWithUnmarshaler struct {
}

// Unmarshal un-marshals the config using the provided custom unmarshaler.
func (f *factoryWithUnmarshaler) Unmarshal(componentViperSection *viper.Viper, intoCfg interface{}) error {
return f.customUnmarshaler(componentViperSection, intoCfg)
func (f *factoryWithUnmarshaler) Unmarshal(componentSection map[string]interface{}, intoCfg interface{}) error {
return f.customUnmarshaler(componentSection, intoCfg)
}
3 changes: 1 addition & 2 deletions receiver/receiverhelper/factory_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,6 @@ import (
"errors"
"testing"

"github.com/spf13/viper"
"github.com/stretchr/testify/assert"

"go.opentelemetry.io/collector/component"
Expand Down Expand Up @@ -91,6 +90,6 @@ func createLogsReceiver(context.Context, component.ReceiverCreateParams, configm
return nil, nil
}

func customUnmarshaler(*viper.Viper, interface{}) error {
func customUnmarshaler(map[string]interface{}, interface{}) error {
return errors.New("my error")
}