🔧 chore: remove redundant slack logging & metrics

src/sentry/integrations/discord/actions/issue_alert/notification.py

@@ -60,18 +60,15 @@ def send_notification(event: GroupEvent, futures: Sequence[RuleFuture]) -> None:
                     client.send_message(channel_id, message, notification_uuid=notification_uuid)
                 except Exception as e:
                     # TODO(iamrajjoshi): Update some of these failures to halts
-                    lifecycle.record_failure(e)
-                    # TODO(iamrajjoshi): Remove the logger after we audit lifecycle
-                    self.logger.error(
-                        "discord.notification.message_send_failure",
-                        extra={
-                            "error": str(e),
+                    lifecycle.add_extras(
+                        {
                             "project_id": event.project_id,
                             "event_id": event.event_id,
                             "guild_id": integration.external_id,
                             "channel_id": channel_id,
-                        },
+                        }
                     )
+                    lifecycle.record_failure(e)
 
             rule = rules[0] if rules else None
             self.record_notification_sent(event, channel_id, rule, notification_uuid)

src/sentry/integrations/discord/actions/metric_alert.py

@@ -64,11 +64,12 @@ def send_incident_alert_notification(
             client.send_message(channel, message)
         except Exception as error:
             # TODO(iamrajjoshi): Update some of these failures to halts
-            lifecycle.record_failure(error)
-            # TODO(iamrajjoshi): Remove the logger after we audit lifecycle
-            logger.warning(
-                "discord.metric_alert.message_send_failure",
-                extra={"error": error, "incident_id": incident.id, "channel_id": channel},
+            lifecycle.add_extras(
+                {
+                    "incident_id": incident.id,
+                    "channel_id": channel,
+                }
             )
+            lifecycle.record_failure(error)
             return False
         return True

src/sentry/integrations/services/integration/impl.py

@@ -460,8 +460,6 @@ def send_msteams_incident_alert_notification(
                 client.send_card(channel, attachment)
                 return True
             except Exception as e:
-                # TODO(iamrajjoshi): Remove the logger after we audit lifecycle
-                logger.info("rule.fail.msteams_post", exc_info=True)
                 lifecycle.add_extras({"integration_id": integration_id, "channel": channel})
                 lifecycle.record_failure(e)
             return False

src/sentry/integrations/slack/metrics.py

@@ -10,12 +10,6 @@
 
 SLACK_ISSUE_ALERT_SUCCESS_DATADOG_METRIC = "sentry.integrations.slack.issue_alert.success"
 SLACK_ISSUE_ALERT_FAILURE_DATADOG_METRIC = "sentry.integrations.slack.issue_alert.failure"
-SLACK_ACTIVITY_THREAD_SUCCESS_DATADOG_METRIC = "sentry.integrations.slack.activity_thread.success"
-SLACK_ACTIVITY_THREAD_FAILURE_DATADOG_METRIC = "sentry.integrations.slack.activity_thread.failure"
-SLACK_METRIC_ALERT_SUCCESS_DATADOG_METRIC = "sentry.integrations.slack.metric_alert.success"
-SLACK_METRIC_ALERT_FAILURE_DATADOG_METRIC = "sentry.integrations.slack.metric_alert.failure"
-SLACK_NOTIFY_RECIPIENT_SUCCESS_DATADOG_METRIC = "sentry.integrations.slack.notify_recipient.success"
-SLACK_NOTIFY_RECIPIENT_FAILURE_DATADOG_METRIC = "sentry.integrations.slack.notify_recipient.failure"
 
 # Webhooks
 SLACK_WEBHOOK_DM_ENDPOINT_SUCCESS_DATADOG_METRIC = "sentry.integrations.slack.dm_endpoint.success"

src/sentry/integrations/slack/service.py

@@ -24,13 +24,7 @@
 from sentry.integrations.slack.message_builder.base.block import BlockSlackMessageBuilder
 from sentry.integrations.slack.message_builder.notifications import get_message_builder
 from sentry.integrations.slack.message_builder.types import SlackBlock
-from sentry.integrations.slack.metrics import (
-    SLACK_ACTIVITY_THREAD_FAILURE_DATADOG_METRIC,
-    SLACK_ACTIVITY_THREAD_SUCCESS_DATADOG_METRIC,
-    SLACK_NOTIFY_RECIPIENT_FAILURE_DATADOG_METRIC,
-    SLACK_NOTIFY_RECIPIENT_SUCCESS_DATADOG_METRIC,
-    record_lifecycle_termination_level,
-)
+from sentry.integrations.slack.metrics import record_lifecycle_termination_level
 from sentry.integrations.slack.sdk_client import SlackSdkClient
 from sentry.integrations.slack.spec import SlackMessagingSpec
 from sentry.integrations.slack.threads.activity_notifications import (
@@ -58,7 +52,6 @@
 from sentry.silo.base import SiloMode
 from sentry.types.activity import ActivityType
 from sentry.types.actor import Actor
-from sentry.utils import metrics
 
 _default_logger = getLogger(__name__)
 
@@ -206,24 +199,31 @@ def notify_all_threads_for_activity(self, activity: Activity) -> None:
 
         # We don't wrap this in a lifecycle because _handle_parent_notification is already wrapped in a lifecycle
         for parent_notification in parent_notifications:
-            try:
-                self._handle_parent_notification(
-                    parent_notification=parent_notification,
-                    notification_to_send=notification_to_send,
-                    client=slack_client,
-                )
-            except Exception as err:
-                # TODO(iamrajjoshi): We can probably swallow this error once we audit the lifecycle
-                self._logger.info(
-                    "failed to send notification",
-                    exc_info=err,
-                    extra={
+            with MessagingInteractionEvent(
+                interaction_type=MessagingInteractionType.SEND_ACTIVITY_NOTIFICATION,
+                spec=SlackMessagingSpec(),
+            ).capture() as lifecycle:
+                lifecycle.add_extras(
+                    {
                         "activity_id": activity.id,
                         "parent_notification_id": parent_notification.id,
                         "notification_to_send": notification_to_send,
                         "integration_id": integration.id,
-                    },
+                        "group_id": activity.group.id,
+                        "project_id": activity.project.id,
+                    }
                 )
+                try:
+                    self._handle_parent_notification(
+                        parent_notification=parent_notification,
+                        notification_to_send=notification_to_send,
+                        client=slack_client,
+                    )
+                except Exception as err:
+                    if isinstance(err, SlackApiError):
+                        record_lifecycle_termination_level(lifecycle, err)
+                    else:
+                        lifecycle.record_failure(err)
 
     def _handle_parent_notification(
         self,
@@ -266,39 +266,12 @@ def _handle_parent_notification(
         json_blocks = orjson.dumps(payload.get("blocks")).decode()
         payload["blocks"] = json_blocks
 
-        extra = {
-            "channel": channel_id,
-            "thread_ts": parent_notification.message_identifier,
-            "rule_action_uuid": parent_notification.rule_action_uuid,
-        }
-
-        with MessagingInteractionEvent(
-            interaction_type=MessagingInteractionType.SEND_ACTIVITY_NOTIFICATION,
-            spec=SlackMessagingSpec(),
-        ).capture() as lifecycle:
-            try:
-                client.chat_postMessage(
-                    channel=channel_id,
-                    thread_ts=parent_notification.message_identifier,
-                    text=notification_to_send,
-                    blocks=json_blocks,
-                )
-                # TODO(iamrajjoshi): Remove this after we validate lifecycle
-                metrics.incr(SLACK_ACTIVITY_THREAD_SUCCESS_DATADOG_METRIC, sample_rate=1.0)
-            except SlackApiError as e:
-                # TODO(iamrajjoshi): Remove this after we validate lifecycle
-                self._logger.info(
-                    "failed to post message to slack",
-                    extra={"error": str(e), "blocks": json_blocks, **extra},
-                )
-                metrics.incr(
-                    SLACK_ACTIVITY_THREAD_FAILURE_DATADOG_METRIC,
-                    sample_rate=1.0,
-                    tags={"ok": e.response.get("ok", False), "status": e.response.status_code},
-                )
-                lifecycle.add_extras({"rule_action_uuid": parent_notification.rule_action_uuid})
-                record_lifecycle_termination_level(lifecycle, e)
-                raise
+        client.chat_postMessage(
+            channel=channel_id,
+            thread_ts=parent_notification.message_identifier,
+            text=notification_to_send,
+            blocks=json_blocks,
+        )
 
     def _get_notification_message_to_send(self, activity: Activity) -> str | None:
         """
@@ -467,17 +440,7 @@ def send_message_to_slack_channel(
                     unfurl_media=False,
                     callback_id=str(payload.get("callback_id", "")),
                 )
-                # TODO(iamrajjoshi): Remove this after we validate lifecycle
-                metrics.incr(SLACK_NOTIFY_RECIPIENT_SUCCESS_DATADOG_METRIC, sample_rate=1.0)
             except SlackApiError as e:
-                # TODO(iamrajjoshi): Remove this after we validate lifecycle
-                extra = {"error": str(e), **log_params}
-                self._logger.info(log_error_message, extra=extra)
-                metrics.incr(
-                    SLACK_NOTIFY_RECIPIENT_FAILURE_DATADOG_METRIC,
-                    sample_rate=1.0,
-                    tags={"ok": e.response.get("ok", False), "status": e.response.status_code},
-                )
                 lifecycle.add_extras(
                     {k: str(v) for k, v in log_params.items() if isinstance(v, (int, str))}
                 )

src/sentry/integrations/slack/utils/notifications.py

@@ -29,8 +29,6 @@
 from sentry.integrations.slack.metrics import (
     SLACK_LINK_IDENTITY_MSG_FAILURE_DATADOG_METRIC,
     SLACK_LINK_IDENTITY_MSG_SUCCESS_DATADOG_METRIC,
-    SLACK_METRIC_ALERT_FAILURE_DATADOG_METRIC,
-    SLACK_METRIC_ALERT_SUCCESS_DATADOG_METRIC,
     record_lifecycle_termination_level,
 )
 from sentry.integrations.slack.sdk_client import SlackSdkClient
@@ -140,7 +138,6 @@ def send_incident_alert_notification(
                 unfurl_links=False,
                 unfurl_media=False,
             )
-            metrics.incr(SLACK_METRIC_ALERT_SUCCESS_DATADOG_METRIC, sample_rate=1.0)
         except SlackApiError as e:
             # Record the error code and details from the exception
             new_notification_message_object.error_code = e.response.status_code
@@ -161,15 +158,6 @@ def send_incident_alert_notification(
             if action.target_display:
                 log_params["channel_name"] = action.target_display
 
-            # TODO(iamrajjoshi): Remove this after we validate lifecycle
-            _logger.info("slack.metric_alert.error", exc_info=True, extra=log_params)
-
-            metrics.incr(
-                SLACK_METRIC_ALERT_FAILURE_DATADOG_METRIC,
-                sample_rate=1.0,
-                tags={"ok": e.response.get("ok", False), "status": e.response.status_code},
-            )
-
             lifecycle.add_extras(log_params)
             # If the error is a channel not found or archived, we can halt the flow
             # This means that the channel was deleted or archived after the alert rule was created

tests/sentry/incidents/action_handlers/test_slack.py

@@ -12,10 +12,6 @@
 from sentry.incidents.models.incident import Incident, IncidentStatus, IncidentStatusMethod
 from sentry.integrations.messaging.spec import MessagingActionHandler
 from sentry.integrations.slack.message_builder.incidents import SlackIncidentsMessageBuilder
-from sentry.integrations.slack.metrics import (
-    SLACK_METRIC_ALERT_FAILURE_DATADOG_METRIC,
-    SLACK_METRIC_ALERT_SUCCESS_DATADOG_METRIC,
-)
 from sentry.integrations.slack.spec import SlackMessagingSpec
 from sentry.integrations.types import EventLifecycleOutcome
 from sentry.models.options.organization_option import OrganizationOption
@@ -122,10 +118,6 @@ def test_fire_metric_alert_sdk(self, mock_metrics, mock_post, mock_api_call, moc
         self._assert_blocks(mock_post, incident, 1000, chart_url)
 
         assert NotificationMessage.objects.all().count() == 1
-        mock_metrics.incr.assert_called_with(
-            SLACK_METRIC_ALERT_SUCCESS_DATADOG_METRIC,
-            sample_rate=1.0,
-        )
 
         assert len(mock_record.mock_calls) == 4
         thread_ts_start, thread_ts_success, send_notification_start, send_notification_success = (
@@ -147,12 +139,6 @@ def test_fire_metric_alert_sdk_error(self, mock_metrics, mock_record):
         assert msg.error_details is not None
         assert msg.error_details["data"] == {"ok": False, "error": "invalid_auth"}
 
-        mock_metrics.incr.assert_called_with(
-            SLACK_METRIC_ALERT_FAILURE_DATADOG_METRIC,
-            sample_rate=1.0,
-            tags={"ok": False, "status": 200},
-        )
-
         assert len(mock_record.mock_calls) == 4
         thread_ts_start, thread_ts_failure, send_notification_start, send_notification_failure = (
             mock_record.mock_calls

tests/sentry/integrations/slack/service/test_slack_service.py

@@ -15,7 +15,6 @@
 from sentry.models.rulefirehistory import RuleFireHistory
 from sentry.notifications.models.notificationmessage import NotificationMessage
 from sentry.silo.base import SiloMode
-from sentry.testutils.asserts import assert_slo_metric
 from sentry.testutils.cases import TestCase
 from sentry.testutils.silo import assume_test_silo_mode
 from sentry.types.activity import ActivityType
@@ -161,7 +160,7 @@ def test_no_parent_notification(self, mock_handle):
 
     @mock.patch("sentry.integrations.utils.metrics.EventLifecycle.record_event")
     @mock.patch("sentry.integrations.slack.service.SlackService._handle_parent_notification")
-    def test_calls_handle_parent_notification_sdk_client(self, mock_handle, mock_record):
+    def test_calls_handle_parent_notification(self, mock_handle, mock_record):
         parent_notification = IssueAlertNotificationMessage.from_model(
             instance=self.parent_notification
         )
@@ -173,7 +172,12 @@ def test_calls_handle_parent_notification_sdk_client(self, mock_handle, mock_rec
         # check client type
         assert isinstance(mock_handle.call_args.kwargs["client"], SlackSdkClient)
 
-        assert_slo_metric(mock_record, EventLifecycleOutcome.SUCCESS)
+        assert len(mock_record.mock_calls) == 4
+        start_1, end_1, start_2, end_2 = mock_record.mock_calls
+        assert start_1.args[0] == EventLifecycleOutcome.STARTED
+        assert start_2.args[0] == EventLifecycleOutcome.STARTED
+        assert end_1.args[0] == EventLifecycleOutcome.SUCCESS
+        assert end_2.args[0] == EventLifecycleOutcome.SUCCESS
 
 
 class TestHandleParentNotification(TestCase):
@@ -240,7 +244,7 @@ def setUp(self) -> None:
 
     @mock.patch("sentry.integrations.utils.metrics.EventLifecycle.record_event")
     @mock.patch("slack_sdk.web.client.WebClient._perform_urllib_http_request")
-    def test_handles_parent_notification_sdk(self, mock_api_call, mock_record):
+    def test_handles_parent_notification(self, mock_api_call, mock_record):
         mock_api_call.return_value = {
             "body": orjson.dumps({"ok": True}).decode(),
             "headers": {},
@@ -252,10 +256,8 @@ def test_handles_parent_notification_sdk(self, mock_api_call, mock_record):
             client=SlackSdkClient(integration_id=self.integration.id),
         )
 
-        assert_slo_metric(mock_record, EventLifecycleOutcome.SUCCESS)
-
     @mock.patch("sentry.integrations.utils.metrics.EventLifecycle.record_event")
-    def test_handles_parent_notification_sdk_error(
+    def test_handles_parent_notification_error(
         self,
         mock_record,
     ) -> None:
@@ -266,8 +268,6 @@ def test_handles_parent_notification_sdk_error(
                 client=SlackSdkClient(integration_id=self.integration.id),
             )
 
-        assert_slo_metric(mock_record, EventLifecycleOutcome.FAILURE)
-
     def test_raises_exception_when_parent_notification_does_not_have_rule_fire_history_data(
         self,
     ) -> None:

tests/sentry/integrations/slack/test_notifications.py

@@ -4,10 +4,6 @@
 from slack_sdk.errors import SlackApiError
 from slack_sdk.web import SlackResponse
 
-from sentry.integrations.slack.metrics import (
-    SLACK_NOTIFY_RECIPIENT_FAILURE_DATADOG_METRIC,
-    SLACK_NOTIFY_RECIPIENT_SUCCESS_DATADOG_METRIC,
-)
 from sentry.integrations.slack.notifications import send_notification_as_slack
 from sentry.integrations.types import ExternalProviders
 from sentry.notifications.additional_attachment_manager import manager
@@ -27,8 +23,7 @@ def setUp(self):
         super().setUp()
         self.notification = DummyNotification(self.organization)
 
-    @patch("sentry.integrations.slack.service.metrics")
-    def test_additional_attachment(self, mock_metrics):
+    def test_additional_attachment(self):
         with (
             patch.dict(
                 manager.attachment_generators,
@@ -68,13 +63,7 @@ def test_additional_attachment(self, mock_metrics):
             assert blocks[3]["text"]["text"] == self.organization.slug
             assert blocks[4]["text"]["text"] == self.integration.id
 
-        mock_metrics.incr.assert_called_with(
-            SLACK_NOTIFY_RECIPIENT_SUCCESS_DATADOG_METRIC,
-            sample_rate=1.0,
-        )
-
-    @patch("sentry.integrations.slack.service.metrics")
-    def test_no_additional_attachment(self, mock_metrics):
+    def test_no_additional_attachment(self):
         with self.tasks():
             send_notification_as_slack(self.notification, [self.user], {}, {})
 
@@ -105,22 +94,15 @@ def test_no_additional_attachment(self, mock_metrics):
             "type": "actions",
         }
 
-    @patch("sentry.integrations.slack.service.metrics")
-    def test_send_notification_as_slack(self, mock_metrics):
+    def test_send_notification_as_slack(self):
         with patch.dict(
             manager.attachment_generators,
             {ExternalProviders.SLACK: additional_attachment_generator_block_kit},
         ):
             with self.tasks():
                 send_notification_as_slack(self.notification, [self.user], {}, {})
 
-        mock_metrics.incr.assert_called_with(
-            SLACK_NOTIFY_RECIPIENT_SUCCESS_DATADOG_METRIC,
-            sample_rate=1.0,
-        )
-
-    @patch("sentry.integrations.slack.service.metrics")
-    def test_send_notification_as_slack_error(self, mock_metrics):
+    def test_send_notification_as_slack_error(self):
         mock_slack_response = SlackResponse(
             client=None,
             http_verb="POST",
@@ -143,9 +125,3 @@ def test_send_notification_as_slack_error(self, mock_metrics):
         ):
             with self.tasks():
                 send_notification_as_slack(self.notification, [self.user], {}, {})
-
-        mock_metrics.incr.assert_called_with(
-            SLACK_NOTIFY_RECIPIENT_FAILURE_DATADOG_METRIC,
-            sample_rate=1.0,
-            tags={"ok": False, "status": 200},
-        )