From dd92465ef35809d9459c81ac11795f1d370227b6 Mon Sep 17 00:00:00 2001 From: Bogdan Drutu Date: Tue, 23 Mar 2021 16:29:04 -0700 Subject: [PATCH] Use Option pattern for componenthelper Signed-off-by: Bogdan Drutu --- CHANGELOG.md | 9 ++-- component/componenthelper/component.go | 48 +++++++++++++++----- component/componenthelper/component_test.go | 24 ++++------ component/componenttest/nop_exporter.go | 2 +- component/componenttest/nop_extension.go | 2 +- component/componenttest/nop_processor.go | 2 +- component/componenttest/nop_receiver.go | 2 +- exporter/exporterhelper/common.go | 21 ++++----- internal/testcomponents/example_extension.go | 2 +- processor/processorhelper/processor.go | 13 +++--- receiver/scraperhelper/scraper.go | 22 +++++---- 11 files changed, 86 insertions(+), 61 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index a21cf98658c..db9c5797dcc 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,9 +4,12 @@ ## 🛑 Breaking changes 🛑 -- Refactored `consumererror` package (#2768) - - Eliminated `PartialError` type in favor of signal-specific types - - Renamed `CombineErrors` to `Combine` +- Refactor `consumererror` package (#2768) + - Remove `PartialError` type in favor of signal-specific types + - Rename `CombineErrors()` to `Combine()` +- Refactor `componenthelper` package (#2778) + - Remove `ComponentSettings` and `DefaultComponentSettings()` + - Rename `NewComponent()` to `New()` ## v0.23.0 Beta diff --git a/component/componenthelper/component.go b/component/componenthelper/component.go index 0ecc6e5a610..e402952615d 100644 --- a/component/componenthelper/component.go +++ b/component/componenthelper/component.go @@ -26,17 +26,28 @@ type Start func(context.Context, component.Host) error // Shutdown specifies the function invoked when the exporter is being shutdown. type Shutdown func(context.Context) error -// ComponentSettings represents a settings struct to create components. -type ComponentSettings struct { +// baseSettings represents a settings struct to create components. +type baseSettings struct { Start Shutdown } -// DefaultComponentSettings returns the default settings for a component. The Start and Shutdown are no-op. -func DefaultComponentSettings() *ComponentSettings { - return &ComponentSettings{ - Start: func(ctx context.Context, host component.Host) error { return nil }, - Shutdown: func(ctx context.Context) error { return nil }, +// Option represents the possible options for New. +type Option func(*baseSettings) + +// WithStart overrides the default Start function for a processor. +// The default shutdown function does nothing and always returns nil. +func WithStart(start Start) Option { + return func(o *baseSettings) { + o.Start = start + } +} + +// WithShutdown overrides the default Shutdown function for a processor. +// The default shutdown function does nothing and always returns nil. +func WithShutdown(shutdown Shutdown) Option { + return func(o *baseSettings) { + o.Shutdown = shutdown } } @@ -55,10 +66,25 @@ func (be *baseComponent) Shutdown(ctx context.Context) error { return be.shutdown(ctx) } -// NewComponent returns a component.Component that calls the given Start and Shutdown. -func NewComponent(s *ComponentSettings) component.Component { +// fromOptions returns the internal settings starting from the default and applying all options. +func fromOptions(options []Option) *baseSettings { + opts := &baseSettings{ + Start: func(ctx context.Context, host component.Host) error { return nil }, + Shutdown: func(ctx context.Context) error { return nil }, + } + + for _, op := range options { + op(opts) + } + + return opts +} + +// New returns a component.Component configured with the provided Options. +func New(options ...Option) component.Component { + bs := fromOptions(options) return &baseComponent{ - start: s.Start, - shutdown: s.Shutdown, + start: bs.Start, + shutdown: bs.Shutdown, } } diff --git a/component/componenthelper/component_test.go b/component/componenthelper/component_test.go index 3273fc9a4b8..c1bd3d1464a 100644 --- a/component/componenthelper/component_test.go +++ b/component/componenthelper/component_test.go @@ -28,43 +28,37 @@ import ( ) func TestDefaultSettings(t *testing.T) { - st := componenthelper.DefaultComponentSettings() - require.NotNil(t, st) - cp := componenthelper.NewComponent(st) + cp := componenthelper.New() require.NoError(t, cp.Start(context.Background(), componenttest.NewNopHost())) require.NoError(t, cp.Shutdown(context.Background())) } func TestWithStart(t *testing.T) { startCalled := false - st := componenthelper.DefaultComponentSettings() - st.Start = func(context.Context, component.Host) error { startCalled = true; return nil } - cp := componenthelper.NewComponent(st) + start := func(context.Context, component.Host) error { startCalled = true; return nil } + cp := componenthelper.New(componenthelper.WithStart(start)) assert.NoError(t, cp.Start(context.Background(), componenttest.NewNopHost())) assert.True(t, startCalled) } func TestWithStart_ReturnError(t *testing.T) { want := errors.New("my_error") - st := componenthelper.DefaultComponentSettings() - st.Start = func(context.Context, component.Host) error { return want } - cp := componenthelper.NewComponent(st) + start := func(context.Context, component.Host) error { return want } + cp := componenthelper.New(componenthelper.WithStart(start)) assert.Equal(t, want, cp.Start(context.Background(), componenttest.NewNopHost())) } func TestWithShutdown(t *testing.T) { shutdownCalled := false - st := componenthelper.DefaultComponentSettings() - st.Shutdown = func(context.Context) error { shutdownCalled = true; return nil } - cp := componenthelper.NewComponent(st) + shutdown := func(context.Context) error { shutdownCalled = true; return nil } + cp := componenthelper.New(componenthelper.WithShutdown(shutdown)) assert.NoError(t, cp.Shutdown(context.Background())) assert.True(t, shutdownCalled) } func TestWithShutdown_ReturnError(t *testing.T) { want := errors.New("my_error") - st := componenthelper.DefaultComponentSettings() - st.Shutdown = func(context.Context) error { return want } - cp := componenthelper.NewComponent(st) + shutdown := func(context.Context) error { return want } + cp := componenthelper.New(componenthelper.WithShutdown(shutdown)) assert.Equal(t, want, cp.Shutdown(context.Background())) } diff --git a/component/componenttest/nop_exporter.go b/component/componenttest/nop_exporter.go index b08a25e69af..5233e644811 100644 --- a/component/componenttest/nop_exporter.go +++ b/component/componenttest/nop_exporter.go @@ -74,7 +74,7 @@ func (f *nopExporterFactory) CreateLogsExporter( } var nopExporterInstance = &nopExporter{ - Component: componenthelper.NewComponent(componenthelper.DefaultComponentSettings()), + Component: componenthelper.New(), Traces: consumertest.NewTracesNop(), Metrics: consumertest.NewMetricsNop(), Logs: consumertest.NewLogsNop(), diff --git a/component/componenttest/nop_extension.go b/component/componenttest/nop_extension.go index 810d570d822..9f5622044d1 100644 --- a/component/componenttest/nop_extension.go +++ b/component/componenttest/nop_extension.go @@ -54,7 +54,7 @@ func (f *nopExtensionFactory) CreateExtension( } var nopExtensionInstance = &nopExtension{ - Component: componenthelper.NewComponent(componenthelper.DefaultComponentSettings()), + Component: componenthelper.New(), } // nopExtension stores consumed traces and metrics for testing purposes. diff --git a/component/componenttest/nop_processor.go b/component/componenttest/nop_processor.go index e8ff907c168..682d790b6d6 100644 --- a/component/componenttest/nop_processor.go +++ b/component/componenttest/nop_processor.go @@ -77,7 +77,7 @@ func (f *nopProcessorFactory) CreateLogsProcessor( } var nopProcessorInstance = &nopProcessor{ - Component: componenthelper.NewComponent(componenthelper.DefaultComponentSettings()), + Component: componenthelper.New(), Traces: consumertest.NewTracesNop(), Metrics: consumertest.NewMetricsNop(), Logs: consumertest.NewLogsNop(), diff --git a/component/componenttest/nop_receiver.go b/component/componenttest/nop_receiver.go index 869afa88b83..d3ee52ef02b 100644 --- a/component/componenttest/nop_receiver.go +++ b/component/componenttest/nop_receiver.go @@ -76,7 +76,7 @@ func (f *nopReceiverFactory) CreateLogsReceiver( } var nopReceiverInstance = &nopReceiver{ - Component: componenthelper.NewComponent(componenthelper.DefaultComponentSettings()), + Component: componenthelper.New(), } // nopReceiver stores consumed traces and metrics for testing purposes. diff --git a/exporter/exporterhelper/common.go b/exporter/exporterhelper/common.go index 88111c1c6ed..ee389e15719 100644 --- a/exporter/exporterhelper/common.go +++ b/exporter/exporterhelper/common.go @@ -25,7 +25,7 @@ import ( "go.opentelemetry.io/collector/config/configmodels" ) -// ComponentSettings for timeout. The timeout applies to individual attempts to send data to the backend. +// TimeoutSettings for timeout. The timeout applies to individual attempts to send data to the backend. type TimeoutSettings struct { // Timeout is the timeout for every attempt to send data to the backend. Timeout time.Duration `mapstructure:"timeout"` @@ -72,7 +72,7 @@ func (req *baseRequest) setContext(ctx context.Context) { // baseSettings represents all the options that users can configure. type baseSettings struct { - *componenthelper.ComponentSettings + componentOptions []componenthelper.Option TimeoutSettings QueueSettings RetrySettings @@ -83,8 +83,7 @@ type baseSettings struct { func fromOptions(options []Option) *baseSettings { // Start from the default options: opts := &baseSettings{ - ComponentSettings: componenthelper.DefaultComponentSettings(), - TimeoutSettings: DefaultTimeoutSettings(), + TimeoutSettings: DefaultTimeoutSettings(), // TODO: Enable queuing by default (call DefaultQueueSettings) QueueSettings: QueueSettings{Enabled: false}, // TODO: Enable retry by default (call DefaultRetrySettings) @@ -102,19 +101,19 @@ func fromOptions(options []Option) *baseSettings { // Option apply changes to baseSettings. type Option func(*baseSettings) -// WithShutdown overrides the default Shutdown function for an exporter. +// WithStart overrides the default Start function for an exporter. // The default shutdown function does nothing and always returns nil. -func WithShutdown(shutdown componenthelper.Shutdown) Option { +func WithStart(start componenthelper.Start) Option { return func(o *baseSettings) { - o.Shutdown = shutdown + o.componentOptions = append(o.componentOptions, componenthelper.WithStart(start)) } } -// WithStart overrides the default Start function for an exporter. +// WithShutdown overrides the default Shutdown function for an exporter. // The default shutdown function does nothing and always returns nil. -func WithStart(start componenthelper.Start) Option { +func WithShutdown(shutdown componenthelper.Shutdown) Option { return func(o *baseSettings) { - o.Start = start + o.componentOptions = append(o.componentOptions, componenthelper.WithShutdown(shutdown)) } } @@ -162,7 +161,7 @@ type baseExporter struct { func newBaseExporter(cfg configmodels.Exporter, logger *zap.Logger, options ...Option) *baseExporter { bs := fromOptions(options) be := &baseExporter{ - Component: componenthelper.NewComponent(bs.ComponentSettings), + Component: componenthelper.New(bs.componentOptions...), cfg: cfg, convertResourceToTelemetry: bs.ResourceToTelemetrySettings.Enabled, } diff --git a/internal/testcomponents/example_extension.go b/internal/testcomponents/example_extension.go index 0027e5cb470..e038adcca76 100644 --- a/internal/testcomponents/example_extension.go +++ b/internal/testcomponents/example_extension.go @@ -52,5 +52,5 @@ func createExtensionDefaultConfig() configmodels.Extension { // CreateExtension creates an Extension based on this config. func createExtension(context.Context, component.ExtensionCreateParams, configmodels.Extension) (component.Extension, error) { - return componenthelper.NewComponent(componenthelper.DefaultComponentSettings()), nil + return componenthelper.New(), nil } diff --git a/processor/processorhelper/processor.go b/processor/processorhelper/processor.go index e68339f39c1..54d40a968ff 100644 --- a/processor/processorhelper/processor.go +++ b/processor/processorhelper/processor.go @@ -62,7 +62,7 @@ type Option func(*baseSettings) // The default shutdown function does nothing and always returns nil. func WithStart(start componenthelper.Start) Option { return func(o *baseSettings) { - o.Start = start + o.componentOptions = append(o.componentOptions, componenthelper.WithStart(start)) } } @@ -70,7 +70,7 @@ func WithStart(start componenthelper.Start) Option { // The default shutdown function does nothing and always returns nil. func WithShutdown(shutdown componenthelper.Shutdown) Option { return func(o *baseSettings) { - o.Shutdown = shutdown + o.componentOptions = append(o.componentOptions, componenthelper.WithShutdown(shutdown)) } } @@ -83,16 +83,15 @@ func WithCapabilities(capabilities component.ProcessorCapabilities) Option { } type baseSettings struct { - *componenthelper.ComponentSettings - capabilities component.ProcessorCapabilities + componentOptions []componenthelper.Option + capabilities component.ProcessorCapabilities } // fromOptions returns the internal settings starting from the default and applying all options. func fromOptions(options []Option) *baseSettings { // Start from the default options: opts := &baseSettings{ - ComponentSettings: componenthelper.DefaultComponentSettings(), - capabilities: component.ProcessorCapabilities{MutatesConsumedData: true}, + capabilities: component.ProcessorCapabilities{MutatesConsumedData: true}, } for _, op := range options { @@ -114,7 +113,7 @@ type baseProcessor struct { func newBaseProcessor(fullName string, options ...Option) baseProcessor { bs := fromOptions(options) be := baseProcessor{ - Component: componenthelper.NewComponent(bs.ComponentSettings), + Component: componenthelper.New(bs.componentOptions...), fullName: fullName, capabilities: bs.capabilities, traceAttributes: []trace.Attribute{ diff --git a/receiver/scraperhelper/scraper.go b/receiver/scraperhelper/scraper.go index 06e0aa2b52d..9472b1b7128 100644 --- a/receiver/scraperhelper/scraper.go +++ b/receiver/scraperhelper/scraper.go @@ -29,8 +29,12 @@ type ScrapeMetrics func(context.Context) (pdata.MetricSlice, error) // Scrape resource metrics. type ScrapeResourceMetrics func(context.Context) (pdata.ResourceMetricsSlice, error) +type baseSettings struct { + componentOptions []componenthelper.Option +} + // ScraperOption apply changes to internal options. -type ScraperOption func(*componenthelper.ComponentSettings) +type ScraperOption func(*baseSettings) type BaseScraper interface { component.Component @@ -64,15 +68,15 @@ func (b baseScraper) Name() string { // WithStart sets the function that will be called on startup. func WithStart(start componenthelper.Start) ScraperOption { - return func(s *componenthelper.ComponentSettings) { - s.Start = start + return func(o *baseSettings) { + o.componentOptions = append(o.componentOptions, componenthelper.WithStart(start)) } } // WithShutdown sets the function that will be called on shutdown. func WithShutdown(shutdown componenthelper.Shutdown) ScraperOption { - return func(s *componenthelper.ComponentSettings) { - s.Shutdown = shutdown + return func(o *baseSettings) { + o.componentOptions = append(o.componentOptions, componenthelper.WithShutdown(shutdown)) } } @@ -91,14 +95,14 @@ func NewMetricsScraper( scrape ScrapeMetrics, options ...ScraperOption, ) MetricsScraper { - set := componenthelper.DefaultComponentSettings() + set := &baseSettings{} for _, op := range options { op(set) } ms := &metricsScraper{ baseScraper: baseScraper{ - Component: componenthelper.NewComponent(set), + Component: componenthelper.New(set.componentOptions...), name: name, }, ScrapeMetrics: scrape, @@ -130,14 +134,14 @@ func NewResourceMetricsScraper( scrape ScrapeResourceMetrics, options ...ScraperOption, ) ResourceMetricsScraper { - set := componenthelper.DefaultComponentSettings() + set := &baseSettings{} for _, op := range options { op(set) } rms := &resourceMetricsScraper{ baseScraper: baseScraper{ - Component: componenthelper.NewComponent(set), + Component: componenthelper.New(set.componentOptions...), name: name, }, ScrapeResourceMetrics: scrape,