WINC-1181: Add pod CPU and Memory metrics #1949

sebsoto · 2023-11-16T18:13:34Z

Adds recording rules which translates the windows exporter's CPU and memory metrics into the time series required for display in the OpenShift console.

openshift-ci · 2023-11-16T18:13:46Z

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

sebsoto · 2023-11-16T18:13:48Z

sebsoto · 2023-11-16T18:14:17Z

/approve cancel

mtnbikenc · 2023-11-16T18:31:47Z

@sebsoto This is for Pods not Nodes, correct? That distinction might be helpful in the title and commit message.

sebsoto · 2023-11-17T18:44:57Z

Correct, I'll add that to the commit/pr message

jrvaldes

/lgtm

Should this be documented? So that users are aware such metrics are not available.

jrvaldes · 2023-11-20T15:46:13Z

What about backports?

openshift-ci-robot · 2023-12-06T17:02:11Z

@sebsoto: This pull request references WINC-568 which is a valid jira issue.

In response to this:

Adds recording rules which translates the windows exporter's CPU and memory metrics into the time series required for display in the OpenShift console.

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.

openshift-ci-robot · 2023-12-06T17:14:50Z

@sebsoto: This pull request references WINC-1181 which is a valid jira issue.

In response to this:

Adds recording rules which translates the windows exporter's CPU and memory metrics into the time series required for display in the OpenShift console.

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.

mansikulkarni96 · 2023-12-06T17:29:59Z

bundle/manifests/windows-prometheus-k8s-rules_monitoring.coreos.com_v1_prometheusrule.yaml

+      record: pod:container_cpu_usage:sum
+    - expr: |
+        label_replace(windows_container_memory_usage_private_working_set_bytes * on(container_id) group_left(namespace, pod, container) kube_pod_container_info{container_id!=""},"container","","","")
+      record: container_memory_working_set_bytes


It maybe out of scope here, but a quick thing to give the number of running pods

expr: |
windows_container_available
record: kubelet_running_pods

@mansikulkarni96 I'd prefer that be done as a followup PR as this work is targetting the graphs

mansikulkarni96

Thanks for working on this @sebsoto, PTAL at my comments.

mansikulkarni96 · 2023-12-06T17:34:50Z

bundle/manifests/windows-prometheus-k8s-rules_monitoring.coreos.com_v1_prometheusrule.yaml

@@ -39,3 +39,9 @@ spec:
    - expr: |
        windows_cpu_info
      record: node_cpu_info
+    - expr: |
+        sum(rate(windows_container_cpu_usage_seconds_total[5m]) * on(container_id) group_left(namespace, pod, container) kube_pod_container_info{container_id!=""}) by (pod,namespace)


I may have to test this out but AFAIK pod label is not available through windows_exporter, an option here to remove * on(container_id) is to do a label mapping in:
[bundle/manifests/windows-exporter_monitoring.coreos.com_v1_servicemonitor.yaml]

