Skip to content

Commit

Permalink
Standarize Settings, Params and Parameters in Extensions (#3294)
Browse files Browse the repository at this point in the history
* Replace ExtensionCreateParams with ExtensionCreateSettings.
Replace all dependencies in Extensions.

Signed-off-by: Patryk Matyjasek <pmatyjasek@sumologic.com>

* Update changelog

Signed-off-by: Patryk Matyjasek <pmatyjasek@sumologic.com>

# Conflicts:
#	CHANGELOG.md

* Add missing files

Signed-off-by: Patryk Matyjasek <pmatyjasek@sumologic.com>
  • Loading branch information
pmatyjasek-sumo authored Jun 2, 2021
1 parent 146973f commit 827f5e1
Show file tree
Hide file tree
Showing 22 changed files with 39 additions and 38 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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 💡

Expand Down
2 changes: 1 addition & 1 deletion component/componenttest/nop_extension.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
2 changes: 1 addition & 1 deletion component/componenttest/nop_extension_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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()))
Expand Down
6 changes: 3 additions & 3 deletions component/extension.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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)
}
2 changes: 1 addition & 1 deletion config/configcheck/configcheck_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
4 changes: 2 additions & 2 deletions extension/ballastextension/factory.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
4 changes: 2 additions & 2 deletions extension/ballastextension/factory_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
6 changes: 3 additions & 3 deletions extension/extensionhelper/factory.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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)
}
4 changes: 2 additions & 2 deletions extension/extensionhelper/factory_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
Expand All @@ -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
}

Expand Down
4 changes: 2 additions & 2 deletions extension/healthcheckextension/factory.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
4 changes: 2 additions & 2 deletions extension/healthcheckextension/factory_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
Expand All @@ -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)
}
4 changes: 2 additions & 2 deletions extension/oidcauthextension/factory.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
2 changes: 1 addition & 1 deletion extension/oidcauthextension/factory_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
Expand Down
4 changes: 2 additions & 2 deletions extension/pprofextension/factory.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
4 changes: 2 additions & 2 deletions extension/pprofextension/factory_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
Expand All @@ -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)
}
4 changes: 2 additions & 2 deletions extension/zpagesextension/factory.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
4 changes: 2 additions & 2 deletions extension/zpagesextension/factory_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
Expand All @@ -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)
}
2 changes: 1 addition & 1 deletion internal/testcomponents/example_extension.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
6 changes: 3 additions & 3 deletions service/defaultcomponents/default_extensions_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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(),
}
Expand All @@ -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))
Expand Down
4 changes: 2 additions & 2 deletions service/internal/builder/extensions_builder.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
Expand Down
2 changes: 1 addition & 1 deletion service/internal/builder/extensions_builder_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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")
},
)
Expand Down
2 changes: 1 addition & 1 deletion service/internal/builder/factories_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
},
)
Expand Down

0 comments on commit 827f5e1

Please sign in to comment.