Skip to content

Commit 8dbc68f

Browse files
authored
fix(AD): Check for NaNs (#104909)
[This](https://app.datadoghq.com/monitors/234175842?group=sentry_region%3Ade%2Ctype%3Aanomaly_detection&from_ts=1765573302000&to_ts=1765574502000&event_id=8410610600228720311&link_source=monitor_notif) Datadog monitor has been flapping because Seer keeps getting "None" values, but thanks to the logging added in #104766 we can see that it's actually NaN. Now we will check for that and not send the data to Seer if it's either `None` or NaN.
1 parent a7223b8 commit 8dbc68f

File tree

2 files changed

+49
-8
lines changed

2 files changed

+49
-8
lines changed

src/sentry/seer/anomaly_detection/get_anomaly_data.py

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -69,7 +69,8 @@ def get_anomaly_data_from_seer(
6969
aggregation_value = subscription_update.get("value")
7070
source_id = subscription.id
7171
source_type = DataSourceType.SNUBA_QUERY_SUBSCRIPTION
72-
if aggregation_value is None:
72+
73+
if aggregation_value is None or str(aggregation_value) == "nan":
7374
logger.error(
7475
"Invalid aggregation value", extra={"source_id": source_id, "source_type": source_type}
7576
)

tests/sentry/incidents/handlers/condition/test_anomaly_detection_handler.py

Lines changed: 47 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,7 @@
1515
)
1616
from sentry.snuba.subscriptions import create_snuba_subscription
1717
from sentry.workflow_engine.models import Condition, DataPacket
18-
from sentry.workflow_engine.types import DetectorPriorityLevel
18+
from sentry.workflow_engine.types import ConditionError, DetectorPriorityLevel
1919
from tests.sentry.workflow_engine.handlers.condition.test_base import ConditionTestCase
2020

2121

@@ -30,6 +30,8 @@ def setUp(self) -> None:
3030
(self.workflow, self.detector, self.detector_workflow, self.workflow_triggers) = (
3131
self.create_detector_and_workflow()
3232
)
33+
self.detector.update(config={"detection_type": "dynamic", "comparison_delta": None})
34+
self.detector.save()
3335

3436
packet = AnomalyDetectionUpdate(
3537
subscription_id=str(self.subscription.id),
@@ -64,12 +66,7 @@ def setUp(self) -> None:
6466
condition_result=DetectorPriorityLevel.HIGH,
6567
condition_group=self.workflow_triggers,
6668
)
67-
68-
@mock.patch(
69-
"sentry.seer.anomaly_detection.get_anomaly_data.SEER_ANOMALY_DETECTION_CONNECTION_POOL.urlopen"
70-
)
71-
def test_passes(self, mock_seer_request: mock.MagicMock) -> None:
72-
seer_return_value: DetectAnomaliesResponse = {
69+
self.high_confidence_seer_response: DetectAnomaliesResponse = {
7370
"success": True,
7471
"timeseries": [
7572
{
@@ -82,6 +79,12 @@ def test_passes(self, mock_seer_request: mock.MagicMock) -> None:
8279
}
8380
],
8481
}
82+
83+
@mock.patch(
84+
"sentry.seer.anomaly_detection.get_anomaly_data.SEER_ANOMALY_DETECTION_CONNECTION_POOL.urlopen"
85+
)
86+
def test_passes(self, mock_seer_request: mock.MagicMock) -> None:
87+
seer_return_value = self.high_confidence_seer_response
8588
mock_seer_request.return_value = HTTPResponse(orjson.dumps(seer_return_value), status=200)
8689
assert (
8790
self.dc.evaluate_value(self.data_packet.packet.values)
@@ -110,6 +113,43 @@ def test_passes_medium(self, mock_seer_request: mock.MagicMock) -> None:
110113
self.dc.evaluate_value(self.data_packet.packet.values) == DetectorPriorityLevel.OK.value
111114
)
112115

116+
@mock.patch(
117+
"sentry.seer.anomaly_detection.get_anomaly_data.SEER_ANOMALY_DETECTION_CONNECTION_POOL.urlopen"
118+
)
119+
@mock.patch("sentry.seer.anomaly_detection.get_anomaly_data.logger")
120+
def test_seer_call_nan_aggregation_value(
121+
self, mock_logger: mock.MagicMock, mock_seer_request: mock.MagicMock
122+
) -> None:
123+
seer_return_value = self.high_confidence_seer_response
124+
mock_seer_request.return_value = HTTPResponse(orjson.dumps(seer_return_value), status=200)
125+
126+
packet = AnomalyDetectionUpdate(
127+
subscription_id=str(self.subscription.id),
128+
values={
129+
"value": float("nan"),
130+
"source_id": str(self.subscription.id),
131+
"subscription_id": str(self.subscription.id),
132+
"timestamp": datetime.now(UTC),
133+
},
134+
timestamp=datetime.now(UTC),
135+
entity="test-entity",
136+
)
137+
data_packet = DataPacket[AnomalyDetectionUpdate](
138+
source_id=str(packet.subscription_id),
139+
packet=packet,
140+
)
141+
142+
assert self.dc.evaluate_value(data_packet.packet.values) == ConditionError(
143+
msg="Error during Seer data evaluation process."
144+
)
145+
mock_logger.error.assert_called_with(
146+
"Invalid aggregation value",
147+
extra={
148+
"source_id": self.subscription.id,
149+
"source_type": DataSourceType.SNUBA_QUERY_SUBSCRIPTION,
150+
},
151+
)
152+
113153
@mock.patch(
114154
"sentry.seer.anomaly_detection.get_anomaly_data.SEER_ANOMALY_DETECTION_CONNECTION_POOL.urlopen"
115155
)

0 commit comments

Comments
 (0)