Skip to content

feat(replay): Stop replay when event buffer exceeds max. size #8315

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 1 commit into from
Jun 19, 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
3 changes: 3 additions & 0 deletions packages/replay/src/constants.ts
Original file line number Diff line number Diff line change
Expand Up @@ -44,3 +44,6 @@ export const SLOW_CLICK_THRESHOLD = 3_000;
export const SLOW_CLICK_SCROLL_TIMEOUT = 300;
/* Clicks in this time period are considered e.g. double/triple clicks. */
export const MULTI_CLICK_TIMEOUT = 1_000;

/** When encountering a total segment size exceeding this size, stop the replay (as we cannot properly ingest it). */
export const REPLAY_MAX_EVENT_BUFFER_SIZE = 20_000_000; // ~20MB
12 changes: 11 additions & 1 deletion packages/replay/src/eventBuffer/EventBufferArray.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
import { REPLAY_MAX_EVENT_BUFFER_SIZE } from '../constants';
import type { AddEventResult, EventBuffer, EventBufferType, RecordingEvent } from '../types';
import { timestampToMs } from '../util/timestampToMs';
import { EventBufferSizeExceededError } from '.';

/**
* A basic event buffer that does not do any compression.
Expand All @@ -8,6 +10,7 @@ import { timestampToMs } from '../util/timestampToMs';
export class EventBufferArray implements EventBuffer {
/** All the events that are buffered to be sent. */
public events: RecordingEvent[];
private _totalSize = 0;

public constructor() {
this.events = [];
Expand All @@ -30,6 +33,12 @@ export class EventBufferArray implements EventBuffer {

/** @inheritdoc */
public async addEvent(event: RecordingEvent): Promise<AddEventResult> {
const eventSize = JSON.stringify(event).length;
Copy link
Member

Choose a reason for hiding this comment

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

This seems expensive to be running on each event. This is running before we pass the event to the web worker?
Do we stringify this in the transport? Or elsewhere so we can measure at that point?

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes, in the non-worker based event buffer it adds overhead as we need to stringify as we go. In the buffer-based worker we already do that, so there it adds no overhead. If we want to keep track in any way, we will have to add overhead to the non-worker based buffer (currently, we only stringify on finish(), which is the most performant - but gives us no idea how large we are as we go).

this._totalSize += eventSize;
if (this._totalSize > REPLAY_MAX_EVENT_BUFFER_SIZE) {
throw new EventBufferSizeExceededError();
}

this.events.push(event);
}

Expand All @@ -40,14 +49,15 @@ export class EventBufferArray implements EventBuffer {
// events member so that we do not lose new events while uploading
// attachment.
const eventsRet = this.events;
this.events = [];
this.clear();
resolve(JSON.stringify(eventsRet));
});
}

/** @inheritdoc */
public clear(): void {
this.events = [];
this._totalSize = 0;
}

/** @inheritdoc */
Expand Down
18 changes: 15 additions & 3 deletions packages/replay/src/eventBuffer/EventBufferCompressionWorker.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,9 @@
import type { ReplayRecordingData } from '@sentry/types';

import { REPLAY_MAX_EVENT_BUFFER_SIZE } from '../constants';
import type { AddEventResult, EventBuffer, EventBufferType, RecordingEvent } from '../types';
import { timestampToMs } from '../util/timestampToMs';
import { EventBufferSizeExceededError } from '.';
import { WorkerHandler } from './WorkerHandler';

