Skip to content

Commit a39a24d

Browse files
Spirax-M1claude
andcommitted
fix(site24x7): resolve status always firing and tags never forwarded (#6195, #6196)
- Add STATUS_MAP to correctly map "UP" → RESOLVED and DOWN/TROUBLE/CRITICAL → FIRING; previously _format_alert() never passed `status` to AlertDto so it always defaulted to FIRING via the root_validator fallback. - Parse TAGS field from webhook payload into AlertDto.labels; supports comma-separated "key:value" pairs, plain tag names, and dict payloads. - Add `source=["site24x7"]` which was missing from _format_alert(). - Add 16 unit tests covering all STATUS values, tag parsing edge cases, and core fields. Closes #6195 Closes #6196 Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
1 parent 7239e31 commit a39a24d

2 files changed

Lines changed: 169 additions & 2 deletions

File tree

keep/providers/site24x7_provider/site24x7_provider.py

Lines changed: 32 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,7 @@
99
import pydantic
1010
import requests
1111

12-
from keep.api.models.alert import AlertDto, AlertSeverity
12+
from keep.api.models.alert import AlertDto, AlertSeverity, AlertStatus
1313
from keep.contextmanager.contextmanager import ContextManager
1414
from keep.providers.base.base_provider import BaseProvider
1515
from keep.providers.models.provider_config import ProviderConfig, ProviderScope
@@ -86,6 +86,12 @@ class Site24X7Provider(BaseProvider):
8686
"UP": AlertSeverity.INFO,
8787
"CRITICAL": AlertSeverity.CRITICAL,
8888
}
89+
STATUS_MAP = {
90+
"UP": AlertStatus.RESOLVED,
91+
"DOWN": AlertStatus.FIRING,
92+
"TROUBLE": AlertStatus.FIRING,
93+
"CRITICAL": AlertStatus.FIRING,
94+
}
8995

