Rework Settings structure to fit all requirements.

Split it into ServiceSettings and ApplicationSettings.

Signed-off-by: Patryk Matyjasek <[email protected]>

Author: pmatyjasek-sumo

CHANGELOG.md

@@ -10,6 +10,7 @@
 - Remove `tracetranslator.TagHTTPStatusCode`, use `conventions.AttributeHTTPStatusCode` (#3111)
 - Remove OpenCensus status constants and transformation (#3110)
 - Remove `tracetranslator.AttributeArrayToSlice`, not used in core or contrib (#3109)
+- Introduce `ServiceSettings` and `ApplicationSettings` instead of `Parameters` (#3163)
 
 ## v0.26.0 Beta
 

cmd/otelcol/main.go

@@ -32,20 +32,21 @@ func main() {
 		log.Fatalf("failed to build default components: %v", err)
 	}
 
-	componentSettings := component.ComponentSettings{
+	commonSettings := service.CommonSettings{
 		BuildInfo: component.BuildInfo{
 			Command:     "otelcol",
 			Description: "OpenTelemetry Collector",
 			Version:     version.Version,
 		},
+		Factories: factories,
 	}
 
-	if err := run(service.Settings{ComponentSettings: componentSettings, Factories: factories}); err != nil {
+	if err := run(service.ApplicationSettings{CommonSettings: commonSettings}); err != nil {
 		log.Fatal(err)
 	}
 }
 
-func runInteractive(settings service.Settings) error {
+func runInteractive(settings service.ApplicationSettings) error {
 	app, err := service.New(settings)
 	if err != nil {
 		return fmt.Errorf("failed to construct the application: %w", err)

cmd/otelcol/main_others.go

@@ -18,6 +18,6 @@ package main
 
 import "go.opentelemetry.io/collector/service"
 
-func run(settings service.Settings) error {
+func run(settings service.ApplicationSettings) error {
 	return runInteractive(settings)
 }

cmd/otelcol/main_windows.go

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

service/application.go

@@ -80,20 +80,20 @@ type Application struct {
 }
 
 // New creates and returns a new instance of Application.
-func New(set Settings) (*Application, error) {
-	if err := configcheck.ValidateConfigFromFactories(set.Factories); err != nil {
+func New(set ApplicationSettings) (*Application, error) {
+	if err := configcheck.ValidateConfigFromFactories(set.CommonSettings.Factories); err != nil {
 		return nil, err
 	}
 
 	app := &Application{
-		info:         set.ComponentSettings.BuildInfo,
-		factories:    set.Factories,
+		info:         set.CommonSettings.BuildInfo,
+		factories:    set.CommonSettings.Factories,
 		stateChannel: make(chan State, Closed+1),
 	}
 
 	rootCmd := &cobra.Command{
-		Use:     set.ComponentSettings.BuildInfo.Command,
-		Version: set.ComponentSettings.BuildInfo.Version,
+		Use:     set.CommonSettings.BuildInfo.Command,
+		Version: set.CommonSettings.BuildInfo.Version,
 		RunE: func(cmd *cobra.Command, args []string) error {
 			var err error
 			if app.logger, err = newLogger(set.LoggingOptions); err != nil {
@@ -223,15 +223,16 @@ func (app *Application) setupConfigurationComponents(ctx context.Context) error
 	}
 
 	app.logger.Info("Applying configuration...")
-	componentSettings := component.ComponentSettings{
+
+	commonSettings := CommonSettings{
 		BuildInfo: app.info,
-		Logger:    app.logger,
+		Factories: app.factories,
 	}
 
-	service, err := newService(&Settings{
-		Factories:         app.factories,
-		ComponentSettings: componentSettings,
+	service, err := newService(&ServiceSettings{
+		CommonSettings:    commonSettings,
 		Config:            cfg,
+		Logger:            app.logger,
 		AsyncErrorChannel: app.asyncErrorChannel,
 	})
 	if err != nil {

service/application_test.go

@@ -51,11 +51,12 @@ func TestApplication_Start(t *testing.T) {
 		return nil
 	}
 
-	componentSettings := component.ComponentSettings{
+	commonSettings := CommonSettings{
 		BuildInfo: component.DefaultBuildInfo(),
+		Factories: factories,
 	}
 
-	app, err := New(Settings{Factories: factories, ComponentSettings: componentSettings, LoggingOptions: []zap.Option{zap.Hooks(hook)}})
+	app, err := New(ApplicationSettings{CommonSettings: commonSettings, LoggingOptions: []zap.Option{zap.Hooks(hook)}})
 	require.NoError(t, err)
 	assert.Equal(t, app.rootCmd, app.Command())
 
@@ -123,11 +124,12 @@ func TestApplication_ReportError(t *testing.T) {
 	factories, err := defaultcomponents.Components()
 	require.NoError(t, err)
 
-	componentSettings := component.ComponentSettings{
+	commonSettings := CommonSettings{
 		BuildInfo: component.DefaultBuildInfo(),
+		Factories: factories,
 	}
 
-	app, err := New(Settings{Factories: factories, ComponentSettings: componentSettings})
+	app, err := New(ApplicationSettings{CommonSettings: commonSettings})
 	require.NoError(t, err)
 
 	app.rootCmd.SetArgs([]string{"--config=testdata/otelcol-config-minimal.yaml"})
@@ -150,14 +152,14 @@ func TestApplication_StartAsGoRoutine(t *testing.T) {
 	factories, err := defaultcomponents.Components()
 	require.NoError(t, err)
 
-	componentSettings := component.ComponentSettings{
+	commonSettings := CommonSettings{
 		BuildInfo: component.DefaultBuildInfo(),
+		Factories: factories,
 	}
 
-	params := Settings{
-		ComponentSettings: componentSettings,
-		ParserProvider:    new(minimalParserLoader),
-		Factories:         factories,
+	params := ApplicationSettings{
+		CommonSettings: commonSettings,
+		ParserProvider: new(minimalParserLoader),
 	}
 	app, err := New(params)
 	require.NoError(t, err)

service/application_windows.go

@@ -27,12 +27,12 @@ import (
 )
 
 type WindowsService struct {
-	settings Settings
+	settings ApplicationSettings
 	app      *Application
 }
 
-func NewWindowsService(settings Settings) *WindowsService {
-	return &WindowsService{settings: settings}
+func NewWindowsService(set ApplicationSettings) *WindowsService {
+	return &WindowsService{settings: set}
 }
 
 // Execute implements https://godoc.org/golang.org/x/sys/windows/svc#Handler
@@ -120,9 +120,9 @@ func openEventLog(serviceName string) (*eventlog.Log, error) {
 	return elog, nil
 }
 
-func newWithEventViewerLoggingHook(settings Settings, elog *eventlog.Log) (*Application, error) {
-	settings.LoggingOptions = append(
-		settings.LoggingOptions,
+func newWithEventViewerLoggingHook(set ApplicationSettings, elog *eventlog.Log) (*Application, error) {
+	set.LoggingOptions = append(
+		set.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(settings Settings, elog *eventlog.Log) (*Appl
 		}),
 	)
 
-	return New(settings)
+	return New(set)
 }

service/service.go

@@ -24,33 +24,8 @@ 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 {
-	// Factories component factories.
-	Factories component.Factories
-
-	// ComponentSettings contains logger and build info configuration
-	ComponentSettings component.ComponentSettings
-
-	// Config represents the configuration of the service.
-	Config *config.Config
-
-	// 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
-}
-
 // service represents the implementation of a component.Host.
 type service struct {
 	factories         component.Factories
@@ -65,13 +40,13 @@ type service struct {
 	builtExtensions builder.Extensions
 }
 
-func newService(settings *Settings) (*service, error) {
+func newService(set *ServiceSettings) (*service, error) {
 	srv := &service{
-		factories:         settings.Factories,
-		buildInfo:         settings.ComponentSettings.BuildInfo,
-		config:            settings.Config,
-		logger:            settings.ComponentSettings.Logger,
-		asyncErrorChannel: settings.AsyncErrorChannel,
+		factories:         set.CommonSettings.Factories,
+		buildInfo:         set.CommonSettings.BuildInfo,
+		config:            set.Config,
+		logger:            set.Logger,
+		asyncErrorChannel: set.AsyncErrorChannel,
 	}
 
 	if err := srv.config.Validate(); err != nil {

service/service_test.go

@@ -103,15 +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)
-	componentSettings := component.ComponentSettings{
+	commonSettings := CommonSettings{
 		BuildInfo: component.DefaultBuildInfo(),
-		Logger:    zap.NewNop(),
+		Factories: factories,
 	}
 
-	srv, err := newService(&Settings{
-		Factories:         factories,
-		ComponentSettings: componentSettings,
-		Config:            cfg,
+	srv, err := newService(&ServiceSettings{
+		CommonSettings: commonSettings,
+		Logger:         zap.NewNop(),
+		Config:         cfg,
 	})
 	require.NoError(t, err)
 	return srv

service/settings.go

@@ -0,0 +1,61 @@
+// Copyright The OpenTelemetry Authors
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//       http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package service
+
+import (
+	"go.opentelemetry.io/collector/component"
+	"go.opentelemetry.io/collector/config"
+	"go.opentelemetry.io/collector/service/parserprovider"
+	"go.uber.org/zap"
+)
+
+// CommonSettings holds common settings for Service and Application
+type CommonSettings struct {
+	// Factories component factories.
+	Factories component.Factories
+
+	// BuildInfo provides application start information.
+	BuildInfo component.BuildInfo
+}
+
+// ServiceSettings holds configuration for building a new service.
+type ServiceSettings struct {
+	// CommonSettings contains Factories and BuildInfo
+	CommonSettings CommonSettings
+
+	// Config represents the configuration of the service.
+	Config *config.Config
+
+	// Logger represents the logger used for all the components.
+	Logger *zap.Logger
+
+	// AsyncErrorChannel is the channel that is used to report fatal errors.
+	AsyncErrorChannel chan error
+}
+
+// ApplicationSettings holds configuration for creating a new Application.
+type ApplicationSettings struct {
+	// CommonSettings contains Factories and BuildInfo
+	CommonSettings CommonSettings
+
+	// 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
+}

testbed/testbed/otelcol_runner.go

@@ -84,16 +84,16 @@ func (ipp *InProcessCollector) PrepareConfig(configStr string) (configCleanup fu
 }
 
 func (ipp *InProcessCollector) Start(args StartParams) error {
-	componentSettings := component.ComponentSettings{
+	commonSettings := service.CommonSettings{
 		BuildInfo: component.BuildInfo{
 			Command: "otelcol",
 			Version: version.Version,
 		},
+		Factories: ipp.factories,
 	}
-	settings := service.Settings{
-		ComponentSettings: componentSettings,
-		ParserProvider:    parserprovider.NewInMemory(strings.NewReader(ipp.configStr)),
-		Factories:         ipp.factories,
+	settings := service.ApplicationSettings{
+		CommonSettings: commonSettings,
+		ParserProvider: parserprovider.NewInMemory(strings.NewReader(ipp.configStr)),
 	}
 	var err error
 	ipp.svc, err = service.New(settings)