Skip to content

Commit

Permalink
Rework Settings structure to fit all requirements.
Browse files Browse the repository at this point in the history
Split it into ServiceSettings and ApplicationSettings.

Signed-off-by: Patryk Matyjasek <pmatyjasek@sumologic.com>
  • Loading branch information
pmatyjasek-sumo committed May 18, 2021
1 parent 0f5ba54 commit 22bcedc
Show file tree
Hide file tree
Showing 11 changed files with 119 additions and 78 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
7 changes: 4 additions & 3 deletions cmd/otelcol/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
2 changes: 1 addition & 1 deletion 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(settings service.Settings) error {
func run(settings service.ApplicationSettings) 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(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)
}
}

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

Expand Down
23 changes: 12 additions & 11 deletions service/application.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -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 {
Expand Down
20 changes: 11 additions & 9 deletions service/application_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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())

Expand Down Expand Up @@ -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"})
Expand All @@ -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)
Expand Down
14 changes: 7 additions & 7 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 {
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
Expand Down Expand Up @@ -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)

Expand All @@ -143,5 +143,5 @@ func newWithEventViewerLoggingHook(settings Settings, elog *eventlog.Log) (*Appl
}),
)

return New(settings)
return New(set)
}
37 changes: 6 additions & 31 deletions service/service.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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 {
Expand Down
12 changes: 6 additions & 6 deletions service/service_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
61 changes: 61 additions & 0 deletions service/settings.go
Original file line number Diff line number Diff line change
@@ -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
}
10 changes: 5 additions & 5 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 {
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)
Expand Down

0 comments on commit 22bcedc

Please sign in to comment.