From 2c779d7c73cfc68261b43da79d638d93e71562c6 Mon Sep 17 00:00:00 2001
From: Ryan Albrecht <ryan.albrecht@sentry.io>
Date: Tue, 16 Apr 2024 15:01:38 -0700
Subject: [PATCH 1/2] fix(replay): Remove duplicate nav events from the
 Replay>Breadcrumbs list

---
 static/app/utils/replays/replayReader.tsx | 60 +++++++++++++++++------
 1 file changed, 46 insertions(+), 14 deletions(-)

diff --git a/static/app/utils/replays/replayReader.tsx b/static/app/utils/replays/replayReader.tsx
index 6b5a62bc3b492a..8232828ba70600 100644
--- a/static/app/utils/replays/replayReader.tsx
+++ b/static/app/utils/replays/replayReader.tsx
@@ -103,6 +103,36 @@ function removeDuplicateClicks(frames: BreadcrumbFrame[]) {
   return uniqueClickFrames.concat(otherFrames).concat(slowClickFrames);
 }
 
+// If a `navigation` crumb and `navigation.*` span happen within this timeframe,
+// we'll consider them duplicates.
+const DUPLICATE_NAV_THRESHOLD_MS = 2;
+
+/**
+ * Return a list of BreadcrumbFrames, where any navigation crumb is removed if
+ * there is a matching navigation.* span to replace it.
+ *
+ * SpanFrame is preferred because they render with more specific titles.
+ */
+function removeDuplicateNavCrumbs(
+  crumbFrames: BreadcrumbFrame[],
+  spanFrames: SpanFrame[]
+) {
+  const navCrumbs = crumbFrames.filter(crumb => crumb.category === 'navigation');
+  const otherBreadcrumbFrames = crumbFrames.filter(
+    crumb => crumb.category !== 'navigation'
+  );
+
+  const navSpans = spanFrames.filter(span => span.op.startsWith('navigation.'));
+
+  const uniqueNavCrumbs = navCrumbs.filter(
+    crumb =>
+      !navSpans.some(
+        span => Math.abs(crumb.offsetMs - span.offsetMs) <= DUPLICATE_NAV_THRESHOLD_MS
+      )
+  );
+  return otherBreadcrumbFrames.concat(uniqueNavCrumbs);
+}
+
 export default class ReplayReader {
   static factory({attachments, errors, replayRecord, clipWindow}: ReplayReaderParams) {
     if (!attachments || !replayRecord || !errors) {
@@ -446,20 +476,22 @@ export default class ReplayReader {
     )
   );
 
-  getPerfFrames = memoize(() =>
-    [
-      ...removeDuplicateClicks(
-        this._sortedBreadcrumbFrames.filter(
-          frame =>
-            ['navigation', 'ui.click'].includes(frame.category) ||
-            (frame.category === 'ui.slowClickDetected' &&
-              (isDeadClick(frame as SlowClickFrame) ||
-                isDeadRageClick(frame as SlowClickFrame)))
-        )
-      ),
-      ...this._sortedSpanFrames.filter(frame => frame.op.startsWith('navigation.')),
-    ].sort(sortFrames)
-  );
+  getPerfFrames = memoize(() => {
+    const crumbs = removeDuplicateClicks(
+      this._sortedBreadcrumbFrames.filter(
+        frame =>
+          ['navigation', 'ui.click'].includes(frame.category) ||
+          (frame.category === 'ui.slowClickDetected' &&
+            (isDeadClick(frame as SlowClickFrame) ||
+              isDeadRageClick(frame as SlowClickFrame)))
+      )
+    );
+    const spans = this._sortedSpanFrames.filter(frame =>
+      frame.op.startsWith('navigation.')
+    );
+    const uniqueCrumbs = removeDuplicateNavCrumbs(crumbs, spans);
+    return [...uniqueCrumbs, ...spans].sort(sortFrames);
+  });
 
   getLPCFrames = memoize(() => this._sortedSpanFrames.filter(isLCPFrame));
 

From 2769c309ea82fed5a26011d7f8af370c51be171a Mon Sep 17 00:00:00 2001
From: Ryan Albrecht <ryan.albrecht@sentry.io>
Date: Tue, 16 Apr 2024 16:04:02 -0700
Subject: [PATCH 2/2] update tests to only allow one nav crumb

---
 static/app/utils/replays/replayReader.spec.tsx | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/static/app/utils/replays/replayReader.spec.tsx b/static/app/utils/replays/replayReader.spec.tsx
index 190b5ac43b8668..1f180c8feed9e3 100644
--- a/static/app/utils/replays/replayReader.spec.tsx
+++ b/static/app/utils/replays/replayReader.spec.tsx
@@ -200,8 +200,7 @@ describe('ReplayReader', () => {
         expected: [
           expect.objectContaining({category: 'replay.init'}),
           expect.objectContaining({category: 'ui.slowClickDetected'}),
-          expect.objectContaining({category: 'navigation'}),
-          expect.objectContaining({op: 'navigation.navigate'}),
+          expect.objectContaining({op: 'navigation.navigate'}), // prefer the nav span over the breadcrumb
           expect.objectContaining({category: 'ui.click'}),
           expect.objectContaining({category: 'ui.click'}),
         ],