Refactor monitoring recording-rules #10651

machadovilaca · 2023-10-30T11:23:33Z

What this PR does / why we need it:

Following the work started in #10044, and according to the kubevirt/community#219 proposal, this PR:

refactors recording rules and precedes a following PR to also refactor alerts, in a similar way
updates the old name of the github.com/coreos/prometheus-operator dependency to github.com/prometheus-operator/prometheus-operator
replaces the usage of k8s.io/utils/pointer with k8s.io/utils/ptr, because with the deps updates k8s.io/utils/pointer became deprecated

Checklist

This checklist is not enforcing, but it's a reminder of items that could be relevant to every PR.
Approvers are expected to review this list.

Design: A design document was considered and is present (link) or not required
PR: The PR description is expressive enough and will help future contributors
Code: Write code that humans can understand and Keep it simple
Refactor: You have left the code cleaner than you found it (Boy Scout Rule)
Upgrade: Impact of this change on upgrade flows was considered and addressed if required
Testing: New code requires new unit tests. New features and bug fixes require at least on e2e test
Documentation: A user-guide update was considered and is present (link) or not required. You want a user-guide update if it's a user facing feature / API change.
Community: Announcement to kubevirt-dev was considered

Release note:

Refactor monitoring  recording-rules

JIRA ticket:

https://issues.redhat.com/browse/CNV-27303

kubevirt-bot · 2023-10-30T11:23:35Z

Skipping CI for Draft Pull Request.
If you want CI signal for your change, please convert it to an actual PR.
You can still manually trigger a test run with /test all

machadovilaca · 2023-10-31T12:21:33Z

/test all

machadovilaca · 2023-10-31T15:35:06Z

/cc @enp0s3

enp0s3

@machadovilaca Hi, thank you for taking care of it! Can you please elaborate in the PR description on the need to bump the vendor packages?

hack/dep-update.sh

enp0s3 · 2023-11-08T08:02:13Z

docs/metrics.md

@@ -253,7 +253,7 @@ Returns the total number of virtual machine disks restored from the source virtu
 Returns the amount of space in bytes restored from the source virtual machine. Type: Gauge.

 ### kubevirt_vmsnapshot_persistentvolumeclaim_labels
-Returns the labels of the persistent volume claims that are used for restoring virtual machines. Type: Info.
+Returns the labels of the persistent volume claims that are used for restoring virtual machines. Type: Gauge.


Is this change related to the refactoring or is it a bug you've fixed?

50/50, metric type Info does not exist: https://prometheus.io/docs/concepts/metric_types/

and therefore the package does not provide it

pkg/monitoring/rules/recording_rules_api.go

pkg/monitoring/rules/recording_rules_nodes.go

pkg/monitoring/rules/recording_rules_virt.go

pkg/monitoring/rules/recording_rules_vm.go

pkg/monitoring/rules/recording_rules_vmi.go

pkg/monitoring/rules/recording_rules_vmsnapshot.go

pkg/monitoring/rules/rules.go

machadovilaca · 2023-11-08T11:26:22Z

updated @enp0s3

…rometheus-operator/prometheus-operator Signed-off-by: João Vilaça <jvilaca@redhat.com>

Signed-off-by: João Vilaça <jvilaca@redhat.com>

machadovilaca · 2023-11-15T16:32:56Z

/retest

enp0s3

@machadovilaca Thank you!

/approve

kubevirt-bot · 2023-11-24T17:43:14Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: enp0s3

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:

~~OWNERS~~ [enp0s3]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

xpivarc

/hold
@enp0s3

xpivarc · 2023-11-27T07:10:14Z

hack/dep-update.sh

@@ -29,7 +29,7 @@ done
    echo $_sync_only
    cd staging/src/kubevirt.io/client-go
    if [ "${_sync_only}" == "false" ]; then go get $@ ./...; fi
-    go mod tidy
+    go mod tidy -compat=1.17


Can you explain this change?

@xpivarc Thanks! @machadovilaca I think I commented on it in the previous review cycle, why did you still keep it?

my bad, sorry, this was indeed resolved in a previous interaction, but after some reviews eventually it was changed back because we then discussed some unnecessary dependency updates in staging/src/kubevirt.io/client-go/go.mod, namely, onsi/ginkgo/v2 and onsi/gomega, that didn't seem necessary

by default the flag defaults to ensure compatibility with the previous version (-compat=1.16 since this go.mod declares go 1.17) which causes dependency incompatibilities and required manual updates of many dependencies, this change guaranties compatibility with declared go 1.17

an alternative to -compat=1.17 addition would be adding on top off this PR changes this commit: f348aca

@enp0s3 @xpivarc

The required changes look good.

added @xpivarc

To ensure compatibility we need to update some client-go dependencies after the update of other related dependencies Signed-off-by: João Vilaça <jvilaca@redhat.com>

xpivarc · 2023-11-28T13:23:36Z

/hold cancel

sradco · 2023-11-28T13:48:47Z

/lgtm

kubevirt-commenter-bot · 2023-11-29T03:20:22Z

/retest-required
This bot automatically retries required jobs that failed/flaked on approved PRs.
Silence the bot with an /lgtm cancel or /hold comment for consistent failures.

kubevirt-bot requested review from kbidarkar, rthallisey and xpivarc October 30, 2023 11:23

