ensure --dry-run doesn't report expanded config values

cmd/otelcol/main.go

@@ -72,6 +72,7 @@ func main() {
 		log.Fatalf("failed to create discovery provider: %v", err)
 	}
 
+	hooks := []configprovider.Hook{configServer, dryRun}
 	envProvider := envprovider.New()
 	fileProvider := fileprovider.New()
 	serviceConfigProvider, err := otelcol.NewConfigProvider(
@@ -82,16 +83,16 @@ func main() {
 					discovery.ConfigDScheme(): configprovider.NewConfigSourceConfigMapProvider(
 						discovery.ConfigDProvider(),
 						zap.NewNop(), // The service logger is not available yet, setting it to Nop.
-						info, configServer, configsources.Get()...,
+						info, hooks, configsources.Get()...,
 					),
 					discovery.DiscoveryModeScheme(): configprovider.NewConfigSourceConfigMapProvider(
-						discovery.DiscoveryModeProvider(), zap.NewNop(), info, configServer, configsources.Get()...,
+						discovery.DiscoveryModeProvider(), zap.NewNop(), info, hooks, configsources.Get()...,
 					),
 					envProvider.Scheme(): configprovider.NewConfigSourceConfigMapProvider(
-						envProvider, zap.NewNop(), info, configServer, configsources.Get()...,
+						envProvider, zap.NewNop(), info, hooks, configsources.Get()...,
 					),
 					fileProvider.Scheme(): configprovider.NewConfigSourceConfigMapProvider(
-						fileProvider, zap.NewNop(), info, configServer, configsources.Get()...,
+						fileProvider, zap.NewNop(), info, hooks, configsources.Get()...,
 					),
 				}, Converters: confMapConverters,
 			},

internal/configconverter/config_server.go

