Omit probe_icmp_duration rtt metric when echo reply is missing - #1635
Open
locker95 wants to merge 1 commit into
Open
Omit probe_icmp_duration rtt metric when echo reply is missing#1635locker95 wants to merge 1 commit into
locker95 wants to merge 1 commit into
Conversation
phase="rtt" was pre-created at 0 for every probe. Failed pings therefore
exported rtt=0, which looks like an instant success and skews
avg_over_time / quantiles / min_over_time over mixed success and loss.
Only export phase="rtt" when a matching echo reply is received. resolve
and setup phases are unchanged. Successful probes still report a positive
rtt.
Behavior change: on probe_success=0, probe_icmp_duration_seconds{phase="rtt"}
is absent instead of 0. Aggregations that only care about successful RTTs
no longer need subqueries to filter zeros.
Fixes prometheus#984
Signed-off-by: Dean Chen <862469039@qq.com>
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
1 participant
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.
Description
probe_icmp_duration_seconds{phase="rtt"}was pre-created for every probe, so a missing echo reply exported 0. That looks like an instant success and pollutes range aggregations:avg_over_time(...[5m])min_over_time(...[5m])quantile_over_time(...)A failed RTT is not zero latency. Prefer not exporting the series when there is no reply (same idea as skipping
probe_icmp_reply_hop_limitwhen TTL is unavailable).Behavior change: on failed ICMP probes,
phase="rtt"is absent instead of0.resolve/setupare unchanged. Successful probes still export a positivertt.Dashboards that assumed
rttis always present should treat a missing series as “no sample” (Prometheus range aggregations already skip gaps).Fixes #984
How to verify
go test ./prober/ -run TestProbeICMPDurationRTT -count=1 -v