Enhance NodeExporterPath JSON option to pass additional node_exporter arguments

[jecassis]

Example from config.json:
"NodeExporterPath": "./node_exporter --collector.systemd"

This should result in the following process being launched when metrics are enabled:
./node_exporter --collector.systemd --web.listen-address=:9100 --web.telemetry-path=/metrics

As of now the changing the NodeExporterPath as above causes StartProcess to error and does not launch node_exporter.

[CLAassistant]

All committers have signed the CLA.

[tsachiherman]

The reasoning here seems valid, however, I would have the code so that if any of the parsed values contains web.listen-address or web.telemetry-path, we would omit the ones that we try to set.

[tsachiherman]

Moreover, since the code now generates a list of arguments that are going to be sent to the node_exporter process, I think it would be best if we've have a function that would parse the input argument ( i.e. NodeExporterPath ), and generate a list of argument that would be sent to node exporter.
Then, once we have this function and using it, we can create a series of unit tests that would validate that the passed parameters are constructed correctly in various test cases.

As an alternative, consider modifying the node_exporter process itself and add support for environment variable. The fact that the node_exporter doesn't have an environment variable that can be set to override the default parameters force all the derived applications to have "special" cases.

[jecassis]

Moreover, since the code now generates a list of arguments that are going to be sent to the node_exporter process, I think it would be best if we've have a function that would parse the input argument ( i.e. NodeExporterPath ), and generate a list of argument that would be sent to node exporter.
Then, once we have this function and using it, we can create a series of unit tests that would validate that the passed parameters are constructed correctly in various test cases.

I think this the direction I'd probably explore with a filter for the existing ones.

As an alternative, consider modifying the node_exporter process itself and add support for environment variable. The fact that the node_exporter doesn't have an environment variable that can be set to override the default parameters force all the derived applications to have "special" cases.

My suspicion is that doing so adds an additional level of obfuscation/abstraction and in addition one could also make the same request to algod.

[Karmastic]

This seems good to me.

[tsachiherman]

Units tests and updated functionality looks great.
Just one small request :
Within the unit test, can you use the require package instead of using the t.Errorf directly ?
When adding the require package, please place it in a separate line-separated “group”, like in the rest of the unit tests.

[jecassis]

Much cleaner to use the require package. Had never used it before :) The changes are in flight. And the separate line grouping is enforced by gofmt, so it goes without saying.

[tsachiherman]

Looks great @jecassis, thank you for your contribution !