Skip to content

Commit

Permalink
Add validate interface in configmodels to force each component do con…
Browse files Browse the repository at this point in the history
…figuration validation
  • Loading branch information
mxiamxia committed Mar 30, 2021
1 parent 2dbc66c commit 69aab76
Show file tree
Hide file tree
Showing 18 changed files with 149 additions and 24 deletions.
18 changes: 16 additions & 2 deletions component/componenttest/nop_receiver.go
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ package componenttest

import (
"context"
"fmt"

"go.opentelemetry.io/collector/component"
"go.opentelemetry.io/collector/component/componenthelper"
Expand All @@ -38,10 +39,23 @@ func (f *nopReceiverFactory) Type() config.Type {
return "nop"
}

type NopConfig struct {
config.ReceiverSettings
}

func (nc *NopConfig) Validate() error {
if nc.TypeVal != "nop" {
return fmt.Errorf("invalid receiver config")
}
return nil
}

// CreateDefaultConfig creates the default configuration for the Receiver.
func (f *nopReceiverFactory) CreateDefaultConfig() config.Receiver {
return &config.ReceiverSettings{
TypeVal: f.Type(),
return &NopConfig{
ReceiverSettings: config.ReceiverSettings{
TypeVal: f.Type(),
},
}
}

Expand Down
2 changes: 1 addition & 1 deletion component/componenttest/nop_receiver_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ func TestNewNopReceiverFactory(t *testing.T) {
require.NotNil(t, factory)
assert.Equal(t, config.Type("nop"), factory.Type())
cfg := factory.CreateDefaultConfig()
assert.Equal(t, &config.ReceiverSettings{TypeVal: factory.Type()}, cfg)
assert.Equal(t, &NopConfig{ReceiverSettings: config.ReceiverSettings{TypeVal: factory.Type()}}, cfg)

traces, err := factory.CreateTracesReceiver(context.Background(), component.ReceiverCreateParams{}, cfg, consumertest.NewTracesNop())
require.NoError(t, err)
Expand Down
7 changes: 7 additions & 0 deletions config/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -108,6 +108,13 @@ func (cfg *Config) validateServicePipelines() error {
if cfg.Receivers[ref] == nil {
return fmt.Errorf("pipeline %q references receiver %q which does not exist", pipeline.Name, ref)
}

// Validate the receiver configuration by the custom configuration validator
recCfg := cfg.Receivers[ref]
if err := recCfg.Validate(); err != nil {
return fmt.Errorf("pipeline %q references receiver %q which has invalid configuration with error: %v", pipeline.Name, ref, err)
}

}

// Validate pipeline processor name references
Expand Down
31 changes: 30 additions & 1 deletion config/config_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -16,11 +16,23 @@ package config

import (
"errors"
"fmt"
"testing"

"github.com/stretchr/testify/assert"
)

type NopConfig struct {
ReceiverSettings
}

func (nc *NopConfig) Validate() error {
if nc.TypeVal != "nop" {
return fmt.Errorf("invalid receiver config")
}
return nil
}

func TestConfigValidate(t *testing.T) {
var testCases = []struct {
name string // test case name (also file name containing config yaml)
Expand Down Expand Up @@ -118,6 +130,19 @@ func TestConfigValidate(t *testing.T) {
},
expected: errMissingServicePipelines,
},
{
name: "invalid-receiver-config",
cfgFn: func() *Config {
cfg := generateConfig()
cfg.Receivers["nop"] = &NopConfig{
ReceiverSettings: ReceiverSettings{
TypeVal: "invalid_rec_type",
},
}
return cfg
},
expected: fmt.Errorf(`pipeline "traces" references receiver "nop" which has invalid configuration with error: invalid receiver config`),
},
}

for _, test := range testCases {
Expand All @@ -131,7 +156,11 @@ func TestConfigValidate(t *testing.T) {
func generateConfig() *Config {
return &Config{
Receivers: map[string]Receiver{
"nop": &ReceiverSettings{TypeVal: "nop"},
"nop": &NopConfig{
ReceiverSettings: ReceiverSettings{
TypeVal: "nop",
},
},
},
Exporters: map[string]Exporter{
"nop": &ExporterSettings{TypeVal: "nop"},
Expand Down
22 changes: 21 additions & 1 deletion config/receiver.go
Original file line number Diff line number Diff line change
Expand Up @@ -16,10 +16,30 @@ package config

// Receiver is the configuration of a receiver. Specific receivers must implement this
// interface and will typically embed ReceiverSettings struct or a struct that extends it.
// Embedded CustomConfigOptions will force each receiver to implement Validate() function
type Receiver interface {
NamedEntity
CustomConfigOptions
}

// CustomConfigOptions defines the interfaces for configuration customization options
// Different custom interfaces can be added like CustomConfigValidator, CustomConfigUnmarshaller, etc.
type CustomConfigOptions interface {
CustomConfigValidator
// CustomConfigUnmarshaller
}

// CustomConfigValidator defines the interface for the custom configuration validation on each component
type CustomConfigValidator interface {
Validate() error
}

// CustomValidator is a function that runs the customized validation
// on component level configuration.
// intoCfg interface{}
// An empty interface wrapping a pointer to the config struct.
type CustomValidator func() error

// Receivers is a map of names to Receivers.
type Receivers map[string]Receiver

Expand All @@ -30,7 +50,7 @@ type ReceiverSettings struct {
NameVal string `mapstructure:"-"`
}

var _ Receiver = (*ReceiverSettings)(nil)
var _ NamedEntity = (*ReceiverSettings)(nil)

// Name gets the receiver name.
func (rs *ReceiverSettings) Name() string {
Expand Down
4 changes: 4 additions & 0 deletions internal/testcomponents/example_receiver.go
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,10 @@ type ExampleReceiver struct {
ExtraListSetting []string `mapstructure:"extra_list"`
}

func (ecfg *ExampleReceiver) Validate() error {
return nil
}

const recvType = "examplereceiver"

// ExampleReceiverFactory is factory for ExampleReceiver.
Expand Down
8 changes: 8 additions & 0 deletions receiver/hostmetricsreceiver/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
package hostmetricsreceiver

import (
"go.opentelemetry.io/collector/config"
"go.opentelemetry.io/collector/receiver/hostmetricsreceiver/internal"
"go.opentelemetry.io/collector/receiver/scraperhelper"
)
Expand All @@ -24,3 +25,10 @@ type Config struct {
scraperhelper.ScraperControllerSettings `mapstructure:",squash"`
Scrapers map[string]internal.Config `mapstructure:"-"`
}

var _ config.CustomConfigOptions = (*Config)(nil)

// Validate implements the custom validation check on configuration
func (cfg *Config) Validate() error {
return nil
}
7 changes: 7 additions & 0 deletions receiver/jaegerreceiver/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -72,3 +72,10 @@ type Config struct {
Protocols `mapstructure:"protocols"`
RemoteSampling *RemoteSamplingConfig `mapstructure:"remote_sampling"`
}

var _ config.CustomConfigOptions = (*Config)(nil)

// Validate implements the custom validation check on configuration
func (cfg *Config) Validate() error {
return nil
}
7 changes: 7 additions & 0 deletions receiver/kafkareceiver/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -41,3 +41,10 @@ type Config struct {

Authentication kafkaexporter.Authentication `mapstructure:"auth"`
}

var _ config.CustomConfigOptions = (*Config)(nil)

// Validate implements the custom validation check on configuration
func (cfg *Config) Validate() error {
return nil
}
15 changes: 11 additions & 4 deletions receiver/opencensusreceiver/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -33,13 +33,13 @@ type Config struct {
CorsOrigins []string `mapstructure:"cors_allowed_origins"`
}

func (rOpts *Config) buildOptions() ([]ocOption, error) {
func (cfg *Config) buildOptions() ([]ocOption, error) {
var opts []ocOption
if len(rOpts.CorsOrigins) > 0 {
opts = append(opts, withCorsOrigins(rOpts.CorsOrigins))
if len(cfg.CorsOrigins) > 0 {
opts = append(opts, withCorsOrigins(cfg.CorsOrigins))
}

grpcServerOptions, err := rOpts.GRPCServerSettings.ToServerOption()
grpcServerOptions, err := cfg.GRPCServerSettings.ToServerOption()
if err != nil {
return nil, err
}
Expand All @@ -49,3 +49,10 @@ func (rOpts *Config) buildOptions() ([]ocOption, error) {

return opts, nil
}

var _ config.CustomConfigOptions = (*Config)(nil)

// Validate implements the custom validation check on configuration
func (cfg *Config) Validate() error {
return nil
}
7 changes: 7 additions & 0 deletions receiver/otlpreceiver/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -32,3 +32,10 @@ type Config struct {
// Protocols is the configuration for the supported protocols, currently gRPC and HTTP (Proto and JSON).
Protocols `mapstructure:"protocols"`
}

var _ config.CustomConfigOptions = (*Config)(nil)

// Validate implements the custom validation check on configuration
func (cfg *Config) Validate() error {
return nil
}
7 changes: 7 additions & 0 deletions receiver/prometheusreceiver/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -36,3 +36,10 @@ type Config struct {
// structure, ie.: it will error if an unknown key is present.
ConfigPlaceholder interface{} `mapstructure:"config"`
}

var _ config2.CustomConfigOptions = (*Config)(nil)

// Validate implements the custom validation check on configuration
func (cfg *Config) Validate() error {
return nil
}
3 changes: 0 additions & 3 deletions receiver/prometheusreceiver/factory.go
Original file line number Diff line number Diff line change
Expand Up @@ -102,8 +102,5 @@ func createMetricsReceiver(
nextConsumer consumer.Metrics,
) (component.MetricsReceiver, error) {
config := cfg.(*Config)
if config.PrometheusConfig == nil || len(config.PrometheusConfig.ScrapeConfigs) == 0 {
return nil, errNilScrapeConfig
}
return newPrometheusReceiver(params.Logger, config, nextConsumer), nil
}
5 changes: 2 additions & 3 deletions receiver/prometheusreceiver/factory_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -40,9 +40,8 @@ func TestCreateReceiver(t *testing.T) {
// The default config does not provide scrape_config so we expect that metrics receiver
// creation must also fail.
creationParams := component.ReceiverCreateParams{Logger: zap.NewNop()}
mReceiver, err := createMetricsReceiver(context.Background(), creationParams, cfg, nil)
assert.Equal(t, err, errNilScrapeConfig)
assert.Nil(t, mReceiver)
mReceiver, _ := createMetricsReceiver(context.Background(), creationParams, cfg, nil)
assert.NotNil(t, mReceiver)
}

func TestFactoryCanParseServiceDiscoveryConfigs(t *testing.T) {
Expand Down
9 changes: 6 additions & 3 deletions receiver/receiverhelper/factory_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -23,15 +23,18 @@ import (
"github.com/stretchr/testify/assert"

"go.opentelemetry.io/collector/component"
"go.opentelemetry.io/collector/component/componenttest"
"go.opentelemetry.io/collector/config"
"go.opentelemetry.io/collector/consumer"
)

const typeStr = "test"

var defaultCfg = &config.ReceiverSettings{
TypeVal: typeStr,
NameVal: typeStr,
var defaultCfg = &componenttest.NopConfig{
ReceiverSettings: config.ReceiverSettings{
TypeVal: typeStr,
NameVal: typeStr,
},
}

func TestNewFactory(t *testing.T) {
Expand Down
9 changes: 9 additions & 0 deletions receiver/zipkinreceiver/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -29,4 +29,13 @@ type Config struct {
// If enabled the zipkin receiver will attempt to parse string tags/binary annotations into int/bool/float.
// Disabled by default
ParseStringTags bool `mapstructure:"parse_string_tags"`

config.CustomConfigOptions
}

var _ config.CustomConfigOptions = (*Config)(nil)

// Validate implements the custom validation check on configuration
func (cfg *Config) Validate() error {
return nil
}
5 changes: 3 additions & 2 deletions service/internal/builder/factories_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ import (
"context"

"go.opentelemetry.io/collector/component"
"go.opentelemetry.io/collector/component/componenttest"
"go.opentelemetry.io/collector/config"
"go.opentelemetry.io/collector/exporter/exporterhelper"
"go.opentelemetry.io/collector/extension/extensionhelper"
Expand Down Expand Up @@ -58,8 +59,8 @@ func createTestFactories() component.Factories {

func newBadReceiverFactory() component.ReceiverFactory {
return receiverhelper.NewFactory("bf", func() config.Receiver {
return &config.ReceiverSettings{
TypeVal: "bf",
return &componenttest.NopConfig{
ReceiverSettings: config.ReceiverSettings{TypeVal: "bf"},
}
})
}
Expand Down
7 changes: 3 additions & 4 deletions service/internal/builder/receivers_builder_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,6 @@ import (

"go.opentelemetry.io/collector/component"
"go.opentelemetry.io/collector/component/componenttest"
"go.opentelemetry.io/collector/config"
"go.opentelemetry.io/collector/config/configtest"
"go.opentelemetry.io/collector/consumer/pdata"
"go.opentelemetry.io/collector/internal/testcomponents"
Expand Down Expand Up @@ -261,7 +260,7 @@ func TestBuildReceivers_BuildCustom(t *testing.T) {

func TestBuildReceivers_StartAll(t *testing.T) {
receivers := make(Receivers)
rcvCfg := &config.ReceiverSettings{}
rcvCfg := &componenttest.NopConfig{}

receiver := &testcomponents.ExampleReceiverProducer{}

Expand All @@ -280,7 +279,7 @@ func TestBuildReceivers_StartAll(t *testing.T) {

func TestBuildReceivers_StopAll(t *testing.T) {
receivers := make(Receivers)
rcvCfg := &config.ReceiverSettings{}
rcvCfg := &componenttest.NopConfig{}

receiver := &testcomponents.ExampleReceiverProducer{}

Expand Down Expand Up @@ -337,7 +336,7 @@ func TestBuildReceivers_NotSupportedDataType(t *testing.T) {
t.Run(test.configFile, func(t *testing.T) {

cfg, err := configtest.LoadConfigFile(t, path.Join("testdata", test.configFile), factories)
require.Nil(t, err)
assert.Error(t, err)

allExporters, err := BuildExporters(zap.NewNop(), component.DefaultApplicationStartInfo(), cfg, factories.Exporters)
assert.NoError(t, err)
Expand Down

0 comments on commit 69aab76

Please sign in to comment.