Avoid using deprecated config.Config in servicetest (#6393)

Signed-off-by: Bogdan <[email protected]> Signed-off-by: Bogdan <[email protected]>
open-telemetry · Oct 25, 2022 · 2f42fa1 · 2f42fa1
1 parent 71e6952
commit 2f42fa1
Show file tree

Hide file tree

Showing 7 changed files with 70 additions and 36 deletions.
diff --git a/service/collector.go b/service/collector.go
@@ -132,6 +132,10 @@ func (col *Collector) setupConfigurationComponents(ctx context.Context) error {
 		return fmt.Errorf("failed to get config: %w", err)
 	}
 
+	if err = cfg.Validate(); err != nil {
+		return fmt.Errorf("invalid configuration: %w", err)
+	}
+
 	col.service, err = newService(&settings{
 		BuildInfo:         col.set.BuildInfo,
 		Factories:         col.set.Factories,

diff --git a/service/collector_test.go b/service/collector_test.go
@@ -236,6 +236,23 @@ func TestCollectorFailedShutdown(t *testing.T) {
 	assert.Equal(t, Closed, col.GetState())
 }
 
+func TestCollectorStartInvalidConfig(t *testing.T) {
+	factories, err := componenttest.NopFactories()
+	require.NoError(t, err)
+
+	cfgProvider, err := NewConfigProvider(newDefaultConfigProviderSettings([]string{filepath.Join("testdata", "otelcol-invalid.yaml")}))
+	require.NoError(t, err)
+
+	col, err := New(CollectorSettings{
+		BuildInfo:      component.NewDefaultBuildInfo(),
+		Factories:      factories,
+		ConfigProvider: cfgProvider,
+		telemetry:      newColTelemetry(featuregate.NewRegistry()),
+	})
+	require.NoError(t, err)
+	assert.Error(t, col.Run(context.Background()))
+}
+
 // mapConverter applies extraMap of config settings. Useful for overriding the config
 // for testing purposes. Keys must use "::" delimiter between levels.
 type mapConverter struct {

diff --git a/service/config_provider.go b/service/config_provider.go
@@ -110,10 +110,6 @@ func (cm *configProvider) Get(ctx context.Context, factories component.Factories
 		return nil, fmt.Errorf("cannot unmarshal the configuration: %w", err)
 	}
 
-	if err = cfg.Validate(); err != nil {
-		return nil, fmt.Errorf("invalid configuration: %w", err)
-	}
-
 	return cfg, nil
 }
 

diff --git a/service/config_provider_test.go b/service/config_provider_test.go
@@ -33,21 +33,6 @@ import (
 	"go.opentelemetry.io/collector/service/telemetry"
 )
 
-func TestConfigProviderValidationError(t *testing.T) {
-	factories, errF := componenttest.NopFactories()
-	require.NoError(t, errF)
-
-	set := newDefaultConfigProviderSettings([]string{filepath.Join("testdata", "otelcol-invalid.yaml")})
-
-	cfgW, err := NewConfigProvider(set)
-	assert.NoError(t, err)
-
-	_, err = cfgW.Get(context.Background(), factories)
-	assert.Error(t, err)
-
-	assert.NoError(t, cfgW.Shutdown(context.Background()))
-}
-
 var configNop = &Config{
 	Receivers:  map[config.ComponentID]config.Receiver{config.NewComponentID("nop"): componenttest.NewNopReceiverFactory().CreateDefaultConfig()},
 	Processors: map[config.ComponentID]config.Processor{config.NewComponentID("nop"): componenttest.NewNopProcessorFactory().CreateDefaultConfig()},

diff --git a/service/internal/pipelines/pipelines_test.go b/service/internal/pipelines/pipelines_test.go
@@ -26,11 +26,12 @@ import (
 	"go.opentelemetry.io/collector/component"
 	"go.opentelemetry.io/collector/component/componenttest"
 	"go.opentelemetry.io/collector/config"
+	"go.opentelemetry.io/collector/confmap/confmaptest"
 	"go.opentelemetry.io/collector/consumer"
 	"go.opentelemetry.io/collector/consumer/consumertest"
 	"go.opentelemetry.io/collector/internal/testdata"
+	"go.opentelemetry.io/collector/service/internal/configunmarshaler"
 	"go.opentelemetry.io/collector/service/internal/testcomponents"
-	"go.opentelemetry.io/collector/service/servicetest"
 )
 
 func TestBuild(t *testing.T) {
@@ -87,8 +88,7 @@ func TestBuild(t *testing.T) {
 			factories, err := testcomponents.ExampleComponents()
 			assert.NoError(t, err)
 
-			cfg, err := servicetest.LoadConfigAndValidate(filepath.Join("testdata", test.name), factories)
-			require.NoError(t, err)
+			cfg := loadConfigAndValidate(t, filepath.Join("testdata", test.name), factories)
 
 			// Build the pipeline
 			pipelines, err := Build(context.Background(), toSettings(factories, cfg))
@@ -249,15 +249,14 @@ func TestBuildErrors(t *testing.T) {
 			}
 
 			// Need the unknown factories to do unmarshalling.
-			cfg, err := servicetest.LoadConfig(filepath.Join("testdata", test.configFile), factories)
-			require.NoError(t, err)
+			cfg := loadConfig(t, filepath.Join("testdata", test.configFile), factories)
 
 			// Remove the unknown factories, so they are NOT available during building.
 			delete(factories.Exporters, "unknown")
 			delete(factories.Processors, "unknown")
 			delete(factories.Receivers, "unknown")
 
-			_, err = Build(context.Background(), toSettings(factories, cfg))
+			_, err := Build(context.Background(), toSettings(factories, cfg))
 			assert.Error(t, err)
 		})
 	}
@@ -464,3 +463,18 @@ func (e errComponent) Start(context.Context, component.Host) error {
 func (e errComponent) Shutdown(context.Context) error {
 	return errors.New("my error")
 }
+
+func loadConfig(t *testing.T, fileName string, factories component.Factories) *config.Config {
+	// Read yaml config from file
+	conf, err := confmaptest.LoadConf(fileName)
+	require.NoError(t, err)
+	cfg, err := configunmarshaler.Unmarshal(conf, factories)
+	require.NoError(t, err)
+	return cfg
+}
+
+func loadConfigAndValidate(t *testing.T, fileName string, factories component.Factories) *config.Config {
+	cfg := loadConfig(t, fileName, factories)
+	require.NoError(t, cfg.Validate())
+	return cfg
+}
diff --git a/service/service_test.go b/service/service_test.go
@@ -25,9 +25,7 @@ import (
 	"go.opentelemetry.io/collector/component"
 	"go.opentelemetry.io/collector/component/componenttest"
 	"go.opentelemetry.io/collector/config"
-	"go.opentelemetry.io/collector/confmap/confmaptest"
 	"go.opentelemetry.io/collector/featuregate"
-	"go.opentelemetry.io/collector/service/internal/configunmarshaler"
 )
 
 func TestService_GetFactory(t *testing.T) {
@@ -99,9 +97,9 @@ func TestServiceTelemetryCleanupOnError(t *testing.T) {
 	require.NoError(t, err)
 
 	// Read invalid yaml config from file
-	invalidConf, err := confmaptest.LoadConf(filepath.Join("testdata", "otelcol-invalid.yaml"))
+	invalidProvider, err := NewConfigProvider(newDefaultConfigProviderSettings([]string{filepath.Join("testdata", "otelcol-invalid.yaml")}))
 	require.NoError(t, err)
-	invalidCfg, err := configunmarshaler.Unmarshal(invalidConf, factories)
+	invalidCfg, err := invalidProvider.Get(context.Background(), factories)
 	require.NoError(t, err)
 
 	// Read valid yaml config from file

diff --git a/service/servicetest/configprovider.go b/service/servicetest/configprovider.go
@@ -15,27 +15,47 @@
 package servicetest // import "go.opentelemetry.io/collector/service/servicetest"
 
 import (
+	"context"
+
 	"go.opentelemetry.io/collector/component"
-	"go.opentelemetry.io/collector/config"
-	"go.opentelemetry.io/collector/confmap/confmaptest"
-	"go.opentelemetry.io/collector/service/internal/configunmarshaler"
+	"go.opentelemetry.io/collector/confmap"
+	"go.opentelemetry.io/collector/confmap/converter/expandconverter"
+	"go.opentelemetry.io/collector/confmap/provider/envprovider"
+	"go.opentelemetry.io/collector/confmap/provider/fileprovider"
+	"go.opentelemetry.io/collector/confmap/provider/httpprovider"
+	"go.opentelemetry.io/collector/confmap/provider/yamlprovider"
+	"go.opentelemetry.io/collector/service"
 )
 
 // LoadConfig loads a config.Config  from file, and does NOT validate the configuration.
-func LoadConfig(fileName string, factories component.Factories) (*config.Config, error) {
+func LoadConfig(fileName string, factories component.Factories) (*service.Config, error) {
 	// Read yaml config from file
-	conf, err := confmaptest.LoadConf(fileName)
+	provider, err := service.NewConfigProvider(service.ConfigProviderSettings{
+		ResolverSettings: confmap.ResolverSettings{
+			URIs:       []string{fileName},
+			Providers:  makeMapProvidersMap(fileprovider.New(), envprovider.New(), yamlprovider.New(), httpprovider.New()),
+			Converters: []confmap.Converter{expandconverter.New()},
+		},
+	})
 	if err != nil {
 		return nil, err
 	}
-	return configunmarshaler.Unmarshal(conf, factories)
+	return provider.Get(context.Background(), factories)
 }
 
 // LoadConfigAndValidate loads a config from the file, and validates the configuration.
-func LoadConfigAndValidate(fileName string, factories component.Factories) (*config.Config, error) {
+func LoadConfigAndValidate(fileName string, factories component.Factories) (*service.Config, error) {
 	cfg, err := LoadConfig(fileName, factories)
 	if err != nil {
 		return nil, err
 	}
 	return cfg, cfg.Validate()
 }
+
+func makeMapProvidersMap(providers ...confmap.Provider) map[string]confmap.Provider {
+	ret := make(map[string]confmap.Provider, len(providers))
+	for _, provider := range providers {
+		ret[provider.Scheme()] = provider
+	}
+	return ret
+}