Add double dollar config source parsing compatibility

CHANGELOG.md

@@ -2,10 +2,19 @@
 
 ## Unreleased
 
+### 🛑 Breaking changes 🛑
+
+- With the adoption of OpenTelemetry Core 0.42.0 the configuration parsing process has been updated. Use of `$`-escaped `$` characters may not function as did previously.
+
 ### 🚀 New components 🚀
 
 - [`docker_observer`](https://github.com/open-telemetry/opentelemetry-collector-contrib/tree/main/extension/observer/dockerobserver) to detect and create container endpoints, to be used with the [`receiver_creator`](https://github.com/open-telemetry/opentelemetry-collector-contrib/tree/main/receiver/receivercreator).
 
+### 💡 Enhancements 💡
+
+- Update Collector Core and Contrib deps to 0.42.0 (#1099)
+- Provide a temporary, backward-compatible `$${config_source:value}` parsing rule controlled by `SPLUNK_DOUBLE_DOLLAR_CONFIG_SOURCE_COMPATIBLE` environment variable (default `"true"`) to continue supporting the updating configs from #930 and #935. This will be removed in a future release (#1099).
+
 ## v0.41.0
 
 This Splunk OpenTelemetry Collector release includes changes from the [opentelemetry-collector v0.41.0](https://github.com/open-telemetry/opentelemetry-collector/releases/tag/v0.41.0) and the [opentelemetry-collector-contrib v0.41.0](https://github.com/open-telemetry/opentelemetry-collector-contrib/releases/tag/v0.41.0) releases.

internal/configprovider/manager.go

@@ -20,6 +20,7 @@ import (
 	"fmt"
 	"net/url"
 	"os"
+	"strconv"
 	"strings"
 	"sync"
 
@@ -42,13 +43,22 @@ const (
 	// typeAndNameSeparator is the separator that is used between type and name in type/name
 	// composite keys.
 	typeAndNameSeparator = '/'
+	// dollarDollarCompatEnvVar is a temporary env var to disable backward compatibility (true by default)
+	dollarDollarCompatEnvVar = "SPLUNK_DOUBLE_DOLLAR_CONFIG_SOURCE_COMPATIBLE"
 )
 
 // private error types to help with testability
 type (
 	errUnknownConfigSource struct{ error }
 )
 
+var ddBackwardCompatible = func() bool {
+	if v, err := strconv.ParseBool(strings.ToLower(os.Getenv(dollarDollarCompatEnvVar))); err == nil {
+		return v
+	}
+	return true
+}()
+
 // Manager is used to inject data from config sources into a configuration and also
 // to monitor for updates on the items injected into the configuration.  All methods
 // of a Manager must be called only once and have an expected sequence:
@@ -160,20 +170,12 @@ type (
 //
 // For an overview about the internals of the Manager refer to the package README.md.
 type Manager struct {
-	// configSources is map from ConfigSource names (as defined in the configuration)
-	// and the respective instances.
 	configSources map[string]configsource.ConfigSource
-	// watchingCh is used to notify users of the Manager that the WatchForUpdate function
-	// is ready and waiting for notifications.
-	watchingCh chan struct{}
-	// closeCh is used to notify the Manager WatchForUpdate function that the manager
-	// is being closed.
-	closeCh chan struct{}
-	// watchers keeps track of all WatchForUpdate functions for retrieved values.
-	watchers []configsource.Watchable
-	// watchersWG is used to ensure that Close waits for all WatchForUpdate calls
-	// to complete.
-	watchersWG sync.WaitGroup
+	watchingCh    chan struct{}
+	closeCh       chan struct{}
+	logger        *zap.Logger
+	watchers      []configsource.Watchable
+	watchersWG    sync.WaitGroup
 }
 
 // NewManager creates a new instance of a Manager to be used to inject data from
@@ -194,7 +196,7 @@ func NewManager(configMap *config.Map, logger *zap.Logger, buildInfo component.B
 		return nil, err
 	}
 
-	return newManager(cfgSources), nil
+	return newManager(cfgSources, logger), nil
 }
 
 // Resolve inspects the given config.Map and resolves all config sources referenced
@@ -290,11 +292,12 @@ func (m *Manager) Close(ctx context.Context) error {
 	return errs
 }
 
-func newManager(configSources map[string]configsource.ConfigSource) *Manager {
+func newManager(configSources map[string]configsource.ConfigSource, logger *zap.Logger) *Manager {
 	return &Manager{
 		configSources: configSources,
 		watchingCh:    make(chan struct{}),
 		closeCh:       make(chan struct{}),
+		logger:        logger,
 	}
 }
 
@@ -376,37 +379,51 @@ func (m *Manager) parseStringValue(ctx context.Context, s string) (interface{},
 
 			switch {
 			case s[j+1] == expandPrefixChar:
-				// Escaping the prefix so $$ becomes a single $ without attempting
-				// to treat the string after it as a config source or env var.
-				expandableContent = string(expandPrefixChar)
-				w = 1 // consumed a single char
+				// temporary backward compatibility to support updated $${config_source:value} functionality
+				// in provided configs from 0.37.0 until 0.42.0
+				bwCompatibilityRequired := false
+
+				var expanded, sourceName string
+				var ww int
+				if ddBackwardCompatible && len(s[j+1:]) > 2 {
+					if s[j+2] == '{' {
+						if expanded, ww, sourceName = getBracketedExpandableContent(s, j+2); sourceName != "" {
+							bwCompatibilityRequired = true
+						}
+					} else {
+						if expanded, ww, sourceName = getBareExpandableContent(s, j+2); sourceName != "" {
+							if len(expanded) > (len(sourceName) + 1) {
+								if !strings.ContainsAny(expanded[len(sourceName)+1:], "$:") {
+									bwCompatibilityRequired = true
+								}
+							}
+						}
+					}
+				}
 
-			case s[j+1] == '{':
-				// Bracketed usage, consume everything until first '}' exactly as os.Expand.
-				expandableContent, w = scanToClosingBracket(s[j+1:])
-				expandableContent = strings.Trim(expandableContent, " ") // Allow for some spaces.
-				delimIndex := strings.Index(expandableContent, string(configSourceNameDelimChar))
-				if len(expandableContent) > 1 && delimIndex > -1 {
-					// Bracket expandableContent contains ':' treating it as a config source.
-					cfgSrcName = expandableContent[:delimIndex]
+				if bwCompatibilityRequired {
+					m.logger.Warn(
+						fmt.Sprintf(
+							"Deprecated config source directive %q has been replaced with %q. Please update your config as necessary as this will be removed in future release.",
+							s[j:j+2+ww], s[j+1:j+2+ww],
+						),
+					)
+					expandableContent = expanded
+					w = ww + 1
+					cfgSrcName = sourceName
+				} else {
+					// Escaping the prefix so $$ becomes a single $ without attempting
+					// to treat the string after it as a config source or env var.
+					expandableContent = string(expandPrefixChar)
+					w = 1 // consumed a single char
 				}
 
+			case s[j+1] == '{':
+				expandableContent, w, cfgSrcName = getBracketedExpandableContent(s, j+1)
+
 			default:
-				// Non-bracketed usage, ie.: found the prefix char, it can be either a config
-				// source or an environment variable.
-				var name string
-				name, w = getTokenName(s[j+1:])
-				expandableContent = name // Assume for now that it is an env var.
-
-				// Peek next char after name, if it is a config source name delimiter treat the remaining of the
-				// string as a config source.
-				possibleDelimCharIndex := j + w + 1
-				if possibleDelimCharIndex < len(s) && s[possibleDelimCharIndex] == configSourceNameDelimChar {
-					// This is a config source, since it is not delimited it will consume until end of the string.
-					cfgSrcName = name
-					expandableContent = s[j+1:]
-					w = len(expandableContent) // Set consumed bytes to the length of expandableContent
-				}
+				expandableContent, w, cfgSrcName = getBareExpandableContent(s, j+1)
+
 			}
 
 			// At this point expandableContent contains a string to be expanded, evaluate and expand it.
@@ -473,6 +490,37 @@ func (m *Manager) parseStringValue(ctx context.Context, s string) (interface{},
 	return string(buf) + s[i:], nil
 }
 
+func getBracketedExpandableContent(s string, i int) (expandableContent string, consumed int, cfgSrcName string) {
+	// Bracketed usage, consume everything until first '}' exactly as os.Expand.
+	expandableContent, consumed = scanToClosingBracket(s[i:])
+	expandableContent = strings.Trim(expandableContent, " ") // Allow for some spaces.
+	delimIndex := strings.Index(expandableContent, string(configSourceNameDelimChar))
+	if len(expandableContent) > 1 && delimIndex > -1 {
+		// Bracket expandableContent contains ':' treating it as a config source.
+		cfgSrcName = expandableContent[:delimIndex]
+	}
+	return
+}
+
+func getBareExpandableContent(s string, i int) (expandableContent string, consumed int, cfgSrcName string) {
+	// Non-bracketed usage, ie.: found the prefix char, it can be either a config
+	// source or an environment variable.
+	var name string
+	name, consumed = getTokenName(s[i:])
+	expandableContent = name // Assume for now that it is an env var.
+
+	// Peek next char after name, if it is a config source name delimiter treat the remaining of the
+	// string as a config source.
+	possibleDelimCharIndex := i + consumed
+	if possibleDelimCharIndex < len(s) && s[possibleDelimCharIndex] == configSourceNameDelimChar {
+		// This is a config source, since it is not delimited it will consume until end of the string.
+		cfgSrcName = name
+		expandableContent = s[i:]
+		consumed = len(expandableContent) // Set consumed bytes to the length of expandableContent
+	}
+	return
+}
+
 // retrieveConfigSourceData retrieves data from the specified config source and injects them into
 // the configuration. The Manager tracks sessions and watcher objects as needed.
 func (m *Manager) retrieveConfigSourceData(ctx context.Context, cfgSrcName, cfgSrcInvocation string) (interface{}, error) {

internal/configprovider/manager_test.go

@@ -88,7 +88,7 @@ func TestConfigSourceManager_Simple(t *testing.T) {
 				"test_selector": {Value: "test_value"},
 			},
 		},
-	})
+	}, zap.NewNop())
 
 	originalCfg := map[string]interface{}{
 		"top0": map[string]interface{}{
@@ -134,7 +134,7 @@ func TestConfigSourceManager_ResolveRemoveConfigSourceSection(t *testing.T) {
 
 	manager := newManager(map[string]configsource.ConfigSource{
 		"tstcfgsrc": &testConfigSource{},
-	})
+	}, zap.NewNop())
 
 	res, err := manager.Resolve(context.Background(), config.NewMapFromStringMap(cfg))
 	require.NoError(t, err)
@@ -174,7 +174,7 @@ func TestConfigSourceManager_ResolveErrors(t *testing.T) {
 	}
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {
-			manager := newManager(tt.configSourceMap)
+			manager := newManager(tt.configSourceMap, zap.NewNop())
 
 			res, err := manager.Resolve(ctx, config.NewMapFromStringMap(tt.config))
 			require.Error(t, err)
@@ -209,7 +209,7 @@ map:
 				"invalid_yaml_byte_slice": {Value: []byte(":")},
 			},
 		},
-	})
+	}, zap.NewNop())
 
 	file := path.Join("testdata", "yaml_injection.yaml")
 	cp, err := configtest.LoadConfigMap(file)
