Add disk and memory percent_used #825

kasbert · 2023-09-26T11:18:55Z

Add percentage of disk and memory usage based on the values in disk/bytes_used and memory/bytes_used .

Includes stackdriver configuration with "custom.googleapis.com" as it is not possible to add metrics to "compute.googleapis.com".

Previous PR 824 had a wrong committer.

k8s-ci-robot · 2023-09-26T11:19:04Z

Hi @kasbert. Thanks for your PR.

I'm waiting for a kubernetes member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

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.

hakman · 2023-09-26T12:42:34Z

/ok-to-test

hakman · 2023-10-08T06:24:43Z

/lgtm
/cc @MartinForReal @mmiranda96
/assign @dchen1107 @vteratipally

MartinForReal · 2023-10-08T06:49:01Z

/lgtm

mmiranda96 · 2023-10-09T16:39:08Z

pkg/systemstatsmonitor/memory_collector_unix.go

@@ -58,6 +58,29 @@ func (mc *memoryCollector) collect() {
 		}
 	}

+	if mc.mPercentUsed != nil && meminfo.MemTotal != nil && *meminfo.MemTotal > 0{


Why do we need so many percentage memory metrics (and only for Unix)? Shouldn't it just be "used"?

That is a copy from mBytesUsed section.
Personally I don't find much use for other than 'used' metrics and I am happy to remove them.

Yes, please remove it. Thanks!

@mmiranda96 I made the changes, please check.

kasbert · 2023-10-29T06:56:07Z

/retest-required

kasbert · 2023-10-31T03:53:07Z

/retest

hakman · 2023-10-31T04:30:07Z

@kasbert Seems unrelated:

Oct 31 04:03:58 systemd[1]: Started Node problem detector.
 node-problem-detector[723]: /home/kubernetes/bin/node-problem-detector: /lib64/libc.so.6: version `GLIBC_2.34' not found (required by /home/kubernetes/bin/node-problem-detector)
Oct 31 04:03:58 systemd[1]: node-problem-detector.service: Main process exited, code=exited, status=1/FAILURE
Oct 31 04:03:58 systemd[1]: node-problem-detector.service: Failed with result 'exit-code'.
Oct 31 04:04:08 systemd[1]: node-problem-detector.service: Scheduled restart job, restart counter is at 1.
Oct 31 04:04:08 systemd[1]: Stopped Node problem detector.

hakman · 2023-11-01T02:54:47Z

/retest

mmiranda96

One small suggestion, but not blocking.

/lgtm

mmiranda96 · 2023-11-17T19:11:15Z

pkg/systemstatsmonitor/disk_collector.go

@@ -291,6 +303,9 @@ func (dc *diskCollector) collect() {
 		opttypes := strings.Join(partition.Opts, ",")
 		dc.mBytesUsed.Record(map[string]string{deviceNameLabel: deviceName, fsTypeLabel: fstype, mountOptionLabel: opttypes, stateLabel: "free"}, int64(usageStat.Free))
 		dc.mBytesUsed.Record(map[string]string{deviceNameLabel: deviceName, fsTypeLabel: fstype, mountOptionLabel: opttypes, stateLabel: "used"}, int64(usageStat.Used))
+		if dc.mPercentUsed != nil {
+			dc.mPercentUsed.Record(map[string]string{deviceNameLabel: deviceName, fsTypeLabel: fstype, mountOptionLabel: opttypes, stateLabel: "used"}, float64(usageStat.UsedPercent))


Wouldn't dc.mPercentused always be initialized? If we fail to do so, we fail in line 162, right? We can probably remove the nil check (to match the other metrics).

vteratipally · 2023-12-15T13:11:19Z

/lgtm
/approve

k8s-ci-robot · 2023-12-15T13:11:27Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: hakman, kasbert, MartinForReal, mmiranda96, vteratipally

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

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

wangzhen127 · 2025-03-19T03:53:00Z

@kasbert This PR has caused us trouble in GKE as the custom metric unexpectedly charged GKE customers. Even though we are switching over to compute.googleapis.com in #1024, I wonder why we need this given we have bytes_used for disk and memory. Aren't those good enough already? We are consulting with COS team within Google and we are considering reverting this PR if this is not really needed. So please let me know if you have any use case here. Thanks!

kasbert · 2025-03-19T06:11:27Z

@wangzhen127 Percentage is more useful for alerts and metrics. Having them calculated makes things easier. CPU utilization has a percentage already. GCP Ops Agent has these percentages, too.

k8s-ci-robot added the cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. label Sep 26, 2023

k8s-ci-robot requested review from derekwaynecarr and mmiranda96 September 26, 2023 11:19

k8s-ci-robot added the needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. label Sep 26, 2023

k8s-ci-robot added the size/M Denotes a PR that changes 30-99 lines, ignoring generated files. label Sep 26, 2023

k8s-ci-robot added ok-to-test Indicates a non-member PR verified by an org member that is safe to test. and removed needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. labels Sep 26, 2023

k8s-ci-robot assigned dchen1107 and vteratipally Oct 8, 2023

k8s-ci-robot requested a review from MartinForReal October 8, 2023 06:24

k8s-ci-robot assigned hakman Oct 8, 2023

k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Oct 8, 2023

k8s-ci-robot assigned MartinForReal Oct 8, 2023

mmiranda96 reviewed Oct 9, 2023

View reviewed changes

Add disk and memory percent_used

0790063

kasbert force-pushed the master branch from 73a2d8f to 0790063 Compare October 28, 2023 13:04

k8s-ci-robot removed the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Oct 28, 2023

hakman mentioned this pull request Oct 31, 2023

npd: Switch pull-npd-e2e-test to cos-stable kubernetes/test-infra#31159

Merged

mmiranda96 approved these changes Nov 17, 2023

View reviewed changes

k8s-ci-robot assigned mmiranda96 Nov 17, 2023

k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Nov 17, 2023

k8s-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Dec 15, 2023

k8s-ci-robot merged commit bdaa44e into kubernetes:master Dec 15, 2023

AndreMiras mentioned this pull request Dec 16, 2023

Introduce disk/percent_used metric, refs #822 #841

Closed

AndreMiras mentioned this pull request Jan 29, 2024

Disk and memory percent used #822

Closed

wangzhen127 mentioned this pull request Mar 11, 2025

Move disk and memory metrics in custom group to compute #1024

Merged

Add disk and memory percent_used #825

Add disk and memory percent_used #825

Conversation

kasbert commented Sep 26, 2023

Uh oh!

k8s-ci-robot commented Sep 26, 2023

Uh oh!

hakman commented Sep 26, 2023

Uh oh!

hakman commented Oct 8, 2023

Uh oh!

MartinForReal commented Oct 8, 2023

Uh oh!

mmiranda96 Oct 9, 2023

Choose a reason for hiding this comment

Uh oh!

kasbert Oct 22, 2023

Choose a reason for hiding this comment

Uh oh!

mmiranda96 Oct 23, 2023

Choose a reason for hiding this comment

Uh oh!

kasbert Nov 12, 2023

Choose a reason for hiding this comment

Uh oh!

kasbert commented Oct 29, 2023

Uh oh!

kasbert commented Oct 31, 2023

Uh oh!

hakman commented Oct 31, 2023

Uh oh!

hakman commented Nov 1, 2023

Uh oh!

mmiranda96 left a comment

Choose a reason for hiding this comment

Uh oh!

mmiranda96 Nov 17, 2023

Choose a reason for hiding this comment

Uh oh!

vteratipally commented Dec 15, 2023

Uh oh!

k8s-ci-robot commented Dec 15, 2023

Uh oh!

wangzhen127 commented Mar 19, 2025

Uh oh!

kasbert commented Mar 19, 2025

Uh oh!

Uh oh!