[OTel] `--set` should merge with existing config

[pickypg]

I added a few tests to exemplify this scenario:

c652afc

But, imagine that you have a configuration that looks like:

elastic-agent/internal/pkg/otel/samples/linux/autoops_es.yml

Lines 1 to 23 in 278440a

	receivers:
	metricbeatreceiver:
	metricbeat:
	modules:
	# Metrics
	- module: autoops_es
	hosts: ${env:AUTOOPS_ES_URL}
	period: 10s
	metricsets:
	- cat_shards
	- cluster_health
	- cluster_settings
	- license
	- node_stats
	- tasks_management
	# Templates
	- module: autoops_es
	hosts: ${env:AUTOOPS_ES_URL}
	period: 24h
	metricsets:
	- cat_template
	- component_template
	- index_template

Ideally, we should be able to add ssl.verification_mode to "both" modules by a simply:

--set "receivers::metricbeatreceiver::metricbeat::modules=[{ ssl: { verification_mode: none } }, { ssl: { verification_mode: none } }]"

This is parsed, but it ultimately fails because it overwrites the existing modules. Separately, it seems like we should be able to manipulate the array directly:

--set "receivers::metricbeatreceiver::metricbeat::modules::0::ssl::verification_mode=none" \
--set "receivers::metricbeatreceiver::metricbeat::modules::1::ssl::verification_mode=none"

As a result, the only way to add or change settings within that array is to provide all settings at the same time:

--set 'receivers::metricbeatreceiver::metricbeat::modules=[{module: autoops_es, hosts: "${env:AUTOOPS_ES_URL}", period: 10s, metricsets: [cat_shards, cluster_health, cluster_settings, license, node_stats, tasks_management], ssl: { verification_mode: none } }, {module: autoops_es, hosts: "${env:AUTOOPS_ES_URL}", period: 24h, metricsets: [cat_template, component_template, index_template], ssl: { verification_mode: none } }]'

The way that --set is described, it seems like merging is the desired behavior rather than replacement. And, the way that I would also like to use it from the outside is to append. If I want to overwrite, I can change the relevant key(s).

[elasticmachine]

Pinging @elastic/elastic-agent-data-plane (Team:Elastic-Agent-Data-Plane)

[cmacknz]

I think this is probably an upstream collector problem, @VihasMakwana recently had an upstream RFC accepted to properly support merging list like you are describing and he's probably the best person to comment on the state of this.

• [chore][RFC] - Configuration Merging revamped open-telemetry/opentelemetry-collector#13256

[VihasMakwana]

@pickypg I have not personally tested my approach with the --set flag, but looking at the code of config merging, it should work.

[VihasMakwana]

here's the implementation PR: open-telemetry/opentelemetry-collector#13551

I expect to find some time today/tomorrow to mark it for review and hopefully, get it merged soon.

[pickypg]

Thanks @VihasMakwana. I've intentionally added tests that will no longer throw an error if the behavior changes to what you're implementing:

elastic-agent/internal/pkg/agent/cmd/validate_test.go

Lines 31 to 45 in e5704c1

	{
	"otel config with set missing action field",
	[]string{filepath.Join("testdata", "otel", "otel.yml"), "yaml:processors::resource::attributes: [{ key: service.name, value: elastic-otel-test2 }]"},
	true,
	},
	{
	"otel config with set missing key field",
	[]string{filepath.Join("testdata", "otel", "otel.yml"), "yaml:processors::resource::attributes: [{ action: insert, value: elastic-otel-test3 }]"},
	true,
	},
	{
	"otel config with set missing key and action fields",
	[]string{filepath.Join("testdata", "otel", "otel.yml"), "yaml:processors::resource::attributes: [{ value: elastic-otel-test4 }]"},
	true,
	},