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

Move Unmarshaler interface to Config instead of Factory #2867

Merged
merged 14 commits into from
Apr 12, 2021
21 changes: 0 additions & 21 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"
)

Expand Down Expand Up @@ -78,22 +76,3 @@ type Factory interface {
// Type gets the type of the component created by this factory.
Type() config.Type
}

// ConfigUnmarshaler interface is an optional interface that if implemented by a Factory,
// 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.
// componentViperSection *viper.Viper
// 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
}

// CustomUnmarshaler is a function that un-marshals a viper data into a config struct
// in a custom way.
// componentViperSection *viper.Viper
// 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
10 changes: 10 additions & 0 deletions config/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,8 @@ package config
import (
"errors"
"fmt"

"github.com/spf13/viper"
)

var (
Expand Down Expand Up @@ -168,6 +170,14 @@ type validatable interface {
Validate() error
}

// Unmarshable defines the interface for the configuration unmarshaling.
mx-psi marked this conversation as resolved.
Show resolved Hide resolved
type Unmarshable interface {
// Unmarshal is a function that un-marshals a viper data into the Unmarshable struct in a custom way.
// componentViperSection *viper.Viper
// The config for this specific component. May be nil or empty if no config available.
Unmarshal(componentViperSection *viper.Viper) error
mx-psi marked this conversation as resolved.
Show resolved Hide resolved
}

// DataType is the data type that is supported for collection. We currently support
// collecting metrics, traces and logs, this can expand in the future.
type DataType string
Expand Down
25 changes: 8 additions & 17 deletions config/configparser/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -245,8 +245,7 @@ func loadExtensions(exts map[string]interface{}, factories map[config.Type]compo

// 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 := unmarshal(componentConfig, extensionCfg); err != nil {
return nil, errorUnmarshalError(extensionsKeyName, fullName, err)
}

Expand Down Expand Up @@ -281,8 +280,7 @@ func LoadReceiver(componentConfig *config.Parser, fullName string, factory compo

// 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 := unmarshal(componentConfig, receiverCfg); err != nil {
return nil, errorUnmarshalError(receiversKeyName, fullName, err)
}

Expand Down Expand Up @@ -354,8 +352,7 @@ func loadExporters(exps map[string]interface{}, factories map[config.Type]compon

// 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 := unmarshal(componentConfig, exporterCfg); err != nil {
return nil, errorUnmarshalError(exportersKeyName, fullName, err)
}

Expand Down Expand Up @@ -397,8 +394,7 @@ func loadProcessors(procs map[string]interface{}, factories map[config.Type]comp

// 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 := unmarshal(componentConfig, processorCfg); err != nil {
return nil, errorUnmarshalError(processorsKeyName, fullName, err)
}

Expand Down Expand Up @@ -533,15 +529,10 @@ func expandEnv(s string) string {
})
}

