diff --git a/src/sentry/sentry_apps/metrics.py b/src/sentry/sentry_apps/metrics.py index 3bd52c53f1f4d3..d8135f79204e8f 100644 --- a/src/sentry/sentry_apps/metrics.py +++ b/src/sentry/sentry_apps/metrics.py @@ -49,6 +49,7 @@ class SentryAppWebhookFailureReason(StrEnum): INVALID_EVENT = "invalid_event" MISSING_SERVICEHOOK = "missing_servicehook" EVENT_NOT_IN_SERVCEHOOK = "event_not_in_servicehook" + MISSING_ISSUE_OCCURRENCE = "missing_issue_occurrence" MISSING_USER = "missing_user" diff --git a/src/sentry/sentry_apps/tasks/sentry_apps.py b/src/sentry/sentry_apps/tasks/sentry_apps.py index 358ddfd3879edc..acd1d1b621fd8f 100644 --- a/src/sentry/sentry_apps/tasks/sentry_apps.py +++ b/src/sentry/sentry_apps/tasks/sentry_apps.py @@ -119,7 +119,7 @@ def _webhook_event_data( @instrumented_task(name="sentry.sentry_apps.tasks.sentry_apps.send_alert_webhook", **TASK_OPTIONS) @retry_decorator def send_alert_webhook( - rule: str, + rule_label: str, sentry_app_id: int, instance_id: str, group_id: int, @@ -128,79 +128,72 @@ def send_alert_webhook( additional_payload: Mapping[str, Any] | None = None, **kwargs: Any, ): - group = Group.objects.get_from_cache(id=group_id) - assert group, "Group must exist to get related attributes" - project = Project.objects.get_from_cache(id=group.project_id) - organization = Organization.objects.get_from_cache(id=project.organization_id) - extra = { - "sentry_app_id": sentry_app_id, - "project_slug": project.slug, - "organization_slug": organization.slug, - "rule": rule, - } + with SentryAppInteractionEvent( + operation_type=SentryAppInteractionType.PREPARE_WEBHOOK, + event_type=SentryAppEventType.EVENT_ALERT_TRIGGERED, + ).capture() as lifecycle: + group = Group.objects.get_from_cache(id=group_id) + assert group, "Group must exist to get related attributes" + project = Project.objects.get_from_cache(id=group.project_id) + organization = Organization.objects.get_from_cache(id=project.organization_id) + extra: dict[str, int | str] = { + "sentry_app_id": sentry_app_id, + "project_id": project.id, + "organization_slug": organization.slug, + "rule": rule_label, + } + lifecycle.add_extras(extra) - sentry_app = app_service.get_sentry_app_by_id(id=sentry_app_id) - if sentry_app is None: - logger.info("event_alert_webhook.missing_sentry_app", extra=extra) - return + sentry_app = app_service.get_sentry_app_by_id(id=sentry_app_id) + if sentry_app is None: + raise SentryAppSentryError(message=SentryAppWebhookFailureReason.MISSING_SENTRY_APP) - installations = app_service.get_many( - filter=dict( - organization_id=organization.id, - app_ids=[sentry_app.id], - status=SentryAppInstallationStatus.INSTALLED, + installations = app_service.get_many( + filter=dict( + organization_id=organization.id, + app_ids=[sentry_app.id], + status=SentryAppInstallationStatus.INSTALLED, + ) ) - ) - if not installations: - logger.info("event_alert_webhook.missing_installation", extra=extra) - return - (install,) = installations + if not installations: + raise SentryAppSentryError(message=SentryAppWebhookFailureReason.MISSING_INSTALLATION) + (install,) = installations - nodedata = nodestore.backend.get( - BaseEvent.generate_node_id(project_id=project.id, event_id=instance_id) - ) + nodedata = nodestore.backend.get( + BaseEvent.generate_node_id(project_id=project.id, event_id=instance_id) + ) - if not nodedata: - extra = { - "event_id": instance_id, - "occurrence_id": occurrence_id, - "rule": rule, - "sentry_app": sentry_app.slug, - "group_id": group_id, - } - logger.info("send_alert_event.missing_event", extra=extra) - return + if not nodedata: + raise SentryAppSentryError(message=SentryAppWebhookFailureReason.MISSING_EVENT) - occurrence = None - if occurrence_id: - occurrence = IssueOccurrence.fetch(occurrence_id, project_id=project.id) + occurrence = None + if occurrence_id: + occurrence = IssueOccurrence.fetch(occurrence_id, project_id=project.id) - if not occurrence: - logger.info( - "send_alert_event.missing_occurrence", - extra={"occurrence_id": occurrence_id, "project_id": project.id}, - ) - return - - group_event = GroupEvent( - project_id=project.id, - event_id=instance_id, - group=group, - data=nodedata, - occurrence=occurrence, - ) + if not occurrence: + raise SentryAppSentryError( + message=SentryAppWebhookFailureReason.MISSING_ISSUE_OCCURRENCE + ) - event_context = _webhook_event_data(group_event, group.id, project.id) + group_event = GroupEvent( + project_id=project.id, + event_id=instance_id, + group=group, + data=nodedata, + occurrence=occurrence, + ) - data = {"event": event_context, "triggered_rule": rule} + event_context = _webhook_event_data(group_event, group.id, project.id) - # Attach extra payload to the webhook - if additional_payload_key and additional_payload: - data[additional_payload_key] = additional_payload + data = {"event": event_context, "triggered_rule": rule_label} - request_data = AppPlatformEvent( - resource="event_alert", action="triggered", install=install, data=data - ) + # Attach extra payload to the webhook + if additional_payload_key and additional_payload: + data[additional_payload_key] = additional_payload + + request_data = AppPlatformEvent( + resource="event_alert", action="triggered", install=install, data=data + ) send_and_save_webhook_request(sentry_app, request_data) @@ -210,7 +203,7 @@ def send_alert_webhook( "alert_rule_ui_component_webhook.sent", organization_id=organization.id, sentry_app_id=sentry_app_id, - event=f"{request_data.resource}.{request_data.action}", + event=SentryAppEventType.EVENT_ALERT_TRIGGERED, ) @@ -498,6 +491,10 @@ def send_resource_change_webhook( def notify_sentry_app(event: GroupEvent, futures: Sequence[RuleFuture]): for f in futures: if not f.kwargs.get("sentry_app"): + logger.error( + "notify_sentry_app.future_missing_sentry_app", + extra={"event": event.as_dict(), "future": f, "event_id": event.event_id}, + ) continue extra_kwargs: dict[str, Any] = { @@ -519,7 +516,7 @@ def notify_sentry_app(event: GroupEvent, futures: Sequence[RuleFuture]): instance_id=event.event_id, group_id=event.group_id, occurrence_id=event.occurrence_id if hasattr(event, "occurrence_id") else None, - rule=f.rule.label, + rule_label=f.rule.label, sentry_app_id=f.kwargs["sentry_app"].id, **extra_kwargs, ) diff --git a/tests/sentry/sentry_apps/tasks/test_sentry_apps.py b/tests/sentry/sentry_apps/tasks/test_sentry_apps.py index 0e05caf8674274..da63b635e51180 100644 --- a/tests/sentry/sentry_apps/tasks/test_sentry_apps.py +++ b/tests/sentry/sentry_apps/tasks/test_sentry_apps.py @@ -43,6 +43,7 @@ from sentry.testutils.asserts import ( assert_count_of_metric, assert_failure_metric, + assert_halt_metric, assert_many_halt_metrics, assert_success_metric, ) @@ -115,7 +116,8 @@ def setUp(self): ) @patch("sentry.utils.sentry_apps.webhooks.safe_urlopen") - def test_no_sentry_app_for_send_alert_event_v2(self, safe_urlopen): + @patch("sentry.integrations.utils.metrics.EventLifecycle.record_event") + def test_no_sentry_app_for_send_alert_event_v2(self, mock_record, safe_urlopen): event = self.store_event(data={}, project_id=self.project.id) assert event.group is not None group_event = GroupEvent.from_event(event, event.group) @@ -123,12 +125,53 @@ def test_no_sentry_app_for_send_alert_event_v2(self, safe_urlopen): instance_id=group_event.event_id, group_id=group_event.group_id, occurrence_id=None, - rule=self.rule, + rule_label=self.rule.label, sentry_app_id=9999, ) assert not safe_urlopen.called + assert_failure_metric( + mock_record=mock_record, + error_msg=SentryAppSentryError( + message=SentryAppWebhookFailureReason.MISSING_SENTRY_APP + ), + ) + # PREPARE_WEBHOOK (failure) + assert_count_of_metric( + mock_record=mock_record, outcome=EventLifecycleOutcome.STARTED, outcome_count=1 + ) + assert_count_of_metric( + mock_record=mock_record, outcome=EventLifecycleOutcome.FAILURE, outcome_count=1 + ) + + @patch("sentry.utils.sentry_apps.webhooks.safe_urlopen", return_value=MockResponseInstance) + @patch("sentry.integrations.utils.metrics.EventLifecycle.record_event") + def test_missing_event(self, mock_record, safe_urlopen): + project = self.create_project() + issue = self.create_group(project=project) + + send_alert_webhook( + instance_id=123, + group_id=issue.id, + occurrence_id=None, + rule_label=self.rule.label, + sentry_app_id=self.sentry_app.id, + ) + + assert not safe_urlopen.called + + assert_failure_metric( + mock_record, SentryAppSentryError(message=SentryAppWebhookFailureReason.MISSING_EVENT) + ) + # PREPARE_WEBHOOK (failure) + assert_count_of_metric( + mock_record=mock_record, outcome=EventLifecycleOutcome.STARTED, outcome_count=1 + ) + assert_count_of_metric( + mock_record=mock_record, outcome=EventLifecycleOutcome.FAILURE, outcome_count=1 + ) + @patch("sentry.utils.sentry_apps.webhooks.safe_urlopen") def test_no_sentry_app_in_future(self, safe_urlopen): event = self.store_event(data={}, project_id=self.project.id) @@ -142,7 +185,8 @@ def test_no_sentry_app_in_future(self, safe_urlopen): assert not safe_urlopen.called @patch("sentry.utils.sentry_apps.webhooks.safe_urlopen") - def test_no_installation(self, safe_urlopen): + @patch("sentry.integrations.utils.metrics.EventLifecycle.record_event") + def test_no_installation(self, mock_record, safe_urlopen): sentry_app = self.create_sentry_app(organization=self.organization) event = self.store_event(data={}, project_id=self.project.id) assert event.group is not None @@ -153,9 +197,23 @@ def test_no_installation(self, safe_urlopen): notify_sentry_app(group_event, [rule_future]) assert not safe_urlopen.called + assert_failure_metric( + mock_record=mock_record, + error_msg=SentryAppSentryError( + message=SentryAppWebhookFailureReason.MISSING_INSTALLATION + ), + ) + # PREPARE_WEBHOOK (failure) + assert_count_of_metric( + mock_record=mock_record, outcome=EventLifecycleOutcome.STARTED, outcome_count=1 + ) + assert_count_of_metric( + mock_record=mock_record, outcome=EventLifecycleOutcome.FAILURE, outcome_count=1 + ) @patch("sentry.utils.sentry_apps.webhooks.safe_urlopen", return_value=MockResponseInstance) - def test_send_alert_event(self, safe_urlopen): + @patch("sentry.integrations.utils.metrics.EventLifecycle.record_event") + def test_send_alert_event(self, mock_record, safe_urlopen): event = self.store_event(data={}, project_id=self.project.id) assert event.group is not None group = event.group @@ -208,8 +266,19 @@ def test_send_alert_event(self, safe_urlopen): assert requests[0]["response_code"] == 200 assert requests[0]["event_type"] == "event_alert.triggered" + # SLO validation + assert_success_metric(mock_record=mock_record) + # PREPARE_WEBHOOK (success) -> SEND_WEBHOOK (success) + assert_count_of_metric( + mock_record=mock_record, outcome=EventLifecycleOutcome.STARTED, outcome_count=2 + ) + assert_count_of_metric( + mock_record=mock_record, outcome=EventLifecycleOutcome.SUCCESS, outcome_count=2 + ) + @patch("sentry.utils.sentry_apps.webhooks.safe_urlopen", return_value=MockResponseInstance) - def test_send_alert_event_with_additional_payload(self, safe_urlopen): + @patch("sentry.integrations.utils.metrics.EventLifecycle.record_event") + def test_send_alert_event_with_additional_payload(self, mock_record, safe_urlopen): event = self.store_event(data={}, project_id=self.project.id) assert event.group is not None @@ -249,8 +318,19 @@ def test_send_alert_event_with_additional_payload(self, safe_urlopen): assert requests[0]["response_code"] == 200 assert requests[0]["event_type"] == "event_alert.triggered" + # SLO validation + assert_success_metric(mock_record=mock_record) + # PREPARE_WEBHOOK (success) -> SEND_WEBHOOK (success) + assert_count_of_metric( + mock_record=mock_record, outcome=EventLifecycleOutcome.STARTED, outcome_count=2 + ) + assert_count_of_metric( + mock_record=mock_record, outcome=EventLifecycleOutcome.SUCCESS, outcome_count=2 + ) + @patch("sentry.utils.sentry_apps.webhooks.safe_urlopen", return_value=MockResponseInstance) - def test_send_alert_event_with_groupevent(self, safe_urlopen): + @patch("sentry.integrations.utils.metrics.EventLifecycle.record_event") + def test_send_alert_event_with_groupevent(self, mock_record, safe_urlopen): event = self.store_event(data={}, project_id=self.project.id) occurrence_data = self.build_occurrence_data( event_id=event.event_id, project_id=self.project.id @@ -312,6 +392,74 @@ def test_send_alert_event_with_groupevent(self, safe_urlopen): assert requests[0]["response_code"] == 200 assert requests[0]["event_type"] == "event_alert.triggered" + # SLO validation + assert_success_metric(mock_record=mock_record) + # PREPARE_WEBHOOK (success) -> SEND_WEBHOOK (success) + assert_count_of_metric( + mock_record=mock_record, outcome=EventLifecycleOutcome.STARTED, outcome_count=2 + ) + assert_count_of_metric( + mock_record=mock_record, outcome=EventLifecycleOutcome.SUCCESS, outcome_count=2 + ) + + @patch("sentry.utils.sentry_apps.webhooks.safe_urlopen", return_value=MockResponse404) + @patch("sentry.integrations.utils.metrics.EventLifecycle.record_event") + def test_send_alert_event_with_3p_failure(self, mock_record, safe_urlopen): + event = self.store_event(data={}, project_id=self.project.id) + assert event.group is not None + + group_event = GroupEvent.from_event(event, event.group) + settings = [ + {"name": "alert_prefix", "value": "[Not Good]"}, + {"name": "channel", "value": "#ignored-errors"}, + {"name": "best_emoji", "value": ":fire:"}, + {"name": "teamId", "value": 1}, + {"name": "assigneeId", "value": 3}, + ] + + rule_future = RuleFuture( + rule=self.rule, + kwargs={"sentry_app": self.sentry_app, "schema_defined_settings": settings}, + ) + + with self.tasks(): + notify_sentry_app(group_event, [rule_future]) + + ((args, kwargs),) = safe_urlopen.call_args_list + payload = json.loads(kwargs["data"]) + + assert payload["action"] == "triggered" + assert payload["data"]["triggered_rule"] == self.rule.label + assert payload["data"]["issue_alert"] == { + "id": self.rule.id, + "title": self.rule.label, + "sentry_app_id": self.sentry_app.id, + "settings": settings, + } + + buffer = SentryAppWebhookRequestsBuffer(self.sentry_app) + requests = buffer.get_requests() + + assert len(requests) == 1 + assert requests[0]["event_type"] == "event_alert.triggered" + + # SLO validation + assert_success_metric(mock_record=mock_record) + assert_halt_metric( + mock_record=mock_record, + error_msg=f"send_and_save_webhook_request.{SentryAppWebhookHaltReason.GOT_CLIENT_ERROR}_{404}", + ) + # PREPARE_WEBHOOK (success) -> SEND_WEBHOOK (halt) + assert_count_of_metric( + mock_record=mock_record, outcome=EventLifecycleOutcome.STARTED, outcome_count=2 + ) + assert_count_of_metric( + mock_record=mock_record, outcome=EventLifecycleOutcome.SUCCESS, outcome_count=1 + ) + assert_count_of_metric( + mock_record=mock_record, outcome=EventLifecycleOutcome.HALTED, outcome_count=1 + ) + @patch("sentry.utils.sentry_apps.webhooks.safe_urlopen", return_value=MockResponseInstance) class TestProcessResourceChange(TestCase):