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

Compatibily with new flag web.listen-address #1096

Merged
merged 1 commit into from
Nov 23, 2022
Merged

Compatibily with new flag web.listen-address #1096

merged 1 commit into from
Nov 23, 2022

Conversation

bck01215
Copy link
Contributor

Signed-off-by: Brandon Kauffman bck01215@gmail.com

Signed-off-by: Brandon Kauffman <bck01215@gmail.com>
@bck01215 bck01215 requested a review from a team as a code owner November 19, 2022 01:02
Copy link
Contributor

@breed808 breed808 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for updating this, I must have missed it in e9c5944

@breed808 breed808 merged commit 53b6816 into prometheus-community:master Nov 23, 2022
@JDA88
Copy link
Contributor

JDA88 commented Feb 22, 2023

I agree this change is necessary but as it’s a breaking change, can we have both parameter working at least until the 0.22.* to ease the transition for organization using it?

@breed808
Copy link
Contributor

There's no easy way to have both options working correctly, as the exporter toolkit we rely on requires the new flag.

The v0.21.0 release could be pinned to 63fb570, but would miss out on most of the recent changes & fixes.

There is the option of continuing to run v0.20.0 until you are ready for v0.21.0 or v0.22.0.

@JDA88
Copy link
Contributor

JDA88 commented Feb 25, 2023

Ok, I understand.
I didn't have the opportunity to test is but what would append if you set both flags in 0.20 and 0.21? If the additional flag is ignored that would be perfect.
The issue for us is to find a way to ease the migration and it's currently a case our deployment scripts do not handle.

@breed808
Copy link
Contributor

Technically both flags could be included, but from memory the only the new flag would be properly used by the exporter, due to the change in behavior for the exporter toolkit.

@JDA88
Copy link
Contributor

JDA88 commented Feb 26, 2023

I'll try to test this. It should ease our migration path.

@JDA88
Copy link
Contributor

JDA88 commented Mar 1, 2023

OK after some test you can't use both flag at the same time, you get the error message:
windows_exporter-0.21.0-amd64.exe: error: unknown long flag '--telemetry.addr', try --help

We will adjust our deployement script to check for the version of the binary and change the command line in consequence

anubhavg-icpl pushed a commit to anubhavg-icpl/windows_exporter that referenced this pull request Sep 22, 2024
Compatibily with new flag web.listen-address
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.

3 participants