openshift / cluster-monitoring-operator

Manage the OpenShift monitoring stack
Apache License 2.0
247 stars 363 forks source link

MON-3940: Add the collection of MTV migration metrics to Telemetry #2461

Open bkhizgiy opened 1 month ago

bkhizgiy commented 1 month ago

Following this JIRA issue https://issues.redhat.com/browse/MON-3940

Related to this PRs for adding metrics on the MTV side: https://github.com/kubev2v/forklift/pull/916 https://github.com/kubev2v/forklift/pull/932 https://github.com/kubev2v/forklift/pull/978

openshift-ci[bot] commented 1 month ago

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: bkhizgiy Once this PR has been reviewed and has the lgtm label, please assign simonpasquier for approval. For more information see the Kubernetes Code Review Process.

The full list of commands accepted by this bot can be found here.

Needs approval from an approver in each of these files: - **[OWNERS](https://github.com/openshift/cluster-monitoring-operator/blob/master/OWNERS)** Approvers can indicate their approval by writing `/approve` in a comment Approvers can cancel approval by writing `/approve cancel` in a comment
fabiand commented 1 month ago

@simonpasquier please review

openshift-ci[bot] commented 1 month ago

@bkhizgiy: The following tests failed, say /retest to rerun all failed tests or /retest-required to rerun all mandatory failed tests:

Test name Commit Details Required Rerun command
ci/prow/e2e-aws-ovn-single-node ddd0170ed23f2642d1cef2bf2e47086f971dcee3 link false /test e2e-aws-ovn-single-node
ci/prow/e2e-aws-ovn-techpreview ddd0170ed23f2642d1cef2bf2e47086f971dcee3 link true /test e2e-aws-ovn-techpreview

Full PR test history. Your PR dashboard.

Instructions for interacting with me using PR comments are available [here](https://git.k8s.io/community/contributors/guide/pull-requests.md). If you have questions or suggestions related to my behavior, please file an issue against the [kubernetes-sigs/prow](https://github.com/kubernetes-sigs/prow/issues/new?title=Prow%20issue:) repository. I understand the commands that are listed [here](https://go.k8s.io/bot-commands).
simonpasquier commented 1 month ago

/retitle MON-3940: Add the collection of MTV migration metrics to Telemetry /hold

until explicit approval

openshift-ci-robot commented 1 month ago

@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](https://github.com/openshift/cluster-monitoring-operator/pull/2461): >Following this JIRA issue https://issues.redhat.com/browse/MON-3940 > >Related to this PRs for adding metrics on the MTV side: >https://github.com/kubev2v/forklift/pull/916 >https://github.com/kubev2v/forklift/pull/932 >https://github.com/kubev2v/forklift/pull/978 > Instructions for interacting with me using PR comments are available [here](https://prow.ci.openshift.org/command-help?repo=openshift%2Fcluster-monitoring-operator). If you have questions or suggestions related to my behavior, please file an issue against the [openshift-eng/jira-lifecycle-plugin](https://github.com/openshift-eng/jira-lifecycle-plugin/issues/new) repository.
bkhizgiy commented 1 month ago

@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 commented 1 month ago

PR needs rebase.

Instructions for interacting with me using PR comments are available [here](https://git.k8s.io/community/contributors/guide/pull-requests.md). If you have questions or suggestions related to my behavior, please file an issue against the [kubernetes-sigs/prow](https://github.com/kubernetes-sigs/prow/issues/new?title=Prow%20issue:) repository.
fabiand commented 1 month ago

@simonpasquier any update?

simonpasquier commented 3 weeks ago

Should the recording rule be part of the MTV code, or can we specify the expression in the whitelist?

MTV code. The telemetry allow-list only references metric names (and labels).

This metric consists only of the mentioned fields, so there aren't any labels to remove or modify.

We still ask to remove the labels that are useless to keep at the Telemetry server level like instance and pod.

simonpasquier commented 3 weeks ago

see https://rhobs-handbook.netlify.app/products/openshiftmonitoring/telemetry.md/#configure-recording-rules