From 1d4fa7673896b81428a37aa485e7c71c41855db6 Mon Sep 17 00:00:00 2001 From: Antoine Toulme Date: Wed, 6 Mar 2024 17:06:59 -0800 Subject: [PATCH] [confmap] Remove provider.New (#9698) **Description:** Follow up to #9443 - deleting the deprecated `New` methods on providers. --- .chloggen/remove_new.yaml | 25 +++++++++++++++++++ cmd/mdatagen/loader.go | 2 +- confmap/provider/envprovider/provider.go | 9 ------- confmap/provider/envprovider/provider_test.go | 10 ++++---- confmap/provider/fileprovider/provider.go | 22 +--------------- .../provider/fileprovider/provider_test.go | 14 +++++------ confmap/provider/httpprovider/provider.go | 10 -------- .../provider/httpprovider/provider_test.go | 4 ++- confmap/provider/httpsprovider/provider.go | 11 -------- .../provider/httpsprovider/provider_test.go | 4 ++- confmap/provider/yamlprovider/provider.go | 14 ----------- .../provider/yamlprovider/provider_test.go | 19 +++++++------- otelcol/configprovider_test.go | 6 ++--- 13 files changed, 58 insertions(+), 92 deletions(-) create mode 100755 .chloggen/remove_new.yaml diff --git a/.chloggen/remove_new.yaml b/.chloggen/remove_new.yaml new file mode 100755 index 00000000000..e271f44a76c --- /dev/null +++ b/.chloggen/remove_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: Remove deprecated `provider.New` methods, use `NewWithSettings` moving forward. + +# One or more tracking issues or pull requests related to the change +issues: [9443] + +# (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: + +# 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/cmd/mdatagen/loader.go b/cmd/mdatagen/loader.go index 184063c3c47..aef532516a7 100644 --- a/cmd/mdatagen/loader.go +++ b/cmd/mdatagen/loader.go @@ -243,7 +243,7 @@ type templateContext struct { } func loadMetadata(filePath string) (metadata, error) { - cp, err := fileprovider.New().Retrieve(context.Background(), "file:"+filePath, nil) + cp, err := fileprovider.NewWithSettings(confmap.ProviderSettings{}).Retrieve(context.Background(), "file:"+filePath, nil) if err != nil { return metadata{}, err } diff --git a/confmap/provider/envprovider/provider.go b/confmap/provider/envprovider/provider.go index 61e8df8fbbf..2587d6c14e2 100644 --- a/confmap/provider/envprovider/provider.go +++ b/confmap/provider/envprovider/provider.go @@ -25,15 +25,6 @@ func NewWithSettings(confmap.ProviderSettings) confmap.Provider { return &provider{} } -// New 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` -// Deprecated: [v0.94.0] Use NewWithSettings instead. -func New() confmap.Provider { - return NewWithSettings(confmap.ProviderSettings{}) -} - 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 d7bb0e2d992..31b28b5be4b 100644 --- a/confmap/provider/envprovider/provider_test.go +++ b/confmap/provider/envprovider/provider_test.go @@ -25,18 +25,18 @@ exporters: ` func TestValidateProviderScheme(t *testing.T) { - assert.NoError(t, confmaptest.ValidateProviderScheme(New())) + assert.NoError(t, confmaptest.ValidateProviderScheme(NewWithSettings(confmap.ProviderSettings{}))) } func TestEmptyName(t *testing.T) { - env := New() + env := NewWithSettings(confmap.ProviderSettings{}) _, err := env.Retrieve(context.Background(), "", nil) require.Error(t, err) assert.NoError(t, env.Shutdown(context.Background())) } func TestUnsupportedScheme(t *testing.T) { - env := New() + env := NewWithSettings(confmap.ProviderSettings{}) _, err := env.Retrieve(context.Background(), "https://", nil) assert.Error(t, err) assert.NoError(t, env.Shutdown(context.Background())) @@ -45,7 +45,7 @@ func TestUnsupportedScheme(t *testing.T) { func TestInvalidYAML(t *testing.T) { const envName = "invalid-yaml" t.Setenv(envName, "[invalid,") - env := New() + env := NewWithSettings(confmap.ProviderSettings{}) _, err := env.Retrieve(context.Background(), envSchemePrefix+envName, nil) assert.Error(t, err) assert.NoError(t, env.Shutdown(context.Background())) @@ -55,7 +55,7 @@ func TestEnv(t *testing.T) { const envName = "default-config" t.Setenv(envName, validYAML) - env := New() + env := NewWithSettings(confmap.ProviderSettings{}) 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 a4da3af387f..3d7c3340f08 100644 --- a/confmap/provider/fileprovider/provider.go +++ b/confmap/provider/fileprovider/provider.go @@ -18,7 +18,7 @@ const schemeName = "file" type provider struct{} -// New returns a new confmap.Provider that reads the configuration from a file. +// 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: // @@ -37,26 +37,6 @@ func NewWithSettings(confmap.ProviderSettings) confmap.Provider { return &provider{} } -// New 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.94.0] Use NewWithSettings instead. -func New() confmap.Provider { - return NewWithSettings(confmap.ProviderSettings{}) -} - 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/fileprovider/provider_test.go b/confmap/provider/fileprovider/provider_test.go index dfc70756d99..19ce7625bef 100644 --- a/confmap/provider/fileprovider/provider_test.go +++ b/confmap/provider/fileprovider/provider_test.go @@ -19,25 +19,25 @@ import ( const fileSchemePrefix = schemeName + ":" func TestValidateProviderScheme(t *testing.T) { - assert.NoError(t, confmaptest.ValidateProviderScheme(New())) + assert.NoError(t, confmaptest.ValidateProviderScheme(NewWithSettings(confmap.ProviderSettings{}))) } func TestEmptyName(t *testing.T) { - fp := New() + fp := NewWithSettings(confmap.ProviderSettings{}) _, err := fp.Retrieve(context.Background(), "", nil) require.Error(t, err) require.NoError(t, fp.Shutdown(context.Background())) } func TestUnsupportedScheme(t *testing.T) { - fp := New() + fp := NewWithSettings(confmap.ProviderSettings{}) _, err := fp.Retrieve(context.Background(), "https://", nil) assert.Error(t, err) assert.NoError(t, fp.Shutdown(context.Background())) } func TestNonExistent(t *testing.T) { - fp := New() + fp := NewWithSettings(confmap.ProviderSettings{}) _, err := fp.Retrieve(context.Background(), fileSchemePrefix+filepath.Join("testdata", "non-existent.yaml"), nil) assert.Error(t, err) _, err = fp.Retrieve(context.Background(), fileSchemePrefix+absolutePath(t, filepath.Join("testdata", "non-existent.yaml")), nil) @@ -46,7 +46,7 @@ func TestNonExistent(t *testing.T) { } func TestInvalidYAML(t *testing.T) { - fp := New() + fp := NewWithSettings(confmap.ProviderSettings{}) _, err := fp.Retrieve(context.Background(), fileSchemePrefix+filepath.Join("testdata", "invalid-yaml.yaml"), nil) assert.Error(t, err) _, err = fp.Retrieve(context.Background(), fileSchemePrefix+absolutePath(t, filepath.Join("testdata", "invalid-yaml.yaml")), nil) @@ -55,7 +55,7 @@ func TestInvalidYAML(t *testing.T) { } func TestRelativePath(t *testing.T) { - fp := New() + fp := NewWithSettings(confmap.ProviderSettings{}) ret, err := fp.Retrieve(context.Background(), fileSchemePrefix+filepath.Join("testdata", "default-config.yaml"), nil) require.NoError(t, err) retMap, err := ret.AsConf() @@ -69,7 +69,7 @@ func TestRelativePath(t *testing.T) { } func TestAbsolutePath(t *testing.T) { - fp := New() + fp := NewWithSettings(confmap.ProviderSettings{}) ret, err := fp.Retrieve(context.Background(), fileSchemePrefix+absolutePath(t, filepath.Join("testdata", "default-config.yaml")), nil) require.NoError(t, err) retMap, err := ret.AsConf() diff --git a/confmap/provider/httpprovider/provider.go b/confmap/provider/httpprovider/provider.go index 966faa510fb..4c762adc237 100644 --- a/confmap/provider/httpprovider/provider.go +++ b/confmap/provider/httpprovider/provider.go @@ -16,13 +16,3 @@ import ( func NewWithSettings(set confmap.ProviderSettings) confmap.Provider { return configurablehttpprovider.New(configurablehttpprovider.HTTPScheme, set) } - -// New 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.94.0] Use NewWithSettings instead. -func New() confmap.Provider { - return NewWithSettings(confmap.ProviderSettings{}) -} diff --git a/confmap/provider/httpprovider/provider_test.go b/confmap/provider/httpprovider/provider_test.go index b649b5d3d08..b529aca0ea9 100644 --- a/confmap/provider/httpprovider/provider_test.go +++ b/confmap/provider/httpprovider/provider_test.go @@ -9,10 +9,12 @@ import ( "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" + + "go.opentelemetry.io/collector/confmap" ) func TestSupportedScheme(t *testing.T) { - fp := New() + fp := NewWithSettings(confmap.ProviderSettings{}) assert.Equal(t, "http", fp.Scheme()) require.NoError(t, fp.Shutdown(context.Background())) } diff --git a/confmap/provider/httpsprovider/provider.go b/confmap/provider/httpsprovider/provider.go index 6db2cc9b151..c228a29621d 100644 --- a/confmap/provider/httpsprovider/provider.go +++ b/confmap/provider/httpsprovider/provider.go @@ -17,14 +17,3 @@ import ( func NewWithSettings(set confmap.ProviderSettings) confmap.Provider { return configurablehttpprovider.New(configurablehttpprovider.HTTPSScheme, set) } - -// New 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.94.0] Use NewWithSettings instead. -func New() confmap.Provider { - return NewWithSettings(confmap.ProviderSettings{}) -} diff --git a/confmap/provider/httpsprovider/provider_test.go b/confmap/provider/httpsprovider/provider_test.go index f488fcc6abd..eac1d132d16 100644 --- a/confmap/provider/httpsprovider/provider_test.go +++ b/confmap/provider/httpsprovider/provider_test.go @@ -7,9 +7,11 @@ import ( "testing" "github.com/stretchr/testify/assert" + + "go.opentelemetry.io/collector/confmap" ) func TestSupportedScheme(t *testing.T) { - fp := New() + fp := NewWithSettings(confmap.ProviderSettings{}) assert.Equal(t, "https", fp.Scheme()) } diff --git a/confmap/provider/yamlprovider/provider.go b/confmap/provider/yamlprovider/provider.go index 45fca8a1fc4..01a4875580d 100644 --- a/confmap/provider/yamlprovider/provider.go +++ b/confmap/provider/yamlprovider/provider.go @@ -29,20 +29,6 @@ func NewWithSettings(confmap.ProviderSettings) confmap.Provider { return &provider{} } -// New 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.94.0] Use NewWithSettings instead. -func New() confmap.Provider { - return NewWithSettings(confmap.ProviderSettings{}) -} - 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/provider/yamlprovider/provider_test.go b/confmap/provider/yamlprovider/provider_test.go index 6e366c1c273..22fe358c480 100644 --- a/confmap/provider/yamlprovider/provider_test.go +++ b/confmap/provider/yamlprovider/provider_test.go @@ -9,29 +9,30 @@ import ( "github.com/stretchr/testify/assert" + "go.opentelemetry.io/collector/confmap" "go.opentelemetry.io/collector/confmap/confmaptest" ) func TestValidateProviderScheme(t *testing.T) { - assert.NoError(t, confmaptest.ValidateProviderScheme(New())) + assert.NoError(t, confmaptest.ValidateProviderScheme(NewWithSettings(confmap.ProviderSettings{}))) } func TestEmpty(t *testing.T) { - sp := New() + sp := NewWithSettings(confmap.ProviderSettings{}) _, err := sp.Retrieve(context.Background(), "", nil) assert.Error(t, err) assert.NoError(t, sp.Shutdown(context.Background())) } func TestInvalidYAML(t *testing.T) { - sp := New() + sp := NewWithSettings(confmap.ProviderSettings{}) _, err := sp.Retrieve(context.Background(), "yaml:[invalid,", nil) assert.Error(t, err) assert.NoError(t, sp.Shutdown(context.Background())) } func TestOneValue(t *testing.T) { - sp := New() + sp := NewWithSettings(confmap.ProviderSettings{}) ret, err := sp.Retrieve(context.Background(), "yaml:processors::batch::timeout: 2s", nil) assert.NoError(t, err) retMap, err := ret.AsConf() @@ -47,7 +48,7 @@ func TestOneValue(t *testing.T) { } func TestNamedComponent(t *testing.T) { - sp := New() + sp := NewWithSettings(confmap.ProviderSettings{}) ret, err := sp.Retrieve(context.Background(), "yaml:processors::batch/foo::timeout: 3s", nil) assert.NoError(t, err) retMap, err := ret.AsConf() @@ -63,7 +64,7 @@ func TestNamedComponent(t *testing.T) { } func TestMapEntry(t *testing.T) { - sp := New() + sp := NewWithSettings(confmap.ProviderSettings{}) ret, err := sp.Retrieve(context.Background(), "yaml:processors: {batch/foo::timeout: 3s, batch::timeout: 2s}", nil) assert.NoError(t, err) retMap, err := ret.AsConf() @@ -82,7 +83,7 @@ func TestMapEntry(t *testing.T) { } func TestArrayEntry(t *testing.T) { - sp := New() + sp := NewWithSettings(confmap.ProviderSettings{}) ret, err := sp.Retrieve(context.Background(), "yaml:service::extensions: [zpages, zpages/foo]", nil) assert.NoError(t, err) retMap, err := ret.AsConf() @@ -99,7 +100,7 @@ func TestArrayEntry(t *testing.T) { } func TestNewLine(t *testing.T) { - sp := New() + sp := NewWithSettings(confmap.ProviderSettings{}) ret, err := sp.Retrieve(context.Background(), "yaml:processors::batch/foo::timeout: 3s\nprocessors::batch::timeout: 2s", nil) assert.NoError(t, err) retMap, err := ret.AsConf() @@ -118,7 +119,7 @@ func TestNewLine(t *testing.T) { } func TestDotSeparator(t *testing.T) { - sp := New() + sp := NewWithSettings(confmap.ProviderSettings{}) ret, err := sp.Retrieve(context.Background(), "yaml:processors.batch.timeout: 4s", nil) assert.NoError(t, err) retMap, err := ret.AsConf() diff --git a/otelcol/configprovider_test.go b/otelcol/configprovider_test.go index a1c5be3eb49..0182dd26dfb 100644 --- a/otelcol/configprovider_test.go +++ b/otelcol/configprovider_test.go @@ -48,7 +48,7 @@ func TestConfigProviderYaml(t *testing.T) { require.NoError(t, err) uriLocation := "yaml:" + string(yamlBytes) - provider := yamlprovider.New() + provider := yamlprovider.NewWithSettings(confmap.ProviderSettings{}) set := ConfigProviderSettings{ ResolverSettings: confmap.ResolverSettings{ URIs: []string{uriLocation}, @@ -73,7 +73,7 @@ func TestConfigProviderYaml(t *testing.T) { func TestConfigProviderFile(t *testing.T) { uriLocation := "file:" + filepath.Join("testdata", "otelcol-nop.yaml") - provider := fileprovider.New() + provider := fileprovider.NewWithSettings(confmap.ProviderSettings{}) set := ConfigProviderSettings{ ResolverSettings: confmap.ResolverSettings{ URIs: []string{uriLocation}, @@ -101,7 +101,7 @@ func TestConfigProviderFile(t *testing.T) { func TestGetConfmap(t *testing.T) { uriLocation := "file:" + filepath.Join("testdata", "otelcol-nop.yaml") - provider := fileprovider.New() + provider := fileprovider.NewWithSettings(confmap.ProviderSettings{}) set := ConfigProviderSettings{ ResolverSettings: confmap.ResolverSettings{ URIs: []string{uriLocation},