diff --git a/CHANGELOG.md b/CHANGELOG.md index ae6f21ce44d..5ba07213dda 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,6 +5,7 @@ ## 🛑 Breaking changes 🛑 - Remove unused logstest package (#3222) +- Introduce `AppSettings` instead of `Parameters` (#3163) ## v0.27.0 Beta diff --git a/cmd/otelcol/main.go b/cmd/otelcol/main.go index 2ed6336fd22..524e94d5da9 100644 --- a/cmd/otelcol/main.go +++ b/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) } diff --git a/cmd/otelcol/main_others.go b/cmd/otelcol/main_others.go index e1957c2fb71..1998328880b 100644 --- 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.AppSettings) error { + return runInteractive(settings) } diff --git a/cmd/otelcol/main_windows.go b/cmd/otelcol/main_windows.go index c066c39616b..e5e600898bc 100644 --- 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(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) } diff --git a/service/application.go b/service/application.go index 63066eb09df..fb662339257 100644 --- 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 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, diff --git a/service/application_test.go b/service/application_test.go index 40a2f60f59a..61afc86ca58 100644 --- 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)}}) + 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{}) diff --git a/service/application_windows.go b/service/application_windows.go index 69e9b741b63..5420ab3c481 100644 --- a/service/application_windows.go +++ b/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) diff --git a/service/application_windows_test.go b/service/application_windows_test.go index d665969917f..f4c92223aa6 100644 --- a/service/application_windows_test.go +++ b/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) diff --git a/service/service.go b/service/service.go index f3b88c97e2e..b15458d6889 100644 --- a/service/service.go +++ b/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 { diff --git a/service/service_test.go b/service/service_test.go index 2d1631a2217..44a452007e9 100644 --- a/service/service_test.go +++ b/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 diff --git a/service/settings.go b/service/settings.go new file mode 100644 index 00000000000..5cae7cbb7f6 --- /dev/null +++ b/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 +} diff --git a/testbed/testbed/otelcol_runner.go b/testbed/testbed/otelcol_runner.go index 63e53efb36c..80a2c97310c 100644 --- a/testbed/testbed/otelcol_runner.go +++ b/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 }