SR-3424 feat(session-replay-browser): add remote evaluation gate for capture eligibility#1610
Draft
lewgordon-amplitude wants to merge 11 commits intomainfrom
Draft
SR-3424 feat(session-replay-browser): add remote evaluation gate for capture eligibility#1610lewgordon-amplitude wants to merge 11 commits intomainfrom
lewgordon-amplitude wants to merge 11 commits intomainfrom
Conversation
Session Replay Browser E2E Results
Details
|
Collaborator
Author
|
bugbot run |
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
Collaborator
Author
|
bugbot run |
Collaborator
Author
|
bugbot run |
Collaborator
Author
|
bugbot run |
Collaborator
Author
|
bugbot run |
Collaborator
Author
|
bugbot run |
…eligibility
Integrates Amplitude Experiment as a remote gate that runs once per session
(and on session change) to decide whether a session is eligible for capture,
before local TRC evaluation runs. Enables server-side targeting logic (cohorts,
feature flags) as a precondition for Session Replay capture.
- New `remoteTargeting` config option with `deploymentKey`, `flagKey`,
`timeoutMs`, and `decisionStrategy` ('conservative' | 'lookback')
- Conservative strategy: hold recording until remote decision arrives;
no capture on timeout or error
- Lookback strategy: record immediately, hold uploads until decision,
then flush or discard based on result
- Re-evaluates on `setSessionId` for new session targeting
- Direct integration with Amplitude Experiment eval API
(api.lab.amplitude.com/sdk/v2/vardata) — no proxy required
- Unit tests (100% coverage), mocked e2e tests, and live e2e tests
against real Amplitude Experiment API (project 631773)
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
… in e2e tests Prevents race condition where the remote eval request fires and completes before the Playwright listener is set up, causing CI timeouts. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Adds `lookbackBufferMaxEvents` to `RemoteTargetingConfig` (default 1000) to bound memory use when buffering rrweb events in lookback mode while waiting for the remote eval decision. Events beyond the cap are silently dropped (oldest events are kept). Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…, and lookback flush issues - Add latestRemoteEvalId counter to discard stale remote eval results when setSessionId is called while a previous eval is still in-flight (medium bugbot issue) - Remove dead user_properties collection in runRemoteEval — fetchRemoteDecision only accepts device_id/user_id as query params (low bugbot issue) - Use addCompressedEvent directly (bypassing idle-callback deferral) when flushing the lookback buffer, so events are in the manager before sendEvents() fires (low bugbot issue) - Add staleness guard unit test Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
… gate is used When remoteTargeting is enabled, the init path calls runRemoteEval instead of evaluateTargetingAndCapture, bypassing the isInit=true code path that triggers eventsManager.sendStoredEvents. Replay events persisted in IDB from prior sessions were silently lost. Fix: call initialize(true) (conservative, no TRC) or pass isInit=true to evaluateTargetingAndCapture (conservative + TRC) so sendStoredEvents is always called. For the lookback path, explicitly call sendStoredEvents after flushing the lookback buffer. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…eval response
HTTP error responses (401, 500, etc.) that return parseable JSON were
silently treated as capture=false with no reason, making bad API keys
and server errors indistinguishable from a legitimate off evaluation.
Now returns { capture: false, reason: 'http-<status>' } for any non-2xx
response, matching the existing timeout/error reason pattern.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…on change; unblock lookback+TRC Conservative session change: asyncSetSessionId returned early without calling stopRecordingEvents(), leaving the previous session's rrweb recording running. Leaked CPU/memory and orphaned events captured under the old session ID after the new session's identifiers were updated. Lookback + TRC: recordEvents() calls getShouldRecord() which checked sessionTargetingMatch — always false before TRC evaluation runs. Recording never started, defeating the entire lookback strategy when TRC was also configured. Bypass the TRC match check in getShouldRecord when lookbackHoldUploads=true; TRC is evaluated correctly after the remote decision arrives in runRemoteEval. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…hold When targetingConfig is set and lookbackHoldUploads=true, the previous fix caused getShouldRecord to fall through to the sample-rate else branch. With the default sampleRate=0 (typical when TRC is configured), recording never started, defeating lookback entirely. Fix: add an early return true when lookbackHoldUploads=true, bypassing both TRC and sample-rate checks. Eligibility is determined in runRemoteEval after the remote decision and TRC evaluation complete. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…ndpoint fetchRemoteDecision was hardcoded to the US eval URL (api.lab.amplitude.com). EU customers would have their eval requests routed to the wrong region, failing with auth errors or returning incorrect results. Add serverZone parameter to fetchRemoteDecision and map EU to api.lab.eu.amplitude.com. Unknown zones (e.g. STAGING) fall back to US. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…rage Covers: serverZone forwarding, lookback+TRC both-pass flush, conservative+TRC both-pass recording, asyncSetSessionId stops prior lookback recording, and session change clears old lookback buffer before buffering new-session events. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
lewgordon-amplitude
force-pushed
the
remote-eval-trc
branch
from
26c2f4e to
a30166f
Compare
…onfig remote config Parses sr_experiment_config from the remote config fetch response and merges its deploymentKey into remoteTargeting config. SDK init options take precedence over remote-config-delivered deploymentKey (remote value only applied when remoteTargeting.deploymentKey is absent from init options). Adds ExperimentConfig type and sr_experiment_config field to SessionReplayRemoteConfig. 695 tests, 100% coverage. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
lewgordon-amplitude
changed the title
This file contains hidden or 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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
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.
Summary
Integrates Amplitude Experiment as a remote gate that runs once per session (and on session change) to decide whether a session is eligible for capture, before local TRC evaluation runs. Enables server-side targeting logic (cohorts, feature flags) as a precondition for Session Replay capture.
remoteTargetingconfig option withdeploymentKey,flagKey,timeoutMs, anddecisionStrategy('conservative' | 'lookback')setSessionIdfor new session targetingTech Design Doc: https://amplitude.atlassian.net/wiki/spaces/IG/pages/3710976009
Checklist
Note
Medium Risk
Changes core capture-start/stop behavior by adding an async remote Experiment evaluation step (including buffering/flush logic) that can prevent recording on timeout/error or session change. Risk is mitigated by extensive unit and e2e coverage, but mistakes could reduce capture rates or increase event buffering overhead.
Overview
Adds an optional remote targeting gate (
remoteTargeting) that calls the Amplitude Experiment eval API once per session (and onsetSessionId) to decide whether Session Replay is eligible to capture.Implements two strategies in
SessionReplay: conservative waits for the remote decision before recording, while lookback starts rrweb immediately, buffers events (bounded bylookbackBufferMaxEvents), then flushes or discards once the decision (and any local targeting) resolves.Plumbs the new config through local/joined config and the plugin wrapper, exports the new
RemoteTargetingConfig/RemoteDecisiontypes, and addsfetchRemoteDecisionplus comprehensive unit tests and Playwright e2e tests (mocked and live) for request shape and capture/flush behavior.Written by Cursor Bugbot for commit 838a2b2. This will update automatically on new commits. Configure here.