[otelcol] Preserve internal representation for components' configurat…

…ions (#10897)

#### Description

The custom unmarshalling for components copied a map without preserving
the internal representation. This led to the issues mentioned on #10552
not being fully fixed (they were only fixed if they happened in the
`service::telemetry` section for example).

#### Link to tracking issue
Fixes issues mentioned on #10552

#### Testing

This adds one unit test at the `otelcol` level. Since we didn't catch
this with our current `confmap/internal/e2e` tests, we likely also want
to refactor those.

---------

Co-authored-by: Dmitrii Anoshin <[email protected]>
Co-authored-by: Curtis Robert <[email protected]>
Co-authored-by: Alex Boten <[email protected]>

.chloggen/mx-psi_preserve-internal-repr.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. otlpreceiver)
+component: otelcol
+
+# A brief description of the change.  Surround your text with quotes ("") if it needs to start with a backtick (`).
+note: Preserve internal representation when unmarshaling component configs
+
+# One or more tracking issues or pull requests related to the change
+issues: [10552]
+
+# (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: []

otelcol/internal/configunmarshaler/configs.go

@@ -31,19 +31,25 @@ func (c *Configs[F]) Unmarshal(conf *confmap.Conf) error {
 	// Prepare resulting map.
 	c.cfgs = make(map[component.ID]component.Config)
 	// Iterate over raw configs and create a config for each.
-	for id, value := range rawCfgs {
+	for id := range rawCfgs {
 		// Find factory based on component kind and type that we read from config source.
 		factory, ok := c.factories[id.Type()]
 		if !ok {
 			return errorUnknownType(id, maps.Keys(c.factories))
 		}
 
+		// Get the configuration from the confmap.Conf to preserve internal representation.
+		sub, err := conf.Sub(id.String())
+		if err != nil {
+			return errorUnmarshalError(id, err)
+		}
+
 		// Create the default config for this component.
 		cfg := factory.CreateDefaultConfig()
 
 		// Now that the default config struct is created we can Unmarshal into it,
 		// and it will apply user-defined config on top of the default.
-		if err := confmap.NewFromStringMap(value).Unmarshal(&cfg); err != nil {
+		if err := sub.Unmarshal(&cfg); err != nil {
 			return errorUnmarshalError(id, err)
 		}
 

otelcol/unmarshal_dry_run_test.go

@@ -0,0 +1,115 @@
+// Copyright The OpenTelemetry Authors
+// SPDX-License-Identifier: Apache-2.0
+
+package otelcol
+
+import (
+	"context"
+	"testing"
+
+	"github.com/stretchr/testify/require"
+
+	"go.opentelemetry.io/collector/component"
+	"go.opentelemetry.io/collector/confmap"
+	"go.opentelemetry.io/collector/receiver"
+)
+
+var _ component.Config = (*Config)(nil)
+
+type ValidateTestConfig struct {
+	Number int    `mapstructure:"number"`
+	String string `mapstructure:"string"`
+}
+
+var genericType component.Type = component.MustNewType("generic")
+
+func NewFactories(_ *testing.T) func() (Factories, error) {
+	return func() (Factories, error) {
+		factories, err := nopFactories()
+		if err != nil {
+			return Factories{}, err
+		}
+		factories.Receivers[genericType] = receiver.NewFactory(
+			genericType,
+			func() component.Config {
+				return &ValidateTestConfig{
+					Number: 1,
+					String: "default",
+				}
+			})
+
+		return factories, nil
+	}
+}
+
+var sampleYAMLConfig = `
+receivers:
+  generic:
+    number: ${mock:number}
+    string: ${mock:number}
+
+exporters:
+  nop:
+
+service:
+  pipelines:
+    traces:
+      receivers: [generic]
+      exporters: [nop]
+`
+
+func TestDryRunWithExpandedValues(t *testing.T) {
+	tests := []struct {
+		name       string
+		yamlConfig string
+		mockMap    map[string]string
+		expectErr  bool
+	}{
+		{
+			name:       "string that looks like an integer",
+			yamlConfig: sampleYAMLConfig,
+			mockMap: map[string]string{
+				"number": "123",
+			},
+		},
+		{
+			name:       "string that looks like a bool",
+			yamlConfig: sampleYAMLConfig,
+			mockMap: map[string]string{
+				"number": "true",
+			},
+			expectErr: true,
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			collector, err := NewCollector(CollectorSettings{
+				Factories: NewFactories(t),
+				ConfigProviderSettings: ConfigProviderSettings{
+					ResolverSettings: confmap.ResolverSettings{
+						URIs:          []string{"file:file"},
+						DefaultScheme: "mock",
+						ProviderFactories: []confmap.ProviderFactory{
+							newFakeProvider("mock", func(_ context.Context, uri string, _ confmap.WatcherFunc) (*confmap.Retrieved, error) {
+								return confmap.NewRetrievedFromYAML([]byte(tt.mockMap[uri[len("mock:"):]]))
+							}),
+							newFakeProvider("file", func(_ context.Context, _ string, _ confmap.WatcherFunc) (*confmap.Retrieved, error) {
+								return confmap.NewRetrievedFromYAML([]byte(tt.yamlConfig))
+							}),
+						},
+					},
+				},
+				SkipSettingGRPCLogger: true,
+			})
+			require.NoError(t, err)
+
+			err = collector.DryRun(context.Background())
+			if tt.expectErr {
+				require.Error(t, err)
+				return
+			}
+			require.NoError(t, err)
+		})
+	}
+}