Skip to content

feat(replay): Do not restart replays on "keydown" #8226

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.

Sign up for GitHub

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
merged 3 commits into from
Jun 1, 2023
Merged

feat(replay): Do not restart replays on "keydown" #8226

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
013bb4d
feat(replay): Do not trigger user activity on "keydown"
billyvg May 26, 2023
bdf5dc0
Merge branch 'develop' into feat-replay-remove-trigger-user-activity-…
billyvg May 30, 2023
3a3eb89
update activity without restarting recording
billyvg May 30, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 4 additions & 1 deletion packages/replay/src/coreHandlers/handleKeyboardEvent.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,10 @@ export function handleKeyboardEvent(replay: ReplayContainer, event: KeyboardEven
return;
}

replay.triggerUserActivity();
Copy link
Member Author

Choose a reason for hiding this comment

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

Alternatively, we could call _updateUserActivity(), the issue with triggerUserActivity is that it will resume recording and create a new session.

cc @mydea

Copy link
Member

Choose a reason for hiding this comment

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

hmm, maybe this... IMHO the "best" solution would be to find the cases where we do not want to capture this (e.g. when tab is involved, ...?) and handle them, but this may be tricky. So probably OK to remove this for now and revisit this later...!

// Update user activity, but do not restart recording as it can create
// noisy/low-value replays (e.g. user comes back from idle, hits alt-tab, new
// session with a single "keydown" breadcrumb is created)
replay.updateUserActivity();

const breadcrumb = getKeyboardBreadcrumb(event);

Expand Down
12 changes: 12 additions & 0 deletions packages/replay/src/replay.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -480,6 +480,18 @@ export class ReplayContainer implements ReplayContainerInterface {
this._updateSessionActivity();
}

/**
* Updates the user activity timestamp *without* resuming
* recording. Some user events (e.g. keydown) can be create
* low-value replays that only contain the keypress as a
* breadcrumb. Instead this would require other events to
* create a new replay after a session has expired.
*/
public updateUserActivity(): void {
this._updateUserActivity();
this._updateSessionActivity();
}

/**
* Only flush if `this.recordingMode === 'session'`
*/
Expand Down
1 change: 1 addition & 0 deletions packages/replay/src/types.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -542,6 +542,7 @@ export interface ReplayContainer {
flushImmediate(): Promise<void>;
cancelFlush(): void;
triggerUserActivity(): void;
updateUserActivity(): void;
addUpdate(cb: AddUpdateCallback): void;
getOptions(): ReplayPluginOptions;
getSessionId(): string | undefined;
Expand Down