chore(slack): introduce SlackClient using slack_sdk (#72017)

src/sentry/integrations/slack/actions/notification.py

@@ -5,6 +5,7 @@
 from typing import Any
 
 import orjson
+from slack_sdk.errors import SlackApiError
 
 from sentry import features
 from sentry.api.serializers.rest_framework.rule import ACTION_UUID_KEY
@@ -22,7 +23,9 @@
 from sentry.integrations.slack.message_builder.notifications.rule_save_edit import (
     SlackRuleSaveEditMessageBuilder,
 )
+from sentry.integrations.slack.sdk_client import SlackSdkClient
 from sentry.integrations.slack.utils import get_channel_id
+from sentry.integrations.slack.utils.options import has_slack_sdk_flag
 from sentry.models.integrations.integration import Integration
 from sentry.models.options.organization_option import OrganizationOption
 from sentry.models.rule import Rule
@@ -103,8 +106,10 @@ def send_notification(event: GroupEvent, futures: Sequence[RuleFuture]) -> None:
                 "unfurl_links": False,
                 "unfurl_media": False,
             }
+            json_blocks = None
             if payload_blocks := blocks.get("blocks"):
-                payload["blocks"] = orjson.dumps(payload_blocks).decode()
+                json_blocks = orjson.dumps(payload_blocks).decode()
+                payload["blocks"] = json_blocks
 
             rule = rules[0] if rules else None
             rule_to_use = self.rule if self.rule else rule
@@ -128,6 +133,8 @@ def send_notification(event: GroupEvent, futures: Sequence[RuleFuture]) -> None:
 
             # Only try to get the parent notification message if the organization is in the FF
             # We need to search by rule action uuid and rule id, so only search if they exist
