diff --git a/CHANGELOG.md b/CHANGELOG.md index ef0e278a8a..7ec31b153c 100644 --- a/CHANGELOG.md +++ b/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. diff --git a/internal/configprovider/manager.go b/internal/configprovider/manager.go index 62827f608b..27d240e1e0 100644 --- a/internal/configprovider/manager.go +++ b/internal/configprovider/manager.go @@ -20,6 +20,7 @@ import ( "fmt" "net/url" "os" + "strconv" "strings" "sync" @@ -42,6 +43,8 @@ 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 @@ -49,6 +52,13 @@ 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) { diff --git a/internal/configprovider/manager_test.go b/internal/configprovider/manager_test.go index d59322891a..4f39a2b141 100644 --- a/internal/configprovider/manager_test.go +++ b/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) }) } diff --git a/internal/configprovider/parser.go b/internal/configprovider/parser.go index c6fcfc4641..dcf2962dfd 100644 --- a/internal/configprovider/parser.go +++ b/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) }() diff --git a/internal/configprovider/testdata/envvar_cfgsrc_mix.yaml b/internal/configprovider/testdata/envvar_cfgsrc_mix.yaml index 6560b52de0..134a76724f 100644 --- a/internal/configprovider/testdata/envvar_cfgsrc_mix.yaml +++ b/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 diff --git a/internal/configprovider/testdata/envvar_cfgsrc_mix_expected.yaml b/internal/configprovider/testdata/envvar_cfgsrc_mix_expected.yaml index e9e9ba862c..002ceb7b38 100644 --- a/internal/configprovider/testdata/envvar_cfgsrc_mix_expected.yaml +++ b/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 diff --git a/tests/general/envvar_expansion_test.go b/tests/general/envvar_expansion_test.go index afbc65ed34..2407344253 100644 --- a/tests/general/envvar_expansion_test.go +++ b/tests/general/envvar_expansion_test.go @@ -57,7 +57,6 @@ func TestExpandedDollarSignsViaStandardEnvVar(t *testing.T) { require.NoError(t, tc.OTLPMetricsReceiverSink.AssertAllMetricsReceived(t, *expectedResourceMetrics, 30*time.Second)) } - func TestExpandedDollarSignsViaEnvConfigSource(t *testing.T) { tc := testutils.NewTestcase(t) defer tc.PrintLogsOnFailure() @@ -88,3 +87,35 @@ 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() + + image := os.Getenv("SPLUNK_OTEL_COLLECTOR_IMAGE") + if strings.TrimSpace(image) == "" { + t.Skipf("skipping container-only test") + } + + hostMetricsEnvVars := map[string]string{ + "OTLP_ENDPOINT": tc.OTLPEndpoint, + "SPLUNK_DOUBLE_DOLLAR_CONFIG_SOURCE_COMPATIBLE": "false", + "SPLUNK_TEST_ID": tc.ID, + "AN_ENVVAR": "an-envvar-value"} + + collector, err := testutils.NewCollectorContainer(). + WithImage(image). + WithEnv(hostMetricsEnvVars). + WithConfigPath(path.Join(".", "testdata", "env_config_source_labels.yaml")). + WithLogger(tc.Logger). + Build() + + require.NoError(t, err) + require.NotNil(t, collector) + require.NoError(t, collector.Start()) + defer func() { require.NoError(t, collector.Shutdown()) }() + + expectedResourceMetrics := tc.ResourceMetrics("incompat_env_config_source_labels.yaml") + require.NoError(t, tc.OTLPMetricsReceiverSink.AssertAllMetricsReceived(t, *expectedResourceMetrics, 30*time.Second)) +} diff --git a/tests/general/testdata/resource_metrics/env_config_source_labels.yaml b/tests/general/testdata/resource_metrics/env_config_source_labels.yaml index 6bddf4dc88..6f8b0df03e 100644 --- a/tests/general/testdata/resource_metrics/env_config_source_labels.yaml +++ b/tests/general/testdata/resource_metrics/env_config_source_labels.yaml @@ -5,13 +5,13 @@ resource_metrics: type: IntGauge labels: state: used - single-dollar: -suffix + single-dollar: an-envvar-value-suffix 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 + triple-dollar: $an-envvar-value-suffix + quadruple-dollar: $an-envvar-value-suffix + quintuple-dollar: $$an-envvar-value-suffix + sextuple-dollar: $$an-envvar-value-suffix + septuple-dollar: $$$an-envvar-value-suffix + octuple-dollar: $$$an-envvar-value-suffix + nonuple-dollar: $$$$an-envvar-value-suffix + decuple-dollar: $$$$an-envvar-value-suffix diff --git a/tests/general/testdata/resource_metrics/envvar_labels.yaml b/tests/general/testdata/resource_metrics/envvar_labels.yaml index 09b3bd8141..ba83fa96ba 100644 --- a/tests/general/testdata/resource_metrics/envvar_labels.yaml +++ b/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 + diff --git a/tests/general/testdata/resource_metrics/incompat_env_config_source_labels.yaml b/tests/general/testdata/resource_metrics/incompat_env_config_source_labels.yaml new file mode 100644 index 0000000000..ede8a35dce --- /dev/null +++ b/tests/general/testdata/resource_metrics/incompat_env_config_source_labels.yaml @@ -0,0 +1,17 @@ +resource_metrics: + - instrumentation_library_metrics: + - metrics: + - name: system.memory.usage + type: IntGauge + labels: + state: used + single-dollar: an-envvar-value-suffix + double-dollar: ${env:AN_ENVVAR}-suffix + triple-dollar: $an-envvar-value-suffix + quadruple-dollar: $${env:AN_ENVVAR}-suffix + quintuple-dollar: $$an-envvar-value-suffix + sextuple-dollar: $$${env:AN_ENVVAR}-suffix + septuple-dollar: $$$an-envvar-value-suffix + octuple-dollar: $$$${env:AN_ENVVAR}-suffix + nonuple-dollar: $$$$an-envvar-value-suffix + decuple-dollar: $$$$${env:AN_ENVVAR}-suffix