Browse Source

chore(hybrid-cloud): Refactor PagerDutyClient (#52569)

Alberto Leal 1 year ago
parent
commit
0046fe87fd

+ 14 - 2
src/sentry/integrations/pagerduty/actions/notification.py

@@ -4,9 +4,10 @@ import logging
 from typing import Sequence
 
 from sentry.integrations.pagerduty.actions import PagerDutyNotifyServiceForm
-from sentry.integrations.pagerduty.client import PagerDutyClient
+from sentry.integrations.pagerduty.client import PagerDutyProxyClient
 from sentry.models import PagerDutyService
 from sentry.rules.actions import IntegrationEventAction
+from sentry.shared_integrations.client.proxy import infer_org_integration
 from sentry.shared_integrations.exceptions import ApiError
 
 logger = logging.getLogger("sentry.integrations.pagerduty")
@@ -47,7 +48,18 @@ class PagerDutyNotifyServiceAction(IntegrationEventAction):
             return
 
         def send_notification(event, futures):
-            client = PagerDutyClient(integration_key=service.integration_key)
+            org_integration = self.get_organization_integration()
+            org_integration_id = None
+            if org_integration:
+                org_integration_id = org_integration.id
+            else:
+                org_integration_id = infer_org_integration(
+                    integration_id=service.integration_id, ctx_logger=logger
+                )
+            client = PagerDutyProxyClient(
+                org_integration_id=org_integration_id,
+                integration_key=service.integration_key,
+            )
             try:
                 resp = client.send_trigger(event)
             except ApiError as e:

+ 63 - 6
src/sentry/integrations/pagerduty/client.py

@@ -1,6 +1,12 @@
+from __future__ import annotations
+
+from typing import Any
+
 from sentry.api.serializers import ExternalEventSerializer, serialize
 from sentry.eventstore.models import Event, GroupEvent
 from sentry.integrations.client import ApiClient
+from sentry.shared_integrations.client.base import BaseApiResponseX
+from sentry.shared_integrations.client.proxy import IntegrationProxyClient
 
 LEVEL_SEVERITY_MAP = {
     "debug": "info",
@@ -11,6 +17,63 @@ LEVEL_SEVERITY_MAP = {
 }
 
 
+class PagerDutyProxyClient(IntegrationProxyClient):
+    allow_redirects = False
+    integration_name = "pagerduty"
+    base_url = "https://events.pagerduty.com/v2/enqueue"
+
+    def __init__(
+        self,
+        org_integration_id: int | None,
+        integration_key: str,
+    ) -> None:
+        self.integration_key = integration_key
+        super().__init__(org_integration_id=org_integration_id)
+
+    def request(self, method: str, *args: Any, **kwargs: Any) -> BaseApiResponseX:
+        headers = kwargs.pop("headers", None)
+        if headers is None:
+            headers = {"Content-Type": "application/json"}
+        return self._request(method, *args, headers=headers, **kwargs)
+
+    def send_trigger(self, data):
+        # expected payload: https://v2.developer.pagerduty.com/docs/send-an-event-events-api-v2
+        if isinstance(data, (Event, GroupEvent)):
+            source = data.transaction or data.culprit or "<unknown>"
+            group = data.group
+            level = data.get_tag("level") or "error"
+            custom_details = serialize(data, None, ExternalEventSerializer())
+            summary = custom_details["message"][:1024] or custom_details["title"]
+            payload = {
+                "routing_key": self.integration_key,
+                "event_action": "trigger",
+                "dedup_key": group.qualified_short_id,
+                "payload": {
+                    "summary": summary,
+                    "severity": LEVEL_SEVERITY_MAP[level],
+                    "source": source,
+                    "component": group.project.slug,
+                    "custom_details": custom_details,
+                },
+                "links": [
+                    {
+                        "href": group.get_absolute_url(
+                            params={"referrer": "pagerduty_integration"}
+                        ),
+                        "text": "View Sentry Issue Details",
+                    }
+                ],
+            }
+        else:
+            # the payload is for a metric alert
+            payload = data
+
+        return self.post("/", data=payload)
+
+
+# This exists since getsentry depends on the previous implementation of the API client.
+
+
 class PagerDutyClient(ApiClient):
     allow_redirects = False
     integration_name = "pagerduty"
@@ -59,9 +122,3 @@ class PagerDutyClient(ApiClient):
             payload = data
 
         return self.post("/", data=payload)
-
-    def send_acknowledge(self, data):
-        pass
-
-    def send_resolve(self, data):
-        pass

+ 5 - 2
src/sentry/integrations/pagerduty/integration.py

@@ -23,7 +23,7 @@ from sentry.shared_integrations.exceptions import IntegrationError
 from sentry.utils import json
 from sentry.utils.http import absolute_uri
 
-from .client import PagerDutyClient
+from .client import PagerDutyProxyClient
 
 logger = logging.getLogger("sentry.integrations.pagerduty")
 
@@ -66,7 +66,10 @@ metadata = IntegrationMetadata(
 
 class PagerDutyIntegration(IntegrationInstallation):
     def get_client(self, integration_key):
-        return PagerDutyClient(integration_key=integration_key)
+        return PagerDutyProxyClient(
+            org_integration_id=self.org_integration.id,
+            integration_key=integration_key,
+        )
 
     def get_organization_config(self):
         fields = [

+ 16 - 2
src/sentry/integrations/pagerduty/utils.py

@@ -5,9 +5,11 @@ from django.http import Http404
 from sentry.incidents.models import AlertRuleTriggerAction, Incident, IncidentStatus
 from sentry.integrations.metric_alerts import incident_attachment_info
 from sentry.models import PagerDutyService
+from sentry.services.hybrid_cloud.integration import integration_service
+from sentry.shared_integrations.client.proxy import infer_org_integration
 from sentry.shared_integrations.exceptions import ApiError
 
-from .client import PagerDutyClient
+from .client import PagerDutyProxyClient
 
 logger = logging.getLogger("sentry.integrations.pagerduty")
 
@@ -62,8 +64,20 @@ def send_incident_alert_notification(
             },
         )
         raise Http404
+    org_integration = integration_service.get_organization_integration(
+        integration_id=service.integration_id,
+        organization_id=service.organization_id,
+    )
+    if org_integration is None:
+        org_integration_id = infer_org_integration(
+            integration_id=service.integration_id, ctx_logger=logger
+        )
+    else:
+        org_integration_id = org_integration.id
     integration_key = service.integration_key
-    client = PagerDutyClient(integration_key=integration_key)
+    client = PagerDutyProxyClient(
+        org_integration_id=org_integration_id, integration_key=integration_key
+    )
     attachment = build_incident_attachment(incident, integration_key, new_status, metric_value)
     try:
         client.send_trigger(attachment)

+ 10 - 1
src/sentry/rules/actions/integrations/base.py

@@ -7,7 +7,11 @@ from django import forms
 
 from sentry.models import OrganizationStatus
 from sentry.rules.actions import EventAction
-from sentry.services.hybrid_cloud.integration import RpcIntegration, integration_service
+from sentry.services.hybrid_cloud.integration import (
+    RpcIntegration,
+    RpcOrganizationIntegration,
+    integration_service,
+)
 
 INTEGRATION_KEY = "integration"
 
@@ -72,5 +76,10 @@ class IntegrationEventAction(EventAction, abc.ABC):
                 return integration
         return None
 
+    def get_organization_integration(self) -> RpcOrganizationIntegration | None:
+        return integration_service.get_organization_integration(
+            integration_id=self.get_integration_id(), organization_id=self.project.organization_id
+        )
+
     def get_form_instance(self) -> forms.Form:
         return self.form_cls(self.data, integrations=self.get_integrations())

+ 1 - 1
src/sentry_plugins/pagerduty/client.py

@@ -5,7 +5,7 @@ from sentry_plugins.client import ApiClient
 INTEGRATION_API_URL = "https://events.pagerduty.com/generic/2010-04-15/create_event.json"
 
 
-class PagerDutyClient(ApiClient):
+class PagerDutyProxyClient(ApiClient):
     client = "sentry"
     plugin_name = "pagerduty"
     allow_redirects = False

+ 2 - 2
src/sentry_plugins/pagerduty/plugin.py

@@ -4,7 +4,7 @@ from sentry.utils.http import absolute_uri
 from sentry_plugins.base import CorePluginMixin
 from sentry_plugins.utils import get_secret_field_config
 
-from .client import PagerDutyClient
+from .client import PagerDutyProxyClient
 
 
 class PagerDutyPlugin(CorePluginMixin, NotifyPlugin):
@@ -100,7 +100,7 @@ class PagerDutyPlugin(CorePluginMixin, NotifyPlugin):
                 service_key = route_service_key
                 break
 
-        client = PagerDutyClient(service_key=service_key)
+        client = PagerDutyProxyClient(service_key=service_key)
         try:
             response = client.trigger_incident(
                 description=description,

+ 171 - 12
tests/sentry/integrations/pagerduty/test_client.py

@@ -1,8 +1,17 @@
 import copy
-from unittest.mock import patch
+from unittest import mock
+from unittest.mock import call
+
+import pytest
+import responses
+from django.test import override_settings
+from responses import matchers
 
 from sentry.api.serializers import ExternalEventSerializer, serialize
+from sentry.integrations.pagerduty.client import PagerDutyProxyClient
 from sentry.models import Integration, PagerDutyService
+from sentry.silo.base import SiloMode
+from sentry.silo.util import PROXY_BASE_PATH, PROXY_OI_HEADER, PROXY_SIGNATURE_HEADER
 from sentry.testutils import APITestCase
 from sentry.testutils.factories import DEFAULT_EVENT_DATA
 from sentry.testutils.helpers.datetime import before_now, iso_format
@@ -19,9 +28,14 @@ SERVICES = [
 ]
 
 
-class PagerDutyClientTest(APITestCase):
+class PagerDutyProxyClientTest(APITestCase):
     provider = "pagerduty"
 
+    @pytest.fixture(autouse=True)
+    def _setup_metric_patch(self):
+        with mock.patch("sentry.shared_integrations.track_response.metrics") as self.metrics:
+            yield
+
     def setUp(self):
         self.login_as(self.user)
         self.integration = Integration.objects.create(
@@ -39,8 +53,10 @@ class PagerDutyClientTest(APITestCase):
         self.installation = self.integration.get_installation(self.organization.id)
         self.min_ago = iso_format(before_now(minutes=1))
 
-    @patch("sentry.integrations.pagerduty.client.PagerDutyClient.request")
-    def test_send_trigger(self, mock_request):
+    @responses.activate
+    def test_send_trigger(self):
+        integration_key = self.service.integration_key
+
         event = self.store_event(
             data={
                 "event_id": "a" * 32,
@@ -50,14 +66,9 @@ class PagerDutyClientTest(APITestCase):
             },
             project_id=self.project.id,
         )
-        group = event.group
-
-        integration_key = self.service.integration_key
-        client = self.installation.get_client(integration_key=integration_key)
         custom_details = serialize(event, None, ExternalEventSerializer())
-
-        client.send_trigger(event)
-        data = {
+        group = event.group
+        expected_data = {
             "routing_key": integration_key,
             "event_action": "trigger",
             "dedup_key": group.qualified_short_id,
@@ -75,4 +86,152 @@ class PagerDutyClientTest(APITestCase):
                 }
             ],
         }
-        mock_request.assert_called_once_with("POST", "/", data=data)
+
+        responses.add(
+            responses.POST,
+            "https://events.pagerduty.com/v2/enqueue/",
+            body=b"{}",
+            match=[
+                matchers.header_matcher(
+                    {
+                        "Content-Type": "application/json",
+                    }
+                ),
+                matchers.json_params_matcher(expected_data),
+            ],
+        )
+
+        client = self.installation.get_client(integration_key=integration_key)
+        client.send_trigger(event)
+
+        assert len(responses.calls) == 1
+        request = responses.calls[0].request
+        assert "https://events.pagerduty.com/v2/enqueue/" == request.url
+        assert client.base_url and (client.base_url.lower() in request.url)
+
+        # Check if metrics is generated properly
+        calls = [
+            call(
+                "integrations.http_response",
+                sample_rate=1.0,
+                tags={"integration": "pagerduty", "status": 200},
+            )
+        ]
+        assert self.metrics.incr.mock_calls == calls
+
+
+def assert_proxy_request(request, is_proxy=True):
+    assert (PROXY_BASE_PATH in request.url) == is_proxy
+    assert (PROXY_OI_HEADER in request.headers) == is_proxy
+    assert (PROXY_SIGNATURE_HEADER in request.headers) == is_proxy
+    # PagerDuty API does not require the Authorization header.
+    # The secret is instead passed in the body payload called routing_key/integration key
+    assert "Authorization" not in request.headers
+    if is_proxy:
+        assert request.headers[PROXY_OI_HEADER] is not None
+
+
+@override_settings(
+    SENTRY_SUBNET_SECRET="hush-hush-im-invisible",
+    SENTRY_CONTROL_ADDRESS="http://controlserver",
+)
+class PagerDutyProxyApiClientTest(APITestCase):
+    def setUp(self):
+        self.login_as(self.user)
+        self.integration = Integration.objects.create(
+            provider="pagerduty",
+            name="Example PagerDuty",
+            external_id=EXTERNAL_ID,
+            metadata={"services": SERVICES},
+        )
+        self.integration.add_organization(self.organization, self.user)
+        self.service = PagerDutyService.objects.create(
+            service_name=SERVICES[0]["service_name"],
+            integration_key=SERVICES[0]["integration_key"],
+            organization_integration_id=self.integration.organizationintegration_set.first().id,
+        )
+        self.installation = self.integration.get_installation(self.organization.id)
+        self.min_ago = iso_format(before_now(minutes=1))
+
+    @responses.activate
+    def test_integration_proxy_is_active(self):
+        responses.add(
+            responses.POST,
+            "https://events.pagerduty.com/v2/enqueue/",
+            body=b"{}",
+            match=[
+                matchers.header_matcher(
+                    {
+                        "Content-Type": "application/json",
+                    }
+                ),
+            ],
+        )
+
+        responses.add(
+            responses.POST,
+            "http://controlserver/api/0/internal/integration-proxy/",
+            body=b"{}",
+            match=[
+                matchers.header_matcher(
+                    {
+                        "Content-Type": "application/json",
+                    }
+                ),
+            ],
+        )
+
+        class PagerDutyProxyApiTestClient(PagerDutyProxyClient):
+            _use_proxy_url_for_tests = True
+
+        event = self.store_event(
+            data={
+                "event_id": "a" * 32,
+                "message": "message",
+                "timestamp": self.min_ago,
+                "stacktrace": copy.deepcopy(DEFAULT_EVENT_DATA["stacktrace"]),
+            },
+            project_id=self.project.id,
+        )
+
+        responses.calls.reset()
+        with override_settings(SILO_MODE=SiloMode.MONOLITH):
+            client = PagerDutyProxyApiTestClient(
+                org_integration_id=self.installation.org_integration.id,
+                integration_key=self.service.integration_key,
+            )
+            client.send_trigger(event)
+
+            assert len(responses.calls) == 1
+            request = responses.calls[0].request
+            assert "https://events.pagerduty.com/v2/enqueue/" == request.url
+            assert client.base_url and (client.base_url.lower() in request.url)
+            assert_proxy_request(request, is_proxy=False)
+
+        responses.calls.reset()
+        with override_settings(SILO_MODE=SiloMode.CONTROL):
+            client = PagerDutyProxyApiTestClient(
+                org_integration_id=self.installation.org_integration.id,
+                integration_key=self.service.integration_key,
+            )
+            client.send_trigger(event)
+
+            assert len(responses.calls) == 1
+            request = responses.calls[0].request
+            assert "https://events.pagerduty.com/v2/enqueue/" == request.url
+            assert client.base_url and (client.base_url.lower() in request.url)
+            assert_proxy_request(request, is_proxy=False)
+
+        responses.calls.reset()
+        with override_settings(SILO_MODE=SiloMode.REGION):
+            client = PagerDutyProxyApiTestClient(
+                org_integration_id=self.installation.org_integration.id,
+                integration_key=self.service.integration_key,
+            )
+            client.send_trigger(event)
+
+            assert len(responses.calls) == 1
+            request = responses.calls[0].request
+            assert "http://controlserver/api/0/internal/integration-proxy/" == request.url
+            assert client.base_url and (client.base_url.lower() not in request.url)
+            assert_proxy_request(request, is_proxy=True)