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
9 changes: 9 additions & 0 deletions config/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -168,6 +168,15 @@ type validatable interface {
Validate() error
}

// CustomUnmarshable defines an optional interface for custom configuration unmarshaling.
// A configuration struct can implement this interface to override the default unmarshaling.
type CustomUnmarshable interface {
// Unmarshal is a function that un-marshals a Parser into the unmarshable struct in a custom way.
// componentSection *Parser
// The config for this specific component. May be nil or empty if no config available.
Unmarshal(componentSection *Parser) error
}

// 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
30 changes: 24 additions & 6 deletions config/configparser/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ import (
"strings"

"github.com/spf13/cast"
"github.com/spf13/viper"

"go.opentelemetry.io/collector/component"
"go.opentelemetry.io/collector/config"
Expand Down Expand Up @@ -533,15 +534,32 @@ func expandEnv(s string) string {
})
}

// deprecatedUnmarshaler interface is a deprecated optional interface that if implemented by a Factory,
// the configuration loading system will use to unmarshal the config.
// Implement config.CustomUnmarshable on the configuration struct instead.
type deprecatedUnmarshaler 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
}

func unmarshal(componentSection *config.Parser, intoCfg interface{}) error {
bogdandrutu marked this conversation as resolved.
Show resolved Hide resolved
if cu, ok := intoCfg.(config.CustomUnmarshable); ok {
return cu.Unmarshal(componentSection)
}

return componentSection.UnmarshalExact(intoCfg)
}

// unmarshaler returns an unmarshaling function. It should be removed when deprecatedUnmarshaler is removed.
func unmarshaler(factory component.Factory) func(componentViperSection *config.Parser, intoCfg interface{}) error {
if fu, ok := factory.(component.ConfigUnmarshaler); ok {
if fu, ok := factory.(deprecatedUnmarshaler); ok {
return func(componentParser *config.Parser, intoCfg interface{}) error {
return fu.Unmarshal(componentParser.Viper(), intoCfg)
}
}
return defaultUnmarshaler
}

func defaultUnmarshaler(componentParser *config.Parser, intoCfg interface{}) error {
return componentParser.UnmarshalExact(intoCfg)
return unmarshal
}
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
14 changes: 7 additions & 7 deletions internal/testcomponents/example_exporter.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,14 +17,14 @@ package testcomponents
import (
"context"

"github.com/spf13/viper"

"go.opentelemetry.io/collector/component"
"go.opentelemetry.io/collector/config"
"go.opentelemetry.io/collector/consumer/pdata"
"go.opentelemetry.io/collector/exporter/exporterhelper"
)

var _ config.CustomUnmarshable = (*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 +35,17 @@ type ExampleExporter struct {
ExtraListSetting []string `mapstructure:"extra_list"`
}

// Unmarshal a viper data into the config struct
func (cfg *ExampleExporter) Unmarshal(componentParser *config.Parser) error {
return componentParser.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 +63,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