Skip to content

Commit 9872f45

Browse files
nikkikapadiaandrewshie-sentry
authored andcommitted
chore(spans-migration): add post and put validation for EAP extrapolation mode (#102970)
We've added a new extrapolation mode field in metrics alerts for EAP. We want to make sure the `server_weighted` value is not put in by the user. They also shouldn't be able to set this extrapolation mode if there already is another mode when updating. I've added tests to confirm but let me know if there are better ways to set and validate the extrapolation mode in the new alerts code.
1 parent 84183f7 commit 9872f45

File tree

8 files changed

+181
-2
lines changed

8 files changed

+181
-2
lines changed

src/sentry/incidents/endpoints/organization_alert_rule_details.py

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -38,6 +38,7 @@
3838
delete_alert_rule,
3939
get_slack_actions_with_async_lookups,
4040
)
41+
from sentry.incidents.metric_issue_detector import is_invalid_extrapolation_mode
4142
from sentry.incidents.models.alert_rule import AlertRule
4243
from sentry.incidents.serializers import AlertRuleSerializer as DrfAlertRuleSerializer
4344
from sentry.incidents.utils.sentry_apps import trigger_sentry_app_action_creators_for_incidents
@@ -49,6 +50,8 @@
4950
from sentry.models.rulesnooze import RuleSnooze
5051
from sentry.sentry_apps.services.app import app_service
5152
from sentry.sentry_apps.utils.errors import SentryAppBaseError
53+
from sentry.snuba.dataset import Dataset
54+
from sentry.snuba.models import ExtrapolationMode
5255
from sentry.users.services.user.service import user_service
5356
from sentry.workflow_engine.migration_helpers.alert_rule import dual_delete_migrated_alert_rule
5457
from sentry.workflow_engine.models import Detector
@@ -122,6 +125,17 @@ def update_alert_rule(
122125
partial=True,
123126
)
124127
if validator.is_valid():
128+
if data.get("dataset") == Dataset.EventsAnalyticsPlatform.value:
129+
if data.get("extrapolation_mode"):
130+
old_extrapolation_mode = ExtrapolationMode(
131+
alert_rule.snuba_query.extrapolation_mode
132+
).name.lower()
133+
new_extrapolation_mode = data.get("extrapolation_mode", old_extrapolation_mode)
134+
if is_invalid_extrapolation_mode(old_extrapolation_mode, new_extrapolation_mode):
135+
raise serializers.ValidationError(
136+
"Invalid extrapolation mode for this alert type."
137+
)
138+
125139
try:
126140
trigger_sentry_app_action_creators_for_incidents(validator.validated_data)
127141
except SentryAppBaseError as e:

src/sentry/incidents/endpoints/organization_alert_rule_index.py

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -73,6 +73,7 @@
7373
from sentry.sentry_apps.services.app import app_service
7474
from sentry.sentry_apps.utils.errors import SentryAppBaseError
7575
from sentry.snuba.dataset import Dataset
76+
from sentry.snuba.models import ExtrapolationMode
7677
from sentry.uptime.types import (
7778
DATA_SOURCE_UPTIME_SUBSCRIPTION,
7879
GROUP_TYPE_UPTIME_DOMAIN_CHECK_FAILURE,
@@ -101,6 +102,9 @@ def create_metric_alert(
101102
"Creation of transaction-based alerts is disabled, as we migrate to the span dataset. Create span-based alerts (dataset: events_analytics_platform) with the is_transaction:true filter instead."
102103
)
103104

105+
if data.get("extrapolation_mode") == ExtrapolationMode.SERVER_WEIGHTED.name.lower():
106+
raise ValidationError("server_weighted extrapolation mode is not supported for new alerts.")
107+
104108
if project:
105109
data["projects"] = [project.slug]
106110

src/sentry/incidents/metric_issue_detector.py

Lines changed: 39 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,12 @@
1111
from sentry.seer.anomaly_detection.store_data_workflow_engine import send_new_detector_data
1212
from sentry.snuba.dataset import Dataset
1313
from sentry.snuba.metrics.extraction import should_use_on_demand_metrics
14-
from sentry.snuba.models import QuerySubscription, SnubaQuery, SnubaQueryEventType
14+
from sentry.snuba.models import (
15+
ExtrapolationMode,
16+
QuerySubscription,
17+
SnubaQuery,
18+
SnubaQueryEventType,
19+
)
1520
from sentry.snuba.snuba_query_validator import SnubaQueryValidator
1621
from sentry.snuba.subscriptions import update_snuba_query
1722
from sentry.tasks.relay import schedule_invalidate_project_config
@@ -147,6 +152,19 @@ def validate_conditions(self, value):
147152
return value
148153

149154

155+
def is_invalid_extrapolation_mode(old_extrapolation_mode, new_extrapolation_mode) -> bool:
156+
if type(new_extrapolation_mode) is int:
157+
new_extrapolation_mode = ExtrapolationMode(new_extrapolation_mode).name.lower()
158+
if type(old_extrapolation_mode) is int:
159+
old_extrapolation_mode = ExtrapolationMode(old_extrapolation_mode).name.lower()
160+
if (
161+
new_extrapolation_mode == ExtrapolationMode.SERVER_WEIGHTED.name.lower()
162+
and old_extrapolation_mode != ExtrapolationMode.SERVER_WEIGHTED.name.lower()
163+
):
164+
return True
165+
return False
166+
167+
150168
class MetricIssueDetectorValidator(BaseDetectorTypeValidator):
151169
data_sources = serializers.ListField(
152170
child=SnubaQueryValidator(timeWindowSeconds=True), required=False
@@ -174,6 +192,12 @@ def _validate_transaction_dataset_deprecation(self, dataset: Dataset) -> None:
174192
"Creation of transaction-based alerts is disabled, as we migrate to the span dataset. Create span-based alerts (dataset: events_analytics_platform) with the is_transaction:true filter instead."
175193
)
176194

195+
def _validate_extrapolation_mode(self, extrapolation_mode: str) -> None:
196+
if extrapolation_mode == ExtrapolationMode.SERVER_WEIGHTED.value:
197+
raise serializers.ValidationError(
198+
"server_weighted extrapolation mode is not supported for new detectors."
199+
)
200+
177201
def get_quota(self) -> DetectorQuota:
178202
organization = self.context.get("organization")
179203
request = self.context.get("request")
@@ -236,6 +260,16 @@ def update_data_source(self, instance: Detector, data_source: SnubaQueryDataSour
236260
"Updates to transaction-based alerts is disabled, as we migrate to the span dataset. Create span-based alerts (dataset: events_analytics_platform) with the is_transaction:true filter instead."
237261
)
238262

263+
old_extrapolation_mode = snuba_query.extrapolation_mode
264+
new_extrapolation_mode = data_source.get(
265+
"extrapolation_mode", snuba_query.extrapolation_mode
266+
)
267+
if data_source.get("dataset") == Dataset.EventsAnalyticsPlatform:
268+
if is_invalid_extrapolation_mode(old_extrapolation_mode, new_extrapolation_mode):
269+
raise serializers.ValidationError(
270+
"Invalid extrapolation mode for this detector type."
271+
)
272+
239273
update_snuba_query(
240274
snuba_query=snuba_query,
241275
query_type=data_source.get("query_type", snuba_query.type),
@@ -246,6 +280,9 @@ def update_data_source(self, instance: Detector, data_source: SnubaQueryDataSour
246280
resolution=timedelta(seconds=data_source.get("resolution", snuba_query.resolution)),
247281
environment=data_source.get("environment", snuba_query.environment),
248282
event_types=data_source.get("event_types", [event_type for event_type in event_types]),
283+
extrapolation_mode=data_source.get(
284+
"extrapolation_mode", snuba_query.extrapolation_mode
285+
),
249286
)
250287

251288
def update(self, instance: Detector, validated_data: dict[str, Any]):
@@ -279,6 +316,7 @@ def create(self, validated_data: dict[str, Any]):
279316
if "data_sources" in validated_data:
280317
for validated_data_source in validated_data["data_sources"]:
281318
self._validate_transaction_dataset_deprecation(validated_data_source.get("dataset"))
319+
self._validate_extrapolation_mode(validated_data_source.get("extrapolation_mode"))
282320

283321
detector = super().create(validated_data)
284322

src/sentry/snuba/snuba_query_validator.py

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,7 @@
3131
)
3232
from sentry.snuba.metrics.naming_layer.mri import is_mri
3333
from sentry.snuba.models import (
34+
ExtrapolationMode,
3435
QuerySubscription,
3536
QuerySubscriptionDataSourceHandler,
3637
SnubaQuery,
@@ -86,6 +87,7 @@ class SnubaQueryValidator(BaseDataSourceValidator[QuerySubscription]):
8687
required=False,
8788
allow_empty=False,
8889
)
90+
extrapolation_mode = serializers.IntegerField(required=False, allow_null=True)
8991

9092
class Meta:
9193
model = QuerySubscription
@@ -98,6 +100,7 @@ class Meta:
98100
"environment",
99101
"event_types",
100102
"group_by",
103+
"extrapolation_mode",
101104
]
102105

