ref(slack): Add APM to Slack and use use ApiClient (#18346)

* ref(slack): Add APM to Slack and use use ApiClient

Author: MeredithAnya

src/sentry/api/endpoints/project_rule_details.py

@@ -9,11 +9,13 @@
 from sentry.integrations.slack import tasks
 from sentry.mediators import project_rules
 from sentry.models import AuditLogEntryEvent, Rule, RuleStatus
+from sentry.web.decorators import transaction_start
 
 
 class ProjectRuleDetailsEndpoint(ProjectEndpoint):
     permission_classes = [ProjectSettingPermission]
 
+    @transaction_start("ProjectRuleDetailsEndpoint")
     def get(self, request, project, rule_id):
         """
         Retrieve a rule
@@ -28,6 +30,7 @@ def get(self, request, project, rule_id):
         )
         return Response(serialize(rule, request.user))
 
+    @transaction_start("ProjectRuleDetailsEndpoint")
     def put(self, request, project, rule_id):
         """
         Update a rule
@@ -81,6 +84,7 @@ def put(self, request, project, rule_id):
 
         return Response(serializer.errors, status=status.HTTP_400_BAD_REQUEST)
 
+    @transaction_start("ProjectRuleDetailsEndpoint")
     def delete(self, request, project, rule_id):
         """
         Delete a rule

src/sentry/api/endpoints/project_rules.py

@@ -11,9 +11,11 @@
 from sentry.mediators import project_rules
 from sentry.models import AuditLogEntryEvent, Rule, RuleStatus
 from sentry.signals import alert_rule_created
+from sentry.web.decorators import transaction_start
 
 
 class ProjectRulesEndpoint(ProjectEndpoint):
+    @transaction_start("ProjectRulesEndpoint")
     def get(self, request, project):
         """
         List a project's rules
@@ -34,6 +36,7 @@ def get(self, request, project):
             on_results=lambda x: serialize(x, request.user),
         )
 
