feat(replay): Rework slow click & multi click detection (#8322)

This PR reworks the slow click detection to accommodate rage click detection as well. This required substantial changes, as we need to keep track of stuff much more. Now, we keep a list of clicks that come in in a new class. We register a single set of listeners (mutation observer, click listener, scroll listener), and then try to route things to the correct clicks as much as possible. Any clicks within 1 second count as "multi click", so are not considered for slow clicks at all, but counted on the first click. After a second, a click (even on the same element) will be treated as a "new" click. ref #8300
getsentry · Jun 16, 2023 · dba9a3d · dba9a3d
1 parent ea45d20
commit dba9a3d
Show file tree

Hide file tree

Showing 17 changed files with 1,142 additions and 237 deletions.
diff --git a/packages/browser-integration-tests/suites/replay/slowClick/clickTargets/test.ts b/packages/browser-integration-tests/suites/replay/slowClick/clickTargets/test.ts
@@ -65,8 +65,10 @@ import { getCustomRecordingEvents, shouldSkipReplayTest, waitForReplayRequest }
       expect(slowClickBreadcrumbs).toEqual([
         {
           category: 'ui.slowClickDetected',
+          type: 'default',
           data: {
             endReason: 'timeout',
+            clickCount: 1,
             node: {
               attributes: expect.objectContaining({
                 id,

diff --git a/packages/browser-integration-tests/suites/replay/slowClick/multiClick/test.ts b/packages/browser-integration-tests/suites/replay/slowClick/multiClick/test.ts
@@ -0,0 +1,60 @@
+import { expect } from '@playwright/test';
+
+import { sentryTest } from '../../../../utils/fixtures';
+import { getCustomRecordingEvents, shouldSkipReplayTest, waitForReplayRequest } from '../../../../utils/replayHelpers';
+
+sentryTest('captures multi click when not detecting slow click', async ({ getLocalTestUrl, page }) => {
+  if (shouldSkipReplayTest()) {
+    sentryTest.skip();
+  }
+
+  const reqPromise0 = waitForReplayRequest(page, 0);
+
+  await page.route('https://dsn.ingest.sentry.io/**/*', route => {
+    return route.fulfill({
+      status: 200,
+      contentType: 'application/json',
+      body: JSON.stringify({ id: 'test-id' }),
+    });
+  });
+
+  const url = await getLocalTestUrl({ testDir: __dirname });
+
+  await page.goto(url);
+  await reqPromise0;
+
+  const reqPromise1 = waitForReplayRequest(page, (event, res) => {
+    const { breadcrumbs } = getCustomRecordingEvents(res);
+
+    return breadcrumbs.some(breadcrumb => breadcrumb.category === 'ui.multiClick');
+  });
+
+  await page.click('#mutationButtonImmediately', { clickCount: 4 });
+
+  const { breadcrumbs } = getCustomRecordingEvents(await reqPromise1);
+
+  const slowClickBreadcrumbs = breadcrumbs.filter(breadcrumb => breadcrumb.category === 'ui.multiClick');
+
+  expect(slowClickBreadcrumbs).toEqual([
+    {
+      category: 'ui.multiClick',
+      type: 'default',
+      data: {
+        clickCount: 4,
+        metric: true,
+        node: {
+          attributes: {
+            id: 'mutationButtonImmediately',
+          },
+          id: expect.any(Number),
+          tagName: 'button',
+          textContent: '******* ******** ***********',
+        },
+        nodeId: expect.any(Number),
+        url: 'http://sentry-test.io/index.html',
+      },
+      message: 'body > button#mutationButtonImmediately',
+      timestamp: expect.any(Number),
+    },
+  ]);
+});
diff --git a/packages/browser-integration-tests/suites/replay/slowClick/mutation/test.ts b/packages/browser-integration-tests/suites/replay/slowClick/mutation/test.ts
@@ -29,8 +29,6 @@ sentryTest('mutation after threshold results in slow click', async ({ getLocalTe
     return breadcrumbs.some(breadcrumb => breadcrumb.category === 'ui.slowClickDetected');
   });
 
-  // Trigger this twice, sometimes this was flaky otherwise...
-  await page.click('#mutationButton');
   await page.click('#mutationButton');
 
   const { breadcrumbs } = getCustomRecordingEvents(await reqPromise1);
@@ -40,8 +38,71 @@ sentryTest('mutation after threshold results in slow click', async ({ getLocalTe
   expect(slowClickBreadcrumbs).toEqual([
     {
       category: 'ui.slowClickDetected',
+      type: 'default',
+      data: {
+        endReason: 'mutation',
+        clickCount: 1,
+        node: {
+          attributes: {
+            id: 'mutationButton',
+          },
+          id: expect.any(Number),
+          tagName: 'button',
+          textContent: '******* ********',
+        },
+        nodeId: expect.any(Number),
+        timeAfterClickMs: expect.any(Number),
+        url: 'http://sentry-test.io/index.html',
+      },
+      message: 'body > button#mutationButton',
+      timestamp: expect.any(Number),
+    },
+  ]);
+
+  expect(slowClickBreadcrumbs[0]?.data?.timeAfterClickMs).toBeGreaterThan(3000);
+  expect(slowClickBreadcrumbs[0]?.data?.timeAfterClickMs).toBeLessThan(3100);
+});
+
+sentryTest('multiple clicks are counted', async ({ getLocalTestUrl, page }) => {
+  if (shouldSkipReplayTest()) {
+    sentryTest.skip();
+  }
+
+  const reqPromise0 = waitForReplayRequest(page, 0);
+
+  await page.route('https://dsn.ingest.sentry.io/**/*', route => {
+    return route.fulfill({
+      status: 200,
+      contentType: 'application/json',
+      body: JSON.stringify({ id: 'test-id' }),
+    });
+  });
+
+  const url = await getLocalTestUrl({ testDir: __dirname });
+
+  await page.goto(url);
+  await reqPromise0;
+
+  const reqPromise1 = waitForReplayRequest(page, (event, res) => {
+    const { breadcrumbs } = getCustomRecordingEvents(res);
+
+    return breadcrumbs.some(breadcrumb => breadcrumb.category === 'ui.slowClickDetected');
+  });
+
+  void page.click('#mutationButton', { clickCount: 4 });
+
+  const { breadcrumbs } = getCustomRecordingEvents(await reqPromise1);
+
+  const slowClickBreadcrumbs = breadcrumbs.filter(breadcrumb => breadcrumb.category === 'ui.slowClickDetected');
+  const multiClickBreadcrumbs = breadcrumbs.filter(breadcrumb => breadcrumb.category === 'ui.multiClick');
+
+  expect(slowClickBreadcrumbs).toEqual([
+    {
+      category: 'ui.slowClickDetected',
+      type: 'default',
       data: {
         endReason: 'mutation',
+        clickCount: 4,
         node: {
           attributes: {
             id: 'mutationButton',
@@ -58,6 +119,7 @@ sentryTest('mutation after threshold results in slow click', async ({ getLocalTe
       timestamp: expect.any(Number),
     },
   ]);
+  expect(multiClickBreadcrumbs.length).toEqual(0);
 
   expect(slowClickBreadcrumbs[0]?.data?.timeAfterClickMs).toBeGreaterThan(3000);
   expect(slowClickBreadcrumbs[0]?.data?.timeAfterClickMs).toBeLessThan(3100);
@@ -165,3 +227,55 @@ sentryTest('inline click handler does not trigger slow click', async ({ getLocal
     },
   ]);
 });
+
+sentryTest('mouseDown events are considered', async ({ browserName, getLocalTestUrl, page }) => {
+  // This test seems to only be flakey on firefox
+  if (shouldSkipReplayTest() || ['firefox'].includes(browserName)) {
+    sentryTest.skip();
+  }
+
+  const reqPromise0 = waitForReplayRequest(page, 0);
+
+  await page.route('https://dsn.ingest.sentry.io/**/*', route => {
+    return route.fulfill({
+      status: 200,
+      contentType: 'application/json',
+      body: JSON.stringify({ id: 'test-id' }),
+    });
+  });
+
+  const url = await getLocalTestUrl({ testDir: __dirname });
+
+  await page.goto(url);
+  await reqPromise0;
+
+  const reqPromise1 = waitForReplayRequest(page, (event, res) => {
+    const { breadcrumbs } = getCustomRecordingEvents(res);
+
+    return breadcrumbs.some(breadcrumb => breadcrumb.category === 'ui.click');
+  });
+
+  await page.click('#mouseDownButton');
+
+  const { breadcrumbs } = getCustomRecordingEvents(await reqPromise1);
+
+  expect(breadcrumbs).toEqual([
+    {
+      category: 'ui.click',
+      data: {
+        node: {
+          attributes: {
+            id: 'mouseDownButton',
+          },
+          id: expect.any(Number),
+          tagName: 'button',
+          textContent: '******* ******** ** ***** ****',
+        },
+        nodeId: expect.any(Number),
+      },
+      message: 'body > button#mouseDownButton',
+      timestamp: expect.any(Number),
+      type: 'default',
+    },
+  ]);
+});
diff --git a/packages/browser-integration-tests/suites/replay/slowClick/scroll/test.ts b/packages/browser-integration-tests/suites/replay/slowClick/scroll/test.ts
@@ -89,8 +89,10 @@ sentryTest('late scroll triggers slow click', async ({ getLocalTestUrl, page })
   expect(slowClickBreadcrumbs).toEqual([
     {
       category: 'ui.slowClickDetected',
+      type: 'default',
       data: {
         endReason: 'timeout',
+        clickCount: 1,
         node: {
           attributes: {
             id: 'scrollLateButton',

diff --git a/packages/browser-integration-tests/suites/replay/slowClick/template.html b/packages/browser-integration-tests/suites/replay/slowClick/template.html
@@ -18,6 +18,7 @@
     <button id="scrollButton">Trigger scroll</button>
     <button id="scrollLateButton">Trigger scroll late</button>
     <button id="mutationIgnoreButton" class="ignore-class">Trigger scroll late</button>
+    <button id="mouseDownButton">Trigger mutation on mouse down</button>
 
     <a href="#" id="link">Link</a>
     <a href="#" target="_blank" id="linkExternal">Link external</a>
@@ -69,6 +70,9 @@ <h1 id="h2">Bottom</h1>
           console.log('DONE');
         }, 3001);
       });
+      document.getElementById('mouseDownButton').addEventListener('mousedown', () => {
+        document.getElementById('out').innerHTML += 'mutationButton clicked<br>';
+      });
 
       // Do nothing on these elements
       document

diff --git a/packages/browser-integration-tests/suites/replay/slowClick/timeout/test.ts b/packages/browser-integration-tests/suites/replay/slowClick/timeout/test.ts
@@ -38,8 +38,10 @@ sentryTest('mutation after timeout results in slow click', async ({ getLocalTest
   expect(slowClickBreadcrumbs).toEqual([
     {
       category: 'ui.slowClickDetected',
+      type: 'default',
       data: {
         endReason: 'timeout',
+        clickCount: 1,
         node: {
           attributes: {
             id: 'mutationButtonLate',
@@ -93,8 +95,10 @@ sentryTest('console.log results in slow click', async ({ getLocalTestUrl, page }
   expect(slowClickBreadcrumbs).toEqual([
     {
       category: 'ui.slowClickDetected',
+      type: 'default',
       data: {
         endReason: 'timeout',
+        clickCount: 1,
         node: {
           attributes: {
             id: 'consoleLogButton',

diff --git a/packages/replay/src/constants.ts b/packages/replay/src/constants.ts
@@ -42,3 +42,5 @@ export const CONSOLE_ARG_MAX_SIZE = 5_000;
 export const SLOW_CLICK_THRESHOLD = 3_000;
 /* For scroll actions after a click, we only look for a very short time period to detect programmatic scrolling. */
 export const SLOW_CLICK_SCROLL_TIMEOUT = 300;
+/* Clicks in this time period are considered e.g. double/triple clicks. */
+export const MULTI_CLICK_TIMEOUT = 1_000;