feat(replay): Capture slow clicks (GA)

This moves the slow click detection out of GA and makes it generally available. You can opt-out of this by setting `slowClickTimeout: 0`.
getsentry · Jun 7, 2023 · 6b4bda7 · 6b4bda7
1 parent 8ffde2a
commit 6b4bda7
Show file tree

Hide file tree

Showing 15 changed files with 210 additions and 88 deletions.
diff --git a/packages/browser-integration-tests/suites/replay/slowClick/disable/init.js b/packages/browser-integration-tests/suites/replay/slowClick/disable/init.js
@@ -0,0 +1,17 @@
+import * as Sentry from '@sentry/browser';
+
+window.Sentry = Sentry;
+window.Replay = new Sentry.Replay({
+  flushMinDelay: 500,
+  flushMaxDelay: 500,
+  slowClickTimeout: 0,
+});
+
+Sentry.init({
+  dsn: 'https://public@dsn.ingest.sentry.io/1337',
+  sampleRate: 0,
+  replaysSessionSampleRate: 1.0,
+  replaysOnErrorSampleRate: 0.0,
+
+  integrations: [window.Replay],
+});
diff --git a/packages/browser-integration-tests/suites/replay/slowClick/disable/test.ts b/packages/browser-integration-tests/suites/replay/slowClick/disable/test.ts
@@ -0,0 +1,55 @@
+import { expect } from '@playwright/test';
+
+import { sentryTest } from '../../../../utils/fixtures';
+import { getCustomRecordingEvents, shouldSkipReplayTest, waitForReplayRequest } from '../../../../utils/replayHelpers';
+
+sentryTest('does not capture slow click when slowClickTimeout === 0', 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.click');
+  });
+
+  await page.click('#mutationButton');
+
+  const { breadcrumbs } = getCustomRecordingEvents(await reqPromise1);
+
+  expect(breadcrumbs).toEqual([
+    {
+      category: 'ui.click',
+      data: {
+        node: {
+          attributes: {
+            id: 'mutationButton',
+          },
+          id: expect.any(Number),
+          tagName: 'button',
+          textContent: '******* ********',
+        },
+        nodeId: expect.any(Number),
+      },
+      message: 'body > button#mutationButton',
+      timestamp: expect.any(Number),
+      type: 'default',
+    },
+  ]);
+});
diff --git a/packages/browser-integration-tests/suites/replay/slowClick/ignore/test.ts b/packages/browser-integration-tests/suites/replay/slowClick/ignore/test.ts
@@ -54,3 +54,54 @@ sentryTest('click is ignored on ignoreSelectors', async ({ getLocalTestUrl, page
     },
   ]);
 });
+
+sentryTest('click is ignored on div', 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.click');
+  });
+
+  await page.click('#mutationDiv');
+
+  const { breadcrumbs } = getCustomRecordingEvents(await reqPromise1);
+
+  expect(breadcrumbs).toEqual([
+    {
+      category: 'ui.click',
+      data: {
+        node: {
+          attributes: {
+            id: 'mutationDiv',
+          },
+          id: expect.any(Number),
+          tagName: 'div',
+          textContent: '******* ********',
+        },
+        nodeId: expect.any(Number),
+      },
+      message: 'body > div#mutationDiv',
+      timestamp: expect.any(Number),
+      type: 'default',
+    },
+  ]);
+});
diff --git a/packages/browser-integration-tests/suites/replay/slowClick/init.js b/packages/browser-integration-tests/suites/replay/slowClick/init.js
@@ -4,14 +4,8 @@ window.Sentry = Sentry;
 window.Replay = new Sentry.Replay({
   flushMinDelay: 500,
   flushMaxDelay: 500,
-  _experiments: {
-    slowClicks: {
-      threshold: 300,
-      scrollThreshold: 300,
-      timeout: 2000,
-      ignoreSelectors: ['.ignore-class', '[ignore-attribute]'],
-    },
-  },
+  slowClickTimeout: 3100,
+  slowClickIgnoreSelectors: ['.ignore-class', '[ignore-attribute]'],
 });
 
 Sentry.init({

diff --git a/packages/browser-integration-tests/suites/replay/slowClick/mutation/test.ts b/packages/browser-integration-tests/suites/replay/slowClick/mutation/test.ts
@@ -59,8 +59,8 @@ sentryTest('mutation after threshold results in slow click', async ({ getLocalTe
     },
   ]);
 
-  expect(slowClickBreadcrumbs[0]?.data?.timeAfterClickMs).toBeGreaterThan(300);
-  expect(slowClickBreadcrumbs[0]?.data?.timeAfterClickMs).toBeLessThan(2000);
+  expect(slowClickBreadcrumbs[0]?.data?.timeAfterClickMs).toBeGreaterThan(3000);
+  expect(slowClickBreadcrumbs[0]?.data?.timeAfterClickMs).toBeLessThan(3100);
 });
 
 sentryTest('immediate mutation does not trigger slow click', async ({ browserName, getLocalTestUrl, page }) => {
@@ -165,56 +165,3 @@ sentryTest('inline click handler does not trigger slow click', async ({ getLocal
     },
   ]);
 });
