Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: add recording url blocklist #1500

Merged
merged 18 commits into from
Nov 12, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
72 changes: 72 additions & 0 deletions src/__tests__/extensions/replay/sessionrecording.test.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
/// <reference lib="dom" />

import '@testing-library/jest-dom'

import { PostHogPersistence } from '../../../posthog-persistence'
import {
CONSOLE_LOG_RECORDING_ENABLED_SERVER_SIDE,
Expand Down Expand Up @@ -45,6 +47,7 @@ import {
} from '@rrweb/types'
import Mock = jest.Mock
import { ConsentManager } from '../../../consent'
import { waitFor } from '@testing-library/preact'

// Type and source defined here designate a non-user-generated recording event

Expand Down Expand Up @@ -2156,4 +2159,73 @@ describe('SessionRecording', () => {
)
})
})

describe('URL blocking', () => {
beforeEach(() => {
sessionRecording.startIfEnabledOrStop()
sessionRecording.afterDecideResponse(
makeDecideResponse({
sessionRecording: {
endpoint: '/s/',
urlBlocklist: [
{
matching: 'regex',
url: '/blocked',
},
],
},
})
)
})

it('flushes buffer and includes pause event when hitting blocked URL', async () => {
// Emit some events before hitting blocked URL
_emit(createIncrementalSnapshot({ data: { source: 1 } }))
_emit(createIncrementalSnapshot({ data: { source: 2 } }))

// Simulate URL change to blocked URL
fakeNavigateTo('https://test.com/blocked')
_emit(createIncrementalSnapshot({ data: { source: 3 } }))
expect(document.body).toHaveClass('ph-no-capture')

await waitFor(() => {
// Verify the buffer was flushed with all events including pause
expect(posthog.capture).toHaveBeenCalledWith(
'$snapshot',
{
$session_id: sessionId,
$window_id: 'windowId',
$snapshot_bytes: expect.any(Number),
$snapshot_data: [
{ type: 3, data: { source: 1 } },
{ type: 3, data: { source: 2 } },
],
},
expect.any(Object)
)
})

// Verify subsequent events are not captured while on blocked URL
_emit(createIncrementalSnapshot({ data: { source: 4 } }))
expect(sessionRecording['buffer'].data).toHaveLength(0)

// Simulate URL change to allowed URL
fakeNavigateTo('https://test.com/allowed')

// Verify recording resumes with resume event
_emit(createIncrementalSnapshot({ data: { source: 5 } }))

expect(document.body).not.toHaveClass('ph-no-capture')

expect(sessionRecording['buffer'].data).toStrictEqual([
expect.objectContaining({
type: 2,
}),
expect.objectContaining({
type: 3,
data: { source: 5 },
}),
])
})
})
})
103 changes: 86 additions & 17 deletions src/extensions/replay/sessionrecording.ts
richard-better marked this conversation as resolved.
Show resolved Hide resolved
Original file line number Diff line number Diff line change
Expand Up @@ -87,7 +87,7 @@ type TriggerStatus = typeof TRIGGER_STATUSES[number]
* When sampled that means a sample rate is set and the last time the session id was rotated
* the sample rate determined this session should be sent to the server.
*/
type SessionRecordingStatus = 'disabled' | 'sampled' | 'active' | 'buffering'
type SessionRecordingStatus = 'disabled' | 'sampled' | 'active' | 'buffering' | 'paused'

export interface SnapshotBuffer {
size: number
Expand Down Expand Up @@ -214,6 +214,24 @@ function isSessionIdleEvent(e: eventWithTime): e is eventWithTime & customEvent
return e.type === EventType.Custom && e.data.tag === 'sessionIdle'
}

function sessionRecordingUrlTriggerMatches(url: string, triggers: SessionRecordingUrlTrigger[]) {
return triggers.some((trigger) => {
switch (trigger.matching) {
case 'regex':
return new RegExp(trigger.url).test(url)
default:
return false
}
})
}

/** When we put the recording into a paused state, we add a custom event.
* However in the paused state, events are dropped, and never make it to the buffer,
* so we need to manually let this one through */
function isRecordingPausedEvent(e: eventWithTime) {
return e.type === EventType.Custom && e.data.tag === 'recording paused'
}

