[confmap] Pass ConverterSettings and ProviderSettings to converters a…

…nd providers (#9443)

**Description:** 

For both #5615 and #9162 we need to be able to log during the confmap
resolution.

My proposed solution is to pass a `*zap.Logger` to converters and
providers during initialization. These components can then use this to
log any warnings they need. This PR does the first step: being able to
pass anything to converters and providers during initialization.

The obvious alternative to this is to change the interface of
`confmap.Provider` and `confmap.Converter` to pass any warnings in an
explicit struct. I think the `*zap.Logger` alternative is more natural
for developers of providers and converters: you just use a logger like
everywhere else in the Collector.

One problem for the Collector usage of `confmap` is: How does one pass a
`*zap.Logger` before knowing how a `*zap.Logger` should be configured? I
think we can work around this by:
1. Passing a special 'deferred' Logger that just stores the warnings
without actually logging them (we can use something like
`zaptest/observer` for this)
2. Resolving configuration
3. Building a `*zap.Logger` with said configuration
4. Logging the entries stored in (1) with the logger from (3) (using
`zaptest/observer` we can do that by taking the `zapcore.Core` out of
the logger and manually writing)

**We don't actually need ProviderSettings today, just ConverterSettings,
but I think it can still be useful.**

**Link to tracking Issue:** Relates to #5615 and #9162

---------

Co-authored-by: Evan Bradley <[email protected]>

.chloggen/convertersettings.yaml

@@ -0,0 +1,26 @@
+# 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/converter/expandconverter
+
+# A brief description of the change.  Surround your text with quotes ("") if it needs to start with a backtick (`).
+note: Add `confmap.ConverterSettings` argument to experimental `expandconverter.New` function.
+
+# One or more tracking issues or pull requests related to the change
+issues: [5615, 9162]
+
+# (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: | 
+  - The `confmap.ConverterSettings` struct currently has no fields. It will be used to pass a logger.
+
+# 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]

.chloggen/providersettings.yaml

@@ -0,0 +1,26 @@
+# Use this changelog template to create an entry for release notes.
+
+# One of 'breaking', 'deprecation', 'new_component', 'enhancement', 'bug_fix'
+change_type: deprecation
+
+# The name of the component, or a single word describing the area of concern, (e.g. otlpreceiver)
+component: confmap/provider
+
+# A brief description of the change.  Surround your text with quotes ("") if it needs to start with a backtick (`).
+note: Deprecate <provider>.New in favor of <provider>.NewWithSettings for all core providers
+
+# One or more tracking issues or pull requests related to the change
+issues: [5615, 9162]
+
+# (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: | 
+  - NewWithSettings now takes an empty confmap.ProviderSettings struct. This will be used to pass a logger in the future.
+
+# 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]

confmap/converter.go

@@ -7,6 +7,10 @@ import (
 	"context"
 )
 
+// ConverterSettings are the settings to initialize a Converter.
+// Any Converter should take this as a parameter in its constructor.
+type ConverterSettings struct{}
+
 // 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 {

confmap/converter/expandconverter/expand.go

@@ -15,7 +15,7 @@ type converter struct{}
 // New returns a confmap.Converter, that expands all environment variables for a given confmap.Conf.
 //
 // Notice: This API is experimental.
-func New() confmap.Converter {
+func New(_ confmap.ConverterSettings) confmap.Converter {
 	return converter{}
 }
 

confmap/converter/expandconverter/expand_test.go

@@ -45,7 +45,7 @@ func TestNewExpandConverter(t *testing.T) {
 			require.NoError(t, err, "Unable to get config")
 
 			// Test that expanded configs are the same with the simple config with no env vars.
-			require.NoError(t, New().Convert(context.Background(), conf))
+			require.NoError(t, New(confmap.ConverterSettings{}).Convert(context.Background(), conf))
 			assert.Equal(t, expectedCfgMap.ToStringMap(), conf.ToStringMap())
 		})
 	}
@@ -64,7 +64,7 @@ func TestNewExpandConverter_EscapedMaps(t *testing.T) {
 				"recv": "$MAP_VALUE",
 			}},
 	)
-	require.NoError(t, New().Convert(context.Background(), conf))
+	require.NoError(t, New(confmap.ConverterSettings{}).Convert(context.Background(), conf))
 
 	expectedMap := map[string]any{
 		"test_string_map": map[string]any{
@@ -101,7 +101,7 @@ func TestNewExpandConverter_EscapedEnvVars(t *testing.T) {
 			// escaped $ alone
 			"recv.7": "$",
 		}}
-	require.NoError(t, New().Convert(context.Background(), conf))
+	require.NoError(t, New(confmap.ConverterSettings{}).Convert(context.Background(), conf))
 	assert.Equal(t, expectedMap, conf.ToStringMap())
 }
 
