Repository navigation
feat(web-record): browser session recording over OTLP - #6
Conversation
Migrate softprobe-web-record-sdk into the monorepo as @softprobe/web-record, flushing rrweb batches to thelake via OTLP spans and shared tracing/config helpers. Co-authored-by: Cursor <cursoragent@cursor.com>
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 8e2b4351aa
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| }, | ||
| "dependencies": { | ||
| "@rrweb/types": "2.0.0-alpha.4", | ||
| "@softprobe/tracing": "file:../tracing", |
There was a problem hiding this comment.
Publishing this package leaves @softprobe/tracing as a local file:../tracing dependency, but that sibling directory is not included in the @softprobe/web-record tarball. Registry consumers therefore cannot resolve the import from @softprobe/tracing/config; use the workspace protocol or a published version so the packed manifest contains an installable registry dependency.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Addressed: is now workspace (npm workspaces, matches sibling packages) so the published tarball resolves the registry dependency.
There was a problem hiding this comment.
Addressed: @softprobe/tracing is now workspace "*" (npm workspaces, matches sibling packages) so the published tarball resolves the registry dependency.
| `OTLP export failed: ${res.status} ${await res.text()}`, | ||
| ); | ||
| } | ||
| this.events = []; |
There was a problem hiding this comment.
When rrweb emits events while the fetch is awaiting its response, those events are appended to the same array captured as batch, but they were not included in the body because JSON.stringify already ran. Replacing the entire queue after the response consequently discards every event recorded during network latency; detach the batch before exporting or remove only the events actually serialized.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Addressed: flush splices the batch out of the queue before , so events recorded while export is in flight stay queued for the next flush (and are restored on failure).
There was a problem hiding this comment.
Addressed: flush splices the batch out of the queue before await fetch, so events recorded while export is in flight stay queued for the next flush (and are restored on failure).
| setSessionId(sessionId: string): void { | ||
| const next = asNonEmptyString(sessionId); | ||
| if (!next) return; | ||
| this._sessionId = next; | ||
| if (this.enabled) { |
There was a problem hiding this comment.
If the documented setSessionId call occurs before the current five-second buffer has flushed, the already-recorded events remain queued while _sessionId changes. Since flush() reads _sessionId only when constructing the export, all of those events are then attributed to the new chat session, corrupting session replay correlation; retain the session ID with each batch or rotate the queue synchronously before switching.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Addressed: setSessionId now drains/flushes under the old id (drainFlush) before rebinding _sessionId / the HTTP interceptor.
| next.headers = { | ||
| ...(headers as Record<string, string>), | ||
| [HEADER_X_SP_SESSION_ID]: sid, | ||
| }; |
There was a problem hiding this comment.
Once recording is enabled, this wrapper adds the session identifier to every fetch request, including requests to analytics, CDNs, or other third-party origins; the XHR patch behaves likewise. On cross-origin calls the non-safelisted header can introduce a CORS preflight and break requests whose server does not allow it, while permissive servers receive a private correlation ID, so injection needs an origin or endpoint check.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Addressed: session header is only injected for same-origin requests (isSameOriginRequest); third-party/CDN URLs are skipped.
| const next: RequestInit = { ...(options ?? {}) }; | ||
| const headers = next.headers ?? {}; |
There was a problem hiding this comment.
For fetch(new Request(url, { headers: ... })) without options.headers, next.headers starts as an empty object and is then set to only the session header. Passing that init alongside the existing Request overrides rather than merges its headers, silently dropping values such as Authorization and Content-Type; initialize from the Request headers as well as from options.headers.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Addressed: fetch(Request) merges existing Request headers (+ init headers) before setting x-sp-session-id.
Adopt npm workspaces from main, keep @softprobe/web-record, and address PR review: flush race, setSessionId drain, same-origin session header.
Return flush success, abort drain on failure, chain session switches, and only advance batch_index after a successful OTLP export. Co-authored-by: Cursor <cursoragent@cursor.com>
Summary
@softprobe/web-record(migrated from softprobe-web-record-sdk) that captures rrweb events and exports batches to thelake via OTLP/v1/traces.@softprobe/tracing/configfor Softprobe credentials and{baseUrl}/v1/tracesderivation; soft-disable when credentials are missing.softprobe.web.recording,sp.observation.type=recording, eventsp.recording.batch/sp.recording.events.Test plan
pnpm --filter @softprobe/web-record testpnpm --filter @softprobe/web-record buildsp.session.idMade with Cursor