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

[chart] Create a service monitor for each script #112

Merged
merged 3 commits into from
Feb 8, 2024

Conversation

BapRx
Copy link
Contributor

@BapRx BapRx commented Jan 27, 2024

Hi, this PR allows us to track the scripts using a service monitor instead of manually configuring the prometheus additional scrape configs.

If you prefer adding a separate option under .Values.serviceMonitor to enable those extra resources I can edit this PR.

I also bumped the image tag to v2.17.0 in the appVersion.

@ricoberger
Copy link
Owner

Hi @BapRx thanks for your contribution 🙂

If you prefer adding a separate option under .Values.serviceMonitor to enable those extra resources I can edit this PR.

I think a separate option would make sense. Can you please add it?

@ricoberger ricoberger added the changelog: added A new feature was added label Jan 30, 2024
@BapRx
Copy link
Contributor Author

BapRx commented Jan 31, 2024

Another option while we're still implementing this would be to handle this like prometheus-blackbox-exporter does, it's not automatically creating a serviceMonitor for each target but allows the user to enable or disable it per probe. The selfMonitor is also optional and disabled by default (we could set it to true to avoid a breaking change).

What method do you prefer?

@ricoberger
Copy link
Owner

Hi, the implementation from the blackbox exporter Helm chart looks awesome and providing the same functionality would be great.

But I'm also fine with creating a default ServiceMonitor per probe in a first step, because this is already a huge improvement.

@BapRx
Copy link
Contributor Author

BapRx commented Feb 2, 2024

I just pushed a new commit which brings the best of both worlds (I hope!), it defaults to the automatic discovery but also allows the users to override this behavior by providing a list of the scripts to scrape.

I enabled the self-serviceMonitor by default.

@ricoberger
Copy link
Owner

Awesome @BapRx, thanks again 🙂

@ricoberger ricoberger merged commit 44ddcaf into ricoberger:main Feb 8, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
changelog: added A new feature was added
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants