Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Standarize Settings, Params and Parameters #3163

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
## 🛑 Breaking changes 🛑

- Remove unused logstest package (#3222)
- Introduce `AppSettings` instead of `Parameters` (#3163)

## v0.27.0 Beta

Expand Down
7 changes: 3 additions & 4 deletions cmd/otelcol/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
Expand Down
4 changes: 2 additions & 2 deletions cmd/otelcol/main_others.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
10 changes: 5 additions & 5 deletions cmd/otelcol/main_windows.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
}

Expand All @@ -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)
}

Expand Down
35 changes: 10 additions & 25 deletions service/application.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}

Expand All @@ -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()
Expand Down Expand Up @@ -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,
Expand Down
14 changes: 9 additions & 5 deletions service/application_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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())

Expand Down Expand Up @@ -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"})
Expand All @@ -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{})
Expand Down
18 changes: 9 additions & 9 deletions service/application_windows.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
}
Expand Down Expand Up @@ -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)
Expand Down
2 changes: 1 addition & 1 deletion service/application_windows_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
30 changes: 6 additions & 24 deletions service/service.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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 {
Expand Down
6 changes: 3 additions & 3 deletions service/service_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
59 changes: 59 additions & 0 deletions service/settings.go
Original file line number Diff line number Diff line change
@@ -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
}
6 changes: 3 additions & 3 deletions testbed/testbed/otelcol_runner.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
Expand Down