Skip to content

Commit 45d12a0

Browse files
committed
fix: never suppress alerts during a real outage-share spike
_should_suppress_alert only checked prev_overall != curr_overall before folding upstream-flap deltas into the noise-suppression path. If overall has already been parked on one bucket (e.g. DEGRADED) for an extended period and a new outage manifests as more 5xx-flavored probes without moving that bucket, the alert -- including to the LOCOSP #health mirror -- could get suppressed as "just more flap." Wire the existing outage-share signal (same fraction --watch already uses for outage-aware backoff) into the suppression decision: if outage_share >= outage_threshold, never suppress, regardless of classification. New params default to a no-op (0.0 >= 1.0 is always false) so existing 3-arg callers and the pre-existing test suite are unaffected.
1 parent ffa8bac commit 45d12a0

2 files changed

Lines changed: 64 additions & 2 deletions

File tree

test_suppression.py

Lines changed: 51 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -195,5 +195,56 @@ def test_unclassified_present_does_not_suppress(self):
195195
self.assertIn("unclassified", reason)
196196

197197

198+
class TestOutageShareOverridesSuppression(unittest.TestCase):
199+
"""A real outage must never be classified away as upstream flap just
200+
because `overall` was already sitting on the same bucket (e.g. DEGRADED
201+
for weeks) when the outage hit. See BrainVault Meta note on wdgo-health
202+
silence: the LOCOSP mirror only hears about state that reaches this
203+
function as non-suppressed, so this is the "is the API down" gate."""
204+
205+
def _summary(self, **kw) -> dict:
206+
base = {"improved": 0, "regressed": 0, "sideways": 0,
207+
"upstream_flap_count": 0, "total_classified": 0,
208+
"unclassified": 0}
209+
base.update(kw)
210+
return base
211+
212+
def test_outage_share_at_threshold_overrides_a_would_be_suppression(self):
213+
# Same shape as test_all_flaps_no_net_regression_suppresses, which
214+
# normally suppresses -- but a 50% outage share must win.
215+
s = self._summary(upstream_flap_count=4, total_classified=4,
216+
improved=2, regressed=2)
217+
suppress, reason = _should_suppress_alert(
218+
"DEGRADED", "DEGRADED", s, outage_share=0.5, outage_threshold=0.30)
219+
self.assertFalse(suppress)
220+
self.assertIn("outage-share", reason)
221+
222+
def test_outage_share_below_threshold_preserves_existing_behavior(self):
223+
s = self._summary(upstream_flap_count=4, total_classified=4,
224+
improved=2, regressed=2)
225+
suppress, reason = _should_suppress_alert(
226+
"DEGRADED", "DEGRADED", s, outage_share=0.1, outage_threshold=0.30)
227+
self.assertTrue(suppress)
228+
self.assertIn("upstream flap", reason)
229+
230+
def test_default_params_preserve_old_call_signature(self):
231+
# Existing callers (and the 90/90 suite predating this change) call
232+
# with 3 positional args -- defaults must be a no-op.
233+
s = self._summary(upstream_flap_count=3, total_classified=3,
234+
improved=3)
235+
suppress, _ = _should_suppress_alert("DEGRADED", "DEGRADED", s)
236+
self.assertTrue(suppress)
237+
238+
def test_outage_share_wins_even_on_overall_state_change(self):
239+
# Belt-and-suspenders: outage-share check runs first, so its reason
240+
# takes priority even when the overall-changed check would also fire.
241+
s = self._summary(upstream_flap_count=3, total_classified=3,
242+
improved=3)
243+
suppress, reason = _should_suppress_alert(
244+
"HEALTHY", "OUTAGE", s, outage_share=0.8, outage_threshold=0.30)
245+
self.assertFalse(suppress)
246+
self.assertIn("outage-share", reason)
247+
248+
198249
if __name__ == "__main__":
199250
unittest.main()

wdgwars_api_tester.py

Lines changed: 13 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -909,15 +909,25 @@ def _annotate_deltas(deltas: list[str]) -> tuple[list[str], dict]:
909909

910910

911911
def _should_suppress_alert(prev_overall: str, curr_overall: str,
912-
delta_summary: dict) -> tuple[bool, str]:
912+
delta_summary: dict,
913+
outage_share: float = 0.0,
914+
outage_threshold: float = 1.0) -> tuple[bool, str]:
913915
"""Decide whether to suppress the webhook + exec-on-change for this tick.
914916
915917
Suppress when:
916918
- overall state didn't change AND
917919
- all classified deltas are upstream flaps AND
918920
- net direction isn't getting worse (regressed <= improved)
921+
Never suppress when this sweep's outage_share (fraction of 429/transport-
922+
error verdicts, the same signal --watch uses for outage-aware backoff)
923+
meets outage_threshold — a real outage must never be classified away as
924+
upstream flap just because `overall` hadn't moved yet.
919925
Returns (suppress: bool, reason: str).
920926
"""
927+
if outage_share >= outage_threshold:
928+
return False, (f"outage-share {outage_share:.0%} >= "
929+
f"{outage_threshold:.0%} threshold (real outage, "
930+
"never suppress)")
921931
if prev_overall != curr_overall:
922932
return False, "overall state changed"
923933
if delta_summary["unclassified"] > 0:
@@ -2094,7 +2104,8 @@ def emit(results: list[Result], s: dict) -> None:
20942104
"sent" if ok else "FAILED")
20952105
_, _dsum = _annotate_deltas(deltas)
20962106
suppress, reason = _should_suppress_alert(
2097-
last_overall, overall, _dsum)
2107+
last_overall, overall, _dsum,
2108+
_outage_share(results), args.outage_backoff_threshold)
20982109
if suppress:
20992110
log.info(" alert SUPPRESSED: %s", reason)
21002111
if args.silent_webhook:

0 commit comments

Comments
 (0)