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/client.py

@@ -0,0 +1,57 @@
+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 __init__(self):
+        super(SlackClient, self).__init__()
+
+    def track_response_data(self, code, span, error=None, resp=None):
+        span.set_http_status(code)
+        span.set_tag("integration", "slack")
+
+        # 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:
+                span.set_tag("slack_error", response.get("error"))
+
+        metrics.incr(
+            SLACK_DATADOG_METRIC,
+            sample_rate=1.0,
+            tags={"ok": False if is_ok is False else True, "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, 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=False
+        )
+        if not response.json.get("ok"):
+            raise ApiError(response.get("error", ""))
+        return response

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):
@@ -114,17 +117,14 @@ 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,
                     },

src/sentry/integrations/slack/utils.py

@@ -2,18 +2,18 @@
 
 import logging
 import time
+import six
 from datetime import timedelta
 
 from django.core.cache import cache
 from django.core.urlresolvers import reverse
 
-from sentry import http
 from sentry import tagstore
 from sentry.api.fields.actor import Actor
 from sentry.incidents.logic import get_incident_aggregates
 from sentry.incidents.models import IncidentStatus, IncidentTrigger
 from sentry.snuba.models import QueryAggregations
-from sentry.utils import metrics, json
+from sentry.utils import json
 from sentry.utils.assets import get_asset_url
 from sentry.utils.dates import to_timestamp
 from sentry.utils.http import absolute_uri
@@ -29,6 +29,9 @@
     ReleaseProject,
 )
 
+from sentry.shared_integrations.exceptions import ApiError
+from .client import SlackClient
+
 logger = logging.getLogger("sentry.integrations.slack")
 
 # Attachment colors used for issues with no actions take
@@ -46,15 +49,6 @@
 strip_channel_chars = "".join([MEMBER_PREFIX, CHANNEL_PREFIX])
 SLACK_DEFAULT_TIMEOUT = 10
 QUERY_AGGREGATION_DISPLAY = ["events", "users affected"]
-SLACK_DATADOG_METRIC = "integrations.slack.http_response"
-
-
-def track_response_code(status_code, is_ok):
-    metrics.incr(
-        SLACK_DATADOG_METRIC,
-        sample_rate=1.0,
-        tags={"ok": False if is_ok is False else True, "status": status_code},
-    )
 
 
 def format_actor_option(actor):
@@ -409,21 +403,19 @@ def get_channel_id_with_timeout(integration, name, timeout):
     payload = dict(token_payload, **{"exclude_archived": False, "exclude_members": True})
 
     time_to_quit = time.time() + timeout
-    session = http.build_session()
+
+    client = SlackClient()
+    # session = http.build_session()
     for list_type, result_name, prefix in LIST_TYPES:
         cursor = ""
         while True:
-            items = session.get(
-                "https://slack.com/api/%s.list" % list_type,
+            endpoint = "/%s.list" % list_type
+            try:
                 # Slack limits the response of `<list_type>.list` to 1000 channels
-                params=dict(payload, cursor=cursor, limit=1000),
-            )
-            status_code = items.status_code
-            items = items.json()
-            track_response_code(status_code, items.get("ok"))
-            if not items.get("ok"):
+                items = client.get(endpoint, params=dict(payload, cursor=cursor, limit=1000))
+            except ApiError as e:
                 logger.info(
-                    "rule.slack.%s_list_failed" % list_type, extra={"error": items.get("error")}
+                    "rule.slack.%s_list_failed" % list_type, extra={"error": six.text_type(e)}
                 )
                 return (prefix, None, False)
 
@@ -451,11 +443,11 @@ def send_incident_alert_notification(action, incident):
         "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"):
-        logger.info("rule.fail.slack_post", extra={"error": response.get("error")})
+    client = SlackClient()
+
+    try:
+        client.post("/chat.postMessage", data=payload, timeout=5)
+    except ApiError as e:
+        logger.info(
+            "rule.fail.slack_post", extra={"error": six.text_type(e)},
+        )

src/sentry/shared_integrations/client.py

@@ -148,7 +148,7 @@ def name(cls):
     def get_cache_prefix(self):
         return u"%s.%s.client:" % (self.integration_type, self.name)
 
-    def track_response_data(self, code, span, error=None):
+    def track_response_data(self, code, span, error=None, resp=None):
         metrics.incr(
             u"%s.http_response" % (self.datadog_prefix),
             sample_rate=1.0,
@@ -242,7 +242,7 @@ def _request(
                 self.track_response_data(resp.status_code, span, e)
                 raise ApiError.from_response(resp)
 
-            self.track_response_data(resp.status_code, span)
+            self.track_response_data(resp.status_code, span, None, resp)
 
             if resp.status_code == 204:
                 return {}