From d9ff3a9856b8b0372827690daf71c727ab8e0446 Mon Sep 17 00:00:00 2001 From: Paul D'Ambra Date: Thu, 3 Oct 2024 23:11:40 +0100 Subject: [PATCH 1/9] feat: report reason for recording start --- src/extensions/replay/sessionrecording.ts | 15 ++++++++++++++- src/types.ts | 6 ++++++ 2 files changed, 20 insertions(+), 1 deletion(-) diff --git a/src/extensions/replay/sessionrecording.ts b/src/extensions/replay/sessionrecording.ts index f7c6d5162..87e364fb1 100644 --- a/src/extensions/replay/sessionrecording.ts +++ b/src/extensions/replay/sessionrecording.ts @@ -495,6 +495,10 @@ export class SessionRecording { if (makeDecision) { const randomNumber = Math.random() shouldSample = randomNumber < currentSampleRate + this._reportStarted('sampling', { + sampleRate: currentSampleRate, + randomNumber, + }) } else { shouldSample = storedIsSampled } @@ -539,6 +543,7 @@ export class SessionRecording { const tag = 'linked flag matched' logger.info(LOGGER_PREFIX + ' ' + tag, payload) this._tryAddCustomEvent(tag, payload) + this._reportStarted('linked_flag_match', payload) } this._linkedFlagSeen = linkedFlagMatches }) @@ -711,7 +716,7 @@ export class SessionRecording { } // We only want to extend the session if it is an interactive event. - const { windowId, sessionId } = this.sessionManager.checkAndGetSessionAndWindowId( + const { windowId, sessionId, changeReason } = this.sessionManager.checkAndGetSessionAndWindowId( !isUserInteraction, event.timestamp ) @@ -725,6 +730,7 @@ export class SessionRecording { if (sessionIdChanged || windowIdChanged) { this.stopRecording() this.startIfEnabledOrStop() + this._reportStarted('session_id_change', { changeReason }) } else if (returningFromIdle) { this._scheduleFullSnapshot() } @@ -1099,5 +1105,12 @@ export class SessionRecording { * */ public overrideLinkedFlag() { this._linkedFlagSeen = true + this._reportStarted('linked flag override') + } + + private _reportStarted(reason: string, properties?: Properties) { + if (this.instance.config.session_recording.report_recording_started) { + this.instance.capture('$session_recording_started', { ...(properties || {}), reason }) + } } } diff --git a/src/types.ts b/src/types.ts index 9133fad89..994d5daae 100644 --- a/src/types.ts +++ b/src/types.ts @@ -289,6 +289,12 @@ export interface SessionRecordingOptions { Default is 5 minutes. */ session_idle_threshold_ms?: number + /* + Sends an event whenever a recording starts, with the reason the recording started. + Useful, for example, when using ingestion controls + to have a signal in batch exports to know which sessions have recordings + */ + report_recording_started?: boolean } export type SessionIdChangedCallback = ( From 7b055b03e0dfa4af8859dbf643eb52f2692934de Mon Sep 17 00:00:00 2001 From: Paul D'Ambra Date: Fri, 4 Oct 2024 10:56:21 +0100 Subject: [PATCH 2/9] Update src/extensions/replay/sessionrecording.ts Co-authored-by: Manoel Aranda Neto <5731772+marandaneto@users.noreply.github.com> --- src/extensions/replay/sessionrecording.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/extensions/replay/sessionrecording.ts b/src/extensions/replay/sessionrecording.ts index 87e364fb1..005d2e7dd 100644 --- a/src/extensions/replay/sessionrecording.ts +++ b/src/extensions/replay/sessionrecording.ts @@ -1105,7 +1105,7 @@ export class SessionRecording { * */ public overrideLinkedFlag() { this._linkedFlagSeen = true - this._reportStarted('linked flag override') + this._reportStarted('linked_flag_override') } private _reportStarted(reason: string, properties?: Properties) { From 30af3967cc73ce76b96164b6cb077663b2564f8b Mon Sep 17 00:00:00 2001 From: Paul D'Ambra Date: Fri, 4 Oct 2024 12:05:17 +0100 Subject: [PATCH 3/9] and report when recording boots and starts --- src/extensions/replay/sessionrecording.ts | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/extensions/replay/sessionrecording.ts b/src/extensions/replay/sessionrecording.ts index 005d2e7dd..eab1458b9 100644 --- a/src/extensions/replay/sessionrecording.ts +++ b/src/extensions/replay/sessionrecording.ts @@ -848,7 +848,8 @@ export class SessionRecording { config: this.instance.config, }) - logger.info(LOGGER_PREFIX + ' started', { + logger.info(LOGGER_PREFIX + ' started') + this._reportStarted('recording_initialized', { idleThreshold: this.sessionIdleThresholdMilliseconds, maxIdleTime: this.sessionManager.sessionTimeoutMs, }) From 4dee56b96ece7c12fd4c3aa51f488824202195d3 Mon Sep 17 00:00:00 2001 From: Paul D'Ambra Date: Fri, 4 Oct 2024 12:30:15 +0100 Subject: [PATCH 4/9] a little easier to search for --- src/extensions/replay/sessionrecording.ts | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/src/extensions/replay/sessionrecording.ts b/src/extensions/replay/sessionrecording.ts index eab1458b9..c28cfaa8e 100644 --- a/src/extensions/replay/sessionrecording.ts +++ b/src/extensions/replay/sessionrecording.ts @@ -1111,7 +1111,10 @@ export class SessionRecording { private _reportStarted(reason: string, properties?: Properties) { if (this.instance.config.session_recording.report_recording_started) { - this.instance.capture('$session_recording_started', { ...(properties || {}), reason }) + this.instance.capture('$session_recording_started', { + ...(properties || {}), + $session_recording_started_reason: reason, + }) } } } From 4de2b28952c50915958ea837c5abfd7bca124cb7 Mon Sep 17 00:00:00 2001 From: Paul D'Ambra Date: Fri, 4 Oct 2024 23:17:29 +0100 Subject: [PATCH 5/9] fixes --- cypress/e2e/session-recording.cy.ts | 153 +++++++++++++++------- src/extensions/replay/sessionrecording.ts | 88 +++++++++---- src/posthog-core.ts | 22 ++-- 3 files changed, 179 insertions(+), 84 deletions(-) diff --git a/cypress/e2e/session-recording.cy.ts b/cypress/e2e/session-recording.cy.ts index 4c3fa0d03..286652d27 100644 --- a/cypress/e2e/session-recording.cy.ts +++ b/cypress/e2e/session-recording.cy.ts @@ -60,23 +60,25 @@ function ensureActivitySendsSnapshots(initial = true) { .wait('@session-recording') .then(() => { cy.phCaptures({ full: true }).then((captures) => { - expect(captures.map((c) => c.event)).to.deep.equal(['$snapshot']) - expect(captures[0]['properties']['$snapshot_data']).to.have.length.above(14).and.below(40) + const capturedSnapshot = captures.find((e) => e.event === '$snapshot') + expect(capturedSnapshot).not.to.be.undefined + + expect(capturedSnapshot['properties']['$snapshot_data']).to.have.length.above(14).and.below(40) // a meta and then a full snapshot - expect(captures[0]['properties']['$snapshot_data'][0].type).to.equal(4) // meta - expect(captures[0]['properties']['$snapshot_data'][1].type).to.equal(2) // full_snapshot + expect(capturedSnapshot['properties']['$snapshot_data'][0].type).to.equal(4) // meta + expect(capturedSnapshot['properties']['$snapshot_data'][1].type).to.equal(2) // full_snapshot if (initial) { - expectSessionOptionsCustomEvent(captures[0]['properties']['$snapshot_data'][2]) - expectPostHogConfigCustomEvent(captures[0]['properties']['$snapshot_data'][3]) + expectSessionOptionsCustomEvent(capturedSnapshot['properties']['$snapshot_data'][2]) + expectPostHogConfigCustomEvent(capturedSnapshot['properties']['$snapshot_data'][3]) } else { - expectSessionOptionsCustomEvent(captures[0]['properties']['$snapshot_data'][2]) - expectPostHogConfigCustomEvent(captures[0]['properties']['$snapshot_data'][3]) - expectSessionIdChangedCustomEvent(captures[0]['properties']['$snapshot_data'][4]) + expectSessionOptionsCustomEvent(capturedSnapshot['properties']['$snapshot_data'][2]) + expectPostHogConfigCustomEvent(capturedSnapshot['properties']['$snapshot_data'][3]) + expectSessionIdChangedCustomEvent(capturedSnapshot['properties']['$snapshot_data'][4]) } // Making a set from the rest should all be 3 - incremental snapshots - const remainder = captures[0]['properties']['$snapshot_data'].slice(initial ? 4 : 5) + const remainder = capturedSnapshot['properties']['$snapshot_data'].slice(initial ? 4 : 5) expect(Array.from(new Set(remainder.map((s) => s.type)))).to.deep.equal([3]) }) }) @@ -112,6 +114,11 @@ describe('Session recording', () => { describe('array.full.js', () => { it('captures session events', () => { start({ + options: { + session_recording: { + report_recording_started: true, + }, + }, decideResponseOverrides: { isAuthenticated: false, sessionRecording: { @@ -129,17 +136,24 @@ describe('Session recording', () => { .wait('@session-recording') .then(() => { cy.phCaptures({ full: true }).then((captures) => { - // should be a pageview and a $snapshot - expect(captures.map((c) => c.event)).to.deep.equal(['$pageview', '$snapshot']) + expect(captures.map((c) => c.event)).to.deep.equal([ + '$pageview', + '$session_recording_started', + '$snapshot', + ]) + + expect(captures[1]['properties']['$session_recording_started_reason']).to.equal( + 'recording_initialized' + ) - expect(captures[1]['properties']['$snapshot_data']).to.have.length.above(33).and.below(40) + expect(captures[2]['properties']['$snapshot_data']).to.have.length.above(33).and.below(40) // a meta and then a full snapshot - expect(captures[1]['properties']['$snapshot_data'][0].type).to.equal(4) // meta - expect(captures[1]['properties']['$snapshot_data'][1].type).to.equal(2) // full_snapshot - expect(captures[1]['properties']['$snapshot_data'][2].type).to.equal(5) // custom event with options - expect(captures[1]['properties']['$snapshot_data'][3].type).to.equal(5) // custom event with posthog config + expect(captures[2]['properties']['$snapshot_data'][0].type).to.equal(4) // meta + expect(captures[2]['properties']['$snapshot_data'][1].type).to.equal(2) // full_snapshot + expect(captures[2]['properties']['$snapshot_data'][2].type).to.equal(5) // custom event with options + expect(captures[2]['properties']['$snapshot_data'][3].type).to.equal(5) // custom event with posthog config // Making a set from the rest should all be 3 - incremental snapshots - const incrementalSnapshots = captures[1]['properties']['$snapshot_data'].slice(4) + const incrementalSnapshots = captures[2]['properties']['$snapshot_data'].slice(4) expect(Array.from(new Set(incrementalSnapshots.map((s) => s.type)))).to.deep.eq([3]) }) }) @@ -180,6 +194,10 @@ describe('Session recording', () => { loaded: (ph) => { ph.sessionRecording._forceAllowLocalhostNetworkCapture = true }, + + session_recording: { + report_recording_started: true, + }, }, }) @@ -271,6 +289,11 @@ describe('Session recording', () => { describe('array.js', () => { beforeEach(() => { start({ + options: { + session_recording: { + report_recording_started: true, + }, + }, decideResponseOverrides: { isAuthenticated: false, sessionRecording: { @@ -287,7 +310,7 @@ describe('Session recording', () => { it('captures session events', () => { cy.phCaptures({ full: true }).then((captures) => { // should be a pageview at the beginning - expect(captures.map((c) => c.event)).to.deep.equal(['$pageview']) + expect(captures.map((c) => c.event)).to.deep.equal(['$pageview', '$session_recording_started']) }) cy.resetPhCaptures() @@ -386,10 +409,18 @@ describe('Session recording', () => { it('continues capturing to the same session when the page reloads', () => { let sessionId: string | null = null - // cypress time handling can confuse when to run full snapshot, let's force that to happen... cy.get('[data-cy-input]').type('hello world! ') cy.wait('@session-recording').then(() => { cy.phCaptures({ full: true }).then((captures) => { + expect(captures.map((c) => c.event)).to.deep.equal([ + '$pageview', + '$session_recording_started', + '$snapshot', + ]) + expect(captures[1]['properties']['$session_recording_started_reason']).to.equal( + 'recording_initialized' + ) + captures.forEach((c) => { if (isNull(sessionId)) { sessionId = c.properties['$session_id'] @@ -404,7 +435,11 @@ describe('Session recording', () => { cy.resetPhCaptures() // and refresh the page cy.reload() - cy.posthogInit({}) + cy.posthogInit({ + session_recording: { + report_recording_started: true, + }, + }) cy.wait('@decide') cy.wait('@recorder') @@ -418,10 +453,13 @@ describe('Session recording', () => { cy.phCaptures({ full: true }).then((captures) => { // should be a $snapshot for the current session expect(captures.map((c) => c.event)).to.deep.equal(['$pageview', '$snapshot']) + expect(captures[0].properties['$session_id']).to.equal(sessionId) - expect(captures[1].properties['$session_id']).to.equal(sessionId) - expect(captures[1]['properties']['$snapshot_data']).to.have.length.above(0) + const capturedSnapshot = captures[1] + expect(capturedSnapshot.properties['$session_id']).to.equal(sessionId) + + expect(capturedSnapshot['properties']['$snapshot_data']).to.have.length.above(0) /** * the snapshots will look a little like: @@ -433,28 +471,28 @@ describe('Session recording', () => { // page reloaded so we will start with a full snapshot // a meta and then a full snapshot - expect(captures[1]['properties']['$snapshot_data'][0].type).to.equal(4) // meta - expect(captures[1]['properties']['$snapshot_data'][1].type).to.equal(2) // full_snapshot + expect(capturedSnapshot['properties']['$snapshot_data'][0].type).to.equal(4) // meta + expect(capturedSnapshot['properties']['$snapshot_data'][1].type).to.equal(2) // full_snapshot // these custom events should always be in the same order, but computers // we don't care if they are present and in a changing order const customEvents = sortByTag([ - captures[1]['properties']['$snapshot_data'][2], - captures[1]['properties']['$snapshot_data'][3], - captures[1]['properties']['$snapshot_data'][4], + capturedSnapshot['properties']['$snapshot_data'][2], + capturedSnapshot['properties']['$snapshot_data'][3], + capturedSnapshot['properties']['$snapshot_data'][4], ]) expectPageViewCustomEvent(customEvents[0]) expectPostHogConfigCustomEvent(customEvents[1]) expectSessionOptionsCustomEvent(customEvents[2]) const xPositions = [] - for (let i = 5; i < captures[1]['properties']['$snapshot_data'].length; i++) { - expect(captures[1]['properties']['$snapshot_data'][i].type).to.equal(3) - expect(captures[1]['properties']['$snapshot_data'][i].data.source).to.equal( + for (let i = 5; i < capturedSnapshot['properties']['$snapshot_data'].length; i++) { + expect(capturedSnapshot['properties']['$snapshot_data'][i].type).to.equal(3) + expect(capturedSnapshot['properties']['$snapshot_data'][i].data.source).to.equal( 6, - JSON.stringify(captures[1]['properties']['$snapshot_data'][i]) + JSON.stringify(capturedSnapshot['properties']['$snapshot_data'][i]) ) - xPositions.push(captures[1]['properties']['$snapshot_data'][i].data.positions[0].x) + xPositions.push(capturedSnapshot['properties']['$snapshot_data'][i].data.positions[0].x) } // even though we trigger 4 events, only 2 snapshots should be captured @@ -480,8 +518,15 @@ describe('Session recording', () => { .then(() => { cy.phCaptures({ full: true }).then((captures) => { // should be a pageview and a $snapshot - expect(captures.map((c) => c.event)).to.deep.equal(['$pageview', '$snapshot']) - expect(captures[1]['properties']['$session_id']).to.be.a('string') + expect(captures.map((c) => c.event)).to.deep.equal([ + '$pageview', + '$session_recording_started', + '$snapshot', + ]) + expect(captures[1]['properties']['$session_recording_started_reason']).to.equal( + 'recording_initialized' + ) + expect(captures[2]['properties']['$session_id']).to.be.a('string') firstSessionId = captures[1]['properties']['$session_id'] }) }) @@ -506,15 +551,26 @@ describe('Session recording', () => { .wait('@session-recording', { timeout: 10000 }) .then(() => { cy.phCaptures({ full: true }).then((captures) => { - // should be a pageview and a $snapshot - expect(captures[0].event).to.equal('$snapshot') + const capturedSessionStartedReport = captures[0] + expect(capturedSessionStartedReport.event).to.equal('$session_recording_started') + expect(capturedSessionStartedReport.properties['$session_recording_started_reason']).to.equal( + 'session_id_changed' + ) + expect(capturedSessionStartedReport.properties['$session_id']).not.to.eq(firstSessionId) + + const capturedSnapshot = captures[1] + expect(capturedSnapshot.event).to.equal('$snapshot') + + expect(capturedSnapshot['properties']['$session_id']).to.be.a('string') + expect(capturedSnapshot['properties']['$session_id']).not.to.eq(firstSessionId) - expect(captures[0]['properties']['$session_id']).to.be.a('string') - expect(captures[0]['properties']['$session_id']).not.to.eq(firstSessionId) + expect(capturedSnapshot['properties']['$session_id']).to.eq( + capturedSessionStartedReport.properties['$session_id'] + ) - expect(captures[0]['properties']['$snapshot_data']).to.have.length.above(0) - expect(captures[0]['properties']['$snapshot_data'][0].type).to.equal(4) // meta - expect(captures[0]['properties']['$snapshot_data'][1].type).to.equal(2) // full_snapshot + expect(capturedSnapshot['properties']['$snapshot_data']).to.have.length.above(0) + expect(capturedSnapshot['properties']['$snapshot_data'][0].type).to.equal(4) // meta + expect(capturedSnapshot['properties']['$snapshot_data'][1].type).to.equal(2) // full_snapshot }) }) }) @@ -522,7 +578,7 @@ describe('Session recording', () => { it('starts a new recording after calling reset', () => { cy.phCaptures({ full: true }).then((captures) => { // should be a pageview at the beginning - expect(captures.map((c) => c.event)).to.deep.equal(['$pageview']) + expect(captures.map((c) => c.event)).to.deep.equal(['$pageview', '$session_recording_started']) }) cy.resetPhCaptures() @@ -553,6 +609,11 @@ describe('Session recording', () => { describe('with sampling', () => { beforeEach(() => { start({ + options: { + session_recording: { + report_recording_started: true, + }, + }, decideResponseOverrides: { isAuthenticated: false, sessionRecording: { @@ -575,7 +636,6 @@ describe('Session recording', () => { .wait(200) // can't wait on call to session recording, it's not going to happen .then(() => { cy.phCaptures({ full: true }).then((captures) => { - // should be a pageview and a $snapshot expect(captures.map((c) => c.event)).to.deep.equal(['$pageview']) }) }) @@ -611,6 +671,11 @@ describe('Session recording', () => { // no call to session-recording yet }) + cy.phCaptures({ full: true }).then((captures) => { + expect((captures || []).map((c) => c.event)).to.deep.equal(['$pageview', '$session_recording_started']) + expect(captures[1]['properties']['$session_recording_started_reason']).to.equal('sampling_override') + }) + cy.resetPhCaptures() cy.get('[data-cy-input]').type('hello posthog!') diff --git a/src/extensions/replay/sessionrecording.ts b/src/extensions/replay/sessionrecording.ts index c28cfaa8e..c36d32eab 100644 --- a/src/extensions/replay/sessionrecording.ts +++ b/src/extensions/replay/sessionrecording.ts @@ -90,6 +90,11 @@ interface SessionIdlePayload { bufferSize: number } +interface StartReportingOptions { + reason: string + properties?: Properties +} + const newQueuedEvent = (rrwebMethod: () => void): QueuedRRWebEvent => ({ rrwebMethod, enqueuedAt: Date.now(), @@ -395,9 +400,9 @@ export class SessionRecording { } } - startIfEnabledOrStop() { + startIfEnabledOrStop(reportingOptions?: StartReportingOptions) { if (this.isRecordingEnabled) { - this._startCapture() + this._startCapture(reportingOptions) // calling addEventListener multiple times is safe and will not add duplicates window?.addEventListener('beforeunload', this._onBeforeUnload) @@ -495,23 +500,30 @@ export class SessionRecording { if (makeDecision) { const randomNumber = Math.random() shouldSample = randomNumber < currentSampleRate - this._reportStarted('sampling', { - sampleRate: currentSampleRate, - randomNumber, - }) } else { shouldSample = storedIsSampled } - if (!shouldSample && makeDecision) { - logger.warn( - LOGGER_PREFIX + - ` Sample rate (${currentSampleRate}) has determined that this sessionId (${sessionId}) will not be sent to the server.` - ) + if (makeDecision) { + if (shouldSample) { + this._reportStarted({ + reason: 'sampling', + properties: { + sampleRate: currentSampleRate, + }, + }) + } else { + logger.warn( + LOGGER_PREFIX + + ` Sample rate (${currentSampleRate}) has determined that this sessionId (${sessionId}) will not be sent to the server.` + ) + } + + this._tryAddCustomEvent('samplingDecisionMade', { + sampleRate: currentSampleRate, + isSampled: shouldSample, + }) } - this._tryAddCustomEvent('samplingDecisionMade', { - sampleRate: currentSampleRate, - }) this.instance.persistence?.register({ [SESSION_RECORDING_IS_SAMPLED]: shouldSample, @@ -543,7 +555,7 @@ export class SessionRecording { const tag = 'linked flag matched' logger.info(LOGGER_PREFIX + ' ' + tag, payload) this._tryAddCustomEvent(tag, payload) - this._reportStarted('linked_flag_match', payload) + this._reportStarted({ reason: 'linked_flag_match', properties: payload }) } this._linkedFlagSeen = linkedFlagMatches }) @@ -617,7 +629,7 @@ export class SessionRecording { }) } - private _startCapture() { + private _startCapture(reportingOptions?: StartReportingOptions) { if (isUndefined(Object.assign)) { // According to the rrweb docs, rrweb is not supported on IE11 and below: // "rrweb does not support IE11 and below because it uses the MutationObserver API which was supported by these browsers." @@ -656,6 +668,18 @@ export class SessionRecording { } else { this._onScriptLoaded() } + + logger.info(LOGGER_PREFIX + ' starting') + if (this.status === 'active') { + this._reportStarted({ + reason: reportingOptions?.reason || 'recording_initialized', + properties: { + idleThreshold: this.sessionIdleThresholdMilliseconds, + maxIdleTime: this.sessionManager.sessionTimeoutMs, + ...(reportingOptions?.properties || {}), + }, + }) + } } private isInteractiveEvent(event: eventWithTime) { @@ -729,8 +753,7 @@ export class SessionRecording { if (sessionIdChanged || windowIdChanged) { this.stopRecording() - this.startIfEnabledOrStop() - this._reportStarted('session_id_change', { changeReason }) + this.startIfEnabledOrStop({ reason: 'session_id_changed', properties: { changeReason } }) } else if (returningFromIdle) { this._scheduleFullSnapshot() } @@ -847,12 +870,6 @@ export class SessionRecording { this._tryAddCustomEvent('$posthog_config', { config: this.instance.config, }) - - logger.info(LOGGER_PREFIX + ' started') - this._reportStarted('recording_initialized', { - idleThreshold: this.sessionIdleThresholdMilliseconds, - maxIdleTime: this.sessionManager.sessionTimeoutMs, - }) } private _scheduleFullSnapshot(): void { @@ -1106,14 +1123,29 @@ export class SessionRecording { * */ public overrideLinkedFlag() { this._linkedFlagSeen = true - this._reportStarted('linked_flag_override') + this._reportStarted({ reason: 'linked_flag_override' }) + } + + /** + * this ignores the sampling config and causes capture to start + * (if recording would have started had the flag been received i.e. it does not override other config). + * + * It is not usual to call this directly, + * instead call `posthog.startSessionRecording({sampling: true})` + * */ + public overrideSampling() { + this.instance.persistence?.register({ + // short-circuits the `makeSamplingDecision` function in the session recording module + [SESSION_RECORDING_IS_SAMPLED]: true, + }) + this._reportStarted({ reason: 'sampling_override' }) } - private _reportStarted(reason: string, properties?: Properties) { + private _reportStarted(report: StartReportingOptions) { if (this.instance.config.session_recording.report_recording_started) { this.instance.capture('$session_recording_started', { - ...(properties || {}), - $session_recording_started_reason: reason, + ...(report.properties || {}), + $session_recording_started_reason: report.reason, }) } } diff --git a/src/posthog-core.ts b/src/posthog-core.ts index 3dee01b08..32dfe5309 100644 --- a/src/posthog-core.ts +++ b/src/posthog-core.ts @@ -17,7 +17,6 @@ import { ALIAS_ID_KEY, FLAG_CALL_REPORTED, PEOPLE_DISTINCT_ID_KEY, - SESSION_RECORDING_IS_SAMPLED, USER_STATE, ENABLE_PERSON_PROCESSING, } from './constants' @@ -320,7 +319,7 @@ export class PostHog { }, } - this.on('eventCaptured', (data) => logger.info('send', data)) + this.on('eventCaptured', (data) => logger.info(`send "${data?.event}"`, data)) } // Initialization methods @@ -1788,18 +1787,17 @@ export class PostHog { */ startSessionRecording(override?: { sampling?: boolean; linked_flag?: boolean } | true): void { const overrideAll = isBoolean(override) && override - if (overrideAll || override?.sampling) { + if (overrideAll || override?.sampling || override?.linked_flag) { // allow the session id check to rotate session id if necessary const ids = this.sessionManager?.checkAndGetSessionAndWindowId() - this.persistence?.register({ - // short-circuits the `makeSamplingDecision` function in the session recording module - [SESSION_RECORDING_IS_SAMPLED]: true, - }) - logger.info('Session recording started with sampling override for session: ', ids?.sessionId) - } - if (overrideAll || override?.linked_flag) { - this.sessionRecording?.overrideLinkedFlag() - logger.info('Session recording started with linked_flags override') + if (overrideAll || override?.sampling) { + this.sessionRecording?.overrideSampling() + logger.info('Session recording started with sampling override for session: ', ids?.sessionId) + } + if (overrideAll || override?.linked_flag) { + this.sessionRecording?.overrideLinkedFlag() + logger.info('Session recording started with linked_flags override') + } } this.set_config({ disable_session_recording: false }) } From 0882fe90668305ba06b0fb4c4d5bfaacb724cba4 Mon Sep 17 00:00:00 2001 From: Paul D'Ambra Date: Sun, 6 Oct 2024 08:55:13 +0100 Subject: [PATCH 6/9] fiddling --- cypress/support/setup.ts | 2 +- src/extensions/replay/sessionrecording.ts | 13 +++++++------ 2 files changed, 8 insertions(+), 7 deletions(-) diff --git a/cypress/support/setup.ts b/cypress/support/setup.ts index 655b3e070..ae9326a78 100644 --- a/cypress/support/setup.ts +++ b/cypress/support/setup.ts @@ -24,7 +24,7 @@ export const start = ({ // sometimes we have too many listeners in this test environment // that breaks the event emitter listeners in error tracking tests // we don't see the error in production, so it's fine to increase the limit here - EventEmitter.prototype._maxListeners = 100 + EventEmitter.prototype.setMaxListeners(100) const decideResponse = { editorParams: {}, diff --git a/src/extensions/replay/sessionrecording.ts b/src/extensions/replay/sessionrecording.ts index c36d32eab..d033a4d73 100644 --- a/src/extensions/replay/sessionrecording.ts +++ b/src/extensions/replay/sessionrecording.ts @@ -670,16 +670,17 @@ export class SessionRecording { } logger.info(LOGGER_PREFIX + ' starting') - if (this.status === 'active') { - this._reportStarted({ + this._reportStarted( + { reason: reportingOptions?.reason || 'recording_initialized', properties: { idleThreshold: this.sessionIdleThresholdMilliseconds, maxIdleTime: this.sessionManager.sessionTimeoutMs, ...(reportingOptions?.properties || {}), }, - }) - } + }, + () => this.status === 'active' + ) } private isInteractiveEvent(event: eventWithTime) { @@ -1141,8 +1142,8 @@ export class SessionRecording { this._reportStarted({ reason: 'sampling_override' }) } - private _reportStarted(report: StartReportingOptions) { - if (this.instance.config.session_recording.report_recording_started) { + private _reportStarted(report: StartReportingOptions, shouldReport: () => boolean = () => true) { + if (this.instance.config.session_recording.report_recording_started && shouldReport()) { this.instance.capture('$session_recording_started', { ...(report.properties || {}), $session_recording_started_reason: report.reason, From be17b723a8c8fc406f21ccb98ad8f928a7fd7022 Mon Sep 17 00:00:00 2001 From: Paul D'Ambra Date: Tue, 15 Oct 2024 11:18:10 +0100 Subject: [PATCH 7/9] switch to session property --- cypress/e2e/session-recording.cy.ts | 73 ++++++++++------------- src/extensions/replay/sessionrecording.ts | 55 +++++++---------- 2 files changed, 53 insertions(+), 75 deletions(-) diff --git a/cypress/e2e/session-recording.cy.ts b/cypress/e2e/session-recording.cy.ts index 286652d27..b0f79bac2 100644 --- a/cypress/e2e/session-recording.cy.ts +++ b/cypress/e2e/session-recording.cy.ts @@ -135,26 +135,27 @@ describe('Session recording', () => { .type('hello posthog!') .wait('@session-recording') .then(() => { + cy.posthog().invoke('capture', 'test_registered_property') cy.phCaptures({ full: true }).then((captures) => { expect(captures.map((c) => c.event)).to.deep.equal([ '$pageview', - '$session_recording_started', '$snapshot', + 'test_registered_property', ]) - expect(captures[1]['properties']['$session_recording_started_reason']).to.equal( - 'recording_initialized' - ) - - expect(captures[2]['properties']['$snapshot_data']).to.have.length.above(33).and.below(40) + expect(captures[1]['properties']['$snapshot_data']).to.have.length.above(33).and.below(40) // a meta and then a full snapshot - expect(captures[2]['properties']['$snapshot_data'][0].type).to.equal(4) // meta - expect(captures[2]['properties']['$snapshot_data'][1].type).to.equal(2) // full_snapshot - expect(captures[2]['properties']['$snapshot_data'][2].type).to.equal(5) // custom event with options - expect(captures[2]['properties']['$snapshot_data'][3].type).to.equal(5) // custom event with posthog config + expect(captures[1]['properties']['$snapshot_data'][0].type).to.equal(4) // meta + expect(captures[1]['properties']['$snapshot_data'][1].type).to.equal(2) // full_snapshot + expect(captures[1]['properties']['$snapshot_data'][2].type).to.equal(5) // custom event with options + expect(captures[1]['properties']['$snapshot_data'][3].type).to.equal(5) // custom event with posthog config // Making a set from the rest should all be 3 - incremental snapshots - const incrementalSnapshots = captures[2]['properties']['$snapshot_data'].slice(4) + const incrementalSnapshots = captures[1]['properties']['$snapshot_data'].slice(4) expect(Array.from(new Set(incrementalSnapshots.map((s) => s.type)))).to.deep.eq([3]) + + expect(captures[2]['properties']['$session_recording_start_reason']).to.equal( + 'recording_initialized' + ) }) }) }) @@ -310,7 +311,7 @@ describe('Session recording', () => { it('captures session events', () => { cy.phCaptures({ full: true }).then((captures) => { // should be a pageview at the beginning - expect(captures.map((c) => c.event)).to.deep.equal(['$pageview', '$session_recording_started']) + expect(captures.map((c) => c.event)).to.deep.equal(['$pageview']) }) cy.resetPhCaptures() @@ -412,14 +413,7 @@ describe('Session recording', () => { cy.get('[data-cy-input]').type('hello world! ') cy.wait('@session-recording').then(() => { cy.phCaptures({ full: true }).then((captures) => { - expect(captures.map((c) => c.event)).to.deep.equal([ - '$pageview', - '$session_recording_started', - '$snapshot', - ]) - expect(captures[1]['properties']['$session_recording_started_reason']).to.equal( - 'recording_initialized' - ) + expect(captures.map((c) => c.event)).to.deep.equal(['$pageview', '$snapshot']) captures.forEach((c) => { if (isNull(sessionId)) { @@ -516,18 +510,20 @@ describe('Session recording', () => { .type('hello posthog!') .wait('@session-recording') .then(() => { + cy.posthog().invoke('capture', 'test_registered_property') cy.phCaptures({ full: true }).then((captures) => { - // should be a pageview and a $snapshot expect(captures.map((c) => c.event)).to.deep.equal([ '$pageview', - '$session_recording_started', '$snapshot', + 'test_registered_property', ]) - expect(captures[1]['properties']['$session_recording_started_reason']).to.equal( + + expect(captures[1]['properties']['$session_id']).to.be.a('string') + firstSessionId = captures[1]['properties']['$session_id'] + + expect(captures[2]['properties']['$session_recording_start_reason']).to.equal( 'recording_initialized' ) - expect(captures[2]['properties']['$session_id']).to.be.a('string') - firstSessionId = captures[1]['properties']['$session_id'] }) }) @@ -550,35 +546,29 @@ describe('Session recording', () => { .type('hello posthog!') .wait('@session-recording', { timeout: 10000 }) .then(() => { + cy.posthog().invoke('capture', 'test_registered_property') cy.phCaptures({ full: true }).then((captures) => { - const capturedSessionStartedReport = captures[0] - expect(capturedSessionStartedReport.event).to.equal('$session_recording_started') - expect(capturedSessionStartedReport.properties['$session_recording_started_reason']).to.equal( - 'session_id_changed' - ) - expect(capturedSessionStartedReport.properties['$session_id']).not.to.eq(firstSessionId) - - const capturedSnapshot = captures[1] + const capturedSnapshot = captures[0] expect(capturedSnapshot.event).to.equal('$snapshot') expect(capturedSnapshot['properties']['$session_id']).to.be.a('string') expect(capturedSnapshot['properties']['$session_id']).not.to.eq(firstSessionId) - expect(capturedSnapshot['properties']['$session_id']).to.eq( - capturedSessionStartedReport.properties['$session_id'] - ) - expect(capturedSnapshot['properties']['$snapshot_data']).to.have.length.above(0) expect(capturedSnapshot['properties']['$snapshot_data'][0].type).to.equal(4) // meta expect(capturedSnapshot['properties']['$snapshot_data'][1].type).to.equal(2) // full_snapshot + + expect(captures[1].event).to.equal('test_registered_property') + expect(captures[1]['properties']['$session_recording_start_reason']).to.equal( + 'session_id_changed' + ) }) }) }) it('starts a new recording after calling reset', () => { cy.phCaptures({ full: true }).then((captures) => { - // should be a pageview at the beginning - expect(captures.map((c) => c.event)).to.deep.equal(['$pageview', '$session_recording_started']) + expect(captures[0].event).to.eq('$pageview') }) cy.resetPhCaptures() @@ -671,9 +661,10 @@ describe('Session recording', () => { // no call to session-recording yet }) + cy.posthog().invoke('capture', 'test_registered_property') cy.phCaptures({ full: true }).then((captures) => { - expect((captures || []).map((c) => c.event)).to.deep.equal(['$pageview', '$session_recording_started']) - expect(captures[1]['properties']['$session_recording_started_reason']).to.equal('sampling_override') + expect((captures || []).map((c) => c.event)).to.deep.equal(['$pageview', 'test_registered_property']) + expect(captures[1]['properties']['$session_recording_start_reason']).to.equal('sampling_override') }) cy.resetPhCaptures() diff --git a/src/extensions/replay/sessionrecording.ts b/src/extensions/replay/sessionrecording.ts index d033a4d73..c0486b85a 100644 --- a/src/extensions/replay/sessionrecording.ts +++ b/src/extensions/replay/sessionrecording.ts @@ -34,6 +34,14 @@ import { isLocalhost } from '../../utils/request-utils' import { MutationRateLimiter } from './mutation-rate-limiter' import { gzipSync, strFromU8, strToU8 } from 'fflate' +type SessionStartReason = + | 'sampling_override' + | 'recording_initialized' + | 'linked_flag_match' + | 'linked_flag_override' + | 'sampling' + | 'session_id_changed' + const BASE_ENDPOINT = '/s/' const FIVE_MINUTES = 1000 * 60 * 5 @@ -90,11 +98,6 @@ interface SessionIdlePayload { bufferSize: number } -interface StartReportingOptions { - reason: string - properties?: Properties -} - const newQueuedEvent = (rrwebMethod: () => void): QueuedRRWebEvent => ({ rrwebMethod, enqueuedAt: Date.now(), @@ -400,9 +403,9 @@ export class SessionRecording { } } - startIfEnabledOrStop(reportingOptions?: StartReportingOptions) { + startIfEnabledOrStop(startReason?: SessionStartReason) { if (this.isRecordingEnabled) { - this._startCapture(reportingOptions) + this._startCapture(startReason) // calling addEventListener multiple times is safe and will not add duplicates window?.addEventListener('beforeunload', this._onBeforeUnload) @@ -506,12 +509,7 @@ export class SessionRecording { if (makeDecision) { if (shouldSample) { - this._reportStarted({ - reason: 'sampling', - properties: { - sampleRate: currentSampleRate, - }, - }) + this._reportStarted('sampling') } else { logger.warn( LOGGER_PREFIX + @@ -555,7 +553,7 @@ export class SessionRecording { const tag = 'linked flag matched' logger.info(LOGGER_PREFIX + ' ' + tag, payload) this._tryAddCustomEvent(tag, payload) - this._reportStarted({ reason: 'linked_flag_match', properties: payload }) + this._reportStarted('linked_flag_match') } this._linkedFlagSeen = linkedFlagMatches }) @@ -629,7 +627,7 @@ export class SessionRecording { }) } - private _startCapture(reportingOptions?: StartReportingOptions) { + private _startCapture(startReason?: SessionStartReason) { if (isUndefined(Object.assign)) { // According to the rrweb docs, rrweb is not supported on IE11 and below: // "rrweb does not support IE11 and below because it uses the MutationObserver API which was supported by these browsers." @@ -670,17 +668,7 @@ export class SessionRecording { } logger.info(LOGGER_PREFIX + ' starting') - this._reportStarted( - { - reason: reportingOptions?.reason || 'recording_initialized', - properties: { - idleThreshold: this.sessionIdleThresholdMilliseconds, - maxIdleTime: this.sessionManager.sessionTimeoutMs, - ...(reportingOptions?.properties || {}), - }, - }, - () => this.status === 'active' - ) + this._reportStarted(startReason || 'recording_initialized', () => this.status === 'active') } private isInteractiveEvent(event: eventWithTime) { @@ -741,7 +729,7 @@ export class SessionRecording { } // We only want to extend the session if it is an interactive event. - const { windowId, sessionId, changeReason } = this.sessionManager.checkAndGetSessionAndWindowId( + const { windowId, sessionId } = this.sessionManager.checkAndGetSessionAndWindowId( !isUserInteraction, event.timestamp ) @@ -754,7 +742,7 @@ export class SessionRecording { if (sessionIdChanged || windowIdChanged) { this.stopRecording() - this.startIfEnabledOrStop({ reason: 'session_id_changed', properties: { changeReason } }) + this.startIfEnabledOrStop('session_id_changed') } else if (returningFromIdle) { this._scheduleFullSnapshot() } @@ -1124,7 +1112,7 @@ export class SessionRecording { * */ public overrideLinkedFlag() { this._linkedFlagSeen = true - this._reportStarted({ reason: 'linked_flag_override' }) + this._reportStarted('linked_flag_override') } /** @@ -1139,14 +1127,13 @@ export class SessionRecording { // short-circuits the `makeSamplingDecision` function in the session recording module [SESSION_RECORDING_IS_SAMPLED]: true, }) - this._reportStarted({ reason: 'sampling_override' }) + this._reportStarted('sampling_override') } - private _reportStarted(report: StartReportingOptions, shouldReport: () => boolean = () => true) { + private _reportStarted(startReason: SessionStartReason, shouldReport: () => boolean = () => true) { if (this.instance.config.session_recording.report_recording_started && shouldReport()) { - this.instance.capture('$session_recording_started', { - ...(report.properties || {}), - $session_recording_started_reason: report.reason, + this.instance.register_for_session({ + $session_recording_start_reason: startReason, }) } } From 042a7aab921ae53aac5773504ab9742753fbfe96 Mon Sep 17 00:00:00 2001 From: Paul D'Ambra Date: Tue, 15 Oct 2024 12:48:16 +0100 Subject: [PATCH 8/9] no need to config for a property --- cypress/e2e/session-recording.cy.ts | 20 +++++--------------- src/extensions/replay/sessionrecording.ts | 6 ++++-- src/types.ts | 6 ------ 3 files changed, 9 insertions(+), 23 deletions(-) diff --git a/cypress/e2e/session-recording.cy.ts b/cypress/e2e/session-recording.cy.ts index b0f79bac2..a0b596e60 100644 --- a/cypress/e2e/session-recording.cy.ts +++ b/cypress/e2e/session-recording.cy.ts @@ -115,9 +115,7 @@ describe('Session recording', () => { it('captures session events', () => { start({ options: { - session_recording: { - report_recording_started: true, - }, + session_recording: {}, }, decideResponseOverrides: { isAuthenticated: false, @@ -196,9 +194,7 @@ describe('Session recording', () => { ph.sessionRecording._forceAllowLocalhostNetworkCapture = true }, - session_recording: { - report_recording_started: true, - }, + session_recording: {}, }, }) @@ -291,9 +287,7 @@ describe('Session recording', () => { beforeEach(() => { start({ options: { - session_recording: { - report_recording_started: true, - }, + session_recording: {}, }, decideResponseOverrides: { isAuthenticated: false, @@ -430,9 +424,7 @@ describe('Session recording', () => { // and refresh the page cy.reload() cy.posthogInit({ - session_recording: { - report_recording_started: true, - }, + session_recording: {}, }) cy.wait('@decide') cy.wait('@recorder') @@ -600,9 +592,7 @@ describe('Session recording', () => { beforeEach(() => { start({ options: { - session_recording: { - report_recording_started: true, - }, + session_recording: {}, }, decideResponseOverrides: { isAuthenticated: false, diff --git a/src/extensions/replay/sessionrecording.ts b/src/extensions/replay/sessionrecording.ts index c0486b85a..054a9759a 100644 --- a/src/extensions/replay/sessionrecording.ts +++ b/src/extensions/replay/sessionrecording.ts @@ -668,7 +668,9 @@ export class SessionRecording { } logger.info(LOGGER_PREFIX + ' starting') - this._reportStarted(startReason || 'recording_initialized', () => this.status === 'active') + if (this.status === 'active') { + this._reportStarted(startReason || 'recording_initialized') + } } private isInteractiveEvent(event: eventWithTime) { @@ -1131,7 +1133,7 @@ export class SessionRecording { } private _reportStarted(startReason: SessionStartReason, shouldReport: () => boolean = () => true) { - if (this.instance.config.session_recording.report_recording_started && shouldReport()) { + if (shouldReport()) { this.instance.register_for_session({ $session_recording_start_reason: startReason, }) diff --git a/src/types.ts b/src/types.ts index 994d5daae..9133fad89 100644 --- a/src/types.ts +++ b/src/types.ts @@ -289,12 +289,6 @@ export interface SessionRecordingOptions { Default is 5 minutes. */ session_idle_threshold_ms?: number - /* - Sends an event whenever a recording starts, with the reason the recording started. - Useful, for example, when using ingestion controls - to have a signal in batch exports to know which sessions have recordings - */ - report_recording_started?: boolean } export type SessionIdChangedCallback = ( From ba92753357be1762db20211be8a908fcb5bad0de Mon Sep 17 00:00:00 2001 From: Paul D'Ambra Date: Tue, 15 Oct 2024 12:57:11 +0100 Subject: [PATCH 9/9] fix --- .../extensions/replay/sessionrecording.test.ts | 15 ++++++++++++--- 1 file changed, 12 insertions(+), 3 deletions(-) diff --git a/src/__tests__/extensions/replay/sessionrecording.test.ts b/src/__tests__/extensions/replay/sessionrecording.test.ts index fc110b397..bbb709194 100644 --- a/src/__tests__/extensions/replay/sessionrecording.test.ts +++ b/src/__tests__/extensions/replay/sessionrecording.test.ts @@ -44,6 +44,7 @@ import { pluginEvent, } from '@rrweb/types' import Mock = jest.Mock +import { ConsentManager } from '../../../consent' // Type and source defined here designate a non-user-generated recording event @@ -243,14 +244,22 @@ describe('SessionRecording', () => { config: config, capture: jest.fn(), persistence: postHogPersistence, - onFeatureFlags: (cb: (flags: string[]) => void) => { + onFeatureFlags: ( + cb: (flags: string[], variants: Record) => void + ): (() => void) => { onFeatureFlagsCallback = cb + return () => {} }, sessionManager: sessionManager, requestRouter: new RequestRouter({ config } as any), _addCaptureHook: addCaptureHookMock, - consent: { isOptedOut: () => false }, - } as unknown as PostHog + consent: { + isOptedOut(): boolean { + return false + }, + } as unknown as ConsentManager, + register_for_session() {}, + } as Partial as PostHog loadScriptMock.mockImplementation((_ph, _path, callback) => { addRRwebToWindow()