-
Notifications
You must be signed in to change notification settings - Fork 0
fix: correct CPU usage graph pinned at 100% #14
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 our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -37,6 +37,7 @@ type HostMetrics struct { | |
| MemoryUsedBytes int64 | ||
| MemoryFreeBytes int64 | ||
| CpuSecondsTotal float64 | ||
| CpuSecondsIdle float64 | ||
| LoadAvg1 float64 | ||
| LoadAvg5 float64 | ||
| LoadAvg15 float64 | ||
|
|
@@ -145,6 +146,10 @@ func ScrapePrometheus(metricsPort int) ScrapeResult { | |
| sr.Host.MemoryFreeBytes += int64(value) | ||
| case "host_cpu_seconds_total": | ||
| sr.Host.CpuSecondsTotal += value | ||
| mode := labels["mode"] | ||
| if mode == "idle" || mode == "iowait" { | ||
| sr.Host.CpuSecondsIdle += value | ||
| } | ||
|
Comment on lines
+150
to
+152
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Consider excluding Classifying Standard tools like If the intent is "CPU not doing compute work", renaming the field to Prompt To Fix With AIThis is a comment left during a code review.
Path: agent/internal/metrics/scraper.go
Line: 150-152
Comment:
**Consider excluding `iowait` from the idle bucket**
Classifying `iowait` as "idle" means I/O-bound workloads will show artificially low CPU utilization on the graph. For example, a system that's 80% blocked on disk reads will report ~20% CPU busy even though it's clearly under stress.
Standard tools like `iostat` and `htop` report `iowait` as a separate category precisely to make I/O pressure visible. The field is also named `CpuSecondsIdle`, which implies pure idle time.
If the intent is "CPU not doing compute work", renaming the field to `CpuSecondsNonBusy` (and documenting that it includes iowait) would at least make the semantics explicit. Alternatively, tracking `idle` only and displaying `iowait` as a separate series in the chart gives users richer diagnostic information.
How can I resolve this? If you propose a fix, please make it concise. |
||
| case "host_load1": | ||
| sr.Host.LoadAvg1 += value | ||
| case "host_load5": | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,2 @@ | ||
| -- AlterTable | ||
| ALTER TABLE "NodeMetric" ADD COLUMN "cpuSecondsIdle" DOUBLE PRECISION NOT NULL DEFAULT 0; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Misaligned indentation on new field
The
CpuSecondsIdleline uses one fewer tab than every other field in the same struct literal. Runninggofmtwould flag this. While it compiles fine, it breaks visual alignment and will cause noisy diffs in future edits.Prompt To Fix With AI