diff --git a/src/sentry/features/temporary.py b/src/sentry/features/temporary.py index c8c72dce352abd..032a744ebf6713 100644 --- a/src/sentry/features/temporary.py +++ b/src/sentry/features/temporary.py @@ -393,8 +393,6 @@ def register_temporary_features(manager: FeatureManager) -> None: manager.add("organizations:workflow-engine-metric-detector-limit", OrganizationFeature, FeatureHandlerStrategy.FLAGPOLE, api_expose=True) # Enable seer activities to be evaluated in workflow engine manager.add("organizations:workflow-engine-evaluate-seer-activities", OrganizationFeature, FeatureHandlerStrategy.FLAGPOLE, api_expose=False) - # Route group status change activities through the workflow activity registry (replaces group_status_update_registry) - manager.add("organizations:workflow-engine-status-change-via-activity", OrganizationFeature, FeatureHandlerStrategy.FLAGPOLE, api_expose=False) # Enable our logs product (known internally as ourlogs) in UI and backend manager.add("organizations:ourlogs-enabled", OrganizationFeature, FeatureHandlerStrategy.FLAGPOLE, api_expose=True) # Enable our logs product to be ingested via Relay. diff --git a/src/sentry/issues/status_change_consumer.py b/src/sentry/issues/status_change_consumer.py index c5787b0a148fcb..e4b9ed8ea64ee3 100644 --- a/src/sentry/issues/status_change_consumer.py +++ b/src/sentry/issues/status_change_consumer.py @@ -2,7 +2,7 @@ import logging from collections import defaultdict -from collections.abc import Callable, Iterable, Mapping, Sequence +from collections.abc import Iterable, Mapping, Sequence from typing import Any import sentry_sdk @@ -12,7 +12,6 @@ from sentry.issues.action_log import SYSTEM_ACTOR, ActionSource, action_context_scope from sentry.issues.escalating.escalating import manage_issue_states from sentry.issues.status_change_message import StatusChangeMessageData -from sentry.models.activity import Activity from sentry.models.group import Group, GroupStatus from sentry.models.grouphash import GroupHash from sentry.models.groupinbox import ( @@ -26,7 +25,6 @@ from sentry.types.activity import ActivityType from sentry.types.group import IGNORED_SUBSTATUS_CHOICES, GroupSubStatus from sentry.utils import metrics -from sentry.utils.registry import Registry logger = logging.getLogger(__name__) @@ -151,35 +149,6 @@ def update_status(group: Group, status_change: StatusChangeMessageData) -> None: f"Unsupported status: {status_change['new_status']} {status_change['new_substatus']}" ) - if activity_type is not None: - """ - If we have set created an activity, then we'll also notify any registered handlers - that the group status has changed. - - This is used to trigger the `workflow_engine` processing status changes. - """ - latest_activity = ( - Activity.objects.filter(group_id=group.id, type=activity_type.value) - .order_by("-datetime") - .first() - ) - if latest_activity is not None: - metrics.incr( - "workflow_engine.issue_platform.status_change_handler", - amount=len(group_status_update_registry.registrations.keys()), - tags={"activity_type": activity_type.value}, - sample_rate=1.0, - ) - for handler in group_status_update_registry.registrations.values(): - logger.info( - "group.status_change.activity_created.handler", - extra={ - "group_id": group.id, - "activity_type": activity_type, - }, - ) - handler(group, status_change, latest_activity) - def get_group_from_fingerprint(project_id: int, fingerprint: Sequence[str]) -> Group | None: results = bulk_get_groups_from_fingerprints([(project_id, fingerprint)]) @@ -307,7 +276,3 @@ def process_status_change_message( update_status(group, status_change_data) return group - - -GroupUpdateHandler = Callable[[Group, StatusChangeMessageData, Activity], None] -group_status_update_registry = Registry[GroupUpdateHandler](enable_reverse_lookup=False) diff --git a/src/sentry/workflow_engine/handlers/workflow/__init__.py b/src/sentry/workflow_engine/handlers/workflow/__init__.py index b953b238f0bfdc..f45846f259eecd 100644 --- a/src/sentry/workflow_engine/handlers/workflow/__init__.py +++ b/src/sentry/workflow_engine/handlers/workflow/__init__.py @@ -1,8 +1,6 @@ from .workflow_activity_handlers import activity_handler, seer_activity_handler -from .workflow_status_update_handler import workflow_status_update_handler __all__ = [ "activity_handler", "seer_activity_handler", - "workflow_status_update_handler", ] diff --git a/src/sentry/workflow_engine/handlers/workflow/workflow_activity_handlers.py b/src/sentry/workflow_engine/handlers/workflow/workflow_activity_handlers.py index f24f1fadeb0868..f0068ed227df25 100644 --- a/src/sentry/workflow_engine/handlers/workflow/workflow_activity_handlers.py +++ b/src/sentry/workflow_engine/handlers/workflow/workflow_activity_handlers.py @@ -25,15 +25,11 @@ ActivityType.SEER_ITERATION_COMPLETED, ] -# Activity types handled by the generic activity_handler. This replaces the -# group_status_update_registry path (see workflow_status_update_handler) once the -# organizations:workflow-engine-status-change-via-activity flag is fully rolled out. +# Activity types handled by the generic activity_handler. SUPPORTED_ACTIVITIES = [ ActivityType.SET_RESOLVED, ] -STATUS_CHANGE_VIA_ACTIVITY_FLAG = "organizations:workflow-engine-status-change-via-activity" - @workflow_activity_registry.register("seer_activity") def seer_activity_handler( @@ -127,12 +123,6 @@ def activity_handler( if activity_type not in SUPPORTED_ACTIVITIES: return - if not features.has( - STATUS_CHANGE_VIA_ACTIVITY_FLAG, - group.organization, - ): - return - event_data = WorkflowEventData(event=activity, group=group) try: diff --git a/src/sentry/workflow_engine/handlers/workflow/workflow_status_update_handler.py b/src/sentry/workflow_engine/handlers/workflow/workflow_status_update_handler.py deleted file mode 100644 index 532a768f02c0b3..00000000000000 --- a/src/sentry/workflow_engine/handlers/workflow/workflow_status_update_handler.py +++ /dev/null @@ -1,91 +0,0 @@ -import logging - -from sentry import features -from sentry.issues.status_change_consumer import group_status_update_registry -from sentry.issues.status_change_message import StatusChangeMessageData -from sentry.models.activity import Activity -from sentry.models.group import Group -from sentry.models.organization import Organization -from sentry.types.activity import ActivityType -from sentry.utils import metrics -from sentry.workflow_engine.handlers.workflow.workflow_activity_handlers import ( - STATUS_CHANGE_VIA_ACTIVITY_FLAG, -) - -logger = logging.getLogger(__name__) - -SEER_ACTIVITIES = [ - ActivityType.SEER_RCA_STARTED.value, - ActivityType.SEER_RCA_COMPLETED.value, - ActivityType.SEER_SOLUTION_STARTED.value, - ActivityType.SEER_SOLUTION_COMPLETED.value, - ActivityType.SEER_CODING_STARTED.value, - ActivityType.SEER_CODING_COMPLETED.value, - ActivityType.SEER_PR_CREATED.value, - ActivityType.SEER_ITERATION_STARTED.value, - ActivityType.SEER_ITERATION_COMPLETED.value, -] - -SUPPORTED_ACTIVITIES = [ - ActivityType.SET_RESOLVED.value, - *SEER_ACTIVITIES, -] - - -@group_status_update_registry.register("workflow_status_update") -def workflow_status_update_handler( - group: Group, - status_change_message: StatusChangeMessageData, - activity: Activity, -) -> None: - """ - Hook the process_workflow_task into the activity creation registry. - - Since this handler is called in process for the activity, we want - to queue a task to process workflows asynchronously. - """ - - from sentry.workflow_engine.tasks.workflows import process_workflow_activity - - metrics.incr( - "workflow_engine.tasks.process_workflows.activity_update", - tags={"activity_type": activity.type}, - ) - if activity.type not in SUPPORTED_ACTIVITIES: - # If the activity type is not supported, we do not need to process it. - return - - detector_id = status_change_message.get("detector_id") - - if detector_id is None: - # We should not hit this case, it's should only occur if there is a bug - # passing it from the workflow_engine to the issue platform. - metrics.incr( - "workflow_engine.tasks.error.no_detector_id", - tags={"activity_type": activity.type}, - ) - return - - organization = Organization.objects.get_from_cache(pk=activity.project.organization_id) - - if activity.type == ActivityType.SET_RESOLVED.value and features.has( - STATUS_CHANGE_VIA_ACTIVITY_FLAG, organization - ): - # The generic activity_handler (invoked via create_group_activity) now owns - # status change activities. Skip here to avoid queuing the task twice. - return - - can_process_seer_activities = features.has( - "organizations:workflow-engine-evaluate-seer-activities", organization - ) - - if activity.type in SEER_ACTIVITIES and not can_process_seer_activities: - # Don't process these activities yet - # If the processing is enabled, then it's ok because no workflows can be triggered - return - - process_workflow_activity.delay( - activity_id=activity.id, - group_id=group.id, - detector_id=detector_id, - ) diff --git a/tests/sentry/incidents/test_metric_issue_post_process.py b/tests/sentry/incidents/test_metric_issue_post_process.py index f764808a5041c2..26e5bc5fc7ac0d 100644 --- a/tests/sentry/incidents/test_metric_issue_post_process.py +++ b/tests/sentry/incidents/test_metric_issue_post_process.py @@ -4,11 +4,12 @@ from sentry.issues.issue_occurrence import IssueOccurrence from sentry.issues.status_change_consumer import update_status from sentry.issues.status_change_message import StatusChangeMessage -from sentry.models.group import Group +from sentry.models.group import Group, GroupStatus from sentry.notifications.models.notificationaction import ActionTarget from sentry.services import eventstore from sentry.tasks.post_process import post_process_group from sentry.types.activity import ActivityType +from sentry.types.group import GroupSubStatus from sentry.workflow_engine.models import Detector from sentry.workflow_engine.models.data_condition import Condition from sentry.workflow_engine.types import DetectorPriorityLevel @@ -202,6 +203,9 @@ def test_resolution_from_critical(self, mock_trigger: MagicMock) -> None: evaluation_result = self.process_packet_and_return_result(data_packet) assert isinstance(evaluation_result, StatusChangeMessage) message = evaluation_result.to_dict() + # Packet processing already resolved the group; reset it so update_status creates a + # genuine SET_RESOLVED activity, which is what dispatches workflow processing. + group.update(status=GroupStatus.UNRESOLVED, substatus=GroupSubStatus.ONGOING) # TODO: Actions don't trigger on resolution yet. Update this test when this functionality exists. with patch("sentry.workflow_engine.tasks.workflows.metrics.incr") as mock_incr: with self.tasks(): @@ -233,6 +237,9 @@ def test_resolution_from_warning(self, mock_trigger: MagicMock) -> None: evaluation_result = self.process_packet_and_return_result(data_packet) assert isinstance(evaluation_result, StatusChangeMessage) message = evaluation_result.to_dict() + # Packet processing already resolved the group; reset it so update_status creates a + # genuine SET_RESOLVED activity, which is what dispatches workflow processing. + group.update(status=GroupStatus.UNRESOLVED, substatus=GroupSubStatus.ONGOING) # TODO: Actions don't trigger on resolution yet. Update this test when this functionality exists. with patch("sentry.workflow_engine.tasks.workflows.metrics.incr") as mock_incr: with self.tasks(): diff --git a/tests/sentry/issues/test_status_change_consumer.py b/tests/sentry/issues/test_status_change_consumer.py index 9f05e454cfed5d..7398a375ef7baa 100644 --- a/tests/sentry/issues/test_status_change_consumer.py +++ b/tests/sentry/issues/test_status_change_consumer.py @@ -1,20 +1,17 @@ from __future__ import annotations -from datetime import datetime, timedelta +from datetime import datetime from typing import Any from unittest.mock import MagicMock, patch -from django.utils import timezone - from sentry.incidents.grouptype import MetricIssue from sentry.issues.occurrence_consumer import _process_message -from sentry.issues.status_change_consumer import bulk_get_groups_from_fingerprints, update_status -from sentry.issues.status_change_message import StatusChangeMessageData +from sentry.issues.status_change_consumer import bulk_get_groups_from_fingerprints from sentry.models.activity import Activity from sentry.models.group import Group, GroupStatus from sentry.models.grouphistory import GroupHistory, GroupHistoryStatus from sentry.models.groupinbox import GroupInbox, GroupInboxReason -from sentry.models.groupopenperiod import GroupOpenPeriod, get_latest_open_period +from sentry.models.groupopenperiod import get_latest_open_period from sentry.models.groupopenperiodactivity import GroupOpenPeriodActivity, OpenPeriodActivityType from sentry.testutils.pytest.fixtures import django_db_all from sentry.types.activity import ActivityType @@ -398,152 +395,3 @@ def test_bulk_get_single_project_multiple_hash(self) -> None: tuple([*other_occurrence.fingerprint, *self.occurrence.fingerprint]), ): other_group } - - -class TestStatusChangeRegistry(IssueOccurrenceTestBase): - def setUp(self) -> None: - super().setUp() - self.detector = self.create_detector() - self.group = self.create_group( - project=self.project, - status=GroupStatus.UNRESOLVED, - substatus=GroupSubStatus.ESCALATING, - ) - - status_change = get_test_message_status_change( - self.project.id, - new_status=GroupStatus.RESOLVED, - detector_id=self.detector.id, - ) - - self.message = StatusChangeMessageData( - id="test-id", - project_id=status_change["project_id"], - fingerprint=status_change["fingerprint"], - new_status=status_change["new_status"], - new_substatus=status_change.get("new_substatus"), - detector_id=status_change.get("detector_id"), - activity_data=status_change.get("activity_data"), - ) - - def get_latest_activity(self, activity_type: ActivityType) -> Activity: - latest_activity = ( - Activity.objects.filter(group_id=self.group.id, type=activity_type.value) - .order_by("-datetime") - .first() - ) - - if latest_activity is None: - raise AssertionError(f"No activity found for type {activity_type}") - - return latest_activity - - def test_handler_is_called__resolved(self) -> None: - with patch( - "sentry.issues.status_change_consumer.group_status_update_registry", - ) as mock_registry: - mock_handler = MagicMock() - mock_registry.registrations = { - "test_status_change": mock_handler, - } - - update_status(self.group, self.message) - latest_activity = self.get_latest_activity(ActivityType.SET_RESOLVED) - - assert latest_activity.data == {"test": "test"} - - mock_handler.assert_called_once_with(self.group, self.message, latest_activity) - - def test_handler_is_not_called__unresolved_escalating(self) -> None: - # There will be an issue occurrence that triggers this instead - - self.message["new_status"] = GroupStatus.UNRESOLVED - self.message["new_substatus"] = GroupSubStatus.ESCALATING - with patch( - "sentry.issues.status_change_consumer.group_status_update_registry", - ) as mock_registry: - mock_handler = MagicMock() - mock_registry.registrations = { - "test_status_change": mock_handler, - } - - update_status(self.group, self.message) - assert mock_handler.call_count == 0 - - def test_handler_is_called_unresolved_ongoing(self) -> None: - self.message["new_status"] = GroupStatus.UNRESOLVED - self.message["new_substatus"] = GroupSubStatus.ONGOING - - with patch( - "sentry.issues.status_change_consumer.group_status_update_registry", - ) as mock_registry: - mock_handler = MagicMock() - mock_registry.registrations = { - "test_status_change": mock_handler, - } - - update_status(self.group, self.message) - latest_activity = self.get_latest_activity(ActivityType.AUTO_SET_ONGOING) - - assert latest_activity.data == {"test": "test"} - - mock_handler.assert_called_once_with(self.group, self.message, latest_activity) - - def test_handler_is_called__unresolved_regressed(self) -> None: - self.message["new_status"] = GroupStatus.UNRESOLVED - self.message["new_substatus"] = GroupSubStatus.REGRESSED - - with patch( - "sentry.issues.status_change_consumer.group_status_update_registry", - ) as mock_registry: - mock_handler = MagicMock() - mock_registry.registrations = { - "test_status_change": mock_handler, - } - - update_status(self.group, self.message) - latest_activity = self.get_latest_activity(ActivityType.SET_REGRESSION) - - assert latest_activity.data == {"test": "test"} - - mock_handler.assert_called_once_with(self.group, self.message, latest_activity) - - def test_handler_is_called__ignored(self) -> None: - self.message["new_status"] = GroupStatus.IGNORED - self.message["new_substatus"] = GroupSubStatus.FOREVER - - with patch( - "sentry.issues.status_change_consumer.group_status_update_registry", - ) as mock_registry: - mock_handler = MagicMock() - mock_registry.registrations = { - "test_status_change": mock_handler, - } - - update_status(self.group, self.message) - latest_activity = self.get_latest_activity(ActivityType.SET_IGNORED) - - assert latest_activity.data == {"test": "test"} - - mock_handler.assert_called_once_with(self.group, self.message, latest_activity) - - def test_update_status_with_custom_update_date(self) -> None: - self.message["new_status"] = GroupStatus.RESOLVED - self.message["new_substatus"] = None - custom_datetime = timezone.now() + timedelta(hours=1) - self.message["update_date"] = custom_datetime - - update_status(self.group, self.message) - - self.group.refresh_from_db() - assert self.group.status == GroupStatus.RESOLVED - assert self.group.resolved_at == custom_datetime - - activity = Activity.objects.filter( - group=self.group, type=ActivityType.SET_RESOLVED.value - ).first() - assert activity is not None - assert activity.datetime == custom_datetime - - open_period = GroupOpenPeriod.objects.get(group=self.group) - assert open_period.date_ended == custom_datetime diff --git a/tests/sentry/workflow_engine/handlers/workflow/test_workflow_activity_handlers.py b/tests/sentry/workflow_engine/handlers/workflow/test_workflow_activity_handlers.py index 3b9cd2adfd8e62..8673d489d47138 100644 --- a/tests/sentry/workflow_engine/handlers/workflow/test_workflow_activity_handlers.py +++ b/tests/sentry/workflow_engine/handlers/workflow/test_workflow_activity_handlers.py @@ -8,7 +8,6 @@ from sentry.types.activity import ActivityType from sentry.workflow_engine.handlers.workflow.workflow_activity_handlers import ( SEER_WORKFLOW_ACTIVITIES, - STATUS_CHANGE_VIA_ACTIVITY_FLAG, activity_handler, seer_activity_handler, ) @@ -133,20 +132,12 @@ def setUp(self) -> None: ) self.detector = Detector.objects.get(project=self.project, type=ErrorGroupType.slug) - @mock.patch( - "sentry.workflow_engine.handlers.workflow.workflow_activity_handlers.process_workflow_activity" - ) - def test_feature_flag_disabled(self, mock_process_workflow_activity: MagicMock) -> None: - activity_handler(self.group, self.activity, self.detector.id) - mock_process_workflow_activity.delay.assert_not_called() - @mock.patch("sentry.workflow_engine.handlers.workflow.workflow_activity_handlers.metrics") def test_invalid_activity_type(self, mock_metrics: MagicMock) -> None: self.activity.type = -1 activity_handler(self.group, self.activity, self.detector.id) mock_metrics.incr.assert_not_called() - @with_feature(STATUS_CHANGE_VIA_ACTIVITY_FLAG) @mock.patch( "sentry.workflow_engine.handlers.workflow.workflow_activity_handlers.process_workflow_activity" ) @@ -158,7 +149,6 @@ def test_skips_unsupported_activity_type( mock_process_workflow_activity.delay.assert_not_called() - @with_feature(STATUS_CHANGE_VIA_ACTIVITY_FLAG) @mock.patch( "sentry.workflow_engine.handlers.workflow.workflow_activity_handlers.process_workflow_activity" ) @@ -173,7 +163,6 @@ def test_dispatches_with_provided_detector_id( detector_id=self.detector.id, ) - @with_feature(STATUS_CHANGE_VIA_ACTIVITY_FLAG) @mock.patch( "sentry.workflow_engine.handlers.workflow.workflow_activity_handlers.process_workflow_activity" ) @@ -189,7 +178,6 @@ def test_falls_back_to_preferred_detector( detector_id=self.detector.id, ) - @with_feature(STATUS_CHANGE_VIA_ACTIVITY_FLAG) @mock.patch( "sentry.workflow_engine.handlers.workflow.workflow_activity_handlers.process_workflow_activity" ) diff --git a/tests/sentry/workflow_engine/test_task.py b/tests/sentry/workflow_engine/test_task.py index 66cbc81cdb8f80..e9a1e491723751 100644 --- a/tests/sentry/workflow_engine/test_task.py +++ b/tests/sentry/workflow_engine/test_task.py @@ -7,12 +7,11 @@ from sentry.issues.status_change_consumer import process_status_change_message, update_status from sentry.issues.status_change_message import StatusChangeMessageData from sentry.models.activity import Activity -from sentry.models.group import Group, GroupStatus +from sentry.models.group import GroupStatus from sentry.testutils.cases import TestCase from sentry.testutils.helpers.options import override_options from sentry.testutils.silo import assume_test_silo_mode_of from sentry.types.activity import ActivityType -from sentry.workflow_engine.handlers.workflow import workflow_status_update_handler from sentry.workflow_engine.processors.data_condition_group import TriggerResult from sentry.workflow_engine.processors.workflow import EvaluationStats from sentry.workflow_engine.tasks.utils import fetch_event @@ -40,151 +39,6 @@ def test_fetch_event_retries_on_retry_error(self) -> None: assert result is not None -class WorkflowStatusUpdateHandlerTests(TestCase): - def test__no_detector_id(self) -> None: - group = self.create_group(project=self.project) - activity = Activity( - project=self.project, - group=group, - type=ActivityType.SET_RESOLVED.value, - data={"fingerprint": ["test_fingerprint"]}, - ) - - message = StatusChangeMessageData( - id="test_message_id", - project_id=self.project.id, - new_status=GroupStatus.RESOLVED, - new_substatus=None, - fingerprint=["test_fingerprint"], - detector_id=None, # No detector_id provided - activity_data=None, - ) - - with mock.patch("sentry.workflow_engine.tasks.workflows.metrics.incr") as mock_incr: - workflow_status_update_handler(group, message, activity) - mock_incr.assert_called_with( - "workflow_engine.tasks.error.no_detector_id", - tags={"activity_type": ActivityType.SET_RESOLVED.value}, - ) - - def test_single_processing(self) -> None: - detector = self.create_detector(project=self.project) - group = self.create_group(project=self.project, type=MetricIssue.type_id) - activity = Activity( - project=self.project, - group=group, - type=ActivityType.SET_RESOLVED.value, - data={"fingerprint": ["test_fingerprint"]}, - ) - message = StatusChangeMessageData( - id="test_message_id", - project_id=self.project.id, - new_status=GroupStatus.RESOLVED, - new_substatus=None, - fingerprint=["test_fingerprint"], - detector_id=detector.id, - activity_data={"test": "test"}, - ) - - with mock.patch( - "sentry.workflow_engine.tasks.workflows.process_workflow_activity.delay" - ) as mock_delay: - workflow_status_update_handler(group, message, activity) - mock_delay.assert_called_once_with( - activity_id=activity.id, - group_id=group.id, - detector_id=detector.id, - ) - - def test_dual_processing(self) -> None: - detector = self.create_detector(project=self.project) - group = self.create_group(project=self.project, type=MetricIssue.type_id) - activity = Activity( - project=self.project, - group=group, - type=ActivityType.SET_RESOLVED.value, - data={"fingerprint": ["test_fingerprint"]}, - ) - message = StatusChangeMessageData( - id="test_message_id", - project_id=self.project.id, - new_status=GroupStatus.RESOLVED, - new_substatus=None, - fingerprint=["test_fingerprint"], - detector_id=detector.id, - activity_data={"test": "test"}, - ) - - with mock.patch( - "sentry.workflow_engine.tasks.workflows.process_workflow_activity.delay" - ) as mock_delay: - workflow_status_update_handler(group, message, activity) - mock_delay.assert_called_once_with( - activity_id=activity.id, - group_id=group.id, - detector_id=detector.id, - ) - - def _build_seer_activity_and_message( - self, activity_type: int - ) -> tuple[Group, Activity, StatusChangeMessageData]: - detector = self.create_detector(project=self.project) - group = self.create_group(project=self.project, type=MetricIssue.type_id) - activity = Activity( - project=self.project, - group=group, - type=activity_type, - data={"fingerprint": ["test_fingerprint"]}, - ) - message = StatusChangeMessageData( - id="test_message_id", - project_id=self.project.id, - new_status=GroupStatus.RESOLVED, - new_substatus=None, - fingerprint=["test_fingerprint"], - detector_id=detector.id, - activity_data={"test": "test"}, - ) - return group, activity, message - - def test_seer_activity_blocked_without_feature(self) -> None: - group, activity, message = self._build_seer_activity_and_message( - ActivityType.SEER_RCA_STARTED.value - ) - - with mock.patch( - "sentry.workflow_engine.tasks.workflows.process_workflow_activity.delay" - ) as mock_delay: - workflow_status_update_handler(group, message, activity) - mock_delay.assert_not_called() - - def test_seer_activity_processed_with_feature(self) -> None: - group, activity, message = self._build_seer_activity_and_message( - ActivityType.SEER_RCA_STARTED.value - ) - - with ( - self.feature("organizations:workflow-engine-evaluate-seer-activities"), - mock.patch( - "sentry.workflow_engine.tasks.workflows.process_workflow_activity.delay" - ) as mock_delay, - ): - workflow_status_update_handler(group, message, activity) - mock_delay.assert_called_once() - - def test_seer_activity_increments_metric(self) -> None: - group, activity, message = self._build_seer_activity_and_message( - ActivityType.SEER_RCA_STARTED.value - ) - - with mock.patch("sentry.workflow_engine.tasks.workflows.metrics.incr") as mock_incr: - workflow_status_update_handler(group, message, activity) - mock_incr.assert_any_call( - "workflow_engine.tasks.process_workflows.activity_update", - tags={"activity_type": ActivityType.SEER_RCA_STARTED.value}, - ) - - class TestProcessWorkflowActivity(TestCase): def setUp(self) -> None: self.group = self.create_group(project=self.project, type=MetricIssue.type_id) @@ -386,20 +240,6 @@ def test__e2e__issue_plat_to_processed( with self.tasks(): update_status(self.group, self.message) - # Issue platform is forwarding the activity update - mock_incr.assert_any_call( - "workflow_engine.issue_platform.status_change_handler", - amount=1, - tags={"activity_type": self.activity.type}, - sample_rate=1.0, - ) - - # Workflow engine is correctly registered for the activity update - mock_incr.assert_any_call( - "workflow_engine.tasks.process_workflows.activity_update", - tags={"activity_type": self.activity.type}, - ) - # Workflow engine evaluated activity update in process_workflows mock_incr.assert_any_call( "workflow_engine.tasks.process_workflows.activity_update.executed", @@ -442,20 +282,6 @@ def test__e2e__issue_plat_to_processed_activity_data_is_set( ): process_status_change_message(self.message, txn) - # Issue platform is forwarding the activity update - mock_incr.assert_any_call( - "workflow_engine.issue_platform.status_change_handler", - amount=1, - tags={"activity_type": self.activity.type}, - sample_rate=1.0, - ) - - # Workflow engine is correctly registered for the activity update - mock_incr.assert_any_call( - "workflow_engine.tasks.process_workflows.activity_update", - tags={"activity_type": self.activity.type}, - ) - # Workflow engine evaluated activity update in process_workflows mock_incr.assert_any_call( "workflow_engine.tasks.process_workflows.activity_update.executed", diff --git a/tests/sentry/workflow_engine/test_task_integration.py b/tests/sentry/workflow_engine/test_task_integration.py index 546229cb225309..1fb2ecac098a2a 100644 --- a/tests/sentry/workflow_engine/test_task_integration.py +++ b/tests/sentry/workflow_engine/test_task_integration.py @@ -7,12 +7,8 @@ from sentry.models.group import GroupStatus from sentry.models.grouphash import GroupHash from sentry.testutils.cases import TestCase -from sentry.testutils.helpers.features import with_feature from sentry.types.activity import ActivityType from sentry.types.group import GroupSubStatus -from sentry.workflow_engine.handlers.workflow.workflow_activity_handlers import ( - STATUS_CHANGE_VIA_ACTIVITY_FLAG, -) from tests.sentry.issues.test_status_change_consumer import get_test_message_status_change @@ -35,9 +31,7 @@ def setUp(self) -> None: def test_handler_invoked__when_update_status_called(self) -> None: """ - Integration test to ensure the `update_status` method - will correctly invoke the `workflow_status_update_handler` - and increment the metric. + Integration test to ensure the `update_status` method trigger workflow_engine """ message = get_test_message_status_change( project_id=self.project.id, @@ -46,17 +40,21 @@ def test_handler_invoked__when_update_status_called(self) -> None: ) with mock.patch("sentry.workflow_engine.tasks.workflows.metrics.incr") as mock_incr: - _process_message(message) + with self.tasks(): + _process_message(message) mock_incr.assert_any_call( - "workflow_engine.tasks.process_workflows.activity_update", - tags={"activity_type": ActivityType.SET_RESOLVED.value}, + "workflow_engine.tasks.process_workflows.activity_update.executed", + tags={ + "activity_type": ActivityType.SET_RESOLVED.value, + "detector_type": self.detector.type, + }, + sample_rate=1.0, ) def test_handler_invoked__when_resolved(self) -> None: """ - Integration test to ensure the `update_status` method - will correctly invoke the `workflow_state_update_handler` + Integration test to ensure the `update_status` method trigger workflow_engine and increment the metric. """ message = StatusChangeMessageData( @@ -70,10 +68,15 @@ def test_handler_invoked__when_resolved(self) -> None: ) with mock.patch("sentry.workflow_engine.tasks.workflows.metrics.incr") as mock_incr: - update_status(self.group, message) + with self.tasks(): + update_status(self.group, message) mock_incr.assert_any_call( - "workflow_engine.tasks.process_workflows.activity_update", - tags={"activity_type": ActivityType.SET_RESOLVED.value}, + "workflow_engine.tasks.process_workflows.activity_update.executed", + tags={ + "activity_type": ActivityType.SET_RESOLVED.value, + "detector_type": self.detector.type, + }, + sample_rate=1.0, ) def _resolved_message(self) -> StatusChangeMessageData: @@ -86,29 +89,3 @@ def _resolved_message(self) -> StatusChangeMessageData: detector_id=self.detector.id, activity_data={"test": "test"}, ) - - @mock.patch("sentry.workflow_engine.tasks.workflows.process_workflow_activity.delay") - def test_single_dispatch__flag_off(self, mock_delay: mock.MagicMock) -> None: - """ - With the flag off, only the legacy group_status_update_registry path dispatches - the task. The generic activity_handler bails at the flag check, so we get exactly - one dispatch (no double-processing). - """ - update_status(self.group, self._resolved_message()) - assert mock_delay.call_count == 1 - - @with_feature(STATUS_CHANGE_VIA_ACTIVITY_FLAG) - @mock.patch("sentry.workflow_engine.tasks.workflows.process_workflow_activity.delay") - def test_single_dispatch__flag_on(self, mock_delay: mock.MagicMock) -> None: - """ - With the flag on, the generic activity_handler (via create_group_activity) owns - the dispatch and the legacy handler bails for SET_RESOLVED, so we still get - exactly one dispatch. - """ - update_status(self.group, self._resolved_message()) - assert mock_delay.call_count == 1 - mock_delay.assert_called_once_with( - activity_id=mock.ANY, - group_id=self.group.id, - detector_id=self.detector.id, - )