Standarize Settings, Params and Parameters (#3163)

* 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]>

# Conflicts:
#	service/application_windows.go

* Fix unnecessary file

* Rework Settings structure to fit all requirements.
Split it into ServiceSettings and ApplicationSettings.

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

# Conflicts:
#	CHANGELOG.md
#	service/application_windows.go

* Fix linter issues:
- Fix import orders
- Rename stutter structs

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

# Conflicts:
#	CHANGELOG.md
#	service/application_windows.go

* Remove CommonSettings struct,
Make SvcSettings private

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

* Remove unused Settings struct.
Fix Windows tests

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

* Update CHANGELOG.md

Co-authored-by: Bogdan Drutu <[email protected]>

Author: pmatyjasek-sumo

CHANGELOG.md

@@ -5,6 +5,7 @@
 ## 🛑 Breaking changes 🛑
 
 - Remove unused logstest package (#3222)
+- Introduce `AppSettings` instead of `Parameters` (#3163)
 
 ## v0.27.0 Beta
 

cmd/otelcol/main.go

@@ -31,20 +31,19 @@ func main() {
 	if err != nil {
 		log.Fatalf("failed to build default components: %v", err)
 	}
-
 	info := 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.AppSettings{BuildInfo: info, Factories: factories}); err != nil {
 		log.Fatal(err)
 	}
 }
 
-func runInteractive(params service.Parameters) error {
-	app, err := service.New(params)
+func runInteractive(settings service.AppSettings) 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(params service.Parameters) error {
-	return runInteractive(params)
+func run(settings service.AppSettings) error {
+	return runInteractive(settings)
 }

cmd/otelcol/main_windows.go

@@ -25,13 +25,13 @@ import (
 	"go.opentelemetry.io/collector/service"
 )
 
-func run(params service.Parameters) error {
+func run(set service.AppSettings) error {
 	if useInteractiveMode, err := checkUseInteractiveMode(); err != nil {
 		return err
 	} else if useInteractiveMode {
-		return runInteractive(params)
+		return runInteractive(set)
 	} else {
-		return runService(params)
+		return runService(set)
 	}
 }
 
@@ -51,9 +51,9 @@ func checkUseInteractiveMode() (bool, error) {
 	}
 }
 
-func runService(params service.Parameters) error {
+func runService(set service.AppSettings) 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(set)); err != nil {
 		return fmt.Errorf("failed to start service %w", err)
 	}
 

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 AppSettings) (*Application, error) {
+	if err := configcheck.ValidateConfigFromFactories(set.Factories); err != nil {
 		return nil, err
 	}
 
 	app := &Application{
-		info:         params.BuildInfo,
-		factories:    params.Factories,
+		info:         set.BuildInfo,
+		factories:    set.Factories,
 		stateChannel: make(chan State, Closed+1),
 	}
 
 	rootCmd := &cobra.Command{
-		Use:     params.BuildInfo.Command,
-		Version: params.BuildInfo.Version,
+		Use:     set.BuildInfo.Command,
+		Version: set.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)
 			}
 
@@ -134,7 +119,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()
@@ -235,9 +220,9 @@ func (app *Application) setupConfigurationComponents(ctx context.Context) error
 
 	app.logger.Info("Applying configuration...")
 
-	service, err := newService(&settings{
-		Factories:         app.factories,
+	service, err := newService(&svcSettings{
 		BuildInfo:         app.info,
+		Factories:         app.factories,
 		Config:            cfg,
 		Logger:            app.logger,
 		AsyncErrorChannel: app.asyncErrorChannel,

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)}})
+	app, err := New(AppSettings{
+		BuildInfo:      component.DefaultBuildInfo(),
+		Factories:      factories,
+		LoggingOptions: []zap.Option{zap.Hooks(hook)},
+	})
 	require.NoError(t, err)
 	assert.Equal(t, app.rootCmd, app.Command())
 
@@ -119,7 +123,7 @@ func TestApplication_ReportError(t *testing.T) {
 	factories, err := defaultcomponents.Components()
 	require.NoError(t, err)
 
-	app, err := New(Parameters{Factories: factories, BuildInfo: component.DefaultBuildInfo()})
+	app, err := New(AppSettings{BuildInfo: component.DefaultBuildInfo(), Factories: factories})
 	require.NoError(t, err)
 
 	app.rootCmd.SetArgs([]string{"--config=testdata/otelcol-config-minimal.yaml"})
@@ -142,12 +146,12 @@ func TestApplication_StartAsGoRoutine(t *testing.T) {
 	factories, err := defaultcomponents.Components()
 	require.NoError(t, err)
 
-	params := Parameters{
+	set := AppSettings{
 		BuildInfo:      component.DefaultBuildInfo(),
-		ParserProvider: new(minimalParserLoader),
 		Factories:      factories,
+		ParserProvider: new(minimalParserLoader),
 	}
-	app, err := New(params)
+	app, err := New(set)
 	require.NoError(t, err)
 
 	appDone := make(chan struct{})

service/application_windows.go

@@ -27,12 +27,12 @@ import (
 )
 
 type WindowsService struct {
-	params Parameters
-	app    *Application
+	settings AppSettings
+	app      *Application
 }
 
-func NewWindowsService(params Parameters) *WindowsService {
-	return &WindowsService{params: params}
+func NewWindowsService(set AppSettings) *WindowsService {
+	return &WindowsService{settings: set}
 }
 
 // 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 = newWithWindowsEventLogCore(s.params, elog)
+	s.app, err = newWithWindowsEventLogCore(s.settings, elog)
 	if err != nil {
 		return err
 	}
@@ -120,12 +120,12 @@ func openEventLog(serviceName string) (*eventlog.Log, error) {
 	return elog, nil
 }
 
-func newWithWindowsEventLogCore(params Parameters, elog *eventlog.Log) (*Application, error) {
-	params.LoggingOptions = append(
-		params.LoggingOptions,
+func newWithWindowsEventLogCore(set AppSettings, elog *eventlog.Log) (*Application, error) {
+	set.LoggingOptions = append(
+		set.LoggingOptions,
 		zap.WrapCore(withWindowsCore(elog)),
 	)
-	return New(params)
+	return New(set)
 }
 
 var _ zapcore.Core = (*windowsEventLogCore)(nil)

service/application_windows_test.go

@@ -34,7 +34,7 @@ func TestWindowsService_Execute(t *testing.T) {
 	factories, err := defaultcomponents.Components()
 	require.NoError(t, err)
 
-	s := NewWindowsService(Parameters{Factories: factories, BuildInfo: component.DefaultBuildInfo()})
+	s := NewWindowsService(AppSettings{BuildInfo: component.DefaultBuildInfo(), Factories: factories})
 
 	appDone := make(chan struct{})
 	requests := make(chan svc.ChangeRequest)

service/service.go

@@ -26,24 +26,6 @@ import (
 	"go.opentelemetry.io/collector/service/internal/builder"
 )
 
-// settings holds configuration for building a new service.
-type settings struct {
-	// Factories component factories.
-	Factories component.Factories
-
-	// BuildInfo provides application start information.
-	BuildInfo component.BuildInfo
-
-	// 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
-}
-
 // service represents the implementation of a component.Host.
 type service struct {
 	factories         component.Factories
@@ -58,13 +40,13 @@ type service struct {
 	builtExtensions builder.Extensions
 }
 
-func newService(settings *settings) (*service, error) {
+func newService(set *svcSettings) (*service, error) {
 	srv := &service{
-		factories:         settings.Factories,
-		buildInfo:         settings.BuildInfo,
-		config:            settings.Config,
-		logger:            settings.Logger,
-		asyncErrorChannel: settings.AsyncErrorChannel,
+		factories:         set.Factories,
+		buildInfo:         set.BuildInfo,
+		config:            set.Config,
+		logger:            set.Logger,
+		asyncErrorChannel: set.AsyncErrorChannel,
 	}
 
 	if err := srv.config.Validate(); err != nil {

service/service_test.go

@@ -104,11 +104,11 @@ func createExampleService(t *testing.T) *service {
 	cfg, err := configtest.LoadConfigFile(t, path.Join(".", "testdata", "otelcol-nop.yaml"), factories)
 	require.NoError(t, err)
 
-	srv, err := newService(&settings{
-		Factories: factories,
+	srv, err := newService(&svcSettings{
 		BuildInfo: component.DefaultBuildInfo(),
-		Config:    cfg,
+		Factories: factories,
 		Logger:    zap.NewNop(),
+		Config:    cfg,
 	})
 	require.NoError(t, err)
 	return srv

service/settings.go

@@ -0,0 +1,59 @@
+// 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.uber.org/zap"
+
+	"go.opentelemetry.io/collector/component"
+	"go.opentelemetry.io/collector/config"
+	"go.opentelemetry.io/collector/service/parserprovider"
+)
+
+// svcSettings holds configuration for building a new service.
+type svcSettings struct {
+	// Factories component factories.
+	Factories component.Factories
+
+	// BuildInfo provides application start information.
+	BuildInfo component.BuildInfo
+
+	// 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
+}
+
+// AppSettings holds configuration for creating a new Application.
+type AppSettings 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
+}

testbed/testbed/otelcol_runner.go

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