-
-
Notifications
You must be signed in to change notification settings - Fork 4.4k
feat(workflow_engine): Setup the Task for process_workflow_updates #93553
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
8b6f227
4a2faaa
e7011de
5495d63
62fc6a1
8e94ad9
a90fcdb
1e0f9c4
4a13c46
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,71 @@ | ||
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.silo.base import SiloMode | ||
from sentry.tasks.base import instrumented_task | ||
from sentry.taskworker import config, namespaces, retry | ||
from sentry.types.activity import ActivityType | ||
from sentry.utils import metrics | ||
|
||
SUPPORTED_ACTIVITIES = [ActivityType.SET_RESOLVED.value] | ||
|
||
|
||
@instrumented_task( | ||
name="sentry.workflow_engine.tasks.process_workflow_activity", | ||
queue="workflow_engine.process_workflows", | ||
acks_late=True, | ||
default_retry_delay=5, | ||
max_retries=3, | ||
soft_time_limit=50, | ||
time_limit=60, | ||
silo_mode=SiloMode.REGION, | ||
taskworker_config=config.TaskworkerConfig( | ||
namespace=namespaces.workflow_engine_tasks, | ||
processing_deadline_duration=60, | ||
retry=retry.Retry( | ||
times=3, | ||
delay=5, | ||
), | ||
), | ||
) | ||
def process_workflow_activity(activity_id: int, detector_id: int) -> None: | ||
""" | ||
Process a workflow task identified by the given Activity ID and Detector ID. | ||
|
||
The task will get the Activity from the database, create a WorkflowEventData object, | ||
and then process the data in `process_workflows`. | ||
""" | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Any ideas on what kind of throughput tasks in the workflow_tasks namespace will have? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Hmm - not sure about throughput, i think that it will vary pretty heavily depending on configurations / products. for example, a metric issue with anomaly detection will be more computing time than evaluating if an event is new. In the end, the throughput would be similar to issue alerts / metric alerts / crons -- all three of those product verticals will be executing here, but they're all fairly different rates. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Ok. Knowing that it will be similar to alerts is helpful. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We can add this to the alerts taskworker pools when we get US deployed. In other regions throughput will be small enough that it isn't a concern. |
||
# TODO - @saponifi3d - implement this in a follow-up PR. This update will require WorkflowEventData | ||
# to allow for an activity in the `event` attribute. That refactor is a bit noisy | ||
# and will be done in a subsequent pr. | ||
pass | ||
|
||
|
||
@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. | ||
""" | ||
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.error.tasks.no_detector_id") | ||
return | ||
|
||
# TODO - implement in follow-up PR for now, just track a metric that we are seeing the activities. | ||
# process_workflow_task.delay(activity.id, detector_id) | ||
metrics.incr( | ||
"workflow_engine.process_workflow.activity_update", tags={"activity_type": activity.type} | ||
) |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,69 @@ | ||
from unittest import mock | ||
|
||
from sentry.issues.status_change_consumer import update_status | ||
from sentry.issues.status_change_message import StatusChangeMessageData | ||
from sentry.models.activity import Activity | ||
from sentry.models.group import GroupStatus | ||
from sentry.testutils.cases import TestCase | ||
from sentry.types.activity import ActivityType | ||
from sentry.types.group import GroupSubStatus | ||
from sentry.workflow_engine.tasks import workflow_status_update_handler | ||
|
||
|
||
class IssuePlatformIntegrationTests(TestCase): | ||
def test_handler_invoked__when_resolved(self): | ||
""" | ||
Integration test to ensure the `update_status` method | ||
will correctly invoke the `workflow_state_update_handler` | ||
and increment the metric. | ||
""" | ||
detector = self.create_detector() | ||
group = self.create_group( | ||
project=self.project, | ||
status=GroupStatus.UNRESOLVED, | ||
substatus=GroupSubStatus.ESCALATING, | ||
) | ||
|
||
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, | ||
) | ||
|
||
with mock.patch("sentry.workflow_engine.tasks.metrics.incr") as mock_incr: | ||
update_status(group, message) | ||
mock_incr.assert_called_with( | ||
"workflow_engine.process_workflow.activity_update", | ||
tags={"activity_type": ActivityType.SET_RESOLVED.value}, | ||
) | ||
|
||
|
||
class WorkflowStatusUpdateHandlerTests(TestCase): | ||
def test__no_detector_id(self): | ||
""" | ||
Test that the workflow_status_update_handler does not crash | ||
when no detector_id is provided in the status change message. | ||
""" | ||
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 | ||
) | ||
|
||
with mock.patch("sentry.workflow_engine.tasks.metrics.incr") as mock_incr: | ||
workflow_status_update_handler(group, message, activity) | ||
mock_incr.assert_called_with("workflow_engine.error.tasks.no_detector_id") |
Uh oh!
There was an error while loading. Please reload this page.