CA-404597 - rrd: Fix incorrect processing of Gauge and Absolute data sources #6233
Conversation
This file contains 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
…es correctly Other unit tests only verify the interoperability of the RRDs - dumping them to JSON/XML and reading back in, verifying that the same data was decoded. We're now seeing a problem where Gauge data sources, which should be absolute values provided by the plugin, fluctuate wildly when processed by the RRD library. Ensure we have an easy way to test this for both Gauge and Absolute data sources - these values should be passed as-is by the RRD library, without any time-based transformations. This test currently fails and will be passing with the fix commits. Signed-off-by: Andrii Sultanov <[email protected]>
lindig
approved these changes
Jan 16, 2025
psafont
reviewed
Jan 16, 2025
Some recent changes related to RRDs likely exposed a long-standing latent issue where the RRD library would process the passed-in values for Gauge and Absolute data sources incorrectly leading to constant values changing from update to update, for example: ``` $ rrd2csv memory_total_kib timestamp, AVERAGE:host:8b533333-91e1-4698-bd17-95b9732ffbb6:memory_total_kib 2025-01-15T08:41:40Z, 33351000 2025-01-15T08:41:45Z, 33350000 2025-01-15T08:41:50Z, 33346000 2025-01-15T08:41:55Z, 33352000 ``` Instead of treating Gauge and Absolute data sources as a variation on the rate-based Derive data source type, expecting time-based calculations to cancel each other out, do not undertake any calculations on non-rate data sources at all. This makes the unit test added in the previous commit pass. Signed-off-by: Andrii Sultanov <[email protected]>
last-genius
force-pushed
the
private/asultanov/CA-404597
branch
from
0fc9c5f to
73ca3cc
Compare
robhoes
approved these changes
Jan 16, 2025
psafont
approved these changes
Jan 16, 2025
|
I'm retesting this just in case after the latest changes suggested by Pau and will merge if it looks good. |
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.
Some recent changes related to RRDs likely exposed a long-standing latent issue
where the RRD library would process the passed-in values for Gauge and Absolute
data sources incorrectly leading to constant values changing from update to
update, for example:
Instead of treating Gauge and Absolute data sources as a
variation on the rate-based Derive data source type, expecting time-based
calculations to cancel each other out, do not undertake any calculations on
non-rate data sources at all.
First commit adds a failing unit test, second makes it pass.
===
I've verified these changes through manual testing, they've also passed the testcases that discovered this issue: SNMP memory testcases (JobIDs 4197305, 4196759, 4196744) and ShimMemory testcase (4197050). This branch also passed Ring3 BST+BVT (210577)