Skip to content

Commit

Permalink
feat(replay): Do not capture replays < 5 seconds
Browse files Browse the repository at this point in the history
Promotes the feature from #7949 to GA.

Do not immediately flush on snapshot checkouts, instead delay by minimum flush delay (5 seconds). This means that we will not collect replays < 5 seconds. e.g. User opens site and immediately closes the tab.

Closes getsentry/team-replay#65
  • Loading branch information
billyvg committed Jun 6, 2023
1 parent 8ffde2a commit b08fdc2
Show file tree
Hide file tree
Showing 6 changed files with 160 additions and 970 deletions.
7 changes: 7 additions & 0 deletions packages/replay/src/replay.ts
Original file line number Diff line number Diff line change
Expand Up @@ -524,6 +524,13 @@ export class ReplayContainer implements ReplayContainerInterface {
return this.flushImmediate();
}

/**
* Flush using debounce flush
*/
public flush(): Promise<void> {
return this._debouncedFlush() as Promise<void>;
}

/**
* Always flush via `_debouncedFlush` so that we do not have flushes triggered
* from calling both `flush` and `_debouncedFlush`. Otherwise, there could be
Expand Down
2 changes: 1 addition & 1 deletion packages/replay/src/types/replay.ts
Original file line number Diff line number Diff line change
Expand Up @@ -184,7 +184,6 @@ export interface ReplayPluginOptions extends ReplayNetworkOptions {
scrollTimeout: number;
ignoreSelectors: string[];
};
delayFlushOnCheckout: number;
}>;
}

Expand Down Expand Up @@ -421,6 +420,7 @@ export interface ReplayContainer {
stopRecording(): boolean;
sendBufferedReplayOrFlush(options?: SendBufferedReplayOptions): Promise<void>;
conditionalFlush(): Promise<void>;
flush(): Promise<void>;
flushImmediate(): Promise<void>;
cancelFlush(): void;
triggerUserActivity(): void;
Expand Down
33 changes: 2 additions & 31 deletions packages/replay/src/util/handleRecordingEmit.ts
Original file line number Diff line number Diff line change
Expand Up @@ -80,41 +80,12 @@ export function getHandleRecordingEmit(replay: ReplayContainer): RecordingEmitCa
}
}

const options = replay.getOptions();

// TODO: We want this as an experiment so that we can test
// internally and create metrics before making this the default
if (options._experiments.delayFlushOnCheckout) {
if (replay.recordingMode === 'session') {
// If the full snapshot is due to an initial load, we will not have
// a previous session ID. In this case, we want to buffer events
// for a set amount of time before flushing. This can help avoid
// capturing replays of users that immediately close the window.
// TODO: We should check `recordingMode` here and do nothing if it's
// buffer, instead of checking inside of timeout, this will make our
// tests a bit cleaner as we will need to wait on the delay in order to
// do nothing.
setTimeout(() => replay.conditionalFlush(), options._experiments.delayFlushOnCheckout);

// Cancel any previously debounced flushes to ensure there are no [near]
// simultaneous flushes happening. The latter request should be
// insignificant in this case, so wait for additional user interaction to
// trigger a new flush.
//
// This can happen because there's no guarantee that a recording event
// happens first. e.g. a mouse click can happen and trigger a debounced
// flush before the checkout.
replay.cancelFlush();

return true;
}

// Flush immediately so that we do not miss the first segment, otherwise
// it can prevent loading on the UI. This will cause an increase in short
// replays (e.g. opening and closing a tab quickly), but these can be
// filtered on the UI.
if (replay.recordingMode === 'session') {
// We want to ensure the worker is ready, as otherwise we'd always send the first event uncompressed
void replay.flushImmediate();
void replay.flush();
}

return true;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -148,9 +148,13 @@ describe('Integration | coreHandlers | handleAfterSendEvent', () => {

jest.runAllTimers();
await new Promise(process.nextTick);

// Send twice, one for the error & one right after for the session conversion
expect(mockSend).toHaveBeenCalledTimes(1);

jest.runAllTimers();
await new Promise(process.nextTick);
expect(mockSend).toHaveBeenCalledTimes(2);

// This is removed now, because it has been converted to a "session" session
expect(Array.from(replay.getContext().errorIds)).toEqual([]);
expect(replay.isEnabled()).toBe(true);
Expand Down
Loading

0 comments on commit b08fdc2

Please sign in to comment.