Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Merged
merged 5 commits into from
Jun 16, 2023
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -29,19 +29,83 @@ 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');

const { breadcrumbs } = getCustomRecordingEvents(await reqPromise1);

const slowClickBreadcrumbs = breadcrumbs.filter(breadcrumb => breadcrumb.category === 'ui.slowClickDetected');

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');
});

await page.click('#mutationButton');
await page.click('#mutationButton');
await page.click('#mutationButton');
await page.click('#mutationButton');

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',
Expand All @@ -58,6 +122,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);
Expand Down Expand Up @@ -165,3 +230,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',
},
]);
});
Original file line number Diff line number Diff line change
@@ -0,0 +1,63 @@
import { expect } from '@playwright/test';

import { sentryTest } from '../../../../utils/fixtures';
import { getCustomRecordingEvents, shouldSkipReplayTest, waitForReplayRequest } from '../../../../utils/replayHelpers';

sentryTest('captures rage 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');
await page.click('#mutationButtonImmediately');
await page.click('#mutationButtonImmediately');
await page.click('#mutationButtonImmediately');

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),
},
]);
});
Original file line number Diff line number Diff line change
Expand Up @@ -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',
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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>
Expand Down Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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',
Expand Down Expand Up @@ -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',
Expand Down
2 changes: 2 additions & 0 deletions packages/replay/src/constants.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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;