Skip to content

fix(diskio): alert on the bitrate, and decorate the field the WebUI reads - #3684

Merged
nicolargo merged 1 commit into
nicolargo:developfrom
ntdatt812:fix/diskio-alert-on-rate
Aug 26, 2026
Merged

nicolargo merged 1 commit into
nicolargo:developfrom
ntdatt812:fix/diskio-alert-on-rate

Conversation

@ntdatt812

Copy link
Copy Markdown
Contributor

Two things, one line apart, and the network plugin next door already does both correctly.

The alert measured a lifetime counter against a bitrate threshold

alert_rx = self.get_alert(i['read_bytes'], header='rx', action_key=disk_real_name)

read_bytes carries rate: True in fields_description, which is what produces read_bytes_rate_per_sec — read_bytes itself is psutil's counter since boot. It only ever grows, so on any machine with real uptime a configured ..._rx_critical was crossed within seconds of the first read and stayed crossed for the life of the process, regardless of what the disk was doing. An operator who set thresholds got a permanently red column; one who set none saw nothing, which is why this could sit unnoticed.

The network plugin is the shape this should have had:

bps_rx = int(i['bytes_recv_rate_per_sec'] * 8)
alert_rx = self.get_alert(bps_rx, header='rx', action_key=if_real_name)

The decoration went to a key one interface reads and the other does not

update_views set it only on read_bytes / write_bytes:

  • the TUI prints read_bytes_rate_per_sec (auto_unit(i.get('read_bytes_rate_per_sec'))) but reads read_bytes' decoration — a colour computed from a different quantity than the number beside it;
  • the WebUI asks for getDecoration(disk.name, 'read_bytes_rate_per_sec'), which nothing ever set, so it rendered no colour at all.

Network sets both keys for exactly this reason:

self.views[...]['bytes_recv']['decoration'] = alert_rx
self.views[...]['bytes_recv_rate_per_sec']['decoration'] = alert_rx

Diskio now does the same, guarded: the _rate_per_sec view only exists once there is a timespan to divide by, and setting it unconditionally raises KeyError on the first sample — the repo's existing TestDiskioPluginViews cases catch that, which is how I found it.

Tests

tests/test_plugin_diskio.py, new TestDiskioBitrateAlert, four cases over injected stats carrying a large lifetime counter and a separate rate:

  • quiet disk (rate 10) with 5 GB of history → OK — this is the case the old code got wrong
  • busy disk (rate 95) → CRITICAL
  • read_bytes_rate_per_sec decorated CRITICAL while write_bytes_rate_per_sec is OK — the WebUI's key, and asymmetric so a blanket assignment cannot pass it
  • first sample with no rate field at all → no KeyError

Thresholds in the fixture are named diskio_sda_rx_*: get_stat_name() composes plugin + action_key + header, so a per-disk threshold carries the disk name, exactly as the network plugin's docstring documents with network_wlan0_rx_careful.

Mutation-checked, one per defect:

mutation result
alert on i['read_bytes'] again 2 fail — the quiet-disk case and the WebUI-key case
stop setting *_rate_per_sec 1 fail — only the WebUI-key case
python -m pytest tests/test_plugin_diskio.py -q
41 passed

(37 before this PR, plus the 4 added.) Python only, so no WebUI rebuild — the bundle already reads whatever the plugin publishes.

…eads

The rx/tx thresholds describe a bitrate, but update_views measured them
against read_bytes / write_bytes -- the lifetime counters psutil returns.
Those only grow, so on any machine with uptime a configured threshold latched
on within seconds and never came back down, whatever the disk was doing.
The network plugin next door already alerts on bytes_recv_rate_per_sec.

The decoration also went only to the counter keys. The TUI prints the rate and
reads read_bytes' decoration, so it showed a colour derived from the wrong
quantity; the WebUI asks for read_bytes_rate_per_sec, which nothing set, so it
showed no colour at all. Both keys now carry it, matching network again.

The rate view exists only once there is a timespan to divide by, so the rate
key is set when present -- setting it unconditionally raises KeyError on the
first sample, which the existing view tests catch.
@nicolargo
nicolargo merged commit 32344ad into nicolargo:develop Aug 26, 2026
17 of 19 checks passed
@nicolargo

Copy link
Copy Markdown
Owner

Thanks for the PR @ntdatt812

Merged into develop.

@nicolargo nicolargo added this to the Glances 4.5.7 milestone Aug 26, 2026
nicolargo pushed a commit that referenced this pull request Aug 27, 2026
`update_views` skipped the whole interface when either direction measured
0 bytes/s:

    if not i.get('bytes_recv_rate_per_sec') or not i.get('bytes_sent_rate_per_sec'):
        continue

The guard is `or`, so one idle direction suppressed the alert for *both*. A
send-only interface saturating its uplink went undecorated because its rx rate
happened to be exactly 0, and a receive-only one — a monitor/SPAN port — the
same way. 0 bytes/s is a real measurement, not a missing one.

The rates now default to 0 instead of skipping, which is what the diskio plugin
already does: it dropped this same guard in #3684 and reads the rate through
`or 0` for the same reason.

The WebUI needs no change — `plugin-network.vue` renders `getDecoration(...)`
straight from the server-supplied view, so it inherits this.

Tests: four cases in `TestNetworkPluginZeroRateAlerts`. Three fail before this
change (saturated tx with idle rx, saturated rx with idle tx, and a fully idle
interface reading DEFAULT instead of OK); the fourth — both directions busy —
passes before and after, which is the path that already worked.
`tests/test_plugin_network.py` 39 passed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants