Lightweight modules parses hosts twice

[sayden]

The Lightweight modules executes the WithHostParser twice. This is currently creating a bug when using a custom host parser like the DSN parsers that you can use in MySQL.

How to reproduce

• Add a custom host parser to the MySQL module, query metricset https://github.com/elastic/beats/blob/master/metricbeat/module/mysql/query/query.go#L39 that uses the HostParser defined here https://github.com/elastic/beats/blob/master/metricbeat/module/mysql/mysql.go#L56
• Activate performance metricset and execute and configure the host, for example with root:root@tcp(127.0.0.1:3306)/.
• Fails with:

Exiting: 1 error: host parser failed on light metricset factory for 'mysql/performance': error parsing mysql host: default addr for network 'root:/' unknown

What is happening is that it gets the config data from mysql.yml creates the uri and parses it. Then it mutates incoming config with the output of the parser. Finally, the parser is executed again with the mutated data and it fails.

Failling block is probably this one https://github.com/elastic/beats/blob/master/metricbeat/mb/lightmetricset.go#L86

It seems that the solution would be to avoid the second execution of the host parser.

Related issues

#16205

[elasticmachine]

Pinging @elastic/integrations-services (Team:Services)

[urso]

What is happening is that it gets the config data from mysql.yml creates the uri and parses it. Then it mutates incoming config with the output of the parser. Finally, the parser is executed again with the mutated data and it fails.

Why do we need to mutate the config object. We should try to never mutate the config object, as it is a source for potential bugs, race conditions and inconsistent configurations applied in Beats. Using the setters should be reserved for a few edge cases that need to create a config object only.
Can we store the parsed connection string in another, properly typed variable?

[jsoriano]

What is happening is that it gets the config data from mysql.yml creates the uri and parses it. Then it mutates incoming config with the output of the parser. Finally, the parser is executed again with the mutated data and it fails.

Why do we need to mutate the config object. We should try to never mutate the config object, as it is a source for potential bugs, race conditions and inconsistent configurations applied in Beats. Using the setters should be reserved for a few edge cases that need to create a config object only.

In light modules a new configuration is created with some initial default values, we then update this configuration with the user defined settings from the original base module, and this configuration is used to create a new base module, that is the one actually used at the end. Original base module or its configuration is not modified during this process.

Host parsers also don't mutate the config. The problem is that we use to call the host parser with the value of the host in the base module, and then we modify the host in the base module, so we lose the original string. Then if we make multiple calls to the host parser, we can have different results (as happens with light modules now).

Can we store the parsed connection string in another, properly typed variable?

The parsed host is already stored in a host data object, that is something like this. The problem is that from the information there we cannot know the original value of the host setting in case we want to call the host parser again.

I am exploring two options to fix this issue:

• Avoid modifying the host in the base module (Avoid modifications of base module host when building metricsets #20143), so we can use it to call the host parser multiple times, the problem with this change is that it affects all metricbeat (and auditbeat) modules and can have unexpected consequences.
• Call the host parser only "the second time" in light modules, by disabling the host parser in the registration object created for the metricset (Call host parser only once when building light metricsets #20149). This looks safer as would only affect light modules.