Skip to content

fix(replay): Ignore max session life for buffered sessions #8258

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 6, 2023
Merged

fix(replay): Ignore max session life for buffered sessions #8258

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
88781b1
fix(replay): Ignore max session life for buffered sessions
billyvg May 30, 2023
13cfd6d
add/update tests + comments
billyvg Jun 1, 2023
e6876bc
lint
billyvg Jun 1, 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
11 changes: 10 additions & 1 deletion packages/replay/src/replay.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -417,6 +417,15 @@ export class ReplayContainer implements ReplayContainerInterface {
// Once this session ends, we do not want to refresh it
if (this.session) {
this.session.shouldRefresh = false;

// It's possible that the session lifespan is > max session lifespan
// because we have been buffering (which ignores expiration given that
// `shouldRefresh` is true). Since we flip `shouldRefresh`, the session
// could be considered expired due to lifespan. Update session start date
Copy link
Member

@mydea mydea May 31, 2023

Choose a reason for hiding this comment

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

contrary to the comment, session start date is always current date - which should be correct? 😅

Copy link
Member Author

@billyvg billyvg May 31, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

Oops, yeah it should be current timestamp

edit nope, now thinking about it, we should peek into the buffer for earliest timestamp.

Copy link
Member Author

@billyvg billyvg Jun 1, 2023

Choose a reason for hiding this comment

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

As an update we are going to use the current timestamp as it is a bit more straightforward (especially when it comes to tests, as we create some unrealistic scenarios in tests as we do not replicate the "checkoutEvery" behavior in tests). This means that we can have sessions that are a bit longer than MAX_SESSION_LIFE (only 30-60s longer, i.e. the length of buffer before an error occurs).

// to the earliest event in buffer, or current timestamp.
this._updateUserActivity();
this._updateSessionActivity();
this.session.started = Date.now();
this._maybeSaveSession();
}

Expand Down Expand Up @@ -657,7 +666,7 @@ export class ReplayContainer implements ReplayContainerInterface {
stickySession: Boolean(this._options.stickySession),
currentSession: this.session,
sessionSampleRate: this._options.sessionSampleRate,
allowBuffering: this._options.errorSampleRate > 0,
allowBuffering: this._options.errorSampleRate > 0 || this.recordingMode === 'buffer',
});

