timex: filter unreasonable offset values from kernel adjtimex() overflow - #3766
Open
dongjiang1989 wants to merge 2 commits into
Open
timex: filter unreasonable offset values from kernel adjtimex() overflow#3766dongjiang1989 wants to merge 2 commits into
dongjiang1989 wants to merge 2 commits into
Conversation
|
Can someone please review this PR? |
SuperQ
requested changes
Aug 18, 2026
SuperQ
left a comment
SuperQ
left a comment
Member
There was a problem hiding this comment.
Improvements to the alert mixin would be reasonable, but they should be much more relaxed than 1 second. What if the actual offset is a year out of date? This would create a false negative situation.
Comment on lines
+188
to
+196
| offsetSec := float64(timex.Offset) / divisor | ||
| if offsetSec > 1.0 || offsetSec < -1.0 { | ||
| c.logger.Warn("Discarding unreasonable timex offset value", | ||
| "offset_seconds", offsetSec, "status", status) | ||
| offsetSec = 0 | ||
| } | ||
|
|
||
| ch <- c.syncStatus.mustNewConstMetric(syncStatus) | ||
| ch <- c.offset.mustNewConstMetric(float64(timex.Offset) / divisor) | ||
| ch <- c.offset.mustNewConstMetric(offsetSec) |
Member
There was a problem hiding this comment.
Sorry, this is not acceptable.
- This is far too tight a constraint.
- We avoid filtering values at collection time.
Member
Author
There was a problem hiding this comment.
Thanks @SuperQ . I Got it.
-
Removed collector-side filtering — raw
adjtimex()values are now preserved as-is. No more masking or replacing offset values. -
Added diagnostic counter
node_timex_offset_overflow_total— increments when|offset| > 4s, providing visibility into KVM/pvclock overflow events without modifying the primary metric. -
Relaxed alert threshold to ±60s — this filters the 4.29s overflow artifact while still catching genuinely broken clocks (minutes/hours off).
On KVM/pvclock guests, a race condition in the kernel NTP PLL can cause adjtimex() to return 2^32 ns (4.294967296s) as a transient overflow value, triggering false-positive NodeClockSkewDetected alerts hundreds of times per day despite NTP being correctly synchronized. Add a ±1.0s sanity bound in the timex collector: values exceeding this threshold are discarded (set to 0) with a warning log, as real NTP sync never produces offsets of this magnitude. Apply the same bound in the NodeClockSkewDetected alert expression to provide defense-in-depth at the rule layer. Fixes prometheus#3764 Signed-off-by: dongjiang <dongjiang1989@126.com>
Address review feedback from @SuperQ: 1. Remove silent offset filtering at collection time - node_exporter should faithfully report kernel values - Do not mask or replace offset values 2. Add node_timex_offset_overflow_total counter - Monotonically counts readings where |offset| > 4s - Provides diagnostic signal for KVM/pvclock overflow artifacts - Does not modify the raw offset metric 3. Relax NodeClockSkewDetected alert threshold to ±60s - Previous ±1.0s was too tight and risked false negatives - 60s provides margin above the 4.29s overflow artifact - Still catches genuinely broken clocks (minutes/hours off) Fixes prometheus#3764 Signed-off-by: dongjiang <dongjiang1989@126.com>
dongjiang1989
force-pushed
the
fix-time-offset
branch
from
August 21, 2026 07:27
4a2e480 to
9341e72
Compare
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
3 participants
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.
Fixes #3764
On KVM/pvclock guests,
adjtimex()can transiently return2^32 ns(4.294967296s) due to a kernel race condition, causing false-positiveNodeClockSkewDetectedalerts.Changes:
Add diagnostic counter
node_timex_offset_overflow_total|offset| > 4sRelax alert threshold to
±60s(was unbounded above)No filtering at collection time — raw kernel values are preserved.