Revert to first deprecate things before removing, MoveTo no destination

Signed-off-by: Bogdan Drutu <bogdandrutu@gmail.com>

CHANGELOG.md

@@ -11,6 +11,7 @@
 
 - Deprecate `configunmarshaler` package, move it to internal (#5151)
 - Deprecate all API in `model/semconv`. The package is moved to a new `semcomv` module (#5196)
+- Deprecate access to `config.Retrieved` fields, use the newly added funcs to interact with the internal fields (#5198)
 
 ### 💡 Enhancements 💡
 

config/configtest/configtest.go

@@ -37,10 +37,7 @@ func LoadConfigMap(fileName string) (*config.Map, error) {
 		return nil, err
 	}
 	retMap := config.NewMap()
-	if err = ret.MergeTo(retMap, ""); err != nil {
-		return nil, err
-	}
-	return retMap, err
+	return retMap, ret.MergeTo(retMap)
 }
 
 // CheckConfigStruct enforces that given configuration object is following the patterns

config/configtest/configtest_test.go

@@ -16,13 +16,25 @@ package configtest
 
 import (
 	"io"
+	"path/filepath"
 	"strings"
 	"testing"
 
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
 )
 
+func TestLoadConfigMap_FileNotFound(t *testing.T) {
+	_, err := LoadConfigMap("file/not/found")
+	assert.Error(t, err)
+}
+
+func TestLoadConfigMap(t *testing.T) {
+	cfg, err := LoadConfigMap(filepath.Join("testdata", "simple.yaml"))
+	require.NoError(t, err)
+	assert.Equal(t, map[string]interface{}{"floating": 3.14}, cfg.ToStringMap())
+}
+
 func TestCheckConfigStructPointerAndValue(t *testing.T) {
 	config := struct {
 		SomeFiled string `mapstructure:"test"`

config/configtest/testdata/simple.yaml

@@ -0,0 +1 @@
+floating:  3.14

config/mapprovider.go

@@ -80,14 +80,10 @@ type ChangeEvent struct {
 }
 
 // Retrieved holds the result of a call to the Retrieve method of a Provider object.
-type Retrieved interface {
-	MergeTo(dest *Map, destPath string) error
-	Close(ctx context.Context) error
-}
-
-// mapRetrieved holds the result of a call to the Retrieve method of a Provider object.
-type mapRetrieved struct {
-	cfgMap *Map
+type Retrieved struct {
+	// Deprecated: Use NewRetrievedFromMap to initialize, and Retrieved.MergeTo to access.
+	*Map
+	// Deprecated: Use NewRetrievedFromMap to initialize, and Retrieved.Close to access.
 	CloseFunc
 }
 
@@ -97,24 +93,27 @@ type mapRetrieved struct {
 //   used to retrieve values is no longer in use and should close and release any watchers
 //	 that it may have created.
 func NewRetrievedFromMap(cfgMap *Map, close CloseFunc) Retrieved {
-	return &mapRetrieved{cfgMap: cfgMap, CloseFunc: close}
+	return Retrieved{Map: cfgMap, CloseFunc: close}
 }
 
-func (r *mapRetrieved) MergeTo(dest *Map, destPath string) error {
-	return dest.k.MergeAt(r.cfgMap.k, destPath)
+// MergeTo merges the retrieved configuration into the given Map.
+// Note that the retrieved configuration must not be modified.
+func (r Retrieved) MergeTo(dest *Map) error {
+	return dest.Merge(r.Map)
 }
 
-// CloseFunc a function to close and release any watchers that it may have created.
+// Close and release any watchers that MapProvider.Retrieve may have created.
 //
 // Should block until all resources are closed, and guarantee that `onChange` is not
 // going to be called after it returns except when `ctx` is cancelled.
 //
 // Should never be called concurrently with itself.
-type CloseFunc func(context.Context) error
-
-func (f CloseFunc) Close(ctx context.Context) error {
-	if f == nil {
+func (r Retrieved) Close(ctx context.Context) error {
+	if r.CloseFunc == nil {
 		return nil
 	}
-	return f(ctx)
+	return r.CloseFunc(ctx)
 }
+
+// CloseFunc a function equivalent to Retrieved.Close.
+type CloseFunc func(context.Context) error

config/mapprovider/envmapprovider/mapprovider.go

@@ -39,13 +39,13 @@ func New() config.MapProvider {
 
 func (emp *mapProvider) Retrieve(_ context.Context, uri string, _ config.WatcherFunc) (config.Retrieved, error) {
 	if !strings.HasPrefix(uri, schemeName+":") {
-		return nil, fmt.Errorf("%v uri is not supported by %v provider", uri, schemeName)
+		return config.Retrieved{}, fmt.Errorf("%v uri is not supported by %v provider", uri, schemeName)
 	}
 
 	content := os.Getenv(uri[len(schemeName)+1:])
 	var data map[string]interface{}
 	if err := yaml.Unmarshal([]byte(content), &data); err != nil {
-		return nil, fmt.Errorf("unable to parse yaml: %w", err)
+		return config.Retrieved{}, fmt.Errorf("unable to parse yaml: %w", err)
 	}
 
 	return config.NewRetrievedFromMap(config.NewMapFromStringMap(data), nil), nil

config/mapprovider/envmapprovider/mapprovider_test.go

@@ -63,7 +63,7 @@ func TestEnv(t *testing.T) {
 	ret, err := env.Retrieve(context.Background(), envSchemePrefix+envName, nil)
 	require.NoError(t, err)
 	retMap := config.NewMap()
-	assert.NoError(t, ret.MergeTo(retMap, ""))
+	assert.NoError(t, ret.MergeTo(retMap))
 	expectedMap := config.NewMapFromStringMap(map[string]interface{}{
 		"processors::batch":         nil,
 		"exporters::otlp::endpoint": "localhost:4317",

config/mapprovider/filemapprovider/mapprovider.go

@@ -49,18 +49,18 @@ func New() config.MapProvider {
 
 func (fmp *mapProvider) Retrieve(_ context.Context, uri string, _ config.WatcherFunc) (config.Retrieved, error) {
 	if !strings.HasPrefix(uri, schemeName+":") {
-		return nil, fmt.Errorf("%v uri is not supported by %v provider", uri, schemeName)
+		return config.Retrieved{}, fmt.Errorf("%v uri is not supported by %v provider", uri, schemeName)
 	}
 
 	// Clean the path before using it.
 	content, err := ioutil.ReadFile(filepath.Clean(uri[len(schemeName)+1:]))
 	if err != nil {
-		return nil, fmt.Errorf("unable to read the file %v: %w", uri, err)
+		return config.Retrieved{}, fmt.Errorf("unable to read the file %v: %w", uri, err)
 	}
 
 	var data map[string]interface{}
 	if err = yaml.Unmarshal(content, &data); err != nil {
-		return nil, fmt.Errorf("unable to parse yaml: %w", err)
+		return config.Retrieved{}, fmt.Errorf("unable to parse yaml: %w", err)
 	}
 
 	return config.NewRetrievedFromMap(config.NewMapFromStringMap(data), nil), nil

config/mapprovider/filemapprovider/mapprovider_test.go

@@ -65,7 +65,7 @@ func TestRelativePath(t *testing.T) {
 	ret, err := fp.Retrieve(context.Background(), fileSchemePrefix+filepath.Join("testdata", "default-config.yaml"), nil)
 	require.NoError(t, err)
 	retMap := config.NewMap()
-	assert.NoError(t, ret.MergeTo(retMap, ""))
+	assert.NoError(t, ret.MergeTo(retMap))
 	expectedMap := config.NewMapFromStringMap(map[string]interface{}{
 		"processors::batch":         nil,
 		"exporters::otlp::endpoint": "localhost:4317",
@@ -79,7 +79,7 @@ func TestAbsolutePath(t *testing.T) {
 	ret, err := fp.Retrieve(context.Background(), fileSchemePrefix+absolutePath(t, filepath.Join("testdata", "default-config.yaml")), nil)
 	require.NoError(t, err)
 	retMap := config.NewMap()
-	assert.NoError(t, ret.MergeTo(retMap, ""))
+	assert.NoError(t, ret.MergeTo(retMap))
 	expectedMap := config.NewMapFromStringMap(map[string]interface{}{
 		"processors::batch":         nil,
 		"exporters::otlp::endpoint": "localhost:4317",

config/mapprovider/yamlmapprovider/mapprovider.go

@@ -42,12 +42,12 @@ func New() config.MapProvider {
 
 func (s *mapProvider) Retrieve(_ context.Context, uri string, _ config.WatcherFunc) (config.Retrieved, error) {
 	if !strings.HasPrefix(uri, schemeName+":") {
-		return nil, fmt.Errorf("%v uri is not supported by %v provider", uri, schemeName)
+		return config.Retrieved{}, fmt.Errorf("%v uri is not supported by %v provider", uri, schemeName)
 	}
 
 	var data map[string]interface{}
 	if err := yaml.Unmarshal([]byte(uri[len(schemeName)+1:]), &data); err != nil {
-		return nil, fmt.Errorf("unable to parse yaml: %w", err)
+		return config.Retrieved{}, fmt.Errorf("unable to parse yaml: %w", err)
 	}
 
 	return config.NewRetrievedFromMap(config.NewMapFromStringMap(data), nil), nil

config/mapprovider/yamlmapprovider/mapprovider_test.go

@@ -42,7 +42,7 @@ func TestOneValue(t *testing.T) {
 	ret, err := sp.Retrieve(context.Background(), "yaml:processors::batch::timeout: 2s", nil)
 	assert.NoError(t, err)
 	retMap := config.NewMap()
-	assert.NoError(t, ret.MergeTo(retMap, ""))
+	assert.NoError(t, ret.MergeTo(retMap))
 	assert.Equal(t, map[string]interface{}{
 		"processors": map[string]interface{}{
 			"batch": map[string]interface{}{
@@ -58,7 +58,7 @@ func TestNamedComponent(t *testing.T) {
 	ret, err := sp.Retrieve(context.Background(), "yaml:processors::batch/foo::timeout: 3s", nil)
 	assert.NoError(t, err)
 	retMap := config.NewMap()
-	assert.NoError(t, ret.MergeTo(retMap, ""))
+	assert.NoError(t, ret.MergeTo(retMap))
 	assert.Equal(t, map[string]interface{}{
 		"processors": map[string]interface{}{
 			"batch/foo": map[string]interface{}{
@@ -74,7 +74,7 @@ func TestMapEntry(t *testing.T) {
 	ret, err := sp.Retrieve(context.Background(), "yaml:processors: {batch/foo::timeout: 3s, batch::timeout: 2s}", nil)
 	assert.NoError(t, err)
 	retMap := config.NewMap()
-	assert.NoError(t, ret.MergeTo(retMap, ""))
+	assert.NoError(t, ret.MergeTo(retMap))
 	assert.Equal(t, map[string]interface{}{
 		"processors": map[string]interface{}{
 			"batch/foo": map[string]interface{}{
@@ -93,7 +93,7 @@ func TestNewLine(t *testing.T) {
 	ret, err := sp.Retrieve(context.Background(), "yaml:processors::batch/foo::timeout: 3s\nprocessors::batch::timeout: 2s", nil)
 	assert.NoError(t, err)
 	retMap := config.NewMap()
-	assert.NoError(t, ret.MergeTo(retMap, ""))
+	assert.NoError(t, ret.MergeTo(retMap))
 	assert.Equal(t, map[string]interface{}{
 		"processors": map[string]interface{}{
 			"batch/foo": map[string]interface{}{
@@ -112,7 +112,7 @@ func TestDotSeparator(t *testing.T) {
 	ret, err := sp.Retrieve(context.Background(), "yaml:processors.batch.timeout: 4s", nil)
 	assert.NoError(t, err)
 	retMap := config.NewMap()
-	assert.NoError(t, ret.MergeTo(retMap, ""))
+	assert.NoError(t, ret.MergeTo(retMap))
 	assert.Equal(t, map[string]interface{}{"processors.batch.timeout": "4s"}, retMap.ToStringMap())
 	assert.NoError(t, sp.Shutdown(context.Background()))
 }

service/config_provider.go

@@ -225,7 +225,7 @@ func (cm *configProvider) mergeRetrieve(ctx context.Context) (*config.Map, confi
 		if err != nil {
 			return nil, nil, err
 		}
-		if err = ret.MergeTo(retCfgMap, ""); err != nil {
+		if err = ret.MergeTo(retCfgMap); err != nil {
 			return nil, nil, err
 		}
 		closers = append(closers, ret.Close)

service/config_provider_test.go

@@ -44,7 +44,7 @@ type mockProvider struct {
 
 func (m *mockProvider) Retrieve(_ context.Context, _ string, watcher config.WatcherFunc) (config.Retrieved, error) {
 	if m.errR != nil {
-		return nil, m.errR
+		return config.Retrieved{}, m.errR
 	}
 	if m.retM == nil {
 		return config.NewRetrievedFromMap(config.NewMap(), nil), nil