From 49d994c4139363e74c84fdb060808a82d2be4b93 Mon Sep 17 00:00:00 2001 From: Evan Bradley <11745660+evan-bradley@users.noreply.github.com> Date: Fri, 10 May 2024 12:56:02 -0400 Subject: [PATCH] [confmap] Deprecate `NewWithSettings` and `New` (#10134) #### Description Each of these was deprecated in v0.99.0, so I think removing them in v0.101.0 is a safe deprecation period for an API that is likely only used by vendor distros built without ocb. If we would like to extend the deprecation period or break this change into PRs for each module, let me know and I'll make the necessary changes. --- .chloggen/deprecate-confmap-new.yaml | 25 +++++++++++++++++ confmap/converter/expandconverter/expand.go | 19 +++++-------- confmap/provider/envprovider/provider.go | 18 +++++-------- confmap/provider/envprovider/provider_test.go | 6 ++--- confmap/provider/fileprovider/provider.go | 27 ++++--------------- confmap/provider/httpprovider/provider.go | 17 ++++-------- confmap/provider/httpsprovider/provider.go | 18 ++++--------- confmap/provider/yamlprovider/provider.go | 21 ++++----------- 8 files changed, 61 insertions(+), 90 deletions(-) create mode 100644 .chloggen/deprecate-confmap-new.yaml diff --git a/.chloggen/deprecate-confmap-new.yaml b/.chloggen/deprecate-confmap-new.yaml new file mode 100644 index 00000000000..d10d3a8a89e --- /dev/null +++ b/.chloggen/deprecate-confmap-new.yaml @@ -0,0 +1,25 @@ +# Use this changelog template to create an entry for release notes. + +# One of 'breaking', 'deprecation', 'new_component', 'enhancement', 'bug_fix' +change_type: breaking + +# The name of the component, or a single word describing the area of concern, (e.g. otlpreceiver) +component: confmap + +# A brief description of the change. Surround your text with quotes ("") if it needs to start with a backtick (`). +note: Deprecate `NewWithSettings` for all Providers and `New` for all Converters + +# One or more tracking issues or pull requests related to the change +issues: [10134] + +# (Optional) One or more lines of additional information to render under the primary note. +# These lines will be padded with 2 spaces and then inserted directly into the document. +# Use pipe (|) for multiline entries. +subtext: Use `NewFactory` instead for all affected modules. + +# Optional: The change log or logs in which this entry should be included. +# e.g. '[user]' or '[user, api]' +# Include 'user' if the change is relevant to end users. +# Include 'api' if there is a change to a library API. +# Default: '[user]' +change_logs: [api] diff --git a/confmap/converter/expandconverter/expand.go b/confmap/converter/expandconverter/expand.go index f07617cd3d6..ea6977fda0f 100644 --- a/confmap/converter/expandconverter/expand.go +++ b/confmap/converter/expandconverter/expand.go @@ -21,24 +21,19 @@ type converter struct { loggedDeprecations map[string]struct{} } -// New returns a confmap.Converter, that expands all environment variables for a given confmap.Conf. -// -// Notice: This API is experimental. -// -// Deprecated: [v0.99.0] Use NewFactory instead. -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(newConverter) +} + +func newConverter(_ confmap.ConverterSettings) confmap.Converter { return converter{ loggedDeprecations: make(map[string]struct{}), logger: zap.NewNop(), // TODO: pass logger in ConverterSettings } } -// 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/envprovider/provider.go b/confmap/provider/envprovider/provider.go index c153893cb87..9819a9e654a 100644 --- a/confmap/provider/envprovider/provider.go +++ b/confmap/provider/envprovider/provider.go @@ -21,26 +21,20 @@ type provider struct { logger *zap.Logger } -// NewWithSettings returns a new confmap.Provider that reads the configuration from the given environment variable. +// NewFactory returns a factory for a 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` -// -// Deprecated: [v0.99.0] Use NewFactory instead. -func NewWithSettings(ps confmap.ProviderSettings) confmap.Provider { +func NewFactory() confmap.ProviderFactory { + return confmap.NewProviderFactory(newProvider) +} + +func newProvider(ps confmap.ProviderSettings) confmap.Provider { return &provider{ logger: ps.Logger, } } -// NewFactory returns a factory for a 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/envprovider/provider_test.go b/confmap/provider/envprovider/provider_test.go index c5fcbd91eee..f1f5931018c 100644 --- a/confmap/provider/envprovider/provider_test.go +++ b/confmap/provider/envprovider/provider_test.go @@ -77,7 +77,7 @@ func TestEnvWithLogger(t *testing.T) { core, ol := observer.New(zap.WarnLevel) logger := zap.New(core) - env := NewWithSettings(confmap.ProviderSettings{Logger: logger}) + env := NewFactory().Create(confmap.ProviderSettings{Logger: logger}) ret, err := env.Retrieve(context.Background(), envSchemePrefix+envName, nil) require.NoError(t, err) retMap, err := ret.AsConf() @@ -97,7 +97,7 @@ func TestUnsetEnvWithLoggerWarn(t *testing.T) { core, ol := observer.New(zap.WarnLevel) logger := zap.New(core) - env := NewWithSettings(confmap.ProviderSettings{Logger: logger}) + env := NewFactory().Create(confmap.ProviderSettings{Logger: logger}) ret, err := env.Retrieve(context.Background(), envSchemePrefix+envName, nil) require.NoError(t, err) retMap, err := ret.AsConf() @@ -121,7 +121,7 @@ func TestEmptyEnvWithLoggerWarn(t *testing.T) { core, ol := observer.New(zap.InfoLevel) logger := zap.New(core) - env := NewWithSettings(confmap.ProviderSettings{Logger: logger}) + env := NewFactory().Create(confmap.ProviderSettings{Logger: logger}) ret, err := env.Retrieve(context.Background(), envSchemePrefix+envName, nil) require.NoError(t, err) retMap, err := ret.AsConf() diff --git a/confmap/provider/fileprovider/provider.go b/confmap/provider/fileprovider/provider.go index b1723ad16c6..fe958280cfb 100644 --- a/confmap/provider/fileprovider/provider.go +++ b/confmap/provider/fileprovider/provider.go @@ -18,27 +18,6 @@ const schemeName = "file" type provider struct{} -// NewWithSettings returns a new 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) -// -// Deprecated: [v0.99.0] Use NewFactory instead. -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: @@ -55,7 +34,11 @@ func NewWithSettings(confmap.ProviderSettings) confmap.Provider { // `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) + return confmap.NewProviderFactory(newProvider) +} + +func newProvider(confmap.ProviderSettings) confmap.Provider { + return &provider{} } func (fmp *provider) Retrieve(_ context.Context, uri string, _ confmap.WatcherFunc) (*confmap.Retrieved, error) { diff --git a/confmap/provider/httpprovider/provider.go b/confmap/provider/httpprovider/provider.go index 6ed2b6d104c..c47c1df5d99 100644 --- a/confmap/provider/httpprovider/provider.go +++ b/confmap/provider/httpprovider/provider.go @@ -8,22 +8,15 @@ import ( "go.opentelemetry.io/collector/confmap/provider/internal/configurablehttpprovider" ) -// NewWithSettings returns a new 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 -// -// Deprecated: [v0.99.0] Use NewFactory instead. -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) + return confmap.NewProviderFactory(newProvider) +} + +func newProvider(set confmap.ProviderSettings) confmap.Provider { + return configurablehttpprovider.New(configurablehttpprovider.HTTPScheme, set) } diff --git a/confmap/provider/httpsprovider/provider.go b/confmap/provider/httpsprovider/provider.go index 0ee3672cf86..579c15babf2 100644 --- a/confmap/provider/httpsprovider/provider.go +++ b/confmap/provider/httpsprovider/provider.go @@ -8,18 +8,6 @@ import ( "go.opentelemetry.io/collector/confmap/provider/internal/configurablehttpprovider" ) -// NewWithSettings returns a new 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. -// -// Deprecated: [v0.99.0] Use NewFactory instead. -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 @@ -27,5 +15,9 @@ func NewWithSettings(set confmap.ProviderSettings) confmap.Provider { // 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) + return confmap.NewProviderFactory(newProvider) +} + +func newProvider(set confmap.ProviderSettings) confmap.Provider { + return configurablehttpprovider.New(configurablehttpprovider.HTTPSScheme, set) } diff --git a/confmap/provider/yamlprovider/provider.go b/confmap/provider/yamlprovider/provider.go index 4f5c327ce4b..949fc71bf7d 100644 --- a/confmap/provider/yamlprovider/provider.go +++ b/confmap/provider/yamlprovider/provider.go @@ -16,21 +16,6 @@ const schemeName = "yaml" type provider struct{} -// NewWithSettings returns a new confmap.Provider that allows to provide yaml bytes. -// -// 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` -// -// Deprecated: [v0.99.0] Use NewFactory instead. -func NewWithSettings(confmap.ProviderSettings) confmap.Provider { - return &provider{} -} - // NewFactory returns a factory for a confmap.Provider that allows to provide yaml bytes. // // This Provider supports "yaml" scheme, and can be called with a "uri" that follows: @@ -41,7 +26,11 @@ func NewWithSettings(confmap.ProviderSettings) confmap.Provider { // `yaml:processors::batch::timeout: 2s` // `yaml:processors::batch/foo::timeout: 3s` func NewFactory() confmap.ProviderFactory { - return confmap.NewProviderFactory(NewWithSettings) + return confmap.NewProviderFactory(newProvider) +} + +func newProvider(confmap.ProviderSettings) confmap.Provider { + return &provider{} } func (s *provider) Retrieve(_ context.Context, uri string, _ confmap.WatcherFunc) (*confmap.Retrieved, error) {