-
Notifications
You must be signed in to change notification settings - Fork 5k
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
[alertmanager] Add prometheus servicemonitor support #4631
base: main
Are you sure you want to change the base?
[alertmanager] Add prometheus servicemonitor support #4631
Conversation
Signed-off-by: Bernd May <b.may@syseleven.de>
@monotek it appears the lint check failed for no apparent reason? |
nvm i see the errors now; though they are not related to anything I touched and I don't see what the linter wants me to do. there appear to be some problems wit the comments around l 115 and 116 🤷 |
Signed-off-by: Bernd May <b.may@syseleven.de>
mucking around with yamllint did the trick but now I face the consequence of adding a resource that requires the prometheus crd for a service monitor... so either I add the crd chart s as a dependency (preferred) or exclude the resource based on a capabilities test. WDYT? |
We cannot assume that prometheus is installed therefore the user needs to decide this consciously Signed-off-by: Bernd May <b.may@syseleven.de>
Signed-off-by: Bernd May <b.may@syseleven.de>
9a22ddd
to
f9a7399
Compare
well should work now - please have a look |
my 2 cents,
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
see code comments...
That was the other alternative I considered; I'm fine with either way. This contains the 'risk' of not deploying the servicemonitor even though someone enabled it though. I have added an additional if - fail instead; that way one has to have the capability or explicitly fake it with --api-versions |
In order to automatically monitor alertmanager with prometheus and prometheus operator we need a servicemonitor resource. Signed-off-by: Bernd May <b.may@syseleven.de>
ef820ef
to
8b49899
Compare
I noticed a side effect of the current change; due to the chosen label selectors we get duplicate metrics. The headless and regular services have the same labels, which is why the servicemonitors label selector matches both. In order to distinguish them, I could add a label to the headless service. I could also switch to using a pod monitor. There is also the option of using a targetLimit though I have never used that and will need to test if it does what we want. |
Signed-off-by: Bernd May <b.may@syseleven.de>
What this PR does / why we need it
Add servicemonitor support to alertmanager
Which issue this PR fixes
Special notes for your reviewer
Checklist
[prometheus-couchdb-exporter]
)