Feat: Add disk utilization column to webUI #2375
Conversation
WalkthroughExtends per-disk IO data to include a new utilization metric, updates data collection to read six fields from /proc/diskstats, and adjusts UI rendering to add a new percentage column. All related table colspans and totals/slots/preclear sections are incremented to align with the additional column. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant Kernel as Kernel (/proc/diskstats)
participant DiskLoad as nchan/disk_load
participant Nchan as nchan broker
participant DeviceList as nchan/device_list (renderer)
participant Browser as Web UI
Note over DiskLoad: Initialization
Kernel-->>DiskLoad: Read per-disk stats (6 fields)
DiskLoad->>DiskLoad: Store reads/writes/use_time
Note over DiskLoad: Poll loop (interval t)
Kernel-->>DiskLoad: Read updated stats (6 fields)
DiskLoad->>DiskLoad: Compute util = Δuse_time / (t·1000) · 100<br/>Clamp to 0–100
DiskLoad-->>Nchan: Publish metrics [reads, writes, ..., util]
Nchan-->>DeviceList: Push updated metrics
DeviceList->>DeviceList: Map data[4] → percentage column<br/>Adjust colspans/totals/slots
DeviceList-->>Browser: Render table with new % column
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches🧪 Generate unit tests
Tip 👮 Agentic pre-merge checks are now available in preview!Pro plan users can now enable pre-merge checks in their settings to enforce checklists before merging PRs.
Please see the documentation for more information. Example: reviews:
pre_merge_checks:
custom_checks:
- name: "Undocumented Breaking Changes"
mode: "warning"
instructions: |
Pass/fail criteria: All breaking changes to public APIs, CLI flags, environment variables, configuration keys, database schemas, or HTTP/GraphQL endpoints must be documented in the "Breaking Change" section of the PR description and in CHANGELOG.md. Exclude purely internal or private changes (e.g., code not exported from package entry points or explicitly marked as internal).Please share your feedback with us on this Discord post. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
emhttp/plugins/dynamix/nchan/device_list (1)
303-307: Colspan off by one in DISK_NP_DSBL (online).This path still uses colspan='4', but the table now has one extra column. Use 5 to align.
- $echo[] = "<td colspan='4'></td>"; + $echo[] = "<td colspan='5'></td>";
🧹 Nitpick comments (5)
emhttp/plugins/dynamix/nchan/disk_load (4)
10-10: Anchor to device field and include all NVMe namespaces.Limit the AWK match to $3 (device name) and support nvmeNnM (not only n1) to avoid false matches and omissions.
-stats=($(awk '/(sd[a-z]*|nvme[0-9]*n1|vd[a-z]*) /{print $3,$6,$10,$4,$8,$13}' /proc/diskstats)) +stats=($(awk '$3 ~ /^(sd[a-z]+|nvme[0-9]+n[0-9]+|vd[a-z]+)$/{print $3,$6,$10,$4,$8,$13}' /proc/diskstats))And similarly inside the loop:
- stats=($(awk '/(sd[a-z]*|nvme[0-9]*n1|vd[a-z]*) /{print $3,$6,$10,$4,$8,$13}' /proc/diskstats)) + stats=($(awk '$3 ~ /^(sd[a-z]+|nvme[0-9]+n[0-9]+|vd[a-z]+)$/{print $3,$6,$10,$4,$8,$13}' /proc/diskstats))Also applies to: 21-21
23-31: Clamp util to [0,100] and handle counter reset/wrap.Negative deltas can occur after device reset or wrap. Guard and clamp.
- util=$((((stats[i+5]-use_time[c])*100)/(t*1000))) - if [ $util -gt 100 ]; then util=100; fi + delta_ms=$((stats[i+5]-use_time[c])) + if [ $delta_ms -lt 0 ]; then delta_ms=0; fi + util=$(( (delta_ms*100)/(t*1000) )) + if [ $util -gt 100 ]; then util=100; fi + if [ $util -lt 0 ]; then util=0; fi
20-20: Ensure atomic refresh by truncating the temp file each cycle.If the script crashes before mv, a stale tmp file could accumulate lines. Truncate at loop start.
while :; do - stats=($(awk '$3 ~ /^(sd[a-z]+|nvme[0-9]+n[0-9]+|vd[a-z]+)$/{print $3,$6,$10,$4,$8,$13}' /proc/diskstats)) + : > "$tmp" + stats=($(awk '$3 ~ /^(sd[a-z]+|nvme[0-9]+n[0-9]+|vd[a-z]+)$/{print $3,$6,$10,$4,$8,$13}' /proc/diskstats))Also applies to: 28-28
4-4: Consider keying history by device name (associative arrays).Indexing by position breaks if device enumeration order changes between polls. Associative arrays keyed by ${stats[i]} would be more robust.
emhttp/plugins/dynamix/nchan/device_list (1)
315-315: Nit: format util as integer with non-breaking space.Optional polish for consistency with other numeric cells.
- $echo[] = "<td>" . (_var($data, 4, 0)) . " %</td>"; + $echo[] = "<td>" . intval(_var($data, 4, 0)) . " %</td>";Also applies to: 496-496, 558-558
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
emhttp/plugins/dynamix/nchan/device_list(14 hunks)emhttp/plugins/dynamix/nchan/disk_load(1 hunks)
🔇 Additional comments (3)
emhttp/plugins/dynamix/nchan/device_list (3)
298-298: LGTM on new util column and matching totals/colspans.
- Rendering new “% util” cell (Line 315/496/558): OK.
- Totals row placeholder “-” (Line 339): OK.
- Updated colspans for added column in several spots (Lines 298, 356, 473, 485, 549): OK.
Also applies to: 315-315, 339-339, 356-356, 473-473, 485-485, 496-496, 549-549, 558-558
219-219: LGTM on offline-path colspan adjustments.Updated spans line up with the added util column across offline states.
Also applies to: 224-225, 231-233, 244-249, 256-261
272-276: LGTM on widened diskio vector parsing.Defaulting to five values and summing reads/writes matches the producer format.
| } else { | ||
| $echo[$a][] = "<td colspan='4'></td>"; | ||
| $echo[$a][] = "<td colspan='5'></td>"; | ||
| } |
There was a problem hiding this comment.
Open devices preclear/info colspans should be 4, not 5.
Row now has 7 cells before the filler. To reach 11, use colspan='4'.
- $echo[$a][] = "<td colspan='5'><em>$text</em></td>";
+ $echo[$a][] = "<td colspan='4'><em>$text</em></td>";
...
- $echo[$a][] = "<td colspan='5'></td>";
+ $echo[$a][] = "<td colspan='4'></td>";📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| } else { | |
| $echo[$a][] = "<td colspan='4'></td>"; | |
| $echo[$a][] = "<td colspan='5'></td>"; | |
| } | |
| } else { | |
| $echo[$a][] = "<td colspan='4'></td>"; | |
| } |
🤖 Prompt for AI Agents
In emhttp/plugins/dynamix/nchan/device_list around lines 564 to 566, the filler
cell uses colspan='5' but the row already contains 7 cells before the filler so
the filler should be colspan='4' to make the total 11; change the hardcoded
colspan value from 5 to 4 in that else branch so the table layout aligns
correctly.
🔧 PR Test Plugin AvailableA test plugin has been generated for this PR that includes the modified files. Version: 📥 Installation Instructions:Install via Unraid Web UI:
Alternative: Direct Download
|
2 participants
Summary by CodeRabbit