[confmap] - new feature flag for append merging strategy (#12097)

Koanf's default merging strategy currently overrides static values such
as slices, numbers, and strings. However, lists of components should be
treated as a special case. This pull request introduces a new command
line option to allow for merging lists instead of discarding the
existing ones.

With this new merging strategy:
- All the lists are merged rather than replaced.
- The merging logic is name-aware, meaning that if components with the
same name appear in both lists, they will only appear once in the final
merged list.

<!-- Issue number if applicable -->
#### Link to tracking issue
Related issues:
- #8754
- #8394
- #10370

<!--Describe what testing was performed and which tests were added.-->
#### Testing
- Added

<!--Describe the documentation added.-->
#### Documentation
- Added under readme.


##### Example
Consider the following configs,

```yaml
# main.yaml
receivers:
  otlp/in:
processors:
  batch:
exporters:
  otlp/out:
extensions:
  file_storage:

service:
  pipelines:
    traces:
      receivers: [ otlp/in ]
      processors: [ batch ]
      exporters: [ otlp/out ]
  extensions: [ file_storage ]
```


```yaml
# extra_extension.yaml
extensions:
  healthcheckv2:

service:
  extensions: [ healthcheckv2 ]
```

If you run the collector with following command,
```
otelcol --config=main.yaml --config=extra_extension.yaml --feature-gates=-confmap.enableMergeAppendOption
```
then the final configuration after config resolution will look like
following:

```yaml
# main.yaml
receivers:
  otlp/in:
processors:
  batch:
exporters:
  otlp/out:
extensions:
  file_storage:
  healthcheckv2:

service:
  pipelines:
    traces:
      receivers: [ otlp/in ]
      processors: [ batch ]
      exporters: [ otlp/out ]
  extensions: [ file_storage, healthcheckv2 ]
```

For backward compatibly, the default behaviour is **not** to merge
lists. Users who want to explicitly merge lists can enable the command
line option.

Note: I’d appreciate your feedback on this 🙏

Author: VihasMakwana

.chloggen/new-merge-strategy.yaml

@@ -0,0 +1,27 @@
+# Use this changelog template to create an entry for release notes.
+
+# One of 'breaking', 'deprecation', 'new_component', 'enhancement', 'bug_fix'
+change_type: enhancement
+
+# The name of the component, or a single word describing the area of concern, (e.g. otlpreceiver)
+component: confmap
+
+# A brief description of the change.  Surround your text with quotes ("") if it needs to start with a backtick (`).
+note: Introduce a new feature flag to allow for merging lists instead of discarding the existing ones.
+
+# One or more tracking issues or pull requests related to the change
+issues: [8394, 8754, 10370]
+
+# (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: |
+  You can enable this option via the command line by running following command:
+  otelcol --config=main.yaml --config=extra_config.yaml --feature-gates=-confmap.enableMergeAppendOption
+
+# 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: []

cmd/mdatagen/go.mod

@@ -36,6 +36,7 @@ require (
 	github.com/go-viper/mapstructure/v2 v2.2.1 // indirect
 	github.com/gogo/protobuf v1.3.2 // indirect
 	github.com/google/uuid v1.6.0 // indirect
+	github.com/hashicorp/go-version v1.7.0 // indirect
 	github.com/inconshreveable/mousetrap v1.1.0 // indirect
 	github.com/json-iterator/go v1.1.12 // indirect
 	github.com/knadh/koanf/maps v0.1.1 // indirect
@@ -51,6 +52,7 @@ require (
 	go.opentelemetry.io/collector/component/componentstatus v0.121.0 // indirect
 	go.opentelemetry.io/collector/consumer/consumererror v0.121.0 // indirect
 	go.opentelemetry.io/collector/consumer/xconsumer v0.121.0 // indirect
+	go.opentelemetry.io/collector/featuregate v1.26.0 // indirect
 	go.opentelemetry.io/collector/pdata/pprofile v0.121.0 // indirect
 	go.opentelemetry.io/collector/pdata/testdata v0.121.0 // indirect
 	go.opentelemetry.io/collector/pipeline v0.121.0 // indirect
@@ -118,3 +120,5 @@ replace go.opentelemetry.io/collector/consumer/consumererror => ../../consumer/c
 replace go.opentelemetry.io/collector/scraper => ../../scraper
 
 replace go.opentelemetry.io/collector/scraper/scrapertest => ../../scraper/scrapertest
+
+replace go.opentelemetry.io/collector/featuregate => ../../featuregate

cmd/mdatagen/go.sum

@@ -93,6 +93,91 @@ The `Resolve` method proceeds in the following steps:
 4. For each "Converter", call "Convert" for the "result".
 5. Return the "result", aka effective, configuration.
 
+#### (Experimental) Append merging strategy for lists
+
+You can opt-in to experimentally combine slices instead of discarding the existing ones by enabling the `confmap.enableMergeAppendOption` feature flag. Lists are appended in the order in which they appear in their configuration sources.
+This will **not** become the default in the future, we are still deciding how this should be configured and want your feedback on [this issue](https://github.com/open-telemetry/opentelemetry-collector/issues/8754).
+
+##### Example
+Consider the following configs,
+
+```yaml
+# main.yaml
+receivers:
+  otlp/in:
+processors:
+  attributes/example:
+    actions:
+      - key: key
+        value: "value"
+        action: upsert
+
+exporters:
+  otlp/out:
+extensions:
+  file_storage:
+
+service:
+  pipelines:
+    traces:
+      receivers: [ otlp/in ]
+      processors: [ attributes/example ]
+      exporters: [ otlp/out ]
+  extensions: [ file_storage ]
+```
+
+
+```yaml
+# extra_extension.yaml
+processors:
+  batch:
+extensions:
+  healthcheckv2:
+
+service:
+  extensions: [ healthcheckv2 ]
+  pipelines:
+    traces:
+      processors: [ batch ]
+```
+
+If you run the Collector with following command,
+```
+otelcol --config=main.yaml --config=extra_extension.yaml --feature-gates=confmap.enableMergeAppendOption
+```
+then the final configuration after config resolution will look like following:
+
+```yaml
+# main.yaml
+receivers:
+  otlp/in:
+processors:
+  attributes/example:
+    actions:
+      - key: key
+        value: "value"
+        action: upsert
+  batch:
+exporters:
+  otlp/out:
+extensions:
+  file_storage:
+  healthcheckv2:
+
+service:
+  pipelines:
+    traces:
+      receivers: [ otlp/in ]
+      processors: [ attributes/example, batch ]
+      exporters: [ otlp/out ]
+  extensions: [ file_storage, healthcheckv2 ]
+```
+
+Notice that the `service::extensions` list is a combination of both configurations. By default, the value of the last configuration source passed, `extra_extension`, would be used, so the extensions list would be: `service::extensions: [healthcheckv2]`.
+
+> [!NOTE]
+> By enabling this feature gate, all the lists in the given configuration will be merged. 
+
 ### Watching for Updates
 After the configuration was processed, the `Resolver` can be used as a single point to watch for updates in the
 configuration retrieved via the `Provider` used to retrieve the “initial” configuration and to generate the “effective” one.

confmap/README.md

@@ -171,6 +171,15 @@ func (l *Conf) Merge(in *Conf) error {
 	return l.k.Merge(in.k)
 }
 
+// mergeAppend merges the input given configuration into the existing config.
+// Note that the given map may be modified.
+// Additionally, mergeAppend performs deduplication when merging lists.
+// For example, if listA = [extension1, extension2] and listB = [extension1, extension3],
+// the resulting list will be [extension1, extension2, extension3].
+func (l *Conf) mergeAppend(in *Conf) error {
+	return l.k.Load(confmap.Provider(in.ToStringMap(), ""), nil, koanf.WithMergeFunc(mergeAppend))
+}
+
 // Sub returns new Conf instance representing a sub-config of this instance.
 // It returns an error is the sub-config is not a map[string]any (use Get()), and an empty Map if none exists.
 func (l *Conf) Sub(key string) (*Conf, error) {

confmap/confmap.go

@@ -8,6 +8,7 @@ require (
 	github.com/knadh/koanf/providers/confmap v0.1.0
 	github.com/knadh/koanf/v2 v2.1.2
 	github.com/stretchr/testify v1.10.0
+	go.opentelemetry.io/collector/featuregate v1.25.0
 	go.uber.org/goleak v1.3.0
 	go.uber.org/multierr v1.11.0
 	go.uber.org/zap v1.27.0
@@ -16,15 +17,15 @@ require (
 
 require (
 	github.com/davecgh/go-spew v1.1.1 // indirect
-	github.com/kr/pretty v0.3.1 // indirect
+	github.com/hashicorp/go-version v1.7.0 // indirect
 	github.com/mitchellh/copystructure v1.2.0 // indirect
 	github.com/mitchellh/reflectwalk v1.0.2 // indirect
 	github.com/pmezard/go-difflib v1.0.0 // indirect
-	github.com/rogpeppe/go-internal v1.10.0 // indirect
-	gopkg.in/check.v1 v1.0.0-20201130134442-10cb98267c6c // indirect
 )
 
 retract (
 	v0.76.0 // Depends on retracted pdata v1.0.0-rc10 module, use v0.76.1
 	v0.69.0 // Release failed, use v0.69.1
 )
+
+replace go.opentelemetry.io/collector/featuregate => ../featuregate

confmap/go.mod

@@ -13,12 +13,14 @@ require (
 require (
 	github.com/davecgh/go-spew v1.1.1 // indirect
 	github.com/go-viper/mapstructure/v2 v2.2.1 // indirect
+	github.com/hashicorp/go-version v1.7.0 // indirect
 	github.com/knadh/koanf/maps v0.1.1 // indirect
 	github.com/knadh/koanf/providers/confmap v0.1.0 // indirect
 	github.com/knadh/koanf/v2 v2.1.2 // indirect
 	github.com/mitchellh/copystructure v1.2.0 // indirect
 	github.com/mitchellh/reflectwalk v1.0.2 // indirect
 	github.com/pmezard/go-difflib v1.0.0 // indirect
+	go.opentelemetry.io/collector/featuregate v1.25.0 // indirect
 	go.uber.org/multierr v1.11.0 // indirect
 	go.uber.org/zap v1.27.0 // indirect
 	gopkg.in/yaml.v3 v3.0.1 // indirect
@@ -31,3 +33,5 @@ replace go.opentelemetry.io/collector/confmap/provider/fileprovider => ../../pro
 replace go.opentelemetry.io/collector/confmap/provider/envprovider => ../../provider/envprovider
 
 replace go.opentelemetry.io/collector/config/configopaque => ../../../config/configopaque
+
+replace go.opentelemetry.io/collector/featuregate => ../../../featuregate

confmap/go.sum

@@ -0,0 +1,71 @@
+// Copyright The OpenTelemetry Authors
+// SPDX-License-Identifier: Apache-2.0
+
+package confmap // import "go.opentelemetry.io/collector/confmap"
+
+import (
+	"reflect"
+)
+
+func mergeAppend(src, dest map[string]any) error {
+	// mergeAppend recursively merges the src map into the dest map (left to right),
+	// modifying and expanding the dest map in the process.
+	// This function does not overwrite lists, and ensures that the final value is a name-aware
+	// copy of lists from src and dest.
+
+	for sKey, sVal := range src {
+		dVal, dOk := dest[sKey]
+		if !dOk {
+			// key is not present in destination config. Hence, add it to destination map
+			dest[sKey] = sVal
+			continue
+		}
+
+		srcVal := reflect.ValueOf(sVal)
+		destVal := reflect.ValueOf(dVal)
+
+		if destVal.Kind() != srcVal.Kind() {
+			// different kinds. Override the destination map
+			dest[sKey] = sVal
+			continue
+		}
+
+		switch srcVal.Kind() {
+		case reflect.Array, reflect.Slice:
+			// both of them are array. Merge them
+			dest[sKey] = mergeSlice(srcVal, destVal)
+		case reflect.Map:
+			// both of them are maps. Recursively call the mergeAppend
+			_ = mergeAppend(sVal.(map[string]any), dVal.(map[string]any))
+		default:
+			// any other datatype. Override the destination map
+			dest[sKey] = sVal
+		}
+	}
+
+	return nil
+}
+
+func mergeSlice(src, dest reflect.Value) any {
+	slice := reflect.MakeSlice(src.Type(), 0, src.Cap()+dest.Cap())
+	for i := 0; i < dest.Len(); i++ {
+		slice = reflect.Append(slice, dest.Index(i))
+	}
+
+	for i := 0; i < src.Len(); i++ {
+		if isPresent(slice, src.Index(i)) {
+			continue
+		}
+		slice = reflect.Append(slice, src.Index(i))
+	}
+	return slice.Interface()
+}
+
+func isPresent(slice reflect.Value, val reflect.Value) bool {
+	for i := 0; i < slice.Len(); i++ {
+		if slice.Index(i).Equal(val) {
+			return true
+		}
+	}
+	return false
+}

confmap/internal/e2e/go.mod

@@ -12,14 +12,18 @@ require (
 require (
 	github.com/davecgh/go-spew v1.1.1 // indirect
 	github.com/go-viper/mapstructure/v2 v2.2.1 // indirect
+	github.com/hashicorp/go-version v1.7.0 // indirect
 	github.com/knadh/koanf/maps v0.1.1 // indirect
 	github.com/knadh/koanf/providers/confmap v0.1.0 // indirect
 	github.com/knadh/koanf/v2 v2.1.2 // indirect
 	github.com/mitchellh/copystructure v1.2.0 // indirect
 	github.com/mitchellh/reflectwalk v1.0.2 // indirect
 	github.com/pmezard/go-difflib v1.0.0 // indirect
+	go.opentelemetry.io/collector/featuregate v1.25.0 // indirect
 	go.uber.org/multierr v1.11.0 // indirect
 	gopkg.in/yaml.v3 v3.0.1 // indirect
 )
 
 replace go.opentelemetry.io/collector/confmap => ../../
+
+replace go.opentelemetry.io/collector/featuregate => ../../../featuregate

confmap/internal/e2e/go.sum

@@ -11,15 +11,19 @@ require (
 require (
 	github.com/davecgh/go-spew v1.1.1 // indirect
 	github.com/go-viper/mapstructure/v2 v2.2.1 // indirect
+	github.com/hashicorp/go-version v1.7.0 // indirect
 	github.com/knadh/koanf/maps v0.1.1 // indirect
 	github.com/knadh/koanf/providers/confmap v0.1.0 // indirect
 	github.com/knadh/koanf/v2 v2.1.2 // indirect
 	github.com/mitchellh/copystructure v1.2.0 // indirect
 	github.com/mitchellh/reflectwalk v1.0.2 // indirect
 	github.com/pmezard/go-difflib v1.0.0 // indirect
+	go.opentelemetry.io/collector/featuregate v1.25.0 // indirect
 	go.uber.org/multierr v1.11.0 // indirect
 	go.uber.org/zap v1.27.0 // indirect
 	gopkg.in/yaml.v3 v3.0.1 // indirect
 )
 
 replace go.opentelemetry.io/collector/confmap => ../../
+
+replace go.opentelemetry.io/collector/featuregate => ../../../featuregate