@@ -154,7 +154,7 @@ func TestNewExpandConverterHostPort(t *testing.T) {
 	for _, tt := range testCases {
 		t.Run(tt.name, func(t *testing.T) {
 			conf := confmap.NewFromStringMap(tt.input)
-			require.NoError(t, New().Convert(context.Background(), conf))
+			require.NoError(t, New(confmap.ConverterSettings{}).Convert(context.Background(), conf))
 			assert.Equal(t, tt.expected, conf.ToStringMap())
 		})
 	}

confmap/provider.go

@@ -8,6 +8,10 @@ import (
 	"fmt"
 )
 
+// ProviderSettings are the settings to initialize a Provider.
+// Any Provider should take this as a parameter in its constructor.
+type ProviderSettings struct{}
+
 // 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.

confmap/provider/envprovider/provider.go

@@ -17,12 +17,21 @@ const schemeName = "env"
 
 type provider struct{}
 
+// 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 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 &provider{}
+	return NewWithSettings(confmap.ProviderSettings{})
 }
 
 func (emp *provider) Retrieve(_ context.Context, uri string, _ confmap.WatcherFunc) (*confmap.Retrieved, error) {

confmap/provider/fileprovider/provider.go

@@ -33,10 +33,30 @@ type provider struct{}
 // `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 New() confmap.Provider {
+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)

confmap/provider/httpprovider/provider.go

@@ -8,11 +8,21 @@ 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
+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 configurablehttpprovider.New(configurablehttpprovider.HTTPScheme)
+	return NewWithSettings(confmap.ProviderSettings{})
 }

confmap/provider/httpsprovider/provider.go

@@ -14,6 +14,17 @@ import (
 //
 // 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 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 configurablehttpprovider.New(configurablehttpprovider.HTTPSScheme)
+	return NewWithSettings(confmap.ProviderSettings{})
 }

confmap/provider/internal/configurablehttpprovider/provider.go

