diff --git a/src/sentry/event_manager.py b/src/sentry/event_manager.py index e5800f7556f5f1..07576521edb3fc 100644 --- a/src/sentry/event_manager.py +++ b/src/sentry/event_manager.py @@ -2006,7 +2006,7 @@ def _get_severity_metadata_for_group( Returns {} if conditions aren't met or on exception. """ - from sentry.receivers.rules import PLATFORMS_WITH_PRIORITY_ALERTS + from sentry.workflow_engine.receivers.project_workflows import PLATFORMS_WITH_PRIORITY_ALERTS if killswitch_matches_context( "issues.severity.skip-seer-requests", {"project_id": event.project_id} diff --git a/src/sentry/receivers/__init__.py b/src/sentry/receivers/__init__.py index 83b66df9be0948..e9af536ddc46a4 100644 --- a/src/sentry/receivers/__init__.py +++ b/src/sentry/receivers/__init__.py @@ -11,7 +11,6 @@ from .owners import * # noqa: F401,F403 from .releases import * # noqa: F401,F403 from .rule_snooze import * # noqa: F401,F403 -from .rules import * # noqa: F401,F403 from .sentry_apps import * # noqa: F401,F403 from .stats import * # noqa: F401,F403 from .superuser import * # noqa: F401,F403 diff --git a/src/sentry/workflow_engine/defaults/workflows.py b/src/sentry/workflow_engine/defaults/workflows.py new file mode 100644 index 00000000000000..058cec2654c395 --- /dev/null +++ b/src/sentry/workflow_engine/defaults/workflows.py @@ -0,0 +1,116 @@ +from typing import Sequence + +from django.db import router, transaction + +from sentry.models.organization import Organization +from sentry.models.project import Project +from sentry.notifications.models.notificationaction import ActionTarget +from sentry.notifications.types import FallthroughChoiceType +from sentry.workflow_engine.defaults.detectors import _ensure_detector +from sentry.workflow_engine.models import ( + Action, + DataCondition, + DataConditionGroup, + DataConditionGroupAction, + DetectorWorkflow, + Workflow, + WorkflowDataConditionGroup, +) +from sentry.workflow_engine.models.data_condition import Condition +from sentry.workflow_engine.typings.grouptype import IssueStreamGroupType + +DEFAULT_WORKFLOW_LABEL = "Send a notification for high priority issues" + + +def connect_workflows_to_issue_stream( + project: Project, + workflows: list[Workflow], +) -> Sequence[DetectorWorkflow]: + # Because we don't know if this signal is handled already or not... + issue_stream_detector = _ensure_detector(project, IssueStreamGroupType.slug) + + connections = [ + DetectorWorkflow( + workflow=workflow, + detector=issue_stream_detector, + ) + for workflow in workflows + ] + return DetectorWorkflow.objects.bulk_create( + connections, + ignore_conflicts=True, + ) + + +def create_priority_workflow(org: Organization) -> Workflow: + existing = Workflow.objects.filter(organization=org, name=DEFAULT_WORKFLOW_LABEL).first() + if existing: + return existing + + with transaction.atomic(router.db_for_write(Workflow)): + when_condition_group = DataConditionGroup.objects.create( + logic_type=DataConditionGroup.Type.ANY_SHORT_CIRCUIT, + organization=org, + ) + + workflow = Workflow.objects.create( + organization=org, + name=DEFAULT_WORKFLOW_LABEL, + when_condition_group=when_condition_group, + config={"frequency": 0}, + ) + + # Create the workflow trigger conditions + conditions: list[DataCondition] = [] + conditions.append( + DataCondition( + type=Condition.NEW_HIGH_PRIORITY_ISSUE, + condition_group=workflow.when_condition_group, + comparison=True, + condition_result=True, + ) + ) + conditions.append( + DataCondition( + type=Condition.EXISTING_HIGH_PRIORITY_ISSUE, + condition_group=workflow.when_condition_group, + comparison=True, + condition_result=True, + ) + ) + DataCondition.objects.bulk_create(conditions) + + # Create the Action + action_filter = DataConditionGroup.objects.create( + logic_type=DataConditionGroup.Type.ANY_SHORT_CIRCUIT, + organization=org, + ) + + action = Action.objects.create( + type=Action.Type.EMAIL, + config={ + "target_type": ActionTarget.ISSUE_OWNERS, + "target_identifier": None, + }, + data={ + "fallthrough_type": FallthroughChoiceType.ACTIVE_MEMBERS.value, + }, + ) + DataConditionGroupAction.objects.create( + action=action, + condition_group=action_filter, + ) + + WorkflowDataConditionGroup.objects.create( + workflow=workflow, + condition_group=action_filter, + ) + + return workflow + + +def ensure_default_workflows(project: Project) -> list[Workflow]: + workflows = [create_priority_workflow(project.organization)] + connect_workflows_to_issue_stream(project, workflows) + + return workflows diff --git a/src/sentry/workflow_engine/migration_helpers/issue_alert_migration.py b/src/sentry/workflow_engine/migration_helpers/issue_alert_migration.py index 29a4edf3354ccd..88966534a5289c 100644 --- a/src/sentry/workflow_engine/migration_helpers/issue_alert_migration.py +++ b/src/sentry/workflow_engine/migration_helpers/issue_alert_migration.py @@ -77,7 +77,7 @@ def run(self) -> Workflow: return workflow - def _create_detector_lookups(self) -> list[Detector | None]: + def _create_detector_lookups(self) -> list[Detector]: if self.rule.source == RuleSource.CRON_MONITOR: # Find the cron detector that was created before the rule monitor_slug = None @@ -87,7 +87,7 @@ def _create_detector_lookups(self) -> list[Detector | None]: break if not monitor_slug: - return [None] + return [] try: with in_test_hide_transaction_boundary(): @@ -105,7 +105,7 @@ def _create_detector_lookups(self) -> list[Detector | None]: except (Monitor.DoesNotExist, Detector.DoesNotExist): pass - return [None] + return [] if self.is_dry_run: error_detector = Detector.objects.filter( @@ -135,13 +135,30 @@ def _create_detector_lookups(self) -> list[Detector | None]: defaults={"config": {}, "name": ISSUE_STREAM_DETECTOR_NAME}, ) - return [error_detector, issue_stream_detector] + # We are not returning the error_detector here to simplify + # _connect_default_detectors + return [issue_stream_detector] def _connect_default_detectors(self, workflow: Workflow) -> None: default_detectors = self._create_detector_lookups() - for detector in default_detectors: - if detector: - DetectorWorkflow.objects.get_or_create(detector=detector, workflow=workflow) + + # do not add references to both issue stream and error group types + # it seems like other types might be relying on this as well, + # so this just says not to link the error groups. + # TODO - provide helpers to more easily create these classes + # and references in code, so we can remove the reliance on this code + references_to_create = [ + DetectorWorkflow( + detector=detector, + workflow=workflow, + ) + for detector in default_detectors + ] + + DetectorWorkflow.objects.bulk_create( + references_to_create, + ignore_conflicts=True, + ) def _bulk_create_data_conditions( self, diff --git a/src/sentry/workflow_engine/receivers/__init__.py b/src/sentry/workflow_engine/receivers/__init__.py index 0434ce88dd83ea..f5aadccdd370e3 100644 --- a/src/sentry/workflow_engine/receivers/__init__.py +++ b/src/sentry/workflow_engine/receivers/__init__.py @@ -6,5 +6,6 @@ from .detector import * # NOQA from .detector_workflow import * # NOQA from .project_detectors import * # noqa: F401,F403 +from .project_workflows import * # noqa: F401,F403 from .workflow import * # NOQA from .workflow_data_condition_group import * # NOQA diff --git a/src/sentry/receivers/rules.py b/src/sentry/workflow_engine/receivers/project_workflows.py similarity index 66% rename from src/sentry/receivers/rules.py rename to src/sentry/workflow_engine/receivers/project_workflows.py index b9699cb29c940f..021f294c88c3b9 100644 --- a/src/sentry/receivers/rules.py +++ b/src/sentry/workflow_engine/receivers/project_workflows.py @@ -1,4 +1,5 @@ import logging +from typing import Any from django.db import router, transaction @@ -7,7 +8,8 @@ from sentry.notifications.types import FallthroughChoiceType from sentry.signals import alert_rule_created, project_created from sentry.users.services.user.model import RpcUser -from sentry.workflow_engine.migration_helpers.issue_alert_migration import IssueAlertMigrator +from sentry.workflow_engine.defaults.workflows import ensure_default_workflows +from sentry.workflow_engine.models import AlertRuleWorkflow logger = logging.getLogger("sentry") @@ -34,21 +36,38 @@ PLATFORMS_WITH_PRIORITY_ALERTS = ["python", "javascript"] -def create_default_rules(project: Project, default_rules=True, RuleModel=Rule, **kwargs): +def create_default_workflows( + project: Project, + default_rules: bool = True, + RuleModel: type[Rule] = Rule, + **kwargs: Any, +) -> None: + rule_data = DEFAULT_RULE_DATA + if not default_rules: return - rule_data = DEFAULT_RULE_DATA - with transaction.atomic(router.db_for_write(RuleModel)): - rule = RuleModel.objects.create(project=project, label=DEFAULT_RULE_LABEL, data=rule_data) + workflows = ensure_default_workflows(project) - workflow = IssueAlertMigrator(rule).run() - logger.info( - "workflow_engine.default_issue_alert.migrated", - extra={"rule_id": rule.id, "workflow_id": workflow.id}, + # TODO - we can remove the legacy code below once + # we launch the new UI (and stop referencing legacy models) + rule = RuleModel.objects.create( + project=project, + label=DEFAULT_RULE_LABEL, + data=rule_data, ) + legacy_references = [ + AlertRuleWorkflow( + rule_id=rule.id, + workflow=workflow, + ) + for workflow in workflows + ] + + AlertRuleWorkflow.objects.bulk_create(legacy_references) + try: user: RpcUser = project.organization.get_default_owner() except IndexError: @@ -71,4 +90,8 @@ def create_default_rules(project: Project, default_rules=True, RuleModel=Rule, * ) -project_created.connect(create_default_rules, dispatch_uid="create_default_rules", weak=False) +project_created.connect( + create_default_workflows, + dispatch_uid="create_default_workflows", + weak=False, +) diff --git a/tests/sentry/integrations/slack/tasks/test_tasks.py b/tests/sentry/integrations/slack/tasks/test_tasks.py index e4177110153646..8972d116412672 100644 --- a/tests/sentry/integrations/slack/tasks/test_tasks.py +++ b/tests/sentry/integrations/slack/tasks/test_tasks.py @@ -16,10 +16,10 @@ from sentry.integrations.slack.utils.channel import SlackChannelIdData from sentry.integrations.slack.utils.rule_status import RedisRuleStatus from sentry.models.rule import Rule -from sentry.receivers.rules import DEFAULT_RULE_LABEL from sentry.testutils.cases import TestCase from sentry.testutils.helpers import install_slack from sentry.testutils.skips import requires_snuba +from sentry.workflow_engine.receivers.project_workflows import DEFAULT_RULE_LABEL from tests.sentry.integrations.slack.utils.test_mock_slack_response import mock_slack_response pytestmark = [requires_snuba] @@ -192,7 +192,9 @@ def test_task_new_rule_with_owner(self, mock_set_value: MagicMock) -> None: with self.tasks(): find_channel_id_for_rule(**data) - rule = Rule.objects.exclude(label__in=[DEFAULT_RULE_LABEL]).get(project_id=self.project.id) + rule = Rule.objects.exclude(label__in=[DEFAULT_RULE_LABEL]).get( + project_id=self.project.id, + ) mock_set_value.assert_called_with("success", rule.id) assert rule.label == "New Rule with Owner" assert rule.owner_team_id == team.id diff --git a/tests/sentry/receivers/test_featureadoption.py b/tests/sentry/receivers/test_featureadoption.py index 8af0c697c1972a..4a39b5b7db04de 100644 --- a/tests/sentry/receivers/test_featureadoption.py +++ b/tests/sentry/receivers/test_featureadoption.py @@ -6,7 +6,6 @@ from sentry.models.rule import Rule from sentry.plugins.bases.issue2 import IssueTrackingPlugin2 from sentry.plugins.bases.notify import NotificationPlugin -from sentry.receivers.rules import DEFAULT_RULE_DATA from sentry.signals import ( advanced_search, alert_rule_created, @@ -24,6 +23,7 @@ user_feedback_received, ) from sentry.testutils.cases import SnubaTestCase, TestCase +from sentry.workflow_engine.receivers.project_workflows import DEFAULT_RULE_DATA class FeatureAdoptionTest(TestCase, SnubaTestCase): diff --git a/tests/sentry/receivers/test_onboarding.py b/tests/sentry/receivers/test_onboarding.py index 7162c7c166095c..90b6c9939c6e1f 100644 --- a/tests/sentry/receivers/test_onboarding.py +++ b/tests/sentry/receivers/test_onboarding.py @@ -33,7 +33,6 @@ ) from sentry.models.project import Project from sentry.models.rule import Rule -from sentry.receivers.rules import DEFAULT_RULE_LABEL from sentry.signals import ( alert_rule_created, event_processed, @@ -59,6 +58,7 @@ from sentry.testutils.skips import requires_snuba from sentry.utils.event import has_event_minified_stack_trace from sentry.utils.samples import load_data +from sentry.workflow_engine.defaults.workflows import DEFAULT_WORKFLOW_LABEL from sentry.workflow_engine.models import Workflow from sentry.workflow_engine.models.detector import Detector from sentry.workflow_engine.models.detector_workflow import DetectorWorkflow @@ -168,11 +168,24 @@ def test_project_created__default_workflow(self) -> None: project = self.create_project(fire_project_created=True) assert Rule.objects.filter(project=project).exists() - workflow = Workflow.objects.get(organization=project.organization, name=DEFAULT_RULE_LABEL) + workflow = Workflow.objects.get( + organization=project.organization, + name=DEFAULT_WORKFLOW_LABEL, + ) assert Detector.objects.filter(project=project, type=ErrorGroupType.slug).count() == 1 - assert Detector.objects.filter(project=project, type=IssueStreamGroupType.slug).count() == 1 - assert DetectorWorkflow.objects.filter(workflow=workflow).count() == 2 + + issue_stream_detectors = Detector.objects.filter( + project=project, + type=IssueStreamGroupType.slug, + ) + + assert len(issue_stream_detectors) == 1 + + # Ensure we have 1 connection to the issue stream, this triggers for both monitors above. + result_connections = DetectorWorkflow.objects.filter(workflow=workflow) + assert result_connections.count() == 1 + assert result_connections[0].detector_id == issue_stream_detectors[0].id @patch("sentry.analytics.record", wraps=record) def test_project_created_with_origin(self, record_analytics: MagicMock) -> None: diff --git a/tests/sentry/workflow_engine/defaults/test_workflows.py b/tests/sentry/workflow_engine/defaults/test_workflows.py new file mode 100644 index 00000000000000..a8572b63f1048a --- /dev/null +++ b/tests/sentry/workflow_engine/defaults/test_workflows.py @@ -0,0 +1,190 @@ +from sentry.notifications.types import FallthroughChoiceType +from sentry.testutils.cases import TestCase +from sentry.workflow_engine.defaults.detectors import ensure_default_detectors +from sentry.workflow_engine.defaults.workflows import ( + connect_workflows_to_issue_stream, + create_priority_workflow, + ensure_default_workflows, +) +from sentry.workflow_engine.models import ( + Action, + DataCondition, + DataConditionGroup, + DataConditionGroupAction, + Detector, + DetectorWorkflow, + Workflow, + WorkflowDataConditionGroup, +) +from sentry.workflow_engine.models.data_condition import Condition +from sentry.workflow_engine.typings.grouptype import IssueStreamGroupType + + +class TestConnectWorkflowsToIssueStream(TestCase): + def test_creates_detector_workflow_connections(self) -> None: + project = self.create_project(create_default_detectors=False) + workflow1 = self.create_workflow( + name="Test Workflow 1", + organization=project.organization, + ) + workflow2 = self.create_workflow( + name="Test Workflow 2", + organization=project.organization, + ) + + connections = connect_workflows_to_issue_stream(project, [workflow1, workflow2]) + + assert len(connections) == 2 + assert DetectorWorkflow.objects.filter(workflow=workflow1).exists() + assert DetectorWorkflow.objects.filter(workflow=workflow2).exists() + + # Verify all workflows are connected to the same issue stream detector + detector_ids = {c.detector_id for c in connections} + assert len(detector_ids) == 1 + detector = Detector.objects.get(id=detector_ids.pop()) + assert detector.type == IssueStreamGroupType.slug + + def test_uses_issue_stream_detector(self) -> None: + project = self.create_project(create_default_detectors=False) + workflow = self.create_workflow( + organization=project.organization, + name="Test Workflow", + ) + + connections = connect_workflows_to_issue_stream(project, [workflow]) + + connection = connections[0] + assert connection.detector.type == IssueStreamGroupType.slug + assert connection.detector.project_id == project.id + + # Verify only one issue stream detector exists + issue_stream_detectors = Detector.objects.filter( + project=project, type=IssueStreamGroupType.slug + ) + assert issue_stream_detectors.count() == 1 + + def test_uses_preexisting_issue_stream_detector(self) -> None: + """Integration test: verifies that if an issue stream detector already exists, it reuses it.""" + project = self.create_project(create_default_detectors=False) + + # Create the default detectors first (simulating project setup signal) + default_detectors = ensure_default_detectors(project) + existing_detector = default_detectors[IssueStreamGroupType.slug] + + # Now connect workflows - should use the existing detector + workflow = self.create_workflow( + organization=project.organization, + name="Test Workflow", + ) + + connections = connect_workflows_to_issue_stream(project, [workflow]) + + # Verify it used the pre-existing detector + assert connections[0].detector_id == existing_detector.id + + # Verify still only one issue stream detector exists + issue_stream_detectors = Detector.objects.filter( + project=project, type=IssueStreamGroupType.slug + ) + assert issue_stream_detectors.count() == 1 + + +class TestCreatePriorityWorkflow(TestCase): + def test_creates_workflow_with_correct_name(self) -> None: + org = self.create_organization() + workflow = create_priority_workflow(org) + + assert workflow.name == "Send a notification for high priority issues" + assert workflow.organization_id == org.id + + def test_creates_when_condition_group(self) -> None: + org = self.create_organization() + workflow = create_priority_workflow(org) + + assert workflow.when_condition_group is not None + assert workflow.when_condition_group.logic_type == DataConditionGroup.Type.ANY_SHORT_CIRCUIT + + def test_creates_data_conditions(self) -> None: + org = self.create_organization() + workflow = create_priority_workflow(org) + + conditions = DataCondition.objects.filter(condition_group=workflow.when_condition_group) + assert conditions.count() == 2 + + condition_types = {c.type for c in conditions} + assert Condition.NEW_HIGH_PRIORITY_ISSUE in condition_types + assert Condition.EXISTING_HIGH_PRIORITY_ISSUE in condition_types + + for condition in conditions: + assert condition.comparison is True + assert condition.condition_result is True + + def test_creates_email_action(self) -> None: + org = self.create_organization() + + create_priority_workflow(org) + + action = Action.objects.get(type=Action.Type.EMAIL) + assert action.config == { + "target_type": 4, + "target_identifier": None, + } + assert action.data == { + "fallthrough_type": FallthroughChoiceType.ACTIVE_MEMBERS.value, + } + + def test_creates_action_filter_and_links(self) -> None: + org = self.create_organization() + + workflow = create_priority_workflow(org) + + # Verify WorkflowDataConditionGroup exists + workflow_dcg = WorkflowDataConditionGroup.objects.get(workflow=workflow) + action_filter = workflow_dcg.condition_group + + # Verify action is linked to the filter + action = Action.objects.get(type=Action.Type.EMAIL) + dcg_action = DataConditionGroupAction.objects.get(action=action) + assert dcg_action.condition_group == action_filter + + # Verify action filter has correct logic type + assert action_filter.logic_type == DataConditionGroup.Type.ANY_SHORT_CIRCUIT + + def test_idempotent_returns_existing_workflow(self) -> None: + org = self.create_organization() + + workflow1 = create_priority_workflow(org) + workflow2 = create_priority_workflow(org) + + assert workflow1.id == workflow2.id + # Should only have one workflow + assert ( + Workflow.objects.filter( + organization=org, name="Send a notification for high priority issues" + ).count() + == 1 + ) + + +class TestEnsureDefaultWorkflows(TestCase): + def test_creates_and_connects_workflows(self) -> None: + project = self.create_project() + + workflows = ensure_default_workflows(project) + + assert len(workflows) == 1 + workflow = workflows[0] + assert workflow.name == "Send a notification for high priority issues" + + # Verify connection to issue stream detector + connection = DetectorWorkflow.objects.get(workflow=workflow) + assert connection.detector.type == IssueStreamGroupType.slug + assert connection.detector.project_id == project.id + + def test_returns_workflows_list(self) -> None: + project = self.create_project() + + workflows = ensure_default_workflows(project) + + assert isinstance(workflows, list) + assert all(isinstance(w, Workflow) for w in workflows) diff --git a/tests/sentry/workflow_engine/migration_helpers/test_issue_alert_migration.py b/tests/sentry/workflow_engine/migration_helpers/test_issue_alert_migration.py index 3a4059f13c1b57..9887c67a447653 100644 --- a/tests/sentry/workflow_engine/migration_helpers/test_issue_alert_migration.py +++ b/tests/sentry/workflow_engine/migration_helpers/test_issue_alert_migration.py @@ -148,8 +148,14 @@ def assert_error_detector_migrated(self, issue_alert: Rule, workflow: Workflow) assert error_detector.type == ErrorGroupType.slug assert error_detector.config == {} - error_detector_workflow = DetectorWorkflow.objects.get(detector=error_detector) - assert error_detector_workflow.workflow == workflow + ## This ensures that the error detector is not directly connected to the workflow, + # _and_ confirms that the issue stream detector would trigger for the general use case. + assert not DetectorWorkflow.objects.filter(detector=error_detector).exists() + assert DetectorWorkflow.objects.filter( + detector__type=IssueStreamGroupType.slug, + detector__project=self.project, + workflow=workflow, + ).exists() return error_detector diff --git a/tests/sentry/workflow_engine/test_integration.py b/tests/sentry/workflow_engine/test_integration.py index 78d1e52fe85dc8..eff6345eded69a 100644 --- a/tests/sentry/workflow_engine/test_integration.py +++ b/tests/sentry/workflow_engine/test_integration.py @@ -8,7 +8,6 @@ from django.utils import timezone from sentry.eventstream.types import EventStreamEventType -from sentry.grouping.grouptype import ErrorGroupType from sentry.incidents.grouptype import MetricIssue from sentry.incidents.utils.types import DATA_SOURCE_SNUBA_QUERY_SUBSCRIPTION from sentry.issues.ingest import save_issue_occurrence @@ -28,6 +27,7 @@ from sentry.workflow_engine.tasks.delayed_workflows import process_delayed_workflows from sentry.workflow_engine.tasks.workflows import schedule_delayed_workflows from sentry.workflow_engine.types import DetectorPriorityLevel +from sentry.workflow_engine.typings.grouptype import IssueStreamGroupType from tests.sentry.workflow_engine.test_base import BaseWorkflowTest @@ -220,7 +220,7 @@ def test_default_workflow(self, mock_trigger: MagicMock) -> None: from sentry.types.group import GroupSubStatus project = self.create_project(fire_project_created=True) - detector = Detector.objects.get(project=project, type=ErrorGroupType.slug) + detector = Detector.objects.get(project=project, type=IssueStreamGroupType.slug) workflow = DetectorWorkflow.objects.get(detector=detector).workflow workflow.update(config={"frequency": 0}) @@ -263,7 +263,10 @@ def test_default_workflow(self, mock_trigger: MagicMock) -> None: # does not fire for low priority issue mock_trigger.reset_mock() low_priority_event = self.create_error_event( - project=project, detector=detector, fingerprint="asdf", level="warning" + project=project, + detector=detector, + fingerprint="asdf", + level="warning", ) self.post_process_error(low_priority_event, is_new=True) assert not mock_trigger.called