ref(integrations): Refactor the Splunk Plugin to use the DataForwardingPlugin class (#22765)

src/sentry/plugins/bases/data_forwarding.py

@@ -36,8 +36,15 @@ class DataForwardingPlugin(Plugin):
     def get_plugin_type(self):
         return "data-forwarding"
 
-    def post_process(self, event, **kwargs):
-        rl_key = u"{}:{}".format(self.conf_key, event.project.organization_id)
+    def get_rl_key(self, event):
+        return u"{}:{}".format(self.conf_key, event.project.organization_id)
+
+    def initialize_variables(self, event):
+        return
+
+    def is_ratelimited(self, event):
+        self.initialize_variables(event)
+        rl_key = self.get_rl_key(event)
         # limit segment to 50 requests/second
         limit, window = self.get_rate_limit()
         if limit and window and ratelimits.is_limited(rl_key, limit=limit, window=window):
@@ -50,6 +57,11 @@ class DataForwardingPlugin(Plugin):
                     "organization_id": event.project.organization_id,
                 },
             )
+            return True
+        return False
+
+    def post_process(self, event, **kwargs):
+        if self.is_ratelimited(event):
             return
 
         payload = self.get_event_payload(event)

src/sentry_plugins/splunk/plugin.py

@@ -20,14 +20,12 @@ import logging
 import six
 from requests.exceptions import ReadTimeout
 
-from sentry import http, tagstore, tsdb
-from sentry.app import ratelimiter
-from sentry.plugins.base import Plugin
-from sentry.plugins.base.configuration import react_plugin_config
+from sentry import http, tagstore
 from sentry.utils import metrics
 from sentry.utils.hashlib import md5_text
 
 from sentry_plugins.base import CorePluginMixin
+from sentry.plugins.bases.data_forwarding import DataForwardingPlugin
 from sentry_plugins.utils import get_secret_field_config
 from sentry_plugins.anonymizeip import anonymize_ip
 from sentry.integrations import FeatureDescription, IntegrationFeatures
@@ -96,13 +94,18 @@ class SplunkConfigError(SplunkError):
     KNOWN_CODES = frozenset([7, 10, 11])
 
 
-class SplunkPlugin(CorePluginMixin, Plugin):
+class SplunkPlugin(CorePluginMixin, DataForwardingPlugin):
     title = "Splunk"
     slug = "splunk"
     description = DESCRIPTION
     conf_key = "splunk"
     resource_links = [("Splunk Setup Instructions", SETUP_URL)] + CorePluginMixin.resource_links
     required_field = "instance"
+    project_token = None
+    project_index = None
+    project_source = None
+    project_instance = None
+    host = None
     feature_descriptions = [
         FeatureDescription(
             """
@@ -112,14 +115,9 @@ class SplunkPlugin(CorePluginMixin, Plugin):
         )
     ]
 
-    def configure(self, project, request):
-        return react_plugin_config(self, project, request)
-
-    def has_project_conf(self):
-        return True
-
-    def get_plugin_type(self):
-        return "data-forwarding"
+    def get_rate_limit(self):
+        # number of requests, number of seconds (window)
+        return (1000, 1)
 
     def get_config(self, project, **kwargs):
         return [
@@ -163,7 +161,7 @@ class SplunkPlugin(CorePluginMixin, Plugin):
 
         return None
 
-    def get_event_payload(self, event):
+    def get_event_payload_properties(self, event):
         props = {
             "event_id": event.event_id,
             "issue_id": event.group_id,
@@ -211,63 +209,80 @@ class SplunkPlugin(CorePluginMixin, Plugin):
                     props.update(user_payload)
         return props
 
-    # http://dev.splunk.com/view/event-collector/SP-CAAAE6M
-    def post_process(self, event, **kwargs):
-        token = self.get_option("token", event.project)
-        index = self.get_option("index", event.project)
-        instance = self.get_option("instance", event.project)
-        if not (token and index and instance):
+    def initialize_variables(self, event):
+        self.project_token = self.get_option("token", event.project)
+        self.project_index = self.get_option("index", event.project)
+        self.project_instance = self.get_option("instance", event.project)
+        self.host = self.get_host_for_splunk(event)
+
+        if self.project_instance and not self.project_instance.endswith("/services/collector"):
+            self.project_instance = self.project_instance.rstrip("/") + "/services/collector"
+
+        self.project_source = self.get_option("source", event.project) or "sentry"
+
+    def get_rl_key(self, event):
+        return u"{}:{}".format(self.conf_key, md5_text(self.project_token).hexdigest())
+
+    def is_ratelimited(self, event):
+        if super(SplunkPlugin, self).is_ratelimited(event):
             metrics.incr(
-                "integrations.splunk.forward-event.unconfigured",
+                "integrations.splunk.forward-event.rate-limited",
                 tags={
                     "project_id": event.project_id,
                     "organization_id": event.project.organization_id,
                     "event_type": event.get_event_type(),
                 },
             )
-            return
-
-        if not instance.endswith("/services/collector"):
-            instance = instance.rstrip("/") + "/services/collector"
+            return True
+        return False
 
-        source = self.get_option("source", event.project) or "sentry"
+    def get_event_payload(self, event):
+        payload = {
+            "time": int(event.datetime.strftime("%s")),
+            "source": self.project_source,
+            "index": self.project_index,
+            "event": self.get_event_payload_properties(event),
+        }
+        return payload
 
-        rl_key = u"splunk:{}".format(md5_text(token).hexdigest())
-        # limit splunk to 50 requests/second
-        if ratelimiter.is_limited(rl_key, limit=1000, window=1):
+    def forward_event(self, event, payload):
+        if not (self.project_token and self.project_index and self.project_instance):
             metrics.incr(
-                "integrations.splunk.forward-event.rate-limited",
+                "integrations.splunk.forward-event.unconfigured",
                 tags={
                     "project_id": event.project_id,
                     "organization_id": event.project.organization_id,
                     "event_type": event.get_event_type(),
                 },
             )
-            return
+            return False
 
-        payload = {
-            "time": int(event.datetime.strftime("%s")),
-            "source": source,
-            "index": index,
-            "event": self.get_event_payload(event),
-        }
-        host = self.get_host_for_splunk(event)
-        if host:
-            payload["host"] = host
+        if self.host:
+            payload["host"] = self.host
 
         session = http.build_session()
         try:
             # https://docs.splunk.com/Documentation/Splunk/7.2.3/Data/TroubleshootHTTPEventCollector
             resp = session.post(
-                instance,
+                self.project_instance,
                 json=payload,
                 # Splunk cloud instances certifcates dont play nicely
                 verify=False,
-                headers={"Authorization": u"Splunk {}".format(token)},
+                headers={"Authorization": u"Splunk {}".format(self.project_token)},
                 timeout=5,
             )
             if resp.status_code != 200:
                 raise SplunkError.from_response(resp)
+
+            metrics.incr(
+                "integrations.splunk.forward-event.success",
+                tags={
+                    "project_id": event.project_id,
+                    "organization_id": event.project.organization_id,
+                    "event_type": event.get_event_type(),
+                },
+            )
+            return True
         except Exception as exc:
             metric = "integrations.splunk.forward-event.error"
             metrics.incr(
@@ -282,7 +297,7 @@ class SplunkPlugin(CorePluginMixin, Plugin):
             logger.info(
                 metric,
                 extra={
-                    "instance": instance,
+                    "instance": self.project_nstance,
                     "project_id": event.project_id,
                     "organization_id": event.project.organization_id,
                     "error": six.text_type(exc),
@@ -292,20 +307,10 @@ class SplunkPlugin(CorePluginMixin, Plugin):
             if isinstance(exc, ReadTimeout):
                 # If we get a ReadTimeout we don't need to raise an error here.
                 # Just log and return.
-                return
+                return False
 
             if isinstance(exc, SplunkError) and exc.status_code == 403:
                 # 403s are not errors or actionable for us do not re-raise
-                return
+                return False
 
             raise
-
-        metrics.incr(
-            "integrations.splunk.forward-event.success",
-            tags={
-                "project_id": event.project_id,
-                "organization_id": event.project.organization_id,
-                "event_type": event.get_event_type(),
-            },
-        )
-        tsdb.incr(tsdb.models.project_total_forwarded, event.project.id, count=1)

tests/sentry_plugins/splunk/test_plugin.py

@@ -36,12 +36,7 @@ class SplunkPluginTest(PluginTestCase):
 
         request = responses.calls[0].request
         payload = json.loads(request.body)
-        assert payload == {
-            "index": "main",
-            "source": "sentry",
-            "time": int(event.datetime.strftime("%s")),
-            "event": self.plugin.get_event_payload(event),
-        }
+        assert payload == self.plugin.get_event_payload(event)
         headers = request.headers
         assert headers["Authorization"] == "Splunk 12345678-1234-1234-1234-1234567890AB"
 
@@ -58,9 +53,9 @@ class SplunkPluginTest(PluginTestCase):
         )
 
         result = self.plugin.get_event_payload(event)
-        assert result["request_url"] == "http://example.com/"
-        assert result["request_method"] == "POST"
-        assert result["request_referer"] == "http://example.com/foo"
+        assert result["event"]["request_url"] == "http://example.com/"
+        assert result["event"]["request_method"] == "POST"
+        assert result["event"]["request_referer"] == "http://example.com/foo"
 
     def test_error_payload(self):
         event = self.store_event(
@@ -72,9 +67,9 @@ class SplunkPluginTest(PluginTestCase):
         )
 
         result = self.plugin.get_event_payload(event)
-        assert result["type"] == "error"
-        assert result["exception_type"] == "ValueError"
-        assert result["exception_value"] == "foo bar"
+        assert result["event"]["type"] == "error"
+        assert result["event"]["exception_type"] == "ValueError"
+        assert result["event"]["exception_value"] == "foo bar"
 
     def test_csp_payload(self):
         event = self.store_event(
@@ -91,11 +86,11 @@ class SplunkPluginTest(PluginTestCase):
         )
 
         result = self.plugin.get_event_payload(event)
-        assert result["type"] == "csp"
-        assert result["csp_document_uri"] == "http://example.com/"
-        assert result["csp_violated_directive"] == "style-src cdn.example.com"
-        assert result["csp_blocked_uri"] == "http://example.com/style.css"
-        assert result["csp_effective_directive"] == "style-src"
+        assert result["event"]["type"] == "csp"
+        assert result["event"]["csp_document_uri"] == "http://example.com/"
+        assert result["event"]["csp_violated_directive"] == "style-src cdn.example.com"
+        assert result["event"]["csp_blocked_uri"] == "http://example.com/style.css"
+        assert result["event"]["csp_effective_directive"] == "style-src"
 
     def test_user_payload(self):
         event = self.store_event(
@@ -104,6 +99,6 @@ class SplunkPluginTest(PluginTestCase):
         )
 
         result = self.plugin.get_event_payload(event)
-        assert result["user_id"] == "1"
-        assert result["user_email_hash"] == "b48def645758b95537d4424c84d1a9ff"
-        assert result["user_ip_trunc"] == "127.0.0.0"
+        assert result["event"]["user_id"] == "1"
+        assert result["event"]["user_email_hash"] == "b48def645758b95537d4424c84d1a9ff"
+        assert result["event"]["user_ip_trunc"] == "127.0.0.0"