From 4da31109271773eeb636c7b5b772856defc7f739 Mon Sep 17 00:00:00 2001 From: dmathieu <42@dmathieu.com> Date: Fri, 2 Aug 2024 10:27:42 +0200 Subject: [PATCH] bring back extension.Builder and deprecate it --- .chloggen/private-extension-builder.yaml | 4 +- extension/extension.go | 42 +++++++++++++++ extension/extension_test.go | 59 +++++++++++++++++++++ service/extensions/extensions.go | 2 +- service/extensions/extensions_test.go | 8 +-- service/host.go | 2 +- service/internal/builders/extension.go | 11 +++- service/internal/builders/extension_test.go | 6 +-- service/service.go | 11 +++- 9 files changed, 131 insertions(+), 14 deletions(-) diff --git a/.chloggen/private-extension-builder.yaml b/.chloggen/private-extension-builder.yaml index 5d65fc110a9..ae4a99e3524 100644 --- a/.chloggen/private-extension-builder.yaml +++ b/.chloggen/private-extension-builder.yaml @@ -1,13 +1,13 @@ # Use this changelog template to create an entry for release notes. # One of 'breaking', 'deprecation', 'new_component', 'enhancement', 'bug_fix' -change_type: breaking +change_type: deprecation # The name of the component, or a single word describing the area of concern, (e.g. otlpreceiver) component: extension # A brief description of the change. Surround your text with quotes ("") if it needs to start with a backtick (`). -note: Move extension.Builder into internal within the service module +note: Deprecate extension.Builder, and move it into an internal package of the service module # One or more tracking issues or pull requests related to the change issues: [10785] diff --git a/extension/extension.go b/extension/extension.go index 4148628fbfd..de54364827a 100644 --- a/extension/extension.go +++ b/extension/extension.go @@ -136,3 +136,45 @@ func MakeFactoryMap(factories ...Factory) (map[component.Type]Factory, error) { } return fMap, nil } + +// Builder extension is a helper struct that given a set of Configs and Factories helps with creating extensions. +// +// Deprecated: this builder is being internalized within the service module, +// and will be removed soon. +type Builder struct { + cfgs map[component.ID]component.Config + factories map[component.Type]Factory +} + +// NewBuilder creates a new extension.Builder to help with creating components form a set of configs and factories. +// +// Deprecated: this builder is being internalized within the service module, +// and will be removed soon. +func NewBuilder(cfgs map[component.ID]component.Config, factories map[component.Type]Factory) *Builder { + return &Builder{cfgs: cfgs, factories: factories} +} + +// Create creates an extension based on the settings and configs available. +func (b *Builder) Create(ctx context.Context, set Settings) (Extension, error) { + cfg, existsCfg := b.cfgs[set.ID] + if !existsCfg { + return nil, fmt.Errorf("extension %q is not configured", set.ID) + } + + f, existsFactory := b.factories[set.ID.Type()] + if !existsFactory { + return nil, fmt.Errorf("extension factory not available for: %q", set.ID) + } + + sl := f.ExtensionStability() + if sl >= component.StabilityLevelAlpha { + set.Logger.Debug(sl.LogMessage()) + } else { + set.Logger.Info(sl.LogMessage()) + } + return f.CreateExtension(ctx, set, cfg) +} + +func (b *Builder) Factory(componentType component.Type) component.Factory { + return b.factories[componentType] +} diff --git a/extension/extension_test.go b/extension/extension_test.go index 3eba5e38df0..3c4b2968667 100644 --- a/extension/extension_test.go +++ b/extension/extension_test.go @@ -8,8 +8,10 @@ import ( "testing" "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" "go.opentelemetry.io/collector/component" + "go.opentelemetry.io/collector/component/componenttest" ) type nopExtension struct { @@ -75,3 +77,60 @@ func TestMakeFactoryMap(t *testing.T) { }) } } + +func TestBuilder(t *testing.T) { + var testType = component.MustNewType("test") + defaultCfg := struct{}{} + testID := component.NewID(testType) + unknownID := component.MustNewID("unknown") + + factories, err := MakeFactoryMap([]Factory{ + NewFactory( + testType, + func() component.Config { return &defaultCfg }, + func(_ context.Context, settings Settings, _ component.Config) (Extension, error) { + return nopExtension{Settings: settings}, nil + }, + component.StabilityLevelDevelopment), + }...) + require.NoError(t, err) + + cfgs := map[component.ID]component.Config{testID: defaultCfg, unknownID: defaultCfg} + b := NewBuilder(cfgs, factories) + + e, err := b.Create(context.Background(), createSettings(testID)) + assert.NoError(t, err) + assert.NotNil(t, e) + + // Check that the extension has access to the resource attributes. + nop, ok := e.(nopExtension) + assert.True(t, ok) + assert.Equal(t, nop.Settings.Resource.Attributes().Len(), 0) + + missingType, err := b.Create(context.Background(), createSettings(unknownID)) + assert.EqualError(t, err, "extension factory not available for: \"unknown\"") + assert.Nil(t, missingType) + + missingCfg, err := b.Create(context.Background(), createSettings(component.NewIDWithName(testType, "foo"))) + assert.EqualError(t, err, "extension \"test/foo\" is not configured") + assert.Nil(t, missingCfg) +} + +func TestBuilderFactory(t *testing.T) { + factories, err := MakeFactoryMap([]Factory{NewFactory(component.MustNewType("foo"), nil, nil, component.StabilityLevelDevelopment)}...) + require.NoError(t, err) + + cfgs := map[component.ID]component.Config{component.MustNewID("foo"): struct{}{}} + b := NewBuilder(cfgs, factories) + + assert.NotNil(t, b.Factory(component.MustNewID("foo").Type())) + assert.Nil(t, b.Factory(component.MustNewID("bar").Type())) +} + +func createSettings(id component.ID) Settings { + return Settings{ + ID: id, + TelemetrySettings: componenttest.NewNopTelemetrySettings(), + BuildInfo: component.NewDefaultBuildInfo(), + } +} diff --git a/service/extensions/extensions.go b/service/extensions/extensions.go index a8182eee9b9..f7eb499dfad 100644 --- a/service/extensions/extensions.go +++ b/service/extensions/extensions.go @@ -172,7 +172,7 @@ type Settings struct { BuildInfo component.BuildInfo // Extensions builder for extensions. - Extensions *builders.ExtensionBuilder + Extensions builders.Extension } type Option func(*Extensions) diff --git a/service/extensions/extensions_test.go b/service/extensions/extensions_test.go index d577def8633..e7389cf987d 100644 --- a/service/extensions/extensions_test.go +++ b/service/extensions/extensions_test.go @@ -85,7 +85,7 @@ func TestBuildExtensions(t *testing.T) { _, err := New(context.Background(), Settings{ Telemetry: componenttest.NewNopTelemetrySettings(), BuildInfo: component.NewDefaultBuildInfo(), - Extensions: builders.NewExtensionBuilder(tt.extensionsConfigs, tt.factories), + Extensions: builders.NewExtension(tt.extensionsConfigs, tt.factories), }, tt.config) require.Error(t, err) assert.EqualError(t, err, tt.wantErrMsg) @@ -177,7 +177,7 @@ func (tc testOrderCase) testOrdering(t *testing.T) { exts, err := New(context.Background(), Settings{ Telemetry: componenttest.NewNopTelemetrySettings(), BuildInfo: component.NewDefaultBuildInfo(), - Extensions: builders.NewExtensionBuilder( + Extensions: builders.NewExtension( extCfgs, map[component.Type]extension.Factory{ recordingExtensionFactory.Type(): recordingExtensionFactory, @@ -286,7 +286,7 @@ func TestNotifyConfig(t *testing.T) { extensions, err := New(context.Background(), Settings{ Telemetry: componenttest.NewNopTelemetrySettings(), BuildInfo: component.NewDefaultBuildInfo(), - Extensions: builders.NewExtensionBuilder(tt.extensionsConfigs, tt.factories), + Extensions: builders.NewExtension(tt.extensionsConfigs, tt.factories), }, tt.serviceExtensions) assert.NoError(t, err) errs := extensions.NotifyConfig(context.Background(), confmap.NewFromStringMap(map[string]interface{}{})) @@ -433,7 +433,7 @@ func TestStatusReportedOnStartupShutdown(t *testing.T) { Settings{ Telemetry: componenttest.NewNopTelemetrySettings(), BuildInfo: component.NewDefaultBuildInfo(), - Extensions: builders.NewExtensionBuilder(extensionsConfigs, factories), + Extensions: builders.NewExtension(extensionsConfigs, factories), }, []component.ID{compID}, WithReporter(rep), diff --git a/service/host.go b/service/host.go index 94e939e86aa..4c2232fa7d3 100644 --- a/service/host.go +++ b/service/host.go @@ -28,7 +28,7 @@ type serviceHost struct { processors *processor.Builder exporters *exporter.Builder connectors *connector.Builder - extensions *builders.ExtensionBuilder + extensions builders.Extension buildInfo component.BuildInfo diff --git a/service/internal/builders/extension.go b/service/internal/builders/extension.go index bcc51d6e10d..a66a7f2857d 100644 --- a/service/internal/builders/extension.go +++ b/service/internal/builders/extension.go @@ -14,15 +14,22 @@ import ( var nopType = component.MustNewType("nop") +// Extension is an interface that allows using implementations of the builder +// from different packages. +type Extension interface { + Create(context.Context, extension.Settings) (extension.Extension, error) + Factory(component.Type) component.Factory +} + // ExtensionBuilder is a helper struct that given a set of Configs and Factories helps with creating extensions. type ExtensionBuilder struct { cfgs map[component.ID]component.Config factories map[component.Type]extension.Factory } -// NewExtensionBuilder creates a new ExtensionBuilder to help with creating +// NewExtension creates a new ExtensionBuilder to help with creating // components form a set of configs and factories. -func NewExtensionBuilder(cfgs map[component.ID]component.Config, factories map[component.Type]extension.Factory) *ExtensionBuilder { +func NewExtension(cfgs map[component.ID]component.Config, factories map[component.Type]extension.Factory) *ExtensionBuilder { return &ExtensionBuilder{cfgs: cfgs, factories: factories} } diff --git a/service/internal/builders/extension_test.go b/service/internal/builders/extension_test.go index e6e0bff665c..4baa3a8d193 100644 --- a/service/internal/builders/extension_test.go +++ b/service/internal/builders/extension_test.go @@ -34,7 +34,7 @@ func TestExtensionBuilder(t *testing.T) { require.NoError(t, err) cfgs := map[component.ID]component.Config{testID: defaultCfg, unknownID: defaultCfg} - b := NewExtensionBuilder(cfgs, factories) + b := NewExtension(cfgs, factories) e, err := b.Create(context.Background(), createExtensionSettings(testID)) assert.NoError(t, err) @@ -59,7 +59,7 @@ func TestExtensionBuilderFactory(t *testing.T) { require.NoError(t, err) cfgs := map[component.ID]component.Config{component.MustNewID("foo"): struct{}{}} - b := NewExtensionBuilder(cfgs, factories) + b := NewExtension(cfgs, factories) assert.NotNil(t, b.Factory(component.MustNewID("foo").Type())) assert.Nil(t, b.Factory(component.MustNewID("bar").Type())) @@ -67,7 +67,7 @@ func TestExtensionBuilderFactory(t *testing.T) { func TestNewNopExtensionConfigsAndFactories(t *testing.T) { configs, factories := NewNopExtensionConfigsAndFactories() - builder := NewExtensionBuilder(configs, factories) + builder := NewExtension(configs, factories) require.NotNil(t, builder) factory := extensiontest.NewNopFactory() diff --git a/service/service.go b/service/service.go index 9ad842a1340..46a72661045 100644 --- a/service/service.go +++ b/service/service.go @@ -56,6 +56,9 @@ type Settings struct { // Connectors builder for connectors. Connectors *connector.Builder + // Extensions builder for extensions. + Extensions builders.Extension + // Extensions configuration to its builder. ExtensionsConfigs map[component.ID]component.Config ExtensionsFactories map[component.Type]extension.Factory @@ -81,6 +84,12 @@ type Service struct { func New(ctx context.Context, set Settings, cfg Config) (*Service, error) { disableHighCard := obsreportconfig.DisableHighCardinalityMetricsfeatureGate.IsEnabled() extendedConfig := obsreportconfig.UseOtelWithSDKConfigurationForInternalTelemetryFeatureGate.IsEnabled() + + extensions := set.Extensions + if extensions == nil { + extensions = builders.NewExtension(set.ExtensionsConfigs, set.ExtensionsFactories) + } + srv := &Service{ buildInfo: set.BuildInfo, host: &serviceHost{ @@ -88,7 +97,7 @@ func New(ctx context.Context, set Settings, cfg Config) (*Service, error) { processors: set.Processors, exporters: set.Exporters, connectors: set.Connectors, - extensions: builders.NewExtensionBuilder(set.ExtensionsConfigs, set.ExtensionsFactories), + extensions: extensions, buildInfo: set.BuildInfo, asyncErrorChannel: set.AsyncErrorChannel, },