-
-
Notifications
You must be signed in to change notification settings - Fork 4.7k
feat(workflow_engine): Only link workflows to the IssueStream #112276
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
b99f6ff
4081c33
62d3b09
bca4728
3095966
1f5b8ff
ab791ab
b4286e1
c986a48
af6d9d9
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,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 | ||
| ] | ||
sentry-warden[bot] marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| 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 | ||
sentry[bot] marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
|
||
| with transaction.atomic(router.db_for_write(Workflow)): | ||
|
Contributor
Author
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. #todo - might be nice to provide a helper to create workflows in the future. |
||
| 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 | ||
sentry-warden[bot] marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| 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 | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -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, | ||
|
Member
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. curious; why?
Contributor
Author
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. Crons. Crons creates all their workflows / alerts through this layer, so there may be cases where they are already created / have the connection. if i don't ignore_conflicts there's an edge case that those cause 💥 |
||
| ) | ||
|
|
||
| def _bulk_create_data_conditions( | ||
| self, | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Bug: An
UnableToAcquireLockApiErrorinensure_default_workflowsis silently swallowed bysend_robust(), causing workflow creation to fail without any logging or alerts.Severity: MEDIUM
Suggested Fix
Wrap the call to
_ensure_detectorwithinensure_default_workflowsin a try/except block. Explicitly catch theUnableToAcquireLockApiError, log a warning, and capture the exception to Sentry for visibility, similar to the pattern used inproject_detectors.py.Prompt for AI Agent