@@ -238,7 +238,7 @@ func TestConfigSourceManager_ArraysAndMaps(t *testing.T) {
 				"k1":    {Value: "k1_value"},
 			},
 		},
-	})
+	}, zap.NewNop())
 
 	file := path.Join("testdata", "arrays_and_maps.yaml")
 	cp, err := configtest.LoadConfigMap(file)
@@ -292,7 +292,7 @@ func TestConfigSourceManager_ParamsHandling(t *testing.T) {
 
 	manager := newManager(map[string]configsource.ConfigSource{
 		"tstcfgsrc": &tstCfgSrc,
-	})
+	}, zap.NewNop())
 
 	file := path.Join("testdata", "params_handling.yaml")
 	cp, err := configtest.LoadConfigMap(file)
@@ -323,7 +323,7 @@ func TestConfigSourceManager_WatchForUpdate(t *testing.T) {
 				},
 			},
 		},
-	})
+	}, zap.NewNop())
 
 	originalCfg := map[string]interface{}{
 		"top0": map[string]interface{}{
@@ -374,7 +374,7 @@ func TestConfigSourceManager_MultipleWatchForUpdate(t *testing.T) {
 				},
 			},
 		},
-	})
+	}, zap.NewNop())
 
 	originalCfg := map[string]interface{}{
 		"top0": map[string]interface{}{
@@ -432,10 +432,11 @@ func TestConfigSourceManager_EnvVarHandling(t *testing.T) {
 		}
 		return nil
 	}
+	logger, _ := zap.NewProduction()
 
 	manager := newManager(map[string]configsource.ConfigSource{
 		"tstcfgsrc": &tstCfgSrc,
-	})
+	}, logger)
 
 	file := path.Join("testdata", "envvar_cfgsrc_mix.yaml")
 	cp, err := configtest.LoadConfigMap(file)
