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): Add additional properties for UI clicks #7395

Merged
merged 16 commits into from Mar 16, 2023

Conversation

billyvg
Copy link
Member

@billyvg billyvg commented Mar 9, 2023

For replay click breadcrumbs, add the following properties:

  • tag name
  • text content
  • element attributes

Closes #7394

For replay click breadcrumbs, add the following properties:

* tag name
* text content
* element attributes
packages/replay/src/coreHandlers/handleDom.ts Outdated Show resolved Hide resolved
node: {
id: serializedNode.id,
tagName: serializedNode.tagName,
textContent: targetNode ? Array.from(targetNode.childNodes).filter((node: Node | INode) => '__sn' in node && node.__sn.type === NodeType.Text).map(node => node.textContent) : '',
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I plan on capping the size of this value to 256 characters. No action item but I thought I would mention it.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@cmanallen should backend add additional metadata when it changes the payload? This means frontend could more easily tell user what payloads were modified and why.

@github-actions
Copy link
Contributor

github-actions bot commented Mar 9, 2023

size-limit report 📦

Path Size
@sentry/browser - ES5 CDN Bundle (gzipped + minified) 20.41 KB (0%)
@sentry/browser - ES5 CDN Bundle (minified) 63.3 KB (0%)
@sentry/browser - ES6 CDN Bundle (gzipped + minified) 18.99 KB (+0.01% 🔺)
@sentry/browser - ES6 CDN Bundle (minified) 56.21 KB (0%)
@sentry/browser - Webpack (gzipped + minified) 20.65 KB (0%)
@sentry/browser - Webpack (minified) 67.44 KB (0%)
@sentry/react - Webpack (gzipped + minified) 20.68 KB (0%)
@sentry/nextjs Client - Webpack (gzipped + minified) 52.12 KB (+0.07% 🔺)
@sentry/browser + @sentry/tracing - ES5 CDN Bundle (gzipped + minified) 33.66 KB (+0.02% 🔺)
@sentry/browser + @sentry/tracing - ES6 CDN Bundle (gzipped + minified) 26.01 KB (+0.05% 🔺)
@sentry/replay ES6 CDN Bundle (gzipped + minified) 43.45 KB (+0.64% 🔺)
@sentry/replay - Webpack (gzipped + minified) 37.48 KB (+0.78% 🔺)
@sentry/browser + @sentry/tracing + @sentry/replay - ES6 CDN Bundle (gzipped + minified) 61.59 KB (+0.47% 🔺)
@sentry/browser + @sentry/replay - ES6 CDN Bundle (gzipped + minified) 54.66 KB (+0.51% 🔺)

@bruno-garcia bruno-garcia added the CI-Overhead-Measurements Add this label to run SDK overhead measurements on a PR label Mar 13, 2023
@@ -98,7 +98,20 @@ sentryTest(
expect.arrayContaining([
{
...expectedClickBreadcrumb,
message: 'body > button#error',
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Removing this as it can leak attributes (e.g. aria label) that are masked with maskAllText

@@ -5,7 +5,7 @@
</head>
<body>
<button id="go-background">Go to background</button>
<button id="error">Throw Error</button>
<div role="button" id="error" class="btn btn-error" aria-label="Throw Error">Throw Error</div>
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since the errorMode and errorsInSession tests were already capturing this breadcrumb, decided to add attributes here and make sure they were captured.

@@ -45,6 +45,7 @@
"@babel/core": "^7.17.5",
"@sentry-internal/replay-worker": "7.42.0",
"@sentry-internal/rrweb": "1.105.0",
"@sentry-internal/rrweb-snapshot": "1.105.0",
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

imported a type and enum

@billyvg billyvg marked this pull request as ready for review March 13, 2023 20:37
@github-actions
Copy link
Contributor

github-actions bot commented Mar 13, 2023

Replay SDK metrics 🚀

    Plain +Sentry +Replay
Revision Value Value Diff Ratio Value Diff Ratio
LCP This PR ceb4f4e 64.92 ms 66.09 ms +1.17 ms +1.80 % 67.17 ms +2.25 ms +3.47 %
Baseline ef6b3c7 76.12 ms 69.95 ms -6.17 ms -8.11 % 68.42 ms -7.70 ms -10.12 %
CLS This PR ceb4f4e 0.00 ms 0.00 ms 0.00 ms 0.00 % 0.26 ms +0.26 ms +5619.51 %
Baseline ef6b3c7 0.00 ms 0.00 ms 0.00 ms 0.00 % 0.26 ms +0.25 ms +5493.27 %
CPU This PR ceb4f4e 18.29 % 20.59 % +2.30 pp +12.58 % 37.42 % +19.13 pp +104.64 %
Baseline ef6b3c7 20.27 % 20.59 % +0.32 pp +1.57 % 36.55 % +16.28 pp +80.35 %
JS heap avg This PR ceb4f4e 3.53 MB 6.89 MB +3.36 MB +95.00 % 10.95 MB +7.41 MB +209.80 %
Baseline ef6b3c7 3.51 MB 6.86 MB +3.35 MB +95.67 % 11.14 MB +7.64 MB +217.81 %
JS heap max This PR ceb4f4e 3.87 MB 8.33 MB +4.46 MB +115.20 % 14.07 MB +10.2 MB +263.55 %
Baseline ef6b3c7 3.91 MB 8.35 MB +4.44 MB +113.78 % 13.93 MB +10.02 MB +256.56 %
netTx This PR ceb4f4e 0 B 360.48 kB +360.48 kB n/a 107.18 kB +107.18 kB n/a
Baseline ef6b3c7 0 B 360.47 kB +360.47 kB n/a 107.46 kB +107.46 kB n/a
netRx This PR ceb4f4e 20.16 kB 18.08 kB -2.08 kB -10.30 % 19.15 kB -1.01 kB -5.02 %
Baseline ef6b3c7 17.78 kB 18.19 kB +414 B +2.33 % 17.84 kB +57 B +0.32 %
netCount This PR ceb4f4e 1 2 +1 +100.00 % 5 +4 +400.00 %
Baseline ef6b3c7 1 2 +1 +100.00 % 5 +4 +400.00 %
netTime This PR ceb4f4e 546.64 ms 666.42 ms +119.77 ms +21.91 % 672.43 ms +125.79 ms +23.01 %
Baseline ef6b3c7 446.67 ms 523.56 ms +76.89 ms +17.21 % 669.50 ms +222.84 ms +49.89 %

Baseline results on branch: develop

RevisionLCPCLSCPUJS heap avgJS heap maxnetTxnetRxnetCountnetTime
ef6b3c7-10.10 ms+0.23 ms+15.85 pp+7.66 MB+10.56 MB+107.49 kB+836 B+4+209.40 ms
b1ef00d+5.17 ms+0.26 ms+15.95 pp+7.59 MB+10.18 MB+107.02 kB-220 B+4+306.46 ms
42e542e+12.94 ms+0.25 ms+18.24 pp+7.8 MB+10.31 MB+107.39 kB-2.17 kB+4+289.66 ms
2f3c93c+6.35 ms+0.26 ms+16.82 pp+7.44 MB+9.99 MB+106.58 kB+1.53 kB+4+303.23 ms
4bff5a9+0.60 ms+0.26 ms+15.70 pp+7.44 MB+10.2 MB+106.93 kB-2.04 kB+4+163.67 ms
ba99e7c-0.34 ms+0.23 ms+16.45 pp+7.17 MB+10.05 MB+106.7 kB+2.46 kB+4+119.03 ms
a70376e-7.64 ms+0.25 ms+17.86 pp+6.5 MB+10.21 MB+106.82 kB-35 B+4+321.76 ms
e358e16+7.09 ms+0.26 ms+18.60 pp+7.46 MB+10.03 MB+106.61 kB-943 B+4+99.84 ms
5e27e8f-2.27 ms+0.24 ms+16.19 pp+7.72 MB+10.33 MB+107.49 kB+566 B+4+192.11 ms

Previous results on branch: feat-replay-add-more-dom-attributes-on-click

RevisionLCPCLSCPUJS heap avgJS heap maxnetTxnetRxnetCountnetTime
ef6b3c7-7.70 ms+0.25 ms+16.28 pp+7.64 MB+10.02 MB+107.46 kB+57 B+4+222.84 ms
ef6b3c7-10.10 ms+0.23 ms+15.85 pp+7.66 MB+10.56 MB+107.49 kB+836 B+4+209.40 ms
b1ef00d+5.17 ms+0.26 ms+15.95 pp+7.59 MB+10.18 MB+107.02 kB-220 B+4+306.46 ms
42e542e+12.94 ms+0.25 ms+18.24 pp+7.8 MB+10.31 MB+107.39 kB-2.17 kB+4+289.66 ms
2f3c93c+6.35 ms+0.26 ms+16.82 pp+7.44 MB+9.99 MB+106.58 kB+1.53 kB+4+303.23 ms
4bff5a9+0.60 ms+0.26 ms+15.70 pp+7.44 MB+10.2 MB+106.93 kB-2.04 kB+4+163.67 ms
ba99e7c-0.34 ms+0.23 ms+16.45 pp+7.17 MB+10.05 MB+106.7 kB+2.46 kB+4+119.03 ms
a70376e-7.64 ms+0.25 ms+17.86 pp+6.5 MB+10.21 MB+106.82 kB-35 B+4+321.76 ms
e358e16+7.09 ms+0.26 ms+18.60 pp+7.46 MB+10.03 MB+106.61 kB-943 B+4+99.84 ms
5e27e8f-2.27 ms+0.24 ms+16.19 pp+7.72 MB+10.33 MB+107.49 kB+566 B+4+192.11 ms

*) pp - percentage points - an absolute difference between two percentages.
Last updated: Wed, 15 Mar 2023 22:55:00 GMT

Comment on lines +23 to +25
if (key === 'data-testid' || key === 'data-test-id') {
normalizedKey = 'testId';
}
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@ryan953 let's capture both and normal to testId?

@billyvg billyvg merged commit 3715ae7 into develop Mar 16, 2023
48 checks passed
@billyvg billyvg deleted the feat-replay-add-more-dom-attributes-on-click branch March 16, 2023 11:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CI-Overhead-Measurements Add this label to run SDK overhead measurements on a PR
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Augment click events with more / better node data
4 participants