Standarize Settings, Params and Parameters

Replace Parameters and settings structs with new struct Settings Replace dependencies in service and main Update tests Signed-off-by: Patryk Matyjasek <[email protected]>
open-telemetry · May 13, 2021 · 98748f7 · 98748f7
1 parent ca20d0e
commit 98748f7
Show file tree

Hide file tree

Showing 12 changed files with 102 additions and 86 deletions.
diff --git a/cmd/otelcol/main.go b/cmd/otelcol/main.go
@@ -32,19 +32,21 @@ func main() {
 		log.Fatalf("failed to build default components: %v", err)
 	}
 
-	info := component.BuildInfo{
-		Command:     "otelcol",
-		Description: "OpenTelemetry Collector",
-		Version:     version.Version,
+	componentSettings := component.ComponentSettings{
+		BuildInfo: component.BuildInfo{
+			Command:     "otelcol",
+			Description: "OpenTelemetry Collector",
+			Version:     version.Version,
+		},
 	}
 
-	if err := run(service.Parameters{BuildInfo: info, Factories: factories}); err != nil {
+	if err := run(service.Settings{ComponentSettings: componentSettings, Factories: factories}); err != nil {
 		log.Fatal(err)
 	}
 }
 
-func runInteractive(params service.Parameters) error {
-	app, err := service.New(params)
+func runInteractive(settings service.Settings) error {
+	app, err := service.New(settings)
 	if err != nil {
 		return fmt.Errorf("failed to construct the application: %w", err)
 	}

diff --git a/cmd/otelcol/main_others.go b/cmd/otelcol/main_others.go
@@ -18,6 +18,6 @@ package main
 
 import "go.opentelemetry.io/collector/service"
 
-func run(params service.Parameters) error {
-	return runInteractive(params)
+func run(settings service.Settings) error {
+	return runInteractive(settings)
 }
diff --git a/cmd/otelcol/main_windows.go b/cmd/otelcol/main_windows.go
@@ -25,13 +25,13 @@ import (
 	"go.opentelemetry.io/collector/service"
 )
 
-func run(params service.Parameters) error {
+func run(settings service.Settings) error {
 	if useInteractiveMode, err := checkUseInteractiveMode(); err != nil {
 		return err
 	} else if useInteractiveMode {
-		return runInteractive(params)
+		return runInteractive(settings)
 	} else {
-		return runService(params)
+		return runService(settings)
 	}
 }
 
@@ -51,9 +51,9 @@ func checkUseInteractiveMode() (bool, error) {
 	}
 }
 
-func runService(params service.Parameters) error {
+func runService(settings service.Settings) error {
 	// do not need to supply service name when startup is invoked through Service Control Manager directly
-	if err := svc.Run("", service.NewWindowsService(params)); err != nil {
+	if err := svc.Run("", service.NewWindowsService(settings)); err != nil {
 		return fmt.Errorf("failed to start service %w", err)
 	}
 

diff --git a/component/component.go b/component/component.go
@@ -18,6 +18,7 @@ import (
 	"context"
 
 	"go.opentelemetry.io/collector/config"
+	"go.uber.org/zap"
 )
 
 // Component is either a receiver, exporter, processor or an extension.
@@ -60,6 +61,16 @@ type Component interface {
 	Shutdown(ctx context.Context) error
 }
 
+// ComponentSettings is passed to ReceiverFactory.Create* functions.
+type ComponentSettings struct {
+	// Logger that the factory can use during creation and can pass to the created
+	// component to be used later as well.
+	Logger *zap.Logger
+
+	// BuildInfo can be used by components for informational purposes
+	BuildInfo BuildInfo
+}
+
 // Kind represents component kinds.
 type Kind int
 

diff --git a/service/application.go b/service/application.go
@@ -79,39 +79,24 @@ type Application struct {
 	asyncErrorChannel chan error
 }
 
-// Parameters holds configuration for creating a new Application.
-type Parameters struct {
-	// Factories component factories.
-	Factories component.Factories
-	// BuildInfo provides application start information.
-	BuildInfo component.BuildInfo
-	// ParserProvider provides the configuration's Parser.
-	// If it is not provided a default provider is used. The default provider loads the configuration
-	// from a config file define by the --config command line flag and overrides component's configuration
-	// properties supplied via --set command line flag.
-	ParserProvider parserprovider.ParserProvider
-	// LoggingOptions provides a way to change behavior of zap logging.
-	LoggingOptions []zap.Option
-}
-
 // New creates and returns a new instance of Application.
-func New(params Parameters) (*Application, error) {
-	if err := configcheck.ValidateConfigFromFactories(params.Factories); err != nil {
+func New(set Settings) (*Application, error) {
+	if err := configcheck.ValidateConfigFromFactories(set.Factories); err != nil {
 		return nil, err
 	}
 
 	app := &Application{
-		info:         params.BuildInfo,
-		factories:    params.Factories,
+		info:         set.ComponentSettings.BuildInfo,
+		factories:    set.Factories,
 		stateChannel: make(chan State, Closed+1),
 	}
 
 	rootCmd := &cobra.Command{
-		Use:     params.BuildInfo.Command,
-		Version: params.BuildInfo.Version,
+		Use:     set.ComponentSettings.BuildInfo.Command,
+		Version: set.ComponentSettings.BuildInfo.Version,
 		RunE: func(cmd *cobra.Command, args []string) error {
 			var err error
-			if app.logger, err = newLogger(params.LoggingOptions); err != nil {
+			if app.logger, err = newLogger(set.LoggingOptions); err != nil {
 				return fmt.Errorf("failed to get logger: %w", err)
 			}
 
@@ -138,7 +123,7 @@ func New(params Parameters) (*Application, error) {
 	rootCmd.Flags().AddGoFlagSet(flagSet)
 	app.rootCmd = rootCmd
 
-	parserProvider := params.ParserProvider
+	parserProvider := set.ParserProvider
 	if parserProvider == nil {
 		// use default provider.
 		parserProvider = parserprovider.Default()
@@ -238,12 +223,15 @@ func (app *Application) setupConfigurationComponents(ctx context.Context) error
 	}
 
 	app.logger.Info("Applying configuration...")
+	componentSettings := component.ComponentSettings{
+		BuildInfo: app.info,
+		Logger:    app.logger,
+	}
 
-	service, err := newService(&settings{
+	service, err := newService(&Settings{
 		Factories:         app.factories,
-		BuildInfo:         app.info,
+		ComponentSettings: componentSettings,
 		Config:            cfg,
-		Logger:            app.logger,
 		AsyncErrorChannel: app.asyncErrorChannel,
 	})
 	if err != nil {

diff --git a/service/application_test.go b/service/application_test.go
@@ -51,7 +51,11 @@ func TestApplication_Start(t *testing.T) {
 		return nil
 	}
 
-	app, err := New(Parameters{Factories: factories, BuildInfo: component.DefaultBuildInfo(), LoggingOptions: []zap.Option{zap.Hooks(hook)}})
+	componentSettings := component.ComponentSettings{
+		BuildInfo: component.DefaultBuildInfo(),
+	}
+
+	app, err := New(Settings{Factories: factories, ComponentSettings: componentSettings, LoggingOptions: []zap.Option{zap.Hooks(hook)}})
 	require.NoError(t, err)
 	assert.Equal(t, app.rootCmd, app.Command())
 
@@ -119,7 +123,11 @@ func TestApplication_ReportError(t *testing.T) {
 	factories, err := defaultcomponents.Components()
 	require.NoError(t, err)
 
-	app, err := New(Parameters{Factories: factories, BuildInfo: component.DefaultBuildInfo()})
+	componentSettings := component.ComponentSettings{
+		BuildInfo: component.DefaultBuildInfo(),
+	}
+
+	app, err := New(Settings{Factories: factories, ComponentSettings: componentSettings})
 	require.NoError(t, err)
 
 	app.rootCmd.SetArgs([]string{"--config=testdata/otelcol-config-minimal.yaml"})
@@ -142,10 +150,14 @@ func TestApplication_StartAsGoRoutine(t *testing.T) {
 	factories, err := defaultcomponents.Components()
 	require.NoError(t, err)
 
-	params := Parameters{
-		BuildInfo:      component.DefaultBuildInfo(),
-		ParserProvider: new(minimalParserLoader),
-		Factories:      factories,
+	componentSettings := component.ComponentSettings{
+		BuildInfo: component.DefaultBuildInfo(),
+	}
+
+	params := Settings{
+		ComponentSettings: componentSettings,
+		ParserProvider:    new(minimalParserLoader),
+		Factories:         factories,
 	}
 	app, err := New(params)
 	require.NoError(t, err)

diff --git a/service/application_windows.go b/service/application_windows.go
@@ -27,12 +27,12 @@ import (
 )
 
 type WindowsService struct {
-	params Parameters
-	app    *Application
+	settings Settings
+	app      *Application
 }
 
-func NewWindowsService(params Parameters) *WindowsService {
-	return &WindowsService{params: params}
+func NewWindowsService(settings Settings) *WindowsService {
+	return &WindowsService{settings: settings}
 }
 
 // Execute implements https://godoc.org/golang.org/x/sys/windows/svc#Handler
@@ -81,7 +81,7 @@ func (s *WindowsService) Execute(args []string, requests <-chan svc.ChangeReques
 
 func (s *WindowsService) start(elog *eventlog.Log, appErrorChannel chan error) error {
 	var err error
-	s.app, err = newWithEventViewerLoggingHook(s.params, elog)
+	s.app, err = newWithEventViewerLoggingHook(s.settings, elog)
 	if err != nil {
 		return err
 	}
@@ -120,9 +120,9 @@ func openEventLog(serviceName string) (*eventlog.Log, error) {
 	return elog, nil
 }
 
-func newWithEventViewerLoggingHook(params Parameters, elog *eventlog.Log) (*Application, error) {
-	params.LoggingOptions = append(
-		params.LoggingOptions,
+func newWithEventViewerLoggingHook(settings Settings, elog *eventlog.Log) (*Application, error) {
+	settings.LoggingOptions = append(
+		settings.LoggingOptions,
 		zap.Hooks(func(entry zapcore.Entry) error {
 			msg := fmt.Sprintf("%v\r\n\r\nStack Trace:\r\n%v", entry.Message, entry.Stack)
 
@@ -143,5 +143,5 @@ func newWithEventViewerLoggingHook(params Parameters, elog *eventlog.Log) (*Appl
 		}),
 	)
 
-	return New(params)
+	return New(settings)
 }
diff --git a/service/application_windows_test.go b/service/application_windows_test.go
@@ -34,7 +34,11 @@ func TestWindowsService_Execute(t *testing.T) {
 	factories, err := defaultcomponents.Components()
 	require.NoError(t, err)
 
-	s := NewWindowsService(Parameters{Factories: factories, BuildInfo: component.DefaultBuildInfo()})
+	componentSettings := component.ComponentSettings{
+		BuildInfo: component.DefaultBuildInfo(),
+	}
+
+	s := NewWindowsService(Settings{Factories: factories, ComponentSettings: componentSettings})
 
 	appDone := make(chan struct{})
 	requests := make(chan svc.ChangeRequest)

diff --git a/service/internal/zpages/tmplgen/resources.go b/service/internal/zpages/tmplgen/resources.go
diff --git a/service/service.go b/service/service.go
@@ -24,21 +24,28 @@ import (
 	"go.opentelemetry.io/collector/config"
 	"go.opentelemetry.io/collector/consumer/consumererror"
 	"go.opentelemetry.io/collector/service/internal/builder"
+	"go.opentelemetry.io/collector/service/parserprovider"
 )
 
 // settings holds configuration for building a new service.
-type settings struct {
+type Settings struct {
 	// Factories component factories.
 	Factories component.Factories
 
-	// BuildInfo provides application start information.
-	BuildInfo component.BuildInfo
+	// ComponentSettings contains logger and build info configuration
+	ComponentSettings component.ComponentSettings
 
 	// Config represents the configuration of the service.
 	Config *config.Config
 
-	// Logger represents the logger used for all the components.
-	Logger *zap.Logger
+	// ParserProvider provides the configuration's Parser.
+	// If it is not provided a default provider is used. The default provider loads the configuration
+	// from a config file define by the --config command line flag and overrides component's configuration
+	// properties supplied via --set command line flag.
+	ParserProvider parserprovider.ParserProvider
+
+	// LoggingOptions provides a way to change behavior of zap logging.
+	LoggingOptions []zap.Option
 
 	// AsyncErrorChannel is the channel that is used to report fatal errors.
 	AsyncErrorChannel chan error
@@ -58,12 +65,12 @@ type service struct {
 	builtExtensions builder.Extensions
 }
 
-func newService(settings *settings) (*service, error) {
+func newService(settings *Settings) (*service, error) {
 	srv := &service{
 		factories:         settings.Factories,
-		buildInfo:         settings.BuildInfo,
+		buildInfo:         settings.ComponentSettings.BuildInfo,
 		config:            settings.Config,
-		logger:            settings.Logger,
+		logger:            settings.ComponentSettings.Logger,
 		asyncErrorChannel: settings.AsyncErrorChannel,
 	}
 

diff --git a/service/service_test.go b/service/service_test.go
@@ -103,12 +103,15 @@ func createExampleService(t *testing.T) *service {
 	require.NoError(t, err)
 	cfg, err := configtest.LoadConfigFile(t, path.Join(".", "testdata", "otelcol-nop.yaml"), factories)
 	require.NoError(t, err)
-
-	srv, err := newService(&settings{
-		Factories: factories,
+	componentSettings := component.ComponentSettings{
 		BuildInfo: component.DefaultBuildInfo(),
-		Config:    cfg,
 		Logger:    zap.NewNop(),
+	}
+
+	srv, err := newService(&Settings{
+		Factories:         factories,
+		ComponentSettings: componentSettings,
+		Config:            cfg,
 	})
 	require.NoError(t, err)
 	return srv

diff --git a/testbed/testbed/otelcol_runner.go b/testbed/testbed/otelcol_runner.go
@@ -84,16 +84,19 @@ func (ipp *InProcessCollector) PrepareConfig(configStr string) (configCleanup fu
 }
 
 func (ipp *InProcessCollector) Start(args StartParams) error {
-	params := service.Parameters{
+	componentSettings := component.ComponentSettings{
 		BuildInfo: component.BuildInfo{
 			Command: "otelcol",
 			Version: version.Version,
 		},
-		ParserProvider: parserprovider.NewInMemory(strings.NewReader(ipp.configStr)),
-		Factories:      ipp.factories,
+	}
+	settings := service.Settings{
+		ComponentSettings: componentSettings,
+		ParserProvider:    parserprovider.NewInMemory(strings.NewReader(ipp.configStr)),
+		Factories:         ipp.factories,
 	}
 	var err error
-	ipp.svc, err = service.New(params)
+	ipp.svc, err = service.New(settings)
 	if err != nil {
 		return err
 	}