diff --git a/src/sentry/incidents/endpoints/serializers/workflow_engine_detector.py b/src/sentry/incidents/endpoints/serializers/workflow_engine_detector.py index 4050ea65052e2c..72c8b5964092d7 100644 --- a/src/sentry/incidents/endpoints/serializers/workflow_engine_detector.py +++ b/src/sentry/incidents/endpoints/serializers/workflow_engine_detector.py @@ -24,13 +24,13 @@ from sentry.users.services.user.service import user_service from sentry.workflow_engine.models import ( Action, - ActionGroupStatus, AlertRuleDetector, DataCondition, DataConditionGroupAction, DataSourceDetector, Detector, ) +from sentry.workflow_engine.models.workflow_action_group_status import WorkflowActionGroupStatus from sentry.workflow_engine.types import DetectorPriorityLevel @@ -177,20 +177,22 @@ def add_latest_incident( for action_ids in detector_to_action_ids.values(): all_action_ids.extend(action_ids) - action_group_statuses = ActionGroupStatus.objects.filter(action_id__in=all_action_ids) + wf_action_group_statuses = WorkflowActionGroupStatus.objects.filter( + action_id__in=all_action_ids + ) - detector_to_group_ids = defaultdict(list) - for action_group_status in action_group_statuses: + detector_to_group_ids = defaultdict(set) + for wf_action_group_status in wf_action_group_statuses: for detector, action_ids in detector_to_action_ids.items(): - if action_group_status.action_id in action_ids: - detector_to_group_ids[detector].append(action_group_status.group_id) + if wf_action_group_status.action_id in action_ids: + detector_to_group_ids[detector].add(wf_action_group_status.group_id) open_periods = None - group_ids = [action_group_status.group_id for action_group_status in action_group_statuses] + group_ids = { + wf_action_group_status.group_id for wf_action_group_status in wf_action_group_statuses + } if group_ids: - open_periods = GroupOpenPeriod.objects.filter( - group__in=[group_id for group_id in group_ids] - ) + open_periods = GroupOpenPeriod.objects.filter(group__in=group_ids) for detector in detectors.values(): # TODO: this serializer is half baked diff --git a/src/sentry/incidents/endpoints/serializers/workflow_engine_incident.py b/src/sentry/incidents/endpoints/serializers/workflow_engine_incident.py index 200e18f77f4b49..61993624d24631 100644 --- a/src/sentry/incidents/endpoints/serializers/workflow_engine_incident.py +++ b/src/sentry/incidents/endpoints/serializers/workflow_engine_incident.py @@ -27,7 +27,6 @@ from sentry.users.services.user.model import RpcUser from sentry.workflow_engine.models import ( Action, - ActionGroupStatus, AlertRuleDetector, DataCondition, DataConditionGroupAction, @@ -37,6 +36,7 @@ IncidentGroupOpenPeriod, WorkflowDataConditionGroup, ) +from sentry.workflow_engine.models.workflow_action_group_status import WorkflowActionGroupStatus class WorkflowEngineIncidentSerializer(Serializer): @@ -191,14 +191,14 @@ def get_open_period_activities(self, open_period: GroupOpenPeriod) -> list[dict[ def get_open_periods_to_detectors( self, open_periods: Sequence[GroupOpenPeriod] ) -> dict[GroupOpenPeriod, Detector]: - action_group_statuses = ActionGroupStatus.objects.filter( + wf_action_group_statuses = WorkflowActionGroupStatus.objects.filter( group__in=[open_period.group for open_period in open_periods] ) open_periods_to_actions: DefaultDict[GroupOpenPeriod, Action] = defaultdict() for open_period in open_periods: - for action_group_status in action_group_statuses: - if action_group_status.group == open_period.group: - open_periods_to_actions[open_period] = action_group_status.action + for wf_action_group_status in wf_action_group_statuses: + if wf_action_group_status.group == open_period.group: + open_periods_to_actions[open_period] = wf_action_group_status.action break dcgas = DataConditionGroupAction.objects.filter( diff --git a/src/sentry/testutils/helpers/backups.py b/src/sentry/testutils/helpers/backups.py index 461906c8e80e48..14e1ff34f17bb2 100644 --- a/src/sentry/testutils/helpers/backups.py +++ b/src/sentry/testutils/helpers/backups.py @@ -110,7 +110,7 @@ from sentry.users.models.userrole import UserRole, UserRoleUser from sentry.utils import json from sentry.workflow_engine.models import Action, DataConditionAlertRuleTrigger, DataConditionGroup -from sentry.workflow_engine.models.action_group_status import ActionGroupStatus +from sentry.workflow_engine.models.workflow_action_group_status import WorkflowActionGroupStatus __all__ = [ "export_to_file", @@ -669,7 +669,9 @@ def create_exhaustive_organization( self.create_alert_rule_detector(detector=detector, alert_rule_id=alert.id) self.create_alert_rule_workflow(workflow=workflow, alert_rule_id=alert.id) - ActionGroupStatus.objects.create(action=send_notification_action, group=group) + WorkflowActionGroupStatus.objects.create( + action=send_notification_action, group=group, workflow=workflow + ) DataConditionAlertRuleTrigger.objects.create( data_condition=data_condition, alert_rule_trigger_id=trigger.id ) diff --git a/src/sentry/workflow_engine/models/action_group_status.py b/src/sentry/workflow_engine/models/action_group_status.py index 2bbfc375ab95f3..c2a905cb66cb14 100644 --- a/src/sentry/workflow_engine/models/action_group_status.py +++ b/src/sentry/workflow_engine/models/action_group_status.py @@ -7,6 +7,7 @@ @region_silo_model class ActionGroupStatus(DefaultFieldsModel): """ + DEPRECATED: Use WorkflowActionGroupStatus instead. This will be removed soon. Stores when an action last fired for a Group. """ diff --git a/src/sentry/workflow_engine/processors/action.py b/src/sentry/workflow_engine/processors/action.py index 41e7045c613d71..cf14cb07da091b 100644 --- a/src/sentry/workflow_engine/processors/action.py +++ b/src/sentry/workflow_engine/processors/action.py @@ -2,10 +2,6 @@ from collections import defaultdict from datetime import datetime, timedelta -from django.db import models -from django.db.models import DurationField, ExpressionWrapper, F, IntegerField, Value -from django.db.models.fields.json import KeyTextTransform -from django.db.models.functions import Cast, Coalesce from django.utils import timezone from sentry import features @@ -24,7 +20,6 @@ from sentry.rules.actions.services import PluginService from sentry.workflow_engine.models import ( Action, - ActionGroupStatus, DataCondition, DataConditionGroup, DataConditionGroupAction, @@ -41,41 +36,6 @@ EnqueuedAction = tuple[DataConditionGroup, list[DataCondition]] -def get_action_last_updated_statuses(now: datetime, actions: BaseQuerySet[Action], group: Group): - # Annotate the actions with the amount of time since the last update - statuses = ActionGroupStatus.objects.filter(group=group, action__in=actions) - - check_workflow_frequency = Cast( - Coalesce( - KeyTextTransform( - "frequency", - F( - "action__dataconditiongroupaction__condition_group__workflowdataconditiongroup__workflow__config" - ), - ), - Value("0"), # default 0 - ), - output_field=IntegerField(), - ) - - frequency_in_minutes = ExpressionWrapper( - F("frequency") * timedelta(minutes=1), # convert to timedelta - output_field=DurationField(), - ) - - time_since_last_update = ExpressionWrapper( - Value(now) - F("date_updated"), output_field=DurationField() - ) - - statuses = statuses.annotate( - frequency=check_workflow_frequency, - frequency_minutes=frequency_in_minutes, - difference=time_since_last_update, - ) - - return statuses - - def create_workflow_fire_histories( actions_to_fire: BaseQuerySet[Action], event_data: WorkflowEventData ) -> list[WorkflowFireHistory]: @@ -101,52 +61,6 @@ def create_workflow_fire_histories( return WorkflowFireHistory.objects.bulk_create(fire_histories) -# TODO(cathy): only reinforce workflow frequency for certain issue types -def filter_recently_fired_actions( - filtered_action_groups: set[DataConditionGroup], event_data: WorkflowEventData -) -> BaseQuerySet[Action]: - # get the actions for any of the triggered data condition groups - actions = ( - Action.objects.filter(dataconditiongroupaction__condition_group__in=filtered_action_groups) - .annotate( - workflow_id=models.F( - "dataconditiongroupaction__condition_group__workflowdataconditiongroup__workflow__id" - ) - ) - .distinct() - ) - - group = event_data.event.group - - now = timezone.now() - statuses = get_action_last_updated_statuses(now, actions, group) - - actions_without_statuses = actions.exclude(id__in=statuses.values_list("action_id", flat=True)) - actions_to_include = set( - statuses.filter(difference__gt=F("frequency_minutes")).values_list("action_id", flat=True) - ) - - ActionGroupStatus.objects.filter( - action__in=actions_to_include, group=group, date_updated__lt=now - ).order_by("id").update(date_updated=now) - ActionGroupStatus.objects.bulk_create( - [ - ActionGroupStatus(action=action, group=group, date_updated=now) - for action in actions_without_statuses - ], - batch_size=1000, - ignore_conflicts=True, - ) - - actions_without_statuses_ids = {action.id for action in actions_without_statuses} - filtered_actions = actions.filter(id__in=actions_to_include | actions_without_statuses_ids) - - # dual write to WorkflowActionGroupStatus, ignoring results for now until they are canonical - _ = filter_recently_fired_workflow_actions(filtered_action_groups, event_data) - - return filtered_actions - - def get_workflow_action_group_statuses( action_to_workflows_ids: dict[int, set[int]], group: Group, workflow_ids: set[int] ) -> dict[int, list[WorkflowActionGroupStatus]]: diff --git a/src/sentry/workflow_engine/processors/delayed_workflow.py b/src/sentry/workflow_engine/processors/delayed_workflow.py index fc20fc0d72f8c3..759d5718860946 100644 --- a/src/sentry/workflow_engine/processors/delayed_workflow.py +++ b/src/sentry/workflow_engine/processors/delayed_workflow.py @@ -48,7 +48,7 @@ ) from sentry.workflow_engine.processors.action import ( create_workflow_fire_histories, - filter_recently_fired_actions, + filter_recently_fired_workflow_actions, ) from sentry.workflow_engine.processors.data_condition_group import ( evaluate_data_conditions, @@ -483,7 +483,7 @@ def fire_actions_for_groups( threshold_seconds=1, ): workflows_actions = evaluate_workflows_action_filters(workflows, event_data) - filtered_actions = filter_recently_fired_actions( + filtered_actions = filter_recently_fired_workflow_actions( action_filters | workflows_actions, event_data ) create_workflow_fire_histories(filtered_actions, event_data) diff --git a/src/sentry/workflow_engine/processors/workflow.py b/src/sentry/workflow_engine/processors/workflow.py index d935f4b5498475..41913b380651d9 100644 --- a/src/sentry/workflow_engine/processors/workflow.py +++ b/src/sentry/workflow_engine/processors/workflow.py @@ -19,7 +19,7 @@ ) from sentry.workflow_engine.processors.action import ( create_workflow_fire_histories, - filter_recently_fired_actions, + filter_recently_fired_workflow_actions, ) from sentry.workflow_engine.processors.contexts.workflow_event_context import ( WorkflowEventContext, @@ -283,7 +283,7 @@ def process_workflows(event_data: WorkflowEventData) -> set[Workflow]: return set() actions_to_trigger = evaluate_workflows_action_filters(triggered_workflows, event_data) - actions = filter_recently_fired_actions(actions_to_trigger, event_data) + actions = filter_recently_fired_workflow_actions(actions_to_trigger, event_data) if not actions: # If there aren't any actions on the associated workflows, there's nothing to trigger return triggered_workflows diff --git a/tests/sentry/incidents/serializers/test_workflow_engine_base.py b/tests/sentry/incidents/serializers/test_workflow_engine_base.py index f6175951466d93..3cc761aee14318 100644 --- a/tests/sentry/incidents/serializers/test_workflow_engine_base.py +++ b/tests/sentry/incidents/serializers/test_workflow_engine_base.py @@ -17,7 +17,8 @@ migrate_metric_data_conditions, migrate_resolve_threshold_data_condition, ) -from sentry.workflow_engine.models import ActionGroupStatus, IncidentGroupOpenPeriod +from sentry.workflow_engine.models import IncidentGroupOpenPeriod +from sentry.workflow_engine.models.workflow_action_group_status import WorkflowActionGroupStatus @freeze_time("2024-12-11 03:21:34") @@ -133,7 +134,10 @@ def add_incident_data(self) -> None: self.group.priority = PriorityLevel.HIGH self.group.save() - ActionGroupStatus.objects.create(action=self.critical_action, group=self.group) + workflow = self.create_workflow() + WorkflowActionGroupStatus.objects.create( + action=self.critical_action, group=self.group, workflow=workflow + ) self.group_open_period = GroupOpenPeriod.objects.get( group=self.group, project=self.detector.project ) diff --git a/tests/sentry/workflow_engine/models/test_action_group_status.py b/tests/sentry/workflow_engine/models/test_action_group_status.py deleted file mode 100644 index 7c26eb9dcb60cc..00000000000000 --- a/tests/sentry/workflow_engine/models/test_action_group_status.py +++ /dev/null @@ -1,18 +0,0 @@ -from datetime import timedelta - -from django.utils import timezone - -from sentry.testutils.cases import TestCase -from sentry.workflow_engine.models.action_group_status import ActionGroupStatus - - -class TestActionGroupStatus(TestCase): - def test_change_date_updated(self): - now = timezone.now() + timedelta(days=1) - action = self.create_action() - status = ActionGroupStatus.objects.create(action=action, group=self.group) - - assert status.date_updated != now - status.update(date_updated=now) - status.refresh_from_db() - assert status.date_updated == now diff --git a/tests/sentry/workflow_engine/processors/test_action.py b/tests/sentry/workflow_engine/processors/test_action.py index 9a8d9f3d86dac8..e62df901fd24ad 100644 --- a/tests/sentry/workflow_engine/processors/test_action.py +++ b/tests/sentry/workflow_engine/processors/test_action.py @@ -12,10 +12,8 @@ WorkflowActionGroupStatus, WorkflowFireHistory, ) -from sentry.workflow_engine.models.action_group_status import ActionGroupStatus from sentry.workflow_engine.processors.action import ( create_workflow_fire_histories, - filter_recently_fired_actions, filter_recently_fired_workflow_actions, get_workflow_action_group_statuses, is_action_permitted, @@ -26,90 +24,6 @@ from tests.sentry.workflow_engine.test_base import BaseWorkflowTest -@freeze_time("2024-01-09") -class TestFilterRecentlyFiredActions(BaseWorkflowTest): - def setUp(self): - ( - self.workflow, - self.detector, - self.detector_workflow, - self.workflow_triggers, - ) = self.create_detector_and_workflow() - - self.action_group, self.action = self.create_workflow_action(workflow=self.workflow) - - self.group, self.event, self.group_event = self.create_group_event( - occurrence=self.build_occurrence(evidence_data={"detector_id": self.detector.id}) - ) - self.event_data = WorkflowEventData(event=self.group_event) - - def test(self): - # test default frequency when no workflow.config set - status_1 = ActionGroupStatus.objects.create(action=self.action, group=self.group) - status_1.update(date_updated=timezone.now() - timedelta(days=1)) - - _, action = self.create_workflow_action(workflow=self.workflow) - status_2 = ActionGroupStatus.objects.create(action=action, group=self.group) - - triggered_actions = filter_recently_fired_actions( - set(DataConditionGroup.objects.all()), self.event_data - ) - assert set(triggered_actions) == {self.action} - - assert {getattr(action, "workflow_id") for action in triggered_actions} == { - self.workflow.id - } - - for status in [status_1, status_2]: - status.refresh_from_db() - assert status.date_updated == timezone.now() - - def test_multiple_workflows(self): - status_1 = ActionGroupStatus.objects.create(action=self.action, group=self.group) - status_1.update(date_updated=timezone.now() - timedelta(hours=1)) - - workflow = self.create_workflow(organization=self.organization, config={"frequency": 1440}) - self.create_detector_workflow(detector=self.detector, workflow=workflow) - _, action_2 = self.create_workflow_action(workflow=workflow) - status_2 = ActionGroupStatus.objects.create(action=action_2, group=self.group) - - _, action_3 = self.create_workflow_action(workflow=workflow) - status_3 = ActionGroupStatus.objects.create(action=action_3, group=self.group) - status_3.update(date_updated=timezone.now() - timedelta(days=2)) - - triggered_actions = filter_recently_fired_actions( - set(DataConditionGroup.objects.all()), self.event_data - ) - assert set(triggered_actions) == {self.action, action_3} - - assert {getattr(action, "workflow_id") for action in triggered_actions} == { - self.workflow.id, - workflow.id, - } - - for status in [status_1, status_2, status_3]: - status.refresh_from_db() - assert status.date_updated == timezone.now() - - def test_creates_workflow_fire_histories(self): - workflow = self.create_workflow(organization=self.organization, config={"frequency": 1440}) - action_group = self.create_data_condition_group(logic_type="any-short") - self.create_data_condition_group_action( - condition_group=action_group, - action=self.action, - ) # shared action - self.create_workflow_data_condition_group(workflow, action_group) - status = WorkflowActionGroupStatus.objects.create( - workflow=workflow, action=self.action, group=self.group - ) - status.update(date_updated=timezone.now() - timedelta(hours=1)) - - triggered_actions = filter_recently_fired_actions( - set(DataConditionGroup.objects.all()), self.event_data - ) - assert set(triggered_actions) == {self.action} - - @freeze_time("2024-01-09") class TestFilterRecentlyFiredWorkflowActions(BaseWorkflowTest): def setUp(self): diff --git a/tests/sentry/workflow_engine/processors/test_workflow.py b/tests/sentry/workflow_engine/processors/test_workflow.py index d080a23c0877a5..8bb05bf9504351 100644 --- a/tests/sentry/workflow_engine/processors/test_workflow.py +++ b/tests/sentry/workflow_engine/processors/test_workflow.py @@ -108,7 +108,7 @@ def test_error_event__logger(self, mock_logger): }, ) - @patch("sentry.workflow_engine.processors.workflow.filter_recently_fired_actions") + @patch("sentry.workflow_engine.processors.workflow.filter_recently_fired_workflow_actions") def test_populate_workflow_env_for_filters(self, mock_filter): # this should not pass because the environment is not None self.error_workflow.update(environment=self.group_event.get_environment())