@@ -38,7 +38,7 @@ type provider struct {
 // One example for http-uri: http://localhost:3333/getConfig
 // One example for https-uri: https://localhost:3333/getConfig
 // This is used by the http and https external implementations.
-func New(scheme SchemeType) confmap.Provider {
+func New(scheme SchemeType, _ confmap.ProviderSettings) confmap.Provider {
 	return &provider{scheme: scheme}
 }
 

confmap/provider/internal/configurablehttpprovider/provider_test.go

@@ -23,11 +23,12 @@ import (
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
 
+	"go.opentelemetry.io/collector/confmap"
 	"go.opentelemetry.io/collector/confmap/confmaptest"
 )
 
-func newConfigurableHTTPProvider(scheme SchemeType) *provider {
-	return New(scheme).(*provider)
+func newConfigurableHTTPProvider(scheme SchemeType, set confmap.ProviderSettings) *provider {
+	return New(scheme, set).(*provider)
 }
 
 func answerGet(w http.ResponseWriter, _ *http.Request) {
@@ -122,7 +123,7 @@ func generateCertificate(hostname string) (cert string, key string, err error) {
 }
 
 func TestFunctionalityDownloadFileHTTP(t *testing.T) {
-	fp := newConfigurableHTTPProvider(HTTPScheme)
+	fp := newConfigurableHTTPProvider(HTTPScheme, confmap.ProviderSettings{})
 	ts := httptest.NewServer(http.HandlerFunc(answerGet))
 	defer ts.Close()
 	_, err := fp.Retrieve(context.Background(), ts.URL, nil)
@@ -210,7 +211,7 @@ func TestFunctionalityDownloadFileHTTPS(t *testing.T) {
 
 	for _, tt := range tests {
 		t.Run(tt.testName, func(t *testing.T) {
-			fp := newConfigurableHTTPProvider(HTTPSScheme)
+			fp := newConfigurableHTTPProvider(HTTPSScheme, confmap.ProviderSettings{})
 			// Parse url of the test server to get the port number.
 			tsURL, err := url.Parse(ts.URL)
 			require.NoError(t, err)
@@ -229,19 +230,19 @@ func TestFunctionalityDownloadFileHTTPS(t *testing.T) {
 }
 
 func TestUnsupportedScheme(t *testing.T) {
-	fp := New(HTTPScheme)
+	fp := New(HTTPScheme, confmap.ProviderSettings{})
 	_, err := fp.Retrieve(context.Background(), "https://...", nil)
 	assert.Error(t, err)
 	assert.NoError(t, fp.Shutdown(context.Background()))
 
-	fp = New(HTTPSScheme)
+	fp = New(HTTPSScheme, confmap.ProviderSettings{})
 	_, err = fp.Retrieve(context.Background(), "http://...", nil)
 	assert.Error(t, err)
 	assert.NoError(t, fp.Shutdown(context.Background()))
 }
 
 func TestEmptyURI(t *testing.T) {
-	fp := New(HTTPScheme)
+	fp := New(HTTPScheme, confmap.ProviderSettings{})
 	ts := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
 		w.WriteHeader(http.StatusBadRequest)
 	}))
@@ -252,7 +253,7 @@ func TestEmptyURI(t *testing.T) {
 }
 
 func TestRetrieveFromShutdownServer(t *testing.T) {
-	fp := New(HTTPScheme)
+	fp := New(HTTPScheme, confmap.ProviderSettings{})
 	ts := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {}))
 	ts.Close()
 	_, err := fp.Retrieve(context.Background(), ts.URL, nil)
@@ -261,7 +262,7 @@ func TestRetrieveFromShutdownServer(t *testing.T) {
 }
 
 func TestNonExistent(t *testing.T) {
-	fp := New(HTTPScheme)
+	fp := New(HTTPScheme, confmap.ProviderSettings{})
 	ts := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
 		w.WriteHeader(http.StatusNotFound)
 	}))
@@ -272,7 +273,7 @@ func TestNonExistent(t *testing.T) {
 }
 
 func TestInvalidYAML(t *testing.T) {
-	fp := New(HTTPScheme)
+	fp := New(HTTPScheme, confmap.ProviderSettings{})
 	ts := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
 		w.WriteHeader(http.StatusOK)
 		_, err := w.Write([]byte("wrong : ["))
@@ -287,17 +288,17 @@ func TestInvalidYAML(t *testing.T) {
 }
 
 func TestScheme(t *testing.T) {
-	fp := New(HTTPScheme)
+	fp := New(HTTPScheme, confmap.ProviderSettings{})
 	assert.Equal(t, "http", fp.Scheme())
 	require.NoError(t, fp.Shutdown(context.Background()))
 }
 
 func TestValidateProviderScheme(t *testing.T) {
-	assert.NoError(t, confmaptest.ValidateProviderScheme(New(HTTPScheme)))
+	assert.NoError(t, confmaptest.ValidateProviderScheme(New(HTTPScheme, confmap.ProviderSettings{})))
 }
 
 func TestInvalidTransport(t *testing.T) {
-	fp := New("foo")
+	fp := New("foo", confmap.ProviderSettings{})
 
 	_, err := fp.Retrieve(context.Background(), "foo://..", nil)
 	assert.Error(t, err)

confmap/provider/yamlprovider/provider.go

@@ -16,6 +16,19 @@ 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`
+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:
@@ -25,8 +38,9 @@ type provider struct{}
 // Examples:
 // `yaml:processors::batch::timeout: 2s`
 // `yaml:processors::batch/foo::timeout: 3s`
+// Deprecated: [v0.94.0] Use NewWithSettings instead.
 func New() confmap.Provider {
-	return &provider{}
+	return NewWithSettings(confmap.ProviderSettings{})
 }
 
 func (s *provider) Retrieve(_ context.Context, uri string, _ confmap.WatcherFunc) (*confmap.Retrieved, error) {

otelcol/collector_test.go

@@ -436,7 +436,7 @@ func TestPassConfmapToServiceFailure(t *testing.T) {
 		ResolverSettings: confmap.ResolverSettings{
 			URIs:       []string{filepath.Join("testdata", "otelcol-invalid.yaml")},
 			Providers:  makeMapProvidersMap(newFailureProvider()),
-			Converters: []confmap.Converter{expandconverter.New()},
+			Converters: []confmap.Converter{expandconverter.New(confmap.ConverterSettings{})},
 		},
 	})
 	require.NoError(t, err)

otelcol/command_test.go

@@ -52,8 +52,8 @@ func TestNewCommandInvalidComponent(t *testing.T) {
 		ConfigProviderSettings{
 			ResolverSettings: confmap.ResolverSettings{
 				URIs:       []string{filepath.Join("testdata", "otelcol-invalid.yaml")},
-				Providers:  map[string]confmap.Provider{"file": fileprovider.New()},
-				Converters: []confmap.Converter{expandconverter.New()},
+				Providers:  map[string]confmap.Provider{"file": fileprovider.NewWithSettings(confmap.ProviderSettings{})},
+				Converters: []confmap.Converter{expandconverter.New(confmap.ConverterSettings{})},
 			},
 		})
 	require.NoError(t, err)

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")},
-				Providers:  map[string]confmap.Provider{"file": fileprovider.New()},
-				Converters: []confmap.Converter{expandconverter.New()},
+				Providers:  map[string]confmap.Provider{"file": fileprovider.NewWithSettings(confmap.ProviderSettings{})},
+				Converters: []confmap.Converter{expandconverter.New(confmap.ConverterSettings{})},
 			},
 		})
 	require.NoError(t, err)