ansible-node-exporter
ansible-node-exporter copied to clipboard
cloudalchemy
Pass arbitrary command line flags to node_exporter
What is missing? A way to pass arbitrary command line flags to node_exporter
Why do we need it? node_exporter has more parameters than those exposed by the variables in this role. An "escape hatch" to pass any flags would be very useful. Something like:
node_exporter_flags: ["--web.telemetry-path=/foo", "--web.max-requests=99"]
In fact, with this variable in place, node_exporter_web_listen_address could be deprecated IMO as it would be adequately covered by the more general node_exporter_flags.
Whole idea behind having one variable per flag is to have a mechanism which quickly prevents misconfiguration of node_exporter before ansible even gets to install stage. This is covered in tasks/prefilight.yml. If there are mappings which are missing I will happily accept PRs with new additions.
That said I agree about deprecating node_exporter_web_listen_address variable, but the replacement should probably follow pattern done also for collectors. Which in this case would look something like:
node_exporter_web:
listen-address: <<sth>>
max-requests: <<sth>>
telemetry-path: <<sth>>
However this would need to be done in backward-compatible way, to still allow node_exporter_web_listen_address for few releases.
/cc @SuperQ wdyt?
That would work too, though it's higher maintenance if more flags are added in the future.
I'll note that node_exporter refuses to start with incorrect/unknown flags, so there is no risk of a mistake going unnoticed.
I'll note that node_exporter refuses to start with incorrect/unknown flags, so there is no risk of a mistake going unnoticed.
True, but the current mechanism is more about upgrades than the installation itself. A simple scenario which role wouldn't be able to prevent with free-form flags option is this:
- node_exporter is already installed
- user sets incorrect flags and runs ansible role
- systemd service is updated and restarted
- node_exporter is in crash-loop
Above causes downtime. In contrast, failing fast in preflight.yml doesn't update anything on host and prevents incorrect node configuration.
The problem with node_exporter_web is Ansible. Since it doesn't have the concept of attribute merge, you can't have a change to telemetry-path without also having to include the other settings as well. Sometimes you want to have one flag setting at one level of group var, and another setting in another group var or host var.
Adding support for the new node_exporter flags is probably what we want.
Ah, that's problem that I hadn't considered, and also applies to my original idea.
In that case having separate variables for each flag is the only way to go. Consider this enhancement request officially changed :)
In that case having separate variables for each flag is the only way to go. Consider this enhancement request officially changed :)
PRs for new variables are welcome :)
@ritwiksin :
Since this is a collector, you can use similar syntax to textfile collector example in https://github.com/cloudalchemy/ansible-node-exporter/blob/0460c128f8d861d6aa28df83fb15226dfec54269/defaults/main.yml#L14-L17
In your case it will be:
node_exporter_enabled_collectors:
- supervisord:
url: <SOME_URL>
Adding support for the new node_exporter flags is probably what we want.
In that case having separate variables for each flag is the only way to go. Consider this enhancement request officially changed :)
@paol which flags would like to be included? Can you create a PR?
All of them? I mean, what I wanted to do when I originally filed this bug was to pass --web.disable-exporter-metrics, but in principle all flags should be settable.
The ones currently missing seem to be: --web.telemetry-path --web.disable-exporter-metrics --web.max-requests --log.level --log.format
Re: PR, I don't have the time to pick this up right now, sorry.
--web.telemetry-path is added in https://github.com/cloudalchemy/ansible-node-exporter/pull/196