suggestions:

  - renamed featuregate
  - added test with a stringy collection
  - added changelog entry
Co-authored-by: Jade Guiton <[email protected]>

Author: theomagellan

.chloggen/fix-expandedvalue-sanitization-on-struct-collection.yaml

@@ -0,0 +1,25 @@
+# Use this changelog template to create an entry for release notes.
+
+# One of 'breaking', 'deprecation', 'new_component', 'enhancement', 'bug_fix'
+change_type: 'bug_fix'
+
+# The name of the component, or a single word describing the area of concern, (e.g. receiver/otlp)
+component: confmap
+
+# A brief description of the change.  Surround your text with quotes ("") if it needs to start with a backtick (`).
+note: 'fix ExpandedValue sanitization when parsed as non-string but assigned to string fields'
+
+# One or more tracking issues or pull requests related to the change
+issues: []
+
+# (Optional) One or more lines of additional information to render under the primary note.
+# These lines will be padded with 2 spaces and then inserted directly into the document.
+# Use pipe (|) for multiline entries.
+subtext:
+
+# Optional: The change log or logs in which this entry should be included.
+# e.g. '[user]' or '[user, api]'
+# Include 'user' if the change is relevant to end users.
+# Include 'api' if there is a change to a library API.
+# Default: '[user]'
+change_logs: []

confmap/internal/e2e/go.mod

@@ -8,6 +8,7 @@ require (
 	go.opentelemetry.io/collector/confmap v1.49.0
 	go.opentelemetry.io/collector/confmap/provider/envprovider v1.49.0
 	go.opentelemetry.io/collector/confmap/provider/fileprovider v1.49.0
+	go.opentelemetry.io/collector/featuregate v1.49.0
 )
 
 require (
@@ -22,7 +23,6 @@ require (
 	github.com/mitchellh/reflectwalk v1.0.2 // indirect
 	github.com/pmezard/go-difflib v1.0.0 // indirect
 	go.opentelemetry.io/collector/confmap/xconfmap v0.143.0 // indirect
-	go.opentelemetry.io/collector/featuregate v1.49.0 // indirect
 	go.uber.org/multierr v1.11.0 // indirect
 	go.uber.org/zap v1.27.1 // indirect
 	go.yaml.in/yaml/v3 v3.0.4 // indirect

confmap/internal/e2e/maplist_expanded_test.go

@@ -60,12 +60,12 @@ func TestMapListWithExpandedValueIntValue(t *testing.T) {
 		},
 	}
 
-	originalState := internal.DeferExpandedValueSanitizationOnStructCollection.IsEnabled()
+	originalState := internal.NewExpandedValueSanitizer.IsEnabled()
 	defer func() {
-		require.NoError(t, featuregate.GlobalRegistry().Set(internal.DeferExpandedValueSanitizationOnStructCollection.ID(), originalState))
+		require.NoError(t, featuregate.GlobalRegistry().Set(internal.NewExpandedValueSanitizer.ID(), originalState))
 	}()
 
-	require.NoError(t, featuregate.GlobalRegistry().Set(internal.DeferExpandedValueSanitizationOnStructCollection.ID(), true))
+	require.NoError(t, featuregate.GlobalRegistry().Set(internal.NewExpandedValueSanitizer.ID(), true))
 
 	conf := confmap.NewFromStringMap(data)
 	var tc testHeadersConfig
@@ -76,7 +76,7 @@ func TestMapListWithExpandedValueIntValue(t *testing.T) {
 	require.True(t, ok)
 	require.Equal(t, configopaque.String("8080"), val)
 
-	require.NoError(t, featuregate.GlobalRegistry().Set(internal.DeferExpandedValueSanitizationOnStructCollection.ID(), false))
+	require.NoError(t, featuregate.GlobalRegistry().Set(internal.NewExpandedValueSanitizer.ID(), false))
 
 	// This will fail because when reverting to old behavior, ExpandedValues get decoded at collection time and doesn't
 	// take struct collections into account.
@@ -105,3 +105,33 @@ func TestDirectConfigopaqueStringWithExpandedValueIntValue(t *testing.T) {
 	require.NoError(t, err)
 	require.Equal(t, configopaque.String("8080"), tc.Value)
 }
+
+// TestStringyStructureWithExpandedValue tests the isStringyStructure path in useExpandValue
+func TestStringyStructureWithExpandedValue(t *testing.T) {
+	type testConfig struct {
+		Tags []string `mapstructure:"tags"`
+	}
+
+	data := map[string]any{
+		"tags": []any{
+			internal.ExpandedValue{
+				Value:    8080,
+				Original: "8080",
+			},
+		},
+	}
+
+	originalState := internal.NewExpandedValueSanitizer.IsEnabled()
+	defer func() {
+		require.NoError(t, featuregate.GlobalRegistry().Set(internal.NewExpandedValueSanitizer.ID(), originalState))
+	}()
+
+	// With feature gate disabled, useExpandValue should detect []string as stringy
+	require.NoError(t, featuregate.GlobalRegistry().Set(internal.NewExpandedValueSanitizer.ID(), false))
+
+	conf := confmap.NewFromStringMap(data)
+	var tc testConfig
+	err := conf.Unmarshal(&tc)
+	require.NoError(t, err)
+	require.Equal(t, []string{"8080"}, tc.Tags)
+}

confmap/internal/featuregates.go

@@ -13,10 +13,10 @@ var EnableMergeAppendOption = featuregate.GlobalRegistry().MustRegister(
 	featuregate.WithRegisterReferenceURL("https://github.com/open-telemetry/opentelemetry-collector/issues/8754"),
 )
 
-var DeferExpandedValueSanitizationOnStructCollection = featuregate.GlobalRegistry().MustRegister(
-	"confmap.deferExpandedValueSanitizationOnStructCollection",
+var NewExpandedValueSanitizer = featuregate.GlobalRegistry().MustRegister(
+	"confmap.newExpandedValueSanitizer",
 	featuregate.StageBeta,
 	featuregate.WithRegisterFromVersion("v0.144.0"),
-	featuregate.WithRegisterDescription("Disables early sanitization of ExpandedValue during config unmarshalling, allowing mapstructure to handle type conversion at the field level. Fixes decoding errors when environment variable values are parsed as non-string types (e.g., numbers, booleans) but need to be assigned to string fields."),
-	featuregate.WithRegisterReferenceURL("https://github.com/open-telemetry/opentelemetry-collector/pull/14413#issuecomment-3754949484"),
+	featuregate.WithRegisterDescription("Fixes some types of decoding errors where environment variables are parsed as non-string types but assigned to string fields."),
+	featuregate.WithRegisterReferenceURL("https://github.com/open-telemetry/opentelemetry-collector/pull/14413"),
 )