@@ -31,6 +31,8 @@ import (
 	"github.com/spf13/cast"
 	"go.opentelemetry.io/collector/confmap"
 	"gopkg.in/yaml.v2"
+
+	"github.com/signalfx/splunk-otel-collector/internal/configprovider"
 )
 
 const (
@@ -49,6 +51,7 @@ const (
 )
 
 var _ confmap.Converter = (*ConfigServer)(nil)
+var _ configprovider.Hook = (*ConfigServer)(nil)
 
 type ConfigServer struct {
 	// Use get/set methods instead of direct usage
@@ -95,14 +98,14 @@ func (cs *ConfigServer) Convert(_ context.Context, conf *confmap.Conf) error {
 	return nil
 }
 
-func (cs *ConfigServer) Register() {
+func (cs *ConfigServer) OnNew() {
 	cs.wg.Add(1)
 }
 
-func (cs *ConfigServer) SetForScheme(scheme string, config map[string]any) {
+func (cs *ConfigServer) OnRetrieve(scheme string, retrieved map[string]any) {
 	cs.initialMutex.Lock()
 	defer cs.initialMutex.Unlock()
-	cs.initial[scheme] = config
+	cs.initial[scheme] = retrieved
 }
 
 func (cs *ConfigServer) getInitial() map[string]any {
@@ -123,8 +126,8 @@ func (cs *ConfigServer) getEffective() map[string]any {
 	return cs.effective
 }
 
-// start will create and start the singleton http server. It presumes cs.Register() has been
-// called at least once and will tear down the moment the final cs.Unregister() call is made.
+// start will create and start the singleton http server. It presumes cs.OnNew() has been
+// called at least once and will tear down the moment the final cs.OnShutdown() call is made.
 func (cs *ConfigServer) start() {
 	if enabled := os.Getenv(configServerEnabledEnvVar); enabled != "true" {
 		// The config server needs to be explicitly enabled for the time being.
@@ -174,7 +177,7 @@ func (cs *ConfigServer) start() {
 		})
 }
 
-func (cs *ConfigServer) Unregister() {
+func (cs *ConfigServer) OnShutdown() {
 	cs.wg.Done()
 }
 

internal/configconverter/config_server_test.go

@@ -38,8 +38,8 @@ func TestConfigServer_RequireEnvVar(t *testing.T) {
 
 	cs := NewConfigServer()
 	require.NotNil(t, cs)
-	cs.Register()
-	t.Cleanup(cs.Unregister)
+	cs.OnNew()
+	t.Cleanup(cs.OnShutdown)
 	require.NoError(t, cs.Convert(context.Background(), confmap.NewFromStringMap(initial)))
 
 	client := &http.Client{}
@@ -92,10 +92,10 @@ func TestConfigServer_EnvVar(t *testing.T) {
 
 			cs := NewConfigServer()
 			require.NotNil(t, cs)
-			cs.Register()
+			cs.OnNew()
 
 			require.NoError(t, cs.Convert(context.Background(), confmap.NewFromStringMap(initial)))
-			defer cs.Unregister()
+			defer cs.OnShutdown()
 
 			endpoint := tt.endpoint
 			if endpoint == "" {
@@ -147,10 +147,10 @@ func TestConfigServer_Serve(t *testing.T) {
 
 	cs := NewConfigServer()
 	require.NotNil(t, cs)
-	cs.Register()
-	t.Cleanup(cs.Unregister)
+	cs.OnNew()
+	t.Cleanup(cs.OnShutdown)
 
-	cs.SetForScheme("scheme", initial)
+	cs.OnRetrieve("scheme", initial)
 	require.NoError(t, cs.Convert(context.Background(), confmap.NewFromStringMap(initial)))
 
 	// Test for the pages to be actually valid YAML files.

internal/configconverter/dry_run.go

@@ -18,32 +18,66 @@ import (
 	"context"
 	"fmt"
 	"os"
+	"sync"
 
 	"go.opentelemetry.io/collector/confmap"
 	"gopkg.in/yaml.v2"
+
+	"github.com/signalfx/splunk-otel-collector/internal/configprovider"
 )
 
 var _ confmap.Converter = (*DryRun)(nil)
+var _ configprovider.Hook = (*DryRun)(nil)
 
 type DryRun struct {
+	*sync.Mutex
+	configs []map[string]any
 	enabled bool
 }
 
-func NewDryRun(enabled bool) DryRun {
-	return DryRun{enabled: enabled}
+func NewDryRun(enabled bool) *DryRun {
+	return &DryRun{
+		Mutex:   &sync.Mutex{},
+		enabled: enabled,
+		configs: []map[string]any{},
+	}
 }
 
-// Convert is intended to be called as the final service confmap.Converter so
-// that it has access to the final config before exiting, if enabled.
-func (dr DryRun) Convert(_ context.Context, conf *confmap.Conf) error {
-	if dr.enabled {
-		out, err := yaml.Marshal(conf.ToStringMap())
-		if err != nil {
-			panic(fmt.Errorf("failed marshaling --dry-run config: %w", err))
+func (dr *DryRun) OnNew() {}
+
+func (dr *DryRun) OnRetrieve(_ string, retrieved map[string]any) {
+	if dr == nil || !dr.enabled {
+		return
+	}
+	dr.Lock()
+	defer dr.Unlock()
+	dr.configs = append(dr.configs, retrieved)
+}
+
+func (dr *DryRun) OnShutdown() {}
+
+// Convert disregards the provided *confmap.Conf so that it will use
+// unexpanded values (env vars, config source directives) as
+// accrued by OnRetrieve() calls.
+func (dr *DryRun) Convert(context.Context, *confmap.Conf) error {
+	if dr == nil || !dr.enabled {
+		return nil
+	}
+	cm := confmap.New()
+	dr.Lock()
+	for _, cfg := range dr.configs {
+		if err := cm.Merge(confmap.NewFromStringMap(cfg)); err != nil {
+			dr.Unlock()
+			return err
 		}
-		fmt.Fprintf(os.Stdout, "%s", out)
-		os.Stdout.Sync()
-		os.Exit(0)
 	}
+	dr.Unlock() // not deferred because we are exiting
+	out, err := yaml.Marshal(cm.ToStringMap())
+	if err != nil {
+		panic(fmt.Errorf("failed marshaling --dry-run config: %w", err))
+	}
+	fmt.Fprintf(os.Stdout, "%s", out)
+	os.Stdout.Sync()
+	os.Exit(0)
 	return nil
 }

internal/configconverter/dry_run_test.go

@@ -27,13 +27,20 @@ import (
 )
 
 func TestDryRun(t *testing.T) {
-	config := confmap.NewFromStringMap(map[string]interface{}{"one": "one"})
+	cfgOne := confmap.NewFromStringMap(map[string]any{"key.one": "value.one"})
 	dr := NewDryRun(false)
+	dr.OnNew()
+	defer func() { require.NotPanics(t, dr.OnShutdown) }()
+
+	dr.OnRetrieve("some.scheme", cfgOne.ToStringMap())
 	require.NotPanics(t, func() {
-		dr.Convert(context.Background(), config)
+		dr.Convert(context.Background(), cfgOne)
 	})
 
 	origStdOut := os.Stdout
+	t.Cleanup(func() {
+		os.Stdout = origStdOut
+	})
 	stdout, err := os.CreateTemp("", "stdout")
 	require.NoError(t, err)
 	require.NotNil(t, stdout)
@@ -48,14 +55,17 @@ func TestDryRun(t *testing.T) {
 	}())
 
 	dr = NewDryRun(true)
+	cfgTwo := confmap.NewFromStringMap(map[string]any{"key.two": "value.two"})
+	dr.OnRetrieve("some.scheme", cfgOne.ToStringMap())
+	dr.OnRetrieve("another.scheme", cfgTwo.ToStringMap())
 	require.Panics(t, func() {
-		dr.Convert(context.Background(), config)
+		dr.Convert(context.Background(), cfgTwo)
 	})
-	os.Stdout = origStdOut
 	stdout.Seek(0, 0)
 	out, err := io.ReadAll(stdout)
 	require.NoError(t, err)
 	actual := map[string]any{}
 	yaml.Unmarshal(out, &actual)
-	require.Equal(t, config.ToStringMap(), actual)
+	expected := map[string]any{"key.one": "value.one", "key.two": "value.two"}
+	require.Equal(t, expected, actual)
 }

internal/configprovider/config_source_provider.go

@@ -22,17 +22,21 @@ import (
 	"go.opentelemetry.io/collector/component"
 	"go.opentelemetry.io/collector/confmap"
 	"go.uber.org/zap"
-
-	"github.com/signalfx/splunk-otel-collector/internal/configconverter"
 )
 
 var (
 	_ confmap.Provider = (*configSourceConfigMapProvider)(nil)
 )
 
+type Hook interface {
+	OnNew()
+	OnRetrieve(scheme string, retrieved map[string]any)
+	OnShutdown()
+}
+
 type configSourceConfigMapProvider struct {
 	logger           *zap.Logger
-	configServer     *configconverter.ConfigServer
+	hooks            []Hook
 	wrappedProvider  confmap.Provider
 	wrappedRetrieved *confmap.Retrieved
 	buildInfo        component.BuildInfo
@@ -41,10 +45,12 @@ type configSourceConfigMapProvider struct {
 
 // NewConfigSourceConfigMapProvider creates a ParserProvider that uses config sources.
 func NewConfigSourceConfigMapProvider(wrappedProvider confmap.Provider, logger *zap.Logger,
-	buildInfo component.BuildInfo, configServer *configconverter.ConfigServer, factories ...Factory) confmap.Provider {
-	configServer.Register()
+	buildInfo component.BuildInfo, hooks []Hook, factories ...Factory) confmap.Provider {
+	for _, h := range hooks {
+		h.OnNew()
+	}
 	return &configSourceConfigMapProvider{
-		configServer:     configServer,
+		hooks:            hooks,
 		wrappedProvider:  wrappedProvider,
 		logger:           logger,
 		factories:        factories,
@@ -89,7 +95,12 @@ func (c *configSourceConfigMapProvider) Retrieve(ctx context.Context, uri string
 	if err != nil {
 		return nil, err
 	}
-	c.configServer.SetForScheme(c.Scheme(), wrappedMap.ToStringMap())
+
+	scheme, stringMap := c.Scheme(), wrappedMap.ToStringMap()
+	for _, h := range c.hooks {
+		h.OnRetrieve(scheme, stringMap)
+	}
+
 	retrieved, closeFunc, err := Resolve(ctx, wrappedMap, c.logger, c.buildInfo, factories, onChange)
 	if err != nil {
 		return nil, err
@@ -103,7 +114,9 @@ func (c *configSourceConfigMapProvider) Scheme() string {
 }
 
 func (c *configSourceConfigMapProvider) Shutdown(ctx context.Context) error {
-	c.configServer.Unregister()
+	for _, h := range c.hooks {
+		h.OnShutdown()
+	}
 	return c.wrappedProvider.Shutdown(ctx)
 }