-
Notifications
You must be signed in to change notification settings - Fork 212
Add Prometheus servicemonitor for Jupyterhub service #239
Add Prometheus servicemonitor for Jupyterhub service #239
Conversation
Hi @4n4nd. Thanks for your PR. I'm waiting for a opendatahub-io member to verify that this patch is reasonable to test. If it is, they should reply with Once the patch is verified, the new status will be reflected by the I understand the commands that are listed here. Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. |
/ok-to-test |
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.
@4n4nd you will need to update https://github.com/opendatahub-io/odh-manifests/blob/master/prometheus/operator/base/kustomization.yaml as well to link to the service monitor
kind: ServiceMonitor | ||
metadata: | ||
labels: | ||
app: jupyterhub |
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.
Would changing the name to odh-jupyterhub be safer?
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.
I think jupyterhub is better since all other JH components have this label
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.
Fine by me. I'll defer to @nakfour re: the label
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.
The label is bound to the team
right? In that case, I think we're fine
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.
@anishasthana is this supposed to scrape metrics from KF jupyter ? If not, we need to say it is odh-jupyterhub. I know KF does not have jupyterhub but there is always confusion between what odh has and what KF has.
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.
I think a lot of ODH JH components have this label.
Are we planning to rename them to odh-jupyterhub?
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.
Not particular plan, we have other labels too, if you want to be more specific component.opendatahub.io/name: jupyterhub
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.
I think we should keep it as app=jypyterhub rn and create a new PR to update labels for all the JH components.
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.
ok thats fine, please open the issue to change all labels to jupyterhub
e84554c
to
ee328cd
Compare
Update kustomization.yaml
ee328cd
to
ce7f84f
Compare
/lgtm |
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: 4n4nd, nakfour The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
No description provided.