func unmarshaler(factory component.Factory) func(componentViperSection *config.Parser, intoCfg interface{}) error {
if fu, ok := factory.(component.ConfigUnmarshaler); ok {
return func(componentParser *config.Parser, intoCfg interface{}) error {
return fu.Unmarshal(componentParser.Viper(), intoCfg)
}
func unmarshal(componentSection *config.Parser, intoCfg interface{}) error {
bogdandrutu marked this conversation as resolved.
Show resolved Hide resolved
if cu, ok := intoCfg.(config.Unmarshable); ok {
return cu.Unmarshal(componentSection.Viper())
}
return defaultUnmarshaler
}

func defaultUnmarshaler(componentParser *config.Parser, intoCfg interface{}) error {
return componentParser.UnmarshalExact(intoCfg)
return componentSection.UnmarshalExact(intoCfg)
}
29 changes: 1 addition & 28 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"
"go.opentelemetry.io/collector/config/configerror"
Expand All @@ -41,7 +39,6 @@ type CreateLogsExporter func(context.Context, component.ExporterCreateParams, co

type factory struct {
cfgType config.Type
customUnmarshaler component.CustomUnmarshaler
createDefaultConfig CreateDefaultConfig
createTracesExporter CreateTracesExporter
createMetricsExporter CreateMetricsExporter
Expand Down Expand Up @@ -69,13 +66,6 @@ func WithLogs(createLogsExporter CreateLogsExporter) FactoryOption {
}
}

// WithCustomUnmarshaler implements component.ConfigUnmarshaler.
func WithCustomUnmarshaler(customUnmarshaler component.CustomUnmarshaler) FactoryOption {
return func(o *factory) {
o.customUnmarshaler = customUnmarshaler
}
}

// NewFactory returns a component.ExporterFactory.
func NewFactory(
cfgType config.Type,
Expand All @@ -88,13 +78,7 @@ func NewFactory(
for _, opt := range options {
opt(f)
}
var ret component.ExporterFactory
if f.customUnmarshaler != nil {
ret = &factoryWithUnmarshaler{f}
} else {
ret = f
}
return ret
return f
}

// Type gets the type of the Exporter config created by this factory.
Expand Down Expand Up @@ -140,14 +124,3 @@ func (f *factory) CreateLogsExporter(
}
return nil, configerror.ErrDataTypeIsNotSupported
}

var _ component.ConfigUnmarshaler = (*factoryWithUnmarshaler)(nil)

type factoryWithUnmarshaler struct {
*factory
}

// 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)
}
15 changes: 1 addition & 14 deletions exporter/exporterhelper/factory_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -16,10 +16,8 @@ package exporterhelper

import (
"context"
"errors"
"testing"

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

Expand Down Expand Up @@ -53,8 +51,6 @@ func TestNewFactory(t *testing.T) {
defaultConfig)
assert.EqualValues(t, typeStr, factory.Type())
assert.EqualValues(t, defaultCfg, factory.CreateDefaultConfig())
_, ok := factory.(component.ConfigUnmarshaler)
assert.False(t, ok)
_, err := factory.CreateTracesExporter(context.Background(), component.ExporterCreateParams{Logger: zap.NewNop()}, defaultCfg)
assert.Equal(t, configerror.ErrDataTypeIsNotSupported, err)
_, err = factory.CreateMetricsExporter(context.Background(), component.ExporterCreateParams{Logger: zap.NewNop()}, defaultCfg)
Expand All @@ -69,15 +65,10 @@ func TestNewFactory_WithConstructors(t *testing.T) {
defaultConfig,
WithTraces(createTraceExporter),
WithMetrics(createMetricsExporter),
WithLogs(createLogsExporter),
WithCustomUnmarshaler(customUnmarshaler))
WithLogs(createLogsExporter))
assert.EqualValues(t, typeStr, factory.Type())
assert.EqualValues(t, defaultCfg, factory.CreateDefaultConfig())

fu, ok := factory.(component.ConfigUnmarshaler)
assert.True(t, ok)
assert.Equal(t, errors.New("my error"), fu.Unmarshal(nil, nil))

te, err := factory.CreateTracesExporter(context.Background(), component.ExporterCreateParams{Logger: zap.NewNop()}, defaultCfg)
assert.NoError(t, err)
assert.Same(t, nopTracesExporter, te)
Expand Down Expand Up @@ -106,7 +97,3 @@ func createMetricsExporter(context.Context, component.ExporterCreateParams, conf
func createLogsExporter(context.Context, component.ExporterCreateParams, config.Exporter) (component.LogsExporter, error) {
return nopLogsExporter, nil
}

func customUnmarshaler(*viper.Viper, interface{}) error {
return errors.New("my error")
}
29 changes: 1 addition & 28 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"
)
Expand All @@ -34,18 +32,10 @@ type CreateServiceExtension func(context.Context, component.ExtensionCreateParam

type factory struct {
cfgType config.Type
customUnmarshaler component.CustomUnmarshaler
createDefaultConfig CreateDefaultConfig
createServiceExtension CreateServiceExtension
}

// WithCustomUnmarshaler implements component.ConfigUnmarshaler.
func WithCustomUnmarshaler(customUnmarshaler component.CustomUnmarshaler) FactoryOption {
return func(o *factory) {
o.customUnmarshaler = customUnmarshaler
}
}

// NewFactory returns a component.ExtensionFactory.
func NewFactory(
cfgType config.Type,
Expand All @@ -60,13 +50,7 @@ func NewFactory(
for _, opt := range options {
opt(f)
}
var ret component.ExtensionFactory
if f.customUnmarshaler != nil {
ret = &factoryWithUnmarshaler{f}
} else {
ret = f
}
return ret
return f
}

// Type gets the type of the Extension config created by this factory.
Expand All @@ -86,14 +70,3 @@ func (f *factory) CreateExtension(
cfg config.Extension) (component.Extension, error) {
return f.createServiceExtension(ctx, params, cfg)
}

var _ component.ConfigUnmarshaler = (*factoryWithUnmarshaler)(nil)

type factoryWithUnmarshaler struct {
*factory
}

// 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)
}
24 changes: 0 additions & 24 deletions extension/extensionhelper/factory_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -16,10 +16,8 @@ package extensionhelper

import (
"context"
"errors"
"testing"

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

"go.opentelemetry.io/collector/component"
Expand Down Expand Up @@ -48,24 +46,6 @@ func TestNewFactory(t *testing.T) {
assert.Same(t, nopExtensionInstance, ext)
}

func TestNewFactory_WithConstructors(t *testing.T) {
factory := NewFactory(
typeStr,
defaultConfig,
createExtension,
WithCustomUnmarshaler(customUnmarshaler))
assert.EqualValues(t, typeStr, factory.Type())
assert.EqualValues(t, defaultCfg, factory.CreateDefaultConfig())

fu, ok := factory.(component.ConfigUnmarshaler)
assert.True(t, ok)
assert.Equal(t, errors.New("my error"), fu.Unmarshal(nil, nil))

ext, err := factory.CreateExtension(context.Background(), component.ExtensionCreateParams{}, defaultCfg)
assert.NoError(t, err)
assert.Same(t, nopExtensionInstance, ext)
}

func defaultConfig() config.Extension {
return defaultCfg
}
Expand All @@ -74,10 +54,6 @@ func createExtension(context.Context, component.ExtensionCreateParams, config.Ex
return nopExtensionInstance, nil
}

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

type nopExtension struct {
}

Expand Down
12 changes: 7 additions & 5 deletions internal/testcomponents/example_exporter.go
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,8 @@ import (
"go.opentelemetry.io/collector/exporter/exporterhelper"
)

var _ config.Unmarshable = (*ExampleExporter)(nil)

// ExampleExporter is for testing purposes. We are defining an example config and factory
// for "exampleexporter" exporter type.
type ExampleExporter struct {
Expand All @@ -35,13 +37,17 @@ type ExampleExporter struct {
ExtraListSetting []string `mapstructure:"extra_list"`
}

// Unmarshal a viper data into the config struct
func (cfg *ExampleExporter) Unmarshal(componentViperSection *viper.Viper) error {
return componentViperSection.UnmarshalExact(cfg)
}

const expType = "exampleexporter"

// ExampleExporterFactory is factory for ExampleExporter.
var ExampleExporterFactory = exporterhelper.NewFactory(
expType,
createExporterDefaultConfig,
exporterhelper.WithCustomUnmarshaler(customUnmarshal),
exporterhelper.WithTraces(createTracesExporter),
exporterhelper.WithMetrics(createMetricsExporter),
exporterhelper.WithLogs(createLogsExporter))
Expand All @@ -59,10 +65,6 @@ func createExporterDefaultConfig() config.Exporter {
}
}

func customUnmarshal(componentViperSection *viper.Viper, intoCfg interface{}) error {
return componentViperSection.UnmarshalExact(intoCfg)
}

func createTracesExporter(
_ context.Context,
_ component.ExporterCreateParams,
Expand Down
Loading