103106
data_source_type_handler = QuerySubscriptionDataSourceHandler
@@ -402,6 +405,9 @@ def _validate_group_by(self, value: Sequence[str] | None) -> Sequence[str] | Non
402405

403406
@override
404407
def create_source(self, validated_data) -> QuerySubscription:
408+
extrapolation_mode = validated_data.get("extrapolation_mode")
409+
if extrapolation_mode is not None:
410+
extrapolation_mode = ExtrapolationMode(extrapolation_mode)
405411
snuba_query = create_snuba_query(
406412
query_type=validated_data["query_type"],
407413
dataset=validated_data["dataset"],
@@ -412,6 +418,7 @@ def create_source(self, validated_data) -> QuerySubscription:
412418
environment=validated_data["environment"],
413419
event_types=validated_data["event_types"],
414420
group_by=validated_data.get("group_by"),
421+
extrapolation_mode=extrapolation_mode,
415422
)
416423
return create_snuba_subscription(
417424
project=self.context["project"],

src/sentry/snuba/subscriptions.py

Lines changed: 18 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,12 @@
77
from sentry.models.environment import Environment
88
from sentry.models.project import Project
99
from sentry.snuba.dataset import Dataset
10-
from sentry.snuba.models import QuerySubscription, SnubaQuery, SnubaQueryEventType
10+
from sentry.snuba.models import (
11+
ExtrapolationMode,
12+
QuerySubscription,
13+
SnubaQuery,
14+
SnubaQueryEventType,
15+
)
1116
from sentry.snuba.tasks import (
1217
create_subscription_in_snuba,
1318
delete_subscription_from_snuba,
@@ -27,6 +32,7 @@ def create_snuba_query(
2732
environment: Environment | None,
2833
event_types: Collection[SnubaQueryEventType.EventType] = (),
2934
group_by: Sequence[str] | None = None,
35+
extrapolation_mode: ExtrapolationMode | None = None,
3036
):
3137
"""
3238
Constructs a SnubaQuery which is the postgres representation of a query in snuba
@@ -52,6 +58,11 @@ def create_snuba_query(
5258
resolution=int(resolution.total_seconds()),
5359
environment=environment,
5460
group_by=group_by,
61+
extrapolation_mode=(
62+
extrapolation_mode.value
63+
if extrapolation_mode is not None
64+
else ExtrapolationMode.UNKNOWN.value
65+
),
5566
)
5667
if not event_types:
5768
if dataset == Dataset.Events:
@@ -78,6 +89,7 @@ def update_snuba_query(
7889
resolution,
7990
environment,
8091
event_types,
92+
extrapolation_mode=None,
8193
):
8294
"""
8395
Updates a SnubaQuery. Triggers updates to any related QuerySubscriptions.
@@ -118,6 +130,11 @@ def update_snuba_query(
118130
time_window=int(time_window.total_seconds()),
119131
resolution=int(resolution.total_seconds()),
120132
environment=environment,
133+
extrapolation_mode=(
134+
extrapolation_mode
135+
if extrapolation_mode is not None
136+
else ExtrapolationMode.UNKNOWN.value
137+
),
121138
)
122139
if new_event_types:
123140
SnubaQueryEventType.objects.bulk_create(

tests/sentry/incidents/endpoints/test_organization_alert_rule_details.py

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1416,6 +1416,24 @@ def test_change_name_of_existing_alert(self) -> None:
14161416
)
14171417
assert len(audit_log_entry) == 1
14181418

1419+
def test_invalid_extrapolation_mode(self) -> None:
1420+
self.create_member(
1421+
user=self.user, organization=self.organization, role="owner", teams=[self.team]
1422+
)
1423+
self.login_as(self.user)
1424+
alert_rule = self.alert_rule
1425+
# We need the IDs to force update instead of create, so we just get the rule using our own API. Like frontend would.
1426+
alert_rule_dict = deepcopy(self.alert_rule_dict)
1427+
alert_rule_dict["dataset"] = "events_analytics_platform"
1428+
alert_rule_dict["alertType"] = "eap_metrics"
1429+
alert_rule_dict["extrapolation_mode"] = "server_weighted"
1430+
1431+
with self.feature("organizations:incidents"):
1432+
resp = self.get_error_response(
1433+
self.organization.slug, alert_rule.id, status_code=400, **alert_rule_dict
1434+
)
1435+
assert resp.data[0] == "Invalid extrapolation mode for this alert type."
1436+
14191437

14201438
class AlertRuleDetailsSlackPutEndpointTest(AlertRuleDetailsBase):
14211439
method = "put"

tests/sentry/incidents/endpoints/test_organization_alert_rule_index.py

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1705,6 +1705,15 @@ def test_generic_metrics_dataset_deprecation_validation(self) -> None:
17051705
== "Creation of transaction-based alerts is disabled, as we migrate to the span dataset. Create span-based alerts (dataset: events_analytics_platform) with the is_transaction:true filter instead."
17061706
)
17071707

1708+
def test_invalid_extrapolation_mode(self) -> None:
1709+
data = deepcopy(self.alert_rule_dict)
1710+
data["dataset"] = "events_analytics_platform"
1711+
data["alertType"] = "eap_metrics"
1712+
data["extrapolation_mode"] = "server_weighted"
1713+
with self.feature(["organizations:incidents", "organizations:performance-view"]):
1714+
resp = self.get_error_response(self.organization.slug, status_code=400, **data)
1715+
assert resp.data[0] == "server_weighted extrapolation mode is not supported for new alerts."
1716+
17081717

17091718
@freeze_time()
17101719
class AlertRuleCreateEndpointTestCrashRateAlert(AlertRuleIndexBase):

tests/sentry/incidents/endpoints/validators/test_validators.py

Lines changed: 72 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,7 @@
2626
)
2727
from sentry.snuba.dataset import Dataset
2828
from sentry.snuba.models import (
29+
ExtrapolationMode,
2930
QuerySubscription,
3031
QuerySubscriptionDataSourceHandler,
3132
SnubaQuery,
@@ -654,3 +655,74 @@ def test_transaction_dataset_deprecation_generic_metrics_update(self) -> None:
654655
with_feature("organizations:discover-saved-queries-deprecation"),
655656
):
656657
update_validator.save()
658+
659+
def test_invalid_extrapolation_mode_create(self) -> None:
660+
data = {
661+
**self.valid_data,
662+
"dataSources": [
663+
{
664+
"queryType": SnubaQuery.Type.PERFORMANCE.value,
665+
"dataset": Dataset.EventsAnalyticsPlatform.value,
666+
"query": "test query",
667+
"aggregate": "count()",
668+
"timeWindow": 3600,
669+
"environment": self.environment.name,
670+
"eventTypes": [SnubaQueryEventType.EventType.TRACE_ITEM_SPAN.name.lower()],
671+
"extrapolation_mode": ExtrapolationMode.SERVER_WEIGHTED.value,
672+
},
673+
],
674+
}
675+
676+
validator = MetricIssueDetectorValidator(data=data, context=self.context)
677+
assert validator.is_valid(), validator.errors
678+
with self.assertRaisesMessage(
679+
ValidationError,
680+
expected_message="server_weighted extrapolation mode is not supported for new detectors.",
681+
):
682+
validator.save()
683+
684+
def test_invalid_extrapolation_mode_update(self) -> None:
685+
data = {
686+
**self.valid_data,
687+
"dataSources": [
688+
{
689+
"queryType": SnubaQuery.Type.PERFORMANCE.value,
690+
"dataset": Dataset.EventsAnalyticsPlatform.value,
691+
"query": "test query",
692+
"aggregate": "count()",
693+
"timeWindow": 3600,
694+
"environment": self.environment.name,
695+
"eventTypes": [SnubaQueryEventType.EventType.TRACE_ITEM_SPAN.name.lower()],
696+
"extrapolation_mode": ExtrapolationMode.CLIENT_AND_SERVER_WEIGHTED.value,
697+
},
698+
],
699+
}
700+
701+
validator = MetricIssueDetectorValidator(data=data, context=self.context)
702+
assert validator.is_valid(), validator.errors
703+
704+
detector = validator.save()
705+
706+
update_data = {
707+
"dataSources": [
708+
{
709+
"queryType": SnubaQuery.Type.PERFORMANCE.value,
710+
"dataset": Dataset.EventsAnalyticsPlatform.value,
711+
"query": "updated query",
712+
"aggregate": "count()",
713+
"timeWindow": 3600,
714+
"environment": self.environment.name,
715+
"eventTypes": [SnubaQueryEventType.EventType.TRACE_ITEM_SPAN.name.lower()],
716+
"extrapolation_mode": ExtrapolationMode.SERVER_WEIGHTED.value,
717+
}
718+
],
719+
}
720+
update_validator = MetricIssueDetectorValidator(
721+
instance=detector, data=update_data, context=self.context, partial=True
722+
)
723+
assert update_validator.is_valid(), update_validator.errors
724+
with self.assertRaisesMessage(
725+
ValidationError,
726+
expected_message="Invalid extrapolation mode for this detector type.",
727+
):
728+
update_validator.save()

0 commit comments

Comments
 (0)