+    @transaction_start("ProjectRulesEndpoint")
     def post(self, request, project):
         """
         Create a rule

src/sentry/integrations/slack/action_endpoint.py

@@ -3,15 +3,18 @@
 import six
 
 from sentry import analytics
-from sentry import http
+
 from sentry.api import client
 from sentry.api.base import Endpoint
 from sentry.models import Group, Project, Identity, IdentityProvider, ApiKey
 from sentry.utils import json
+from sentry.web.decorators import transaction_start
+from sentry.shared_integrations.exceptions import ApiError
 
+from .client import SlackClient
 from .link_identity import build_linking_url
 from .requests import SlackActionRequest, SlackRequestError
-from .utils import build_group_attachment, logger, track_response_code
+from .utils import build_group_attachment, logger
 
 LINK_IDENTITY_MESSAGE = "Looks like you haven't linked your Sentry account with your Slack identity yet! <{associate_url}|Link your identity now> to perform actions in Sentry through Slack."
 
@@ -120,13 +123,11 @@ def open_resolve_dialog(self, data, group, integration):
             "token": integration.metadata["access_token"],
         }
 
-        session = http.build_session()
-        req = session.post("https://slack.com/api/dialog.open", data=payload)
-        status_code = req.status_code
-        resp = req.json()
-        if not resp.get("ok"):
-            logger.error("slack.action.response-error", extra={"response": resp})
-        track_response_code(status_code, resp.get("ok"))
+        slack_client = SlackClient()
+        try:
+            slack_client.post("/dialog.open", data=payload)
+        except ApiError as e:
+            logger.error("slack.action.response-error", extra={"error": six.text_type(e)})
 
     def construct_reply(self, attachment, is_message=False):
         # XXX(epurkhiser): Slack is inconsistent about it's expected responses
@@ -149,6 +150,7 @@ def is_message(self, data):
         # posted messages will not have the type at all.
         return data.get("original_message", {}).get("type") == "message"
 
+    @transaction_start("SlackActionEndpoint")
     def post(self, request):
         logging_data = {}
 
@@ -229,13 +231,14 @@ def post(self, request):
             )
 
             # use the original response_url to update the link attachment
-            session = http.build_session()
-            req = session.post(slack_request.callback_data["orig_response_url"], json=body)
-            status_code = req.status_code
-            resp = req.json()
-            if not resp.get("ok"):
-                logger.error("slack.action.response-error", extra={"response": resp})
-            track_response_code(status_code, resp.get("ok"))
+            slack_client = SlackClient()
+            try:
+                slack_client.post(
+                    slack_request.callback_data["orig_response_url"], data=body, json=True
+                )
+            except ApiError as e:
+                logger.error("slack.action.response-error", extra={"error": six.text_type(e)})
+
             return self.respond()
 
         # Usually we'll want to respond with the updated attachment including

src/sentry/integrations/slack/client.py

@@ -0,0 +1,52 @@
+from __future__ import absolute_import
+
+import six
+
+from sentry.integrations.client import ApiClient
+from sentry.shared_integrations.exceptions import ApiError
+from sentry.utils import metrics
+
+SLACK_DATADOG_METRIC = "integrations.slack.http_response"
+
+
+class SlackClient(ApiClient):
+    allow_redirects = False
+    integration_name = "slack"
+    base_url = "https://slack.com/api"
+    datadog_prefix = "integrations.slack"
+
+    def track_response_data(self, code, span, error=None, resp=None):
+        span.set_http_status(code)
+        span.set_tag("integration", "slack")
+
+        is_ok = False
+        # If Slack gives us back a 200 we still want to check the 'ok' param
+        if resp:
+            response = resp.json()
+            is_ok = response.get("ok")
+            span.set_tag("ok", is_ok)
+
+            # when 'ok' is False, we can add the error we get back as a tag
+            if not is_ok:
+                error = response.get("error")
+                span.set_tag("slack_error", error)
+
+        metrics.incr(
+            SLACK_DATADOG_METRIC, sample_rate=1.0, tags={"ok": is_ok, "status": code},
+        )
+
+        extra = {
+            self.integration_type: self.name,
+            "status_string": six.text_type(code),
+            "error": six.text_type(error)[:256] if error else None,
+        }
+        extra.update(getattr(self, "logging_context", None) or {})
+        self.logger.info(u"%s.http_response" % (self.integration_type), extra=extra)
+
+    def request(self, method, path, headers=None, data=None, params=None, json=False, timeout=None):
+        # TODO(meredith): Slack actually supports json now for the chat.postMessage so we
+        # can update that so we don't have to pass json=False here
+        response = self._request(method, path, headers=headers, data=data, params=params, json=json)
+        if not response.json.get("ok"):
+            raise ApiError(response.get("error", ""))
+        return response

src/sentry/integrations/slack/event_endpoint.py

@@ -7,13 +7,15 @@
 
 from django.db.models import Q
 
-from sentry import http
 from sentry.api.base import Endpoint
 from sentry.incidents.models import Incident
 from sentry.models import Group, Project
+from sentry.shared_integrations.exceptions import ApiError
+from sentry.web.decorators import transaction_start
 
+from .client import SlackClient
 from .requests import SlackEventRequest, SlackRequestError
-from .utils import build_group_attachment, build_incident_attachment, logger, track_response_code
+from .utils import build_group_attachment, build_incident_attachment, logger
 
 # XXX(dcramer): this could be more tightly bound to our configured domain,
 # but slack limits what we can unfurl anyways so its probably safe
@@ -123,17 +125,16 @@ def on_link_shared(self, request, integration, token, data):
             "unfurls": json.dumps(results),
         }
 
-        session = http.build_session()
-        req = session.post("https://slack.com/api/chat.unfurl", data=payload)
-        status_code = req.status_code
-        response = req.json()
-        track_response_code(status_code, response.get("ok"))
-        req.raise_for_status()
-        if not response.get("ok"):
-            logger.error("slack.event.unfurl-error", extra={"response": response})
+        client = SlackClient()
+        try:
+            client.post("/chat.unfurl", data=payload)
+        except ApiError as e:
+            logger.error("slack.event.unfurl-error", extra={"error": six.text_type(e)})
+
         return self.respond()
 
     # TODO(dcramer): implement app_uninstalled and tokens_revoked
+    @transaction_start("SlackEventEndpoint")
     def post(self, request):
         try:
             slack_request = SlackEventRequest(request)

src/sentry/integrations/slack/integration.py

@@ -1,9 +1,10 @@
 from __future__ import absolute_import
 
+import six
+
 from django.utils.translation import ugettext_lazy as _
 from django.conf import settings
 
-from sentry import http
 from sentry.identity.pipeline import IdentityProviderPipeline
 from sentry.integrations import (
     IntegrationFeatures,
@@ -13,7 +14,10 @@
 )
 from sentry.pipeline import NestedPipelineView
 from sentry.utils.http import absolute_uri
-from .utils import track_response_code, use_slack_v2
+from sentry.shared_integrations.exceptions import ApiError, IntegrationError
+
+from .client import SlackClient
+from .utils import logger, use_slack_v2
 
 DESCRIPTION = """
 Connect your Sentry organization to one or more Slack workspaces, and start
@@ -120,15 +124,12 @@ def get_pipeline_views(self):
     def get_team_info(self, access_token):
         payload = {"token": access_token}
 