machadovilaca force-pushed the refactor-monitoring-recording-rules branch 5 times, most recently from c1c0f74 to b9137e2 Compare October 31, 2023 12:21

machadovilaca marked this pull request as ready for review October 31, 2023 15:34

kubevirt-bot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Oct 31, 2023

kubevirt-bot requested a review from iholder101 October 31, 2023 15:34

kubevirt-bot requested a review from enp0s3 October 31, 2023 15:35

kubevirt-bot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Nov 7, 2023

machadovilaca force-pushed the refactor-monitoring-recording-rules branch from b9137e2 to e819f91 Compare November 7, 2023 23:20

kubevirt-bot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Nov 7, 2023

enp0s3 reviewed Nov 8, 2023

View reviewed changes

machadovilaca force-pushed the refactor-monitoring-recording-rules branch from e819f91 to 66b0f98 Compare November 8, 2023 11:22

machadovilaca mentioned this pull request Nov 8, 2023

Refactor monitoring alerts #10700

Merged

8 tasks

kubevirt-bot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Nov 15, 2023

Update github.com/coreos/prometheus-operator to new name github.com/p…

38c9194

…rometheus-operator/prometheus-operator Signed-off-by: João Vilaça <jvilaca@redhat.com>

machadovilaca force-pushed the refactor-monitoring-recording-rules branch from 66b0f98 to 9716c52 Compare November 15, 2023 14:45

kubevirt-bot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Nov 15, 2023

machadovilaca force-pushed the refactor-monitoring-recording-rules branch from 3c42cac to ccef0ff Compare November 15, 2023 15:37

machadovilaca added 2 commits November 15, 2023 16:12

Refactor monitoring recording rules

82cbf7d

Signed-off-by: João Vilaça <jvilaca@redhat.com>

Remove usage of deprecated "k8s.io/utils/pointer"

4b924d8

Signed-off-by: João Vilaça <jvilaca@redhat.com>

machadovilaca force-pushed the refactor-monitoring-recording-rules branch from 9a3179b to 4b924d8 Compare November 15, 2023 16:12

enp0s3 reviewed Nov 24, 2023

View reviewed changes

kubevirt-bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Nov 24, 2023

xpivarc reviewed Nov 27, 2023

View reviewed changes

kubevirt-bot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Nov 27, 2023

Update client-go dependencies

a9fe7e4

To ensure compatibility we need to update some client-go dependencies after the update of other related dependencies Signed-off-by: João Vilaça <jvilaca@redhat.com>

kubevirt-bot removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Nov 28, 2023

kubevirt-bot assigned sradco Nov 28, 2023

kubevirt-bot added the lgtm Indicates that a PR is ready to be merged. label Nov 28, 2023

kubevirt-bot merged commit cfc6075 into kubevirt:main Nov 29, 2023
39 checks passed

machadovilaca mentioned this pull request Jan 24, 2024

Refactor monitoring metrics #10982

Merged

8 tasks

chenrui333 mentioned this pull request Mar 6, 2024

virtctl 1.2.0 Homebrew/homebrew-core#165266

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Refactor monitoring recording-rules #10651

Refactor monitoring recording-rules #10651

machadovilaca commented Oct 30, 2023 •

edited

kubevirt-bot commented Oct 30, 2023

machadovilaca commented Oct 31, 2023

machadovilaca commented Oct 31, 2023

enp0s3 left a comment

enp0s3 Nov 8, 2023

machadovilaca Nov 8, 2023

machadovilaca commented Nov 8, 2023

machadovilaca commented Nov 15, 2023

enp0s3 left a comment

kubevirt-bot commented Nov 24, 2023

xpivarc left a comment

xpivarc Nov 27, 2023

enp0s3 Nov 27, 2023

machadovilaca Nov 27, 2023

xpivarc Nov 27, 2023

machadovilaca Nov 27, 2023 •

edited

xpivarc commented Nov 28, 2023

sradco commented Nov 28, 2023

kubevirt-commenter-bot commented Nov 29, 2023

Refactor monitoring recording-rules #10651

Refactor monitoring recording-rules #10651

Conversation

machadovilaca commented Oct 30, 2023 • edited

kubevirt-bot commented Oct 30, 2023

machadovilaca commented Oct 31, 2023

machadovilaca commented Oct 31, 2023

enp0s3 left a comment

Choose a reason for hiding this comment

enp0s3 Nov 8, 2023

Choose a reason for hiding this comment

machadovilaca Nov 8, 2023

Choose a reason for hiding this comment

machadovilaca commented Nov 8, 2023

machadovilaca commented Nov 15, 2023

enp0s3 left a comment

Choose a reason for hiding this comment

kubevirt-bot commented Nov 24, 2023

xpivarc left a comment

Choose a reason for hiding this comment

xpivarc Nov 27, 2023

Choose a reason for hiding this comment

enp0s3 Nov 27, 2023

Choose a reason for hiding this comment

machadovilaca Nov 27, 2023

Choose a reason for hiding this comment

xpivarc Nov 27, 2023

Choose a reason for hiding this comment

machadovilaca Nov 27, 2023 • edited

Choose a reason for hiding this comment

xpivarc commented Nov 28, 2023

sradco commented Nov 28, 2023

kubevirt-commenter-bot commented Nov 29, 2023

machadovilaca commented Oct 30, 2023 •

edited

machadovilaca Nov 27, 2023 •

edited