Browse Source

fix: Allow downgrading from hierarchical grouping (#45570)

Hierarchical grouping (or rather groups) use a different form of
grouping hashes. We will now look for those hierarchical hashes as well
when applying the secondary grouping config.
This means that new events will still group into existing hierarchical
groups. However, once the secondary config expires, this will currently
not work, as the hierarchical group is not yet being extended with
non-hierarchical hashes.
Arpad Borsos 2 years ago
parent
commit
9d6d76857e
2 changed files with 72 additions and 2 deletions
  1. 7 2
      src/sentry/event_manager.py
  2. 65 0
      tests/sentry/event_manager/test_event_manager.py

+ 7 - 2
src/sentry/event_manager.py

@@ -518,8 +518,13 @@ class EventManager:
 
         hashes = CalculatedHashes(
             hashes=list(hashes.hashes) + list(secondary_hashes and secondary_hashes.hashes or []),
-            hierarchical_hashes=hashes.hierarchical_hashes,
-            tree_labels=hashes.tree_labels,
+            hierarchical_hashes=(
+                list(hashes.hierarchical_hashes)
+                + list(secondary_hashes and secondary_hashes.hierarchical_hashes or [])
+            ),
+            tree_labels=(
+                hashes.tree_labels or (secondary_hashes and secondary_hashes.tree_labels) or []
+            ),
         )
 
         if not do_background_grouping_before:

+ 65 - 0
tests/sentry/event_manager/test_event_manager.py

@@ -293,6 +293,71 @@ class EventManagerTest(TestCase, SnubaTestCase, EventManagerTestMixin):
         event3 = save_event(4)
         assert event3.group_id == event2.group_id
 
+    def test_applies_downgrade_hierarchical(self):
+        project = self.project
+        project.update_option("sentry:grouping_config", "mobile:2021-02-12")
+        project.update_option("sentry:secondary_grouping_expiry", 0)
+
+        timestamp = time() - 300
+
+        def save_event(ts_offset):
+            ts = timestamp + ts_offset
+            manager = EventManager(
+                make_event(
+                    message="foo 123",
+                    event_id=hex(2**127 + int(ts))[-32:],
+                    timestamp=ts,
+                    exception={
+                        "values": [
+                            {
+                                "type": "Hello",
+                                "stacktrace": {
+                                    "frames": [
+                                        {
+                                            "function": "not_in_app_function",
+                                        },
+                                        {
+                                            "function": "in_app_function",
+                                        },
+                                    ]
+                                },
+                            }
+                        ]
+                    },
+                )
+            )
+            manager.normalize()
+            with self.tasks():
+                return manager.save(project.id)
+
+        event = save_event(0)
+
+        project.update_option("sentry:grouping_config", "legacy:2019-03-12")
+        project.update_option("sentry:secondary_grouping_config", "mobile:2021-02-12")
+        project.update_option("sentry:secondary_grouping_expiry", time() + (24 * 90 * 3600))
+
+        # Switching to newstyle grouping changes hashes as 123 will be removed
+        event2 = save_event(2)
+
+        # make sure that events did get into same group because of fallback grouping, not because of hashes which come from primary grouping only
+        assert not set(event.get_hashes().hashes) & set(event2.get_hashes().hashes)
+        assert event.group_id == event2.group_id
+
+        group = Group.objects.get(id=event.group_id)
+
+        assert group.times_seen == 2
+        assert group.last_seen == event2.datetime
+
+        # FIXME: *Full* downgrade does not yet work. We currently still need the
+        # hierarchical hash to find the proper group. We do not yet update that
+        # "hierarchical group" with new non-hierarchical hashes, and would not
+        # be able to find it anymore at all if we did not have hierarchical hashes.
+
+        # After expiry, new events are still assigned to the same group:
+        # project.update_option("sentry:secondary_grouping_expiry", 0)
+        # event3 = save_event(4)
+        # assert event3.group_id == event2.group_id
+
     @mock.patch("sentry.event_manager._calculate_background_grouping")
     def test_applies_background_grouping(self, mock_calc_grouping):
         timestamp = time() - 300