/**
Expand All @@ -11,6 +13,7 @@ import { WorkerHandler } from './WorkerHandler';
export class EventBufferCompressionWorker implements EventBuffer {
private _worker: WorkerHandler;
private _earliestTimestamp: number | null;
private _totalSize = 0;

public constructor(worker: Worker) {
this._worker = new WorkerHandler(worker);
Expand Down Expand Up @@ -53,7 +56,14 @@ export class EventBufferCompressionWorker implements EventBuffer {
this._earliestTimestamp = timestamp;
}

return this._sendEventToWorker(event);
const data = JSON.stringify(event);
this._totalSize += data.length;

if (this._totalSize > REPLAY_MAX_EVENT_BUFFER_SIZE) {
return Promise.reject(new EventBufferSizeExceededError());
}

return this._sendEventToWorker(data);
}

/**
Expand All @@ -66,6 +76,7 @@ export class EventBufferCompressionWorker implements EventBuffer {
/** @inheritdoc */
public clear(): void {
this._earliestTimestamp = null;
this._totalSize = 0;
// We do not wait on this, as we assume the order of messages is consistent for the worker
void this._worker.postMessage('clear');
}
Expand All @@ -78,8 +89,8 @@ export class EventBufferCompressionWorker implements EventBuffer {
/**
* Send the event to the worker.
*/
private _sendEventToWorker(event: RecordingEvent): Promise<AddEventResult> {
return this._worker.postMessage<void>('addEvent', JSON.stringify(event));
private _sendEventToWorker(data: string): Promise<AddEventResult> {
return this._worker.postMessage<void>('addEvent', data);
}

/**
Expand All @@ -89,6 +100,7 @@ export class EventBufferCompressionWorker implements EventBuffer {
const response = await this._worker.postMessage<Uint8Array>('finish');

this._earliestTimestamp = null;
this._totalSize = 0;

return response;
}
Expand Down
8 changes: 8 additions & 0 deletions packages/replay/src/eventBuffer/index.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import { getWorkerURL } from '@sentry-internal/replay-worker';
import { logger } from '@sentry/utils';

import { REPLAY_MAX_EVENT_BUFFER_SIZE } from '../constants';
import type { EventBuffer } from '../types';
import { EventBufferArray } from './EventBufferArray';
import { EventBufferProxy } from './EventBufferProxy';
Expand Down Expand Up @@ -30,3 +31,10 @@ export function createEventBuffer({ useCompression }: CreateEventBufferParams):
__DEBUG_BUILD__ && logger.log('[Replay] Using simple buffer');
return new EventBufferArray();
}

/** This error indicates that the event buffer size exceeded the limit.. */
export class EventBufferSizeExceededError extends Error {
public constructor() {
super(`Event buffer exceeded maximum size of ${REPLAY_MAX_EVENT_BUFFER_SIZE}.`);
}
}
5 changes: 4 additions & 1 deletion packages/replay/src/util/addEvent.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ import { EventType } from '@sentry-internal/rrweb';
import { getCurrentHub } from '@sentry/core';
import { logger } from '@sentry/utils';

import { EventBufferSizeExceededError } from '../eventBuffer';
import type { AddEventResult, RecordingEvent, ReplayContainer, ReplayFrameEvent } from '../types';
import { timestampToMs } from './timestampToMs';

Expand Down Expand Up @@ -56,8 +57,10 @@ export async function addEvent(

return await replay.eventBuffer.addEvent(eventAfterPossibleCallback);
} catch (error) {
const reason = error && error instanceof EventBufferSizeExceededError ? 'addEventSizeExceeded' : 'addEvent';

__DEBUG_BUILD__ && logger.error(error);
await replay.stop('addEvent');
await replay.stop(reason);

const client = getCurrentHub().getClient();

Expand Down
55 changes: 54 additions & 1 deletion packages/replay/test/unit/eventBuffer/EventBufferArray.test.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import { createEventBuffer } from './../../../src/eventBuffer';
import { REPLAY_MAX_EVENT_BUFFER_SIZE } from '../../../src/constants';
import { createEventBuffer, EventBufferSizeExceededError } from './../../../src/eventBuffer';
import { BASE_TIMESTAMP } from './../../index';

const TEST_EVENT = { data: {}, timestamp: BASE_TIMESTAMP, type: 3 };
Expand Down Expand Up @@ -44,4 +45,56 @@ describe('Unit | eventBuffer | EventBufferArray', () => {
expect(result1).toEqual(JSON.stringify([TEST_EVENT]));
expect(result2).toEqual(JSON.stringify([]));
});

describe('size limit', () => {
it('rejects if size exceeds limit', async function () {
const buffer = createEventBuffer({ useCompression: false });

const largeEvent = {
data: { a: 'a'.repeat(REPLAY_MAX_EVENT_BUFFER_SIZE / 3) },
timestamp: BASE_TIMESTAMP,
type: 3,
};

await buffer.addEvent(largeEvent);
await buffer.addEvent(largeEvent);

// Now it should error
await expect(() => buffer.addEvent(largeEvent)).rejects.toThrowError(EventBufferSizeExceededError);
});

it('resets size limit on clear', async function () {
const buffer = createEventBuffer({ useCompression: false });

const largeEvent = {
data: { a: 'a'.repeat(REPLAY_MAX_EVENT_BUFFER_SIZE / 3) },
timestamp: BASE_TIMESTAMP,
type: 3,
};

await buffer.addEvent(largeEvent);
await buffer.addEvent(largeEvent);

await buffer.clear();

await buffer.addEvent(largeEvent);
});

it('resets size limit on finish', async function () {
const buffer = createEventBuffer({ useCompression: false });

const largeEvent = {
data: { a: 'a'.repeat(REPLAY_MAX_EVENT_BUFFER_SIZE / 3) },
timestamp: BASE_TIMESTAMP,
type: 3,
};

await buffer.addEvent(largeEvent);
await buffer.addEvent(largeEvent);

await buffer.finish();

await buffer.addEvent(largeEvent);
});
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,9 @@ import 'jsdom-worker';
import pako from 'pako';

import { BASE_TIMESTAMP } from '../..';
import { REPLAY_MAX_EVENT_BUFFER_SIZE } from '../../../src/constants';
import { EventBufferProxy } from '../../../src/eventBuffer/EventBufferProxy';
import { createEventBuffer } from './../../../src/eventBuffer';
import { createEventBuffer, EventBufferSizeExceededError } from './../../../src/eventBuffer';

const TEST_EVENT = { data: {}, timestamp: BASE_TIMESTAMP, type: 3 };

Expand Down Expand Up @@ -146,4 +147,71 @@ describe('Unit | eventBuffer | EventBufferCompressionWorker', () => {

await expect(() => buffer.addEvent({ data: { o: 3 }, timestamp: BASE_TIMESTAMP, type: 3 })).rejects.toBeDefined();
});

describe('size limit', () => {
it('rejects if size exceeds limit', async function () {
const buffer = createEventBuffer({
useCompression: true,
}) as EventBufferProxy;

expect(buffer).toBeInstanceOf(EventBufferProxy);
await buffer.ensureWorkerIsLoaded();

const largeEvent = {
data: { a: 'a'.repeat(REPLAY_MAX_EVENT_BUFFER_SIZE / 3) },
timestamp: BASE_TIMESTAMP,
type: 3,
};

await buffer.addEvent(largeEvent);
await buffer.addEvent(largeEvent);

// Now it should error
await expect(() => buffer.addEvent(largeEvent)).rejects.toThrowError(EventBufferSizeExceededError);
});

it('resets size limit on clear', async function () {
const buffer = createEventBuffer({
useCompression: true,
}) as EventBufferProxy;

expect(buffer).toBeInstanceOf(EventBufferProxy);
await buffer.ensureWorkerIsLoaded();

const largeEvent = {
data: { a: 'a'.repeat(REPLAY_MAX_EVENT_BUFFER_SIZE / 3) },
timestamp: BASE_TIMESTAMP,
type: 3,
};

await buffer.addEvent(largeEvent);
await buffer.addEvent(largeEvent);

await buffer.clear();

await buffer.addEvent(largeEvent);
});

it('resets size limit on finish', async function () {
const buffer = createEventBuffer({
useCompression: true,
}) as EventBufferProxy;

expect(buffer).toBeInstanceOf(EventBufferProxy);
await buffer.ensureWorkerIsLoaded();

const largeEvent = {
data: { a: 'a'.repeat(REPLAY_MAX_EVENT_BUFFER_SIZE / 3) },
timestamp: BASE_TIMESTAMP,
type: 3,
};

await buffer.addEvent(largeEvent);
await buffer.addEvent(largeEvent);

await buffer.finish();

await buffer.addEvent(largeEvent);
});
});
});
28 changes: 28 additions & 0 deletions packages/replay/test/unit/util/addEvent.test.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import 'jsdom-worker';

import { BASE_TIMESTAMP } from '../..';
import { REPLAY_MAX_EVENT_BUFFER_SIZE } from '../../../src/constants';
import type { EventBufferProxy } from '../../../src/eventBuffer/EventBufferProxy';
import { addEvent } from '../../../src/util/addEvent';
import { setupReplayContainer } from '../../utils/setupReplayContainer';
Expand Down Expand Up @@ -29,4 +30,31 @@ describe('Unit | util | addEvent', () => {

expect(replay.isEnabled()).toEqual(false);
});

it('stops when exceeding buffer size limit', async function () {
jest.setSystemTime(BASE_TIMESTAMP);

const replay = setupReplayContainer({
options: {
useCompression: true,
},
});

const largeEvent = {
data: { a: 'a'.repeat(REPLAY_MAX_EVENT_BUFFER_SIZE / 3) },
timestamp: BASE_TIMESTAMP,
type: 3,
};

await (replay.eventBuffer as EventBufferProxy).ensureWorkerIsLoaded();

await addEvent(replay, largeEvent);
await addEvent(replay, largeEvent);

expect(replay.isEnabled()).toEqual(true);

await addEvent(replay, largeEvent);

expect(replay.isEnabled()).toEqual(false);
});
});