relabelings: - action: replace regex: (.*) replacement: sourceLabels: - __meta_kubernetes_endpoint_address_target_name targetLabel: instance - action: labelmap source_labels: [container_id] replacement: pod ```

correct Windows exporter does not add the pod label
I am adding it via this method

mansikulkarni96 · 2023-12-07T19:00:43Z

@sebsoto changes look good here, I missed this in my previous review but wouldn't it be beneficial to add some tests here to ensure we continue to get the pod metrics like we do here:

windows-machine-config-operator/test/e2e/metrics_test.go

Line 264 in a05678b

func (tc *testContext) testNodeResourceUsage(t *testing.T) {

Adds recording rules which translates the windows exporter's pod CPU and memory metrics into the time series required for display in the OpenShift console.

Adds makePrometheusQuery() making code more readable, and allowing for the reuse of the functionality elsewhere.

sebsoto · 2023-12-11T18:17:05Z

/test azure-e2e-operator vsphere-e2e-operator

mansikulkarni96 · 2023-12-11T21:36:40Z

Thanks for working on this @sebsoto !
/approve

openshift-ci · 2023-12-11T21:37:00Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: mansikulkarni96

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~~ [mansikulkarni96]

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

jrvaldes · 2023-12-12T13:44:50Z

/lgtm

sebsoto · 2023-12-12T15:19:05Z

/test remaining-required

saifshaikh48

/lgtm

Thanks, finally got to understand the query structure.

saifshaikh48 · 2023-12-12T15:16:44Z

test/e2e/storage_test.go

@@ -70,6 +70,14 @@ func testStorage(t *testing.T) {
 			}
 		}()
 	}
+	t.Run("pod metrics", func(t *testing.T) {


Would such a test be better suited as part of metrics_test.go? May it'd be best in the long run to split the other metrics tests out of the creation suite

This is dependent on a workload, for the sake of not increasing the test runtime anymore I felt it best to put it in the storage test. Ideally in the future we use the same workload for both the storage test, and the network tests.

openshift-ci-robot · 2023-12-12T17:49:16Z

/retest-required

Remaining retests: 0 against base HEAD 40881cf and 2 for PR HEAD 8926229 in total

sebsoto · 2023-12-12T19:20:48Z

/override ci/prow/aws-e2e-upgrade
Test passed, must-gather failed unrelated to WMCO

openshift-ci · 2023-12-12T19:21:44Z

@sebsoto: Overrode contexts on behalf of sebsoto: ci/prow/aws-e2e-upgrade

In response to this:

/override ci/prow/aws-e2e-upgrade
Test passed, must-gather failed unrelated to WMCO

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.

Tests that Windows pod metrics are being properly generated. This will run as part of the storage test in order to not deploy another workload.

saifshaikh48

/lgtm

saifshaikh48 · 2023-12-12T22:12:21Z

test/e2e/storage_test.go

@@ -70,6 +70,17 @@ func testStorage(t *testing.T) {
 			}
 		}()
 	}
+	t.Run("pod metrics", func(t *testing.T) {
+		if skipWorkloadDeletion {


Think it's worth leaving a TODO to remove this condition in 10.16?

sebsoto · 2023-12-13T01:27:13Z

/test remaining-required

sebsoto · 2023-12-13T02:53:02Z

/test nutanix-e2e-operator

openshift-ci-robot · 2023-12-13T03:28:15Z

/retest-required

Remaining retests: 0 against base HEAD 40881cf and 2 for PR HEAD 1c3c17a in total

openshift-ci · 2023-12-13T05:55:04Z

@sebsoto: 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/test-infra repository. I understand the commands that are listed here.

sebsoto · 2023-12-13T14:00:04Z

/cherry-pick release-4.15

openshift-cherrypick-robot · 2023-12-13T14:00:19Z

@sebsoto: new pull request created: #1976

In response to this:

/cherry-pick release-4.15

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.

openshift-cherrypick-robot · 2023-12-13T14:01:01Z

@sebsoto: new pull request could not be created: failed to create pull request against openshift/windows-machine-config-operator#release-4.15 from head openshift-cherrypick-robot:cherry-pick-1949-to-release-4.15: status code 422 not one of [201], body: {"message":"Validation Failed","errors":[{"resource":"PullRequest","code":"custom","message":"A pull request already exists for openshift-cherrypick-robot:cherry-pick-1949-to-release-4.15."}],"documentation_url":"https://docs.github.com/rest/pulls/pulls#create-a-pull-request"}

In response to this:

/cherry-pick release-4.15

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.

openshift-ci bot added the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Nov 16, 2023

sebsoto force-pushed the cpuMemoryGraphs branch from ee5ccc2 to e533c5d Compare November 16, 2023 18:27

jrvaldes reviewed Nov 20, 2023

View reviewed changes

openshift-ci bot assigned jrvaldes Nov 20, 2023

openshift-ci bot added the lgtm Indicates that a PR is ready to be merged. label Nov 20, 2023

sebsoto force-pushed the cpuMemoryGraphs branch from e533c5d to a763c96 Compare November 21, 2023 19:20

openshift-ci bot removed the lgtm Indicates that a PR is ready to be merged. label Nov 21, 2023

sebsoto changed the title ~~[metrics] Add CPU and Memory metrics~~ [WIP] [metrics] Add CPU and Memory metrics Nov 21, 2023

openshift-merge-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Dec 4, 2023

sebsoto force-pushed the cpuMemoryGraphs branch from a763c96 to f3a6cd1 Compare December 6, 2023 17:00

openshift-merge-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Dec 6, 2023

sebsoto changed the title ~~[WIP] [metrics] Add CPU and Memory metrics~~ WINC-568: Add CPU and Memory metrics Dec 6, 2023

openshift-ci-robot added the jira/valid-reference Indicates that this PR references a valid Jira ticket of any type. label Dec 6, 2023

sebsoto changed the title ~~WINC-568: Add CPU and Memory metrics~~ WINC-568: Add pod CPU and Memory metrics Dec 6, 2023

sebsoto changed the title ~~WINC-568: Add pod CPU and Memory metrics~~ WINC-1181: Add pod CPU and Memory metrics Dec 6, 2023

mansikulkarni96 reviewed Dec 6, 2023

View reviewed changes

[metrics] Add pod CPU and Memory metrics

6190faa

Adds recording rules which translates the windows exporter's pod CPU and memory metrics into the time series required for display in the OpenShift console.

sebsoto force-pushed the cpuMemoryGraphs branch from f3a6cd1 to ecf531f Compare December 11, 2023 18:10

[test] Refactor prometheus queries

db9b59f

Adds makePrometheusQuery() making code more readable, and allowing for the reuse of the functionality elsewhere.

sebsoto force-pushed the cpuMemoryGraphs branch from ecf531f to 8926229 Compare December 11, 2023 18:16

openshift-ci bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Dec 11, 2023

openshift-ci bot added the lgtm Indicates that a PR is ready to be merged. label Dec 12, 2023

sebsoto marked this pull request as ready for review December 12, 2023 14:24

openshift-ci bot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Dec 12, 2023

openshift-ci bot requested a review from mansikulkarni96 December 12, 2023 14:25

saifshaikh48 reviewed Dec 12, 2023

View reviewed changes

openshift-ci bot assigned saifshaikh48 Dec 12, 2023

[test] Add pod metrics test

1c3c17a

Tests that Windows pod metrics are being properly generated. This will run as part of the storage test in order to not deploy another workload.

sebsoto force-pushed the cpuMemoryGraphs branch from 8926229 to 1c3c17a Compare December 12, 2023 21:03

openshift-ci bot removed the lgtm Indicates that a PR is ready to be merged. label Dec 12, 2023

saifshaikh48 reviewed Dec 12, 2023

View reviewed changes

openshift-ci bot added the lgtm Indicates that a PR is ready to be merged. label Dec 12, 2023

openshift-merge-bot bot merged commit 9975407 into openshift:master Dec 13, 2023
17 checks passed

openshift-cherrypick-robot mentioned this pull request Dec 13, 2023

[release-4.15] WINC-1181: Add pod CPU and Memory metrics #1976

Merged

WINC-1181: Add pod CPU and Memory metrics #1949

WINC-1181: Add pod CPU and Memory metrics #1949

Conversation

sebsoto commented Nov 16, 2023

openshift-ci bot commented Nov 16, 2023

sebsoto commented Nov 16, 2023

sebsoto commented Nov 16, 2023

mtnbikenc commented Nov 16, 2023

sebsoto commented Nov 17, 2023

jrvaldes left a comment

Choose a reason for hiding this comment

jrvaldes commented Nov 20, 2023

openshift-ci-robot commented Dec 6, 2023 • edited by openshift-ci bot

openshift-ci-robot commented Dec 6, 2023 • edited by openshift-ci bot

mansikulkarni96 Dec 6, 2023

Choose a reason for hiding this comment

sebsoto Dec 7, 2023

Choose a reason for hiding this comment

mansikulkarni96 left a comment

Choose a reason for hiding this comment

mansikulkarni96 Dec 6, 2023 • edited

Choose a reason for hiding this comment

sebsoto Dec 6, 2023

Choose a reason for hiding this comment

mansikulkarni96 commented Dec 7, 2023

sebsoto commented Dec 11, 2023

mansikulkarni96 commented Dec 11, 2023

openshift-ci bot commented Dec 11, 2023

jrvaldes commented Dec 12, 2023

sebsoto commented Dec 12, 2023

saifshaikh48 left a comment

Choose a reason for hiding this comment

saifshaikh48 Dec 12, 2023

Choose a reason for hiding this comment

sebsoto Dec 12, 2023

Choose a reason for hiding this comment

openshift-ci-robot commented Dec 12, 2023

sebsoto commented Dec 12, 2023

openshift-ci bot commented Dec 12, 2023

saifshaikh48 left a comment

Choose a reason for hiding this comment

saifshaikh48 Dec 12, 2023

Choose a reason for hiding this comment

sebsoto commented Dec 13, 2023

sebsoto commented Dec 13, 2023

openshift-ci-robot commented Dec 13, 2023

openshift-ci bot commented Dec 13, 2023

sebsoto commented Dec 13, 2023

openshift-cherrypick-robot commented Dec 13, 2023

openshift-cherrypick-robot commented Dec 13, 2023

openshift-ci-robot commented Dec 6, 2023 •

edited by openshift-ci bot

openshift-ci-robot commented Dec 6, 2023 •

edited by openshift-ci bot

mansikulkarni96 Dec 6, 2023 •

edited