OTel: Factored out Config Factory (#2495)

* Factored out default config factory

Signed-off-by: Joe Elliott <[email protected]>

* lint

Signed-off-by: Joe Elliott <[email protected]>

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

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)

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
 }
 

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) {

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)
 

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)
 

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)
 

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)