-
Notifications
You must be signed in to change notification settings - Fork 162
Helm Chart: Add ServiceMonitor #4539
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
base: main
Are you sure you want to change the base?
Conversation
|
Hi @icelynjennings! Welcome to the project! 🎉 Thanks for opening this pull request! |
|
✅ All required contributors have signed the F5 CLA for this PR. Thank you! |
|
I have hereby read the F5 CLA and agree to its terms |
sjberman
left a comment
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.
Thanks for the contribution! Unfortunately this doesn't fully implement the desired solution. While this would create a ServiceMonitor for the control plane, it doesn't create one for the nginx data plane.
Since the data plane is provisioned by the control plane when a Gateway is created, all of the configuration for the data plane deployment is built into the control plane's code, and not through helm.
We would probably have to follow a similar implementation that we did with HPA (see here: #3702) to enhance our API and code to allow users to enable service monitor for the nginx data plane Deployment.
|
Thanks @sjberman. Would there be benefit to adding a |
|
Honestly, metrics for the data plane are probably the more interesting ones for most users, since that's where the traffic is flowing. The control plane monitor could be an easy one to do (since it's just helm changes), but to truly close the linked issue, I think we would want data plane support. |
|
Thanks for linking a relevant PR @sjberman! That looks like mostly kubebuilder, so I should be able to finish that part. |
|
@icelynjennings No problem. Part of it is kubebuilder to define the API, but there are a decent amount of code changes internally too in order to provision the objects based on the API. |
Proposed changes
Closes #4293
Testing: templated successfully via a local
helm templatecli run, but ideally should be further tested in Prometheus.Checklist
Before creating a PR, run through this checklist and mark each as complete.