Skip to content

feat(replay): Allow to configure maxReplayDuration #8769

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
merged 3 commits into from
Aug 31, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
Original file line number Diff line number Diff line change
Expand Up @@ -19,5 +19,4 @@ Sentry.init({
return event;
},
integrations: [window.Replay],
debug: true,
});
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,6 @@ Sentry.init({
sampleRate: 0,
replaysSessionSampleRate: 1.0,
replaysOnErrorSampleRate: 0.0,
debug: true,

integrations: [window.Replay],
});
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ window.Replay = new Sentry.Replay({
flushMinDelay: 200,
flushMaxDelay: 200,
minReplayDuration: 0,
maxReplayDuration: 2000,
});

Sentry.init({
Expand All @@ -19,5 +20,4 @@ Sentry.init({
window.Replay._replay.timeouts = {
sessionIdlePause: 1000, // this is usually 5min, but we want to test this with shorter times
sessionIdleExpire: 2000, // this is usually 15min, but we want to test this with shorter times
maxSessionLife: 2000, // default: 60min
};
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ import { sentryTest } from '../../../utils/fixtures';
import { getExpectedReplayEvent } from '../../../utils/replayEventTemplates';
import { getReplayEvent, shouldSkipReplayTest, waitForReplayRequest } from '../../../utils/replayHelpers';

const SESSION_MAX_AGE = 2000;
const MAX_REPLAY_DURATION = 2000;

sentryTest('keeps track of max duration across reloads', async ({ getLocalTestPath, page }) => {
if (shouldSkipReplayTest()) {
Expand All @@ -26,15 +26,15 @@ sentryTest('keeps track of max duration across reloads', async ({ getLocalTestPa

await page.goto(url);

await new Promise(resolve => setTimeout(resolve, SESSION_MAX_AGE / 2));
await new Promise(resolve => setTimeout(resolve, MAX_REPLAY_DURATION / 2));

await page.reload();
await page.click('#button1');

// After the second reload, we should have a new session (because we exceeded max age)
const reqPromise3 = waitForReplayRequest(page, 0);

await new Promise(resolve => setTimeout(resolve, SESSION_MAX_AGE / 2 + 100));
await new Promise(resolve => setTimeout(resolve, MAX_REPLAY_DURATION / 2 + 100));

void page.click('#button1');
await page.evaluate(`Object.defineProperty(document, 'visibilityState', {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,6 @@ Sentry.init({
sampleRate: 0,
replaysSessionSampleRate: 1.0,
replaysOnErrorSampleRate: 0.0,
debug: true,

integrations: [window.Replay],
});
Original file line number Diff line number Diff line change
Expand Up @@ -12,13 +12,11 @@ Sentry.init({
sampleRate: 0,
replaysSessionSampleRate: 1.0,
replaysOnErrorSampleRate: 0.0,
debug: true,

integrations: [window.Replay],
});

window.Replay._replay.timeouts = {
sessionIdlePause: 1000, // this is usually 5min, but we want to test this with shorter times
sessionIdleExpire: 2000, // this is usually 15min, but we want to test this with shorter times
maxSessionLife: 3600000, // default: 60min
};
Original file line number Diff line number Diff line change
Expand Up @@ -12,13 +12,11 @@ Sentry.init({
sampleRate: 0,
replaysSessionSampleRate: 1.0,
replaysOnErrorSampleRate: 0.0,
debug: true,

integrations: [window.Replay],
});

window.Replay._replay.timeouts = {
sessionIdlePause: 1000, // this is usually 5min, but we want to test this with shorter times
sessionIdleExpire: 900000, // defayult: 15min
maxSessionLife: 3600000, // default: 60min
};
Original file line number Diff line number Diff line change
Expand Up @@ -5,20 +5,19 @@ window.Replay = new Sentry.Replay({
flushMinDelay: 200,
flushMaxDelay: 200,
minReplayDuration: 0,
maxReplayDuration: 4000,
});

Sentry.init({
dsn: 'https://[email protected]/1337',
sampleRate: 0,
replaysSessionSampleRate: 1.0,
replaysOnErrorSampleRate: 0.0,
debug: true,

integrations: [window.Replay],
});

window.Replay._replay.timeouts = {
sessionIdlePause: 300000, // default: 5min
sessionIdleExpire: 900000, // default: 15min
maxSessionLife: 4000, // this is usually 60min, but we want to test this with shorter times
};
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ import {
} from '../../../utils/replayHelpers';

// Session should be max. 4s long
const SESSION_MAX_AGE = 4000;
const MAX_REPLAY_DURATION = 4000;

/*
The main difference between this and sessionExpiry test, is that here we wait for the overall time (4s)
Expand Down Expand Up @@ -58,7 +58,7 @@ sentryTest('handles session that exceeds max age', async ({ getLocalTestPath, pa
// Wait for an incremental snapshot
// Wait half of the session max age (after initial flush), but account for potentially slow runners
const timePassed1 = Date.now() - startTimestamp;
await new Promise(resolve => setTimeout(resolve, Math.max(SESSION_MAX_AGE / 2 - timePassed1, 0)));
await new Promise(resolve => setTimeout(resolve, Math.max(MAX_REPLAY_DURATION / 2 - timePassed1, 0)));
await page.click('#button1');

const req1 = await reqPromise1;
Expand All @@ -71,7 +71,7 @@ sentryTest('handles session that exceeds max age', async ({ getLocalTestPath, pa

// Wait for session to expire
const timePassed2 = Date.now() - startTimestamp;
await new Promise(resolve => setTimeout(resolve, Math.max(SESSION_MAX_AGE - timePassed2, 0)));
await new Promise(resolve => setTimeout(resolve, Math.max(MAX_REPLAY_DURATION - timePassed2, 0)));
await page.click('#button2');

const req2 = await reqPromise2;
Expand Down
6 changes: 3 additions & 3 deletions packages/replay/src/constants.ts
Original file line number Diff line number Diff line change
Expand Up @@ -17,9 +17,6 @@ export const SESSION_IDLE_PAUSE_DURATION = 300_000; // 5 minutes in ms
// The idle limit for a session after which the session expires.
export const SESSION_IDLE_EXPIRE_DURATION = 900_000; // 15 minutes in ms

// The maximum length of a session
export const MAX_SESSION_LIFE = 3_600_000; // 60 minutes in ms

/** Default flush delays */
export const DEFAULT_FLUSH_MIN_DELAY = 5_000;
// XXX: Temp fix for our debounce logic where `maxWait` would never occur if it
Expand Down Expand Up @@ -50,3 +47,6 @@ export const REPLAY_MAX_EVENT_BUFFER_SIZE = 20_000_000; // ~20MB
export const MIN_REPLAY_DURATION = 4_999;
/* The max. allowed value that the minReplayDuration can be set to. */
export const MIN_REPLAY_DURATION_LIMIT = 15_000;

/** The max. length of a replay. */
export const MAX_REPLAY_DURATION = 3_600_000; // 60 minutes in ms;
3 changes: 3 additions & 0 deletions packages/replay/src/integration.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import { dropUndefinedKeys } from '@sentry/utils';
import {
DEFAULT_FLUSH_MAX_DELAY,
DEFAULT_FLUSH_MIN_DELAY,
MAX_REPLAY_DURATION,
MIN_REPLAY_DURATION,
MIN_REPLAY_DURATION_LIMIT,
} from './constants';
Expand Down Expand Up @@ -57,6 +58,7 @@ export class Replay implements Integration {
flushMinDelay = DEFAULT_FLUSH_MIN_DELAY,
flushMaxDelay = DEFAULT_FLUSH_MAX_DELAY,
minReplayDuration = MIN_REPLAY_DURATION,
maxReplayDuration = MAX_REPLAY_DURATION,
stickySession = true,
useCompression = true,
_experiments = {},
Expand Down Expand Up @@ -136,6 +138,7 @@ export class Replay implements Integration {
flushMinDelay,
flushMaxDelay,
minReplayDuration: Math.min(minReplayDuration, MIN_REPLAY_DURATION_LIMIT),
maxReplayDuration: Math.min(maxReplayDuration, MAX_REPLAY_DURATION),
stickySession,
sessionSampleRate,
errorSampleRate,
Expand Down
27 changes: 16 additions & 11 deletions packages/replay/src/replay.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@ import { logger } from '@sentry/utils';

import {
BUFFER_CHECKOUT_TIME,
MAX_SESSION_LIFE,
SESSION_IDLE_EXPIRE_DURATION,
SESSION_IDLE_PAUSE_DURATION,
SLOW_CLICK_SCROLL_TIMEOUT,
Expand Down Expand Up @@ -150,7 +149,6 @@ export class ReplayContainer implements ReplayContainerInterface {
this.timeouts = {
sessionIdlePause: SESSION_IDLE_PAUSE_DURATION,
sessionIdleExpire: SESSION_IDLE_EXPIRE_DURATION,
maxSessionLife: MAX_SESSION_LIFE,
Copy link
Member

Choose a reason for hiding this comment

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

Why not keep the max duration part of timeouts?

Copy link
Member Author

Choose a reason for hiding this comment

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

🤷 just aligning this - the only reason these timeouts exist was to allow overwriting them in tests, which can now be done "correctly"/publicly via maxReplayDuration?

} as const;
this._lastActivity = Date.now();
this._isEnabled = false;
Expand Down Expand Up @@ -277,7 +275,8 @@ export class ReplayContainer implements ReplayContainerInterface {
const session = loadOrCreateSession(
this.session,
{
timeouts: this.timeouts,
maxReplayDuration: this._options.maxReplayDuration,
sessionIdleExpire: this.timeouts.sessionIdleExpire,
traceInternals: this._options._experiments.traceInternals,
},
{
Expand Down Expand Up @@ -307,7 +306,8 @@ export class ReplayContainer implements ReplayContainerInterface {
const session = loadOrCreateSession(
this.session,
{
timeouts: this.timeouts,
sessionIdleExpire: this.timeouts.sessionIdleExpire,
maxReplayDuration: this._options.maxReplayDuration,
traceInternals: this._options._experiments.traceInternals,
},
{
Expand Down Expand Up @@ -483,7 +483,7 @@ export class ReplayContainer implements ReplayContainerInterface {
// `shouldRefresh`, the session could be considered expired due to
// lifespan, which is not what we want. Update session start date to be
// the current timestamp, so that session is not considered to be
// expired. This means that max replay duration can be MAX_SESSION_LIFE +
// expired. This means that max replay duration can be MAX_REPLAY_DURATION +
// (length of buffer), which we are ok with.
this._updateUserActivity(activityTime);
this._updateSessionActivity(activityTime);
Expand Down Expand Up @@ -764,7 +764,8 @@ export class ReplayContainer implements ReplayContainerInterface {
const session = loadOrCreateSession(
this.session,
{
timeouts: this.timeouts,
sessionIdleExpire: this.timeouts.sessionIdleExpire,
maxReplayDuration: this._options.maxReplayDuration,
traceInternals: this._options._experiments.traceInternals,
},
{
Expand Down Expand Up @@ -793,8 +794,9 @@ export class ReplayContainer implements ReplayContainerInterface {
const newSession = maybeRefreshSession(
currentSession,
{
timeouts: this.timeouts,
sessionIdleExpire: this.timeouts.sessionIdleExpire,
traceInternals: this._options._experiments.traceInternals,
maxReplayDuration: this._options.maxReplayDuration,
},
{
stickySession: Boolean(this._options.stickySession),
Expand Down Expand Up @@ -929,7 +931,10 @@ export class ReplayContainer implements ReplayContainerInterface {
return;
}

const expired = isSessionExpired(this.session, this.timeouts);
const expired = isSessionExpired(this.session, {
maxReplayDuration: this._options.maxReplayDuration,
...this.timeouts,
});

if (breadcrumb && !expired) {
this._createCustomBreadcrumb(breadcrumb);
Expand Down Expand Up @@ -1108,7 +1113,7 @@ export class ReplayContainer implements ReplayContainerInterface {
// Check total duration again, to avoid sending outdated stuff
// We leave 30s wiggle room to accomodate late flushing etc.
// This _could_ happen when the browser is suspended during flushing, in which case we just want to stop
if (timestamp - this._context.initialTimestamp > this.timeouts.maxSessionLife + 30_000) {
if (timestamp - this._context.initialTimestamp > this._options.maxReplayDuration + 30_000) {
throw new Error('Session is too long, not sending replay');
}

Expand Down Expand Up @@ -1181,10 +1186,10 @@ export class ReplayContainer implements ReplayContainerInterface {
// A flush is about to happen, cancel any queued flushes
this._debouncedFlush.cancel();

// If session is too short, or too long (allow some wiggle room over maxSessionLife), do not send it
// If session is too short, or too long (allow some wiggle room over maxReplayDuration), do not send it
// This _should_ not happen, but it may happen if flush is triggered due to a page activity change or similar
const tooShort = duration < this._options.minReplayDuration;
const tooLong = duration > this.timeouts.maxSessionLife + 5_000;
const tooLong = duration > this._options.maxReplayDuration + 5_000;
if (tooShort || tooLong) {
logInfo(
`[Replay] Session duration (${Math.floor(duration / 1000)}s) is too ${
Expand Down
10 changes: 6 additions & 4 deletions packages/replay/src/session/loadOrCreateSession.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import type { Session, SessionOptions, Timeouts } from '../types';
import type { Session, SessionOptions } from '../types';
import { logInfoNextTick } from '../util/log';
import { createSession } from './createSession';
import { fetchSession } from './fetchSession';
Expand All @@ -11,10 +11,12 @@ import { maybeRefreshSession } from './maybeRefreshSession';
export function loadOrCreateSession(
currentSession: Session | undefined,
{
timeouts,
traceInternals,
sessionIdleExpire,
maxReplayDuration,
}: {
timeouts: Timeouts;
sessionIdleExpire: number;
maxReplayDuration: number;
traceInternals?: boolean;
},
sessionOptions: SessionOptions,
Expand All @@ -28,5 +30,5 @@ export function loadOrCreateSession(
return createSession(sessionOptions);
}

return maybeRefreshSession(existingSession, { timeouts, traceInternals }, sessionOptions);
return maybeRefreshSession(existingSession, { sessionIdleExpire, traceInternals, maxReplayDuration }, sessionOptions);
}
10 changes: 6 additions & 4 deletions packages/replay/src/session/maybeRefreshSession.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import type { Session, SessionOptions, Timeouts } from '../types';
import type { Session, SessionOptions } from '../types';
import { isSessionExpired } from '../util/isSessionExpired';
import { logInfoNextTick } from '../util/log';
import { createSession } from './createSession';
Expand All @@ -12,16 +12,18 @@ import { makeSession } from './Session';
export function maybeRefreshSession(
session: Session,
{
timeouts,
traceInternals,
maxReplayDuration,
sessionIdleExpire,
}: {
timeouts: Timeouts;
sessionIdleExpire: number;
maxReplayDuration: number;
traceInternals?: boolean;
},
sessionOptions: SessionOptions,
): Session {
// If not expired, all good, just keep the session
if (!isSessionExpired(session, timeouts)) {
if (!isSessionExpired(session, { sessionIdleExpire, maxReplayDuration })) {
return session;
}

Expand Down
7 changes: 6 additions & 1 deletion packages/replay/src/types/replay.ts
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,6 @@ export interface SendReplayData {
export interface Timeouts {
sessionIdlePause: number;
sessionIdleExpire: number;
maxSessionLife: number;
}

/**
Expand Down Expand Up @@ -187,6 +186,12 @@ export interface ReplayPluginOptions extends ReplayNetworkOptions {
*/
minReplayDuration: number;

/**
* The max. duration (in ms) a replay session may be.
* This is capped at max. 60min.
*/
maxReplayDuration: number;

/**
* Callback before adding a custom recording event
*
Expand Down
4 changes: 2 additions & 2 deletions packages/replay/src/util/addEvent.ts
Original file line number Diff line number Diff line change
Expand Up @@ -41,9 +41,9 @@ export async function addEvent(
}

// Throw out events that are +60min from the initial timestamp
if (timestampInMs > replay.getContext().initialTimestamp + replay.timeouts.maxSessionLife) {
if (timestampInMs > replay.getContext().initialTimestamp + replay.getOptions().maxReplayDuration) {
logInfo(
`[Replay] Skipping event with timestamp ${timestampInMs} because it is after maxSessionLife`,
`[Replay] Skipping event with timestamp ${timestampInMs} because it is after maxReplayDuration`,
replay.getOptions()._experiments.traceInternals,
);
return null;
Expand Down
15 changes: 11 additions & 4 deletions packages/replay/src/util/isSessionExpired.ts
Original file line number Diff line number Diff line change
@@ -1,15 +1,22 @@
import type { Session, Timeouts } from '../types';
import type { Session } from '../types';
import { isExpired } from './isExpired';

/**
* Checks to see if session is expired
*/
export function isSessionExpired(session: Session, timeouts: Timeouts, targetTime: number = +new Date()): boolean {
export function isSessionExpired(
session: Session,
{
maxReplayDuration,
sessionIdleExpire,
targetTime = Date.now(),
}: { maxReplayDuration: number; sessionIdleExpire: number; targetTime?: number },
): boolean {
return (
// First, check that maximum session length has not been exceeded
isExpired(session.started, timeouts.maxSessionLife, targetTime) ||
isExpired(session.started, maxReplayDuration, targetTime) ||
// check that the idle timeout has not been exceeded (i.e. user has
// performed an action within the last `sessionIdleExpire` ms)
isExpired(session.lastActivity, timeouts.sessionIdleExpire, targetTime)
isExpired(session.lastActivity, sessionIdleExpire, targetTime)
);
}
Loading