MON-3940: Add the collection of MTV migration metrics to Telemetry #2461
Conversation
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: bkhizgiy The full list of commands accepted by this bot can be found here.
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
|
@simonpasquier please review |
There was a problem hiding this comment.
You'd need to configure recording rules aggregating on the allowed labels:
- status
- provider
- mode
- target
If the metrics are emitted by a single container then you can do:
record: cluster:mtv_migrations_status_total:max
expr: max by(status, provider, mode, target) (mtv_migrations_status_total)
otherwise you should probably sum
record: cluster:mtv_migrations_status_total:sum
expr: sum by(status, provider, mode, target) (mtv_migrations_status_total)
|
/retitle MON-3940: Add the collection of MTV migration metrics to Telemetry until explicit approval |
openshift-ci
bot
added
the
do-not-merge/hold
openshift-ci
bot
changed the title
openshift-ci-robot
added
the
jira/valid-reference
|
@bkhizgiy: This pull request references MON-3940 which is a valid jira issue. Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the task to target the "4.18.0" version, but no target version was set. In response to this:
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 openshift-eng/jira-lifecycle-plugin repository. |
|
@simonpasquier Thanks for the review. Should the recording rule be part of the MTV code, or can we specify the expression in the whitelist? This metric consists only of the mentioned fields, so there aren't any labels to remove or modify. |
openshift-merge-robot
added
the
needs-rebase
|
@simonpasquier any update? |
MTV code. The telemetry allow-list only references metric names (and labels).
We still ask to remove the labels that are useless to keep at the Telemetry server level like |
Documentation/data-collection.md
Outdated
| # | ||
| # cluster:mtv_migrations_status_total is the total number of VM migrations running on the cluster, | ||
| # labeled with {status}, {provider}, {mode}, and {target}. | ||
| - '{__name__="mtv_migrations_status_total"}' |
There was a problem hiding this comment.
the metrics name mtv_migrations_status_total is not the same as cluster:mtv_migrations_status_total in the annotation
cluster:mtv_migrations_status_total is the total number of VM migrations running on the cluster,
maybe should be cluster:mtv_migrations_status_total:sum as @simonpasquier mentioned in #2461 (review)
Following this Jira issue https://issues.redhat.com/browse/MON-3940 Signed-off-by: Bella Khizgiyaev <[email protected]>
openshift-merge-robot
removed
the
needs-rebase
|
Following the comments, I’ve added a new recording rule for MTV in this PR. |
|
@bkhizgiy: all tests passed! Full PR test history. Your PR dashboard. 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-sigs/prow repository. I understand the commands that are listed here. |
Following the request of adding recording rule in openshift/cluster-monitoring-operator#2461 Signed-off-by: Bella Khizgiyaev <[email protected]>
|
kubev2v/forklift#1172 got merged - what is needed to move this PR forward? |
openshift-merge-robot
added
the
needs-rebase
|
PR needs rebase. 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-sigs/prow repository. |
|
Hi @bkhizgiy, looks like the PR needs a rebase. Is there anything else keeping thos PR from getting merged? |
Following this JIRA issue https://issues.redhat.com/browse/MON-3940
Related to this PRs for adding metrics on the MTV side:
kubev2v/forklift#916
kubev2v/forklift#932
kubev2v/forklift#978