+            reply_broadcast = False
+            thread_ts = None
             if (
                 features.has(
                     "organizations:slack-thread-issue-alert", event.group.project.organization
@@ -161,56 +168,108 @@ def send_notification(event: GroupEvent, futures: Sequence[RuleFuture]) -> None:
                     # To reply to a thread, use the specific key in the payload as referenced by the docs
                     # https://api.slack.com/methods/chat.postMessage#arg_thread_ts
                     payload["thread_ts"] = parent_notification_message.message_identifier
+                    thread_ts = parent_notification_message.message_identifier
                     # If this flow is triggered again for the same issue, we want it to be seen in the main channel
                     payload["reply_broadcast"] = True
+                    reply_broadcast = True
+
+            organization = event.group.project.organization
+
+            if has_slack_sdk_flag(organization.id):
+                sdk_client = SlackSdkClient(integration_id=integration.id)
+                text = str(payload["text"]) if payload["text"] is not None else None
+                try:
+                    sdk_response = sdk_client.chat_postMessage(
+                        blocks=json_blocks,
+                        text=text,
+                        channel=channel,
+                        unfurl_links=False,
+                        unfurl_media=False,
+                        thread_ts=thread_ts,
+                        reply_broadcast=reply_broadcast,
+                    )
+                except SlackApiError as e:
+                    # Record the error code and details from the exception
+                    new_notification_message_object.error_code = e.response.status_code
+                    new_notification_message_object.error_details = {
+                        "msg": str(e),
+                        "data": e.response.data,
+                        "url": e.response.api_url,
+                    }
+
+                    log_params = {
+                        "error": str(e),
+                        "project_id": event.project_id,
+                        "event_id": event.event_id,
+                        "channel_name": self.get_option("channel"),
+                    }
+                    # temporarily log the payload so we can debug message failures
+                    log_params["payload"] = orjson.dumps(payload).decode()
+
+                    self.logger.info(
+                        "slack.issue-alert.error",
+                        extra=log_params,
+                    )
+                else:
+                    ts = None
+                    response_data = sdk_response.data
+                    if isinstance(response_data, dict):
+                        ts = response_data.get("ts")
+
+                    self.logger.info("slack.issue-alert.ts", extra={"ts": ts})
+
+                    new_notification_message_object.message_identifier = (
+                        str(ts) if ts is not None else None
+                    )
 
-            client = SlackClient(integration_id=integration.id)
-            try:
-                response = client.post(
-                    "/chat.postMessage", data=payload, timeout=5, log_response_with_error=True
-                )
-            except ApiError as e:
-                # Record the error code and details from the exception
-                new_notification_message_object.error_code = e.code
-                new_notification_message_object.error_details = {
-                    "url": e.url,
-                    "host": e.host,
-                    "path": e.path,
-                    "data": e.json if e.json else e.text,
-                }
-
-                log_params = {
-                    "error": str(e),
-                    "project_id": event.project_id,
-                    "event_id": event.event_id,
-                    "channel_name": self.get_option("channel"),
-                }
-                # temporarily log the payload so we can debug message failures
-                log_params["payload"] = orjson.dumps(payload).decode()
-
-                self.logger.info(
-                    "rule.fail.slack_post",
-                    extra=log_params,
-                )
             else:
-                # Slack will always send back a ts identifier https://api.slack.com/methods/chat.postMessage#examples
-                # on a successful message
-                ts = None
-                # This is a workaround for typing, and the dynamic nature of the return value
-                if isinstance(response, BaseApiResponse):
-                    ts = response.json.get("ts")
-                elif isinstance(response, MappingApiResponse):
-                    ts = response.get("ts")
+                client = SlackClient(integration_id=integration.id)
+                try:
+                    response = client.post(
+                        "/chat.postMessage", data=payload, timeout=5, log_response_with_error=True
+                    )
+                except ApiError as e:
+                    # Record the error code and details from the exception
+                    new_notification_message_object.error_code = e.code
+                    new_notification_message_object.error_details = {
+                        "url": e.url,
+                        "host": e.host,
+                        "path": e.path,
+                        "data": e.json if e.json else e.text,
+                    }
+
+                    log_params = {
+                        "error": str(e),
+                        "project_id": event.project_id,
+                        "event_id": event.event_id,
+                        "channel_name": self.get_option("channel"),
+                    }
+                    # temporarily log the payload so we can debug message failures
+                    log_params["payload"] = orjson.dumps(payload).decode()
+
+                    self.logger.info(
+                        "rule.fail.slack_post",
+                        extra=log_params,
+                    )
                 else:
-                    _default_logger.info(
-                        "failed to get ts from slack response",
-                        extra={
-                            "response_type": type(response).__name__,
-                        },
+                    # Slack will always send back a ts identifier https://api.slack.com/methods/chat.postMessage#examples
+                    # on a successful message
+                    ts = None
+                    # This is a workaround for typing, and the dynamic nature of the return value
+                    if isinstance(response, BaseApiResponse):
+                        ts = response.json.get("ts")
+                    elif isinstance(response, MappingApiResponse):
+                        ts = response.get("ts")
+                    else:
+                        _default_logger.info(
+                            "failed to get ts from slack response",
+                            extra={
+                                "response_type": type(response).__name__,
+                            },
+                        )
+                    new_notification_message_object.message_identifier = (
+                        str(ts) if ts is not None else None
                     )
-                new_notification_message_object.message_identifier = (
-                    str(ts) if ts is not None else None
-                )
 
             if (
                 features.has(

src/sentry/integrations/slack/sdk_client.py

@@ -0,0 +1,91 @@
+import logging
+from functools import wraps
+from types import FunctionType
+
+from slack_sdk import WebClient
+from slack_sdk.errors import SlackApiError
+from slack_sdk.web import SlackResponse
+
+from sentry.models.integrations import Integration
+from sentry.services.hybrid_cloud.integration import integration_service
+from sentry.silo.base import SiloMode
+from sentry.utils import metrics
+
+SLACK_DATADOG_METRIC = "integrations.slack.http_response"
+
+logger = logging.getLogger(__name__)
+
+
+def track_response_data(response: SlackResponse, error: str | None = None) -> None:
+    is_ok = response.get("ok", False)
+    code = response.status_code
+
+    metrics.incr(
+        SLACK_DATADOG_METRIC,
+        sample_rate=1.0,
+        tags={"ok": is_ok, "status": code},
+    )
+
+    extra = {
+        "integration": "slack",
+        "status_string": str(code),
+        "error": error,
+    }
+    logger.info("integration.http_response", extra=extra)
+
+
+def wrapper(method):
+    @wraps(method)
+    def wrapped(*args, **kwargs):
+        try:
+            response = method(*args, **kwargs)
+            if isinstance(response, SlackResponse):
+                track_response_data(response=response)
+        except SlackApiError as e:
+            if e.response:
+                track_response_data(response=e.response, error=str(e))
+            else:
+                logger.info("slack_sdk.missing_error_response", extra={"error": str(e)})
+            raise
+
+        return response
+
+    return wrapped
+
+
+def wrap_methods_in_class(cls):
+    for name, attribute in vars(cls).items():
+        if isinstance(attribute, FunctionType):
+            setattr(cls, name, wrapper(attribute))
+
+    for base in cls.__bases__:
+        wrap_methods_in_class(base)
+
+
+class MetaClass(type):
+    def __new__(meta, name, bases, dct):
+        cls = super().__new__(meta, name, bases, dct)
+        wrap_methods_in_class(cls)
+        return cls
+
+    def __init__(cls, name, bases, dct):
+        super().__init__(name, bases, dct)
+
+
+class SlackSdkClient(WebClient, metaclass=MetaClass):
+    def __init__(self, integration_id: int):
+        integration = None
+        if SiloMode.get_current_mode() == SiloMode.REGION:
+            integration = integration_service.get_integration(integration_id=integration_id)
+        else:  # control or monolith (local)
+            integration = Integration.objects.filter(id=integration_id).first()
+
+        if integration is None:
+            raise ValueError(f"Integration with id {integration_id} not found")
+
+        access_token = integration.metadata.get("access_token")
+        if not access_token:
+            raise ValueError(f"Missing token for integration with id {integration_id}")
+
+        # TODO: missing from old SlackClient: verify_ssl, logging_context
+        super().__init__(token=access_token)

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

@@ -0,0 +1,5 @@
+from sentry import options
+
+
+def has_slack_sdk_flag(organization_id: int) -> bool:
+    return organization_id in options.get("slack.sdk-web-client")

src/sentry/options/defaults.py

@@ -499,6 +499,12 @@
 # signing-secret is preferred, but need to keep verification-token for apps that use it
 register("slack.verification-token", flags=FLAG_CREDENTIAL | FLAG_PRIORITIZE_DISK)
 register("slack.signing-secret", flags=FLAG_CREDENTIAL | FLAG_PRIORITIZE_DISK)
+register(
+    "slack.sdk-web-client",
+    type=Sequence,
+    default=[],
+    flags=FLAG_ALLOW_EMPTY | FLAG_AUTOMATOR_MODIFIABLE,
+)
 
 # Codecov Integration
 register("codecov.client-secret", flags=FLAG_CREDENTIAL | FLAG_PRIORITIZE_DISK)

tests/sentry/integrations/slack/actions/notification/test_slack_notify_service_action.py

@@ -1,14 +1,17 @@
+from unittest.mock import patch
 from urllib.parse import parse_qs
 from uuid import uuid4
 
 import orjson
 import responses
 
 from sentry.integrations.slack import SlackNotifyServiceAction
+from sentry.integrations.slack.sdk_client import SLACK_DATADOG_METRIC
 from sentry.models.notificationmessage import NotificationMessage
 from sentry.models.rulefirehistory import RuleFireHistory
 from sentry.silo.base import SiloMode
 from sentry.testutils.cases import RuleTestCase
+from sentry.testutils.helpers import override_options
 from sentry.testutils.helpers.features import with_feature
 from sentry.testutils.silo import assume_test_silo_mode
 from sentry.types.rules import RuleFuture
@@ -18,6 +21,10 @@ class TestInit(RuleTestCase):
     rule_cls = SlackNotifyServiceAction
 
     def setUp(self) -> None:
+        with assume_test_silo_mode(SiloMode.REGION):
+            self.organization = self.create_organization(id=1, owner=self.user)
+            self.project = self.create_project(organization=self.organization)
+
         with assume_test_silo_mode(SiloMode.CONTROL):
             self.integration = self.create_integration(
                 organization=self.organization,
@@ -94,6 +101,23 @@ def test_after(self):
 
         assert NotificationMessage.objects.all().count() == 0
 
+    @override_options({"slack.sdk-web-client": [1]})
+    @patch("sentry.integrations.slack.sdk_client.metrics")
+    def test_after_using_sdk(self, mock_metrics):
+        # tests error flow because we're actually trying to POST
+
+        rule = self.get_rule(data=self.action_data)
+        results = list(rule.after(event=self.event))
+        assert len(results) == 1
+
+        results[0].callback(self.event, futures=[])
+
+        mock_metrics.incr.assert_called_with(
+            SLACK_DATADOG_METRIC, sample_rate=1.0, tags={"ok": False, "status": 200}
+        )
+
+        assert NotificationMessage.objects.all().count() == 0
+
     @with_feature("organizations:slack-thread-issue-alert")
     @responses.activate
     def test_after_with_threads(self):