// If session was newly created (i.e. was not loaded from storage), then
Expand Down
8 changes: 5 additions & 3 deletions packages/replay/src/session/getSession.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,11 +34,13 @@ export function getSession({
// within "max session time").
const isExpired = isSessionExpired(session, timeouts);

if (!isExpired) {
if (!isExpired || (allowBuffering && session.shouldRefresh)) {
return { type: 'saved', session };
} else if (!session.shouldRefresh) {
// In this case, stop
// This is the case if we have an error session that is completed (=triggered an error)
// This is the case if we have an error session that is completed
// (=triggered an error). Session will continue as session-based replay,
// and when this session is expired, it will not be renewed until user
// reloads.
const discardedSession = makeSession({ sampled: false });
return { type: 'new', session: discardedSession };
} else {
Expand Down
52 changes: 36 additions & 16 deletions packages/replay/test/integration/errorSampleRate-delayFlush.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -600,7 +600,7 @@ describe('Integration | errorSampleRate with delayed flush', () => {

await waitForBufferFlush();

expect(replay.session?.started).toBe(BASE_TIMESTAMP + ELAPSED + 20);
expect(replay.session?.started).toBe(BASE_TIMESTAMP + ELAPSED + DEFAULT_FLUSH_MIN_DELAY + 80);

// Does not capture mouse click
expect(replay).toHaveSentReplay({
Expand Down Expand Up @@ -662,7 +662,8 @@ describe('Integration | errorSampleRate with delayed flush', () => {
expect(replay.isEnabled()).toBe(false);
});

it('stops replay when session exceeds max length', async () => {
it('stops replay when session exceeds max length after latest captured error', async () => {
const sessionId = replay.session?.id;
jest.setSystemTime(BASE_TIMESTAMP);

const TEST_EVENT = { data: {}, timestamp: BASE_TIMESTAMP, type: 3 };
Expand All @@ -674,38 +675,57 @@ describe('Integration | errorSampleRate with delayed flush', () => {
jest.runAllTimers();
await new Promise(process.nextTick);

captureException(new Error('testing'));
jest.advanceTimersByTime(2 * MAX_SESSION_LIFE);

jest.advanceTimersByTime(DEFAULT_FLUSH_MIN_DELAY);
await new Promise(process.nextTick);

expect(replay).not.toHaveLastSentReplay();
captureException(new Error('testing'));

// Wait a bit, shortly before session expires
jest.advanceTimersByTime(MAX_SESSION_LIFE - 1000);
// Flush due to exception
await new Promise(process.nextTick);
await waitForFlush();

mockRecord._emitter(TEST_EVENT);
replay.triggerUserActivity();
expect(replay.session?.id).toBe(sessionId);
expect(replay).toHaveLastSentReplay({
recordingPayloadHeader: { segment_id: 0 },
});

expect(replay).toHaveLastSentReplay();
// This comes from `startRecording()` in `sendBufferedReplayOrFlush()`
await waitForFlush();
expect(replay).toHaveLastSentReplay({
recordingPayloadHeader: { segment_id: 1 },
recordingData: JSON.stringify([
{
data: {
isCheckout: true,
},
timestamp: BASE_TIMESTAMP + 2 * MAX_SESSION_LIFE + DEFAULT_FLUSH_MIN_DELAY + 40,
type: 2,
},
]),
});

// Now wait after session expires - should stop recording
mockRecord.takeFullSnapshot.mockClear();
(getCurrentHub().getClient()!.getTransport()!.send as unknown as jest.SpyInstance<any>).mockClear();

jest.advanceTimersByTime(10_000);
jest.advanceTimersByTime(MAX_SESSION_LIFE);
await new Promise(process.nextTick);

mockRecord._emitter(TEST_EVENT);
replay.triggerUserActivity();

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

expect(replay).not.toHaveLastSentReplay();
expect(mockRecord.takeFullSnapshot).toHaveBeenCalledTimes(0);
expect(replay.isEnabled()).toBe(false);

// Once the session is stopped after capturing a replay already
// (buffer-mode), another error will not trigger a new replay
captureException(new Error('testing'));

await new Promise(process.nextTick);
jest.advanceTimersByTime(DEFAULT_FLUSH_MIN_DELAY);
await new Promise(process.nextTick);
expect(replay).not.toHaveLastSentReplay();
});
});

Expand Down
67 changes: 49 additions & 18 deletions packages/replay/test/integration/errorSampleRate.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -432,6 +432,9 @@ describe('Integration | errorSampleRate', () => {
['MAX_SESSION_LIFE', MAX_SESSION_LIFE],
['SESSION_IDLE_EXPIRE_DURATION', SESSION_IDLE_EXPIRE_DURATION],
])('continues buffering replay if session had no error and exceeds %s', async (_label, waitTime) => {
const oldSessionId = replay.session?.id;
expect(oldSessionId).toBeDefined();

expect(replay).not.toHaveLastSentReplay();

// Idle for given time
Expand Down Expand Up @@ -475,13 +478,24 @@ describe('Integration | errorSampleRate', () => {
jest.advanceTimersByTime(DEFAULT_FLUSH_MIN_DELAY);
await new Promise(process.nextTick);

expect(replay).toHaveLastSentReplay({
expect(replay.session?.id).toBe(oldSessionId);

// Flush of buffered events
expect(replay).toHaveSentReplay({
recordingPayloadHeader: { segment_id: 0 },
replayEventPayload: expect.objectContaining({
replay_type: 'buffer',
}),
});

// Checkout from `startRecording`
expect(replay).toHaveLastSentReplay({
recordingPayloadHeader: { segment_id: 1 },
replayEventPayload: expect.objectContaining({
replay_type: 'buffer',
}),
});

expect(replay.isEnabled()).toBe(true);
expect(replay.isPaused()).toBe(false);
expect(replay.recordingMode).toBe('session');
Expand All @@ -491,6 +505,9 @@ describe('Integration | errorSampleRate', () => {

// Should behave the same as above test
it('stops replay if user has been idle for more than SESSION_IDLE_EXPIRE_DURATION and does not start a new session thereafter', async () => {
const oldSessionId = replay.session?.id;
expect(oldSessionId).toBeDefined();

// Idle for 15 minutes
jest.advanceTimersByTime(SESSION_IDLE_EXPIRE_DURATION + 1);

Expand All @@ -517,14 +534,24 @@ describe('Integration | errorSampleRate', () => {
await new Promise(process.nextTick);
jest.advanceTimersByTime(DEFAULT_FLUSH_MIN_DELAY);
await new Promise(process.nextTick);
expect(replay.session?.id).toBe(oldSessionId);

expect(replay).toHaveLastSentReplay({
// buffered events
expect(replay).toHaveSentReplay({
recordingPayloadHeader: { segment_id: 0 },
replayEventPayload: expect.objectContaining({
replay_type: 'buffer',
}),
});

// `startRecording` full checkout
expect(replay).toHaveLastSentReplay({
recordingPayloadHeader: { segment_id: 1 },
replayEventPayload: expect.objectContaining({
replay_type: 'buffer',
}),
});

expect(replay.isEnabled()).toBe(true);
expect(replay.isPaused()).toBe(false);
expect(replay.recordingMode).toBe('session');
Expand Down Expand Up @@ -605,7 +632,7 @@ describe('Integration | errorSampleRate', () => {
jest.advanceTimersByTime(20);
await new Promise(process.nextTick);

expect(replay.session?.started).toBe(BASE_TIMESTAMP + ELAPSED + 20);
expect(replay.session?.started).toBe(BASE_TIMESTAMP + ELAPSED + 100);

// Does not capture mouse click
expect(replay).toHaveSentReplay({
Expand Down Expand Up @@ -667,7 +694,8 @@ describe('Integration | errorSampleRate', () => {
expect(replay.isEnabled()).toBe(false);
});

it('stops replay when session exceeds max length', async () => {
it('stops replay when session exceeds max length after latest captured error', async () => {
const sessionId = replay.session?.id;
jest.setSystemTime(BASE_TIMESTAMP);

const TEST_EVENT = { data: {}, timestamp: BASE_TIMESTAMP, type: 3 };
Expand All @@ -679,37 +707,40 @@ describe('Integration | errorSampleRate', () => {
jest.runAllTimers();
await new Promise(process.nextTick);

jest.advanceTimersByTime(2 * MAX_SESSION_LIFE);

captureException(new Error('testing'));

jest.advanceTimersByTime(DEFAULT_FLUSH_MIN_DELAY);
// Flush due to exception
await new Promise(process.nextTick);
expect(replay).not.toHaveLastSentReplay();

// Wait a bit, shortly before session expires
jest.advanceTimersByTime(MAX_SESSION_LIFE - 1000);
jest.advanceTimersByTime(DEFAULT_FLUSH_MIN_DELAY);
await new Promise(process.nextTick);

mockRecord._emitter(TEST_EVENT);
replay.triggerUserActivity();

expect(replay.session?.id).toBe(sessionId);
expect(replay).toHaveLastSentReplay();

// Now wait after session expires - should stop recording
// Now wait after session expires - should re-start into buffering mode
mockRecord.takeFullSnapshot.mockClear();
(getCurrentHub().getClient()!.getTransport()!.send as unknown as jest.SpyInstance<any>).mockClear();

jest.advanceTimersByTime(10_000);
jest.advanceTimersByTime(MAX_SESSION_LIFE);
await new Promise(process.nextTick);

mockRecord._emitter(TEST_EVENT);
replay.triggerUserActivity();

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

expect(replay).not.toHaveLastSentReplay();
expect(mockRecord.takeFullSnapshot).toHaveBeenCalledTimes(0);
expect(replay.isEnabled()).toBe(false);

// Once the session is stopped after capturing a replay already
// (buffer-mode), another error should trigger a new replay
captureException(new Error('testing'));

await new Promise(process.nextTick);
jest.advanceTimersByTime(DEFAULT_FLUSH_MIN_DELAY);
await new Promise(process.nextTick);
expect(replay).not.toHaveLastSentReplay();
});
});

Expand Down
59 changes: 59 additions & 0 deletions packages/replay/test/unit/session/getSession.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -229,4 +229,63 @@ describe('Unit | session | getSession', () => {
expect(session.id).toBe('test_session_uuid_2');
expect(session.segmentId).toBe(0);
});

it('re-uses the same "buffer" session if it is expired and has never sent a buffered replay', function () {
const { type, session } = getSession({
timeouts: {
sessionIdlePause: SESSION_IDLE_PAUSE_DURATION,
sessionIdleExpire: 1000,
maxSessionLife: MAX_SESSION_LIFE,
},
stickySession: false,
...SAMPLE_OPTIONS,
currentSession: makeSession({
id: 'test_session_uuid_2',
lastActivity: +new Date() - MAX_SESSION_LIFE - 1,
started: +new Date() - MAX_SESSION_LIFE - 1,
segmentId: 0,
sampled: 'buffer',
}),
allowBuffering: true,
});

expect(FetchSession.fetchSession).not.toHaveBeenCalled();
expect(CreateSession.createSession).not.toHaveBeenCalled();

expect(type).toBe('saved');
expect(session.id).toBe('test_session_uuid_2');
expect(session.sampled).toBe('buffer');
expect(session.segmentId).toBe(0);
});

it('creates a new session if it is expired and it was a "buffer" session that has sent a replay', function () {
const currentSession = makeSession({
id: 'test_session_uuid_2',
lastActivity: +new Date() - MAX_SESSION_LIFE - 1,
started: +new Date() - MAX_SESSION_LIFE - 1,
segmentId: 0,
sampled: 'buffer',
});
currentSession.shouldRefresh = false;

const { type, session } = getSession({
timeouts: {
sessionIdlePause: SESSION_IDLE_PAUSE_DURATION,
sessionIdleExpire: 1000,
maxSessionLife: MAX_SESSION_LIFE,
},
stickySession: false,
...SAMPLE_OPTIONS,
currentSession,
allowBuffering: true,
});

expect(FetchSession.fetchSession).not.toHaveBeenCalled();
expect(CreateSession.createSession).not.toHaveBeenCalled();

expect(type).toBe('new');
expect(session.id).not.toBe('test_session_uuid_2');
expect(session.sampled).toBe(false);
expect(session.segmentId).toBe(0);
});
});