Skip to content

fix(cli): stat: explicitly specify resource SI prefix #8206

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.

Sign up for GitHub

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

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Jun 26, 2023
Merged

fix(cli): stat: explicitly specify resource SI prefix #8206

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
ceb2b94
fix(cli): move to explicitly specifying units
johnstcn Jun 26, 2023
7ba09b9
make gen
johnstcn Jun 26, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
31 changes: 17 additions & 14 deletions cli/clistat/cgroup.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
Expand Down Expand Up @@ -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)
Expand All @@ -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)
Expand All @@ -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
}

Expand Down
3 changes: 2 additions & 1 deletion cli/clistat/disk.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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 = "/"
}
Expand All @@ -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
}
3 changes: 2 additions & 1 deletion cli/clistat/disk_windows.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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:\`
}
Expand All @@ -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
}
109 changes: 75 additions & 34 deletions cli/clistat/stat.go
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -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"
Expand All @@ -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
Comment on lines +25 to +27
Copy link
Member Author

Choose a reason for hiding this comment

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

These are the canonical prefix names according to NIST: https://physics.nist.gov/cuu/Units/binary.html

)

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.
Expand All @@ -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 {
Copy link
Member

Choose a reason for hiding this comment

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

Is the typed nil check really necessary here? I would expect this to be writable as r.Total != nil.

Copy link
Member Author

Choose a reason for hiding this comment

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

I found it to be necessary, otherwise the unit tests fail.

_, _ = 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("%)")
}

Expand Down Expand Up @@ -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 {
Expand All @@ -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 {
Expand Down
18 changes: 9 additions & 9 deletions cli/clistat/stat_internal_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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"},
},
} {
Expand Down Expand Up @@ -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)
Expand All @@ -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)
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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)
Expand Down
Loading