-
Notifications
You must be signed in to change notification settings - Fork 135
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: start session recording on url trigger #1451
Merged
Merged
Changes from 7 commits
Commits
Show all changes
14 commits
Select commit
Hold shift + click to select a range
cd6e638
if there are trigger patterns, don't start until we match them
2e1e5ab
persist the trigger activation
8f45411
move handling to setter/getter
c5169e6
if we're waiting for a trigger, lower the full snapshot interval
d746362
send custom even when trigger is activated
5097b9b
only keep the latest full snapshot
1a1c12a
switch to using object triggers with matcher
c8633a1
refactor: distinguish trigger and recording status
9a4b88e
feat: handle session id changes
4217e66
bundle size golf
f99471a
remove unused import
2460cce
Merge branch 'main' into record-on-url-trigger
e0a2e82
fix: use trigger statuses correctly
c906b94
Merge branch 'main' into record-on-url-trigger
richard-better File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -6,6 +6,7 @@ import { | |
SESSION_RECORDING_MINIMUM_DURATION, | ||
SESSION_RECORDING_NETWORK_PAYLOAD_CAPTURE, | ||
SESSION_RECORDING_SAMPLE_RATE, | ||
SESSION_RECORDING_URL_TRIGGER_ACTIVATED, | ||
} from '../../constants' | ||
import { | ||
estimateSize, | ||
|
@@ -16,7 +17,14 @@ import { | |
truncateLargeConsoleLogs, | ||
} from './sessionrecording-utils' | ||
import { PostHog } from '../../posthog-core' | ||
import { DecideResponse, FlagVariant, NetworkRecordOptions, NetworkRequest, Properties } from '../../types' | ||
import { | ||
DecideResponse, | ||
FlagVariant, | ||
NetworkRecordOptions, | ||
NetworkRequest, | ||
Properties, | ||
SessionRecordingUrlTrigger, | ||
} from '../../types' | ||
import { | ||
customEvent, | ||
EventType, | ||
|
@@ -36,6 +44,7 @@ import { gzipSync, strFromU8, strToU8 } from 'fflate' | |
|
||
const BASE_ENDPOINT = '/s/' | ||
|
||
const ONE_MINUTE = 1000 * 60 | ||
const FIVE_MINUTES = 1000 * 60 * 5 | ||
const TWO_SECONDS = 2000 | ||
export const RECORDING_IDLE_THRESHOLD_MS = FIVE_MINUTES | ||
|
@@ -60,6 +69,9 @@ const ACTIVE_SOURCES = [ | |
IncrementalSource.Drag, | ||
] | ||
|
||
const TRIGGER_STATUSES = ['activated', 'pending', 'disabled'] as const | ||
type TriggerStatus = typeof TRIGGER_STATUSES[number] | ||
|
||
/** | ||
* Session recording starts in buffering mode while waiting for decide response | ||
* Once the response is received it might be disabled, active or sampled | ||
|
@@ -228,6 +240,8 @@ export class SessionRecording { | |
// then we can manually track href changes | ||
private _lastHref?: string | ||
|
||
private _urlTriggers: SessionRecordingUrlTrigger[] = [] | ||
|
||
// Util to help developers working on this feature manually override | ||
_forceAllowLocalhostNetworkCapture = false | ||
|
||
|
@@ -253,7 +267,11 @@ export class SessionRecording { | |
} | ||
|
||
private get fullSnapshotIntervalMillis(): number { | ||
return this.instance.config.session_recording?.full_snapshot_interval_millis || FIVE_MINUTES | ||
if (this.urlTriggerStatus === 'pending') { | ||
return ONE_MINUTE | ||
} | ||
|
||
return this.instance.config.session_recording?.full_snapshot_interval_millis ?? FIVE_MINUTES | ||
} | ||
|
||
private get isSampled(): boolean | null { | ||
|
@@ -343,13 +361,37 @@ export class SessionRecording { | |
return 'buffering' | ||
} | ||
|
||
if (this.urlTriggerStatus === 'pending') { | ||
return 'buffering' | ||
} | ||
|
||
if (isBoolean(this.isSampled)) { | ||
return this.isSampled ? 'sampled' : 'disabled' | ||
} else { | ||
return 'active' | ||
} | ||
} | ||
|
||
private get urlTriggerStatus(): 'activated' | 'pending' | 'disabled' { | ||
if (this.receivedDecide && this._urlTriggers.length === 0) { | ||
return 'disabled' | ||
richard-better marked this conversation as resolved.
Show resolved
Hide resolved
|
||
} | ||
|
||
const currentValue = this.instance?.get_property(SESSION_RECORDING_URL_TRIGGER_ACTIVATED) | ||
|
||
if (TRIGGER_STATUSES.includes(currentValue)) { | ||
return currentValue as TriggerStatus | ||
} | ||
|
||
return 'pending' | ||
} | ||
|
||
private set urlTriggerStatus(status: TriggerStatus) { | ||
this.instance?.persistence?.register({ | ||
richard-better marked this conversation as resolved.
Show resolved
Hide resolved
|
||
[SESSION_RECORDING_URL_TRIGGER_ACTIVATED]: status, | ||
richard-better marked this conversation as resolved.
Show resolved
Hide resolved
|
||
}) | ||
} | ||
|
||
constructor(private readonly instance: PostHog) { | ||
this._captureStarted = false | ||
this._endpoint = BASE_ENDPOINT | ||
|
@@ -544,6 +586,10 @@ export class SessionRecording { | |
}) | ||
} | ||
|
||
if (response.sessionRecording?.urlTriggers) { | ||
richard-better marked this conversation as resolved.
Show resolved
Hide resolved
|
||
this._urlTriggers = response.sessionRecording.urlTriggers | ||
} | ||
|
||
this.receivedDecide = true | ||
this.startIfEnabledOrStop() | ||
} | ||
|
@@ -909,11 +955,19 @@ export class SessionRecording { | |
this._pageViewFallBack() | ||
} | ||
|
||
// Check if the URL matches any trigger patterns | ||
this._checkUrlTrigger() | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. we could check this only when the URL changes (we have a hook in this class already) or return early in wandering towards early optimisation though - not blocking at all |
||
|
||
// we're processing a full snapshot, so we should reset the timer | ||
if (rawEvent.type === EventType.FullSnapshot) { | ||
this._scheduleFullSnapshot() | ||
} | ||
|
||
// Clear the buffer if waiting for a trigger, and only keep data from after the current full snapshot | ||
if (rawEvent.type === EventType.FullSnapshot && this.urlTriggerStatus === 'pending') { | ||
this.clearBuffer() | ||
pauldambra marked this conversation as resolved.
Show resolved
Hide resolved
|
||
} | ||
|
||
const throttledEvent = this.mutationRateLimiter | ||
? this.mutationRateLimiter.throttleMutations(rawEvent) | ||
: rawEvent | ||
|
@@ -1090,6 +1144,36 @@ export class SessionRecording { | |
}) | ||
} | ||
|
||
private _checkUrlTrigger() { | ||
if (typeof window === 'undefined' || !window.location.href) { | ||
return | ||
} | ||
|
||
const url = window.location.href | ||
|
||
if ( | ||
this._urlTriggers.some((trigger) => { | ||
switch (trigger.matching) { | ||
case 'regex': | ||
richard-better marked this conversation as resolved.
Show resolved
Hide resolved
|
||
return new RegExp(trigger.url).test(url) | ||
default: | ||
return false | ||
} | ||
}) | ||
) { | ||
this._activateUrlTrigger() | ||
} | ||
} | ||
|
||
private _activateUrlTrigger() { | ||
if (this.urlTriggerStatus === 'pending') { | ||
this.urlTriggerStatus = 'activated' | ||
this._tryAddCustomEvent('url trigger activated', {}) | ||
this._flushBuffer() | ||
logger.info(LOGGER_PREFIX + ' recording triggered by URL pattern match') | ||
} | ||
} | ||
|
||
/** | ||
* 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). | ||
|
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
very picky... and will have almost no impact individually but a little code golf helps the bundle size over time so you could have
i tend to dislike that style of writing and would rather fix the bigger problems in the bundle sizing but little things like that tend to help (although the bundler can behave surprisingly 🤣)