Skip to content

Commit 3186f03

Browse files
authored
fix(aci milestone 3): bug fix metric detector serializers (#95920)
Due to incorrect querying, we were fetching all action filter data conditions in the database when attempting to serialize a detector :yikes: Ultimately, the serialized response would be correct, because the data condition serializer had the correct filtering. However, on production data with many data conditions in the DB, the query would time out.
1 parent b073c5a commit 3186f03

File tree

3 files changed

+57
-4
lines changed

3 files changed

+57
-4
lines changed

src/sentry/incidents/endpoints/serializers/workflow_engine_detector.py

Lines changed: 13 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@
33
from typing import Any, DefaultDict
44

55
from django.contrib.auth.models import AnonymousUser
6-
from django.db.models import Q
6+
from django.db.models import Q, Subquery
77

88
from sentry.api.serializers import Serializer, serialize
99
from sentry.incidents.endpoints.serializers.alert_rule import AlertRuleSerializerResponse
@@ -26,9 +26,11 @@
2626
Action,
2727
AlertRuleDetector,
2828
DataCondition,
29+
DataConditionGroup,
2930
DataConditionGroupAction,
3031
DataSourceDetector,
3132
Detector,
33+
DetectorWorkflow,
3234
)
3335
from sentry.workflow_engine.models.workflow_action_group_status import WorkflowActionGroupStatus
3436
from sentry.workflow_engine.types import DetectorPriorityLevel
@@ -209,6 +211,7 @@ def get_attrs(
209211
self, item_list: Sequence[Detector], user: User | RpcUser | AnonymousUser, **kwargs: Any
210212
) -> defaultdict[Detector, dict[str, Any]]:
211213
detectors = {item.id: item for item in item_list}
214+
detector_ids = [item.id for item in item_list]
212215
result: DefaultDict[Detector, dict[str, Any]] = defaultdict(dict)
213216

214217
detector_workflow_condition_group_ids = [
@@ -220,13 +223,20 @@ def get_attrs(
220223
condition_group__in=detector_workflow_condition_group_ids,
221224
condition_result__in=[DetectorPriorityLevel.HIGH, DetectorPriorityLevel.MEDIUM],
222225
)
223-
226+
workflow_dcg_ids = DataConditionGroup.objects.filter(
227+
workflowdataconditiongroup__workflow__in=Subquery(
228+
DetectorWorkflow.objects.filter(detector__in=detector_ids).values_list(
229+
"workflow_id", flat=True
230+
)
231+
)
232+
).values_list("id", flat=True)
224233
action_filter_data_condition_groups = DataCondition.objects.filter(
225234
comparison__in=[
226235
detector_trigger.condition_result
227236
for detector_trigger in detector_trigger_data_conditions
228237
],
229-
).exclude(condition_group__in=detector_workflow_condition_group_ids)
238+
condition_group__in=Subquery(workflow_dcg_ids),
239+
)
230240

231241
dcgas = DataConditionGroupAction.objects.filter(
232242
condition_group__in=[

tests/sentry/incidents/serializers/test_workflow_engine_base.py

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -40,6 +40,7 @@ def setUp(self) -> None:
4040
self.resolve_trigger_data_condition = migrate_resolve_threshold_data_condition(
4141
self.alert_rule
4242
)
43+
4344
self.expected_critical_action = [
4445
{
4546
"id": str(self.critical_trigger_action.id),

tests/sentry/incidents/serializers/test_workflow_engine_detector.py

Lines changed: 43 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,12 @@
88
)
99
from sentry.incidents.models.alert_rule import AlertRuleTriggerAction
1010
from sentry.incidents.models.incident import IncidentTrigger, TriggerStatus
11-
from sentry.workflow_engine.migration_helpers.alert_rule import migrate_metric_action
11+
from sentry.workflow_engine.migration_helpers.alert_rule import (
12+
migrate_alert_rule,
13+
migrate_metric_action,
14+
migrate_metric_data_conditions,
15+
migrate_resolve_threshold_data_condition,
16+
)
1217
from tests.sentry.incidents.serializers.test_workflow_engine_base import (
1318
TestWorkflowEngineSerializer,
1419
)
@@ -68,6 +73,43 @@ def test_sentry_app(self, mock_sentry_app_components_preparer: Any) -> None:
6873
)
6974
self.sentry_app_action, _, _ = migrate_metric_action(self.sentry_app_trigger_action)
7075

76+
other_sentry_app = self.create_sentry_app(
77+
organization=self.organization,
78+
published=True,
79+
verify_install=False,
80+
name="Not Awesome App",
81+
schema={"elements": [self.create_alert_rule_action_schema()]},
82+
)
83+
self.create_sentry_app_installation(
84+
slug=other_sentry_app.slug, organization=self.organization, user=self.user
85+
)
86+
87+
# add some other workflow engine objects to ensure that our filtering is working properly
88+
other_alert_rule = self.create_alert_rule()
89+
critical_trigger = self.create_alert_rule_trigger(
90+
alert_rule=other_alert_rule, label="critical"
91+
)
92+
critical_trigger_action = self.create_alert_rule_trigger_action(
93+
alert_rule_trigger=critical_trigger
94+
)
95+
migrate_alert_rule(other_alert_rule)
96+
migrate_metric_data_conditions(critical_trigger)
97+
98+
migrate_metric_action(critical_trigger_action)
99+
migrate_resolve_threshold_data_condition(other_alert_rule)
100+
101+
other_sentry_app_action = self.create_alert_rule_trigger_action(
102+
alert_rule_trigger=critical_trigger,
103+
type=AlertRuleTriggerAction.Type.SENTRY_APP,
104+
target_identifier=other_sentry_app.id,
105+
target_type=AlertRuleTriggerAction.TargetType.SENTRY_APP,
106+
sentry_app=other_sentry_app,
107+
sentry_app_config=[
108+
{"name": "title", "value": "An alert"},
109+
],
110+
)
111+
migrate_metric_action(other_sentry_app_action)
112+
71113
# add a sentry app action and update expected actions
72114
sentry_app_action_data = {
73115
"id": str(self.sentry_app_trigger_action.id),

0 commit comments

Comments
 (0)