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

monitoring: set port for servicemonitor for ceph-exporter #12825

Merged
merged 1 commit into from
Sep 1, 2023

Conversation

weirdwiz
Copy link
Contributor

@weirdwiz weirdwiz commented Aug 31, 2023

ceph-exporter's ServiceMonitor and Service CRD contain different port
name, which results in no metrics being collected by prometheus.

this commit makes GetCephMonitor configurable, which we use to set
consistent port names in ServiceMonitor and Service for ceph-exporter.

Copy link
Member

@travisn travisn left a comment

Choose a reason for hiding this comment

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

A couple suggestions:

  • Add a commit prefix as described in the development guide, such as monitoring: .
  • The purpose of this change is to set the port name for the exporter, correct? Please describe the behavior change in the commit title or description. It's more important to say the behavior change in the commit message than to say what method it was in.

Thanks!

Copy link
Contributor

@subhamkrai subhamkrai left a comment

Choose a reason for hiding this comment

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

@weirdwiz please look at Travis comments

Copy link
Member

@avanthakkar avanthakkar left a comment

Choose a reason for hiding this comment

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

Tested locally and exporter runs fine. LGTM!

ceph-exporter's ServiceMonitor and Service CRD contain different port
name, which results in no metrics being collected by prometheus.

this commit makes GetCephMonitor configurable, which we use to set
consistent port names in ServiceMonitor and Service for ceph-exporter.

Signed-off-by: Divyansh Kamboj <dkamboj@redhat.com>
@weirdwiz weirdwiz changed the title Add port-name to GetServiceMonitor monitoring: set port for servicemonitor for ceph-exporter Sep 1, 2023
@travisn travisn dismissed subhamkrai’s stale review September 1, 2023 17:58

feedback addressed

@travisn travisn merged commit ded675e into rook:master Sep 1, 2023
49 of 50 checks passed
mergify bot added a commit that referenced this pull request Sep 1, 2023
monitoring: set port for servicemonitor for ceph-exporter (backport #12825)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants