From ceb2b94935e73a672872a53b6cbb7f084cec4421 Mon Sep 17 00:00:00 2001 From: Cian Johnston Date: Mon, 26 Jun 2023 13:35:18 +0100 Subject: [PATCH 1/2] fix(cli): move to explicitly specifying units --- cli/clistat/cgroup.go | 31 +++--- cli/clistat/disk.go | 3 +- cli/clistat/disk_windows.go | 3 +- cli/clistat/stat.go | 109 ++++++++++++++------- cli/clistat/stat_internal_test.go | 18 ++-- cli/stat.go | 45 ++++++--- cli/testdata/coder_stat_cpu_--help.golden | 3 - cli/testdata/coder_stat_disk_--help.golden | 3 + cli/testdata/coder_stat_mem_--help.golden | 3 + 9 files changed, 143 insertions(+), 75 deletions(-) diff --git a/cli/clistat/cgroup.go b/cli/clistat/cgroup.go index e22f0c4309b7a..f94e14405f1ff 100644 --- a/cli/clistat/cgroup.go +++ b/cli/clistat/cgroup.go @@ -84,9 +84,10 @@ func (s *Statter) ContainerCPU() (*Result, error) { } r := &Result{ - Unit: "cores", - Used: used2 - used1, - Total: ptr.To(total), + Unit: "cores", + Used: used2 - used1, + Total: ptr.To(total), + Prefix: PrefixDefault, } return r, nil } @@ -184,20 +185,20 @@ func (s *Statter) cGroupV1CPUUsed() (float64, error) { // ContainerMemory returns the memory usage of the container cgroup. // If the system is not containerized, this always returns nil. -func (s *Statter) ContainerMemory() (*Result, error) { +func (s *Statter) ContainerMemory(p Prefix) (*Result, error) { if ok, err := IsContainerized(s.fs); err != nil || !ok { return nil, nil //nolint:nilnil } if s.isCGroupV2() { - return s.cGroupV2Memory() + return s.cGroupV2Memory(p) } // Fall back to CGroupv1 - return s.cGroupV1Memory() + return s.cGroupV1Memory(p) } -func (s *Statter) cGroupV2Memory() (*Result, error) { +func (s *Statter) cGroupV2Memory(p Prefix) (*Result, error) { maxUsageBytes, err := readInt64(s.fs, cgroupV2MemoryMaxBytes) if err != nil { return nil, xerrors.Errorf("read memory total: %w", err) @@ -214,13 +215,14 @@ func (s *Statter) cGroupV2Memory() (*Result, error) { } return &Result{ - Total: ptr.To(float64(maxUsageBytes)), - Used: float64(currUsageBytes - inactiveFileBytes), - Unit: "B", + Total: ptr.To(float64(maxUsageBytes)), + Used: float64(currUsageBytes - inactiveFileBytes), + Unit: "B", + Prefix: p, }, nil } -func (s *Statter) cGroupV1Memory() (*Result, error) { +func (s *Statter) cGroupV1Memory(p Prefix) (*Result, error) { maxUsageBytes, err := readInt64(s.fs, cgroupV1MemoryMaxUsageBytes) if err != nil { return nil, xerrors.Errorf("read memory total: %w", err) @@ -239,9 +241,10 @@ func (s *Statter) cGroupV1Memory() (*Result, error) { // Total memory used is usage - total_inactive_file return &Result{ - Total: ptr.To(float64(maxUsageBytes)), - Used: float64(usageBytes - totalInactiveFileBytes), - Unit: "B", + Total: ptr.To(float64(maxUsageBytes)), + Used: float64(usageBytes - totalInactiveFileBytes), + Unit: "B", + Prefix: p, }, nil } diff --git a/cli/clistat/disk.go b/cli/clistat/disk.go index 54731dfd9737f..de79fe8a43d45 100644 --- a/cli/clistat/disk.go +++ b/cli/clistat/disk.go @@ -10,7 +10,7 @@ import ( // Disk returns the disk usage of the given path. // If path is empty, it returns the usage of the root directory. -func (*Statter) Disk(path string) (*Result, error) { +func (*Statter) Disk(p Prefix, path string) (*Result, error) { if path == "" { path = "/" } @@ -22,5 +22,6 @@ func (*Statter) Disk(path string) (*Result, error) { r.Total = ptr.To(float64(stat.Blocks * uint64(stat.Bsize))) r.Used = float64(stat.Blocks-stat.Bfree) * float64(stat.Bsize) r.Unit = "B" + r.Prefix = p return &r, nil } diff --git a/cli/clistat/disk_windows.go b/cli/clistat/disk_windows.go index d11995e2c2980..fb7a64db188ac 100644 --- a/cli/clistat/disk_windows.go +++ b/cli/clistat/disk_windows.go @@ -7,7 +7,7 @@ import ( // Disk returns the disk usage of the given path. // If path is empty, it defaults to C:\ -func (*Statter) Disk(path string) (*Result, error) { +func (*Statter) Disk(p Prefix, path string) (*Result, error) { if path == "" { path = `C:\` } @@ -31,5 +31,6 @@ func (*Statter) Disk(path string) (*Result, error) { r.Total = ptr.To(float64(totalBytes)) r.Used = float64(totalBytes - freeBytes) r.Unit = "B" + r.Prefix = p return &r, nil } diff --git a/cli/clistat/stat.go b/cli/clistat/stat.go index dffabd2cec48e..ad3b99c2b264b 100644 --- a/cli/clistat/stat.go +++ b/cli/clistat/stat.go @@ -1,14 +1,12 @@ package clistat import ( - "fmt" "math" "runtime" "strconv" "strings" "time" - "github.com/dustin/go-humanize" "github.com/elastic/go-sysinfo" "github.com/spf13/afero" "golang.org/x/xerrors" @@ -17,15 +15,65 @@ import ( sysinfotypes "github.com/elastic/go-sysinfo/types" ) +// Prefix is a scale multiplier for a result. +// Used when creating a human-readable representation. +type Prefix float64 + +const ( + PrefixDefault = 1.0 + PrefixKibi = 1024.0 + PrefixMebi = PrefixKibi * 1024.0 + PrefixGibi = PrefixMebi * 1024.0 + PrefixTebi = PrefixGibi * 1024.0 +) + +var ( + PrefixHumanKibi = "Ki" + PrefixHumanMebi = "Mi" + PrefixHumanGibi = "Gi" + PrefixHumanTebi = "Ti" +) + +func (s *Prefix) String() string { + switch *s { + case PrefixKibi: + return "Ki" + case PrefixMebi: + return "Mi" + case PrefixGibi: + return "Gi" + case PrefixTebi: + return "Ti" + default: + return "" + } +} + +func ParsePrefix(s string) Prefix { + switch s { + case PrefixHumanKibi: + return PrefixKibi + case PrefixHumanMebi: + return PrefixMebi + case PrefixHumanGibi: + return PrefixGibi + case PrefixHumanTebi: + return PrefixTebi + default: + return PrefixDefault + } +} + // Result is a generic result type for a statistic. // Total is the total amount of the resource available. // It is nil if the resource is not a finite quantity. // Unit is the unit of the resource. // Used is the amount of the resource used. type Result struct { - Total *float64 `json:"total"` - Unit string `json:"unit"` - Used float64 `json:"used"` + Total *float64 `json:"total"` + Unit string `json:"unit"` + Used float64 `json:"used"` + Prefix Prefix `json:"-"` } // String returns a human-readable representation of the result. @@ -34,38 +82,29 @@ func (r *Result) String() string { return "-" } - var usedDisplay, totalDisplay string - var usedScaled, totalScaled float64 - var usedPrefix, totalPrefix string - usedScaled, usedPrefix = humanize.ComputeSI(r.Used) - usedDisplay = humanizeFloat(usedScaled) - if r.Total != (*float64)(nil) { - totalScaled, totalPrefix = humanize.ComputeSI(*r.Total) - totalDisplay = humanizeFloat(totalScaled) + scale := 1.0 + if r.Prefix != 0.0 { + scale = float64(r.Prefix) } var sb strings.Builder - _, _ = sb.WriteString(usedDisplay) - - // 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) - } - - if totalDisplay != "" { + var usedScaled, totalScaled float64 + usedScaled = r.Used / scale + _, _ = sb.WriteString(humanizeFloat(usedScaled)) + if r.Total != (*float64)(nil) { _, _ = sb.WriteString("/") - _, _ = sb.WriteString(totalDisplay) - _, _ = sb.WriteString(" ") - _, _ = sb.WriteString(totalPrefix) - _, _ = sb.WriteString(r.Unit) + totalScaled = *r.Total / scale + _, _ = sb.WriteString(humanizeFloat(totalScaled)) } - if r.Total != nil && *r.Total != 0.0 { + _, _ = sb.WriteString(" ") + _, _ = sb.WriteString(r.Prefix.String()) + _, _ = sb.WriteString(r.Unit) + + if r.Total != (*float64)(nil) && *r.Total > 0 { _, _ = sb.WriteString(" (") - _, _ = sb.WriteString(fmt.Sprintf("%.0f", r.Used/(*r.Total)*100.0)) + pct := r.Used / *r.Total * 100.0 + _, _ = sb.WriteString(strconv.FormatFloat(pct, 'f', 0, 64)) _, _ = sb.WriteString("%)") } @@ -153,8 +192,9 @@ func New(opts ...Option) (*Statter, error) { // Units are in "cores". func (s *Statter) HostCPU() (*Result, error) { r := &Result{ - Unit: "cores", - Total: ptr.To(float64(s.nproc)), + Unit: "cores", + Total: ptr.To(float64(s.nproc)), + Prefix: PrefixDefault, } c1, err := s.hi.CPUTime() if err != nil { @@ -177,9 +217,10 @@ func (s *Statter) HostCPU() (*Result, error) { } // HostMemory returns the memory usage of the host, in gigabytes. -func (s *Statter) HostMemory() (*Result, error) { +func (s *Statter) HostMemory(p Prefix) (*Result, error) { r := &Result{ - Unit: "B", + Unit: "B", + Prefix: p, } hm, err := s.hi.Memory() if err != nil { diff --git a/cli/clistat/stat_internal_test.go b/cli/clistat/stat_internal_test.go index 5433db0135022..d52ef28557d59 100644 --- a/cli/clistat/stat_internal_test.go +++ b/cli/clistat/stat_internal_test.go @@ -33,19 +33,19 @@ func TestResultString(t *testing.T) { Result: Result{Used: 12.34, Total: nil, Unit: ""}, }, { - Expected: "1.54 kB", - Result: Result{Used: 1536, Total: nil, Unit: "B"}, + Expected: "1.5 KiB", + Result: Result{Used: 1536, Total: nil, Unit: "B", Prefix: PrefixKibi}, }, { Expected: "1.23 things", Result: Result{Used: 1.234, Total: nil, Unit: "things"}, }, { - Expected: "1 B/100 TB (0%)", - Result: Result{Used: 1, Total: ptr.To(1000 * 1000 * 1000 * 1000 * 100.0), Unit: "B"}, + Expected: "0/100 TiB (0%)", + Result: Result{Used: 1, Total: ptr.To(100.0 * float64(PrefixTebi)), Unit: "B", Prefix: PrefixTebi}, }, { - Expected: "500 mcores/8 cores (6%)", + Expected: "0.5/8 cores (6%)", Result: Result{Used: 0.5, Total: ptr.To(8.0), Unit: "cores"}, }, } { @@ -76,7 +76,7 @@ func TestStatter(t *testing.T) { t.Run("HostMemory", func(t *testing.T) { t.Parallel() - mem, err := s.HostMemory() + mem, err := s.HostMemory(PrefixDefault) require.NoError(t, err) assert.NotZero(t, mem.Used) assert.NotZero(t, mem.Total) @@ -85,7 +85,7 @@ func TestStatter(t *testing.T) { t.Run("HostDisk", func(t *testing.T) { t.Parallel() - disk, err := s.Disk("") // default to home dir + disk, err := s.Disk(PrefixDefault, "") // default to home dir require.NoError(t, err) assert.NotZero(t, disk.Used) assert.NotZero(t, disk.Total) @@ -159,7 +159,7 @@ func TestStatter(t *testing.T) { fs := initFS(t, fsContainerCgroupV1) s, err := New(WithFS(fs), withNoWait) require.NoError(t, err) - mem, err := s.ContainerMemory() + mem, err := s.ContainerMemory(PrefixDefault) require.NoError(t, err) require.NotNil(t, mem) assert.Equal(t, 268435456.0, mem.Used) @@ -211,7 +211,7 @@ func TestStatter(t *testing.T) { fs := initFS(t, fsContainerCgroupV2) s, err := New(WithFS(fs), withNoWait) require.NoError(t, err) - mem, err := s.ContainerMemory() + mem, err := s.ContainerMemory(PrefixDefault) require.NoError(t, err) require.NotNil(t, mem) assert.Equal(t, 268435456.0, mem.Used) diff --git a/cli/stat.go b/cli/stat.go index 67232caee53c7..3e32c4187f93b 100644 --- a/cli/stat.go +++ b/cli/stat.go @@ -75,7 +75,7 @@ func (r *RootCmd) stat() *clibase.Cmd { } // Host-level stats - ms, err := st.HostMemory() + ms, err := st.HostMemory(clistat.PrefixGibi) if err != nil { return err } @@ -85,7 +85,7 @@ func (r *RootCmd) stat() *clibase.Cmd { if err != nil { return err } - ds, err := st.Disk(home) + ds, err := st.Disk(clistat.PrefixGibi, home) if err != nil { return err } @@ -99,7 +99,7 @@ func (r *RootCmd) stat() *clibase.Cmd { } sr.ContainerCPU = cs - ms, err := st.ContainerMemory() + ms, err := st.ContainerMemory(clistat.PrefixGibi) if err != nil { return err } @@ -120,7 +120,6 @@ func (r *RootCmd) stat() *clibase.Cmd { func (*RootCmd) statCPU(s *clistat.Statter, fs afero.Fs) *clibase.Cmd { var hostArg bool - var prefixArg string formatter := cliui.NewOutputFormatter(cliui.TextFormat(), cliui.JSONFormat()) cmd := &clibase.Cmd{ Use: "cpu", @@ -131,12 +130,6 @@ func (*RootCmd) statCPU(s *clistat.Statter, fs afero.Fs) *clibase.Cmd { Value: clibase.BoolOf(&hostArg), Description: "Force host CPU measurement.", }, - { - Flag: "prefix", - Value: clibase.StringOf(&prefixArg), - Description: "Unit prefix.", - Default: "", - }, }, Handler: func(inv *clibase.Invocation) error { var cs *clistat.Result @@ -164,6 +157,7 @@ func (*RootCmd) statCPU(s *clistat.Statter, fs afero.Fs) *clibase.Cmd { func (*RootCmd) statMem(s *clistat.Statter, fs afero.Fs) *clibase.Cmd { var hostArg bool + var prefixArg string formatter := cliui.NewOutputFormatter(cliui.TextFormat(), cliui.JSONFormat()) cmd := &clibase.Cmd{ Use: "mem", @@ -174,14 +168,26 @@ func (*RootCmd) statMem(s *clistat.Statter, fs afero.Fs) *clibase.Cmd { Value: clibase.BoolOf(&hostArg), Description: "Force host memory measurement.", }, + { + Description: "SI Prefix for memory measurement.", + Default: clistat.PrefixHumanGibi, + Flag: "prefix", + Value: clibase.EnumOf(&prefixArg, + clistat.PrefixHumanKibi, + clistat.PrefixHumanMebi, + clistat.PrefixHumanGibi, + clistat.PrefixHumanTebi, + ), + }, }, Handler: func(inv *clibase.Invocation) error { + pfx := clistat.ParsePrefix(prefixArg) var ms *clistat.Result var err error if ok, _ := clistat.IsContainerized(fs); ok && !hostArg { - ms, err = s.ContainerMemory() + ms, err = s.ContainerMemory(pfx) } else { - ms, err = s.HostMemory() + ms, err = s.HostMemory(pfx) } if err != nil { return err @@ -201,6 +207,7 @@ func (*RootCmd) statMem(s *clistat.Statter, fs afero.Fs) *clibase.Cmd { func (*RootCmd) statDisk(s *clistat.Statter) *clibase.Cmd { var pathArg string + var prefixArg string formatter := cliui.NewOutputFormatter(cliui.TextFormat(), cliui.JSONFormat()) cmd := &clibase.Cmd{ Use: "disk", @@ -212,9 +219,21 @@ func (*RootCmd) statDisk(s *clistat.Statter) *clibase.Cmd { Description: "Path for which to check disk usage.", Default: "/", }, + { + Flag: "prefix", + Default: clistat.PrefixHumanGibi, + Description: "SI Prefix for disk measurement.", + Value: clibase.EnumOf(&prefixArg, + clistat.PrefixHumanKibi, + clistat.PrefixHumanMebi, + clistat.PrefixHumanGibi, + clistat.PrefixHumanTebi, + ), + }, }, Handler: func(inv *clibase.Invocation) error { - ds, err := s.Disk(pathArg) + pfx := clistat.ParsePrefix(prefixArg) + ds, err := s.Disk(pfx, pathArg) if err != nil { return err } diff --git a/cli/testdata/coder_stat_cpu_--help.golden b/cli/testdata/coder_stat_cpu_--help.golden index dba620751ba6d..f9cc1124b2637 100644 --- a/cli/testdata/coder_stat_cpu_--help.golden +++ b/cli/testdata/coder_stat_cpu_--help.golden @@ -9,8 +9,5 @@ Show CPU usage, in cores. -o, --output string (default: text) Output format. Available formats: text, json. - --prefix string - Unit prefix. - --- Run `coder --help` for a list of global options. diff --git a/cli/testdata/coder_stat_disk_--help.golden b/cli/testdata/coder_stat_disk_--help.golden index cb33481f726b0..4a6c0c5cf19da 100644 --- a/cli/testdata/coder_stat_disk_--help.golden +++ b/cli/testdata/coder_stat_disk_--help.golden @@ -9,5 +9,8 @@ Show disk usage, in gigabytes. --path string (default: /) Path for which to check disk usage. + --prefix Ki|Mi|Gi|Ti (default: Gi) + SI Prefix for disk measurement. + --- Run `coder --help` for a list of global options. diff --git a/cli/testdata/coder_stat_mem_--help.golden b/cli/testdata/coder_stat_mem_--help.golden index 0905c38a9639d..189d8da2c36bd 100644 --- a/cli/testdata/coder_stat_mem_--help.golden +++ b/cli/testdata/coder_stat_mem_--help.golden @@ -9,5 +9,8 @@ Show memory usage, in gigabytes. -o, --output string (default: text) Output format. Available formats: text, json. + --prefix Ki|Mi|Gi|Ti (default: Gi) + SI Prefix for memory measurement. + --- Run `coder --help` for a list of global options. From 7ba09b96e449d5cb5f34a16303666c1c74499cdd Mon Sep 17 00:00:00 2001 From: Cian Johnston Date: Mon, 26 Jun 2023 13:51:51 +0100 Subject: [PATCH 2/2] make gen --- docs/cli/stat_cpu.md | 8 -------- docs/cli/stat_disk.md | 9 +++++++++ docs/cli/stat_mem.md | 9 +++++++++ 3 files changed, 18 insertions(+), 8 deletions(-) diff --git a/docs/cli/stat_cpu.md b/docs/cli/stat_cpu.md index 7edc442a1cb33..f86397155d5cc 100644 --- a/docs/cli/stat_cpu.md +++ b/docs/cli/stat_cpu.md @@ -28,11 +28,3 @@ Force host CPU measurement. | Default | text | Output format. Available formats: text, json. - -### --prefix - -| | | -| ---- | ------------------- | -| Type | string | - -Unit prefix. diff --git a/docs/cli/stat_disk.md b/docs/cli/stat_disk.md index 6b6ddc34882c8..be4e8a429e6b2 100644 --- a/docs/cli/stat_disk.md +++ b/docs/cli/stat_disk.md @@ -29,3 +29,12 @@ Output format. Available formats: text, json. | Default | / | Path for which to check disk usage. + +### --prefix + +| | | +| ------- | --------------- | --- | --- | ---------- | +| Type | enum[Ki | Mi | Gi | Ti] | +| Default | Gi | + +SI Prefix for disk measurement. diff --git a/docs/cli/stat_mem.md b/docs/cli/stat_mem.md index 387e7d9ad18cb..f76e2901f9d13 100644 --- a/docs/cli/stat_mem.md +++ b/docs/cli/stat_mem.md @@ -28,3 +28,12 @@ Force host memory measurement. | Default | text | Output format. Available formats: text, json. + +### --prefix + +| | | +| ------- | --------------- | --- | --- | ---------- | +| Type | enum[Ki | Mi | Gi | Ti] | +| Default | Gi | + +SI Prefix for memory measurement.