@@ -466,7 +467,7 @@ func TestManager_expandString(t *testing.T) {
 				"int_key": {Value: 42},
 			},
 		},
-	})
+	}, zap.NewNop())
 
 	require.NoError(t, os.Setenv("envvar", "envvar_value"))
 	defer func() {
@@ -586,7 +587,12 @@ func TestManager_expandString(t *testing.T) {
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {
 			got, err := manager.parseStringValue(ctx, tt.input)
-			require.IsType(t, tt.wantErr, err)
+			if tt.wantErr != nil {
+				require.Error(t, err)
+				require.IsType(t, tt.wantErr, err)
+			} else {
+				require.NoError(t, err)
+			}
 			require.Equal(t, tt.want, got)
 		})
 	}

internal/configprovider/parser.go

@@ -24,6 +24,7 @@ import (
 	"go.opentelemetry.io/collector/config"
 	expcfg "go.opentelemetry.io/collector/config/experimental/config"
 	"go.opentelemetry.io/collector/config/experimental/configsource"
+	"go.uber.org/zap"
 )
 
 const (
@@ -58,7 +59,7 @@ func Load(ctx context.Context, v *config.Map, factories Factories) (map[string]e
 func processParser(ctx context.Context, v *config.Map) (*config.Map, error) {
 	// Use a manager to resolve environment variables with a syntax consistent with
 	// the config source usage.
-	manager := newManager(make(map[string]configsource.ConfigSource))
+	manager := newManager(make(map[string]configsource.ConfigSource), zap.NewNop())
 	defer func() {
 		_ = manager.Close(ctx)
 	}()

internal/configprovider/testdata/envvar_cfgsrc_mix.yaml

@@ -10,6 +10,10 @@ envvar_legacy_05: $1/test
 cfgsrc_suffix: prefix-$tstcfgsrc:int_key
 cfgsrc_middle: prefix-${tstcfgsrc:int_key}-suffix
 cfgsrc_in_str: integer ${tstcfgsrc:int_key} injected as string
+cfgsrc_workaround_suffix: prefix-$$tstcfgsrc:int_key
+cfgsrc_braces_workaround_suffix: prefix-$${     tstcfgsrc:int_key    }
+cfgsrc_braces_workaround_middle: prefix-$${tstcfgsrc:int_key}-suffix
+cfgsrc_braces_workaround_in_str: integer $${      tstcfgsrc:int_key} injected as string
 cfgsrc_params0: ${tstcfgsrc:params_key?p0=true&p1=$envvar&p2=42}
 cfgsrc_params1: "${tstcfgsrc:params_key?p0=false&p1=42&p2=$envvar}"
 cfgsrc_params2: $tstcfgsrc:params_key?p0=$$envvar

internal/configprovider/testdata/envvar_cfgsrc_mix_expected.yaml

@@ -10,6 +10,10 @@ envvar_legacy_05: /test
 cfgsrc_suffix: prefix-42
 cfgsrc_middle: prefix-42-suffix
 cfgsrc_in_str: integer 42 injected as string
+cfgsrc_workaround_suffix: prefix-42
+cfgsrc_braces_workaround_suffix: prefix-42
+cfgsrc_braces_workaround_middle: prefix-42-suffix
+cfgsrc_braces_workaround_in_str: integer 42 injected as string
 cfgsrc_params0:
   p0: true
   p1: envvar_value