From 737adb2f2b8bbbf0ca0e9319467ecd9ef09e4deb Mon Sep 17 00:00:00 2001 From: Francesco Novy Date: Tue, 6 Dec 2022 13:01:39 +0100 Subject: [PATCH 01/15] ref(replay): Extract `overwriteRecordDroppedEvent` into fn --- packages/replay/src/replay.ts | 47 ++----------------- .../src/util/monkeyPatchRecordDroppedEvent.ts | 39 +++++++++++++++ .../test/unit/index-handleGlobalEvent.test.ts | 6 ++- 3 files changed, 47 insertions(+), 45 deletions(-) create mode 100644 packages/replay/src/util/monkeyPatchRecordDroppedEvent.ts diff --git a/packages/replay/src/replay.ts b/packages/replay/src/replay.ts index e5c0007752bf..98cb1cfd559a 100644 --- a/packages/replay/src/replay.ts +++ b/packages/replay/src/replay.ts @@ -1,6 +1,6 @@ /* eslint-disable max-lines */ // TODO: We might want to split this file up import { addGlobalEventProcessor, getCurrentHub, Scope, setContext } from '@sentry/core'; -import { Breadcrumb, Client, DataCategory, Event, EventDropReason } from '@sentry/types'; +import { Breadcrumb, Client, Event } from '@sentry/types'; import { addInstrumentationHandler, createEnvelope, logger } from '@sentry/utils'; import debounce from 'lodash.debounce'; import { PerformanceObserverEntryList } from 'perf_hooks'; @@ -39,6 +39,7 @@ import { createPayload } from './util/createPayload'; import { dedupePerformanceEntries } from './util/dedupePerformanceEntries'; import { isExpired } from './util/isExpired'; import { isSessionExpired } from './util/isSessionExpired'; +import { overwriteRecordDroppedEvent, restoreRecordDroppedEvent } from './util/monkeyPatchRecordDroppedEvent'; /** * Returns true to return control to calling function, otherwise continue with normal batching @@ -108,11 +109,6 @@ export class ReplayContainer { */ private _stopRecording: ReturnType | null = null; - /** - * We overwrite `client.recordDroppedEvent`, but store the original method here so we can restore it. - */ - private _originalRecordDroppedEvent?: Client['recordDroppedEvent']; - private _context: InternalEventContext = { errorIds: new Set(), traceIds: new Set(), @@ -310,7 +306,7 @@ export class ReplayContainer { WINDOW.addEventListener('focus', this.handleWindowFocus); // We need to filter out dropped events captured by `addGlobalEventProcessor(this.handleGlobalEvent)` below - this._overwriteRecordDroppedEvent(); + overwriteRecordDroppedEvent(this._context.errorIds); // There is no way to remove these listeners, so ensure they are only added once if (!this._hasInitializedCoreListeners) { @@ -374,7 +370,7 @@ export class ReplayContainer { WINDOW.removeEventListener('blur', this.handleWindowBlur); WINDOW.removeEventListener('focus', this.handleWindowFocus); - this._restoreRecordDroppedEvent(); + restoreRecordDroppedEvent(); if (this._performanceObserver) { this._performanceObserver.disconnect(); @@ -1249,39 +1245,4 @@ export class ReplayContainer { saveSession(this.session); } } - - private _overwriteRecordDroppedEvent(): void { - const client = getCurrentHub().getClient(); - - if (!client) { - return; - } - - const _originalCallback = client.recordDroppedEvent.bind(client); - - const recordDroppedEvent: Client['recordDroppedEvent'] = ( - reason: EventDropReason, - category: DataCategory, - event?: Event, - ): void => { - if (event && event.event_id) { - this._context.errorIds.delete(event.event_id); - } - - return _originalCallback(reason, category, event); - }; - - client.recordDroppedEvent = recordDroppedEvent; - this._originalRecordDroppedEvent = _originalCallback; - } - - private _restoreRecordDroppedEvent(): void { - const client = getCurrentHub().getClient(); - - if (!client || !this._originalRecordDroppedEvent) { - return; - } - - client.recordDroppedEvent = this._originalRecordDroppedEvent; - } } diff --git a/packages/replay/src/util/monkeyPatchRecordDroppedEvent.ts b/packages/replay/src/util/monkeyPatchRecordDroppedEvent.ts new file mode 100644 index 000000000000..70f629301dc8 --- /dev/null +++ b/packages/replay/src/util/monkeyPatchRecordDroppedEvent.ts @@ -0,0 +1,39 @@ +import { getCurrentHub } from '@sentry/core'; +import { Client, DataCategory, Event, EventDropReason } from '@sentry/types'; + +let _originalRecordDroppedEvent: Client['recordDroppedEvent'] | undefined; + +export function overwriteRecordDroppedEvent(errorIds: Set): void { + const client = getCurrentHub().getClient(); + + if (!client) { + return; + } + + const _originalCallback = client.recordDroppedEvent.bind(client); + + const recordDroppedEvent: Client['recordDroppedEvent'] = ( + reason: EventDropReason, + category: DataCategory, + event?: Event, + ): void => { + if (event && event.event_id) { + errorIds.delete(event.event_id); + } + + return _originalCallback(reason, category, event); + }; + + client.recordDroppedEvent = recordDroppedEvent; + _originalRecordDroppedEvent = _originalCallback; +} + +export function restoreRecordDroppedEvent(): void { + const client = getCurrentHub().getClient(); + + if (!client || !_originalRecordDroppedEvent) { + return; + } + + client.recordDroppedEvent = _originalRecordDroppedEvent; +} diff --git a/packages/replay/test/unit/index-handleGlobalEvent.test.ts b/packages/replay/test/unit/index-handleGlobalEvent.test.ts index aaae032da14b..e833e00de69f 100644 --- a/packages/replay/test/unit/index-handleGlobalEvent.test.ts +++ b/packages/replay/test/unit/index-handleGlobalEvent.test.ts @@ -1,6 +1,7 @@ import { getCurrentHub } from '@sentry/core'; import { REPLAY_EVENT_NAME } from '../../src/constants'; +import { overwriteRecordDroppedEvent, restoreRecordDroppedEvent } from '../../src/util/monkeyPatchRecordDroppedEvent'; import { ReplayContainer } from './../../src/replay'; import { Error } from './../fixtures/error'; import { Transaction } from './../fixtures/transaction'; @@ -93,7 +94,8 @@ it('strips out dropped events from errorIds', async () => { const error2 = Error({ event_id: 'err2' }); const error3 = Error({ event_id: 'err3' }); - replay['_overwriteRecordDroppedEvent'](); + // @ts-ignore private + overwriteRecordDroppedEvent(replay._context.errorIds); const client = getCurrentHub().getClient()!; @@ -106,7 +108,7 @@ it('strips out dropped events from errorIds', async () => { // @ts-ignore private expect(Array.from(replay._context.errorIds)).toEqual(['err1', 'err3']); - replay['_restoreRecordDroppedEvent'](); + restoreRecordDroppedEvent(); }); it('tags errors and transactions with replay id for session samples', async () => { From a558935fb766b504b43bbb2f31d0e4780673a87d Mon Sep 17 00:00:00 2001 From: Francesco Novy Date: Tue, 6 Dec 2022 13:35:53 +0100 Subject: [PATCH 02/15] ref(replay): Extract span listeners out --- .../replay/src/coreHandlers/handleFetch.ts | 27 +++++++++++- .../replay/src/coreHandlers/handleHistory.ts | 31 ++++++++++++- packages/replay/src/coreHandlers/handleXhr.ts | 28 +++++++++++- .../replay/src/coreHandlers/spanHandler.ts | 17 ------- packages/replay/src/replay.ts | 44 +++---------------- packages/replay/src/types.ts | 7 --- 6 files changed, 87 insertions(+), 67 deletions(-) delete mode 100644 packages/replay/src/coreHandlers/spanHandler.ts diff --git a/packages/replay/src/coreHandlers/handleFetch.ts b/packages/replay/src/coreHandlers/handleFetch.ts index e68e2d911134..0ecba530c758 100644 --- a/packages/replay/src/coreHandlers/handleFetch.ts +++ b/packages/replay/src/coreHandlers/handleFetch.ts @@ -1,7 +1,8 @@ import { ReplayPerformanceEntry } from '../createPerformanceEntry'; +import { ReplayContainer } from '../replay'; import { isIngestHost } from '../util/isIngestHost'; -export interface FetchHandlerData { +interface FetchHandlerData { args: Parameters; fetchData: { method: string; @@ -18,6 +19,7 @@ export interface FetchHandlerData { endTimestamp?: number; } +/** only exported for tests */ export function handleFetch(handlerData: FetchHandlerData): null | ReplayPerformanceEntry { if (!handlerData.endTimestamp) { return null; @@ -41,3 +43,26 @@ export function handleFetch(handlerData: FetchHandlerData): null | ReplayPerform }, }; } + +export function handleFetchSpanListener(replay: ReplayContainer): (handlerData: FetchHandlerData) => void { + return (handlerData: FetchHandlerData) => { + // @ts-ignore private + if (!replay._isEnabled) { + return; + } + + const result = handleFetch(handlerData); + + if (result === null) { + return; + } + + replay.addUpdate(() => { + void replay.createPerformanceSpans([result]); + // Returning true will cause `addUpdate` to not flush + // We do not want network requests to cause a flush. This will prevent + // recurring/polling requests from keeping the replay session alive. + return true; + }); + }; +} diff --git a/packages/replay/src/coreHandlers/handleHistory.ts b/packages/replay/src/coreHandlers/handleHistory.ts index a94d19455192..2be88489772c 100644 --- a/packages/replay/src/coreHandlers/handleHistory.ts +++ b/packages/replay/src/coreHandlers/handleHistory.ts @@ -1,11 +1,12 @@ import { ReplayPerformanceEntry } from '../createPerformanceEntry'; +import { ReplayContainer } from '../replay'; -export interface HistoryHandlerData { +interface HistoryHandlerData { from: string; to: string; } -export function handleHistory(handlerData: HistoryHandlerData): ReplayPerformanceEntry { +function handleHistory(handlerData: HistoryHandlerData): ReplayPerformanceEntry { const { from, to } = handlerData; const now = new Date().getTime() / 1000; @@ -20,3 +21,29 @@ export function handleHistory(handlerData: HistoryHandlerData): ReplayPerformanc }, }; } + +export function handleHistorySpanListener(replay: ReplayContainer): (handlerData: HistoryHandlerData) => void { + return (handlerData: HistoryHandlerData) => { + // @ts-ignore private + if (!replay._isEnabled) { + return; + } + + const result = handleHistory(handlerData); + + if (result === null) { + return; + } + + // Need to collect visited URLs + // @ts-ignore private + replay._context.urls.push(result.name); + replay.triggerUserActivity(); + + replay.addUpdate(() => { + void replay.createPerformanceSpans([result]); + // Returning false to flush + return false; + }); + }; +} diff --git a/packages/replay/src/coreHandlers/handleXhr.ts b/packages/replay/src/coreHandlers/handleXhr.ts index a37e7374ab82..154891b6370a 100644 --- a/packages/replay/src/coreHandlers/handleXhr.ts +++ b/packages/replay/src/coreHandlers/handleXhr.ts @@ -1,4 +1,5 @@ import { ReplayPerformanceEntry } from '../createPerformanceEntry'; +import { ReplayContainer } from '../replay'; import { isIngestHost } from '../util/isIngestHost'; // From sentry-javascript @@ -19,14 +20,14 @@ interface SentryWrappedXMLHttpRequest extends XMLHttpRequest { __sentry_own_request__?: boolean; } -export interface XhrHandlerData { +interface XhrHandlerData { args: [string, string]; xhr: SentryWrappedXMLHttpRequest; startTimestamp: number; endTimestamp?: number; } -export function handleXhr(handlerData: XhrHandlerData): ReplayPerformanceEntry | null { +function handleXhr(handlerData: XhrHandlerData): ReplayPerformanceEntry | null { if (handlerData.xhr.__sentry_own_request__) { // Taken from sentry-javascript // Only capture non-sentry requests @@ -61,3 +62,26 @@ export function handleXhr(handlerData: XhrHandlerData): ReplayPerformanceEntry | }, }; } + +export function handleXhrSpanListener(replay: ReplayContainer): (handlerData: XhrHandlerData) => void { + return (handlerData: XhrHandlerData) => { + // @ts-ignore private + if (!replay._isEnabled) { + return; + } + + const result = handleXhr(handlerData); + + if (result === null) { + return; + } + + replay.addUpdate(() => { + void replay.createPerformanceSpans([result]); + // Returning true will cause `addUpdate` to not flush + // We do not want network requests to cause a flush. This will prevent + // recurring/polling requests from keeping the replay session alive. + return true; + }); + }; +} diff --git a/packages/replay/src/coreHandlers/spanHandler.ts b/packages/replay/src/coreHandlers/spanHandler.ts deleted file mode 100644 index c69b908b5137..000000000000 --- a/packages/replay/src/coreHandlers/spanHandler.ts +++ /dev/null @@ -1,17 +0,0 @@ -import { ReplayPerformanceEntry } from '../createPerformanceEntry'; -import { InstrumentationTypeSpan } from '../types'; -import { FetchHandlerData, handleFetch } from './handleFetch'; -import { handleHistory, HistoryHandlerData } from './handleHistory'; -import { handleXhr, XhrHandlerData } from './handleXhr'; - -export function spanHandler(type: InstrumentationTypeSpan, handlerData: unknown): null | ReplayPerformanceEntry { - if (type === 'fetch') { - return handleFetch(handlerData as FetchHandlerData); - } - - if (type === 'xhr') { - return handleXhr(handlerData as XhrHandlerData); - } - - return handleHistory(handlerData as HistoryHandlerData); -} diff --git a/packages/replay/src/replay.ts b/packages/replay/src/replay.ts index 98cb1cfd559a..d8c4c71332ac 100644 --- a/packages/replay/src/replay.ts +++ b/packages/replay/src/replay.ts @@ -14,7 +14,9 @@ import { WINDOW, } from './constants'; import { breadcrumbHandler } from './coreHandlers/breadcrumbHandler'; -import { spanHandler } from './coreHandlers/spanHandler'; +import { handleFetchSpanListener } from './coreHandlers/handleFetch'; +import { handleHistorySpanListener } from './coreHandlers/handleHistory'; +import { handleXhrSpanListener } from './coreHandlers/handleXhr'; import { createMemoryEntry, createPerformanceEntries, ReplayPerformanceEntry } from './createPerformanceEntry'; import { createEventBuffer, EventBuffer } from './eventBuffer'; import { deleteSession } from './session/deleteSession'; @@ -24,7 +26,6 @@ import { Session } from './session/Session'; import type { AllPerformanceEntry, InstrumentationTypeBreadcrumb, - InstrumentationTypeSpan, InternalEventContext, PopEventContext, RecordingEvent, @@ -314,9 +315,9 @@ export class ReplayContainer { const scope = getCurrentHub().getScope(); scope?.addScopeListener(this.handleCoreBreadcrumbListener('scope')); addInstrumentationHandler('dom', this.handleCoreBreadcrumbListener('dom')); - addInstrumentationHandler('fetch', this.handleCoreSpanListener('fetch')); - addInstrumentationHandler('xhr', this.handleCoreSpanListener('xhr')); - addInstrumentationHandler('history', this.handleCoreSpanListener('history')); + addInstrumentationHandler('fetch', handleFetchSpanListener(this)); + addInstrumentationHandler('xhr', handleXhrSpanListener(this)); + addInstrumentationHandler('history', handleHistorySpanListener(this)); // Tag all (non replay) events that get sent to Sentry with the current // replay ID so that we can reference them later in the UI @@ -590,39 +591,6 @@ export class ReplayContainer { this.doChangeToForegroundTasks(breadcrumb); }; - /** - * Handler for Sentry Core SDK events. - * - * These specific events will create span-like objects in the recording. - */ - handleCoreSpanListener: (type: InstrumentationTypeSpan) => (handlerData: unknown) => void = - (type: InstrumentationTypeSpan) => - (handlerData: unknown): void => { - if (!this._isEnabled) { - return; - } - - const result = spanHandler(type, handlerData); - - if (result === null) { - return; - } - - if (type === 'history') { - // Need to collect visited URLs - this._context.urls.push(result.name); - this.triggerUserActivity(); - } - - this.addUpdate(() => { - void this.createPerformanceSpans([result as ReplayPerformanceEntry]); - // Returning true will cause `addUpdate` to not flush - // We do not want network requests to cause a flush. This will prevent - // recurring/polling requests from keeping the replay session alive. - return ['xhr', 'fetch'].includes(type); - }); - }; - /** * Handler for Sentry Core SDK events. * diff --git a/packages/replay/src/types.ts b/packages/replay/src/types.ts index a3d9cafde299..6f247e12d226 100644 --- a/packages/replay/src/types.ts +++ b/packages/replay/src/types.ts @@ -16,13 +16,6 @@ export interface SendReplay { } export type InstrumentationTypeBreadcrumb = 'dom' | 'scope'; -export type InstrumentationTypeSpan = 'fetch' | 'xhr' | 'history'; -export type InstrumentationType = - | InstrumentationTypeBreadcrumb - | InstrumentationTypeSpan - | 'console' - | 'error' - | 'unhandledrejection'; /** * The request payload to worker From db1ab04b1b280adcabd1de581983dd9eb41b7c18 Mon Sep 17 00:00:00 2001 From: Francesco Novy Date: Tue, 6 Dec 2022 16:57:40 +0100 Subject: [PATCH 03/15] ref(replay): Extract handleGlobalEvent handler out --- packages/replay/src/constants.ts | 1 + .../src/coreHandlers/handleGlobalEvent.ts | 74 +++++++++++++++++++ packages/replay/src/replay.ts | 72 +----------------- .../test/unit/index-handleGlobalEvent.test.ts | 21 +++--- 4 files changed, 89 insertions(+), 79 deletions(-) create mode 100644 packages/replay/src/coreHandlers/handleGlobalEvent.ts diff --git a/packages/replay/src/constants.ts b/packages/replay/src/constants.ts index b7cbfe52873d..47da014c0e78 100644 --- a/packages/replay/src/constants.ts +++ b/packages/replay/src/constants.ts @@ -9,6 +9,7 @@ export const WINDOW = GLOBAL_OBJ as typeof GLOBAL_OBJ & Window; export const REPLAY_SESSION_KEY = 'sentryReplaySession'; export const REPLAY_EVENT_NAME = 'replay_event'; export const RECORDING_EVENT_NAME = 'replay_recording'; +export const UNABLE_TO_SEND_REPLAY = 'Unable to send Replay'; // The idle limit for a session export const SESSION_IDLE_DURATION = 300_000; // 5 minutes in ms diff --git a/packages/replay/src/coreHandlers/handleGlobalEvent.ts b/packages/replay/src/coreHandlers/handleGlobalEvent.ts new file mode 100644 index 000000000000..0372fd3b7a0d --- /dev/null +++ b/packages/replay/src/coreHandlers/handleGlobalEvent.ts @@ -0,0 +1,74 @@ +import { Event } from '@sentry/types'; + +import { REPLAY_EVENT_NAME, UNABLE_TO_SEND_REPLAY } from '../constants'; +import { ReplayContainer } from '../replay'; +import { addInternalBreadcrumb } from '../util/addInternalBreadcrumb'; + +export function handleGlobalEventListener(replay: ReplayContainer): (event: Event) => Event { + return (event: Event) => { + // Do not apply replayId to the root event + if ( + // @ts-ignore new event type + event.type === REPLAY_EVENT_NAME + ) { + // Replays have separate set of breadcrumbs, do not include breadcrumbs + // from core SDK + delete event.breadcrumbs; + return event; + } + + // Only tag transactions with replayId if not waiting for an error + // @ts-ignore private + if (event.type !== 'transaction' || !replay._waitForError) { + event.tags = { ...event.tags, replayId: replay.session?.id }; + } + + // Collect traceIds in _context regardless of `_waitForError` - if it's true, + // _context gets cleared on every checkout + if (event.type === 'transaction') { + // @ts-ignore private + replay._context.traceIds.add(String(event.contexts?.trace?.trace_id || '')); + return event; + } + + // XXX: Is it safe to assume that all other events are error events? + // @ts-ignore: Type 'undefined' is not assignable to type 'string'.ts(2345) + replay._context.errorIds.add(event.event_id); + + const exc = event.exception?.values?.[0]; + addInternalBreadcrumb({ + message: `Tagging event (${event.event_id}) - ${event.message} - ${exc?.type || 'Unknown'}: ${ + exc?.value || 'n/a' + }`, + }); + + // Need to be very careful that this does not cause an infinite loop + if ( + // @ts-ignore private + replay._waitForError && + event.exception && + event.message !== UNABLE_TO_SEND_REPLAY // ignore this error because otherwise we could loop indefinitely with trying to capture replay and failing + ) { + setTimeout(async () => { + // Allow flush to complete before resuming as a session recording, otherwise + // the checkout from `startRecording` may be included in the payload. + // Prefer to keep the error replay as a separate (and smaller) segment + // than the session replay. + await replay.flushImmediate(); + + // @ts-ignore private + if (replay._stopRecording) { + // @ts-ignore private + replay._stopRecording(); + // Reset all "capture on error" configuration before + // starting a new recording + // @ts-ignore private + replay._waitForError = false; + replay.startRecording(); + } + }); + } + + return event; + }; +} diff --git a/packages/replay/src/replay.ts b/packages/replay/src/replay.ts index d8c4c71332ac..b45fe5f6b6d2 100644 --- a/packages/replay/src/replay.ts +++ b/packages/replay/src/replay.ts @@ -10,11 +10,13 @@ import { MAX_SESSION_LIFE, REPLAY_EVENT_NAME, SESSION_IDLE_DURATION, + UNABLE_TO_SEND_REPLAY, VISIBILITY_CHANGE_TIMEOUT, WINDOW, } from './constants'; import { breadcrumbHandler } from './coreHandlers/breadcrumbHandler'; import { handleFetchSpanListener } from './coreHandlers/handleFetch'; +import { handleGlobalEventListener } from './coreHandlers/handleGlobalEvent'; import { handleHistorySpanListener } from './coreHandlers/handleHistory'; import { handleXhrSpanListener } from './coreHandlers/handleXhr'; import { createMemoryEntry, createPerformanceEntries, ReplayPerformanceEntry } from './createPerformanceEntry'; @@ -33,7 +35,6 @@ import type { ReplayPluginOptions, SendReplay, } from './types'; -import { addInternalBreadcrumb } from './util/addInternalBreadcrumb'; import { captureInternalException } from './util/captureInternalException'; import { createBreadcrumb } from './util/createBreadcrumb'; import { createPayload } from './util/createPayload'; @@ -49,7 +50,6 @@ type AddUpdateCallback = () => boolean | void; const BASE_RETRY_INTERVAL = 5000; const MAX_RETRY_COUNT = 3; -const UNABLE_TO_SEND_REPLAY = 'Unable to send Replay'; export class ReplayContainer { public eventBuffer: EventBuffer | null = null; @@ -321,7 +321,7 @@ export class ReplayContainer { // Tag all (non replay) events that get sent to Sentry with the current // replay ID so that we can reference them later in the UI - addGlobalEventProcessor(this.handleGlobalEvent); + addGlobalEventProcessor(handleGlobalEventListener(this)); this._hasInitializedCoreListeners = true; } @@ -412,72 +412,6 @@ export class ReplayContainer { this._debouncedFlush(); } - /** - * Core Sentry SDK global event handler. Attaches `replayId` to all [non-replay] - * events as a tag. Also handles the case where we only want to capture a reply - * when an error occurs. - **/ - handleGlobalEvent: (event: Event) => Event = (event: Event) => { - // Do not apply replayId to the root event - if ( - // @ts-ignore new event type - event.type === REPLAY_EVENT_NAME - ) { - // Replays have separate set of breadcrumbs, do not include breadcrumbs - // from core SDK - delete event.breadcrumbs; - return event; - } - - // Only tag transactions with replayId if not waiting for an error - if (event.type !== 'transaction' || !this._waitForError) { - event.tags = { ...event.tags, replayId: this.session?.id }; - } - - // Collect traceIds in _context regardless of `_waitForError` - if it's true, - // _context gets cleared on every checkout - if (event.type === 'transaction') { - this._context.traceIds.add(String(event.contexts?.trace?.trace_id || '')); - return event; - } - - // XXX: Is it safe to assume that all other events are error events? - // @ts-ignore: Type 'undefined' is not assignable to type 'string'.ts(2345) - this._context.errorIds.add(event.event_id); - - const exc = event.exception?.values?.[0]; - addInternalBreadcrumb({ - message: `Tagging event (${event.event_id}) - ${event.message} - ${exc?.type || 'Unknown'}: ${ - exc?.value || 'n/a' - }`, - }); - - // Need to be very careful that this does not cause an infinite loop - if ( - this._waitForError && - event.exception && - event.message !== UNABLE_TO_SEND_REPLAY // ignore this error because otherwise we could loop indefinitely with trying to capture replay and failing - ) { - setTimeout(async () => { - // Allow flush to complete before resuming as a session recording, otherwise - // the checkout from `startRecording` may be included in the payload. - // Prefer to keep the error replay as a separate (and smaller) segment - // than the session replay. - await this.flushImmediate(); - - if (this._stopRecording) { - this._stopRecording(); - // Reset all "capture on error" configuration before - // starting a new recording - this._waitForError = false; - this.startRecording(); - } - }); - } - - return event; - }; - /** * Handler for recording events. * diff --git a/packages/replay/test/unit/index-handleGlobalEvent.test.ts b/packages/replay/test/unit/index-handleGlobalEvent.test.ts index e833e00de69f..ce9678578e20 100644 --- a/packages/replay/test/unit/index-handleGlobalEvent.test.ts +++ b/packages/replay/test/unit/index-handleGlobalEvent.test.ts @@ -1,6 +1,7 @@ import { getCurrentHub } from '@sentry/core'; import { REPLAY_EVENT_NAME } from '../../src/constants'; +import { handleGlobalEventListener } from '../../src/coreHandlers/handleGlobalEvent'; import { overwriteRecordDroppedEvent, restoreRecordDroppedEvent } from '../../src/util/monkeyPatchRecordDroppedEvent'; import { ReplayContainer } from './../../src/replay'; import { Error } from './../fixtures/error'; @@ -34,14 +35,14 @@ it('deletes breadcrumbs from replay events', () => { }; // @ts-ignore replay event type - expect(replay.handleGlobalEvent(replayEvent)).toEqual({ + expect(handleGlobalEventListener(replay)(replayEvent)).toEqual({ type: REPLAY_EVENT_NAME, }); }); it('does not delete breadcrumbs from error and transaction events', () => { expect( - replay.handleGlobalEvent({ + handleGlobalEventListener(replay)({ breadcrumbs: [{ type: 'fakecrumb' }], }), ).toEqual( @@ -50,7 +51,7 @@ it('does not delete breadcrumbs from error and transaction events', () => { }), ); expect( - replay.handleGlobalEvent({ + handleGlobalEventListener(replay)({ type: 'transaction', breadcrumbs: [{ type: 'fakecrumb' }], }), @@ -65,12 +66,12 @@ it('only tags errors with replay id, adds trace and error id to context for erro const transaction = Transaction(); const error = Error(); // @ts-ignore idc - expect(replay.handleGlobalEvent(transaction)).toEqual( + expect(handleGlobalEventListener(replay)(transaction)).toEqual( expect.objectContaining({ tags: expect.not.objectContaining({ replayId: expect.anything() }), }), ); - expect(replay.handleGlobalEvent(error)).toEqual( + expect(handleGlobalEventListener(replay)(error)).toEqual( expect.objectContaining({ tags: expect.objectContaining({ replayId: expect.any(String) }), }), @@ -99,9 +100,9 @@ it('strips out dropped events from errorIds', async () => { const client = getCurrentHub().getClient()!; - replay.handleGlobalEvent(error1); - replay.handleGlobalEvent(error2); - replay.handleGlobalEvent(error3); + handleGlobalEventListener(replay)(error1); + handleGlobalEventListener(replay)(error2); + handleGlobalEventListener(replay)(error3); client.recordDroppedEvent('before_send', 'error', { event_id: 'err2' }); @@ -117,12 +118,12 @@ it('tags errors and transactions with replay id for session samples', async () = const transaction = Transaction(); const error = Error(); // @ts-ignore idc - expect(replay.handleGlobalEvent(transaction)).toEqual( + expect(handleGlobalEventListener(replay)(transaction)).toEqual( expect.objectContaining({ tags: expect.objectContaining({ replayId: expect.any(String) }), }), ); - expect(replay.handleGlobalEvent(error)).toEqual( + expect(handleGlobalEventListener(replay)(error)).toEqual( expect.objectContaining({ tags: expect.objectContaining({ replayId: expect.any(String) }), }), From edd132a3ae9ec6b24efd5d0335b9030561580f1f Mon Sep 17 00:00:00 2001 From: Francesco Novy Date: Tue, 6 Dec 2022 16:58:31 +0100 Subject: [PATCH 04/15] ref(replay): Fix accidental renaming --- packages/replay/src/replay.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/replay/src/replay.ts b/packages/replay/src/replay.ts index b45fe5f6b6d2..ed4cf41d315d 100644 --- a/packages/replay/src/replay.ts +++ b/packages/replay/src/replay.ts @@ -335,7 +335,7 @@ export class ReplayContainer { return; } - this._performanceObserver = new PerformanceObserver(this.handle_performanceObserver); + this._performanceObserver = new PerformanceObserver(this.handlePerformanceObserver); // Observe almost everything for now (no mark/measure) [ @@ -573,7 +573,7 @@ export class ReplayContainer { /** * Keep a list of performance entries that will be sent with a replay */ - handle_performanceObserver: (list: PerformanceObserverEntryList) => void = (list: PerformanceObserverEntryList) => { + handlePerformanceObserver: (list: PerformanceObserverEntryList) => void = (list: PerformanceObserverEntryList) => { // For whatever reason the observer was returning duplicate navigation // entries (the other entry types were not duplicated). const newPerformanceEntries = dedupePerformanceEntries( From 391a22cd2083a91e97de265911f61f79686959f2 Mon Sep 17 00:00:00 2001 From: Francesco Novy Date: Tue, 6 Dec 2022 17:04:56 +0100 Subject: [PATCH 05/15] ref(replay): Extract performanceObserver out --- .../src/coreHandlers/performanceObserver.ts | 41 +++++++++++++++++++ packages/replay/src/replay.ts | 41 +------------------ 2 files changed, 43 insertions(+), 39 deletions(-) create mode 100644 packages/replay/src/coreHandlers/performanceObserver.ts diff --git a/packages/replay/src/coreHandlers/performanceObserver.ts b/packages/replay/src/coreHandlers/performanceObserver.ts new file mode 100644 index 000000000000..0e21e89d10aa --- /dev/null +++ b/packages/replay/src/coreHandlers/performanceObserver.ts @@ -0,0 +1,41 @@ +import { ReplayContainer } from '../replay'; +import { AllPerformanceEntry } from '../types'; +import { dedupePerformanceEntries } from '../util/dedupePerformanceEntries'; + +export function setupPerformanceObserver(replay: ReplayContainer): PerformanceObserver { + const performanceObserverHandler = (list: PerformanceObserverEntryList): void => { + // For whatever reason the observer was returning duplicate navigation + // entries (the other entry types were not duplicated). + const newPerformanceEntries = dedupePerformanceEntries( + replay.performanceEvents, + list.getEntries() as AllPerformanceEntry[], + ); + replay.performanceEvents = newPerformanceEntries; + }; + + const performanceObserver = new PerformanceObserver(performanceObserverHandler); + + [ + 'element', + 'event', + 'first-input', + 'largest-contentful-paint', + 'layout-shift', + 'longtask', + 'navigation', + 'paint', + 'resource', + ].forEach(type => { + try { + performanceObserver.observe({ + type, + buffered: true, + }); + } catch { + // This can throw if an entry type is not supported in the browser. + // Ignore these errors. + } + }); + + return performanceObserver; +} diff --git a/packages/replay/src/replay.ts b/packages/replay/src/replay.ts index ed4cf41d315d..6815b3ef78aa 100644 --- a/packages/replay/src/replay.ts +++ b/packages/replay/src/replay.ts @@ -3,7 +3,6 @@ import { addGlobalEventProcessor, getCurrentHub, Scope, setContext } from '@sent import { Breadcrumb, Client, Event } from '@sentry/types'; import { addInstrumentationHandler, createEnvelope, logger } from '@sentry/utils'; import debounce from 'lodash.debounce'; -import { PerformanceObserverEntryList } from 'perf_hooks'; import { EventType, record } from 'rrweb'; import { @@ -19,6 +18,7 @@ import { handleFetchSpanListener } from './coreHandlers/handleFetch'; import { handleGlobalEventListener } from './coreHandlers/handleGlobalEvent'; import { handleHistorySpanListener } from './coreHandlers/handleHistory'; import { handleXhrSpanListener } from './coreHandlers/handleXhr'; +import { setupPerformanceObserver } from './coreHandlers/performanceObserver'; import { createMemoryEntry, createPerformanceEntries, ReplayPerformanceEntry } from './createPerformanceEntry'; import { createEventBuffer, EventBuffer } from './eventBuffer'; import { deleteSession } from './session/deleteSession'; @@ -38,7 +38,6 @@ import type { import { captureInternalException } from './util/captureInternalException'; import { createBreadcrumb } from './util/createBreadcrumb'; import { createPayload } from './util/createPayload'; -import { dedupePerformanceEntries } from './util/dedupePerformanceEntries'; import { isExpired } from './util/isExpired'; import { isSessionExpired } from './util/isSessionExpired'; import { overwriteRecordDroppedEvent, restoreRecordDroppedEvent } from './util/monkeyPatchRecordDroppedEvent'; @@ -335,30 +334,7 @@ export class ReplayContainer { return; } - this._performanceObserver = new PerformanceObserver(this.handlePerformanceObserver); - - // Observe almost everything for now (no mark/measure) - [ - 'element', - 'event', - 'first-input', - 'largest-contentful-paint', - 'layout-shift', - 'longtask', - 'navigation', - 'paint', - 'resource', - ].forEach(type => { - try { - this._performanceObserver?.observe({ - type, - buffered: true, - }); - } catch { - // This can throw if an entry type is not supported in the browser. - // Ignore these errors. - } - }); + this._performanceObserver = setupPerformanceObserver(this); } /** @@ -570,19 +546,6 @@ export class ReplayContainer { }); }; - /** - * Keep a list of performance entries that will be sent with a replay - */ - handlePerformanceObserver: (list: PerformanceObserverEntryList) => void = (list: PerformanceObserverEntryList) => { - // For whatever reason the observer was returning duplicate navigation - // entries (the other entry types were not duplicated). - const newPerformanceEntries = dedupePerformanceEntries( - this.performanceEvents, - list.getEntries() as AllPerformanceEntry[], - ); - this.performanceEvents = newPerformanceEntries; - }; - /** * Tasks to run when we consider a page to be hidden (via blurring and/or visibility) */ From 229bd5121a44d056bb82bab264276473db815850 Mon Sep 17 00:00:00 2001 From: Francesco Novy Date: Tue, 6 Dec 2022 17:15:47 +0100 Subject: [PATCH 06/15] ref(replay): Make some things public --- .../replay/src/coreHandlers/handleFetch.ts | 3 +-- .../src/coreHandlers/handleGlobalEvent.ts | 11 +++------ .../replay/src/coreHandlers/handleHistory.ts | 6 ++--- packages/replay/src/coreHandlers/handleXhr.ts | 3 +-- packages/replay/src/replay.ts | 23 +++++++++++++++++++ .../test/unit/index-handleGlobalEvent.test.ts | 10 ++++---- packages/replay/test/unit/index.test.ts | 18 +++++---------- 7 files changed, 40 insertions(+), 34 deletions(-) diff --git a/packages/replay/src/coreHandlers/handleFetch.ts b/packages/replay/src/coreHandlers/handleFetch.ts index 0ecba530c758..5327048fc576 100644 --- a/packages/replay/src/coreHandlers/handleFetch.ts +++ b/packages/replay/src/coreHandlers/handleFetch.ts @@ -46,8 +46,7 @@ export function handleFetch(handlerData: FetchHandlerData): null | ReplayPerform export function handleFetchSpanListener(replay: ReplayContainer): (handlerData: FetchHandlerData) => void { return (handlerData: FetchHandlerData) => { - // @ts-ignore private - if (!replay._isEnabled) { + if (!replay.isEnabled()) { return; } diff --git a/packages/replay/src/coreHandlers/handleGlobalEvent.ts b/packages/replay/src/coreHandlers/handleGlobalEvent.ts index 0372fd3b7a0d..cfe738e2dd22 100644 --- a/packages/replay/src/coreHandlers/handleGlobalEvent.ts +++ b/packages/replay/src/coreHandlers/handleGlobalEvent.ts @@ -26,14 +26,12 @@ export function handleGlobalEventListener(replay: ReplayContainer): (event: Even // Collect traceIds in _context regardless of `_waitForError` - if it's true, // _context gets cleared on every checkout if (event.type === 'transaction') { - // @ts-ignore private - replay._context.traceIds.add(String(event.contexts?.trace?.trace_id || '')); + replay.getContext().traceIds.add(String(event.contexts?.trace?.trace_id || '')); return event; } // XXX: Is it safe to assume that all other events are error events? - // @ts-ignore: Type 'undefined' is not assignable to type 'string'.ts(2345) - replay._context.errorIds.add(event.event_id); + replay.getContext().errorIds.add(event.event_id as string); const exc = event.exception?.values?.[0]; addInternalBreadcrumb({ @@ -56,10 +54,7 @@ export function handleGlobalEventListener(replay: ReplayContainer): (event: Even // than the session replay. await replay.flushImmediate(); - // @ts-ignore private - if (replay._stopRecording) { - // @ts-ignore private - replay._stopRecording(); + if (replay.stopRecording()) { // Reset all "capture on error" configuration before // starting a new recording // @ts-ignore private diff --git a/packages/replay/src/coreHandlers/handleHistory.ts b/packages/replay/src/coreHandlers/handleHistory.ts index 2be88489772c..41f4bf94aeca 100644 --- a/packages/replay/src/coreHandlers/handleHistory.ts +++ b/packages/replay/src/coreHandlers/handleHistory.ts @@ -24,8 +24,7 @@ function handleHistory(handlerData: HistoryHandlerData): ReplayPerformanceEntry export function handleHistorySpanListener(replay: ReplayContainer): (handlerData: HistoryHandlerData) => void { return (handlerData: HistoryHandlerData) => { - // @ts-ignore private - if (!replay._isEnabled) { + if (!replay.isEnabled()) { return; } @@ -36,8 +35,7 @@ export function handleHistorySpanListener(replay: ReplayContainer): (handlerData } // Need to collect visited URLs - // @ts-ignore private - replay._context.urls.push(result.name); + replay.getContext().urls.push(result.name); replay.triggerUserActivity(); replay.addUpdate(() => { diff --git a/packages/replay/src/coreHandlers/handleXhr.ts b/packages/replay/src/coreHandlers/handleXhr.ts index 154891b6370a..b57255430930 100644 --- a/packages/replay/src/coreHandlers/handleXhr.ts +++ b/packages/replay/src/coreHandlers/handleXhr.ts @@ -65,8 +65,7 @@ function handleXhr(handlerData: XhrHandlerData): ReplayPerformanceEntry | null { export function handleXhrSpanListener(replay: ReplayContainer): (handlerData: XhrHandlerData) => void { return (handlerData: XhrHandlerData) => { - // @ts-ignore private - if (!replay._isEnabled) { + if (!replay.isEnabled()) { return; } diff --git a/packages/replay/src/replay.ts b/packages/replay/src/replay.ts index 6815b3ef78aa..0f068db3b09a 100644 --- a/packages/replay/src/replay.ts +++ b/packages/replay/src/replay.ts @@ -127,6 +127,16 @@ export class ReplayContainer { }); } + /** Get the event context. */ + public getContext(): InternalEventContext { + return this._context; + } + + /** If recording is currently enabled. */ + public isEnabled(): boolean { + return this._isEnabled; + } + /** * Initializes the plugin. * @@ -193,6 +203,19 @@ export class ReplayContainer { } } + /** + * Stops the recording, if it was running. + * Returns true if it was stopped, else false. + */ + public stopRecording(): boolean { + if (this._stopRecording) { + this._stopRecording(); + return true; + } + + return false; + } + /** * Currently, this needs to be manually called (e.g. for tests). Sentry SDK * does not support a teardown diff --git a/packages/replay/test/unit/index-handleGlobalEvent.test.ts b/packages/replay/test/unit/index-handleGlobalEvent.test.ts index ce9678578e20..cb005faf7256 100644 --- a/packages/replay/test/unit/index-handleGlobalEvent.test.ts +++ b/packages/replay/test/unit/index-handleGlobalEvent.test.ts @@ -77,10 +77,8 @@ it('only tags errors with replay id, adds trace and error id to context for erro }), ); - // @ts-ignore private - expect(replay._context.traceIds).toContain('trace_id'); - // @ts-ignore private - expect(replay._context.errorIds).toContain('event_id'); + expect(replay.getContext().traceIds).toContain('trace_id'); + expect(replay.getContext().errorIds).toContain('event_id'); jest.runAllTimers(); await new Promise(process.nextTick); // wait for flush @@ -96,7 +94,7 @@ it('strips out dropped events from errorIds', async () => { const error3 = Error({ event_id: 'err3' }); // @ts-ignore private - overwriteRecordDroppedEvent(replay._context.errorIds); + overwriteRecordDroppedEvent(replay.getContext().errorIds); const client = getCurrentHub().getClient()!; @@ -107,7 +105,7 @@ it('strips out dropped events from errorIds', async () => { client.recordDroppedEvent('before_send', 'error', { event_id: 'err2' }); // @ts-ignore private - expect(Array.from(replay._context.errorIds)).toEqual(['err1', 'err3']); + expect(Array.from(replay.getContext().errorIds)).toEqual(['err1', 'err3']); restoreRecordDroppedEvent(); }); diff --git a/packages/replay/test/unit/index.test.ts b/packages/replay/test/unit/index.test.ts index c2032287559c..05bebcdd8308 100644 --- a/packages/replay/test/unit/index.test.ts +++ b/packages/replay/test/unit/index.test.ts @@ -349,8 +349,7 @@ describe('Replay', () => { const initialSession = replay.session; expect(initialSession?.id).toBeDefined(); - // @ts-ignore private member - expect(replay._context).toEqual( + expect(replay.getContext()).toEqual( expect.objectContaining({ initialUrl: 'http://localhost/', initialTimestamp: BASE_TIMESTAMP, @@ -423,8 +422,7 @@ describe('Replay', () => { }); // `_context` should be reset when a new session is created - // @ts-ignore private member - expect(replay._context).toEqual( + expect(replay.getContext()).toEqual( expect.objectContaining({ initialUrl: 'http://dummy/', initialTimestamp: newTimestamp, @@ -437,8 +435,7 @@ describe('Replay', () => { const initialSession = replay.session; expect(initialSession?.id).toBeDefined(); - // @ts-ignore private member - expect(replay._context).toEqual( + expect(replay.getContext()).toEqual( expect.objectContaining({ initialUrl: 'http://localhost/', initialTimestamp: BASE_TIMESTAMP, @@ -536,8 +533,7 @@ describe('Replay', () => { }); // `_context` should be reset when a new session is created - // @ts-ignore private member - expect(replay._context).toEqual( + expect(replay.getContext()).toEqual( expect.objectContaining({ initialUrl: 'http://dummy/', initialTimestamp: newTimestamp, @@ -855,8 +851,7 @@ describe('Replay', () => { ); // This should be null because `addEvent` has not been called yet - // @ts-ignore private member - expect(replay._context.earliestEvent).toBe(null); + expect(replay.getContext().earliestEvent).toBe(null); expect(mockTransportSend).toHaveBeenCalledTimes(0); // A new checkout occurs (i.e. a new session was started) @@ -896,8 +891,7 @@ describe('Replay', () => { }); // This gets reset after sending replay - // @ts-ignore private member - expect(replay._context.earliestEvent).toBe(null); + expect(replay.getContext().earliestEvent).toBe(null); }); it('has single flush when checkout flush and debounce flush happen near simultaneously', async () => { From 721e682282c7f0a429ccdff6df66018edacc60bc Mon Sep 17 00:00:00 2001 From: Francesco Novy Date: Tue, 6 Dec 2022 17:20:20 +0100 Subject: [PATCH 07/15] ref(replay): Extract addEvent out --- packages/replay/src/replay.ts | 53 ++++--------------- packages/replay/src/util/addEvent.ts | 40 ++++++++++++++ .../test/unit/index-errorSampleRate.test.ts | 3 +- .../replay/test/unit/index-noSticky.test.ts | 3 +- packages/replay/test/unit/index.test.ts | 19 +++---- packages/replay/test/unit/stop.test.ts | 5 +- 6 files changed, 67 insertions(+), 56 deletions(-) create mode 100644 packages/replay/src/util/addEvent.ts diff --git a/packages/replay/src/replay.ts b/packages/replay/src/replay.ts index 0f068db3b09a..18a94a8ca18f 100644 --- a/packages/replay/src/replay.ts +++ b/packages/replay/src/replay.ts @@ -41,6 +41,7 @@ import { createPayload } from './util/createPayload'; import { isExpired } from './util/isExpired'; import { isSessionExpired } from './util/isSessionExpired'; import { overwriteRecordDroppedEvent, restoreRecordDroppedEvent } from './util/monkeyPatchRecordDroppedEvent'; +import { addEvent } from './util/addEvent'; /** * Returns true to return control to calling function, otherwise continue with normal batching @@ -137,6 +138,11 @@ export class ReplayContainer { return this._isEnabled; } + /** If recording is currently paused. */ + public isPaused(): boolean { + return this._isPaused; + } + /** * Initializes the plugin. * @@ -439,7 +445,7 @@ export class ReplayContainer { // We need to clear existing events on a checkout, otherwise they are // incremental event updates and should be appended - this.addEvent(event, isCheckout); + addEvent(this, event, isCheckout); // Different behavior for full snapshots (type=2), ignore other event types // See https://github.com/rrweb-io/rrweb/blob/d8f9290ca496712aa1e7d472549480c4e7876594/packages/rrweb/src/types.ts#L16 @@ -553,7 +559,7 @@ export class ReplayContainer { } this.addUpdate(() => { - this.addEvent({ + addEvent(this, { type: EventType.Custom, // TODO: We were converting from ms to seconds for breadcrumbs, spans, // but maybe we should just keep them as milliseconds @@ -623,45 +629,6 @@ export class ReplayContainer { record.takeFullSnapshot(true); } - /** - * Add an event to the event buffer - */ - addEvent(event: RecordingEvent, isCheckout?: boolean): void { - if (!this.eventBuffer) { - // This implies that `_isEnabled` is false - return; - } - - if (this._isPaused) { - // Do not add to event buffer when recording is paused - return; - } - - // TODO: sadness -- we will want to normalize timestamps to be in ms - - // requires coordination with frontend - const isMs = event.timestamp > 9999999999; - const timestampInMs = isMs ? event.timestamp : event.timestamp * 1000; - - // Throw out events that happen more than 5 minutes ago. This can happen if - // page has been left open and idle for a long period of time and user - // comes back to trigger a new session. The performance entries rely on - // `performance.timeOrigin`, which is when the page first opened. - if (timestampInMs + SESSION_IDLE_DURATION < new Date().getTime()) { - return; - } - - // Only record earliest event if a new session was created, otherwise it - // shouldn't be relevant - if ( - this.session?.segmentId === 0 && - (!this._context.earliestEvent || timestampInMs < this._context.earliestEvent) - ) { - this._context.earliestEvent = timestampInMs; - } - - this.eventBuffer.addEvent(event, isCheckout); - } - /** * Update user activity (across session lifespans) */ @@ -710,7 +677,7 @@ export class ReplayContainer { */ createCustomBreadcrumb(breadcrumb: Breadcrumb): void { this.addUpdate(() => { - this.addEvent({ + addEvent(this, { type: EventType.Custom, timestamp: breadcrumb.timestamp || 0, data: { @@ -727,7 +694,7 @@ export class ReplayContainer { createPerformanceSpans(entries: ReplayPerformanceEntry[]): Promise { return Promise.all( entries.map(({ type, start, end, name, data }) => - this.addEvent({ + addEvent(this, { type: EventType.Custom, timestamp: start, data: { diff --git a/packages/replay/src/util/addEvent.ts b/packages/replay/src/util/addEvent.ts new file mode 100644 index 000000000000..a9a45c1cdd4e --- /dev/null +++ b/packages/replay/src/util/addEvent.ts @@ -0,0 +1,40 @@ +import { SESSION_IDLE_DURATION } from '../constants'; +import { ReplayContainer } from '../replay'; +import { RecordingEvent } from '../types'; + +/** + * Add an event to the event buffer + */ +export function addEvent(replay: ReplayContainer, event: RecordingEvent, isCheckout?: boolean): void { + if (!replay.eventBuffer) { + // This implies that `_isEnabled` is false + return; + } + + if (replay.isPaused()) { + // Do not add to event buffer when recording is paused + return; + } + + // TODO: sadness -- we will want to normalize timestamps to be in ms - + // requires coordination with frontend + const isMs = event.timestamp > 9999999999; + const timestampInMs = isMs ? event.timestamp : event.timestamp * 1000; + + // Throw out events that happen more than 5 minutes ago. This can happen if + // page has been left open and idle for a long period of time and user + // comes back to trigger a new session. The performance entries rely on + // `performance.timeOrigin`, which is when the page first opened. + if (timestampInMs + SESSION_IDLE_DURATION < new Date().getTime()) { + return; + } + + // Only record earliest event if a new session was created, otherwise it + // shouldn't be relevant + const earliestEvent = replay.getContext().earliestEvent; + if (replay.session?.segmentId === 0 && (!earliestEvent || timestampInMs < earliestEvent)) { + replay.getContext().earliestEvent = timestampInMs; + } + + replay.eventBuffer.addEvent(event, isCheckout); +} diff --git a/packages/replay/test/unit/index-errorSampleRate.test.ts b/packages/replay/test/unit/index-errorSampleRate.test.ts index dc4240546d02..12eea275bdfe 100644 --- a/packages/replay/test/unit/index-errorSampleRate.test.ts +++ b/packages/replay/test/unit/index-errorSampleRate.test.ts @@ -3,6 +3,7 @@ jest.unmock('@sentry/browser'); import { captureException } from '@sentry/browser'; import { REPLAY_SESSION_KEY, VISIBILITY_CHANGE_TIMEOUT, WINDOW } from '../../src/constants'; +import { addEvent } from '../../src/util/addEvent'; import { ReplayContainer } from './../../src/replay'; import { PerformanceEntryResource } from './../fixtures/performanceEntry/resource'; import { BASE_TIMESTAMP, RecordMock } from './../index'; @@ -195,7 +196,7 @@ describe('Replay (errorSampleRate)', () => { jest.advanceTimersByTime(ELAPSED); const TEST_EVENT = { data: {}, timestamp: BASE_TIMESTAMP, type: 2 }; - replay.addEvent(TEST_EVENT); + addEvent(replay, TEST_EVENT); document.dispatchEvent(new Event('visibilitychange')); diff --git a/packages/replay/test/unit/index-noSticky.test.ts b/packages/replay/test/unit/index-noSticky.test.ts index e1c42d63870a..fa69f34b8e02 100644 --- a/packages/replay/test/unit/index-noSticky.test.ts +++ b/packages/replay/test/unit/index-noSticky.test.ts @@ -3,6 +3,7 @@ import { Transport } from '@sentry/types'; import * as SentryUtils from '@sentry/utils'; import { SESSION_IDLE_DURATION, VISIBILITY_CHANGE_TIMEOUT } from '../../src/constants'; +import { addEvent } from '../../src/util/addEvent'; import { ReplayContainer } from './../../src/replay'; import { BASE_TIMESTAMP, mockRrweb, mockSdk } from './../index'; import { useFakeTimers } from './../utils/use-fake-timers'; @@ -119,7 +120,7 @@ describe('Replay (no sticky)', () => { jest.advanceTimersByTime(ELAPSED); const TEST_EVENT = { data: {}, timestamp: BASE_TIMESTAMP, type: 2 }; - replay.addEvent(TEST_EVENT); + addEvent(replay, TEST_EVENT); document.dispatchEvent(new Event('visibilitychange')); diff --git a/packages/replay/test/unit/index.test.ts b/packages/replay/test/unit/index.test.ts index 05bebcdd8308..fcef7c91f04f 100644 --- a/packages/replay/test/unit/index.test.ts +++ b/packages/replay/test/unit/index.test.ts @@ -7,6 +7,7 @@ import { EventType } from 'rrweb'; import { MAX_SESSION_LIFE, REPLAY_SESSION_KEY, VISIBILITY_CHANGE_TIMEOUT, WINDOW } from '../../src/constants'; import { ReplayContainer } from '../../src/replay'; import { RecordingEvent } from '../../src/types'; +import { addEvent } from '../../src/util/addEvent'; import { useFakeTimers } from '../utils/use-fake-timers'; import { PerformanceEntryResource } from './../fixtures/performanceEntry/resource'; import { BASE_TIMESTAMP, RecordMock } from './../index'; @@ -82,7 +83,7 @@ describe('Replay with custom mock', () => { timestamp: new Date().valueOf(), } as RecordingEvent; - replay.addEvent(event); + addEvent(replay, event); await replay.runFlush(); @@ -250,7 +251,7 @@ describe('Replay', () => { }, }; - replay.addEvent(TEST_EVENT); + addEvent(replay, TEST_EVENT); WINDOW.dispatchEvent(new Event('blur')); await new Promise(process.nextTick); expect(mockRecord.takeFullSnapshot).not.toHaveBeenCalled(); @@ -276,7 +277,7 @@ describe('Replay', () => { const TEST_EVENT = { data: {}, timestamp: BASE_TIMESTAMP, type: 2 }; - replay.addEvent(TEST_EVENT); + addEvent(replay, TEST_EVENT); document.dispatchEvent(new Event('visibilitychange')); jest.runAllTimers(); await new Promise(process.nextTick); @@ -687,7 +688,7 @@ describe('Replay', () => { const TEST_EVENT = { data: {}, timestamp: BASE_TIMESTAMP, type: 2 }; - replay.addEvent(TEST_EVENT); + addEvent(replay, TEST_EVENT); WINDOW.dispatchEvent(new Event('blur')); await new Promise(process.nextTick); expect(replay).toHaveSentReplay({ @@ -695,7 +696,7 @@ describe('Replay', () => { }); expect(replay.session?.segmentId).toBe(1); - replay.addEvent(TEST_EVENT); + addEvent(replay, TEST_EVENT); WINDOW.dispatchEvent(new Event('blur')); jest.runAllTimers(); await new Promise(process.nextTick); @@ -727,7 +728,7 @@ describe('Replay', () => { type: 2, }; - replay.addEvent(TEST_EVENT); + addEvent(replay, TEST_EVENT); WINDOW.dispatchEvent(new Event('blur')); await new Promise(process.nextTick); @@ -812,10 +813,10 @@ describe('Replay', () => { type: 2, }; - replay.addEvent(TEST_EVENT); + addEvent(replay, TEST_EVENT); // Add a fake event that started BEFORE - replay.addEvent({ + addEvent(replay, { data: {}, timestamp: (BASE_TIMESTAMP - 10000) / 1000, type: 5, @@ -861,7 +862,7 @@ describe('Replay', () => { type: 2, }; - replay.addEvent(TEST_EVENT); + addEvent(replay, TEST_EVENT); // This event will trigger a flush WINDOW.dispatchEvent(new Event('blur')); jest.runAllTimers(); diff --git a/packages/replay/test/unit/stop.test.ts b/packages/replay/test/unit/stop.test.ts index 767149e724df..543f7e512a3f 100644 --- a/packages/replay/test/unit/stop.test.ts +++ b/packages/replay/test/unit/stop.test.ts @@ -2,6 +2,7 @@ import * as SentryUtils from '@sentry/utils'; import { SESSION_IDLE_DURATION, WINDOW } from '../../src/constants'; import { ReplayContainer } from '../../src/replay'; +import { addEvent } from '../../src/util/addEvent'; import { Replay } from './../../src'; // mock functions need to be imported first import { BASE_TIMESTAMP, mockRrweb, mockSdk } from './../index'; @@ -73,7 +74,7 @@ describe('Replay - stop', () => { // Pretend 5 seconds have passed jest.advanceTimersByTime(ELAPSED); - replay.addEvent(TEST_EVENT); + addEvent(replay, TEST_EVENT); WINDOW.dispatchEvent(new Event('blur')); await new Promise(process.nextTick); expect(mockRecord.takeFullSnapshot).not.toHaveBeenCalled(); @@ -103,7 +104,7 @@ describe('Replay - stop', () => { }, }; - replay.addEvent(TEST_EVENT); + addEvent(replay, TEST_EVENT); WINDOW.dispatchEvent(new Event('blur')); jest.runAllTimers(); await new Promise(process.nextTick); From d7a3e3afc54ee33b034cd1a36f251169c98585eb Mon Sep 17 00:00:00 2001 From: Francesco Novy Date: Tue, 6 Dec 2022 17:26:25 +0100 Subject: [PATCH 08/15] ref(replay): Extract createPerformanceSpans out --- .../replay/src/coreHandlers/handleFetch.ts | 3 +- .../replay/src/coreHandlers/handleHistory.ts | 3 +- packages/replay/src/coreHandlers/handleXhr.ts | 3 +- packages/replay/src/replay.ts | 33 +++---------------- .../replay/src/util/createPerformanceSpans.ts | 29 ++++++++++++++++ packages/replay/test/unit/flush.test.ts | 4 ++- packages/replay/test/unit/index.test.ts | 3 +- 7 files changed, 45 insertions(+), 33 deletions(-) create mode 100644 packages/replay/src/util/createPerformanceSpans.ts diff --git a/packages/replay/src/coreHandlers/handleFetch.ts b/packages/replay/src/coreHandlers/handleFetch.ts index 5327048fc576..a3564de06e09 100644 --- a/packages/replay/src/coreHandlers/handleFetch.ts +++ b/packages/replay/src/coreHandlers/handleFetch.ts @@ -1,5 +1,6 @@ import { ReplayPerformanceEntry } from '../createPerformanceEntry'; import { ReplayContainer } from '../replay'; +import { createPerformanceSpans } from '../util/createPerformanceSpans'; import { isIngestHost } from '../util/isIngestHost'; interface FetchHandlerData { @@ -57,7 +58,7 @@ export function handleFetchSpanListener(replay: ReplayContainer): (handlerData: } replay.addUpdate(() => { - void replay.createPerformanceSpans([result]); + void createPerformanceSpans(replay, [result]); // Returning true will cause `addUpdate` to not flush // We do not want network requests to cause a flush. This will prevent // recurring/polling requests from keeping the replay session alive. diff --git a/packages/replay/src/coreHandlers/handleHistory.ts b/packages/replay/src/coreHandlers/handleHistory.ts index 41f4bf94aeca..7757da052b42 100644 --- a/packages/replay/src/coreHandlers/handleHistory.ts +++ b/packages/replay/src/coreHandlers/handleHistory.ts @@ -1,5 +1,6 @@ import { ReplayPerformanceEntry } from '../createPerformanceEntry'; import { ReplayContainer } from '../replay'; +import { createPerformanceSpans } from '../util/createPerformanceSpans'; interface HistoryHandlerData { from: string; @@ -39,7 +40,7 @@ export function handleHistorySpanListener(replay: ReplayContainer): (handlerData replay.triggerUserActivity(); replay.addUpdate(() => { - void replay.createPerformanceSpans([result]); + void createPerformanceSpans(replay, [result]); // Returning false to flush return false; }); diff --git a/packages/replay/src/coreHandlers/handleXhr.ts b/packages/replay/src/coreHandlers/handleXhr.ts index b57255430930..cf5138fdbe1d 100644 --- a/packages/replay/src/coreHandlers/handleXhr.ts +++ b/packages/replay/src/coreHandlers/handleXhr.ts @@ -1,5 +1,6 @@ import { ReplayPerformanceEntry } from '../createPerformanceEntry'; import { ReplayContainer } from '../replay'; +import { createPerformanceSpans } from '../util/createPerformanceSpans'; import { isIngestHost } from '../util/isIngestHost'; // From sentry-javascript @@ -76,7 +77,7 @@ export function handleXhrSpanListener(replay: ReplayContainer): (handlerData: Xh } replay.addUpdate(() => { - void replay.createPerformanceSpans([result]); + void createPerformanceSpans(replay, [result]); // Returning true will cause `addUpdate` to not flush // We do not want network requests to cause a flush. This will prevent // recurring/polling requests from keeping the replay session alive. diff --git a/packages/replay/src/replay.ts b/packages/replay/src/replay.ts index 18a94a8ca18f..3f8eed299e8c 100644 --- a/packages/replay/src/replay.ts +++ b/packages/replay/src/replay.ts @@ -19,7 +19,7 @@ import { handleGlobalEventListener } from './coreHandlers/handleGlobalEvent'; import { handleHistorySpanListener } from './coreHandlers/handleHistory'; import { handleXhrSpanListener } from './coreHandlers/handleXhr'; import { setupPerformanceObserver } from './coreHandlers/performanceObserver'; -import { createMemoryEntry, createPerformanceEntries, ReplayPerformanceEntry } from './createPerformanceEntry'; +import { createMemoryEntry, createPerformanceEntries } from './createPerformanceEntry'; import { createEventBuffer, EventBuffer } from './eventBuffer'; import { deleteSession } from './session/deleteSession'; import { getSession } from './session/getSession'; @@ -35,13 +35,14 @@ import type { ReplayPluginOptions, SendReplay, } from './types'; +import { addEvent } from './util/addEvent'; import { captureInternalException } from './util/captureInternalException'; import { createBreadcrumb } from './util/createBreadcrumb'; import { createPayload } from './util/createPayload'; +import { createPerformanceSpans } from './util/createPerformanceSpans'; import { isExpired } from './util/isExpired'; import { isSessionExpired } from './util/isSessionExpired'; import { overwriteRecordDroppedEvent, restoreRecordDroppedEvent } from './util/monkeyPatchRecordDroppedEvent'; -import { addEvent } from './util/addEvent'; /** * Returns true to return control to calling function, otherwise continue with normal batching @@ -688,30 +689,6 @@ export class ReplayContainer { }); } - /** - * Create a "span" for each performance entry. The parent transaction is `this.replayEvent`. - */ - createPerformanceSpans(entries: ReplayPerformanceEntry[]): Promise { - return Promise.all( - entries.map(({ type, start, end, name, data }) => - addEvent(this, { - type: EventType.Custom, - timestamp: start, - data: { - tag: 'performanceSpan', - payload: { - op: type, - description: name, - startTimestamp: start, - endTimestamp: end, - data, - }, - }, - }), - ), - ); - } - /** * Observed performance events are added to `this.performanceEvents`. These * are included in the replay event before it is finished and sent to Sentry. @@ -721,7 +698,7 @@ export class ReplayContainer { const entries = [...this.performanceEvents]; this.performanceEvents = []; - return this.createPerformanceSpans(createPerformanceEntries(entries)); + return createPerformanceSpans(this, createPerformanceEntries(entries)); } /** @@ -735,7 +712,7 @@ export class ReplayContainer { return; } - return this.createPerformanceSpans([ + return createPerformanceSpans(this, [ // @ts-ignore memory doesn't exist on type Performance as the API is non-standard (we check that it exists above) createMemoryEntry(WINDOW.performance.memory), ]); diff --git a/packages/replay/src/util/createPerformanceSpans.ts b/packages/replay/src/util/createPerformanceSpans.ts new file mode 100644 index 000000000000..584fe7d61a60 --- /dev/null +++ b/packages/replay/src/util/createPerformanceSpans.ts @@ -0,0 +1,29 @@ +import { EventType } from 'rrweb'; + +import { ReplayPerformanceEntry } from '../createPerformanceEntry'; +import { ReplayContainer } from '../replay'; +import { addEvent } from './addEvent'; + +/** + * Create a "span" for each performance entry. The parent transaction is `this.replayEvent`. + */ +export function createPerformanceSpans(replay: ReplayContainer, entries: ReplayPerformanceEntry[]): Promise { + return Promise.all( + entries.map(({ type, start, end, name, data }) => + addEvent(replay, { + type: EventType.Custom, + timestamp: start, + data: { + tag: 'performanceSpan', + payload: { + op: type, + description: name, + startTimestamp: start, + endTimestamp: end, + data, + }, + }, + }), + ), + ); +} diff --git a/packages/replay/test/unit/flush.test.ts b/packages/replay/test/unit/flush.test.ts index 3d4149041466..12a2887da8c8 100644 --- a/packages/replay/test/unit/flush.test.ts +++ b/packages/replay/test/unit/flush.test.ts @@ -1,6 +1,7 @@ import * as SentryUtils from '@sentry/utils'; import { SESSION_IDLE_DURATION, WINDOW } from '../../src/constants'; +import { createPerformanceSpans } from '../../src/util/createPerformanceSpans'; import { createPerformanceEntries } from './../../src/createPerformanceEntry'; import { ReplayContainer } from './../../src/replay'; import { useFakeTimers } from './../../test/utils/use-fake-timers'; @@ -179,7 +180,8 @@ it('long first flush enqueues following events', async () => { // Add this to test that segment ID increases mockAddPerformanceEntries.mockImplementationOnce(async () => { - return replay.createPerformanceSpans( + return createPerformanceSpans( + replay, createPerformanceEntries([ { name: 'https://sentry.io/foo.js', diff --git a/packages/replay/test/unit/index.test.ts b/packages/replay/test/unit/index.test.ts index fcef7c91f04f..6d29b2080ab1 100644 --- a/packages/replay/test/unit/index.test.ts +++ b/packages/replay/test/unit/index.test.ts @@ -8,6 +8,7 @@ import { MAX_SESSION_LIFE, REPLAY_SESSION_KEY, VISIBILITY_CHANGE_TIMEOUT, WINDOW import { ReplayContainer } from '../../src/replay'; import { RecordingEvent } from '../../src/types'; import { addEvent } from '../../src/util/addEvent'; +import { createPerformanceSpans } from '../../src/util/createPerformanceSpans'; import { useFakeTimers } from '../utils/use-fake-timers'; import { PerformanceEntryResource } from './../fixtures/performanceEntry/resource'; import { BASE_TIMESTAMP, RecordMock } from './../index'; @@ -461,7 +462,7 @@ describe('Replay', () => { // performance events can still be collected while recording is stopped // TODO: we may want to prevent `addEvent` from adding to buffer when user is inactive replay.addUpdate(() => { - replay.createPerformanceSpans([ + createPerformanceSpans(replay, [ { type: 'navigation.navigate', name: 'foo', From e02624127e69c8fb0da98b2f33a45b66309f5c58 Mon Sep 17 00:00:00 2001 From: Francesco Novy Date: Tue, 6 Dec 2022 17:32:18 +0100 Subject: [PATCH 09/15] ref(replay): Extract addMemoryEntry out --- packages/replay/src/replay.ts | 22 +++------------------- packages/replay/src/util/addMemoryEntry.ts | 20 ++++++++++++++++++++ packages/replay/test/unit/flush.test.ts | 6 +++--- 3 files changed, 26 insertions(+), 22 deletions(-) create mode 100644 packages/replay/src/util/addMemoryEntry.ts diff --git a/packages/replay/src/replay.ts b/packages/replay/src/replay.ts index 3f8eed299e8c..dca4a3bc8263 100644 --- a/packages/replay/src/replay.ts +++ b/packages/replay/src/replay.ts @@ -19,7 +19,7 @@ import { handleGlobalEventListener } from './coreHandlers/handleGlobalEvent'; import { handleHistorySpanListener } from './coreHandlers/handleHistory'; import { handleXhrSpanListener } from './coreHandlers/handleXhr'; import { setupPerformanceObserver } from './coreHandlers/performanceObserver'; -import { createMemoryEntry, createPerformanceEntries } from './createPerformanceEntry'; +import { createPerformanceEntries } from './createPerformanceEntry'; import { createEventBuffer, EventBuffer } from './eventBuffer'; import { deleteSession } from './session/deleteSession'; import { getSession } from './session/getSession'; @@ -36,6 +36,7 @@ import type { SendReplay, } from './types'; import { addEvent } from './util/addEvent'; +import { addMemoryEntry } from './util/addMemoryEntry'; import { captureInternalException } from './util/captureInternalException'; import { createBreadcrumb } from './util/createBreadcrumb'; import { createPayload } from './util/createPayload'; @@ -701,23 +702,6 @@ export class ReplayContainer { return createPerformanceSpans(this, createPerformanceEntries(entries)); } - /** - * Create a "span" for the total amount of memory being used by JS objects - * (including v8 internal objects). - */ - addMemoryEntry(): Promise | undefined { - // window.performance.memory is a non-standard API and doesn't work on all browsers - // so we check before creating the event. - if (!('memory' in WINDOW.performance)) { - return; - } - - return createPerformanceSpans(this, [ - // @ts-ignore memory doesn't exist on type Performance as the API is non-standard (we check that it exists above) - createMemoryEntry(WINDOW.performance.memory), - ]); - } - /** * Checks if recording should be stopped due to user inactivity. Otherwise * check if session is expired and create a new session if so. Triggers a new @@ -819,7 +803,7 @@ export class ReplayContainer { } // Only attach memory event if eventBuffer is not empty - await this.addMemoryEntry(); + await addMemoryEntry(this); try { // Note this empties the event buffer regardless of outcome of sending replay diff --git a/packages/replay/src/util/addMemoryEntry.ts b/packages/replay/src/util/addMemoryEntry.ts new file mode 100644 index 000000000000..0ee3d0a08c12 --- /dev/null +++ b/packages/replay/src/util/addMemoryEntry.ts @@ -0,0 +1,20 @@ +import { WINDOW } from '../constants'; +import { ReplayContainer } from '../replay'; +import { createPerformanceSpans } from './createPerformanceSpans'; + +/** + * Create a "span" for the total amount of memory being used by JS objects + * (including v8 internal objects). + */ +export function addMemoryEntry(replay: ReplayContainer): Promise | undefined { + // window.performance.memory is a non-standard API and doesn't work on all browsers + // so we check before creating the event. + if (!('memory' in WINDOW.performance)) { + return; + } + + return createPerformanceSpans(replay, [ + // @ts-ignore memory doesn't exist on type Performance as the API is non-standard (we check that it exists above) + createMemoryEntry(WINDOW.performance.memory), + ]); +} diff --git a/packages/replay/test/unit/flush.test.ts b/packages/replay/test/unit/flush.test.ts index 12a2887da8c8..6ba542a3dda3 100644 --- a/packages/replay/test/unit/flush.test.ts +++ b/packages/replay/test/unit/flush.test.ts @@ -1,6 +1,7 @@ import * as SentryUtils from '@sentry/utils'; import { SESSION_IDLE_DURATION, WINDOW } from '../../src/constants'; +import * as AddMemoryEntry from '../../src/util/addMemoryEntry'; import { createPerformanceSpans } from '../../src/util/createPerformanceSpans'; import { createPerformanceEntries } from './../../src/createPerformanceEntry'; import { ReplayContainer } from './../../src/replay'; @@ -16,7 +17,7 @@ async function advanceTimers(time: number) { type MockSendReplay = jest.MockedFunction; type MockAddPerformanceEntries = jest.MockedFunction; -type MockAddMemoryEntry = jest.MockedFunction; +type MockAddMemoryEntry = jest.SpyInstance; type MockEventBufferFinish = jest.MockedFunction['finish']>; type MockFlush = jest.MockedFunction; type MockRunFlush = jest.MockedFunction; @@ -63,8 +64,7 @@ beforeAll(async () => { return []; }); - jest.spyOn(replay, 'addMemoryEntry'); - mockAddMemoryEntry = replay.addMemoryEntry as MockAddMemoryEntry; + mockAddMemoryEntry = jest.spyOn(AddMemoryEntry, 'addMemoryEntry'); }); beforeEach(() => { From 3791c69a43af19d08c348bdd0973e7ff4c7b8406 Mon Sep 17 00:00:00 2001 From: Francesco Novy Date: Wed, 7 Dec 2022 09:04:09 +0100 Subject: [PATCH 10/15] doc(replay): Add jsdoc to extracted functions --- packages/replay/src/coreHandlers/handleFetch.ts | 3 +++ packages/replay/src/coreHandlers/handleGlobalEvent.ts | 3 +++ packages/replay/src/coreHandlers/handleHistory.ts | 3 +++ packages/replay/src/coreHandlers/handleXhr.ts | 3 +++ packages/replay/src/coreHandlers/performanceObserver.ts | 3 +++ 5 files changed, 15 insertions(+) diff --git a/packages/replay/src/coreHandlers/handleFetch.ts b/packages/replay/src/coreHandlers/handleFetch.ts index a3564de06e09..8ddeb9784ef8 100644 --- a/packages/replay/src/coreHandlers/handleFetch.ts +++ b/packages/replay/src/coreHandlers/handleFetch.ts @@ -45,6 +45,9 @@ export function handleFetch(handlerData: FetchHandlerData): null | ReplayPerform }; } +/** + * Returns a listener to be added to `addInstrumentationHandler('fetch', listener)`. + */ export function handleFetchSpanListener(replay: ReplayContainer): (handlerData: FetchHandlerData) => void { return (handlerData: FetchHandlerData) => { if (!replay.isEnabled()) { diff --git a/packages/replay/src/coreHandlers/handleGlobalEvent.ts b/packages/replay/src/coreHandlers/handleGlobalEvent.ts index cfe738e2dd22..589a606e1cf1 100644 --- a/packages/replay/src/coreHandlers/handleGlobalEvent.ts +++ b/packages/replay/src/coreHandlers/handleGlobalEvent.ts @@ -4,6 +4,9 @@ import { REPLAY_EVENT_NAME, UNABLE_TO_SEND_REPLAY } from '../constants'; import { ReplayContainer } from '../replay'; import { addInternalBreadcrumb } from '../util/addInternalBreadcrumb'; +/** + * Returns a listener to be added to `addGlobalEventProcessor(listener)`. + */ export function handleGlobalEventListener(replay: ReplayContainer): (event: Event) => Event { return (event: Event) => { // Do not apply replayId to the root event diff --git a/packages/replay/src/coreHandlers/handleHistory.ts b/packages/replay/src/coreHandlers/handleHistory.ts index 7757da052b42..e09e4a511200 100644 --- a/packages/replay/src/coreHandlers/handleHistory.ts +++ b/packages/replay/src/coreHandlers/handleHistory.ts @@ -23,6 +23,9 @@ function handleHistory(handlerData: HistoryHandlerData): ReplayPerformanceEntry }; } +/** + * Returns a listener to be added to `addInstrumentationHandler('history', listener)`. + */ export function handleHistorySpanListener(replay: ReplayContainer): (handlerData: HistoryHandlerData) => void { return (handlerData: HistoryHandlerData) => { if (!replay.isEnabled()) { diff --git a/packages/replay/src/coreHandlers/handleXhr.ts b/packages/replay/src/coreHandlers/handleXhr.ts index cf5138fdbe1d..d004eba7c5fe 100644 --- a/packages/replay/src/coreHandlers/handleXhr.ts +++ b/packages/replay/src/coreHandlers/handleXhr.ts @@ -64,6 +64,9 @@ function handleXhr(handlerData: XhrHandlerData): ReplayPerformanceEntry | null { }; } +/** + * Returns a listener to be added to `addInstrumentationHandler('xhr', listener)`. + */ export function handleXhrSpanListener(replay: ReplayContainer): (handlerData: XhrHandlerData) => void { return (handlerData: XhrHandlerData) => { if (!replay.isEnabled()) { diff --git a/packages/replay/src/coreHandlers/performanceObserver.ts b/packages/replay/src/coreHandlers/performanceObserver.ts index 0e21e89d10aa..0a4325a59c25 100644 --- a/packages/replay/src/coreHandlers/performanceObserver.ts +++ b/packages/replay/src/coreHandlers/performanceObserver.ts @@ -2,6 +2,9 @@ import { ReplayContainer } from '../replay'; import { AllPerformanceEntry } from '../types'; import { dedupePerformanceEntries } from '../util/dedupePerformanceEntries'; +/** + * Sets up a PerformanceObserver to listen to all performance entry types. + */ export function setupPerformanceObserver(replay: ReplayContainer): PerformanceObserver { const performanceObserverHandler = (list: PerformanceObserverEntryList): void => { // For whatever reason the observer was returning duplicate navigation From 37efedb572c9bfee38f1598d0cb7ce162691a5c0 Mon Sep 17 00:00:00 2001 From: Francesco Novy Date: Wed, 7 Dec 2022 09:42:55 +0100 Subject: [PATCH 11/15] ref(replay): Only import types for extracted replay functions --- packages/replay/src/coreHandlers/handleFetch.ts | 4 ++-- packages/replay/src/coreHandlers/handleGlobalEvent.ts | 2 +- packages/replay/src/coreHandlers/handleHistory.ts | 2 +- packages/replay/src/coreHandlers/handleXhr.ts | 2 +- packages/replay/src/coreHandlers/performanceObserver.ts | 2 +- packages/replay/src/util/addEvent.ts | 2 +- packages/replay/src/util/addMemoryEntry.ts | 2 +- packages/replay/src/util/createPerformanceSpans.ts | 2 +- 8 files changed, 9 insertions(+), 9 deletions(-) diff --git a/packages/replay/src/coreHandlers/handleFetch.ts b/packages/replay/src/coreHandlers/handleFetch.ts index 8ddeb9784ef8..21e4602d10a7 100644 --- a/packages/replay/src/coreHandlers/handleFetch.ts +++ b/packages/replay/src/coreHandlers/handleFetch.ts @@ -1,5 +1,5 @@ -import { ReplayPerformanceEntry } from '../createPerformanceEntry'; -import { ReplayContainer } from '../replay'; +import type { ReplayPerformanceEntry } from '../createPerformanceEntry'; +import type { ReplayContainer } from '../replay'; import { createPerformanceSpans } from '../util/createPerformanceSpans'; import { isIngestHost } from '../util/isIngestHost'; diff --git a/packages/replay/src/coreHandlers/handleGlobalEvent.ts b/packages/replay/src/coreHandlers/handleGlobalEvent.ts index 589a606e1cf1..66ffc575fe66 100644 --- a/packages/replay/src/coreHandlers/handleGlobalEvent.ts +++ b/packages/replay/src/coreHandlers/handleGlobalEvent.ts @@ -1,7 +1,7 @@ import { Event } from '@sentry/types'; import { REPLAY_EVENT_NAME, UNABLE_TO_SEND_REPLAY } from '../constants'; -import { ReplayContainer } from '../replay'; +import type { ReplayContainer } from '../replay'; import { addInternalBreadcrumb } from '../util/addInternalBreadcrumb'; /** diff --git a/packages/replay/src/coreHandlers/handleHistory.ts b/packages/replay/src/coreHandlers/handleHistory.ts index e09e4a511200..36382bb36687 100644 --- a/packages/replay/src/coreHandlers/handleHistory.ts +++ b/packages/replay/src/coreHandlers/handleHistory.ts @@ -1,5 +1,5 @@ import { ReplayPerformanceEntry } from '../createPerformanceEntry'; -import { ReplayContainer } from '../replay'; +import type { ReplayContainer } from '../replay'; import { createPerformanceSpans } from '../util/createPerformanceSpans'; interface HistoryHandlerData { diff --git a/packages/replay/src/coreHandlers/handleXhr.ts b/packages/replay/src/coreHandlers/handleXhr.ts index d004eba7c5fe..e6097f4dfca0 100644 --- a/packages/replay/src/coreHandlers/handleXhr.ts +++ b/packages/replay/src/coreHandlers/handleXhr.ts @@ -1,5 +1,5 @@ import { ReplayPerformanceEntry } from '../createPerformanceEntry'; -import { ReplayContainer } from '../replay'; +import type { ReplayContainer } from '../replay'; import { createPerformanceSpans } from '../util/createPerformanceSpans'; import { isIngestHost } from '../util/isIngestHost'; diff --git a/packages/replay/src/coreHandlers/performanceObserver.ts b/packages/replay/src/coreHandlers/performanceObserver.ts index 0a4325a59c25..3b8064f1dc46 100644 --- a/packages/replay/src/coreHandlers/performanceObserver.ts +++ b/packages/replay/src/coreHandlers/performanceObserver.ts @@ -1,4 +1,4 @@ -import { ReplayContainer } from '../replay'; +import type { ReplayContainer } from '../replay'; import { AllPerformanceEntry } from '../types'; import { dedupePerformanceEntries } from '../util/dedupePerformanceEntries'; diff --git a/packages/replay/src/util/addEvent.ts b/packages/replay/src/util/addEvent.ts index a9a45c1cdd4e..0f1df0c9a3d4 100644 --- a/packages/replay/src/util/addEvent.ts +++ b/packages/replay/src/util/addEvent.ts @@ -1,5 +1,5 @@ import { SESSION_IDLE_DURATION } from '../constants'; -import { ReplayContainer } from '../replay'; +import type { ReplayContainer } from '../replay'; import { RecordingEvent } from '../types'; /** diff --git a/packages/replay/src/util/addMemoryEntry.ts b/packages/replay/src/util/addMemoryEntry.ts index 0ee3d0a08c12..643eb2c50e75 100644 --- a/packages/replay/src/util/addMemoryEntry.ts +++ b/packages/replay/src/util/addMemoryEntry.ts @@ -1,5 +1,5 @@ import { WINDOW } from '../constants'; -import { ReplayContainer } from '../replay'; +import type { ReplayContainer } from '../replay'; import { createPerformanceSpans } from './createPerformanceSpans'; /** diff --git a/packages/replay/src/util/createPerformanceSpans.ts b/packages/replay/src/util/createPerformanceSpans.ts index 584fe7d61a60..aa57ba1802ad 100644 --- a/packages/replay/src/util/createPerformanceSpans.ts +++ b/packages/replay/src/util/createPerformanceSpans.ts @@ -1,7 +1,7 @@ import { EventType } from 'rrweb'; import { ReplayPerformanceEntry } from '../createPerformanceEntry'; -import { ReplayContainer } from '../replay'; +import type { ReplayContainer } from '../replay'; import { addEvent } from './addEvent'; /** From efc9525b299146fbc10026f712661b48daa0eade Mon Sep 17 00:00:00 2001 From: Francesco Novy Date: Wed, 7 Dec 2022 10:21:07 +0100 Subject: [PATCH 12/15] build(replay): Skip type imports for circular dependency check --- packages/replay/.madgerc | 7 +++++++ 1 file changed, 7 insertions(+) create mode 100644 packages/replay/.madgerc diff --git a/packages/replay/.madgerc b/packages/replay/.madgerc new file mode 100644 index 000000000000..66f0c66c6457 --- /dev/null +++ b/packages/replay/.madgerc @@ -0,0 +1,7 @@ +{ + "detectiveOptions": { + "ts": { + "skipTypeImports": true + } + } +} From 432da151e3b3b76251c570c4bf123044319055ef Mon Sep 17 00:00:00 2001 From: Francesco Novy Date: Wed, 7 Dec 2022 13:18:42 +0100 Subject: [PATCH 13/15] ref(replay): PR feedback --- .../replay/src/coreHandlers/handleGlobalEvent.ts | 10 ++++++---- packages/replay/src/util/addMemoryEntry.ts | 15 +++++++-------- 2 files changed, 13 insertions(+), 12 deletions(-) diff --git a/packages/replay/src/coreHandlers/handleGlobalEvent.ts b/packages/replay/src/coreHandlers/handleGlobalEvent.ts index 66ffc575fe66..f94b7a83b84a 100644 --- a/packages/replay/src/coreHandlers/handleGlobalEvent.ts +++ b/packages/replay/src/coreHandlers/handleGlobalEvent.ts @@ -28,13 +28,15 @@ export function handleGlobalEventListener(replay: ReplayContainer): (event: Even // Collect traceIds in _context regardless of `_waitForError` - if it's true, // _context gets cleared on every checkout - if (event.type === 'transaction') { - replay.getContext().traceIds.add(String(event.contexts?.trace?.trace_id || '')); + if (event.type === 'transaction' && event.contexts && event.contexts.trace && event.contexts.trace.trace_id) { + replay.getContext().traceIds.add(event.contexts.trace.trace_id as string); return event; } - // XXX: Is it safe to assume that all other events are error events? - replay.getContext().errorIds.add(event.event_id as string); + // no event type means error + if (!event.type) { + replay.getContext().errorIds.add(event.event_id as string); + } const exc = event.exception?.values?.[0]; addInternalBreadcrumb({ diff --git a/packages/replay/src/util/addMemoryEntry.ts b/packages/replay/src/util/addMemoryEntry.ts index 643eb2c50e75..ceba82b03a34 100644 --- a/packages/replay/src/util/addMemoryEntry.ts +++ b/packages/replay/src/util/addMemoryEntry.ts @@ -7,14 +7,13 @@ import { createPerformanceSpans } from './createPerformanceSpans'; * (including v8 internal objects). */ export function addMemoryEntry(replay: ReplayContainer): Promise | undefined { - // window.performance.memory is a non-standard API and doesn't work on all browsers - // so we check before creating the event. - if (!('memory' in WINDOW.performance)) { + // window.performance.memory is a non-standard API and doesn't work on all browsers, so we try-catch this + try { + return createPerformanceSpans(replay, [ + // @ts-ignore memory doesn't exist on type Performance as the API is non-standard (we check that it exists above) + createMemoryEntry(WINDOW.performance.memory), + ]); + } catch (error) { return; } - - return createPerformanceSpans(replay, [ - // @ts-ignore memory doesn't exist on type Performance as the API is non-standard (we check that it exists above) - createMemoryEntry(WINDOW.performance.memory), - ]); } From f7caf6467d9f79253551f85593e1b6114214834f Mon Sep 17 00:00:00 2001 From: Francesco Novy Date: Wed, 7 Dec 2022 13:36:58 +0100 Subject: [PATCH 14/15] ref(replay): Extract types into `types.ts` To avoid circular dependencies. --- packages/replay/.madgerc | 7 --- packages/replay/jest.setup.ts | 3 +- .../src/coreHandlers/breadcrumbHandler.ts | 2 +- .../replay/src/coreHandlers/handleFetch.ts | 2 +- .../src/coreHandlers/handleGlobalEvent.ts | 2 +- .../replay/src/coreHandlers/handleHistory.ts | 2 +- packages/replay/src/coreHandlers/handleXhr.ts | 2 +- .../src/coreHandlers/performanceObserver.ts | 3 +- packages/replay/src/createPerformanceEntry.ts | 2 +- packages/replay/src/eventBuffer.ts | 9 +-- packages/replay/src/replay.ts | 12 ++-- packages/replay/src/session/Session.ts | 33 +---------- packages/replay/src/session/createSession.ts | 4 +- packages/replay/src/session/fetchSession.ts | 3 +- packages/replay/src/session/getSession.ts | 3 +- packages/replay/src/session/saveSession.ts | 2 +- packages/replay/src/types.ts | 59 +++++++++++++++++++ packages/replay/src/util/addEvent.ts | 3 +- packages/replay/src/util/addMemoryEntry.ts | 2 +- packages/replay/src/util/createPayload.ts | 2 +- .../replay/src/util/createPerformanceSpans.ts | 2 +- packages/replay/src/util/isSessionExpired.ts | 2 +- .../test/fixtures/performanceEntry/lcp.ts | 2 +- packages/replay/test/mocks/mockRrweb.ts | 2 +- packages/replay/test/mocks/mockSdk.ts | 2 +- packages/replay/test/mocks/resetSdkMock.ts | 4 +- packages/replay/test/unit/index.test.ts | 2 +- 27 files changed, 93 insertions(+), 80 deletions(-) delete mode 100644 packages/replay/.madgerc diff --git a/packages/replay/.madgerc b/packages/replay/.madgerc deleted file mode 100644 index 66f0c66c6457..000000000000 --- a/packages/replay/.madgerc +++ /dev/null @@ -1,7 +0,0 @@ -{ - "detectiveOptions": { - "ts": { - "skipTypeImports": true - } - } -} diff --git a/packages/replay/jest.setup.ts b/packages/replay/jest.setup.ts index 24e2e2088aa4..481221831f73 100644 --- a/packages/replay/jest.setup.ts +++ b/packages/replay/jest.setup.ts @@ -2,8 +2,7 @@ import { getCurrentHub } from '@sentry/core'; import { Transport } from '@sentry/types'; -import { ReplayContainer } from './src/replay'; -import { Session } from './src/session/Session'; +import type { ReplayContainer, Session } from './src/types'; // @ts-ignore TS error, this is replaced in prod builds bc of rollup global.__SENTRY_REPLAY_VERSION__ = 'version:Test'; diff --git a/packages/replay/src/coreHandlers/breadcrumbHandler.ts b/packages/replay/src/coreHandlers/breadcrumbHandler.ts index 8f3dd1e78983..fe0504b0230f 100644 --- a/packages/replay/src/coreHandlers/breadcrumbHandler.ts +++ b/packages/replay/src/coreHandlers/breadcrumbHandler.ts @@ -1,6 +1,6 @@ import { Breadcrumb, Scope } from '@sentry/types'; -import { InstrumentationTypeBreadcrumb } from '../types'; +import type { InstrumentationTypeBreadcrumb } from '../types'; import { DomHandlerData, handleDom } from './handleDom'; import { handleScope } from './handleScope'; diff --git a/packages/replay/src/coreHandlers/handleFetch.ts b/packages/replay/src/coreHandlers/handleFetch.ts index 21e4602d10a7..135afc24cde1 100644 --- a/packages/replay/src/coreHandlers/handleFetch.ts +++ b/packages/replay/src/coreHandlers/handleFetch.ts @@ -1,5 +1,5 @@ import type { ReplayPerformanceEntry } from '../createPerformanceEntry'; -import type { ReplayContainer } from '../replay'; +import type { ReplayContainer } from '../types'; import { createPerformanceSpans } from '../util/createPerformanceSpans'; import { isIngestHost } from '../util/isIngestHost'; diff --git a/packages/replay/src/coreHandlers/handleGlobalEvent.ts b/packages/replay/src/coreHandlers/handleGlobalEvent.ts index f94b7a83b84a..1fa843a86cfd 100644 --- a/packages/replay/src/coreHandlers/handleGlobalEvent.ts +++ b/packages/replay/src/coreHandlers/handleGlobalEvent.ts @@ -1,7 +1,7 @@ import { Event } from '@sentry/types'; import { REPLAY_EVENT_NAME, UNABLE_TO_SEND_REPLAY } from '../constants'; -import type { ReplayContainer } from '../replay'; +import type { ReplayContainer } from '../types'; import { addInternalBreadcrumb } from '../util/addInternalBreadcrumb'; /** diff --git a/packages/replay/src/coreHandlers/handleHistory.ts b/packages/replay/src/coreHandlers/handleHistory.ts index 36382bb36687..072cbadb5a07 100644 --- a/packages/replay/src/coreHandlers/handleHistory.ts +++ b/packages/replay/src/coreHandlers/handleHistory.ts @@ -1,5 +1,5 @@ import { ReplayPerformanceEntry } from '../createPerformanceEntry'; -import type { ReplayContainer } from '../replay'; +import type { ReplayContainer } from '../types'; import { createPerformanceSpans } from '../util/createPerformanceSpans'; interface HistoryHandlerData { diff --git a/packages/replay/src/coreHandlers/handleXhr.ts b/packages/replay/src/coreHandlers/handleXhr.ts index e6097f4dfca0..fcc073a6d5a5 100644 --- a/packages/replay/src/coreHandlers/handleXhr.ts +++ b/packages/replay/src/coreHandlers/handleXhr.ts @@ -1,5 +1,5 @@ import { ReplayPerformanceEntry } from '../createPerformanceEntry'; -import type { ReplayContainer } from '../replay'; +import type { ReplayContainer } from '../types'; import { createPerformanceSpans } from '../util/createPerformanceSpans'; import { isIngestHost } from '../util/isIngestHost'; diff --git a/packages/replay/src/coreHandlers/performanceObserver.ts b/packages/replay/src/coreHandlers/performanceObserver.ts index 3b8064f1dc46..a7dc1de13244 100644 --- a/packages/replay/src/coreHandlers/performanceObserver.ts +++ b/packages/replay/src/coreHandlers/performanceObserver.ts @@ -1,5 +1,4 @@ -import type { ReplayContainer } from '../replay'; -import { AllPerformanceEntry } from '../types'; +import type { AllPerformanceEntry, ReplayContainer } from '../types'; import { dedupePerformanceEntries } from '../util/dedupePerformanceEntries'; /** diff --git a/packages/replay/src/createPerformanceEntry.ts b/packages/replay/src/createPerformanceEntry.ts index 329366a8a04f..33f6ae1a6a4d 100644 --- a/packages/replay/src/createPerformanceEntry.ts +++ b/packages/replay/src/createPerformanceEntry.ts @@ -2,7 +2,7 @@ import { browserPerformanceTimeOrigin } from '@sentry/utils'; import { record } from 'rrweb'; import { WINDOW } from './constants'; -import { AllPerformanceEntry, PerformanceNavigationTiming, PerformancePaintTiming } from './types'; +import type { AllPerformanceEntry, PerformanceNavigationTiming, PerformancePaintTiming } from './types'; import { isIngestHost } from './util/isIngestHost'; export interface ReplayPerformanceEntry { diff --git a/packages/replay/src/eventBuffer.ts b/packages/replay/src/eventBuffer.ts index 3ebf87229668..b83942be4f67 100644 --- a/packages/replay/src/eventBuffer.ts +++ b/packages/replay/src/eventBuffer.ts @@ -4,7 +4,7 @@ import { captureException } from '@sentry/core'; import { logger } from '@sentry/utils'; -import { RecordingEvent, WorkerRequest, WorkerResponse } from './types'; +import type { EventBuffer, RecordingEvent, WorkerRequest, WorkerResponse } from './types'; import workerString from './worker/worker.js'; interface CreateEventBufferParams { @@ -35,13 +35,6 @@ export function createEventBuffer({ useCompression }: CreateEventBufferParams): return new EventBufferArray(); } -export interface EventBuffer { - readonly length: number; - destroy(): void; - addEvent(event: RecordingEvent, isCheckout?: boolean): void; - finish(): Promise; -} - class EventBufferArray implements EventBuffer { private _events: RecordingEvent[]; diff --git a/packages/replay/src/replay.ts b/packages/replay/src/replay.ts index dca4a3bc8263..ac1b1feff438 100644 --- a/packages/replay/src/replay.ts +++ b/packages/replay/src/replay.ts @@ -20,20 +20,23 @@ import { handleHistorySpanListener } from './coreHandlers/handleHistory'; import { handleXhrSpanListener } from './coreHandlers/handleXhr'; import { setupPerformanceObserver } from './coreHandlers/performanceObserver'; import { createPerformanceEntries } from './createPerformanceEntry'; -import { createEventBuffer, EventBuffer } from './eventBuffer'; +import { createEventBuffer } from './eventBuffer'; import { deleteSession } from './session/deleteSession'; import { getSession } from './session/getSession'; import { saveSession } from './session/saveSession'; -import { Session } from './session/Session'; import type { + AddUpdateCallback, AllPerformanceEntry, + EventBuffer, InstrumentationTypeBreadcrumb, InternalEventContext, PopEventContext, RecordingEvent, RecordingOptions, + ReplayContainer as ReplayContainerInterface, ReplayPluginOptions, SendReplay, + Session, } from './types'; import { addEvent } from './util/addEvent'; import { addMemoryEntry } from './util/addMemoryEntry'; @@ -48,12 +51,11 @@ import { overwriteRecordDroppedEvent, restoreRecordDroppedEvent } from './util/m /** * Returns true to return control to calling function, otherwise continue with normal batching */ -type AddUpdateCallback = () => boolean | void; const BASE_RETRY_INTERVAL = 5000; const MAX_RETRY_COUNT = 3; -export class ReplayContainer { +export class ReplayContainer implements ReplayContainerInterface { public eventBuffer: EventBuffer | null = null; /** @@ -398,7 +400,7 @@ export class ReplayContainer { * Accepts a callback to perform side-effects and returns true to stop batch * processing and hand back control to caller. */ - addUpdate(cb?: AddUpdateCallback): void { + addUpdate(cb: AddUpdateCallback): void { // We need to always run `cb` (e.g. in the case of `this._waitForError == true`) const cbResult = cb?.(); diff --git a/packages/replay/src/session/Session.ts b/packages/replay/src/session/Session.ts index 2936a8c19dda..d20fa0daf33c 100644 --- a/packages/replay/src/session/Session.ts +++ b/packages/replay/src/session/Session.ts @@ -1,39 +1,8 @@ import { uuid4 } from '@sentry/utils'; +import type { Sampled, Session } from '../types'; import { isSampled } from '../util/isSampled'; -type Sampled = false | 'session' | 'error'; - -export interface Session { - id: string; - - /** - * Start time of current session - */ - started: number; - - /** - * Last known activity of the session - */ - lastActivity: number; - - /** - * Segment ID for replay events - */ - segmentId: number; - - /** - * The ID of the previous session. - * If this is empty, there was no previous session. - */ - previousSessionId?: string; - - /** - * Is the session sampled? `false` if not sampled, otherwise, `session` or `error` - */ - sampled: Sampled; -} - /** * Get a session with defaults & applied sampling. */ diff --git a/packages/replay/src/session/createSession.ts b/packages/replay/src/session/createSession.ts index 5dcb015fe615..bd6d18ad33e8 100644 --- a/packages/replay/src/session/createSession.ts +++ b/packages/replay/src/session/createSession.ts @@ -1,8 +1,8 @@ import { logger } from '@sentry/utils'; -import { SessionOptions } from '../types'; +import type { Session, SessionOptions } from '../types'; import { saveSession } from './saveSession'; -import { getSessionSampleType, makeSession, Session } from './Session'; +import { getSessionSampleType, makeSession } from './Session'; /** * Create a new session, which in its current implementation is a Sentry event diff --git a/packages/replay/src/session/fetchSession.ts b/packages/replay/src/session/fetchSession.ts index c2dc95c9454c..4b4b1eccf530 100644 --- a/packages/replay/src/session/fetchSession.ts +++ b/packages/replay/src/session/fetchSession.ts @@ -1,5 +1,6 @@ import { REPLAY_SESSION_KEY, WINDOW } from '../constants'; -import { makeSession, Session } from './Session'; +import type { Session } from '../types'; +import { makeSession } from './Session'; /** * Fetches a session from storage diff --git a/packages/replay/src/session/getSession.ts b/packages/replay/src/session/getSession.ts index 103008061ad0..254fd33866c8 100644 --- a/packages/replay/src/session/getSession.ts +++ b/packages/replay/src/session/getSession.ts @@ -1,10 +1,9 @@ import { logger } from '@sentry/utils'; -import { SessionOptions } from '../types'; +import type { Session, SessionOptions } from '../types'; import { isSessionExpired } from '../util/isSessionExpired'; import { createSession } from './createSession'; import { fetchSession } from './fetchSession'; -import { Session } from './Session'; interface GetSessionParams extends SessionOptions { /** diff --git a/packages/replay/src/session/saveSession.ts b/packages/replay/src/session/saveSession.ts index 4f3e1804a25e..a506625436f8 100644 --- a/packages/replay/src/session/saveSession.ts +++ b/packages/replay/src/session/saveSession.ts @@ -1,5 +1,5 @@ import { REPLAY_SESSION_KEY, WINDOW } from '../constants'; -import { Session } from './Session'; +import type { Session } from '../types'; export function saveSession(session: Session): void { const hasSessionStorage = 'sessionStorage' in WINDOW; diff --git a/packages/replay/src/types.ts b/packages/replay/src/types.ts index 6f247e12d226..18d792d27eac 100644 --- a/packages/replay/src/types.ts +++ b/packages/replay/src/types.ts @@ -160,3 +160,62 @@ export interface InternalEventContext extends CommonEventContext { */ earliestEvent: number | null; } + +export type Sampled = false | 'session' | 'error'; + +export interface Session { + id: string; + + /** + * Start time of current session + */ + started: number; + + /** + * Last known activity of the session + */ + lastActivity: number; + + /** + * Segment ID for replay events + */ + segmentId: number; + + /** + * The ID of the previous session. + * If this is empty, there was no previous session. + */ + previousSessionId?: string; + + /** + * Is the session sampled? `false` if not sampled, otherwise, `session` or `error` + */ + sampled: Sampled; +} + +export interface EventBuffer { + readonly length: number; + destroy(): void; + addEvent(event: RecordingEvent, isCheckout?: boolean): void; + finish(): Promise; +} + +export type AddUpdateCallback = () => boolean | void; + +export interface ReplayContainer { + eventBuffer: EventBuffer | null; + performanceEvents: AllPerformanceEntry[]; + session: Session | undefined; + isEnabled(): boolean; + isPaused(): boolean; + getContext(): InternalEventContext; + start(): void; + stop(): void; + pause(): void; + resume(): void; + startRecording(): void; + stopRecording(): boolean; + flushImmediate(): void; + triggerUserActivity(): void; + addUpdate(cb: AddUpdateCallback): void; +} diff --git a/packages/replay/src/util/addEvent.ts b/packages/replay/src/util/addEvent.ts index 0f1df0c9a3d4..34086a4725c5 100644 --- a/packages/replay/src/util/addEvent.ts +++ b/packages/replay/src/util/addEvent.ts @@ -1,6 +1,5 @@ import { SESSION_IDLE_DURATION } from '../constants'; -import type { ReplayContainer } from '../replay'; -import { RecordingEvent } from '../types'; +import type { RecordingEvent, ReplayContainer } from '../types'; /** * Add an event to the event buffer diff --git a/packages/replay/src/util/addMemoryEntry.ts b/packages/replay/src/util/addMemoryEntry.ts index ceba82b03a34..b7ad3d2c11a0 100644 --- a/packages/replay/src/util/addMemoryEntry.ts +++ b/packages/replay/src/util/addMemoryEntry.ts @@ -1,5 +1,5 @@ import { WINDOW } from '../constants'; -import type { ReplayContainer } from '../replay'; +import type { ReplayContainer } from '../types'; import { createPerformanceSpans } from './createPerformanceSpans'; /** diff --git a/packages/replay/src/util/createPayload.ts b/packages/replay/src/util/createPayload.ts index 222f52cfd4d7..6567703022ef 100644 --- a/packages/replay/src/util/createPayload.ts +++ b/packages/replay/src/util/createPayload.ts @@ -1,4 +1,4 @@ -import { RecordedEvents } from '../types'; +import type { RecordedEvents } from '../types'; export function createPayload({ events, diff --git a/packages/replay/src/util/createPerformanceSpans.ts b/packages/replay/src/util/createPerformanceSpans.ts index aa57ba1802ad..d3d0645d82c5 100644 --- a/packages/replay/src/util/createPerformanceSpans.ts +++ b/packages/replay/src/util/createPerformanceSpans.ts @@ -1,7 +1,7 @@ import { EventType } from 'rrweb'; import { ReplayPerformanceEntry } from '../createPerformanceEntry'; -import type { ReplayContainer } from '../replay'; +import type { ReplayContainer } from '../types'; import { addEvent } from './addEvent'; /** diff --git a/packages/replay/src/util/isSessionExpired.ts b/packages/replay/src/util/isSessionExpired.ts index 78e12ecf87fd..05ce3d21f69a 100644 --- a/packages/replay/src/util/isSessionExpired.ts +++ b/packages/replay/src/util/isSessionExpired.ts @@ -1,5 +1,5 @@ import { MAX_SESSION_LIFE } from '../constants'; -import { Session } from '../session/Session'; +import type { Session } from '../types'; import { isExpired } from './isExpired'; /** diff --git a/packages/replay/test/fixtures/performanceEntry/lcp.ts b/packages/replay/test/fixtures/performanceEntry/lcp.ts index 761e75d56e8f..7ad0bf7b4637 100644 --- a/packages/replay/test/fixtures/performanceEntry/lcp.ts +++ b/packages/replay/test/fixtures/performanceEntry/lcp.ts @@ -1,4 +1,4 @@ -import { PerformancePaintTiming } from '../../../src/types'; +import type { PerformancePaintTiming } from '../../../src/types'; export function PerformanceEntryLcp(obj?: Partial): PerformancePaintTiming { const entry = { diff --git a/packages/replay/test/mocks/mockRrweb.ts b/packages/replay/test/mocks/mockRrweb.ts index 6e4b49aad8fb..92a93e00e885 100644 --- a/packages/replay/test/mocks/mockRrweb.ts +++ b/packages/replay/test/mocks/mockRrweb.ts @@ -1,6 +1,6 @@ import type { record as rrwebRecord } from 'rrweb'; -import { RecordingEvent } from '../../src/types'; +import type { RecordingEvent } from '../../src/types'; type RecordAdditionalProperties = { takeFullSnapshot: jest.Mock; diff --git a/packages/replay/test/mocks/mockSdk.ts b/packages/replay/test/mocks/mockSdk.ts index d9d8f2f4a495..93134081adef 100644 --- a/packages/replay/test/mocks/mockSdk.ts +++ b/packages/replay/test/mocks/mockSdk.ts @@ -3,7 +3,7 @@ import { Envelope, Transport } from '@sentry/types'; import { Replay as ReplayIntegration } from '../../src'; import { ReplayContainer } from '../../src/replay'; -import { ReplayConfiguration } from '../../src/types'; +import type { ReplayConfiguration } from '../../src/types'; export interface MockSdkParams { replayOptions?: ReplayConfiguration; diff --git a/packages/replay/test/mocks/resetSdkMock.ts b/packages/replay/test/mocks/resetSdkMock.ts index fb3d9772be66..bc6b83ab7a08 100644 --- a/packages/replay/test/mocks/resetSdkMock.ts +++ b/packages/replay/test/mocks/resetSdkMock.ts @@ -1,8 +1,8 @@ import { getCurrentHub } from '@sentry/core'; -import { ReplayContainer } from '../../src/replay'; +import type { ReplayContainer } from '../../src/replay'; import { BASE_TIMESTAMP, RecordMock } from './../index'; -import { DomHandler, MockTransportSend } from './../types'; +import type { DomHandler, MockTransportSend } from './../types'; import { mockSdk, MockSdkParams } from './mockSdk'; export async function resetSdkMock({ replayOptions, sentryOptions }: MockSdkParams): Promise<{ diff --git a/packages/replay/test/unit/index.test.ts b/packages/replay/test/unit/index.test.ts index 6d29b2080ab1..8464766734db 100644 --- a/packages/replay/test/unit/index.test.ts +++ b/packages/replay/test/unit/index.test.ts @@ -6,7 +6,7 @@ import { EventType } from 'rrweb'; import { MAX_SESSION_LIFE, REPLAY_SESSION_KEY, VISIBILITY_CHANGE_TIMEOUT, WINDOW } from '../../src/constants'; import { ReplayContainer } from '../../src/replay'; -import { RecordingEvent } from '../../src/types'; +import type { RecordingEvent } from '../../src/types'; import { addEvent } from '../../src/util/addEvent'; import { createPerformanceSpans } from '../../src/util/createPerformanceSpans'; import { useFakeTimers } from '../utils/use-fake-timers'; From cf54a67ac3faea041bc1d9c815fdbfec49edadc3 Mon Sep 17 00:00:00 2001 From: Francesco Novy Date: Wed, 7 Dec 2022 17:07:33 +0100 Subject: [PATCH 15/15] ref(replay): Make `addMemoryEntry` and `createPerformanceSpans` to sync --- .../replay/src/coreHandlers/handleFetch.ts | 2 +- .../replay/src/coreHandlers/handleHistory.ts | 2 +- packages/replay/src/coreHandlers/handleXhr.ts | 2 +- packages/replay/src/replay.ts | 4 +-- packages/replay/src/util/addMemoryEntry.ts | 6 ++-- .../replay/src/util/createPerformanceSpans.ts | 32 +++++++++---------- packages/replay/test/unit/flush.test.ts | 4 +-- 7 files changed, 25 insertions(+), 27 deletions(-) diff --git a/packages/replay/src/coreHandlers/handleFetch.ts b/packages/replay/src/coreHandlers/handleFetch.ts index 135afc24cde1..1b6430d52413 100644 --- a/packages/replay/src/coreHandlers/handleFetch.ts +++ b/packages/replay/src/coreHandlers/handleFetch.ts @@ -61,7 +61,7 @@ export function handleFetchSpanListener(replay: ReplayContainer): (handlerData: } replay.addUpdate(() => { - void createPerformanceSpans(replay, [result]); + createPerformanceSpans(replay, [result]); // Returning true will cause `addUpdate` to not flush // We do not want network requests to cause a flush. This will prevent // recurring/polling requests from keeping the replay session alive. diff --git a/packages/replay/src/coreHandlers/handleHistory.ts b/packages/replay/src/coreHandlers/handleHistory.ts index 072cbadb5a07..f806d2d3c75b 100644 --- a/packages/replay/src/coreHandlers/handleHistory.ts +++ b/packages/replay/src/coreHandlers/handleHistory.ts @@ -43,7 +43,7 @@ export function handleHistorySpanListener(replay: ReplayContainer): (handlerData replay.triggerUserActivity(); replay.addUpdate(() => { - void createPerformanceSpans(replay, [result]); + createPerformanceSpans(replay, [result]); // Returning false to flush return false; }); diff --git a/packages/replay/src/coreHandlers/handleXhr.ts b/packages/replay/src/coreHandlers/handleXhr.ts index fcc073a6d5a5..6577e91f3b92 100644 --- a/packages/replay/src/coreHandlers/handleXhr.ts +++ b/packages/replay/src/coreHandlers/handleXhr.ts @@ -80,7 +80,7 @@ export function handleXhrSpanListener(replay: ReplayContainer): (handlerData: Xh } replay.addUpdate(() => { - void createPerformanceSpans(replay, [result]); + createPerformanceSpans(replay, [result]); // Returning true will cause `addUpdate` to not flush // We do not want network requests to cause a flush. This will prevent // recurring/polling requests from keeping the replay session alive. diff --git a/packages/replay/src/replay.ts b/packages/replay/src/replay.ts index ac1b1feff438..ef5c4636f1fa 100644 --- a/packages/replay/src/replay.ts +++ b/packages/replay/src/replay.ts @@ -696,12 +696,12 @@ export class ReplayContainer implements ReplayContainerInterface { * Observed performance events are added to `this.performanceEvents`. These * are included in the replay event before it is finished and sent to Sentry. */ - addPerformanceEntries(): Promise { + addPerformanceEntries(): void { // Copy and reset entries before processing const entries = [...this.performanceEvents]; this.performanceEvents = []; - return createPerformanceSpans(this, createPerformanceEntries(entries)); + createPerformanceSpans(this, createPerformanceEntries(entries)); } /** diff --git a/packages/replay/src/util/addMemoryEntry.ts b/packages/replay/src/util/addMemoryEntry.ts index b7ad3d2c11a0..bac07200cd6c 100644 --- a/packages/replay/src/util/addMemoryEntry.ts +++ b/packages/replay/src/util/addMemoryEntry.ts @@ -6,14 +6,14 @@ import { createPerformanceSpans } from './createPerformanceSpans'; * Create a "span" for the total amount of memory being used by JS objects * (including v8 internal objects). */ -export function addMemoryEntry(replay: ReplayContainer): Promise | undefined { +export function addMemoryEntry(replay: ReplayContainer): void { // window.performance.memory is a non-standard API and doesn't work on all browsers, so we try-catch this try { - return createPerformanceSpans(replay, [ + createPerformanceSpans(replay, [ // @ts-ignore memory doesn't exist on type Performance as the API is non-standard (we check that it exists above) createMemoryEntry(WINDOW.performance.memory), ]); } catch (error) { - return; + // Do nothing } } diff --git a/packages/replay/src/util/createPerformanceSpans.ts b/packages/replay/src/util/createPerformanceSpans.ts index d3d0645d82c5..9bb999a0faa3 100644 --- a/packages/replay/src/util/createPerformanceSpans.ts +++ b/packages/replay/src/util/createPerformanceSpans.ts @@ -7,23 +7,21 @@ import { addEvent } from './addEvent'; /** * Create a "span" for each performance entry. The parent transaction is `this.replayEvent`. */ -export function createPerformanceSpans(replay: ReplayContainer, entries: ReplayPerformanceEntry[]): Promise { - return Promise.all( - entries.map(({ type, start, end, name, data }) => - addEvent(replay, { - type: EventType.Custom, - timestamp: start, - data: { - tag: 'performanceSpan', - payload: { - op: type, - description: name, - startTimestamp: start, - endTimestamp: end, - data, - }, +export function createPerformanceSpans(replay: ReplayContainer, entries: ReplayPerformanceEntry[]): void { + entries.map(({ type, start, end, name, data }) => + addEvent(replay, { + type: EventType.Custom, + timestamp: start, + data: { + tag: 'performanceSpan', + payload: { + op: type, + description: name, + startTimestamp: start, + endTimestamp: end, + data, }, - }), - ), + }, + }), ); } diff --git a/packages/replay/test/unit/flush.test.ts b/packages/replay/test/unit/flush.test.ts index 6ba542a3dda3..39f1931c73cb 100644 --- a/packages/replay/test/unit/flush.test.ts +++ b/packages/replay/test/unit/flush.test.ts @@ -179,8 +179,8 @@ it('long first flush enqueues following events', async () => { }); // Add this to test that segment ID increases - mockAddPerformanceEntries.mockImplementationOnce(async () => { - return createPerformanceSpans( + mockAddPerformanceEntries.mockImplementationOnce(() => { + createPerformanceSpans( replay, createPerformanceEntries([ {