Skip to content

Commit 5e1a408

Browse files
saponifi3dgeorge-sentry
authored andcommitted
feat(workflow_engine): Only link workflows to the IssueStream (#112276)
## Description Update the on project creation to only link to the issue stream detector, rather than the issue stream and error detectors. Moves the code into workflow_engine to keep things organized.
1 parent 5981e58 commit 5e1a408

File tree

12 files changed

+401
-31
lines changed

12 files changed

+401
-31
lines changed

src/sentry/event_manager.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2006,7 +2006,7 @@ def _get_severity_metadata_for_group(
20062006
20072007
Returns {} if conditions aren't met or on exception.
20082008
"""
2009-
from sentry.receivers.rules import PLATFORMS_WITH_PRIORITY_ALERTS
2009+
from sentry.workflow_engine.receivers.project_workflows import PLATFORMS_WITH_PRIORITY_ALERTS
20102010

20112011
if killswitch_matches_context(
20122012
"issues.severity.skip-seer-requests", {"project_id": event.project_id}

src/sentry/receivers/__init__.py

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,6 @@
1111
from .owners import * # noqa: F401,F403
1212
from .releases import * # noqa: F401,F403
1313
from .rule_snooze import * # noqa: F401,F403
14-
from .rules import * # noqa: F401,F403
1514
from .sentry_apps import * # noqa: F401,F403
1615
from .stats import * # noqa: F401,F403
1716
from .superuser import * # noqa: F401,F403
Lines changed: 116 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,116 @@
1+
from typing import Sequence
2+
3+
from django.db import router, transaction
4+
5+
from sentry.models.organization import Organization
6+
from sentry.models.project import Project
7+
from sentry.notifications.models.notificationaction import ActionTarget
8+
from sentry.notifications.types import FallthroughChoiceType
9+
from sentry.workflow_engine.defaults.detectors import _ensure_detector
10+
from sentry.workflow_engine.models import (
11+
Action,
12+
DataCondition,
13+
DataConditionGroup,
14+
DataConditionGroupAction,
15+
DetectorWorkflow,
16+
Workflow,
17+
WorkflowDataConditionGroup,
18+
)
19+
from sentry.workflow_engine.models.data_condition import Condition
20+
from sentry.workflow_engine.typings.grouptype import IssueStreamGroupType
21+
22+
DEFAULT_WORKFLOW_LABEL = "Send a notification for high priority issues"
23+
24+
25+
def connect_workflows_to_issue_stream(
26+
project: Project,
27+
workflows: list[Workflow],
28+
) -> Sequence[DetectorWorkflow]:
29+
# Because we don't know if this signal is handled already or not...
30+
issue_stream_detector = _ensure_detector(project, IssueStreamGroupType.slug)
31+
32+
connections = [
33+
DetectorWorkflow(
34+
workflow=workflow,
35+
detector=issue_stream_detector,
36+
)
37+
for workflow in workflows
38+
]
39+
return DetectorWorkflow.objects.bulk_create(
40+
connections,
41+
ignore_conflicts=True,
42+
)
43+
44+
45+
def create_priority_workflow(org: Organization) -> Workflow:
46+
existing = Workflow.objects.filter(organization=org, name=DEFAULT_WORKFLOW_LABEL).first()
47+
if existing:
48+
return existing
49+
50+
with transaction.atomic(router.db_for_write(Workflow)):
51+
when_condition_group = DataConditionGroup.objects.create(
52+
logic_type=DataConditionGroup.Type.ANY_SHORT_CIRCUIT,
53+
organization=org,
54+
)
55+
56+
workflow = Workflow.objects.create(
57+
organization=org,
58+
name=DEFAULT_WORKFLOW_LABEL,
59+
when_condition_group=when_condition_group,
60+
config={"frequency": 0},
61+
)
62+
63+
# Create the workflow trigger conditions
64+
conditions: list[DataCondition] = []
65+
conditions.append(
66+
DataCondition(
67+
type=Condition.NEW_HIGH_PRIORITY_ISSUE,
68+
condition_group=workflow.when_condition_group,
69+
comparison=True,
70+
condition_result=True,
71+
)
72+
)
73+
conditions.append(
74+
DataCondition(
75+
type=Condition.EXISTING_HIGH_PRIORITY_ISSUE,
76+
condition_group=workflow.when_condition_group,
77+
comparison=True,
78+
condition_result=True,
79+
)
80+
)
81+
DataCondition.objects.bulk_create(conditions)
82+
83+
# Create the Action
84+
action_filter = DataConditionGroup.objects.create(
85+
logic_type=DataConditionGroup.Type.ANY_SHORT_CIRCUIT,
86+
organization=org,
87+
)
88+
89+
action = Action.objects.create(
90+
type=Action.Type.EMAIL,
91+
config={
92+
"target_type": ActionTarget.ISSUE_OWNERS,
93+
"target_identifier": None,
94+
},
95+
data={
96+
"fallthrough_type": FallthroughChoiceType.ACTIVE_MEMBERS.value,
97+
},
98+
)
99+
DataConditionGroupAction.objects.create(
100+
action=action,
101+
condition_group=action_filter,
102+
)
103+
104+
WorkflowDataConditionGroup.objects.create(
105+
workflow=workflow,
106+
condition_group=action_filter,
107+
)
108+
109+
return workflow
110+
111+
112+
def ensure_default_workflows(project: Project) -> list[Workflow]:
113+
workflows = [create_priority_workflow(project.organization)]
114+
connect_workflows_to_issue_stream(project, workflows)
115+
116+
return workflows

src/sentry/workflow_engine/migration_helpers/issue_alert_migration.py

Lines changed: 24 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -77,7 +77,7 @@ def run(self) -> Workflow:
7777

7878
return workflow
7979

80-
def _create_detector_lookups(self) -> list[Detector | None]:
80+
def _create_detector_lookups(self) -> list[Detector]:
8181
if self.rule.source == RuleSource.CRON_MONITOR:
8282
# Find the cron detector that was created before the rule
8383
monitor_slug = None
@@ -87,7 +87,7 @@ def _create_detector_lookups(self) -> list[Detector | None]:
8787
break
8888

8989
if not monitor_slug:
90-
return [None]
90+
return []
9191

9292
try:
9393
with in_test_hide_transaction_boundary():
@@ -105,7 +105,7 @@ def _create_detector_lookups(self) -> list[Detector | None]:
105105
except (Monitor.DoesNotExist, Detector.DoesNotExist):
106106
pass
107107

108-
return [None]
108+
return []
109109

110110
if self.is_dry_run:
111111
error_detector = Detector.objects.filter(
@@ -135,13 +135,30 @@ def _create_detector_lookups(self) -> list[Detector | None]:
135135
defaults={"config": {}, "name": ISSUE_STREAM_DETECTOR_NAME},
136136
)
137137

138-
return [error_detector, issue_stream_detector]
138+
# We are not returning the error_detector here to simplify
139+
# _connect_default_detectors
140+
return [issue_stream_detector]
139141

140142
def _connect_default_detectors(self, workflow: Workflow) -> None:
141143
default_detectors = self._create_detector_lookups()
142-
for detector in default_detectors:
143-
if detector:
144-
DetectorWorkflow.objects.get_or_create(detector=detector, workflow=workflow)
144+
145+
# do not add references to both issue stream and error group types
146+
# it seems like other types might be relying on this as well,
147+
# so this just says not to link the error groups.
148+
# TODO - provide helpers to more easily create these classes
149+
# and references in code, so we can remove the reliance on this code
150+
references_to_create = [
151+
DetectorWorkflow(
152+
detector=detector,
153+
workflow=workflow,
154+
)
155+
for detector in default_detectors
156+
]
157+
158+
DetectorWorkflow.objects.bulk_create(
159+
references_to_create,
160+
ignore_conflicts=True,
161+
)
145162

146163
def _bulk_create_data_conditions(
147164
self,

src/sentry/workflow_engine/receivers/__init__.py

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,5 +6,6 @@
66
from .detector import * # NOQA
77
from .detector_workflow import * # NOQA
88
from .project_detectors import * # noqa: F401,F403
9+
from .project_workflows import * # noqa: F401,F403
910
from .workflow import * # NOQA
1011
from .workflow_data_condition_group import * # NOQA

src/sentry/receivers/rules.py renamed to src/sentry/workflow_engine/receivers/project_workflows.py

Lines changed: 33 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
import logging
2+
from typing import Any
23

34
from django.db import router, transaction
45

@@ -7,7 +8,8 @@
78
from sentry.notifications.types import FallthroughChoiceType
89
from sentry.signals import alert_rule_created, project_created
910
from sentry.users.services.user.model import RpcUser
10-
from sentry.workflow_engine.migration_helpers.issue_alert_migration import IssueAlertMigrator
11+
from sentry.workflow_engine.defaults.workflows import ensure_default_workflows
12+
from sentry.workflow_engine.models import AlertRuleWorkflow
1113

1214
logger = logging.getLogger("sentry")
1315

@@ -34,21 +36,38 @@
3436
PLATFORMS_WITH_PRIORITY_ALERTS = ["python", "javascript"]
3537

3638

37-
def create_default_rules(project: Project, default_rules=True, RuleModel=Rule, **kwargs):
39+
def create_default_workflows(
40+
project: Project,
41+
default_rules: bool = True,
42+
RuleModel: type[Rule] = Rule,
43+
**kwargs: Any,
44+
) -> None:
45+
rule_data = DEFAULT_RULE_DATA
46+
3847
if not default_rules:
3948
return
4049

41-
rule_data = DEFAULT_RULE_DATA
42-
4350
with transaction.atomic(router.db_for_write(RuleModel)):
44-
rule = RuleModel.objects.create(project=project, label=DEFAULT_RULE_LABEL, data=rule_data)
51+
workflows = ensure_default_workflows(project)
4552

46-
workflow = IssueAlertMigrator(rule).run()
47-
logger.info(
48-
"workflow_engine.default_issue_alert.migrated",
49-
extra={"rule_id": rule.id, "workflow_id": workflow.id},
53+
# TODO - we can remove the legacy code below once
54+
# we launch the new UI (and stop referencing legacy models)
55+
rule = RuleModel.objects.create(
56+
project=project,
57+
label=DEFAULT_RULE_LABEL,
58+
data=rule_data,
5059
)
5160

61+
legacy_references = [
62+
AlertRuleWorkflow(
63+
rule_id=rule.id,
64+
workflow=workflow,
65+
)
66+
for workflow in workflows
67+
]
68+
69+
AlertRuleWorkflow.objects.bulk_create(legacy_references)
70+
5271
try:
5372
user: RpcUser = project.organization.get_default_owner()
5473
except IndexError:
@@ -71,4 +90,8 @@ def create_default_rules(project: Project, default_rules=True, RuleModel=Rule, *
7190
)
7291

7392

74-
project_created.connect(create_default_rules, dispatch_uid="create_default_rules", weak=False)
93+
project_created.connect(
94+
create_default_workflows,
95+
dispatch_uid="create_default_workflows",
96+
weak=False,
97+
)

tests/sentry/integrations/slack/tasks/test_tasks.py

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -16,10 +16,10 @@
1616
from sentry.integrations.slack.utils.channel import SlackChannelIdData
1717
from sentry.integrations.slack.utils.rule_status import RedisRuleStatus
1818
from sentry.models.rule import Rule
19-
from sentry.receivers.rules import DEFAULT_RULE_LABEL
2019
from sentry.testutils.cases import TestCase
2120
from sentry.testutils.helpers import install_slack
2221
from sentry.testutils.skips import requires_snuba
22+
from sentry.workflow_engine.receivers.project_workflows import DEFAULT_RULE_LABEL
2323
from tests.sentry.integrations.slack.utils.test_mock_slack_response import mock_slack_response
2424

2525
pytestmark = [requires_snuba]
@@ -192,7 +192,9 @@ def test_task_new_rule_with_owner(self, mock_set_value: MagicMock) -> None:
192192
with self.tasks():
193193
find_channel_id_for_rule(**data)
194194

195-
rule = Rule.objects.exclude(label__in=[DEFAULT_RULE_LABEL]).get(project_id=self.project.id)
195+
rule = Rule.objects.exclude(label__in=[DEFAULT_RULE_LABEL]).get(
196+
project_id=self.project.id,
197+
)
196198
mock_set_value.assert_called_with("success", rule.id)
197199
assert rule.label == "New Rule with Owner"
198200
assert rule.owner_team_id == team.id

tests/sentry/receivers/test_featureadoption.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,6 @@
66
from sentry.models.rule import Rule
77
from sentry.plugins.bases.issue2 import IssueTrackingPlugin2
88
from sentry.plugins.bases.notify import NotificationPlugin
9-
from sentry.receivers.rules import DEFAULT_RULE_DATA
109
from sentry.signals import (
1110
advanced_search,
1211
alert_rule_created,
@@ -24,6 +23,7 @@
2423
user_feedback_received,
2524
)
2625
from sentry.testutils.cases import SnubaTestCase, TestCase
26+
from sentry.workflow_engine.receivers.project_workflows import DEFAULT_RULE_DATA
2727

2828

2929
class FeatureAdoptionTest(TestCase, SnubaTestCase):

tests/sentry/receivers/test_onboarding.py

Lines changed: 17 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -33,7 +33,6 @@
3333
)
3434
from sentry.models.project import Project
3535
from sentry.models.rule import Rule
36-
from sentry.receivers.rules import DEFAULT_RULE_LABEL
3736
from sentry.signals import (
3837
alert_rule_created,
3938
event_processed,
@@ -59,6 +58,7 @@
5958
from sentry.testutils.skips import requires_snuba
6059
from sentry.utils.event import has_event_minified_stack_trace
6160
from sentry.utils.samples import load_data
61+
from sentry.workflow_engine.defaults.workflows import DEFAULT_WORKFLOW_LABEL
6262
from sentry.workflow_engine.models import Workflow
6363
from sentry.workflow_engine.models.detector import Detector
6464
from sentry.workflow_engine.models.detector_workflow import DetectorWorkflow
@@ -168,11 +168,24 @@ def test_project_created__default_workflow(self) -> None:
168168
project = self.create_project(fire_project_created=True)
169169

170170
assert Rule.objects.filter(project=project).exists()
171-
workflow = Workflow.objects.get(organization=project.organization, name=DEFAULT_RULE_LABEL)
171+
workflow = Workflow.objects.get(
172+
organization=project.organization,
173+
name=DEFAULT_WORKFLOW_LABEL,
174+
)
172175

173176
assert Detector.objects.filter(project=project, type=ErrorGroupType.slug).count() == 1
174-
assert Detector.objects.filter(project=project, type=IssueStreamGroupType.slug).count() == 1
175-
assert DetectorWorkflow.objects.filter(workflow=workflow).count() == 2
177+
178+
issue_stream_detectors = Detector.objects.filter(
179+
project=project,
180+
type=IssueStreamGroupType.slug,
181+
)
182+
183+
assert len(issue_stream_detectors) == 1
184+
185+
# Ensure we have 1 connection to the issue stream, this triggers for both monitors above.
186+
result_connections = DetectorWorkflow.objects.filter(workflow=workflow)
187+
assert result_connections.count() == 1
188+
assert result_connections[0].detector_id == issue_stream_detectors[0].id
176189

177190
@patch("sentry.analytics.record", wraps=record)
178191
def test_project_created_with_origin(self, record_analytics: MagicMock) -> None:

0 commit comments

Comments
 (0)