From 903948103b43c15921ff60c58c8590bf805b9517 Mon Sep 17 00:00:00 2001 From: Joe Elliott Date: Tue, 22 Sep 2020 04:43:40 -0400 Subject: [PATCH] OTel: Factored out Config Factory (#2495) * Factored out default config factory Signed-off-by: Joe Elliott * lint Signed-off-by: Joe Elliott --- .../app/defaultconfig/default_config.go | 45 ++++++++++++++----- .../app/defaultconfig/default_config_test.go | 8 ++-- .../zipkinreceiver/zipkin_receiver.go | 10 +++-- .../zipkinreceiver/zipkin_receiver_test.go | 3 +- cmd/opentelemetry/cmd/agent/main.go | 27 ++--------- cmd/opentelemetry/cmd/all-in-one/main.go | 33 +++----------- cmd/opentelemetry/cmd/collector/main.go | 35 +++------------ cmd/opentelemetry/cmd/ingester/main.go | 30 +++---------- 8 files changed, 66 insertions(+), 125 deletions(-) diff --git a/cmd/opentelemetry/app/defaultconfig/default_config.go b/cmd/opentelemetry/app/defaultconfig/default_config.go index d0e372cc4b8..f907de03cfb 100644 --- a/cmd/opentelemetry/app/defaultconfig/default_config.go +++ b/cmd/opentelemetry/app/defaultconfig/default_config.go @@ -18,6 +18,7 @@ import ( "fmt" "strings" + "github.com/spf13/viper" "go.opentelemetry.io/collector/component" "go.opentelemetry.io/collector/config/configgrpc" "go.opentelemetry.io/collector/config/confighttp" @@ -27,6 +28,8 @@ import ( "go.opentelemetry.io/collector/processor/resourceprocessor" "go.opentelemetry.io/collector/receiver/jaegerreceiver" "go.opentelemetry.io/collector/receiver/zipkinreceiver" + "go.opentelemetry.io/collector/service" + "go.opentelemetry.io/collector/service/builder" "github.com/jaegertracing/jaeger/cmd/opentelemetry/app/exporter/badgerexporter" "github.com/jaegertracing/jaeger/cmd/opentelemetry/app/exporter/cassandraexporter" @@ -60,19 +63,40 @@ type ComponentType int // ComponentSettings struct configures generation of the default config type ComponentSettings struct { - ComponentType ComponentType - Factories component.Factories - StorageType string - ZipkinHostPort string + ComponentType ComponentType + Factories component.Factories + StorageType string } -// CreateDefaultConfig creates default configuration. -func (c ComponentSettings) CreateDefaultConfig() (*configmodels.Config, error) { +// DefaultConfigFactory returns a service.ConfigFactory that merges jaeger and otel configs +func (c *ComponentSettings) DefaultConfigFactory(jaegerViper *viper.Viper) service.ConfigFactory { + return func(otelViper *viper.Viper, f component.Factories) (*configmodels.Config, error) { + cfg, err := c.createDefaultConfig() + if err != nil { + return nil, err + } + if len(builder.GetConfigFile()) > 0 { + otelCfg, err := service.FileLoaderConfigFactory(otelViper, f) + if err != nil { + return nil, err + } + err = MergeConfigs(cfg, otelCfg) + if err != nil { + return nil, err + } + } + + return cfg, nil + } +} + +// createDefaultConfig creates default configuration. +func (c ComponentSettings) createDefaultConfig() (*configmodels.Config, error) { exporters, err := createExporters(c.ComponentType, c.StorageType, c.Factories) if err != nil { return nil, err } - receivers := createReceivers(c.ComponentType, c.ZipkinHostPort, c.Factories) + receivers := createReceivers(c.ComponentType, c.Factories) processors, processorNames := createProcessors(c.Factories) hc := c.Factories.Extensions["health_check"].CreateDefaultConfig() return &configmodels.Config{ @@ -109,7 +133,7 @@ func createProcessors(factories component.Factories) (configmodels.Processors, [ return processors, names } -func createReceivers(component ComponentType, zipkinHostPort string, factories component.Factories) configmodels.Receivers { +func createReceivers(component ComponentType, factories component.Factories) configmodels.Receivers { if component == Ingester { kafkaReceiver := factories.Receivers[kafkareceiver.TypeStr].CreateDefaultConfig().(*kafkareceiver.Config) return configmodels.Receivers{ @@ -139,9 +163,8 @@ func createReceivers(component ComponentType, zipkinHostPort string, factories c "jaeger": jaeger, "otlp": factories.Receivers["otlp"].CreateDefaultConfig(), } - if zipkinHostPort != "" && zipkinHostPort != ports.PortToHostPort(0) { - zipkin := factories.Receivers["zipkin"].CreateDefaultConfig().(*zipkinreceiver.Config) - zipkin.Endpoint = zipkinHostPort + zipkin := factories.Receivers["zipkin"].CreateDefaultConfig().(*zipkinreceiver.Config) + if zipkin.Endpoint != "" && zipkin.Endpoint != ports.PortToHostPort(0) { recvs["zipkin"] = zipkin } return recvs diff --git a/cmd/opentelemetry/app/defaultconfig/default_config_test.go b/cmd/opentelemetry/app/defaultconfig/default_config_test.go index bff1c7ebae9..888da5c3e66 100644 --- a/cmd/opentelemetry/app/defaultconfig/default_config_test.go +++ b/cmd/opentelemetry/app/defaultconfig/default_config_test.go @@ -112,10 +112,10 @@ func TestService(t *testing.T) { }, }, { + viperConfig: map[string]interface{}{"collector.zipkin.host-port": "localhost:9411"}, cfg: ComponentSettings{ - ComponentType: AllInOne, - StorageType: "elasticsearch", - ZipkinHostPort: "localhost:9411", + ComponentType: AllInOne, + StorageType: "elasticsearch", }, service: configmodels.Service{ Extensions: []string{"health_check"}, @@ -145,7 +145,7 @@ func TestService(t *testing.T) { } factories := defaultcomponents.Components(v) test.cfg.Factories = factories - cfg, err := test.cfg.CreateDefaultConfig() + cfg, err := test.cfg.createDefaultConfig() if test.err != "" { require.Nil(t, cfg) assert.Contains(t, err.Error(), test.err) diff --git a/cmd/opentelemetry/app/receiver/zipkinreceiver/zipkin_receiver.go b/cmd/opentelemetry/app/receiver/zipkinreceiver/zipkin_receiver.go index 0e4149feeb6..2dc852f6fe2 100644 --- a/cmd/opentelemetry/app/receiver/zipkinreceiver/zipkin_receiver.go +++ b/cmd/opentelemetry/app/receiver/zipkinreceiver/zipkin_receiver.go @@ -45,9 +45,13 @@ func (f Factory) Type() configmodels.Type { // This function implements OTEL component.ReceiverFactoryBase interface. func (f Factory) CreateDefaultConfig() configmodels.Receiver { cfg := f.Wrapped.CreateDefaultConfig().(*zipkinreceiver.Config) - if f.Viper.IsSet(collectorApp.CollectorZipkinHTTPHostPort) { - cfg.Endpoint = f.Viper.GetString(collectorApp.CollectorZipkinHTTPHostPort) - } + + // using the CollectorOptions to parse the zipkin host port b/c it has special processing + // for combining the port and host:port zipkin flags + collectorOpts := &collectorApp.CollectorOptions{} + collectorOpts.InitFromViper(f.Viper) + cfg.Endpoint = collectorOpts.CollectorZipkinHTTPHostPort + return cfg } diff --git a/cmd/opentelemetry/app/receiver/zipkinreceiver/zipkin_receiver_test.go b/cmd/opentelemetry/app/receiver/zipkinreceiver/zipkin_receiver_test.go index 9356108cffc..52a11b086bb 100644 --- a/cmd/opentelemetry/app/receiver/zipkinreceiver/zipkin_receiver_test.go +++ b/cmd/opentelemetry/app/receiver/zipkinreceiver/zipkin_receiver_test.go @@ -29,6 +29,7 @@ import ( "go.opentelemetry.io/collector/receiver/zipkinreceiver" jConfig "github.com/jaegertracing/jaeger/pkg/config" + "github.com/jaegertracing/jaeger/ports" ) func TestDefaultValues(t *testing.T) { @@ -38,7 +39,7 @@ func TestDefaultValues(t *testing.T) { factory := &Factory{Viper: v, Wrapped: zipkinreceiver.NewFactory()} cfg := factory.CreateDefaultConfig().(*zipkinreceiver.Config) - assert.Equal(t, "0.0.0.0:9411", cfg.Endpoint) + assert.Equal(t, ports.PortToHostPort(0), cfg.Endpoint) } func TestLoadConfigAndFlags(t *testing.T) { diff --git a/cmd/opentelemetry/cmd/agent/main.go b/cmd/opentelemetry/cmd/agent/main.go index 1e78f282393..4b015acdc38 100644 --- a/cmd/opentelemetry/cmd/agent/main.go +++ b/cmd/opentelemetry/cmd/agent/main.go @@ -21,9 +21,7 @@ import ( "github.com/spf13/viper" "go.opentelemetry.io/collector/component" - "go.opentelemetry.io/collector/config/configmodels" "go.opentelemetry.io/collector/service" - "go.opentelemetry.io/collector/service/builder" jflags "github.com/jaegertracing/jaeger/cmd/flags" "github.com/jaegertracing/jaeger/cmd/opentelemetry/app" @@ -55,32 +53,15 @@ func main() { v := viper.New() cmpts := defaultcomponents.Components(v) - cfgFactory := func(otelViper *viper.Viper, f component.Factories) (*configmodels.Config, error) { - cfgConfig := defaultconfig.ComponentSettings{ - ComponentType: defaultconfig.Agent, - Factories: cmpts, - } - cfg, err := cfgConfig.CreateDefaultConfig() - if err != nil { - return nil, err - } - if len(builder.GetConfigFile()) > 0 { - otelCfg, err := service.FileLoaderConfigFactory(otelViper, f) - if err != nil { - return nil, err - } - err = defaultconfig.MergeConfigs(cfg, otelCfg) - if err != nil { - return nil, err - } - } - return cfg, nil + cfgConfig := defaultconfig.ComponentSettings{ + ComponentType: defaultconfig.Agent, + Factories: cmpts, } svc, err := service.New(service.Parameters{ ApplicationStartInfo: info, Factories: cmpts, - ConfigFactory: cfgFactory, + ConfigFactory: cfgConfig.DefaultConfigFactory(v), }) handleErr(err) diff --git a/cmd/opentelemetry/cmd/all-in-one/main.go b/cmd/opentelemetry/cmd/all-in-one/main.go index 69f715d0df0..c5b45451eb0 100644 --- a/cmd/opentelemetry/cmd/all-in-one/main.go +++ b/cmd/opentelemetry/cmd/all-in-one/main.go @@ -29,10 +29,8 @@ import ( "go.opentelemetry.io/collector/component" "go.opentelemetry.io/collector/config/configmodels" "go.opentelemetry.io/collector/service" - "go.opentelemetry.io/collector/service/builder" "go.uber.org/zap" - collectorApp "github.com/jaegertracing/jaeger/cmd/collector/app" jflags "github.com/jaegertracing/jaeger/cmd/flags" "github.com/jaegertracing/jaeger/cmd/opentelemetry/app" "github.com/jaegertracing/jaeger/cmd/opentelemetry/app/defaultcomponents" @@ -80,37 +78,16 @@ func main() { } cmpts := defaultcomponents.Components(v) - cfgFactory := func(otelViper *viper.Viper, f component.Factories) (*configmodels.Config, error) { - collectorOpts := &collectorApp.CollectorOptions{} - collectorOpts.InitFromViper(v) - cfgOpts := defaultconfig.ComponentSettings{ - ComponentType: defaultconfig.AllInOne, - Factories: cmpts, - StorageType: storageType, - ZipkinHostPort: collectorOpts.CollectorZipkinHTTPHostPort, - } - cfg, err := cfgOpts.CreateDefaultConfig() - if err != nil { - return nil, err - } - - if len(builder.GetConfigFile()) > 0 { - otelCfg, err := service.FileLoaderConfigFactory(otelViper, f) - if err != nil { - return nil, err - } - err = defaultconfig.MergeConfigs(cfg, otelCfg) - if err != nil { - return nil, err - } - } - return cfg, nil + cfgConfig := defaultconfig.ComponentSettings{ + ComponentType: defaultconfig.AllInOne, + Factories: cmpts, + StorageType: storageType, } svc, err := service.New(service.Parameters{ ApplicationStartInfo: info, Factories: cmpts, - ConfigFactory: cfgFactory, + ConfigFactory: cfgConfig.DefaultConfigFactory(v), }) handleErr(err) diff --git a/cmd/opentelemetry/cmd/collector/main.go b/cmd/opentelemetry/cmd/collector/main.go index 61597a8ed43..870f22bb169 100644 --- a/cmd/opentelemetry/cmd/collector/main.go +++ b/cmd/opentelemetry/cmd/collector/main.go @@ -21,11 +21,8 @@ import ( "github.com/spf13/viper" "go.opentelemetry.io/collector/component" - "go.opentelemetry.io/collector/config/configmodels" "go.opentelemetry.io/collector/service" - "go.opentelemetry.io/collector/service/builder" - collectorApp "github.com/jaegertracing/jaeger/cmd/collector/app" jflags "github.com/jaegertracing/jaeger/cmd/flags" "github.com/jaegertracing/jaeger/cmd/opentelemetry/app" "github.com/jaegertracing/jaeger/cmd/opentelemetry/app/defaultcomponents" @@ -59,39 +56,17 @@ func main() { if storageType == "" { storageType = "cassandra" } - cmpts := defaultcomponents.Components(v) - cfgFactory := func(otelViper *viper.Viper, f component.Factories) (*configmodels.Config, error) { - collectorOpts := &collectorApp.CollectorOptions{} - collectorOpts.InitFromViper(v) - cfgConfig := defaultconfig.ComponentSettings{ - ComponentType: defaultconfig.Collector, - Factories: cmpts, - StorageType: storageType, - ZipkinHostPort: collectorOpts.CollectorZipkinHTTPHostPort, - } - cfg, err := cfgConfig.CreateDefaultConfig() - if err != nil { - return nil, err - } - - if len(builder.GetConfigFile()) > 0 { - otelCfg, err := service.FileLoaderConfigFactory(otelViper, f) - if err != nil { - return nil, err - } - err = defaultconfig.MergeConfigs(cfg, otelCfg) - if err != nil { - return nil, err - } - } - return cfg, nil + cfgConfig := defaultconfig.ComponentSettings{ + ComponentType: defaultconfig.Collector, + Factories: cmpts, + StorageType: storageType, } svc, err := service.New(service.Parameters{ ApplicationStartInfo: info, Factories: cmpts, - ConfigFactory: cfgFactory, + ConfigFactory: cfgConfig.DefaultConfigFactory(v), }) handleErr(err) diff --git a/cmd/opentelemetry/cmd/ingester/main.go b/cmd/opentelemetry/cmd/ingester/main.go index 2dadc22e184..95a748d1f9e 100644 --- a/cmd/opentelemetry/cmd/ingester/main.go +++ b/cmd/opentelemetry/cmd/ingester/main.go @@ -21,9 +21,7 @@ import ( "github.com/spf13/viper" "go.opentelemetry.io/collector/component" - "go.opentelemetry.io/collector/config/configmodels" "go.opentelemetry.io/collector/service" - "go.opentelemetry.io/collector/service/builder" jflags "github.com/jaegertracing/jaeger/cmd/flags" "github.com/jaegertracing/jaeger/cmd/opentelemetry/app" @@ -60,35 +58,17 @@ func main() { if storageType == "" { storageType = "cassandra" } - cmpts := defaultcomponents.Components(v) - cfgFactory := func(otelViper *viper.Viper, f component.Factories) (*configmodels.Config, error) { - cfgConfig := defaultconfig.ComponentSettings{ - ComponentType: defaultconfig.Ingester, - Factories: cmpts, - StorageType: storageType, - } - cfg, err := cfgConfig.CreateDefaultConfig() - if err != nil { - return nil, err - } - if len(builder.GetConfigFile()) > 0 { - otelCfg, err := service.FileLoaderConfigFactory(otelViper, f) - if err != nil { - return nil, err - } - err = defaultconfig.MergeConfigs(cfg, otelCfg) - if err != nil { - return nil, err - } - } - return cfg, nil + cfgConfig := defaultconfig.ComponentSettings{ + ComponentType: defaultconfig.Ingester, + Factories: cmpts, + StorageType: storageType, } svc, err := service.New(service.Parameters{ ApplicationStartInfo: info, Factories: cmpts, - ConfigFactory: cfgFactory, + ConfigFactory: cfgConfig.DefaultConfigFactory(v), }) handleErr(err)