Move service.ConfigMapConverterFunc to config.MapConverterFunc (#4673)

The drop of the Config prefix from the func name was forced by go lint, which does not like to repeat the package name as the prefix for types. Signed-off-by: Bogdan Drutu <[email protected]>
open-telemetry · Jan 13, 2022 · 0bb3c8c · 0bb3c8c
1 parent f5d646f
commit 0bb3c8c
Show file tree

Hide file tree

Showing 6 changed files with 13 additions and 14 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -8,6 +8,7 @@
 - Change Properties Provider to be a Converter (#4666)
 - Define a type `WatcherFunc` for onChange func instead of func pointer (#4656)
 - Remove deprecated `configtest.LoadConfig` and `configtest.LoadConfigAndValidate` (#4659)
+- Move service.ConfigMapConverterFunc to config.MapConverterFunc (#4673)
 
 ## 💡 Enhancements 💡
 

diff --git a/config/configmap.go b/config/configmap.go
@@ -30,6 +30,11 @@ 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.
+// TODO: Consider to add a context as the first argument.
+type MapConverterFunc func(*Map) error
+
 // NewMap creates a new empty config.Map instance.
 func NewMap() *Map {
 	return &Map{k: koanf.New(KeyDelimiter)}

diff --git a/config/configmapprovider/properties.go b/config/configmapprovider/properties.go
@@ -30,8 +30,7 @@ import (
 // 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"]
-// TODO: Find the right place for service.ConfigMapConverterFunc and return that. Currently this will cause circular dependency.
-func NewOverwritePropertiesConverter(properties []string) func(*config.Map) error {
+func NewOverwritePropertiesConverter(properties []string) config.MapConverterFunc {
 	return func(cfgMap *config.Map) error {
 		return convert(properties, cfgMap)
 	}

diff --git a/service/config_provider.go b/service/config_provider.go
@@ -67,7 +67,7 @@ type ConfigProvider interface {
 type configProvider struct {
 	locations          []string
 	configMapProviders map[string]configmapprovider.Provider
-	cfgMapConverters   []ConfigMapConverterFunc
+	cfgMapConverters   []config.MapConverterFunc
 	configUnmarshaler  configunmarshaler.ConfigUnmarshaler
 
 	sync.Mutex
@@ -86,7 +86,7 @@ type configProvider struct {
 func NewConfigProvider(
 	locations []string,
 	configMapProviders map[string]configmapprovider.Provider,
-	cfgMapConverters []ConfigMapConverterFunc,
+	cfgMapConverters []config.MapConverterFunc,
 	configUnmarshaler configunmarshaler.ConfigUnmarshaler) ConfigProvider {
 	return &configProvider{
 		locations:          locations,
@@ -97,10 +97,6 @@ func NewConfigProvider(
 	}
 }
 
-// ConfigMapConverterFunc is a converter function for the config.Map that allows distributions
-// (in the future components as well) to build backwards compatible config converters.
-type ConfigMapConverterFunc func(*config.Map) error
-
 // NewDefaultConfigProvider returns the default ConfigProvider, and it creates configuration from a file
 // defined by the given configFile and overwrites fields using properties.
 func NewDefaultConfigProvider(configFileName string, properties []string) ConfigProvider {
@@ -110,7 +106,7 @@ func NewDefaultConfigProvider(configFileName string, properties []string) Config
 			"file": configmapprovider.NewFile(),
 			"env":  configmapprovider.NewEnv(),
 		},
-		[]ConfigMapConverterFunc{
+		[]config.MapConverterFunc{
 			configmapprovider.NewOverwritePropertiesConverter(properties),
 			configprovider.NewExpandConverter(),
 		},

diff --git a/service/config_provider_test.go b/service/config_provider_test.go
@@ -100,7 +100,7 @@ func TestConfigProvider_Errors(t *testing.T) {
 		name              string
 		locations         []string
 		parserProvider    map[string]configmapprovider.Provider
-		cfgMapConverters  []ConfigMapConverterFunc
+		cfgMapConverters  []config.MapConverterFunc
 		configUnmarshaler configunmarshaler.ConfigUnmarshaler
 		expectNewErr      bool
 		expectWatchErr    bool
@@ -142,7 +142,7 @@ func TestConfigProvider_Errors(t *testing.T) {
 				"mock": &mockProvider{},
 				"file": configmapprovider.NewFile(),
 			},
-			cfgMapConverters:  []ConfigMapConverterFunc{func(c *config.Map) error { return errors.New("converter_err") }},
+			cfgMapConverters:  []config.MapConverterFunc{func(c *config.Map) error { return errors.New("converter_err") }},
 			configUnmarshaler: configunmarshaler.NewDefault(),
 			expectNewErr:      true,
 		},

diff --git a/service/internal/configprovider/expand.go b/service/internal/configprovider/expand.go
@@ -21,9 +21,7 @@ import (
 )
 
 // NewExpandConverter returns a service.ConfigMapConverterFunc, that expands all environment variables for a given config.Map.
-//
-// This does not directly return service.ConfigMapConverterFunc to avoid circular dependencies, not a problem since it is internal.
-func NewExpandConverter() func(*config.Map) error {
+func NewExpandConverter() config.MapConverterFunc {
 	return func(cfgMap *config.Map) error {
 		for _, k := range cfgMap.AllKeys() {
 			cfgMap.Set(k, expandStringValues(cfgMap.Get(k)))