Skip to content

Commit 4866661

Browse files
committed
fix(alerts): Reject EAP alerts with invalid time windows
1 parent 86eb858 commit 4866661

File tree

4 files changed

+38
-27
lines changed

4 files changed

+38
-27
lines changed

src/sentry/incidents/serializers/alert_rule.py

Lines changed: 0 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -211,13 +211,6 @@ def validate(self, data):
211211
)
212212
self._validate_critical_warning_triggers(threshold_type, critical, warning)
213213

214-
if data.get("dataset") == Dataset.EventsAnalyticsPlatform:
215-
time_window = data["time_window"]
216-
if time_window < 5:
217-
raise serializers.ValidationError(
218-
"Invalid Time Window: Time window for this alert type must be at least 5 minutes."
219-
)
220-
221214
return data
222215

223216
def _translate_thresholds(self, threshold_type, comparison_delta, triggers, data):

src/sentry/snuba/snuba_query_validator.py

Lines changed: 13 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,7 @@
2424
)
2525
from sentry.incidents.utils.constants import INCIDENTS_SNUBA_SUBSCRIPTION_TYPE
2626
from sentry.models.project import Project
27+
from sentry.search.eap.constants import VALID_GRANULARITIES
2728
from sentry.search.eap.trace_metrics.validator import validate_trace_metrics_aggregate
2829
from sentry.snuba.dataset import Dataset
2930
from sentry.snuba.entity_subscription import (
@@ -51,6 +52,13 @@
5152
# Allowed time windows (in seconds) for crash rate alerts
5253
CRASH_RATE_ALERTS_ALLOWED_TIME_WINDOWS = [1800, 3600, 7200, 14400, 43200, 86400]
5354

55+
MIN_EAP_ALERT_TIME_WINDOW_SECONDS = 300
56+
57+
# Valid time windows for EAP alerts: valid Snuba granularities at or above the alert minimum.
58+
EAP_ALERTS_ALLOWED_TIME_WINDOWS = sorted(
59+
g for g in VALID_GRANULARITIES if g >= MIN_EAP_ALERT_TIME_WINDOW_SECONDS
60+
)
61+
5462

5563
QUERY_TYPE_VALID_DATASETS = {
5664
SnubaQuery.Type.ERROR: {Dataset.Events},
@@ -330,6 +338,8 @@ def _validate_query(self, data: dict[str, Any]) -> None:
330338
# TODO(edward): Bypass snql query validation for EAP queries. Do we need validation for rpc requests?
331339
if dataset != Dataset.EventsAnalyticsPlatform:
332340
self._validate_snql_query(data, entity_subscription, projects)
341+
else:
342+
self._validate_time_window(data["time_window"], dataset)
333343

334344
def _validate_snql_query(
335345
self,
@@ -391,9 +401,10 @@ def _validate_time_window(self, value: int, dataset: Dataset) -> int:
391401
"30min, 1h, 2h, 4h, 12h and 24h"
392402
)
393403
if dataset == Dataset.EventsAnalyticsPlatform:
394-
if time_window_seconds < 300:
404+
if time_window_seconds not in EAP_ALERTS_ALLOWED_TIME_WINDOWS:
395405
raise serializers.ValidationError(
396-
"Invalid Time Window: Time window for this alert type must be at least 5 minutes."
406+
f"Invalid Time Window: Allowed time windows (in seconds) for this alert type are: "
407+
f"{EAP_ALERTS_ALLOWED_TIME_WINDOWS}"
397408
)
398409
return time_window_seconds
399410

tests/sentry/incidents/endpoints/test_organization_alert_rule_index.py

Lines changed: 9 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -2067,13 +2067,18 @@ def test_eap_alert_with_invalid_time_window(self) -> None:
20672067
data = deepcopy(self.alert_rule_dict)
20682068
data["dataset"] = "events_analytics_platform"
20692069
data["alertType"] = "eap_metrics"
2070+
2071+
# Below the 5-minute floor
20702072
data["timeWindow"] = 1
20712073
with self.feature(["organizations:incidents", "organizations:performance-view"]):
20722074
resp = self.get_error_response(self.organization.slug, status_code=400, **data)
2073-
assert (
2074-
resp.data["nonFieldErrors"][0]
2075-
== "Invalid Time Window: Time window for this alert type must be at least 5 minutes."
2076-
)
2075+
assert "Invalid Time Window" in resp.data["nonFieldErrors"][0]
2076+
2077+
# Above the floor but not a valid granularity (7 minutes = 420 seconds)
2078+
data["timeWindow"] = 7
2079+
with self.feature(["organizations:incidents", "organizations:performance-view"]):
2080+
resp = self.get_error_response(self.organization.slug, status_code=400, **data)
2081+
assert "Invalid Time Window" in resp.data["nonFieldErrors"][0]
20772082

20782083
def test_transactions_dataset_deprecation_validation(self) -> None:
20792084
data = deepcopy(self.alert_rule_dict)

tests/sentry/incidents/endpoints/test_serializers.py

Lines changed: 16 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -171,21 +171,23 @@ def test_time_window(self) -> None:
171171
)
172172

173173
def test_span_alert_time_window_validation(self) -> None:
174-
params = self.valid_params.copy()
175-
params["dataset"] = Dataset.EventsAnalyticsPlatform.value
176-
params["event_types"] = [SnubaQueryEventType.EventType.TRACE_ITEM_SPAN.name.lower()]
177-
params["time_window"] = 1
178-
params["query"] = "span.op:http.client"
179-
params["aggregate"] = "count()"
174+
base_params = self.valid_params.copy()
175+
base_params["dataset"] = Dataset.EventsAnalyticsPlatform.value
176+
base_params["event_types"] = [SnubaQueryEventType.EventType.TRACE_ITEM_SPAN.name.lower()]
177+
base_params["query"] = "span.op:http.client"
178+
base_params["aggregate"] = "count()"
180179

181-
self.run_fail_validation_test(
182-
params,
183-
{
184-
"nonFieldErrors": [
185-
"Invalid Time Window: Time window for this alert type must be at least 5 minutes."
186-
]
187-
},
188-
)
180+
# Below the 5-minute floor
181+
base_params["time_window"] = 1
182+
serializer = AlertRuleSerializer(context=self.context, data=base_params)
183+
assert not serializer.is_valid()
184+
assert "Invalid Time Window" in serializer.errors["nonFieldErrors"][0]
185+
186+
# Above the floor but not a valid granularity (7 minutes = 420 seconds)
187+
base_params["time_window"] = 7
188+
serializer = AlertRuleSerializer(context=self.context, data=base_params)
189+
assert not serializer.is_valid()
190+
assert "Invalid Time Window" in serializer.errors["nonFieldErrors"][0]
189191

190192
def test_dataset(self) -> None:
191193
invalid_values = ["Invalid dataset, valid values are %s" % [item.value for item in Dataset]]

0 commit comments

Comments
 (0)