From 41e63f6116f02269db8b69307fcdc889a9473b38 Mon Sep 17 00:00:00 2001 From: Evan Bradley Date: Tue, 9 Apr 2024 15:33:02 -0400 Subject: [PATCH] Create confmap module factory types --- confmap/confmap.go | 20 ++++++++++++++++++++ confmap/converter.go | 13 ++++++++++++- confmap/converter/expandconverter/expand.go | 7 +++++++ confmap/provider.go | 13 ++++++++++++- confmap/provider/envprovider/provider.go | 9 +++++++++ confmap/provider/fileprovider/provider.go | 20 ++++++++++++++++++++ confmap/provider/httpprovider/provider.go | 10 ++++++++++ confmap/provider/httpsprovider/provider.go | 13 ++++++++++++- confmap/provider/yamlprovider/provider.go | 14 ++++++++++++++ confmap/resolver.go | 12 ++++-------- confmap/resolver_test.go | 14 +++++++------- otelcol/collector_test.go | 2 +- otelcol/command_test.go | 10 +++++----- otelcol/command_validate_test.go | 4 ++-- otelcol/configprovider.go | 12 ++++++------ otelcol/configprovider_test.go | 6 +++--- otelcol/otelcoltest/config.go | 10 +++++----- 17 files changed, 149 insertions(+), 40 deletions(-) diff --git a/confmap/confmap.go b/confmap/confmap.go index 5bd6d356016..9e602a62689 100644 --- a/confmap/confmap.go +++ b/confmap/confmap.go @@ -417,3 +417,23 @@ func zeroSliceHookFunc() mapstructure.DecodeHookFuncValue { return from.Interface(), nil } } + +type moduleFactory[T any, S any] interface { + Create(s S) T +} + +type createConfmapFunc[T any, S any] func(s S) T + +type confmapModuleFactory[T any, S any] struct { + f createConfmapFunc[T, S] +} + +func (c confmapModuleFactory[T, S]) Create(s S) T { + return c.f(s) +} + +func newConfmapModuleFactory[T any, S any](f createConfmapFunc[T, S]) moduleFactory[T, S] { + return confmapModuleFactory[T, S]{ + f: f, + } +} diff --git a/confmap/converter.go b/confmap/converter.go index 17316d10304..2dce87e496e 100644 --- a/confmap/converter.go +++ b/confmap/converter.go @@ -8,9 +8,20 @@ import ( ) // ConverterSettings are the settings to initialize a Converter. -// Any Converter should take this as a parameter in its constructor. type ConverterSettings struct{} +// ConverterFactory defines a factory that can be used to instantiate +// new instances of a Converter. +type ConverterFactory = moduleFactory[Converter, ConverterSettings] + +// CreateConverterFunc is a function that creates a Converter instance. +type CreateConverterFunc = createConfmapFunc[Converter, ConverterSettings] + +// NewConverterFactory can be used to create a ConverterFactory. +func NewConverterFactory(f CreateConverterFunc) ConverterFactory { + return newConfmapModuleFactory(f) +} + // Converter is a converter interface for the confmap.Conf that allows distributions // (in the future components as well) to build backwards compatible config converters. type Converter interface { diff --git a/confmap/converter/expandconverter/expand.go b/confmap/converter/expandconverter/expand.go index 4ea461fcf7e..71c87a1e701 100644 --- a/confmap/converter/expandconverter/expand.go +++ b/confmap/converter/expandconverter/expand.go @@ -22,6 +22,7 @@ type converter struct { } // New returns a confmap.Converter, that expands all environment variables for a given confmap.Conf. +// Deprecated [v0.99.0]: Use NewFactory instead. // // Notice: This API is experimental. func New(_ confmap.ConverterSettings) confmap.Converter { @@ -31,6 +32,12 @@ func New(_ confmap.ConverterSettings) confmap.Converter { } } +// NewFactory returns a factory for a confmap.Converter, +// which expands all environment variables for a given confmap.Conf. +func NewFactory() confmap.ConverterFactory { + return confmap.NewConverterFactory(New) +} + func (c converter) Convert(_ context.Context, conf *confmap.Conf) error { out := make(map[string]any) for _, k := range conf.AllKeys() { diff --git a/confmap/provider.go b/confmap/provider.go index 8fd31e376ce..476f1660983 100644 --- a/confmap/provider.go +++ b/confmap/provider.go @@ -11,11 +11,22 @@ import ( ) // ProviderSettings are the settings to initialize a Provider. -// Any Provider should take this as a parameter in its constructor. type ProviderSettings struct { Logger *zap.Logger } +// ProviderFactory defines a factory that can be used to instantiate +// new instances of a Provider. +type ProviderFactory = moduleFactory[Provider, ProviderSettings] + +// CreateProviderFunc is a function that creates a Provider instance. +type CreateProviderFunc = createConfmapFunc[Provider, ProviderSettings] + +// NewProviderFactory can be used to create a ProviderFactory. +func NewProviderFactory(f CreateProviderFunc) ProviderFactory { + return newConfmapModuleFactory(f) +} + // Provider is an interface that helps to retrieve a config map and watch for any // changes to the config map. Implementations may load the config from a file, // a database or any other source. diff --git a/confmap/provider/envprovider/provider.go b/confmap/provider/envprovider/provider.go index 9a69f958838..2e4d38219c4 100644 --- a/confmap/provider/envprovider/provider.go +++ b/confmap/provider/envprovider/provider.go @@ -22,6 +22,7 @@ type provider struct { } // NewWithSettings returns a new confmap.Provider that reads the configuration from the given environment variable. +// Deprecated [v0.99.0]: Use NewFactory instead. // // This Provider supports "env" scheme, and can be called with a selector: // `env:NAME_OF_ENVIRONMENT_VARIABLE` @@ -31,6 +32,14 @@ func NewWithSettings(ps confmap.ProviderSettings) confmap.Provider { } } +// NewWithSettings returns a new confmap.Provider that reads the configuration from the given environment variable. +// +// This Provider supports "env" scheme, and can be called with a selector: +// `env:NAME_OF_ENVIRONMENT_VARIABLE` +func NewFactory() confmap.ProviderFactory { + return confmap.NewProviderFactory(NewWithSettings) +} + func (emp *provider) Retrieve(_ context.Context, uri string, _ confmap.WatcherFunc) (*confmap.Retrieved, error) { if !strings.HasPrefix(uri, schemeName+":") { return nil, fmt.Errorf("%q uri is not supported by %q provider", uri, schemeName) diff --git a/confmap/provider/fileprovider/provider.go b/confmap/provider/fileprovider/provider.go index 3d7c3340f08..ee312a71d24 100644 --- a/confmap/provider/fileprovider/provider.go +++ b/confmap/provider/fileprovider/provider.go @@ -19,6 +19,7 @@ const schemeName = "file" type provider struct{} // NewWithSettings returns a new confmap.Provider that reads the configuration from a file. +// Deprecated [v0.99.0]: Use NewFactory instead. // // This Provider supports "file" scheme, and can be called with a "uri" that follows: // @@ -37,6 +38,25 @@ func NewWithSettings(confmap.ProviderSettings) confmap.Provider { return &provider{} } +// NewFactory returns a factory for a confmap.Provider that reads the configuration from a file. +// +// This Provider supports "file" scheme, and can be called with a "uri" that follows: +// +// file-uri = "file:" local-path +// local-path = [ drive-letter ] file-path +// drive-letter = ALPHA ":" +// +// The "file-path" can be relative or absolute, and it can be any OS supported format. +// +// Examples: +// `file:path/to/file` - relative path (unix, windows) +// `file:/path/to/file` - absolute path (unix, windows) +// `file:c:/path/to/file` - absolute path including drive-letter (windows) +// `file:c:\path\to\file` - absolute path including drive-letter (windows) +func NewFactory() confmap.ProviderFactory { + return confmap.NewProviderFactory(NewWithSettings) +} + func (fmp *provider) Retrieve(_ context.Context, uri string, _ confmap.WatcherFunc) (*confmap.Retrieved, error) { if !strings.HasPrefix(uri, schemeName+":") { return nil, fmt.Errorf("%q uri is not supported by %q provider", uri, schemeName) diff --git a/confmap/provider/httpprovider/provider.go b/confmap/provider/httpprovider/provider.go index 4c762adc237..3f8e22ed98f 100644 --- a/confmap/provider/httpprovider/provider.go +++ b/confmap/provider/httpprovider/provider.go @@ -9,6 +9,7 @@ import ( ) // NewWithSettings returns a new confmap.Provider that reads the configuration from a http server. +// Deprecated [v0.99.0]: Use NewFactory instead. // // This Provider supports "http" scheme. // @@ -16,3 +17,12 @@ import ( func NewWithSettings(set confmap.ProviderSettings) confmap.Provider { return configurablehttpprovider.New(configurablehttpprovider.HTTPScheme, set) } + +// NewFactory returns a factory for a confmap.Provider that reads the configuration from a http server. +// +// This Provider supports "http" scheme. +// +// One example for HTTP URI is: http://localhost:3333/getConfig +func NewFactory() confmap.ProviderFactory { + return confmap.NewProviderFactory(NewWithSettings) +} diff --git a/confmap/provider/httpsprovider/provider.go b/confmap/provider/httpsprovider/provider.go index c228a29621d..b5b1e5bd12d 100644 --- a/confmap/provider/httpsprovider/provider.go +++ b/confmap/provider/httpsprovider/provider.go @@ -8,7 +8,8 @@ import ( "go.opentelemetry.io/collector/confmap/provider/internal/configurablehttpprovider" ) -// New returns a new confmap.Provider that reads the configuration from a https server. +// NewWithSettings returns a new confmap.Provider that reads the configuration from a https server. +// Deprecated [v0.99.0]: Use NewFactory instead. // // This Provider supports "https" scheme. One example of an HTTPS URI is: https://localhost:3333/getConfig // @@ -17,3 +18,13 @@ import ( func NewWithSettings(set confmap.ProviderSettings) confmap.Provider { return configurablehttpprovider.New(configurablehttpprovider.HTTPSScheme, set) } + +// NewFactory returns a factory for a confmap.Provider that reads the configuration from a https server. +// +// This Provider supports "https" scheme. One example of an HTTPS URI is: https://localhost:3333/getConfig +// +// To add extra CA certificates you need to install certificates in the system pool. This procedure is operating system +// dependent. E.g.: on Linux please refer to the `update-ca-trust` command. +func NewFactory() confmap.ProviderFactory { + return confmap.NewProviderFactory(NewWithSettings) +} diff --git a/confmap/provider/yamlprovider/provider.go b/confmap/provider/yamlprovider/provider.go index 01a4875580d..fdedf938605 100644 --- a/confmap/provider/yamlprovider/provider.go +++ b/confmap/provider/yamlprovider/provider.go @@ -29,6 +29,20 @@ func NewWithSettings(confmap.ProviderSettings) confmap.Provider { return &provider{} } +// NewFactory returns a factory for a confmap.Provider that allows to provide yaml bytes. +// Deprecated [v0.99.0]: Use NewFactory instead. +// +// This Provider supports "yaml" scheme, and can be called with a "uri" that follows: +// +// bytes-uri = "yaml:" yaml-bytes +// +// Examples: +// `yaml:processors::batch::timeout: 2s` +// `yaml:processors::batch/foo::timeout: 3s` +func NewFactory() confmap.ProviderFactory { + return confmap.NewProviderFactory(NewWithSettings) +} + func (s *provider) Retrieve(_ context.Context, uri string, _ confmap.WatcherFunc) (*confmap.Retrieved, error) { if !strings.HasPrefix(uri, schemeName+":") { return nil, fmt.Errorf("%q uri is not supported by %q provider", uri, schemeName) diff --git a/confmap/resolver.go b/confmap/resolver.go index 6541ff40acb..36c043447d7 100644 --- a/confmap/resolver.go +++ b/confmap/resolver.go @@ -27,10 +27,6 @@ type Resolver struct { watcher chan error } -type ConverterFactory = func(ConverterSettings) Converter - -type ProviderFactory = func(ProviderSettings) Provider - // ResolverSettings are the settings to configure the behavior of the Resolver. type ResolverSettings struct { // URIs locations from where the Conf is retrieved, and merged in the given order. @@ -97,8 +93,8 @@ func NewResolver(set ResolverSettings) (*Resolver, error) { providers = set.Providers } else { providers = make(map[string]Provider, len(set.ProviderFactories)) - for _, newProvider := range set.ProviderFactories { - provider := newProvider(set.ProviderSettings) + for _, factory := range set.ProviderFactories { + provider := factory.Create(set.ProviderSettings) providers[provider.Scheme()] = provider } } @@ -110,8 +106,8 @@ func NewResolver(set ResolverSettings) (*Resolver, error) { converters = set.Converters } else { converters = make([]Converter, len(set.ConverterFactories)) - for i, newConverter := range set.ConverterFactories { - converters[i] = newConverter(set.ConverterSettings) + for i, factory := range set.ConverterFactories { + converters[i] = factory.Create(set.ConverterSettings) } } diff --git a/confmap/resolver_test.go b/confmap/resolver_test.go index 2084df1cdbe..809c553e0a1 100644 --- a/confmap/resolver_test.go +++ b/confmap/resolver_test.go @@ -48,9 +48,9 @@ func (m *mockProvider) Shutdown(context.Context) error { } func newMockProvider(m *mockProvider) ProviderFactory { - return func(_ ProviderSettings) Provider { + return NewProviderFactory(func(_ ProviderSettings) Provider { return m - } + }) } type fakeProvider struct { @@ -65,12 +65,12 @@ func newFileProvider(t testing.TB) ProviderFactory { } func newFakeProvider(scheme string, ret func(ctx context.Context, uri string, watcher WatcherFunc) (*Retrieved, error)) ProviderFactory { - return func(ProviderSettings) Provider { + return NewProviderFactory(func(ProviderSettings) Provider { return &fakeProvider{ scheme: scheme, ret: ret, } - } + }) } func (f *fakeProvider) Retrieve(ctx context.Context, uri string, watcher WatcherFunc) (*Retrieved, error) { @@ -174,12 +174,12 @@ func TestResolverErrors(t *testing.T) { mockProviderFuncs := make([]ProviderFactory, len(tt.providers)) for i, provider := range tt.providers { p := provider - mockProviderFuncs[i] = func(_ ProviderSettings) Provider { return p } + mockProviderFuncs[i] = NewProviderFactory(func(_ ProviderSettings) Provider { return p }) } converterFuncs := make([]ConverterFactory, len(tt.converters)) for i, converter := range tt.converters { c := converter - converterFuncs[i] = func(_ ConverterSettings) Converter { return c } + converterFuncs[i] = NewConverterFactory(func(_ ConverterSettings) Converter { return c }) } resolver, err := NewResolver(ResolverSettings{URIs: tt.locations, ProviderFactories: mockProviderFuncs, ConverterFactories: converterFuncs}) if tt.expectBuildErr { @@ -376,7 +376,7 @@ func TestCantConfigureTwoConverterSettings(t *testing.T) { _, err := NewResolver(ResolverSettings{ URIs: []string{filepath.Join("testdata", "config.yaml")}, ProviderFactories: []ProviderFactory{newFileProvider(t)}, - ConverterFactories: []ConverterFactory{func(_ ConverterSettings) Converter { return &mockConverter{} }}, + ConverterFactories: []ConverterFactory{NewConverterFactory(func(_ ConverterSettings) Converter { return &mockConverter{} })}, Converters: []Converter{&mockConverter{err: errors.New("converter_err")}}, }) require.Error(t, err) diff --git a/otelcol/collector_test.go b/otelcol/collector_test.go index dd33e8d4d73..4970876a0c8 100644 --- a/otelcol/collector_test.go +++ b/otelcol/collector_test.go @@ -440,7 +440,7 @@ func TestPassConfmapToServiceFailure(t *testing.T) { ConfigProviderSettings: ConfigProviderSettings{ ResolverSettings: confmap.ResolverSettings{ URIs: []string{filepath.Join("testdata", "otelcol-invalid.yaml")}, - ProviderFactories: []confmap.ProviderFactory{newFailureProvider}, + ProviderFactories: []confmap.ProviderFactory{confmap.NewProviderFactory(newFailureProvider)}, }, }, } diff --git a/otelcol/command_test.go b/otelcol/command_test.go index 83cc3836611..69e1a943d2b 100644 --- a/otelcol/command_test.go +++ b/otelcol/command_test.go @@ -53,8 +53,8 @@ func TestAddFlagToSettings(t *testing.T) { ConfigProviderSettings: ConfigProviderSettings{ ResolverSettings: confmap.ResolverSettings{ URIs: []string{filepath.Join("testdata", "otelcol-invalid.yaml")}, - ProviderFactories: []confmap.ProviderFactory{fileprovider.NewWithSettings}, - ConverterFactories: []confmap.ConverterFactory{expandconverter.New}, + ProviderFactories: []confmap.ProviderFactory{fileprovider.NewFactory()}, + ConverterFactories: []confmap.ConverterFactory{expandconverter.NewFactory()}, }, }, } @@ -88,7 +88,7 @@ func TestInvalidCollectorSettings(t *testing.T) { set := CollectorSettings{ ConfigProviderSettings: ConfigProviderSettings{ ResolverSettings: confmap.ResolverSettings{ - ConverterFactories: []confmap.ConverterFactory{expandconverter.New}, + ConverterFactories: []confmap.ConverterFactory{expandconverter.NewFactory()}, URIs: []string{"--config=otelcol-nop.yaml"}, }, }, @@ -102,8 +102,8 @@ func TestNewCommandInvalidComponent(t *testing.T) { set := ConfigProviderSettings{ ResolverSettings: confmap.ResolverSettings{ URIs: []string{filepath.Join("testdata", "otelcol-invalid.yaml")}, - ProviderFactories: []confmap.ProviderFactory{fileprovider.NewWithSettings}, - ConverterFactories: []confmap.ConverterFactory{expandconverter.New}, + ProviderFactories: []confmap.ProviderFactory{fileprovider.NewFactory()}, + ConverterFactories: []confmap.ConverterFactory{expandconverter.NewFactory()}, }, } diff --git a/otelcol/command_validate_test.go b/otelcol/command_validate_test.go index 0ac05f88b26..30d291d4a56 100644 --- a/otelcol/command_validate_test.go +++ b/otelcol/command_validate_test.go @@ -27,8 +27,8 @@ func TestValidateSubCommandInvalidComponents(t *testing.T) { ConfigProviderSettings{ ResolverSettings: confmap.ResolverSettings{ URIs: []string{filepath.Join("testdata", "otelcol-invalid-components.yaml")}, - ProviderFactories: []confmap.ProviderFactory{fileprovider.NewWithSettings}, - ConverterFactories: []confmap.ConverterFactory{expandconverter.New}, + ProviderFactories: []confmap.ProviderFactory{fileprovider.NewFactory()}, + ConverterFactories: []confmap.ConverterFactory{expandconverter.NewFactory()}, }, }) require.NoError(t, err) diff --git a/otelcol/configprovider.go b/otelcol/configprovider.go index 5038fec7ad6..477541fd4bd 100644 --- a/otelcol/configprovider.go +++ b/otelcol/configprovider.go @@ -136,13 +136,13 @@ func newDefaultConfigProviderSettings(uris []string) ConfigProviderSettings { ResolverSettings: confmap.ResolverSettings{ URIs: uris, ProviderFactories: []confmap.ProviderFactory{ - fileprovider.NewWithSettings, - envprovider.NewWithSettings, - yamlprovider.NewWithSettings, - httpprovider.NewWithSettings, - httpsprovider.NewWithSettings, + fileprovider.NewFactory(), + envprovider.NewFactory(), + yamlprovider.NewFactory(), + httpprovider.NewFactory(), + httpsprovider.NewFactory(), }, - ConverterFactories: []confmap.ConverterFactory{expandconverter.New}, + ConverterFactories: []confmap.ConverterFactory{expandconverter.NewFactory()}, }, } } diff --git a/otelcol/configprovider_test.go b/otelcol/configprovider_test.go index e13f7915aac..b9ad46cb70d 100644 --- a/otelcol/configprovider_test.go +++ b/otelcol/configprovider_test.go @@ -51,7 +51,7 @@ func TestConfigProviderYaml(t *testing.T) { set := ConfigProviderSettings{ ResolverSettings: confmap.ResolverSettings{ URIs: []string{uriLocation}, - ProviderFactories: []confmap.ProviderFactory{yamlprovider.NewWithSettings}, + ProviderFactories: []confmap.ProviderFactory{yamlprovider.NewFactory()}, }, } @@ -75,7 +75,7 @@ func TestConfigProviderFile(t *testing.T) { set := ConfigProviderSettings{ ResolverSettings: confmap.ResolverSettings{ URIs: []string{uriLocation}, - ProviderFactories: []confmap.ProviderFactory{fileprovider.NewWithSettings}, + ProviderFactories: []confmap.ProviderFactory{fileprovider.NewFactory()}, }, } @@ -102,7 +102,7 @@ func TestGetConfmap(t *testing.T) { set := ConfigProviderSettings{ ResolverSettings: confmap.ResolverSettings{ URIs: []string{uriLocation}, - ProviderFactories: []confmap.ProviderFactory{fileprovider.NewWithSettings}, + ProviderFactories: []confmap.ProviderFactory{fileprovider.NewFactory()}, }, } diff --git a/otelcol/otelcoltest/config.go b/otelcol/otelcoltest/config.go index 6b082461e4f..6191eb9dbec 100644 --- a/otelcol/otelcoltest/config.go +++ b/otelcol/otelcoltest/config.go @@ -22,12 +22,12 @@ func LoadConfig(fileName string, factories otelcol.Factories) (*otelcol.Config, ResolverSettings: confmap.ResolverSettings{ URIs: []string{fileName}, ProviderFactories: []confmap.ProviderFactory{ - fileprovider.NewWithSettings, - envprovider.NewWithSettings, - yamlprovider.NewWithSettings, - httpprovider.NewWithSettings, + fileprovider.NewFactory(), + envprovider.NewFactory(), + yamlprovider.NewFactory(), + httpprovider.NewFactory(), }, - ConverterFactories: []confmap.ConverterFactory{expandconverter.New}, + ConverterFactories: []confmap.ConverterFactory{expandconverter.NewFactory()}, }, }) if err != nil {