export class SessionRecording {
private _endpoint: string
private flushBufferTimer?: any
Expand Down Expand Up @@ -247,6 +265,9 @@ export class SessionRecording {
private _lastHref?: string

private _urlTriggers: SessionRecordingUrlTrigger[] = []
private _urlBlocklist: SessionRecordingUrlTrigger[] = []

private _urlBlocked: boolean = false

// Util to help developers working on this feature manually override
_forceAllowLocalhostNetworkCapture = false
Expand Down Expand Up @@ -371,6 +392,10 @@ export class SessionRecording {
return 'buffering'
}

if (this._urlBlocked) {
return 'paused'
}

if (isBoolean(this.isSampled)) {
return this.isSampled ? 'sampled' : 'disabled'
} else {
Expand All @@ -379,7 +404,7 @@ export class SessionRecording {
}

private get urlTriggerStatus(): TriggerStatus {
if (this.receivedDecide && this._urlTriggers.length === 0) {
if (this._urlTriggers.length === 0) {
return 'trigger_disabled'
}

Expand Down Expand Up @@ -614,6 +639,10 @@ export class SessionRecording {
this._urlTriggers = response.sessionRecording.urlTriggers
}

if (response.sessionRecording?.urlBlocklist) {
this._urlBlocklist = response.sessionRecording.urlBlocklist
}

this.receivedDecide = true
this.startIfEnabledOrStop()
}
Expand Down Expand Up @@ -980,7 +1009,11 @@ export class SessionRecording {
}

// Check if the URL matches any trigger patterns
this._checkUrlTrigger()
this._checkTriggerConditions()

if (this.status === 'paused' && !isRecordingPausedEvent(rawEvent)) {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

So I've added this to make sure this one event can go through even if the recording is paused, but... it seems like onRRwebEmit never gets called with this event?

Copy link
Member

Choose a reason for hiding this comment

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

i ran it locally and these get called for me (in tests iirc there's no real rrweb so custom events don't work - could it be that)

Screenshot 2024-10-31 at 11 43 20

i got lots of paused events... don't know if it's because i had breakpoints set or if i'm getting a paused on every mutation?

return
}

// we're processing a full snapshot, so we should reset the timer
if (rawEvent.type === EventType.FullSnapshot) {
Expand Down Expand Up @@ -1033,11 +1066,12 @@ export class SessionRecording {
$window_id: this.windowId,
}

if (this.status !== 'disabled') {
this._captureSnapshotBuffered(properties)
} else {
if (this.status === 'disabled') {
this.clearBuffer()
return
}

this._captureSnapshotBuffered(properties)
}

private _pageViewFallBack() {
Expand Down Expand Up @@ -1168,23 +1202,23 @@ export class SessionRecording {
})
}

private _checkUrlTrigger() {
private _checkTriggerConditions() {
if (typeof window === 'undefined' || !window.location.href) {
return
}

const url = window.location.href

if (
this._urlTriggers.some((trigger) => {
switch (trigger.matching) {
case 'regex':
return new RegExp(trigger.url).test(url)
default:
return false
}
})
) {
const wasBlocked = this.status === 'paused'
const isNowBlocked = sessionRecordingUrlTriggerMatches(url, this._urlBlocklist)

if (isNowBlocked && !wasBlocked) {
this._pauseRecording()
} else if (!isNowBlocked && wasBlocked) {
this._resumeRecording()
}

if (sessionRecordingUrlTriggerMatches(url, this._urlTriggers)) {
this._activateUrlTrigger()
}
}
Expand All @@ -1198,6 +1232,41 @@ export class SessionRecording {
}
}

private _pauseRecording() {
if (this.status === 'paused') {
return
}
logger.info(LOGGER_PREFIX + ' recording paused due to URL blocker')

this._tryAddCustomEvent('recording paused', { reason: 'url blocker' })

this._urlBlocked = true
document?.body?.classList?.add('ph-no-capture')

// Clear the snapshot timer since we don't want new snapshots while paused
clearInterval(this._fullSnapshotTimer)

// Running this in a timeout to ensure we can
setTimeout(() => {
this._flushBuffer()
}, 100)
}

private _resumeRecording() {
if (this.status !== 'paused') {
return
}

this._urlBlocked = false
document?.body?.classList?.remove('ph-no-capture')

this._tryTakeFullSnapshot()

this._scheduleFullSnapshot()
this._tryAddCustomEvent('recording resumed', { reason: 'left blocked url' })
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Same deal as the paused event, this one never makes it to onRRwebEmit 🤔
@pauldambra, do you have any idea what I'm doing wrong here?

logger.info(LOGGER_PREFIX + ' recording resumed')
}

/**
* this ignores the linked flag config and causes capture to start
* (if recording would have started had the flag been received i.e. it does not override other config).
Expand Down
2 changes: 2 additions & 0 deletions src/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -397,6 +397,7 @@ export interface DecideResponse {
linkedFlag?: string | FlagVariant | null
networkPayloadCapture?: Pick<NetworkRecordOptions, 'recordBody' | 'recordHeaders'>
urlTriggers?: SessionRecordingUrlTrigger[]
urlBlocklist?: SessionRecordingUrlTrigger[]
}
surveys?: boolean
toolbarParams: ToolbarParams
Expand Down Expand Up @@ -614,6 +615,7 @@ export interface ErrorConversions {
}

export interface SessionRecordingUrlTrigger {
urlBlockList?: SessionRecordingUrlTrigger[]
url: string
matching: 'regex'
}
Loading