From 827f5e17020f61d064cb1382a9b717f6662a3ab3 Mon Sep 17 00:00:00 2001 From: Patryk Matyjasek <72449525+pmatyjasek-sumo@users.noreply.github.com> Date: Wed, 2 Jun 2021 07:08:31 +0200 Subject: [PATCH] Standarize Settings, Params and Parameters in Extensions (#3294) * Replace ExtensionCreateParams with ExtensionCreateSettings. Replace all dependencies in Extensions. Signed-off-by: Patryk Matyjasek * Update changelog Signed-off-by: Patryk Matyjasek # Conflicts: # CHANGELOG.md * Add missing files Signed-off-by: Patryk Matyjasek --- CHANGELOG.md | 1 + component/componenttest/nop_extension.go | 2 +- component/componenttest/nop_extension_test.go | 2 +- component/extension.go | 6 +++--- config/configcheck/configcheck_test.go | 2 +- extension/ballastextension/factory.go | 4 ++-- extension/ballastextension/factory_test.go | 4 ++-- extension/extensionhelper/factory.go | 6 +++--- extension/extensionhelper/factory_test.go | 4 ++-- extension/healthcheckextension/factory.go | 4 ++-- extension/healthcheckextension/factory_test.go | 4 ++-- extension/oidcauthextension/factory.go | 4 ++-- extension/oidcauthextension/factory_test.go | 2 +- extension/pprofextension/factory.go | 4 ++-- extension/pprofextension/factory_test.go | 4 ++-- extension/zpagesextension/factory.go | 4 ++-- extension/zpagesextension/factory_test.go | 4 ++-- internal/testcomponents/example_extension.go | 2 +- service/defaultcomponents/default_extensions_test.go | 6 +++--- service/internal/builder/extensions_builder.go | 4 ++-- service/internal/builder/extensions_builder_test.go | 2 +- service/internal/builder/factories_test.go | 2 +- 22 files changed, 39 insertions(+), 38 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 8d889695833..4ff52375529 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -9,6 +9,7 @@ - Remove unused testutil.TempSocketName (#3291) - Move BigEndian helper functions in `tracetranslator` to an internal package.(#3298) - Rename `configtest.LoadConfigFile` to `configtest.LoadConfigAndValidate` (#3306) +- Replace `ExtensionCreateParams` with `ExtensionCreateSettings` (#3294) ## 💡 Enhancements 💡 diff --git a/component/componenttest/nop_extension.go b/component/componenttest/nop_extension.go index 62e0ca9c8fa..3c87ca5b688 100644 --- a/component/componenttest/nop_extension.go +++ b/component/componenttest/nop_extension.go @@ -51,7 +51,7 @@ func (f *nopExtensionFactory) CreateDefaultConfig() config.Extension { // CreateExtension implements component.ExtensionFactory interface. func (f *nopExtensionFactory) CreateExtension( _ context.Context, - _ component.ExtensionCreateParams, + _ component.ExtensionCreateSettings, _ config.Extension, ) (component.Extension, error) { return nopExtensionInstance, nil diff --git a/component/componenttest/nop_extension_test.go b/component/componenttest/nop_extension_test.go index 1b8d21bb4bc..cb20a2a4acb 100644 --- a/component/componenttest/nop_extension_test.go +++ b/component/componenttest/nop_extension_test.go @@ -32,7 +32,7 @@ func TestNewNopExtensionFactory(t *testing.T) { cfg := factory.CreateDefaultConfig() assert.Equal(t, &nopExtensionConfig{ExtensionSettings: config.NewExtensionSettings(config.NewID("nop"))}, cfg) - traces, err := factory.CreateExtension(context.Background(), component.ExtensionCreateParams{}, cfg) + traces, err := factory.CreateExtension(context.Background(), component.ExtensionCreateSettings{}, cfg) require.NoError(t, err) assert.NoError(t, traces.Start(context.Background(), NewNopHost())) assert.NoError(t, traces.Shutdown(context.Background())) diff --git a/component/extension.go b/component/extension.go index d5493829e7e..4615a2139ba 100644 --- a/component/extension.go +++ b/component/extension.go @@ -46,8 +46,8 @@ type PipelineWatcher interface { NotReady() error } -// ExtensionCreateParams is passed to ExtensionFactory.Create* functions. -type ExtensionCreateParams struct { +// ExtensionCreateSettings is passed to ExtensionFactory.Create* functions. +type ExtensionCreateSettings struct { // Logger that the factory can use during creation and can pass to the created // component to be used later as well. Logger *zap.Logger @@ -70,5 +70,5 @@ type ExtensionFactory interface { CreateDefaultConfig() config.Extension // CreateExtension creates a service extension based on the given config. - CreateExtension(ctx context.Context, params ExtensionCreateParams, cfg config.Extension) (Extension, error) + CreateExtension(ctx context.Context, set ExtensionCreateSettings, cfg config.Extension) (Extension, error) } diff --git a/config/configcheck/configcheck_test.go b/config/configcheck/configcheck_test.go index d2702231851..279f766ce3f 100644 --- a/config/configcheck/configcheck_test.go +++ b/config/configcheck/configcheck_test.go @@ -198,6 +198,6 @@ func (b badConfigExtensionFactory) CreateDefaultConfig() config.Extension { }{} } -func (b badConfigExtensionFactory) CreateExtension(_ context.Context, _ component.ExtensionCreateParams, _ config.Extension) (component.Extension, error) { +func (b badConfigExtensionFactory) CreateExtension(_ context.Context, _ component.ExtensionCreateSettings, _ config.Extension) (component.Extension, error) { return nil, nil } diff --git a/extension/ballastextension/factory.go b/extension/ballastextension/factory.go index 828744a19fb..d12ecadf776 100644 --- a/extension/ballastextension/factory.go +++ b/extension/ballastextension/factory.go @@ -41,6 +41,6 @@ func createDefaultConfig() config.Extension { } } -func createExtension(_ context.Context, params component.ExtensionCreateParams, cfg config.Extension) (component.Extension, error) { - return newMemoryBallast(cfg.(*Config), params.Logger), nil +func createExtension(_ context.Context, set component.ExtensionCreateSettings, cfg config.Extension) (component.Extension, error) { + return newMemoryBallast(cfg.(*Config), set.Logger), nil } diff --git a/extension/ballastextension/factory_test.go b/extension/ballastextension/factory_test.go index 7eb954ba005..76f7b36f704 100644 --- a/extension/ballastextension/factory_test.go +++ b/extension/ballastextension/factory_test.go @@ -32,14 +32,14 @@ func TestFactory_CreateDefaultConfig(t *testing.T) { assert.Equal(t, &Config{ExtensionSettings: config.NewExtensionSettings(config.NewID(typeStr))}, cfg) assert.NoError(t, configcheck.ValidateConfig(cfg)) - ext, err := createExtension(context.Background(), component.ExtensionCreateParams{Logger: zap.NewNop()}, cfg) + ext, err := createExtension(context.Background(), component.ExtensionCreateSettings{Logger: zap.NewNop()}, cfg) require.NoError(t, err) require.NotNil(t, ext) } func TestFactory_CreateExtension(t *testing.T) { cfg := createDefaultConfig().(*Config) - ext, err := createExtension(context.Background(), component.ExtensionCreateParams{Logger: zap.NewNop()}, cfg) + ext, err := createExtension(context.Background(), component.ExtensionCreateSettings{Logger: zap.NewNop()}, cfg) require.NoError(t, err) require.NotNil(t, ext) } diff --git a/extension/extensionhelper/factory.go b/extension/extensionhelper/factory.go index b85c79a9816..271cba740a4 100644 --- a/extension/extensionhelper/factory.go +++ b/extension/extensionhelper/factory.go @@ -28,7 +28,7 @@ type FactoryOption func(o *factory) type CreateDefaultConfig func() config.Extension // CreateServiceExtension is the equivalent of component.ExtensionFactory.CreateExtension() -type CreateServiceExtension func(context.Context, component.ExtensionCreateParams, config.Extension) (component.Extension, error) +type CreateServiceExtension func(context.Context, component.ExtensionCreateSettings, config.Extension) (component.Extension, error) type factory struct { cfgType config.Type @@ -66,7 +66,7 @@ func (f *factory) CreateDefaultConfig() config.Extension { // CreateExtension creates a component.TraceExtension based on this config. func (f *factory) CreateExtension( ctx context.Context, - params component.ExtensionCreateParams, + set component.ExtensionCreateSettings, cfg config.Extension) (component.Extension, error) { - return f.createServiceExtension(ctx, params, cfg) + return f.createServiceExtension(ctx, set, cfg) } diff --git a/extension/extensionhelper/factory_test.go b/extension/extensionhelper/factory_test.go index 493f1b9f30c..0710ba699d0 100644 --- a/extension/extensionhelper/factory_test.go +++ b/extension/extensionhelper/factory_test.go @@ -38,7 +38,7 @@ func TestNewFactory(t *testing.T) { createExtension) assert.EqualValues(t, typeStr, factory.Type()) assert.EqualValues(t, &defaultCfg, factory.CreateDefaultConfig()) - ext, err := factory.CreateExtension(context.Background(), component.ExtensionCreateParams{}, &defaultCfg) + ext, err := factory.CreateExtension(context.Background(), component.ExtensionCreateSettings{}, &defaultCfg) assert.NoError(t, err) assert.Same(t, nopExtensionInstance, ext) } @@ -47,7 +47,7 @@ func defaultConfig() config.Extension { return &defaultCfg } -func createExtension(context.Context, component.ExtensionCreateParams, config.Extension) (component.Extension, error) { +func createExtension(context.Context, component.ExtensionCreateSettings, config.Extension) (component.Extension, error) { return nopExtensionInstance, nil } diff --git a/extension/healthcheckextension/factory.go b/extension/healthcheckextension/factory.go index 69719cdb690..9a8976ddae6 100644 --- a/extension/healthcheckextension/factory.go +++ b/extension/healthcheckextension/factory.go @@ -49,8 +49,8 @@ func createDefaultConfig() config.Extension { } } -func createExtension(_ context.Context, params component.ExtensionCreateParams, cfg config.Extension) (component.Extension, error) { +func createExtension(_ context.Context, set component.ExtensionCreateSettings, cfg config.Extension) (component.Extension, error) { config := cfg.(*Config) - return newServer(*config, params.Logger), nil + return newServer(*config, set.Logger), nil } diff --git a/extension/healthcheckextension/factory_test.go b/extension/healthcheckextension/factory_test.go index 297a5dc27d3..c846c696d83 100644 --- a/extension/healthcheckextension/factory_test.go +++ b/extension/healthcheckextension/factory_test.go @@ -39,7 +39,7 @@ func TestFactory_CreateDefaultConfig(t *testing.T) { }, cfg) assert.NoError(t, configcheck.ValidateConfig(cfg)) - ext, err := createExtension(context.Background(), component.ExtensionCreateParams{Logger: zap.NewNop()}, cfg) + ext, err := createExtension(context.Background(), component.ExtensionCreateSettings{Logger: zap.NewNop()}, cfg) require.NoError(t, err) require.NotNil(t, ext) } @@ -48,7 +48,7 @@ func TestFactory_CreateExtension(t *testing.T) { cfg := createDefaultConfig().(*Config) cfg.TCPAddr.Endpoint = testutil.GetAvailableLocalAddress(t) - ext, err := createExtension(context.Background(), component.ExtensionCreateParams{Logger: zap.NewNop()}, cfg) + ext, err := createExtension(context.Background(), component.ExtensionCreateSettings{Logger: zap.NewNop()}, cfg) require.NoError(t, err) require.NotNil(t, ext) } diff --git a/extension/oidcauthextension/factory.go b/extension/oidcauthextension/factory.go index 62f594e7a50..d27fc6386b8 100644 --- a/extension/oidcauthextension/factory.go +++ b/extension/oidcauthextension/factory.go @@ -44,6 +44,6 @@ func createDefaultConfig() config.Extension { } } -func createExtension(_ context.Context, params component.ExtensionCreateParams, cfg config.Extension) (component.Extension, error) { - return newExtension(cfg.(*Config), params.Logger) +func createExtension(_ context.Context, set component.ExtensionCreateSettings, cfg config.Extension) (component.Extension, error) { + return newExtension(cfg.(*Config), set.Logger) } diff --git a/extension/oidcauthextension/factory_test.go b/extension/oidcauthextension/factory_test.go index 18fbdf0caee..5d7c341cbc1 100644 --- a/extension/oidcauthextension/factory_test.go +++ b/extension/oidcauthextension/factory_test.go @@ -46,7 +46,7 @@ func TestCreateExtension(t *testing.T) { cfg.Audience = "collector" cfg.IssuerURL = "https://auth.example.com" - ext, err := createExtension(context.Background(), component.ExtensionCreateParams{Logger: zap.NewNop()}, cfg) + ext, err := createExtension(context.Background(), component.ExtensionCreateSettings{Logger: zap.NewNop()}, cfg) assert.NoError(t, err) assert.NotNil(t, ext) } diff --git a/extension/pprofextension/factory.go b/extension/pprofextension/factory.go index 3bcba3996bc..ac9f161f8db 100644 --- a/extension/pprofextension/factory.go +++ b/extension/pprofextension/factory.go @@ -48,11 +48,11 @@ func createDefaultConfig() config.Extension { } } -func createExtension(_ context.Context, params component.ExtensionCreateParams, cfg config.Extension) (component.Extension, error) { +func createExtension(_ context.Context, set component.ExtensionCreateSettings, cfg config.Extension) (component.Extension, error) { config := cfg.(*Config) if config.TCPAddr.Endpoint == "" { return nil, errors.New("\"endpoint\" is required when using the \"pprof\" extension") } - return newServer(*config, params.Logger), nil + return newServer(*config, set.Logger), nil } diff --git a/extension/pprofextension/factory_test.go b/extension/pprofextension/factory_test.go index 56d5842d0d0..b6bcc491d13 100644 --- a/extension/pprofextension/factory_test.go +++ b/extension/pprofextension/factory_test.go @@ -38,7 +38,7 @@ func TestFactory_CreateDefaultConfig(t *testing.T) { cfg) assert.NoError(t, configcheck.ValidateConfig(cfg)) - ext, err := createExtension(context.Background(), component.ExtensionCreateParams{Logger: zap.NewNop()}, cfg) + ext, err := createExtension(context.Background(), component.ExtensionCreateSettings{Logger: zap.NewNop()}, cfg) require.NoError(t, err) require.NotNil(t, ext) } @@ -47,7 +47,7 @@ func TestFactory_CreateExtension(t *testing.T) { cfg := createDefaultConfig().(*Config) cfg.TCPAddr.Endpoint = testutil.GetAvailableLocalAddress(t) - ext, err := createExtension(context.Background(), component.ExtensionCreateParams{Logger: zap.NewNop()}, cfg) + ext, err := createExtension(context.Background(), component.ExtensionCreateSettings{Logger: zap.NewNop()}, cfg) require.NoError(t, err) require.NotNil(t, ext) } diff --git a/extension/zpagesextension/factory.go b/extension/zpagesextension/factory.go index 710614a415a..88898862c51 100644 --- a/extension/zpagesextension/factory.go +++ b/extension/zpagesextension/factory.go @@ -49,11 +49,11 @@ func createDefaultConfig() config.Extension { } // createExtension creates the extension based on this config. -func createExtension(_ context.Context, params component.ExtensionCreateParams, cfg config.Extension) (component.Extension, error) { +func createExtension(_ context.Context, set component.ExtensionCreateSettings, cfg config.Extension) (component.Extension, error) { config := cfg.(*Config) if config.TCPAddr.Endpoint == "" { return nil, errors.New("\"endpoint\" is required when using the \"zpages\" extension") } - return newServer(*config, params.Logger), nil + return newServer(*config, set.Logger), nil } diff --git a/extension/zpagesextension/factory_test.go b/extension/zpagesextension/factory_test.go index f3705c2c5a3..8db538b1f57 100644 --- a/extension/zpagesextension/factory_test.go +++ b/extension/zpagesextension/factory_test.go @@ -40,7 +40,7 @@ func TestFactory_CreateDefaultConfig(t *testing.T) { cfg) assert.NoError(t, configcheck.ValidateConfig(cfg)) - ext, err := createExtension(context.Background(), component.ExtensionCreateParams{Logger: zap.NewNop()}, cfg) + ext, err := createExtension(context.Background(), component.ExtensionCreateSettings{Logger: zap.NewNop()}, cfg) require.NoError(t, err) require.NotNil(t, ext) } @@ -49,7 +49,7 @@ func TestFactory_CreateExtension(t *testing.T) { cfg := createDefaultConfig().(*Config) cfg.TCPAddr.Endpoint = testutil.GetAvailableLocalAddress(t) - ext, err := createExtension(context.Background(), component.ExtensionCreateParams{Logger: zap.NewNop()}, cfg) + ext, err := createExtension(context.Background(), component.ExtensionCreateSettings{Logger: zap.NewNop()}, cfg) require.NoError(t, err) require.NotNil(t, ext) } diff --git a/internal/testcomponents/example_extension.go b/internal/testcomponents/example_extension.go index 9d674cc6412..f233dbc178e 100644 --- a/internal/testcomponents/example_extension.go +++ b/internal/testcomponents/example_extension.go @@ -48,6 +48,6 @@ func createExtensionDefaultConfig() config.Extension { } // CreateExtension creates an Extension based on this config. -func createExtension(context.Context, component.ExtensionCreateParams, config.Extension) (component.Extension, error) { +func createExtension(context.Context, component.ExtensionCreateSettings, config.Extension) (component.Extension, error) { return componenthelper.New(), nil } diff --git a/service/defaultcomponents/default_extensions_test.go b/service/defaultcomponents/default_extensions_test.go index a8b5d64a265..0a7a07976f0 100644 --- a/service/defaultcomponents/default_extensions_test.go +++ b/service/defaultcomponents/default_extensions_test.go @@ -95,7 +95,7 @@ type getExtensionConfigFn func() config.Extension func verifyExtensionLifecycle(t *testing.T, factory component.ExtensionFactory, getConfigFn getExtensionConfigFn) { ctx := context.Background() host := newAssertNoErrorHost(t) - extCreateParams := component.ExtensionCreateParams{ + extCreateSet := component.ExtensionCreateSettings{ Logger: zap.NewNop(), BuildInfo: component.DefaultBuildInfo(), } @@ -104,12 +104,12 @@ func verifyExtensionLifecycle(t *testing.T, factory component.ExtensionFactory, getConfigFn = factory.CreateDefaultConfig } - firstExt, err := factory.CreateExtension(ctx, extCreateParams, getConfigFn()) + firstExt, err := factory.CreateExtension(ctx, extCreateSet, getConfigFn()) require.NoError(t, err) require.NoError(t, firstExt.Start(ctx, host)) require.NoError(t, firstExt.Shutdown(ctx)) - secondExt, err := factory.CreateExtension(ctx, extCreateParams, getConfigFn()) + secondExt, err := factory.CreateExtension(ctx, extCreateSet, getConfigFn()) require.NoError(t, err) require.NoError(t, secondExt.Start(ctx, host)) require.NoError(t, secondExt.Shutdown(ctx)) diff --git a/service/internal/builder/extensions_builder.go b/service/internal/builder/extensions_builder.go index 54ee4ecc3f8..8719d93ee65 100644 --- a/service/internal/builder/extensions_builder.go +++ b/service/internal/builder/extensions_builder.go @@ -156,12 +156,12 @@ func (eb *extensionsBuilder) buildExtension(logger *zap.Logger, buildInfo compon logger: logger, } - creationParams := component.ExtensionCreateParams{ + creationSet := component.ExtensionCreateSettings{ Logger: logger, BuildInfo: buildInfo, } - ex, err := factory.CreateExtension(context.Background(), creationParams, cfg) + ex, err := factory.CreateExtension(context.Background(), creationSet, cfg) if err != nil { return nil, fmt.Errorf("failed to create extension %v: %w", cfg.ID(), err) } diff --git a/service/internal/builder/extensions_builder_test.go b/service/internal/builder/extensions_builder_test.go index b4985620133..b0281e700cb 100644 --- a/service/internal/builder/extensions_builder_test.go +++ b/service/internal/builder/extensions_builder_test.go @@ -34,7 +34,7 @@ func TestService_setupExtensions(t *testing.T) { cfg := config.NewExtensionSettings(config.NewID("err")) return &cfg }, - func(ctx context.Context, params component.ExtensionCreateParams, extension config.Extension) (component.Extension, error) { + func(ctx context.Context, set component.ExtensionCreateSettings, extension config.Extension) (component.Extension, error) { return nil, fmt.Errorf("cannot create \"err\" extension type") }, ) diff --git a/service/internal/builder/factories_test.go b/service/internal/builder/factories_test.go index add76844a5d..360363e5525 100644 --- a/service/internal/builder/factories_test.go +++ b/service/internal/builder/factories_test.go @@ -96,7 +96,7 @@ func newBadExtensionFactory() component.ExtensionFactory { ExtensionSettings: config.NewExtensionSettings(config.NewID("bf")), } }, - func(ctx context.Context, params component.ExtensionCreateParams, extension config.Extension) (component.Extension, error) { + func(ctx context.Context, set component.ExtensionCreateSettings, extension config.Extension) (component.Extension, error) { return nil, nil }, )