Repository navigation
fix(rate): initialise a stat that is new since the previous sample - #3721
Merged
Merged
Conversation
compute_rate_on_list() looked up each stat's previous sample by key and
skipped the stat entirely when there was none. Interfaces, disks and VMs
appear while Glances is running -- a VPN comes up, a container starts, a
disk is plugged in -- so a stat with no previous sample is not only a
start-up case, and the plugins that carry rate fields on a list are
network, diskio and vms.
A skipped stat kept the raw psutil counter in the delta field and got no
_gauge, no _rate_per_sec and no time_since_update at all. Two consequences:
* That refresh published the whole since-boot total as if it were one
refresh of traffic. On a quiet link the first sample of a new
interface reads in gigabytes.
* NetworkPlugin.update_views() assigns
views/<iface>/bytes_recv_rate_per_sec directly. Views are built only
from the fields a stat actually carries, so there was no view to
decorate and the refresh raised KeyError: 'bytes_recv_rate_per_sec'.
Give compute_rate an empty previous instead of skipping, which is the
path a dict stat already takes on its own first sample: record the gauge,
publish a rate of 0, and measure a real delta on the next sample. The
stat then also reaches its second sample one refresh sooner, because the
gauge it needs is now recorded the moment it appears.
Verified on Windows with the full suite: the set of failing test names is
byte-identical before and after (63 pre-existing environment failures,
xmlrpc and browser-restful servers). Reverting the one changed line turns
4 of the 5 new tests red, including the KeyError one; the fifth is the
guard that the neighbouring stat is undisturbed, so it stays green.
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
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.
compute_rate_on_list()looks up each stat's previous sample by key and skips the stat when there is none:The dict path treats a missing previous as a first sample and initialises the stat. The list path skipped it. Interfaces, disks and VMs appear while Glances is running — a VPN comes up, a container starts, a disk is plugged in — so this is not only a start-up case.
network,diskioandvmsall carry rate fields on a list.A skipped stat keeps the raw psutil counter in the delta field and gets no
_gauge, no_rate_per_secand notime_since_update:Two consequences
The refresh publishes the lifetime total as one refresh of traffic.
bytes_recvis documented as the delta, and for one sample it holds the since-boot counter instead. On a quiet link that first sample reads in gigabytes.NetworkPlugin.update_views()raises. It assignsviews/<iface>/bytes_recv_rate_per_secdirectly, andupdate_views()in the base class builds views only fromlistkeys(i)— the fields a stat actually carries. With no rate field there is no view to decorate:Driving the real plugin from a fake
psutil.net_io_counters, one interface appearing is enough to reach it.diskiohappens to survive because it guards withif f'{field}_rate_per_sec' in disk_views;networkdoes not.The change
One line: pass an empty previous rather than skipping.
compute_ratealready handles an empty previous — it is the branch a dict stat takes on its own first sample. The gauge is recorded, the rate reads 0, and the next sample measures a real delta. That also gets the stat to its second sample one refresh sooner, since the gauge it needs now exists the moment it appears.I did not add a guard in
network/update_views()instead, because the missing views are a symptom: the stat was left in a shape no other stat is ever in, andvmsanddiskiowere publishing wrong numbers from the same cause without raising.Verification
tests/test_rate_on_list.py, 5 tests: four on the shared rate code through a minimal list plugin, one end-to-end throughNetworkPluginfor theKeyError.Checked by breaking it — reverting the one changed line back to the skip:
update_viewssurvives an interface appearingKeyErrorFull suite on Windows, before and after: the set of failing test names is byte-identical (63 pre-existing environment failures — the xmlrpc and browser-restful servers).
ruff checkandruff format --checkclean.