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

list-metrics-warnings made configurable #447

Merged

Conversation

or-shachar
Copy link
Contributor

@or-shachar or-shachar commented Jul 17, 2022

Addresses #442

  • Warnings for list metrics is now configurable globally / per metric.
  • (still WIP) Added explanation in README on how exporter determines which dimensions to scrape. (will create a different PR)

@or-shachar or-shachar force-pushed the configurable-list-metrics-warn branch from c456b3e to 08b5d5e Compare July 17, 2022 09:53
Copy link
Contributor

@matthiasr matthiasr left a comment

Choose a reason for hiding this comment

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

I am not entirely sure about the description of the two "modes" – this seems like an implementation leaking through into the description of the user experience.

I am wondering if it would be easier to follow if we describe the default behavior (minimum necessary configuration, and what happens with that), and then wuat changes when you set one of the other options?

README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
@or-shachar
Copy link
Contributor Author

Thanks for the feedback... Indeed I had a hard time describing the list-dimension process.

My intent was that users would have an easier time troubleshooting "missing metrics" problems (maybe a better user-centric idea is to add a troubleshooting section).

I wonder if I should split the README change and the configurable warning change to separate PR. WDYT?

@matthiasr
Copy link
Contributor

Yes that's a good idea, then we can get the option out sooner

@or-shachar or-shachar force-pushed the configurable-list-metrics-warn branch from 08b5d5e to 92b7def Compare October 16, 2022 11:23
Signed-off-by: or-shachar <or.shachar@wiz.io>
@or-shachar or-shachar force-pushed the configurable-list-metrics-warn branch from c89b19c to f2996e8 Compare October 16, 2022 11:38
@or-shachar
Copy link
Contributor Author

Hey @matthiasr sorry it took me so long to pick it up again.
I've adjusted the PR to the latest HEAD, and removed the confusing README sections.

@matthiasr
Copy link
Contributor

Awesome, thank you for finishing it up!

@matthiasr matthiasr merged commit aca3979 into prometheus:master Oct 25, 2022
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

3 participants