NotificationsYou must be signed in to change notification settings
Fork913
Star10k

feat(cli): add`coder stat` command#8005

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to ourterms of service andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Jump to bottom

Merged

mtojek merged 48 commits intomainfromcj/coder-stat

Jun 20, 2023

Merged

feat(cli): add`coder stat` command#8005

mtojek merged 48 commits intomainfromcj/coder-stat

Jun 20, 2023

Conversation

Copy link

Member

johnstcn commentedJun 13, 2023•
edited
Loading

Adds acoder stat command that emits resource information about the current workspace host or container.

$ coder statHOST CPU        HOST MEMORY     HOME DISK         CONTAINER CPU   CONTAINER MEMORY  7.5/64.0 cores  246.5/251.4 GB  1225.8/3050.4 GB  0.1/64.0 cores  4.6/15.4 GB   $ coder stat cpu0.7/8.0 cores$ coder stat cpu --host0.1/16.0 cores$ coder stat disk --path $HOME --prefix Ti0.3/0.9 TiB$ coder stat disk --path $HOME 308.3/926.4 GiB

Usage in agent metadata:

  metadata {    display_name = "Host CPU"    interval     = 1    timeout      = 1    key          = "0_host_cpu"    script       = "coder stat cpu --host"  }  metadata {    display_name = "Host Memory"    interval     = 1    timeout      = 1    key          = "1_host_mem"    script       = "coder stat mem --host"  }  metadata {    display_name = "Home Disk"    interval     = 1    timeout      = 1    key          = "2_home_disk"    script       = "coder stat disk --path $${HOME}"  }  metadata {    display_name = "Container CPU"    interval     = 1    timeout      = 1    key          = "3_container_cpu"    script       = "coder stat cpu"  }  metadata {    display_name = "Container Memory"    interval     = 1    timeout      = 1    key          = "4_container_cpu"    script       = "coder stat mem"  }

What this looks like in practice:

Also updated docs and sample docker/kubernetes templates with example usage.

~~Open question: would it make sense to break thestat command into individual sub-commands (coder stat cpu,coder stat container_mem)?~~

~~Partially repairs~~Fixes#7076

johnstcnand others added27 commits

June 13, 2023 17:33

add stat command

5db9006

cpu working on mac

d6029b4

add stat memory

18f4942

support values with no total

251fdda

move clistats to its own package

4c081dc

fix container detection to work with sysbox containers

2ba7392

add cross-platform declaration for IsContainerized()

0e1c96a

add a sync.Once to IsContainerized()

0f9859e

make uptime minutes

a220c7f

lint

89f7e8d

extract nproc to variable

c51e245

add skeleton of cgroup stuff

3528c00

initial cgroupv2 cpu implementation

7108c6e

fix disk_windows

4ef5f24

add tests for clistats

f0f7b6a

improve testing

6a878b9

remove unnecessary os-specific implementations now that we have abstr…

be7ba72

…acted out the filesystem

remove uptime stat as it is trivial to implement in bash

3643407

implement cgroupv1 cpu

1c8943e

unskip container memory tests

95b8d1f

flesh out tests

495b5b0

cgroupv1 memory

fa0c4c6

improve tests to allow testing cpu used

70ef79b

refactor cpu usage calc

7eeefc1

fix tests

305675f

fix off-by-10 error

d1bb322

remove --sample-interval and collect CPU stats in parallel

eb2bcf6

johnstcn self-assigned this

Jun 13, 2023

johnstcn added2 commits

June 13, 2023 18:02

fmt; gen

44edcf3

make default_cols consistent to avoid ci surprises

0f3254a

ammario mentioned this pull request

Jun 14, 2023

agent metadata: use magic format to render values nicely#8038

Closed

ammario previously requested changes

Jun 14, 2023

View reviewed changes

Copy link

Member

ammario left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others.Learn more.

In the style of v1, I think we should show percentages alongside the resource values
To make room for percentages and reduce noise, I think we should show at most 3 levels of precision

So246.5/251.4 GB would be246 / 251 GB (90%)

cli/clistat/cgroup.go

Copy link

Member

ammarioJun 14, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others.Learn more.

Happy that you split a lot of this logic into its own package.

cli/clistat/container.goShow resolvedHide resolved

cli/clistat/container.go OutdatedShow resolvedHide resolved

cli/clistat/stat.go OutdatedShow resolvedHide resolved

Copy link

Member

bpmct commentedJun 14, 2023

This is AWESOME! How doescoder stat work if run on a VM?

Copy link

MemberAuthor

johnstcn commentedJun 14, 2023•
edited
Loading

In the style of v1, I think we should show percentages alongside the resource values
To make room for percentages and reduce noise, I think we should show at most 3 levels of precision
So 246.5/251.4 GB would be 246 / 251 GB (90%)

Isn't that 0 levels of precision?
For quantities that are genenerally in the region of 10^1 - 10^2 (such as cores), such precision isn't really useful. For example,2/8 cores (36%).

I'm in favour of percentages, agree with limiting precision up to a point.

Copy link

MemberAuthor

johnstcn commentedJun 14, 2023

This is AWESOME! How doescoder stat work if run on a VM?

There's no difference from its perspective between running in a VM and running on a real piece of metal.

Copy link

Member

ammario commentedJun 14, 2023•
edited
Loading

2. For quantities that are genenerally in the region of 10^1 - 10^2 (such as cores), such precision isn't really useful. For example,2/8 cores (36%).

Precision may not be the right word, I guess I just mean the number of numbers shown.2.23223 or22.3 would all be fine.

Copy link

Member

matifali commentedJun 15, 2023

For quantities that are genenerally in the region of 10^1 - 10^2 (such as cores), such precision isn't really useful. For example,2/8 cores (36%).
Precision may not be the right word, I guess I just mean the number of numbers shown.2.23223 or22.3 would all be fine.

The number of significant figures. I agree with showing 3 at max. for cores that can be limited to 2.

johnstcn added11 commits

June 15, 2023 13:49

add percentages

482db10

remove outdated comments

0775082

handle counter reset

73debf8

add test for large difference between used and total

d0c992a

auto-scale precision, limiting to 3 digits

ef7460a

automatically scale precision, remove --prefix arg

bec527f

make gen

08adba7

improve cli tests

78f76e7

update go.mod

9a82882

Merge remote-tracking branch 'origin/main' into cj/coder-stat

19c8a80

update go.sum

eab2530

johnstcn commented

Jun 15, 2023

View reviewed changes

cli/clistat/stat.go

Comment on lines +50 to +56

		// If the unit prefixes of the used and total values are different,
		// display the used value's prefix to avoid confusion.
		if usedPrefix != totalPrefix \|\| totalDisplay == "" {
		_, _ = sb.WriteString(" ")
		_, _ = sb.WriteString(usedPrefix)
		_, _ = sb.WriteString(r.Unit)
		}

Copy link

MemberAuthor

johnstcnJun 15, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others.Learn more.

review: this means you will see output like16.7 mcores/64 cores (0%) or1 B/100 TB (0%). I investigated scaling to use the same units, but this was non-trivial.