Browse Source

fix(issue_alerts): Fix bug in event_attribute where we use the wrong attribute for message search (#28967)

We changed the definition of event.message a while ago. It used to include the same message that we
use in `Group.message` and the message field in the snuba errors table. This was changed in
https://github.com/getsentry/sentry/pull/16557 so that `event.message` only looks at the
`logentry.formatted` and `logentry.message` properties of the event. These are frequently not
present at all, and so we end up having an empty `message` value to compare to.

`event.search_message` has the definition that we want, and is the same as the value we currently
use in `Group.message` now. This means that issue alerts on the message property have been only
partially working since this change was made.

To make sure we don't break any `message = "<val>"` rules we'll use both `message` and `search_message`
Dan Fuller 3 years ago
parent
commit
e7ee147034

+ 1 - 1
src/sentry/rules/conditions/event_attribute.py

@@ -102,7 +102,7 @@ class EventAttributeCondition(EventCondition):
         if path[0] == "message":
             if len(path) != 1:
                 return []
-            return [event.message]
+            return [event.message, event.search_message]
         elif path[0] == "environment":
             return [event.get_tag("environment")]
 

+ 26 - 0
tests/sentry/rules/conditions/test_event_attribute.py

@@ -129,6 +129,32 @@ class EventAttributeConditionTest(RuleTestCase):
         )
         self.assertDoesNotPass(rule, event)
 
+    def test_contains_message(self):
+        event = self.get_event()
+        rule = self.get_rule(
+            data={"match": MatchType.CONTAINS, "attribute": "message", "value": "hello"}
+        )
+        self.assertPasses(rule, event)
+
+        # Validate that this searches message in the same way that snuba does
+        event = self.get_event(message="")
+        # This should still pass, even though the message is now empty
+        rule = self.get_rule(
+            data={"match": MatchType.CONTAINS, "attribute": "message", "value": "hello"}
+        )
+        self.assertPasses(rule, event)
+
+        # The search should also include info from the exception if present
+        rule = self.get_rule(
+            data={"match": MatchType.CONTAINS, "attribute": "message", "value": "SyntaxError"}
+        )
+        self.assertPasses(rule, event)
+
+        rule = self.get_rule(
+            data={"match": MatchType.CONTAINS, "attribute": "message", "value": "not present"}
+        )
+        self.assertDoesNotPass(rule, event)
+
     def test_does_not_contain(self):
         event = self.get_event()
         rule = self.get_rule(