BreakingChange: Change to use interface instead of func for MapConverter

CHANGELOG.md

@@ -8,6 +8,7 @@
 - Remove deprecated pdata funcs/structs from v0.50.0 (#5345)
 - Remove derecated featuregate funcs/structs from v0.50.0 (#5346)
 - Remove access to deprecated members of the config.Retrieved struct (#5363)
+- Replace usage of `config.MapConverterFunc` with `config.MapConverter` (#5382)
 
 ### 🚩 Deprecations 🚩
 

config/README.md

@@ -18,12 +18,12 @@ characters long to avoid conflicting with a driver-letter identifier as specifie
 
 ## MapConverter
 
-The [MapConverter](configmap.go) allows implementing conversion logic for the provided configuration. One of the most
+The [MapConverter](mapconverter.go) allows implementing conversion logic for the provided configuration. One of the most
 common use-case is to migrate/transform the configuration after a backwards incompatible change.
 
 ## MapResolver
 
-The `MapResolver` handles the use of multiple [MapProviders](./mapprovider.go) and [MapConverters](./configmap.go)
+The `MapResolver` handles the use of multiple [MapProviders](#mapprovider) and [MapConverters](#mapconverter)
 simplifying configuration parsing, monitoring for updates, and the overall life-cycle of the used config providers.
 The `MapResolver` provides two main functionalities: [Configuration Resolving](#configuration-resolving) and
 [Watching for Updates](#watching-for-updates).

config/configmap.go

@@ -15,7 +15,6 @@
 package config // import "go.opentelemetry.io/collector/config"
 
 import (
-	"context"
 	"encoding"
 	"fmt"
 	"reflect"
@@ -31,10 +30,6 @@ const (
 	KeyDelimiter = "::"
 )
 
-// MapConverterFunc is a converter function for the config.Map that allows distributions
-// (in the future components as well) to build backwards compatible config converters.
-type MapConverterFunc func(context.Context, *Map) error
-
 // NewMap creates a new empty config.Map instance.
 func NewMap() *Map {
 	return &Map{k: koanf.New(KeyDelimiter)}

config/mapconverter.go

@@ -0,0 +1,34 @@
+// Copyright The OpenTelemetry Authors
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//       http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package config // import "go.opentelemetry.io/collector/config"
+
+import (
+	"context"
+)
+
+// MapConverter is a converter interface for the config.Map that allows distributions
+// (in the future components as well) to build backwards compatible config converters.
+type MapConverter interface {
+	// Convert applies the conversion logic to the given "cfgMap".
+	Convert(ctx context.Context, cfgMap *Map) error
+}
+
+// Deprecated: Implement MapConverter interface.
+type MapConverterFunc func(context.Context, *Map) error
+
+// Convert implements MapConverter.Convert func.
+func (f MapConverterFunc) Convert(ctx context.Context, cfgMap *Map) error {
+	return f(ctx, cfgMap)
+}

config/mapconverter/expandmapconverter/expand.go

@@ -21,16 +21,20 @@ import (
 	"go.opentelemetry.io/collector/config"
 )
 
-// New returns a config.MapConverterFunc, that expands all environment variables for a given config.Map.
+type converter struct{}
+
+// New returns a config.MapConverter, that expands all environment variables for a given config.Map.
 //
 // Notice: This API is experimental.
-func New() config.MapConverterFunc {
-	return func(_ context.Context, cfgMap *config.Map) error {
-		for _, k := range cfgMap.AllKeys() {
-			cfgMap.Set(k, expandStringValues(cfgMap.Get(k)))
-		}
-		return nil
+func New() config.MapConverter {
+	return converter{}
+}
+
+func (converter) Convert(_ context.Context, cfgMap *config.Map) error {
+	for _, k := range cfgMap.AllKeys() {
+		cfgMap.Set(k, expandStringValues(cfgMap.Get(k)))
 	}
+	return nil
 }
 
 func expandStringValues(value interface{}) interface{} {

config/mapconverter/expandmapconverter/expand_test.go

@@ -56,7 +56,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()(context.Background(), cfgMap))
+			require.NoError(t, New().Convert(context.Background(), cfgMap))
 			assert.Equal(t, expectedCfgMap.ToStringMap(), cfgMap.ToStringMap())
 		})
 	}
@@ -75,7 +75,7 @@ func TestNewExpandConverter_EscapedMaps(t *testing.T) {
 				"recv": "$MAP_VALUE",
 			}},
 	)
-	require.NoError(t, New()(context.Background(), cfgMap))
+	require.NoError(t, New().Convert(context.Background(), cfgMap))
 
 	expectedMap := map[string]interface{}{
 		"test_string_map": map[string]interface{}{
@@ -112,6 +112,6 @@ func TestNewExpandConverter_EscapedEnvVars(t *testing.T) {
 			// escaped $ alone
 			"recv.7": "$",
 		}}
-	require.NoError(t, New()(context.Background(), cfgMap))
+	require.NoError(t, New().Convert(context.Background(), cfgMap))
 	assert.Equal(t, expectedMap, cfgMap.ToStringMap())
 }

config/mapconverter/overwritepropertiesmapconverter/properties.go

@@ -25,25 +25,27 @@ import (
 	"go.opentelemetry.io/collector/config"
 )
 
-// New returns a config.MapConverterFunc, that overrides all the given properties into the
+type converter struct {
+	properties []string
+}
+
+// New returns a config.MapConverter, that overrides all the given properties into the
 // input map.
 //
 // Properties must follow the Java properties format, key-value list separated by equal sign with a "."
 // as key delimiter.
 //  ["processors.batch.timeout=2s", "processors.batch/foo.timeout=3s"]
-func New(properties []string) config.MapConverterFunc {
-	return func(_ context.Context, cfgMap *config.Map) error {
-		return convert(properties, cfgMap)
-	}
+func New(properties []string) config.MapConverter {
+	return &converter{properties: properties}
 }
 
-func convert(propsStr []string, cfgMap *config.Map) error {
-	if len(propsStr) == 0 {
+func (c *converter) Convert(_ context.Context, cfgMap *config.Map) error {
+	if len(c.properties) == 0 {
 		return nil
 	}
 
 	b := &bytes.Buffer{}
-	for _, property := range propsStr {
+	for _, property := range c.properties {
 		property = strings.TrimSpace(property)
 		b.WriteString(property)
 		b.WriteString("\n")

config/mapconverter/overwritepropertiesmapconverter/properties_test.go

@@ -27,7 +27,7 @@ import (
 func TestOverwritePropertiesConverter_Empty(t *testing.T) {
 	pmp := New(nil)
 	cfgMap := config.NewMapFromStringMap(map[string]interface{}{"foo": "bar"})
-	assert.NoError(t, pmp(context.Background(), cfgMap))
+	assert.NoError(t, pmp.Convert(context.Background(), cfgMap))
 	assert.Equal(t, map[string]interface{}{"foo": "bar"}, cfgMap.ToStringMap())
 }
 
@@ -41,7 +41,7 @@ func TestOverwritePropertiesConverter(t *testing.T) {
 
 	pmp := New(props)
 	cfgMap := config.NewMap()
-	require.NoError(t, pmp(context.Background(), cfgMap))
+	require.NoError(t, pmp.Convert(context.Background(), cfgMap))
 	keys := cfgMap.AllKeys()
 	assert.Len(t, keys, 4)
 	assert.Equal(t, "2s", cfgMap.Get("processors::batch::timeout"))
@@ -53,5 +53,5 @@ func TestOverwritePropertiesConverter(t *testing.T) {
 func TestOverwritePropertiesConverter_InvalidProperty(t *testing.T) {
 	pmp := New([]string{"=2s"})
 	cfgMap := config.NewMap()
-	assert.Error(t, pmp(context.Background(), cfgMap))
+	assert.Error(t, pmp.Convert(context.Background(), cfgMap))
 }

service/collector_test.go

@@ -194,7 +194,7 @@ func testCollectorStartHelper(t *testing.T, telemetry collectorTelemetryExporter
 	cfgSet := newDefaultConfigProviderSettings([]string{
 		filepath.Join("testdata", "otelcol-config.yaml"),
 	})
-	cfgSet.MapConverters = append([]config.MapConverterFunc{
+	cfgSet.MapConverters = append([]config.MapConverter{
 		overwritepropertiesmapconverter.New(
 			[]string{"service.telemetry.metrics.address=" + metricsAddr},
 		)},

service/collector_windows.go

@@ -144,7 +144,7 @@ func newWithWindowsEventLogCore(set CollectorSettings, elog *eventlog.Log) (*Col
 		cfgSet := newDefaultConfigProviderSettings(getConfigFlag())
 		// Append the "overwrite properties converter" as the first converter.
 		cfgSet.MapConverters = append(
-			[]config.MapConverterFunc{overwritepropertiesmapconverter.New(getSetFlag())},
+			[]config.MapConverter{overwritepropertiesmapconverter.New(getSetFlag())},
 			cfgSet.MapConverters...)
 		set.ConfigProvider, err = NewConfigProvider(cfgSet)
 		if err != nil {

service/command.go

@@ -35,7 +35,7 @@ func NewCommand(set CollectorSettings) *cobra.Command {
 				cfgSet := newDefaultConfigProviderSettings(getConfigFlag())
 				// Append the "overwrite properties converter" as the first converter.
 				cfgSet.MapConverters = append(
-					[]config.MapConverterFunc{overwritepropertiesmapconverter.New(getSetFlag())},
+					[]config.MapConverter{overwritepropertiesmapconverter.New(getSetFlag())},
 					cfgSet.MapConverters...)
 				set.ConfigProvider, err = NewConfigProvider(cfgSet)
 				if err != nil {

service/config_provider.go

@@ -75,22 +75,22 @@ type ConfigProviderSettings struct {
 	// It is required to have at least one config.MapProvider.
 	MapProviders map[string]config.MapProvider
 
-	// MapConverters is a slice of config.MapConverterFunc.
-	MapConverters []config.MapConverterFunc
+	// MapConverters is a slice of config.MapConverter.
+	MapConverters []config.MapConverter
 }
 
 func newDefaultConfigProviderSettings(locations []string) ConfigProviderSettings {
 	return ConfigProviderSettings{
 		Locations:     locations,
 		MapProviders:  makeMapProvidersMap(filemapprovider.New(), envmapprovider.New(), yamlmapprovider.New()),
-		MapConverters: []config.MapConverterFunc{expandmapconverter.New()},
+		MapConverters: []config.MapConverter{expandmapconverter.New()},
 	}
 }
 
 // NewConfigProvider returns a new ConfigProvider that provides the service configuration:
 // * Initially it resolves the "configuration map":
 //	 * Retrieve the config.Map by merging all retrieved maps from the given `locations` in order.
-// 	 * Then applies all the config.MapConverterFunc in the given order.
+// 	 * Then applies all the config.MapConverter in the given order.
 // * Then unmarshalls the config.Map into the service Config.
 func NewConfigProvider(set ConfigProviderSettings) (ConfigProvider, error) {
 	mr, err := newMapResolver(set.Locations, set.MapProviders, set.MapConverters)

service/mapresolver.go

@@ -36,7 +36,7 @@ var driverLetterRegexp = regexp.MustCompile("^[A-z]:")
 type mapResolver struct {
 	uris          []string
 	mapProviders  map[string]config.MapProvider
-	mapConverters []config.MapConverterFunc
+	mapConverters []config.MapConverter
 
 	sync.Mutex
 	closers []config.CloseFunc
@@ -62,7 +62,7 @@ type mapResolver struct {
 //
 // `uri` must follow the "<scheme>:<opaque_data>" format. This format is compatible with the URI definition
 // (see https://datatracker.ietf.org/doc/html/rfc3986). An empty "<scheme>" defaults to "file" schema.
-func newMapResolver(uris []string, mapProviders map[string]config.MapProvider, mapConverters []config.MapConverterFunc) (*mapResolver, error) {
+func newMapResolver(uris []string, mapProviders map[string]config.MapProvider, mapConverters []config.MapConverter) (*mapResolver, error) {
 	if len(uris) == 0 {
 		return nil, errors.New("invalid map resolver config: no URIs")
 	}
@@ -78,7 +78,7 @@ func newMapResolver(uris []string, mapProviders map[string]config.MapProvider, m
 	for k, v := range mapProviders {
 		mapProvidersCopy[k] = v
 	}
-	mapConvertersCopy := make([]config.MapConverterFunc, len(mapConverters))
+	mapConvertersCopy := make([]config.MapConverter, len(mapConverters))
 	copy(mapConvertersCopy, mapConverters)
 
 	return &mapResolver{
@@ -130,7 +130,7 @@ func (mr *mapResolver) Resolve(ctx context.Context) (*config.Map, error) {
 
 	// Apply the converters in the given order.
 	for _, cfgMapConv := range mr.mapConverters {
-		if err := cfgMapConv(ctx, retMap); err != nil {
+		if err := cfgMapConv.Convert(ctx, retMap); err != nil {
 			return nil, fmt.Errorf("cannot convert the config.Map: %w", err)
 		}
 	}

service/mapresolver_test.go

@@ -65,12 +65,20 @@ func (m *mockProvider) Shutdown(context.Context) error {
 	return m.errS
 }
 
+type mockConverter struct {
+	err error
+}
+
+func (m *mockConverter) Convert(context.Context, *config.Map) error {
+	return errors.New("converter_err")
+}
+
 func TestMapResolver_Errors(t *testing.T) {
 	tests := []struct {
 		name              string
 		locations         []string
 		mapProviders      []config.MapProvider
-		mapConverters     []config.MapConverterFunc
+		mapConverters     []config.MapConverter
 		expectResolveErr  bool
 		expectWatchErr    bool
 		expectCloseErr    bool
@@ -95,7 +103,7 @@ func TestMapResolver_Errors(t *testing.T) {
 			name:             "converter error",
 			locations:        []string{"mock:", filepath.Join("testdata", "otelcol-nop.yaml")},
 			mapProviders:     []config.MapProvider{&mockProvider{}, filemapprovider.New()},
-			mapConverters:    []config.MapConverterFunc{func(context.Context, *config.Map) error { return errors.New("converter_err") }},
+			mapConverters:    []config.MapConverter{&mockConverter{err: errors.New("converter_err")}},
 			expectResolveErr: true,
 		},
 		{