Skip to content

[kuberay] Update helm install command in prometheus doc to set serviceMonitor release=prometheus#53952

Merged
jjyao merged 2 commits intoray-project:masterfrom
owenowenisme:kuberay/update-prometheus-doc-serviceMonitor-command
Jun 19, 2025
Merged

[kuberay] Update helm install command in prometheus doc to set serviceMonitor release=prometheus#53952
jjyao merged 2 commits intoray-project:masterfrom
owenowenisme:kuberay/update-prometheus-doc-serviceMonitor-command

Conversation

@owenowenisme
Copy link
Copy Markdown
Member

Why are these changes needed?

We need to set label when creating serviceMonitor with helm to let prometheus discover our ServiceMonitor, so we updated the command to set the label.

Related issue number

Checks

  • I've signed off every commit(by using the -s flag, i.e., git commit -s) in this PR.
  • I've run scripts/format.sh to lint the changes in this PR.
  • I've included any doc changes needed for https://docs.ray.io/en/master/.
    • I've added any new APIs to the API Reference. For example, if I added a
      method in Tune, I've added it in doc/source/tune/api/ under the
      corresponding .rst file.
  • I've made sure the tests are passing. Note that there might be a few flaky tests, see the recent failures at https://flakey-tests.ray.io/
  • Testing Strategy
    • Unit tests
    • Release tests
    • This PR is not tested :(

Signed-off-by: You-Cheng Lin <mses010108@gmail.com>
@owenowenisme owenowenisme marked this pull request as ready for review June 19, 2025 15:56
Copilot AI review requested due to automatic review settings June 19, 2025 15:56
@owenowenisme owenowenisme requested review from a team, kevin85421 and pcmoritz as code owners June 19, 2025 15:56
@owenowenisme
Copy link
Copy Markdown
Member Author

@troychiu @win5923 @kevin85421 PTAL

Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR updates the Prometheus & Grafana documentation for KubeRay by enhancing the Helm install command to include a selector label, ensuring ServiceMonitor resources are discovered by the Prometheus release.

  • Added --set metrics.serviceMonitor.selector.release=prometheus to the Helm command
  • Broke out Helm flags onto separate lines for readability
Comments suppressed due to low confidence (2)

doc/source/cluster/kubernetes/k8s-ecosystem/prometheus-grafana.md:56

  • [nitpick] Since the Helm command now also sets the ServiceMonitor selector label, update this description to mention adding metrics.serviceMonitor.selector.release so users know why the label is needed.
* Set `metrics.serviceMonitor.enabled=true` when installing the KubeRay operator with Helm to create a ServiceMonitor that scrapes metrics exposed by the KubeRay operator's service.

doc/source/cluster/kubernetes/k8s-ecosystem/prometheus-grafana.md:60

  • Helm charts for Prometheus Operator typically expect selectors under matchLabels. Please use --set metrics.serviceMonitor.selector.matchLabels.release=prometheus to correctly apply the release label.
    --set metrics.serviceMonitor.selector.release=prometheus

@@ -56,7 +56,8 @@ kubectl get all -n prometheus-system
* Set `metrics.serviceMonitor.enabled=true` when installing the KubeRay operator with Helm to create a ServiceMonitor that scrapes metrics exposed by the KubeRay operator's service.
```sh
helm install kuberay-operator kuberay/kuberay-operator --version 1.4.0 \
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Please add comments to explain the two flags.

Signed-off-by: You-Cheng Lin <mses010108@gmail.com>
@owenowenisme owenowenisme changed the title [kuberay] Update helm install command in prometheus doc to set serviceMonitor release= prometheus [kuberay] Update helm install command in prometheus doc to set serviceMonitor release=prometheus Jun 19, 2025
@kevin85421 kevin85421 added the go add ONLY when ready to merge, run all tests label Jun 19, 2025
@kevin85421
Copy link
Copy Markdown
Member

@owenowenisme please ping me when all CI tests pass.

@owenowenisme
Copy link
Copy Markdown
Member Author

@kevin85421 CI passed.

@kevin85421
Copy link
Copy Markdown
Member

cc @jjyao @edoakes would you mind merging this PR? Thanks!

@jjyao jjyao merged commit 3f378ed into ray-project:master Jun 19, 2025
6 checks passed
minerharry pushed a commit to minerharry/ray that referenced this pull request Jun 27, 2025
…eMonitor `release=prometheus` (ray-project#53952)

Signed-off-by: You-Cheng Lin <mses010108@gmail.com>
elliot-barn pushed a commit that referenced this pull request Jul 2, 2025
…eMonitor `release=prometheus` (#53952)

Signed-off-by: You-Cheng Lin <mses010108@gmail.com>
Signed-off-by: elliot-barn <elliot.barnwell@anyscale.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

go add ONLY when ready to merge, run all tests

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants