-
-
Notifications
You must be signed in to change notification settings - Fork 1.7k
feat(replay): Add additional properties for UI clicks #7395
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
Merged
billyvg
merged 16 commits into
develop
from
feat-replay-add-more-dom-attributes-on-click
Mar 16, 2023
Merged
Changes from 8 commits
Commits
Show all changes
16 commits
Select commit
Hold shift + click to select a range
8b97141
feat(replay): Add additional properties for UI clicks
billyvg 8a726a0
add comments
billyvg 8f7d5be
fix tests
billyvg ba52231
extract get Attributes, fix textContent not masked, fix tests
billyvg 8aa9ee1
dupe textContent
billyvg 793e41f
Merge branch 'develop' into feat-replay-add-more-dom-attributes-on-click
billyvg 7525513
add more attributes, create separate test
billyvg 5ed6118
run 100x for flake
billyvg 3436641
Merge branch 'develop' into feat-replay-add-more-dom-attributes-on-click
billyvg 8bfeb83
moved...
billyvg 2f7e73e
fix tests
billyvg e616727
skip firefox/webkit test
billyvg f836c0b
restore `message` in breadcrumb for now
billyvg eb89070
remove bench
billyvg 8e38516
Merge branch 'develop' into feat-replay-add-more-dom-attributes-on-click
billyvg 1269102
remove clickBreadcrumbs, use existing customEvents, skip some flakey …
billyvg File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
18 changes: 18 additions & 0 deletions
18
packages/integration-tests/suites/replay/clickBreadcrumbs/init.js
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,18 @@ | ||
import * as Sentry from '@sentry/browser'; | ||
|
||
window.Sentry = Sentry; | ||
window.Replay = new Sentry.Replay({ | ||
flushMinDelay: 500, | ||
flushMaxDelay: 500, | ||
useCompression: false, | ||
blockAllMedia: false, | ||
}); | ||
|
||
Sentry.init({ | ||
dsn: 'https://[email protected]/1337', | ||
sampleRate: 0, | ||
replaysSessionSampleRate: 1.0, | ||
replaysOnErrorSampleRate: 0.0, | ||
|
||
integrations: [window.Replay], | ||
}); |
17 changes: 17 additions & 0 deletions
17
packages/integration-tests/suites/replay/clickBreadcrumbs/template.html
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,17 @@ | ||
<!DOCTYPE html> | ||
<html> | ||
<head> | ||
<meta charset="utf-8" /> | ||
</head> | ||
<body> | ||
<div role="button" id="error" class="btn btn-error" aria-label="An Error">An Error</div> | ||
<button> | ||
<img id="img" | ||
alt="Alt Text" | ||
/> | ||
</button> | ||
<button class="sentry-unmask" aria-label="Unmasked label"> | ||
Unmasked | ||
</button> | ||
</body> | ||
</html> |
103 changes: 103 additions & 0 deletions
103
packages/integration-tests/suites/replay/clickBreadcrumbs/test.ts
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,103 @@ | ||
import { expect } from '@playwright/test'; | ||
|
||
import { sentryTest } from '../../../utils/fixtures'; | ||
import { expectedClickBreadcrumb } from '../../../utils/replayEventTemplates'; | ||
import { getReplayRecordingContent, shouldSkipReplayTest, waitForReplayRequest } from '../../../utils/replayHelpers'; | ||
|
||
for (let i = 0; i < 100; i++) { | ||
sentryTest( | ||
`replay should have correct click breadcrumbs${i}`, | ||
async ({ forceFlushReplay, getLocalTestPath, page }) => { | ||
if (shouldSkipReplayTest()) { | ||
sentryTest.skip(); | ||
} | ||
|
||
const reqPromise0 = waitForReplayRequest(page, 0); | ||
const reqPromise1 = waitForReplayRequest(page, 1); | ||
|
||
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 getLocalTestPath({ testDir: __dirname }); | ||
|
||
await page.goto(url); | ||
await reqPromise0; | ||
|
||
await page.click('#error'); | ||
await page.click('#img'); | ||
await page.click('.sentry-unmask'); | ||
await forceFlushReplay(); | ||
const req1 = await reqPromise1; | ||
const content1 = getReplayRecordingContent(req1); | ||
expect(content1.breadcrumbs).toEqual( | ||
expect.arrayContaining([ | ||
{ | ||
...expectedClickBreadcrumb, | ||
data: { | ||
nodeId: expect.any(Number), | ||
node: { | ||
attributes: { | ||
'aria-label': '** *****', | ||
class: 'btn btn-error', | ||
id: 'error', | ||
role: 'button', | ||
}, | ||
id: expect.any(Number), | ||
tagName: 'div', | ||
textContent: '** *****', | ||
}, | ||
}, | ||
}, | ||
]), | ||
); | ||
|
||
expect(content1.breadcrumbs).toEqual( | ||
expect.arrayContaining([ | ||
{ | ||
...expectedClickBreadcrumb, | ||
data: { | ||
nodeId: expect.any(Number), | ||
node: { | ||
attributes: { | ||
alt: 'Alt Text', | ||
id: 'img', | ||
}, | ||
id: expect.any(Number), | ||
tagName: 'img', | ||
textContent: '', | ||
}, | ||
}, | ||
}, | ||
]), | ||
); | ||
|
||
expect(content1.breadcrumbs).toEqual( | ||
expect.arrayContaining([ | ||
{ | ||
...expectedClickBreadcrumb, | ||
data: { | ||
nodeId: expect.any(Number), | ||
node: { | ||
attributes: { | ||
// TODO(rrweb): This is a bug in our rrweb fork! | ||
// This attribute should be unmasked. | ||
// 'aria-label': 'Unmasked label', | ||
'aria-label': '******** *****', | ||
class: 'sentry-unmask', | ||
}, | ||
id: expect.any(Number), | ||
tagName: 'button', | ||
textContent: 'Unmasked', | ||
}, | ||
}, | ||
}, | ||
]), | ||
); | ||
}, | ||
); | ||
} |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -45,6 +45,7 @@ | |
"@babel/core": "^7.17.5", | ||
"@sentry-internal/replay-worker": "7.43.0", | ||
"@sentry-internal/rrweb": "1.105.0", | ||
"@sentry-internal/rrweb-snapshot": "1.105.0", | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. imported a type and enum |
||
"jsdom-worker": "^0.2.1", | ||
"tslib": "^1.9.3" | ||
}, | ||
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
32 changes: 32 additions & 0 deletions
32
packages/replay/src/coreHandlers/util/getAttributesToRecord.ts
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,32 @@ | ||
// Attributes we are interested in: | ||
const ATTRIBUTES_TO_RECORD = new Set([ | ||
'id', | ||
'class', | ||
'aria-label', | ||
'role', | ||
'name', | ||
'alt', | ||
'title', | ||
'data-test-id', | ||
'data-testid', | ||
]); | ||
|
||
/** | ||
* Inclusion list of attributes that we want to record from the DOM element | ||
*/ | ||
export function getAttributesToRecord(attributes: Record<string, unknown>): Record<string, unknown> { | ||
const obj: Record<string, unknown> = {}; | ||
for (const key in attributes) { | ||
if (ATTRIBUTES_TO_RECORD.has(key)) { | ||
let normalizedKey = key; | ||
|
||
if (key === 'data-testid' || key === 'data-test-id') { | ||
normalizedKey = 'testId'; | ||
} | ||
Comment on lines
+24
to
+26
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @ryan953 let's capture both and normal to |
||
|
||
obj[normalizedKey] = attributes[key]; | ||
} | ||
} | ||
|
||
return obj; | ||
} |
32 changes: 32 additions & 0 deletions
32
packages/replay/test/unit/coreHandlers/util/getAttributesToRecord.test.ts
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,32 @@ | ||
import { getAttributesToRecord } from '../../../../src/coreHandlers/util/getAttributesToRecord'; | ||
|
||
it('records only included attributes', function () { | ||
expect( | ||
getAttributesToRecord({ | ||
id: 'foo', | ||
classList: ['btn', 'btn-primary'], | ||
}), | ||
).toEqual({ | ||
id: 'foo', | ||
classList: ['btn', 'btn-primary'], | ||
}); | ||
|
||
expect( | ||
getAttributesToRecord({ | ||
id: 'foo', | ||
classList: ['btn', 'btn-primary'], | ||
tabIndex: 2, | ||
ariaDescribedBy: 'tooltip-1', | ||
}), | ||
).toEqual({ | ||
id: 'foo', | ||
classList: ['btn', 'btn-primary'], | ||
}); | ||
|
||
expect( | ||
getAttributesToRecord({ | ||
tabIndex: 2, | ||
ariaDescribedBy: 'tooltip-1', | ||
}), | ||
).toEqual({}); | ||
}); |
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
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