fix: "All Values" over time could fire on a single probe (#2321)#2582
Open
FreZZZeR wants to merge 1 commit into
Open
fix: "All Values" over time could fire on a single probe (#2321)#2582FreZZZeR wants to merge 1 commit into
FreZZZeR wants to merge 1 commit into
Conversation
) Evaluate over time with "All Values" was matching even with one sample in the window. every() on a one-element array is always true, and when the window had no data we fell back to the current probe value, which basically ignores the "over time" part. Result: one failing probe could open an incident instead of waiting the configured minutes. Changes: - add onNoDataPolicy to the over-time options (reuses existing NoDataPolicy, default Ignore) so "no data" is not treated as the current value anymore. - resolveFilterOverTime now requires the window to be actually covered before "All Values" can match. Expected sample count is taken from the monitor interval (cron); if the interval is unknown it falls back to checking the oldest sample reaches near the window start. - pass monitoringInterval from MonitorCriteriaEvaluator to the probe criteria (api, server, dns, ssl, incoming request, external status page, snmp). - tests for the coverage check and the cron interval helper. - short note in monitor docs.
Contributor
✅ Snyk checks have passed. No issues have been found so far.
💻 Catch issues earlier using the plugins for VS Code, JetBrains IDEs, Visual Studio, and Eclipse. |
Author
|
Hello guys! |
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.
Fixes #2321.
Problem
Evaluate over time with "All Values" was matching even when there was only one
sample in the window.
[x].every(...)on a one-element array is always true,and when the window had no data the code fell back to the current probe value —
which ignores the "over time" part completely. So a single failing probe could
open an incident (or flip monitor status) instead of waiting the configured
minutes. Reported in #2321 on 9.5.0 and 10.0.17; still reproducible on 11.3.16.
Root cause
EvaluateOverTime.getValueOverTimereturns the raw sample array and thecomparison uses
Array.every()— trivially true for a single sample.checkProbeAgreement, whichre-evaluates and can see a different window than the summary shown to the
user, so the visible breakdown ("not met") and the action ("met") can disagree.
Fix
onNoDataPolicyto the evaluate-over-time options, reusing the existingNoDataPolicy(defaultIgnore) that metric monitors already use — "no data"no longer means "use the current value".
EvaluateOverTime.resolveFilterOverTime()requires the window to beactually covered before "All Values" can match. Expected sample count is
derived from the monitor's
monitoringInterval(cron); if the interval isunknown/irregular it falls back to checking the oldest sample reaches near the
window start. "Any Value" still matches on a single sample by design.
monitoringIntervalis threaded fromMonitorCriteriaEvaluatorto all probecriteria (API, Server, DNS, SSL, Incoming Request, External Status Page, SNMP).
getValueOverTimeis kept intact for backward compatibility.Tests
EvaluateOverTime.hasSufficientWindowCoverage— single sample vs full window,jitter tolerance, coarse interval, and the no-interval fallback.
CronTab.getIntervalInMinutes.Verified locally: compile (tsc), jest, eslint and prettier all pass.
Notes
cron-parserwas already in use).focuses on the probe monitors from Terraform Monitor Criteria
evaluateOverTimenot waiting #2321.