-        session = http.build_session()
-        resp = session.get("https://slack.com/api/team.info", params=payload)
-        resp.raise_for_status()
-        status_code = resp.status_code
-        resp = resp.json()
-        # TODO: track_response_code won't hit if we have an error status code
-        track_response_code(status_code, resp.get("ok"))
-
-        # TODO: check for resp["ok"]
+        client = SlackClient()
+        try:
+            resp = client.get("/team.info", params=payload)
+        except ApiError as e:
+            logger.error("slack.team-info.response-error", extra={"error": six.text_type(e)})
+            raise IntegrationError("Could not retrieve Slack team information.")
 
         return resp["team"]
 

src/sentry/integrations/slack/link_identity.py

@@ -1,19 +1,23 @@
 from __future__ import absolute_import, print_function
 
+import six
+
 from django.core.urlresolvers import reverse
 from django.db import IntegrityError
 from django.http import Http404
 from django.utils import timezone
 from django.views.decorators.cache import never_cache
 
-from sentry import http
 from sentry.models import Integration, Identity, IdentityProvider, IdentityStatus, Organization
 from sentry.utils.http import absolute_uri
 from sentry.utils.signing import sign, unsign
+from sentry.web.decorators import transaction_start
 from sentry.web.frontend.base import BaseView
 from sentry.web.helpers import render_to_response
+from sentry.shared_integrations.exceptions import ApiError
 
-from .utils import logger, track_response_code
+from .client import SlackClient
+from .utils import logger
 
 
 def build_linking_url(integration, organization, slack_id, channel_id, response_url):
@@ -31,6 +35,7 @@ def build_linking_url(integration, organization, slack_id, channel_id, response_
 
 
 class SlackLinkIdentityView(BaseView):
+    @transaction_start("SlackLinkIdentityView")
     @never_cache
     def handle(self, request, signed_params):
         params = unsign(signed_params.encode("ascii", errors="ignore"))
@@ -82,19 +87,18 @@ def handle(self, request, signed_params):
             "text": "Your Slack identity has been linked to your Sentry account. You're good to go!",
         }
 
-        session = http.build_session()
-        req = session.post(params["response_url"], json=payload)
-        status_code = req.status_code
-        resp = req.json()
-
-        # If the user took their time to link their slack account, we may no
-        # longer be able to respond, and we're not guaranteed able to post into
-        # the channel. Ignore Expired url errors.
-        #
-        # XXX(epurkhiser): Yes the error string has a space in it.
-        if not resp.get("ok") and resp.get("error") != "Expired url":
-            logger.error("slack.link-notify.response-error", extra={"response": resp})
-        track_response_code(status_code, resp.get("ok"))
+        client = SlackClient()
+        try:
+            client.post(params["response_url"], data=payload, json=True)
+        except ApiError as e:
+            message = six.text_type(e)
+            # If the user took their time to link their slack account, we may no
+            # longer be able to respond, and we're not guaranteed able to post into
+            # the channel. Ignore Expired url errors.
+            #
+            # XXX(epurkhiser): Yes the error string has a space in it.
+            if message != "Expired url":
+                logger.error("slack.link-notify.response-error", extra={"error": message})
 
         return render_to_response(
             "sentry/slack-linked.html",

src/sentry/integrations/slack/notify_action.py

@@ -1,14 +1,17 @@
 from __future__ import absolute_import
 
+import six
+
 from django import forms
 from django.utils.translation import ugettext_lazy as _
 
-from sentry import http
 from sentry.rules.actions.base import EventAction
 from sentry.utils import metrics, json
 from sentry.models import Integration
+from sentry.shared_integrations.exceptions import ApiError
 
-from .utils import build_group_attachment, get_channel_id, strip_channel_name, track_response_code
+from .client import SlackClient
+from .utils import build_group_attachment, get_channel_id, strip_channel_name
 
 
 class SlackNotifyServiceForm(forms.Form):
@@ -112,19 +115,17 @@ def send_notification(event, futures):
                 "attachments": json.dumps([attachment]),
             }
 
-            session = http.build_session()
-            resp = session.post("https://slack.com/api/chat.postMessage", data=payload, timeout=5)
-            status_code = resp.status_code
-            response = resp.json()
-            track_response_code(status_code, response.get("ok"))
-            resp.raise_for_status()
-            if not response.get("ok"):
+            client = SlackClient()
+            try:
+                client.post("/chat.postMessage", data=payload, timeout=5)
+            except ApiError as e:
                 self.logger.info(
                     "rule.fail.slack_post",
                     extra={
-                        "error": response.get("error"),
+                        "error": six.text_type(e),
                         "project_id": event.project_id,
                         "event_id": event.event_id,
+                        "channel_name": self.get_option("channel"),
                     },
                 )