fix: JSON/CPU correctness bugs + portability, validation, and accuracy improvements#1
Merged
Merged
Conversation
JSON output appended a trailing comma after each section and relied on a broken `sed -i` branch (no file arg, and the comma was already flushed to stdout) to clean up when --no-disk was set. Result: invalid JSON whenever a trailing section was disabled. Switch to leading-comma separators between sections and drop the dead sed branch, so output is valid for any combination of --no-cpu/--no-mem/--no-disk. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
CPU usage is computed as a delta between two samples. With --once the loop ran a single iteration, so there was never a previous sample and CPU always reported 0.00. Prime an initial sample and wait one interval before the single measurement when running once. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Previously each -p overwrote the prior value, so `-p /a -p /b` monitored only /b. Append instead, so repeated flags and comma-separated lists can be mixed freely (`-p /a -p /b,/c` → /a,/b,/c). The comma-split consumers are unchanged. Update --help, the header usage comment, and the README. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The comma-split loop over MON_PATHS was duplicated in print_table and print_json. Extract a single for_each_path helper that splits the paths and invokes a per-mode callback (disk_row_table / disk_obj_json), so the splitting logic lives in one place. No behavior change. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
`df -PB1` relied on the block-size flag -B, a GNU coreutils extension that BusyBox df (Alpine and most minimal container images) does not support, so disk stats reported N/A there despite the README claiming Alpine support. Use `df -P -k` (POSIX, 1K blocks) and convert KiB to bytes in awk, keeping all downstream math in bytes. Verified disk numbers now populate on a non-GNU df. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Value-taking flags did an unconditional `shift 2`, so passing one as the last argument (e.g. `cgstats.sh -i`) aborted with a raw "can't shift" error. Non-numeric values (`-i abc`) silently propagated into sleep/awk and failed opaquely. Add is_uint/is_num/is_pct helpers and a die() reporter, then validate value presence and type per flag: -i and --cpu-limit accept numbers (decimals allowed), --mem-limit an integer, and the warn/crit thresholds 0-100. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
--output previously accepted any string and silently fell back to table output for anything other than "json", so a typo like `--output jsno` gave table output with no warning. Validate against table|json at parse time and error out otherwise. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
`date +%s.%N` falls back to `date +%s` only when the first command exits non-zero. BusyBox date instead emits a literal "N" (e.g. 1700000000.N) with a zero exit, so the fallback never fired and the timestamp fed garbage into the CPU delta math. Detect a non-numeric timestamp and re-read whole seconds. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
DISK_AVAIL actually held the used-bytes value (df column 3), which was misleading; rename to DISK_USED across disk_calc and both render callbacks. Also correct the stale `# json` comment on OUTPUT_FORMAT. No behavior change. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
memory.current (v2) and memory.usage_in_bytes (v1) include the page cache, so reported memory and the used% could read far above what processes actually hold. Subtract inactive_file (total_inactive_file on v1) from memory.stat to approximate the working set, matching kubectl top / cAdvisor. Guarded so a missing or oversized value leaves the raw figure untouched. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
antholzer
reviewed
Jun 24, 2026
|
Only help docs can be improved otherwise looks good. I tested it locally inside alpine- and archlinux containers and it worked fine. |
Contributor
Author
good point, updated help message and notes in the beginning of the file. |
antholzer
approved these changes
Jun 24, 2026
| Usage: $0 [-i SECONDS] [-p PATHS] | ||
| cgstats.sh — live CPU/MEM/DISK via cgroups (POSIX sh) | ||
|
|
||
| Reports CPU, memory, and disk usage for the current cgroup (v1 or v2) as a |
There was a problem hiding this comment.
Suggested change
| Reports CPU, memory, and disk usage for the current cgroup (v1 or v2) as a | |
| Continuously reports CPU, memory, and disk usage for the current cgroup (v1 or v2) as a |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Fixes two output-correctness bugs and hardens the script for minimal
container environments (BusyBox/Alpine), plus input validation and a more
accurate memory figure.
Bug fixes
sed -icleanup and switched toleading-comma separators, so output is valid for any combination of
--no-cpu/--no-mem/--no-disk(previously--no-disk --output jsonemitted a trailing comma → invalid JSON).
--once: prime an initial sample so a single-shot run reportsreal usage instead of always 0.00.
df: usedf -P -kinstead of GNU-onlydf -PB1, so diskstats work on BusyBox/Alpine (previously reported N/A there).
date: fall back to whole seconds whendatelacks%N(BusyBox emits a literal
Nwith a zero exit, so the old fallback neverfired and corrupted the CPU delta).
inactive_file(reclaimable page cache)from
memory.current/usage_in_bytes, matching kubectl top / cAdvisor.Features
-pcan now be repeated and/or comma-separated; all paths are merged.--outputrejects values other thantable/jsoninstead of silentlyfalling back to table.
validated with clear errors.
Refactor / chore
for_each_pathhelper (deduped the split loops).DISK_AVAIL→DISK_USED; fixed a stale comment..gitignorefor.DS_Store.