Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Modernize exporter-toolkit flag parsing #162

Merged
merged 5 commits into from
Jun 26, 2023

Conversation

dswarbrick
Copy link
Contributor

Use modern method of parsing listener address(es) and TLS config file, provided by prometheus/exporter-toolkit.

Refs: #148

Note that this introduces a breaking change.
The following flags:

      --telemetry.address=":9117"  
                           Address on which to expose metrics.
      --web.config=""      Path to config yaml file that can enable TLS or authentication.

will change to:

      --web.listen-address=:9117 ...  
                                 Addresses on which to expose metrics and web interface. Repeatable for multiple addresses.
      --web.config.file=""       [EXPERIMENTAL] Path to configuration file that can enable TLS or authentication.

Additionally, the systemd socket flag will be offered (on Linux):

      --[no-]web.systemd-socket  Use systemd socket activation listeners instead of port listeners (Linux only).

Signed-off-by: Daniel Swarbrick <daniel.swarbrick@gmail.com>
Signed-off-by: Daniel Swarbrick <daniel.swarbrick@gmail.com>
@blkperl
Copy link
Collaborator

blkperl commented May 30, 2023

@Lusitaniae do you have any thoughts on this change?

@Lusitaniae
Copy link
Owner

in node exporter looks like the toolkit can load the flags directly

prometheus/node_exporter@440a132#diff-7907251477470326df65250bf7fa699e0756ce5d6282e27b34c112132a7ef6b5R162

Also seems we can use it to generate the landing page too
https://github.com/prometheus/node_exporter/pull/2622/files

Maybe need more work here?

@dswarbrick
Copy link
Contributor Author

dswarbrick commented May 31, 2023

@Lusitaniae I'm not sure what you mean by "load the flags directly" or how the node_exporter code differs to what I've proposed - other than the toolkitFlags var being global in this PR, simply because that's where all your other flag vars are.

The landing page functionality in exporter-toolkit is new, and was not available when I opened this PR. Personally, I would recommend keeping that for a separate PR.

@Lusitaniae
Copy link
Owner

Sounds fair

I'll leave it to @blkperl

@roidelapluie
Copy link
Collaborator

This LGTM

@Lusitaniae Lusitaniae merged commit 2452944 into Lusitaniae:master Jun 26, 2023
7 checks passed
@Lusitaniae
Copy link
Owner

Many thanks

Merged and released in https://github.com/Lusitaniae/apache_exporter/releases/tag/v1.0.0

if you'd like to test

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants