Browse Source

fix(perf-issues): Fix subject substitution for Performance issues (#46914)

Closes #46872. Two changes here:

1. When someone puts in `$title`, for a Performance Issue it should use
the issue type name (e.g., "Slow DB Query") since each event doesn't
have its own title of any kind (except if there's an occurrence, but
that's another story)
2. Use the aliased _and_ unaliased form of the tag. I'm not sure what
the root cause is, but when users set `${tag:release}`, we look for the
`sentry:release` tag, instead of the `release` tag. For Performance
issues, this doesn't seem to work, that tag isn't there. I'm honestly
not too sure what's going on, but I set it so that it also tries the
unaliased tag before giving up
George Gritsouk 1 year ago
parent
commit
cd71a09f89

+ 1 - 0
fixtures/emails/performance.txt

@@ -25,6 +25,7 @@ Tags
 * level = info
 * runtime = CPython 3.8.9
 * runtime.name = CPython
+* sentry:release = 0.1
 * sentry:user = ip:127.0.0.1
 * transaction = /books/
 * url = http://127.0.0.1:9007/books/

+ 1 - 0
src/sentry/data/samples/transaction-n-plus-one.json

@@ -9,6 +9,7 @@
     ["device", "Mac"],
     ["device.family", "Mac"],
     ["environment", "production"],
+    ["sentry:release", "0.1"],
     ["http.status_code", "200"],
     ["level", "info"],
     ["runtime", "CPython 3.8.9"],

+ 10 - 5
src/sentry/eventstore/models.py

@@ -778,6 +778,9 @@ class EventSubjectTemplateData:
         if name.startswith("tag:"):
             name = name[4:]
             value = self.event.get_tag(self.tag_aliases.get(name, name))
+            if value is None:
+                value = self.event.get_tag(name)
+
             if value is None:
                 raise KeyError
             return str(value)
@@ -790,11 +793,13 @@ class EventSubjectTemplateData:
         elif name == "orgID":
             return cast(str, self.event.organization.slug)
         elif name == "title":
-            return (
-                self.event.occurrence.issue_title
-                if getattr(self.event, "occurrence", None)
-                else self.event.title
-            )
+            if getattr(self.event, "occurrence", None):
+                return self.event.occurrence.issue_title
+            elif self.event.group and self.event.group.issue_category == GroupCategory.PERFORMANCE:
+                return self.event.group.issue_type.description
+            else:
+                return self.event.title
+
         elif name == "issueType":
             return self.event.group.issue_type.description
         raise KeyError

+ 11 - 1
tests/sentry/eventstore/test_models.py

@@ -12,6 +12,7 @@ from sentry.issues.occurrence_consumer import process_event_and_issue_occurrence
 from sentry.models import Environment
 from sentry.snuba.dataset import Dataset
 from sentry.testutils import TestCase
+from sentry.testutils.cases import PerformanceIssueTestCase
 from sentry.testutils.helpers.datetime import before_now, iso_format
 from sentry.testutils.silo import region_silo_test
 from sentry.utils import snuba
@@ -19,7 +20,7 @@ from tests.sentry.issues.test_utils import OccurrenceTestMixin
 
 
 @region_silo_test
-class EventTest(TestCase):
+class EventTest(TestCase, PerformanceIssueTestCase):
     def test_pickling_compat(self):
         event = self.store_event(
             data={
@@ -105,6 +106,15 @@ class EventTest(TestCase):
 
         assert event1.get_email_subject() == "BAR-1 - production@0 $ baz ${tag:invalid} $invalid"
 
+    def test_transaction_email_subject(self):
+        self.project.update_option(
+            "mail:subject_template",
+            "$shortID - ${tag:environment}@${tag:release} $title",
+        )
+
+        event = self.create_performance_issue()
+        assert event.get_email_subject() == "BAR-1 - production@0.1 N+1 Query"
+
     def test_as_dict_hides_client_ip(self):
         event = self.store_event(
             data={"sdk": {"name": "foo", "version": "1.0", "client_ip": "127.0.0.1"}},