Skip to content

Commit

Permalink
fix(replay): Ignore old events when manually starting replay (#12349)
Browse files Browse the repository at this point in the history
This PR ensures that if a replay is manually started (=no sample rates
are defined at all, and a user later calls `start()` or
`startBuffering()`, we do not back-port the `initialTimestamp` of the
replay based on the event buffer.

By default (for the first segment) we'll backport the `initialTimestamp`
to the time of the first event in the event buffer, to ensure that e.g.
the pageload browser metrics that may be emitted with an earlier
timestamp all show up correctly.

However, this may be unexpected if manually calling `startBuffering()`
and seeing things for stuff that happened before.
Now, we keep track of this and adjust the behavior accordingly.

Fixes #11984

---------

Co-authored-by: Catherine Lee <55311782+c298lee@users.noreply.github.com>
  • Loading branch information
mydea and c298lee committed Jun 6, 2024
1 parent 302f149 commit 4009e3f
Show file tree
Hide file tree
Showing 2 changed files with 119 additions and 2 deletions.
14 changes: 12 additions & 2 deletions packages/replay-internal/src/replay.ts
Original file line number Diff line number Diff line change
Expand Up @@ -100,6 +100,9 @@ export class ReplayContainer implements ReplayContainerInterface {
*/
public readonly timeouts: Timeouts;

/** The replay has to be manually started, because no sample rate (neither session or error) was provided. */
private _requiresManualStart: boolean;

private _throttledAddEvent: (
event: RecordingEvent,
isCheckout?: boolean,
Expand Down Expand Up @@ -170,6 +173,7 @@ export class ReplayContainer implements ReplayContainerInterface {
this._lastActivity = Date.now();
this._isEnabled = false;
this._isPaused = false;
this._requiresManualStart = false;
this._hasInitializedCoreListeners = false;
this._context = {
errorIds: new Set(),
Expand Down Expand Up @@ -246,7 +250,11 @@ export class ReplayContainer implements ReplayContainerInterface {

// If neither sample rate is > 0, then do nothing - user will need to call one of
// `start()` or `startBuffering` themselves.
if (errorSampleRate <= 0 && sessionSampleRate <= 0) {
const requiresManualStart = errorSampleRate <= 0 && sessionSampleRate <= 0;

this._requiresManualStart = requiresManualStart;

if (requiresManualStart) {
return;
}

Expand Down Expand Up @@ -1049,7 +1057,9 @@ export class ReplayContainer implements ReplayContainerInterface {
/** Update the initial timestamp based on the buffer content. */
private _updateInitialTimestampFromEventBuffer(): void {
const { session, eventBuffer } = this;
if (!session || !eventBuffer) {
// If replay was started manually (=no sample rate was given),
// We do not want to back-port the initial timestamp
if (!session || !eventBuffer || this._requiresManualStart) {
return;
}

Expand Down
107 changes: 107 additions & 0 deletions packages/replay-internal/test/integration/earlyEvents.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,107 @@
import { BASE_TIMESTAMP } from '..';
import { resetSdkMock } from '../mocks/resetSdkMock';
import { useFakeTimers } from '../utils/use-fake-timers';

useFakeTimers();

describe('Integration | early events', () => {
beforeEach(() => {
vi.clearAllMocks();
});

it('updates initialTimestamp for early performance entries', async () => {
const earlyTimeStampSeconds = BASE_TIMESTAMP / 1000 - 10;

const { replay } = await resetSdkMock({
replayOptions: {
stickySession: true,
},
sentryOptions: {
replaysSessionSampleRate: 0,
replaysOnErrorSampleRate: 1.0,
},
});

expect(replay.session).toBeDefined();
expect(replay['_requiresManualStart']).toBe(false);

const initialTimestamp = replay.getContext().initialTimestamp;

expect(initialTimestamp).not.toEqual(earlyTimeStampSeconds * 1000);

// A performance entry that happend before should not extend the session when we manually started
replay.replayPerformanceEntries.push({
type: 'largest-contentful-paint',
name: 'largest-contentful-paint',
start: earlyTimeStampSeconds,
end: earlyTimeStampSeconds,
data: {
value: 100,
size: 100,
nodeId: undefined,
},
});

// _too_ early events are always thrown away
replay.replayPerformanceEntries.push({
type: 'largest-contentful-paint',
name: 'largest-contentful-paint',
start: earlyTimeStampSeconds - 999999,
end: earlyTimeStampSeconds - 99999,
data: {
value: 100,
size: 100,
nodeId: undefined,
},
});

await replay.flushImmediate();
vi.runAllTimers();

expect(replay.getContext().initialTimestamp).toEqual(earlyTimeStampSeconds * 1000);
});

it('does not change initialTimestamp when replay is manually started', async () => {
const earlyTimeStampSeconds = Date.now() / 1000 - 5;

const { replay } = await resetSdkMock({
replayOptions: {
stickySession: true,
},
sentryOptions: {
replaysSessionSampleRate: 0.0,
replaysOnErrorSampleRate: 0.0,
},
});

expect(replay.session).toBe(undefined);
expect(replay['_requiresManualStart']).toBe(true);

replay.start();
vi.runAllTimers();

const initialTimestamp = replay.getContext().initialTimestamp;

expect(initialTimestamp).not.toEqual(earlyTimeStampSeconds * 1000);
expect(replay.session).toBeDefined();
expect(replay['_requiresManualStart']).toBe(true);

// A performance entry that happened before should not extend the session when we manually started
replay.replayPerformanceEntries.push({
type: 'largest-contentful-paint',
name: 'largest-contentful-paint',
start: earlyTimeStampSeconds,
end: earlyTimeStampSeconds,
data: {
value: 100,
size: 100,
nodeId: undefined,
},
});

await replay.flushImmediate();
vi.runAllTimers();

expect(replay.getContext().initialTimestamp).toEqual(initialTimestamp);
});
});

0 comments on commit 4009e3f

Please sign in to comment.