Add double dollar config source parsing compatibility

CHANGELOG.md

@@ -2,9 +2,17 @@
 
 ## Unreleased
 
+### 🛑 Breaking changes 🛑
+
+- This version adopts OpenTelemetry Core version 0.42.0, and in doing so the configuration parsing process has changed slightly. The Splunk OpenTelemetry Collector used to [evaluate user configuration twice](https://github.com/signalfx/splunk-otel-collector/issues/628) and this required escaping desired `$` literals with an additional `$` character to prevent unwanted environment variable expansion. This version no longer doubly evaluates configuration so any `$$` instances in your configuration as a workaround should be updated to `$`.  [Config source directives](./internal/configsource) that include and additional `$` are provided with a temporary, backward-compatible `$${config_source:value}` and `$$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](https://github.com/signalfx/splunk-otel-collector/pull/930) and [#935](https://github.com/signalfx/splunk-otel-collector/pull/935). This functionality will be removed in a future release (#1099)
+
 ### 🚀 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).
+- [`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) (#1044)
+
+### 💡 Enhancements 💡
+
+- Update Collector Core and Contrib deps to 0.42.0 (#1099)
 
 ## v0.41.0
 

internal/configprovider/manager.go

@@ -18,8 +18,10 @@ import (
 	"context"
 	"errors"
 	"fmt"
+	"log"
 	"net/url"
 	"os"
+	"strconv"
 	"strings"
 	"sync"
 
@@ -42,13 +44,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 +171,11 @@ 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{}
+	watchers      []configsource.Watchable
+	watchersWG    sync.WaitGroup
 }
 
 // NewManager creates a new instance of a Manager to be used to inject data from
@@ -376,37 +378,49 @@ 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.Contains(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 {
+					log.Printf(
+						`Deprecated config source directive %q has been replaced with %q. Please update your config as necessary as this will be removed in future release. To disable this replacement set the SPLUNK_DOUBLE_DOLLAR_CONFIG_SOURCE_COMPATIBLE environment variable to "false" before restarting the Collector.`,
+						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 +487,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

@@ -432,7 +432,6 @@ func TestConfigSourceManager_EnvVarHandling(t *testing.T) {
 		}
 		return nil
 	}
-
 	manager := newManager(map[string]configsource.ConfigSource{
 		"tstcfgsrc": &tstCfgSrc,
 	})
@@ -586,7 +585,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/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

tests/general/envvar_expansion_test.go

@@ -58,3 +58,24 @@ func TestExpandedDollarSignsViaEnvConfigSource(t *testing.T) {
 	expectedResourceMetrics := tc.ResourceMetrics("env_config_source_labels.yaml")
 	require.NoError(t, tc.OTLPMetricsReceiverSink.AssertAllMetricsReceived(t, *expectedResourceMetrics, 30*time.Second))
 }
+
+func TestIncompatibleExpandedDollarSignsViaEnvConfigSource(t *testing.T) {
+	tc := testutils.NewTestcase(t)
+	defer tc.PrintLogsOnFailure()
+	defer tc.ShutdownOTLPMetricsReceiverSink()
+
+	tc.SkipIfNotContainer()
+
+	_, shutdown := tc.SplunkOtelCollectorWithEnv(
+		"env_config_source_labels.yaml",
+		map[string]string{
+			"SPLUNK_DOUBLE_DOLLAR_CONFIG_SOURCE_COMPATIBLE": "false",
+			"AN_ENVVAR": "an-envvar-value",
+		},
+	)
+
+	defer shutdown()
+
+	expectedResourceMetrics := tc.ResourceMetrics("incompat_env_config_source_labels.yaml")
+	require.NoError(t, tc.OTLPMetricsReceiverSink.AssertAllMetricsReceived(t, *expectedResourceMetrics, 30*time.Second))
+}

tests/general/testdata/env_config_source_labels.yaml

@@ -22,33 +22,63 @@ processors:
           - action: add_label
             new_label: single-dollar
             new_value: ${env:AN_ENVVAR}-suffix
+          - action: add_label
+            new_label: single-dollar-no-curly-braces
+            new_value: prefix-$env:AN_ENVVAR
           - action: add_label
             new_label: double-dollar
             new_value: $${env:AN_ENVVAR}-suffix
+          - action: add_label
+            new_label: double-dollar-no-curly-braces
+            new_value: prefix-$$env:AN_ENVVAR
           - action: add_label
             new_label: triple-dollar
             new_value: $$${env:AN_ENVVAR}-suffix
+          - action: add_label
+            new_label: triple-dollar-no-curly-braces
+            new_value: prefix-$$$env:AN_ENVVAR
           - action: add_label
             new_label: quadruple-dollar
             new_value: $$$${env:AN_ENVVAR}-suffix
+          - action: add_label
+            new_label: quadruple-dollar-no-curly-braces
+            new_value: prefix-$$$$env:AN_ENVVAR
           - action: add_label
             new_label: quintuple-dollar
             new_value: $$$$${env:AN_ENVVAR}-suffix
+          - action: add_label
+            new_label: quintuple-dollar-no-curly-braces
+            new_value: prefix-$$$$$env:AN_ENVVAR
           - action: add_label
             new_label: sextuple-dollar
             new_value: $$$$$${env:AN_ENVVAR}-suffix
+          - action: add_label
+            new_label: sextuple-dollar-no-curly-braces
+            new_value: prefix-$$$$$$env:AN_ENVVAR
           - action: add_label
             new_label: septuple-dollar
             new_value: $$$$$$${env:AN_ENVVAR}-suffix
+          - action: add_label
+            new_label: septuple-dollar-no-curly-braces
+            new_value: prefix-$$$$$$$env:AN_ENVVAR
           - action: add_label
             new_label: octuple-dollar
             new_value: $$$$$$$${env:AN_ENVVAR}-suffix
+          - action: add_label
+            new_label: octuple-dollar-no-curly-braces
+            new_value: prefix-$$$$$$$$env:AN_ENVVAR
           - action: add_label
             new_label: nonuple-dollar
             new_value: $$$$$$$$${env:AN_ENVVAR}-suffix
+          - action: add_label
+            new_label: nonuple-dollar-no-curly-braces
+            new_value: prefix-$$$$$$$$$env:AN_ENVVAR
           - action: add_label
             new_label: decuple-dollar
             new_value: $$$$$$$$$${env:AN_ENVVAR}-suffix
+          - action: add_label
+            new_label: decuple-dollar-no-curly-braces
+            new_value: prefix-$$$$$$$$$$env:AN_ENVVAR
 
 exporters:
   otlp:

tests/general/testdata/resource_metrics/env_config_source_labels.yaml

@@ -5,13 +5,23 @@ resource_metrics:
             type: IntGauge
             labels:
               state: used
-              single-dollar: -suffix
+              single-dollar: an-envvar-value-suffix
+              single-dollar-no-curly-braces: prefix-an-envvar-value
               double-dollar: an-envvar-value-suffix
-              triple-dollar: suffix
-              quadruple-dollar: ${env:AN_ENVVAR}-suffix
-              quintuple-dollar: $-suffix
-              sextuple-dollar: $an-envvar-value-suffix
-              septuple-dollar: $suffix
-              octuple-dollar: $${env:AN_ENVVAR}-suffix
-              nonuple-dollar: $$-suffix
-              decuple-dollar: $$an-envvar-value-suffix
+              double-dollar-no-curly-braces: prefix-an-envvar-value
+              triple-dollar: $an-envvar-value-suffix
+              triple-dollar-no-curly-braces: prefix-$an-envvar-value
+              quadruple-dollar: $an-envvar-value-suffix
+              quadruple-dollar-no-curly-braces: prefix-$an-envvar-value
+              quintuple-dollar: $$an-envvar-value-suffix
+              quintuple-dollar-no-curly-braces: prefix-$$an-envvar-value
+              sextuple-dollar: $$an-envvar-value-suffix
+              sextuple-dollar-no-curly-braces: prefix-$$an-envvar-value
+              septuple-dollar: $$$an-envvar-value-suffix
+              septuple-dollar-no-curly-braces: prefix-$$$an-envvar-value
+              octuple-dollar: $$$an-envvar-value-suffix
+              octuple-dollar-no-curly-braces: prefix-$$$an-envvar-value
+              nonuple-dollar: $$$$an-envvar-value-suffix
+              nonuple-dollar-no-curly-braces: prefix-$$$$an-envvar-value
+              decuple-dollar: $$$$an-envvar-value-suffix
+              decuple-dollar-no-curly-braces: prefix-$$$$an-envvar-value

tests/general/testdata/resource_metrics/envvar_labels.yaml

@@ -7,21 +7,22 @@ resource_metrics:
               state: used
               single-dollar-no-curly-braces: an-envvar-value-suffix
               single-dollar-curly-braces: an-envvar-value-suffix
-              double-dollar-no-curly-braces: an-envvar-value-suffix
-              double-dollar-curly-braces: an-envvar-value-suffix
-              triple-dollar-no-curly-braces: -envvar-value-suffix
-              triple-dollar-curly-braces: -envvar-value-suffix
-              quadruple-dollar-no-curly-braces: $AN_ENVVAR-suffix
-              quadruple-dollar-curly-braces: ${AN_ENVVAR}-suffix
-              quintuple-dollar-no-curly-braces: $an-envvar-value-suffix
-              quintuple-dollar-curly-braces: $an-envvar-value-suffix
-              sextuple-dollar-no-curly-braces: $an-envvar-value-suffix
-              sextuple-dollar-curly-braces: $an-envvar-value-suffix
-              septuple-dollar-no-curly-braces: $-envvar-value-suffix
-              septuple-dollar-curly-braces: $-envvar-value-suffix
-              octuple-dollar-no-curly-braces: $$AN_ENVVAR-suffix
-              octuple-dollar-curly-braces: $${AN_ENVVAR}-suffix
-              nonuple-dollar-no-curly-braces: $$an-envvar-value-suffix
-              nonuple-dollar-curly-braces: $$an-envvar-value-suffix
-              decuple-dollar-no-curly-braces: $$an-envvar-value-suffix
-              decuple-dollar-curly-braces: $$an-envvar-value-suffix
+              double-dollar-no-curly-braces: $AN_ENVVAR-suffix
+              double-dollar-curly-braces: ${AN_ENVVAR}-suffix
+              triple-dollar-no-curly-braces: $an-envvar-value-suffix
+              triple-dollar-curly-braces: $an-envvar-value-suffix
+              quadruple-dollar-no-curly-braces: $$AN_ENVVAR-suffix
+              quadruple-dollar-curly-braces: $${AN_ENVVAR}-suffix
+              quintuple-dollar-no-curly-braces: $$an-envvar-value-suffix
+              quintuple-dollar-curly-braces: $$an-envvar-value-suffix
+              sextuple-dollar-no-curly-braces: $$$AN_ENVVAR-suffix
+              sextuple-dollar-curly-braces: $$${AN_ENVVAR}-suffix
+              septuple-dollar-no-curly-braces: $$$an-envvar-value-suffix
+              septuple-dollar-curly-braces: $$$an-envvar-value-suffix
+              octuple-dollar-no-curly-braces: $$$$AN_ENVVAR-suffix
+              octuple-dollar-curly-braces: $$$${AN_ENVVAR}-suffix
+              nonuple-dollar-no-curly-braces: $$$$an-envvar-value-suffix
+              nonuple-dollar-curly-braces: $$$$an-envvar-value-suffix
+              decuple-dollar-no-curly-braces: $$$$$AN_ENVVAR-suffix
+              decuple-dollar-curly-braces: $$$$${AN_ENVVAR}-suffix
+