Return an error in service.NewConfigProvider if there is no configLocations (v0.45)

CHANGELOG.md

@@ -44,6 +44,7 @@
 
 - Deprecate `pdata.NumberDataPoint.Type()` and `pdata.Exemplar.Type()` in favor of `NumberDataPoint.ValueType()` and
   `Exemplar.ValueType()` (#4850)
+- Deprecate `service.MustNewConfigProvider` and `service.MustNewDefaultConfigProvider`in favor of `service.NewConfigProvider` (#4762)
 
 ## v0.44.0 Beta
 

service/collector_test.go

@@ -36,6 +36,8 @@ import (
 	"go.uber.org/zap/zapcore"
 
 	"go.opentelemetry.io/collector/component"
+	"go.opentelemetry.io/collector/config"
+	"go.opentelemetry.io/collector/config/configmapprovider"
 	"go.opentelemetry.io/collector/internal/testcomponents"
 	"go.opentelemetry.io/collector/internal/testutil"
 )
@@ -51,10 +53,13 @@ func TestCollector_StartAsGoRoutine(t *testing.T) {
 	factories, err := testcomponents.NewDefaultFactories()
 	require.NoError(t, err)
 
+	configProvider, err := NewConfigProvider([]string{filepath.Join("testdata", "otelcol-config.yaml")})
+	require.NoError(t, err)
+
 	set := CollectorSettings{
 		BuildInfo:      component.NewDefaultBuildInfo(),
 		Factories:      factories,
-		ConfigProvider: MustNewDefaultConfigProvider([]string{filepath.Join("testdata", "otelcol-config.yaml")}, nil),
+		ConfigProvider: configProvider,
 	}
 	col, err := New(set)
 	require.NoError(t, err)
@@ -93,12 +98,22 @@ func TestCollector_Start(t *testing.T) {
 	}
 
 	metricsPort := testutil.GetAvailablePort(t)
+
+	cfgMapConverter := []config.MapConverterFunc{
+		configmapprovider.NewOverwritePropertiesConverter(
+			[]string{"service.telemetry.metrics.address=localhost:" + strconv.FormatUint(uint64(metricsPort), 10)},
+		),
+	}
+
+	configProvider, err := NewConfigProvider([]string{filepath.Join("testdata", "otelcol-config.yaml")},
+		WithCfgMapConverters(cfgMapConverter))
+
+	require.NoError(t, err)
+
 	col, err := New(CollectorSettings{
-		BuildInfo: component.NewDefaultBuildInfo(),
-		Factories: factories,
-		ConfigProvider: MustNewDefaultConfigProvider(
-			[]string{filepath.Join("testdata", "otelcol-config.yaml")},
-			[]string{"service.telemetry.metrics.address=localhost:" + strconv.FormatUint(uint64(metricsPort), 10)}),
+		BuildInfo:      component.NewDefaultBuildInfo(),
+		Factories:      factories,
+		ConfigProvider: configProvider,
 		LoggingOptions: []zap.Option{zap.Hooks(hook)},
 	})
 	require.NoError(t, err)
@@ -172,10 +187,13 @@ func TestCollector_ReportError(t *testing.T) {
 	factories, err := testcomponents.NewDefaultFactories()
 	require.NoError(t, err)
 
+	configProvider, err := NewConfigProvider([]string{filepath.Join("testdata", "otelcol-config.yaml")})
+	require.NoError(t, err)
+
 	col, err := New(CollectorSettings{
 		BuildInfo:      component.NewDefaultBuildInfo(),
 		Factories:      factories,
-		ConfigProvider: MustNewDefaultConfigProvider([]string{filepath.Join("testdata", "otelcol-config.yaml")}, nil),
+		ConfigProvider: configProvider,
 	})
 	require.NoError(t, err)
 

service/command.go

@@ -17,6 +17,8 @@ package service // import "go.opentelemetry.io/collector/service"
 import (
 	"github.com/spf13/cobra"
 
+	"go.opentelemetry.io/collector/config"
+	"go.opentelemetry.io/collector/config/configmapprovider"
 	"go.opentelemetry.io/collector/service/featuregate"
 )
 
@@ -30,7 +32,14 @@ func NewCommand(set CollectorSettings) *cobra.Command {
 		RunE: func(cmd *cobra.Command, args []string) error {
 			featuregate.Apply(featuregate.GetFlags())
 			if set.ConfigProvider == nil {
-				set.ConfigProvider = MustNewDefaultConfigProvider(getConfigFlag(), getSetFlag())
+				cfgMapConverter := []config.MapConverterFunc{
+					configmapprovider.NewOverwritePropertiesConverter(getSetFlag()),
+				}
+				configProvider, err := NewConfigProvider(getConfigFlag(), WithCfgMapConverters(cfgMapConverter))
+				if err != nil {
+					return err
+				}
+				set.ConfigProvider = configProvider
 			}
 			col, err := New(set)
 			if err != nil {

service/config_provider.go

@@ -75,6 +75,29 @@ type configProvider struct {
 	watcher chan error
 }
 
+// ConfigProviderOption is an option to change the behavior of ConfigProvider
+// returned by NewConfigProvider()
+type ConfigProviderOption func(opts *configProvider)
+
+func WithConfigMapProviders(c map[string]configmapprovider.Provider) ConfigProviderOption {
+	return func(opts *configProvider) {
+		opts.configMapProviders = c
+	}
+}
+
+func WithCfgMapConverters(c []config.MapConverterFunc) ConfigProviderOption {
+	return func(opts *configProvider) {
+		opts.cfgMapConverters = c
+	}
+}
+
+func WithConfigUnmarshaler(c configunmarshaler.ConfigUnmarshaler) ConfigProviderOption {
+	return func(opts *configProvider) {
+		opts.configUnmarshaler = c
+	}
+}
+
+// Deprecated: use NewConfigProvider instead
 // MustNewConfigProvider returns a new ConfigProvider that provides the configuration:
 // * Retrieve the config.Map by merging all retrieved maps from all the configmapprovider.Provider in order.
 // * Then applies all the ConfigMapConverterFunc in the given order.
@@ -88,10 +111,13 @@ func MustNewConfigProvider(
 	configMapProviders map[string]configmapprovider.Provider,
 	cfgMapConverters []config.MapConverterFunc,
 	configUnmarshaler configunmarshaler.ConfigUnmarshaler) ConfigProvider {
-	return NewConfigProvider(locations, configMapProviders, cfgMapConverters, configUnmarshaler)
+	configProvider, err := NewConfigProvider(locations, WithConfigMapProviders(configMapProviders), WithCfgMapConverters(cfgMapConverters), WithConfigUnmarshaler(configUnmarshaler))
+	if err != nil {
+		panic(err)
+	}
+	return configProvider
 }
 
-// Deprecated: [v0.44.0] use MustNewConfigProvider instead
 // NewConfigProvider returns a new ConfigProvider that provides the configuration:
 // * Retrieve the config.Map by merging all retrieved maps from all the configmapprovider.Provider in order.
 // * Then applies all the config.MapConverterFunc in the given order.
@@ -100,44 +126,53 @@ func MustNewConfigProvider(
 // The `configMapProviders` is a map of pairs <scheme,Provider>.
 //
 // Notice: This API is experimental.
-func NewConfigProvider(
-	locations []string,
-	configMapProviders map[string]configmapprovider.Provider,
-	cfgMapConverters []config.MapConverterFunc,
-	configUnmarshaler configunmarshaler.ConfigUnmarshaler) ConfigProvider {
+func NewConfigProvider(locations []string, opts ...ConfigProviderOption) (ConfigProvider, error) {
+	// Set default ConfigProvider values
+	configMapProvider := map[string]configmapprovider.Provider{
+		"file": configmapprovider.NewFile(),
+		"env":  configmapprovider.NewEnv(),
+	}
+	cfgMapConverter := []config.MapConverterFunc{
+		configmapprovider.NewExpandConverter(),
+	}
+
+	if len(locations) == 0 {
+		return nil, fmt.Errorf("cannot create ConfigProvider: no locations provided")
+	}
 	// Safe copy, ensures the slice cannot be changed from the caller.
 	locationsCopy := make([]string, len(locations))
 	copy(locationsCopy, locations)
-	return &configProvider{
+
+	provider := configProvider{
 		locations:          locationsCopy,
-		configMapProviders: configMapProviders,
-		cfgMapConverters:   cfgMapConverters,
-		configUnmarshaler:  configUnmarshaler,
+		configMapProviders: configMapProvider,
+		cfgMapConverters:   cfgMapConverter,
+		configUnmarshaler:  configunmarshaler.NewDefault(),
 		watcher:            make(chan error, 1),
 	}
+
+	// Override default values with user options
+	for _, o := range opts {
+		o(&provider)
+	}
+
+	return &provider, nil
 }
 
+// Deprecated: use NewConfigProvider instead
 // MustNewDefaultConfigProvider returns the default ConfigProvider, and it creates configuration from a file
 // defined by the given configFile and overwrites fields using properties.
 func MustNewDefaultConfigProvider(configLocations []string, properties []string) ConfigProvider {
-	return NewDefaultConfigProvider(configLocations, properties)
-}
+	cfgMapConverter := []config.MapConverterFunc{
+		configmapprovider.NewOverwritePropertiesConverter(properties),
+		configmapprovider.NewExpandConverter(),
+	}
 
-// Deprecated: [v.0.44.0] use MustNewDefaultConfigProvider instead
-// NewDefaultConfigProvider returns the default ConfigProvider, and it creates configuration from a file
-// defined by the given configFile and overwrites fields using properties.
-func NewDefaultConfigProvider(configLocations []string, properties []string) ConfigProvider {
-	return NewConfigProvider(
-		configLocations,
-		map[string]configmapprovider.Provider{
-			"file": configmapprovider.NewFile(),
-			"env":  configmapprovider.NewEnv(),
-		},
-		[]config.MapConverterFunc{
-			configmapprovider.NewOverwritePropertiesConverter(properties),
-			configmapprovider.NewExpandConverter(),
-		},
-		configunmarshaler.NewDefault())
+	configProvider, err := NewConfigProvider(configLocations, WithCfgMapConverters(cfgMapConverter))
+	if err != nil {
+		panic(err)
+	}
+	return configProvider
 }
 
 func (cm *configProvider) Get(ctx context.Context, factories component.Factories) (*config.Config, error) {

service/config_provider_test.go

@@ -165,7 +165,9 @@ func TestConfigProvider_Errors(t *testing.T) {
 	}
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {
-			cfgW := MustNewConfigProvider(tt.locations, tt.parserProvider, tt.cfgMapConverters, tt.configUnmarshaler)
+			cfgW, err := NewConfigProvider(tt.locations, WithConfigMapProviders(tt.parserProvider), WithCfgMapConverters(tt.cfgMapConverters), WithConfigUnmarshaler(tt.configUnmarshaler))
+			assert.NoError(t, err)
+
 			_, errN := cfgW.Get(context.Background(), factories)
 			if tt.expectNewErr {
 				assert.Error(t, errN)
@@ -200,11 +202,11 @@ func TestConfigProvider(t *testing.T) {
 		return &mockProvider{retM: cfgMap}
 	}()
 
-	cfgW := MustNewConfigProvider(
+	cfgW, err := NewConfigProvider(
 		[]string{"watcher:"},
-		map[string]configmapprovider.Provider{"watcher": configMapProvider},
-		nil,
-		configunmarshaler.NewDefault())
+		WithConfigMapProviders(map[string]configmapprovider.Provider{"watcher": configMapProvider}),
+		WithConfigUnmarshaler(configunmarshaler.NewDefault()))
+	require.NoError(t, err)
 	_, errN := cfgW.Get(context.Background(), factories)
 	assert.NoError(t, errN)
 
@@ -228,11 +230,11 @@ func TestConfigProviderNoWatcher(t *testing.T) {
 	require.NoError(t, errF)
 
 	watcherWG := sync.WaitGroup{}
-	cfgW := MustNewConfigProvider(
+	cfgW, err := NewConfigProvider(
 		[]string{filepath.Join("testdata", "otelcol-nop.yaml")},
-		map[string]configmapprovider.Provider{"file": configmapprovider.NewFile()},
-		nil,
-		configunmarshaler.NewDefault())
+		WithConfigMapProviders(map[string]configmapprovider.Provider{"file": configmapprovider.NewFile()}),
+		WithConfigUnmarshaler(configunmarshaler.NewDefault()))
+	require.NoError(t, err)
 	_, errN := cfgW.Get(context.Background(), factories)
 	assert.NoError(t, errN)
 
@@ -260,12 +262,13 @@ func TestConfigProvider_ShutdownClosesWatch(t *testing.T) {
 	}()
 
 	watcherWG := sync.WaitGroup{}
-	cfgW := MustNewConfigProvider(
+	cfgW, err := NewConfigProvider(
 		[]string{"watcher:"},
-		map[string]configmapprovider.Provider{"watcher": configMapProvider},
-		nil,
-		configunmarshaler.NewDefault())
+		WithConfigMapProviders(map[string]configmapprovider.Provider{"watcher": configMapProvider}),
+		WithConfigUnmarshaler(configunmarshaler.NewDefault()))
 	_, errN := cfgW.Get(context.Background(), factories)
+	require.NoError(t, err)
+
 	assert.NoError(t, errN)
 
 	watcherWG.Add(1)
@@ -322,8 +325,9 @@ func TestBackwardsCompatibilityForFilePath(t *testing.T) {
 		},
 	}
 	for _, tt := range tests {
-		provider := MustNewDefaultConfigProvider([]string{tt.location}, []string{})
-		_, err := provider.Get(context.Background(), factories)
+		provider, err := NewConfigProvider([]string{tt.location})
+		assert.NoError(t, err)
+		_, err = provider.Get(context.Background(), factories)
 		assert.Contains(t, err.Error(), tt.errMessage, tt.name)
 	}