-
-sentryTest('click is not ignored on div', 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');
-  });
-
-  await page.click('#mutationDiv');
-
-  const { breadcrumbs } = getCustomRecordingEvents(await reqPromise1);
-
-  expect(breadcrumbs.filter(({ category }) => category === 'ui.slowClickDetected')).toEqual([
-    {
-      category: 'ui.slowClickDetected',
-      data: {
-        endReason: 'mutation',
-        node: {
-          attributes: {
-            id: 'mutationDiv',
-          },
-          id: expect.any(Number),
-          tagName: 'div',
-          textContent: '******* ********',
-        },
-        nodeId: expect.any(Number),
-        timeAfterClickMs: expect.any(Number),
-        url: 'http://sentry-test.io/index.html',
-      },
-      message: 'body > div#mutationDiv',
-      timestamp: expect.any(Number),
-    },
-  ]);
-});
diff --git a/packages/browser-integration-tests/suites/replay/slowClick/template.html b/packages/browser-integration-tests/suites/replay/slowClick/template.html
@@ -36,22 +36,22 @@ <h1 id="h2">Bottom</h1>
       document.getElementById('mutationButton').addEventListener('click', () => {
         setTimeout(() => {
           document.getElementById('out').innerHTML += 'mutationButton clicked<br>';
-        }, 400);
+        }, 3001);
       });
       document.getElementById('mutationIgnoreButton').addEventListener('click', () => {
         setTimeout(() => {
           document.getElementById('out').innerHTML += 'mutationIgnoreButton clicked<br>';
-        }, 400);
+        }, 3001);
       });
       document.getElementById('mutationDiv').addEventListener('click', () => {
         setTimeout(() => {
           document.getElementById('out').innerHTML += 'mutationDiv clicked<br>';
-        }, 400);
+        }, 3001);
       });
       document.getElementById('mutationButtonLate').addEventListener('click', () => {
         setTimeout(() => {
           document.getElementById('out').innerHTML += 'mutationButtonLate clicked<br>';
-        }, 3000);
+        }, 3101);
       });
       document.getElementById('mutationButtonImmediately').addEventListener('click', () => {
         document.getElementById('out').innerHTML += 'mutationButtonImmediately clicked<br>';
@@ -62,12 +62,12 @@ <h1 id="h2">Bottom</h1>
       document.getElementById('scrollLateButton').addEventListener('click', () => {
         setTimeout(() => {
           document.getElementById('h2').scrollIntoView({ behavior: 'smooth' });
-        }, 400);
+        }, 3001);
       });
       document.getElementById('consoleLogButton').addEventListener('click', () => {
         setTimeout(() => {
           console.log('DONE');
-        }, 400);
+        }, 3001);
       });
 
       // Do nothing on these elements

diff --git a/packages/browser-integration-tests/suites/replay/slowClick/timeout/test.ts b/packages/browser-integration-tests/suites/replay/slowClick/timeout/test.ts
@@ -49,7 +49,7 @@ sentryTest('mutation after timeout results in slow click', async ({ getLocalTest
           textContent: '******* ******** ****',
         },
         nodeId: expect.any(Number),
-        timeAfterClickMs: 2000,
+        timeAfterClickMs: 3100,
         url: 'http://sentry-test.io/index.html',
       },
       message: 'body > button#mutationButtonLate',
@@ -104,7 +104,7 @@ sentryTest('console.log results in slow click', async ({ getLocalTestUrl, page }
           textContent: '******* ******* ***',
         },
         nodeId: expect.any(Number),
-        timeAfterClickMs: 2000,
+        timeAfterClickMs: 3100,
         url: 'http://sentry-test.io/index.html',
       },
       message: 'body > button#consoleLogButton',

diff --git a/packages/replay/src/constants.ts b/packages/replay/src/constants.ts
@@ -37,3 +37,8 @@ export const NETWORK_BODY_MAX_SIZE = 150_000;
 
 /* The max size of a single console arg that is captured. Any arg larger than this will be truncated. */
 export const CONSOLE_ARG_MAX_SIZE = 5_000;
+
+/* Min. time to wait before we consider something a slow click. */
+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;
diff --git a/packages/replay/src/coreHandlers/handleDom.ts b/packages/replay/src/coreHandlers/handleDom.ts
@@ -3,6 +3,7 @@ import { NodeType } from '@sentry-internal/rrweb-snapshot';
 import type { Breadcrumb } from '@sentry/types';
 import { htmlTreeAsString } from '@sentry/utils';
 
+import { SLOW_CLICK_SCROLL_TIMEOUT, SLOW_CLICK_THRESHOLD } from '../constants';
 import type { ReplayContainer, SlowClickConfig } from '../types';
 import { createBreadcrumb } from '../util/createBreadcrumb';
 import { detectSlowClick } from './handleSlowClick';
@@ -17,14 +18,14 @@ export interface DomHandlerData {
 export const handleDomListener: (replay: ReplayContainer) => (handlerData: DomHandlerData) => void = (
   replay: ReplayContainer,
 ) => {
-  const slowClickExperiment = replay.getOptions()._experiments.slowClicks;
+  const { slowClickTimeout, slowClickIgnoreSelectors } = replay.getOptions();
 
-  const slowClickConfig: SlowClickConfig | undefined = slowClickExperiment
+  const slowClickConfig: SlowClickConfig | undefined = slowClickTimeout
     ? {
-        threshold: slowClickExperiment.threshold,
-        timeout: slowClickExperiment.timeout,
-        scrollTimeout: slowClickExperiment.scrollTimeout,
-        ignoreSelector: slowClickExperiment.ignoreSelectors ? slowClickExperiment.ignoreSelectors.join(',') : '',
+        threshold: Math.min(SLOW_CLICK_THRESHOLD, slowClickTimeout),
+        timeout: slowClickTimeout,
+        scrollTimeout: SLOW_CLICK_SCROLL_TIMEOUT,
+        ignoreSelector: slowClickIgnoreSelectors ? slowClickIgnoreSelectors.join(',') : '',
       }
     : undefined;
 

diff --git a/packages/replay/src/coreHandlers/handleSlowClick.ts b/packages/replay/src/coreHandlers/handleSlowClick.ts
@@ -114,15 +114,16 @@ function handleSlowClick(
   addBreadcrumbEvent(replay, breadcrumb);
 }
 
-const SLOW_CLICK_IGNORE_TAGS = ['SELECT', 'OPTION'];
+const SLOW_CLICK_TAGS = ['A', 'BUTTON', 'INPUT'];
 
-function ignoreElement(node: HTMLElement, config: SlowClickConfig): boolean {
-  // If <input> tag, we only want to consider input[type='submit'] & input[type='button']
-  if (node.tagName === 'INPUT' && !['submit', 'button'].includes(node.getAttribute('type') || '')) {
+/** exported for tests only */
+export function ignoreElement(node: HTMLElement, config: SlowClickConfig): boolean {
+  if (!SLOW_CLICK_TAGS.includes(node.tagName)) {
     return true;
   }
 
-  if (SLOW_CLICK_IGNORE_TAGS.includes(node.tagName)) {
+  // If <input> tag, we only want to consider input[type='submit'] & input[type='button']
+  if (node.tagName === 'INPUT' && !['submit', 'button'].includes(node.getAttribute('type') || '')) {
     return true;
   }
 

diff --git a/packages/replay/src/coreHandlers/util/getAttributesToRecord.ts b/packages/replay/src/coreHandlers/util/getAttributesToRecord.ts
@@ -10,6 +10,8 @@ const ATTRIBUTES_TO_RECORD = new Set([
   'title',
   'data-test-id',
   'data-testid',
+  'disabled',
+  'aria-disabled',
 ]);
 
 /**

diff --git a/packages/replay/src/integration.ts b/packages/replay/src/integration.ts
@@ -63,6 +63,9 @@ export class Replay implements Integration {
     mutationBreadcrumbLimit = 750,
     mutationLimit = 10_000,
 
+    slowClickTimeout = 7_000,
+    slowClickIgnoreSelectors = [],
+
     networkDetailAllowUrls = [],
     networkCaptureBodies = true,
     networkRequestHeaders = [],
@@ -132,6 +135,8 @@ export class Replay implements Integration {
       maskAllText,
       mutationBreadcrumbLimit,
       mutationLimit,
+      slowClickTimeout,
+      slowClickIgnoreSelectors,
       networkDetailAllowUrls,
       networkCaptureBodies,
       networkRequestHeaders: _getMergedNetworkHeaders(networkRequestHeaders),

diff --git a/packages/replay/src/types/replay.ts b/packages/replay/src/types/replay.ts
@@ -155,6 +155,20 @@ export interface ReplayPluginOptions extends ReplayNetworkOptions {
    */
   mutationLimit: number;
 
+  /**
+   * The max. time in ms to wait for a slow click to finish.
+   * After this amount of time we stop waiting for actions after a click happened.
+   * Set this to 0 to disable slow click capture.
+   *
+   * Default: 7000ms
+   */
+  slowClickTimeout: number;
+
+  /**
+   * Ignore clicks on elements matching the given selectors for slow click detection.
+   */
+  slowClickIgnoreSelectors: string[];
+
   /**
    * Callback before adding a custom recording event
    *
@@ -178,12 +192,6 @@ export interface ReplayPluginOptions extends ReplayNetworkOptions {
   _experiments: Partial<{
     captureExceptions: boolean;
     traceInternals: boolean;
-    slowClicks: {
-      threshold: number;
-      timeout: number;
-      scrollTimeout: number;
-      ignoreSelectors: string[];
-    };
     delayFlushOnCheckout: number;
   }>;
 }