[Feature]: Add serviceMonitor for kuberay-operator (#3207) #3208
Conversation
feat: Add kuberay-operator serviceMonitor helm template(#0) See merge request cloudml-aie/kuberay!3
|
cc @win5923 Thank you for give some suggestions. This is my first pr on kuberay repo |
|
Hi @dushulin, Thanks for the PR. |
| endpoints: | ||
| - path: /metrics | ||
| targetPort: http |
There was a problem hiding this comment.
Could you add an interval field under the endpoints section? This helps control how frequently Prometheus scrapes the metrics.
There was a problem hiding this comment.
Also add honorLabels, it preserves the metric’s labels when they collide with the target’s labels
| # For kuberay-operator metric export by prometheus operator | ||
| serviceMonitor: | ||
| # If enabled, serviceMonitor will be created, expose /metrics http endpoint to prometheus | ||
| enabled: false |
|
Hi @kevin85421, this PR add serviceMonitor for KubeRay Operator in helm chart, PTAL |
| metadata: | ||
| name: {{ include "kuberay-operator.fullname" . }} |
There was a problem hiding this comment.
I think we also need to provide metadata.labels.release to the name of label selected by Prometheus. Otherwise this config won't work. Right? By default is release: prometheus.
We can provide a flag let user to set it.
There was a problem hiding this comment.
You can refer to ArgoCD's implementation.
There was a problem hiding this comment.
@win5923 Thanks, I will refer to ArgoCD's implementation, and I will test this in my local env. I provide too few parameters for users to set.
There was a problem hiding this comment.
Yeah I also tested this. I have to add the release label so that the serviceMonitor would be selected by prometheus.
|
cc @troychiu @owenowenisme can you also review this PR? Thanks! |
| @@ -161,6 +161,12 @@ env: | |||
| # - name: DELETE_RAYJOB_CR_AFTER_JOB_FINISHES | |||
| # value: "false" | |||
|
|
|||
| # For kuberay-operator metric export by prometheus operator | |||
| serviceMonitor: | |||
| # If enabled, serviceMonitor will be created, expose /metrics http endpoint to prometheus | |||
There was a problem hiding this comment.
IIUC, a serviceMonitor is telling prom where to scrape metrics. This comment seems not like how it works. Do you mind double-checking it? Thank you!
| @@ -161,6 +161,12 @@ env: | |||
| # - name: DELETE_RAYJOB_CR_AFTER_JOB_FINISHES | |||
| # value: "false" | |||
|
|
|||
| # For kuberay-operator metric export by prometheus operator | |||
|
Hi @dushulin. Do you mind taking a look at the comments as well as the lint error? Thank you! |
@kevin85421 @troychiu sorry, please open a new one, because I haven't complete manul test yet, maybe block release. |
|
Sounds good. Let me create one shortly. |
|
Closed in favor of #3717 |
Add serviceMonitor for kuberay-operator