9096
def __init__(
9197
self, context_manager: ContextManager, provider_id: str, config: ProviderConfig
@@ -217,13 +223,37 @@ def setup_webhook(
217223
def _format_alert(
218224
event: dict, provider_instance: "BaseProvider" = None
219225
) -> AlertDto:
226+
site24x7_status = event.get("STATUS", "DOWN")
227+
228+
# Parse TAGS field: Site24x7 sends comma-separated "key:value" pairs
229+
# e.g. "env:prod,team:backend" or plain "tagname"
230+
tags_raw = event.get("TAGS", "")
231+
labels = {}
232+
if isinstance(tags_raw, str) and tags_raw:
233+
for tag in tags_raw.split(","):
234+
tag = tag.strip()
235+
if ":" in tag:
236+
key, value = tag.split(":", 1)
237+
labels[key.strip()] = value.strip()
238+
elif tag:
239+
labels[tag] = tag
240+
elif isinstance(tags_raw, dict):
241+
labels = tags_raw
242+
220243
return AlertDto(
221244
url=event.get("MONITORURL", ""),
222245
lastReceived=event.get("INCIDENT_TIME_ISO", ""),
223246
description=event.get("INCIDENT_REASON", ""),
224247
name=event.get("MONITORNAME", ""),
225248
id=event.get("MONITOR_ID", ""),
226-
severity=Site24X7Provider.SEVERITIES_MAP.get(event.get("STATUS", "DOWN")),
249+
severity=Site24X7Provider.SEVERITIES_MAP.get(
250+
site24x7_status, AlertSeverity.WARNING
251+
),
252+
status=Site24X7Provider.STATUS_MAP.get(
253+
site24x7_status, AlertStatus.FIRING
254+
),
255+
labels=labels,
256+
source=["site24x7"],
227257
)
228258

229259
def _get_alerts(self) -> list[AlertDto]:
Lines changed: 137 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,137 @@
1+
"""
2+
Tests for Site24x7 provider bug fixes.
3+
4+
Bug 1 (issue #6195): STATUS "UP" was never mapped to AlertStatus.RESOLVED —
5+
_format_alert() passed no `status` argument to AlertDto, so it always
6+
defaulted to AlertStatus.FIRING via the root_validator fallback.
7+
8+
Bug 2 (issue #6196): TAGS field from the webhook payload was never read —
9+
_format_alert() ignored TAGS entirely, so AlertDto.labels was always {}.
10+
"""
11+
12+
import pytest
13+
14+
from keep.providers.site24x7_provider.site24x7_provider import Site24X7Provider
15+
16+
17+
def _minimal_event(**kwargs) -> dict:
18+
"""Return a minimal valid Site24x7 webhook payload, with optional overrides."""
19+
base = {
20+
"MONITORURL": "https://example.com",
21+
"INCIDENT_TIME_ISO": "2026-04-04T10:00:00Z",
22+
"INCIDENT_REASON": "Connection refused",
23+
"MONITORNAME": "Website - example.com",
24+
"MONITOR_ID": "12345",
25+
"STATUS": "DOWN",
26+
"TAGS": "",
27+
}
28+
base.update(kwargs)
29+
return base
30+
31+
32+
class TestStatusMapping:
33+
"""Bug #6195: STATUS field must drive AlertDto.status, not just severity."""
34+
35+
def test_status_up_resolves_alert(self):
36+
"""STATUS 'UP' (monitor recovered) must produce AlertStatus.RESOLVED."""
37+
alert = Site24X7Provider._format_alert(_minimal_event(STATUS="UP"))
38+
assert alert.status == "resolved"
39+
40+
def test_status_down_fires_alert(self):
41+
"""STATUS 'DOWN' must produce AlertStatus.FIRING."""
42+
alert = Site24X7Provider._format_alert(_minimal_event(STATUS="DOWN"))
43+
assert alert.status == "firing"
44+
45+
def test_status_trouble_fires_alert(self):
46+
"""STATUS 'TROUBLE' must produce AlertStatus.FIRING."""
47+
alert = Site24X7Provider._format_alert(_minimal_event(STATUS="TROUBLE"))
48+
assert alert.status == "firing"
49+
50+
def test_status_critical_fires_alert(self):
51+
"""STATUS 'CRITICAL' must produce AlertStatus.FIRING."""
52+
alert = Site24X7Provider._format_alert(_minimal_event(STATUS="CRITICAL"))
53+
assert alert.status == "firing"
54+
55+
def test_status_unknown_defaults_to_firing(self):
56+
"""Unknown STATUS values must fall back to AlertStatus.FIRING."""
57+
alert = Site24X7Provider._format_alert(_minimal_event(STATUS="SOMETHING_NEW"))
58+
assert alert.status == "firing"
59+
60+
def test_status_up_preserves_info_severity(self):
61+
"""STATUS 'UP' must still map severity to INFO (existing SEVERITIES_MAP)."""
62+
alert = Site24X7Provider._format_alert(_minimal_event(STATUS="UP"))
63+
assert alert.severity == "info"
64+
65+
66+
class TestTagsParsing:
67+
"""Bug #6196: TAGS field must be parsed and stored in AlertDto.labels."""
68+
69+
def test_tags_key_value_pairs(self):
70+
"""'env:prod,team:backend' must produce {'env': 'prod', 'team': 'backend'}."""
71+
alert = Site24X7Provider._format_alert(
72+
_minimal_event(TAGS="env:prod,team:backend")
73+
)
74+
assert alert.labels == {"env": "prod", "team": "backend"}
75+
76+
def test_tags_empty_string(self):
77+
"""Empty TAGS string must produce an empty labels dict."""
78+
alert = Site24X7Provider._format_alert(_minimal_event(TAGS=""))
79+
assert alert.labels == {}
80+
81+
def test_tags_field_absent(self):
82+
"""Missing TAGS key in payload must produce an empty labels dict."""
83+
event = _minimal_event()
84+
event.pop("TAGS")
85+
alert = Site24X7Provider._format_alert(event)
86+
assert alert.labels == {}
87+
88+
def test_tags_plain_name_without_value(self):
89+
"""Tag without ':' separator must map to {'tagname': 'tagname'}."""
90+
alert = Site24X7Provider._format_alert(_minimal_event(TAGS="tagname"))
91+
assert alert.labels == {"tagname": "tagname"}
92+
93+
def test_tags_mixed_plain_and_kv(self):
94+
"""Mix of plain tags and key:value tags must all be captured."""
95+
alert = Site24X7Provider._format_alert(
96+
_minimal_event(TAGS="env:prod,critical")
97+
)
98+
assert alert.labels == {"env": "prod", "critical": "critical"}
99+
100+
def test_tags_value_containing_colon(self):
101+
"""Tag value may itself contain ':' — only split on the first one."""
102+
alert = Site24X7Provider._format_alert(
103+
_minimal_event(TAGS="url:https://example.com")
104+
)
105+
assert alert.labels == {"url": "https://example.com"}
106+
107+
def test_tags_whitespace_trimmed(self):
108+
"""Spaces around tag names and values must be stripped."""
109+
alert = Site24X7Provider._format_alert(
110+
_minimal_event(TAGS=" env : prod , team : backend ")
111+
)
112+
assert alert.labels == {"env": "prod", "team": "backend"}
113+
114+
def test_tags_as_dict_passed_through(self):
115+
"""If TAGS is already a dict, it must be used as-is."""
116+
alert = Site24X7Provider._format_alert(
117+
_minimal_event(TAGS={"env": "prod", "team": "backend"})
118+
)
119+
assert alert.labels == {"env": "prod", "team": "backend"}
120+
121+
122+
class TestFormatAlertFields:
123+
"""Verify that the other AlertDto fields are still mapped correctly."""
124+
125+
def test_source_is_site24x7(self):
126+
"""source must always be ['site24x7']."""
127+
alert = Site24X7Provider._format_alert(_minimal_event())
128+
assert alert.source == ["site24x7"]
129+
130+
def test_basic_fields_mapped(self):
131+
"""Core fields from the webhook payload must be present."""
132+
alert = Site24X7Provider._format_alert(_minimal_event())
133+
assert "example.com" in str(alert.url)
134+
assert alert.name == "Website - example.com"
135+
assert alert.id == "12345"
136+
assert alert.description == "Connection refused"
137+
assert alert.lastReceived.startswith("2026-04-04T10:00:00")

0 commit comments

Comments
 (0)