Revert "fix(replay): generate session_id client-side to avoid NAT collisions" - #317
Merged
Merged
Conversation
…lisions …" This reverts commit daf45af.
3 tasks
ayushjhanwar-png
added a commit
that referenced
this pull request
Jun 15, 2026
…erly (#318) Without this, the server falls back to `hash(salt + projectId + ip + ua)` to derive deviceId/sessionId. Every user behind the same NAT (office WiFi, home WiFi, mobile carrier NAT) hashes to the same id — their sessions and replay timelines get merged together. Previous attempt (PR #315, reverted in #317) had the SDK generate its own session_id and the server trust it. That bypassed the `sessions` table entirely — chunks landed in `session_replay_chunks` with an id that had no matching row in `sessions`, so the dashboard's "session details" page returned 404. This PR avoids that by keeping session_id server-issued and only changing the deviceId source. Three small changes: 1. packages/sdks/web/src/index.ts - On init, read or create `_op_device_id` in localStorage and stash it on the base SDK as `this.deviceId`. - Call `setGlobalProperties({ __deviceId })` so every event carries the override. The server's track flow already honors this (line 146-155 — pre-existing code path). - Consumers can override with a stable identifier (e.g. Firebase UID) by calling `setGlobalProperties({ __deviceId: user.uid })` after auth resolves. That's strictly better than the auto-generated UUID because it stitches the same human across browsers / devices. - Falls back to legacy IP+UA path if storage is unavailable (sandboxed iframe, quota exhausted, private mode quirks). 2. packages/sdks/sdk/src/index.ts - `fetchDeviceId()` now passes `this.deviceId` as `?deviceId=` query param so the server-side device-id lookup uses our override instead of recomputing from IP+UA. Returns the matching session id so replay chunks reference the correct row in `sessions`. 3. apps/api/src/controllers/track.controller.ts - `fetchDeviceId` endpoint accepts the optional `?deviceId=` query param. When supplied, skips the IP+UA hash and looks up the session directly by the client-supplied id. When omitted (old SDKs), keeps the existing IP+UA derivation — fully backward compatible. - Track ingest flow itself is UNCHANGED; the `__deviceId` override mechanism it uses (line 146-155) has been there all along. What this fixes: - Office colleagues no longer share session/replay timelines (each browser has its own localStorage UUID, or its own Firebase UID). - Sessions table and session_replay_chunks now share the same session_id because both are server-issued (looked up via the same deviceId). Dashboard "session details" page will find the row this time. - Track-event attribution is also corrected (was NAT-merged before; now per-browser / per-user). What it doesn't change: - Track ingestion HTTP path, controller logic, buffer flow — all unchanged. - Old SDKs without `__deviceId` still hit the IP+UA fallback — no regression for clients that haven't upgraded. - Historical events stay keyed by their old IP+UA hash. New events post-upgrade get the corrected key (expected migration behavior). Bumps @openpanel/web 1.2.0-local -> 1.3.0-local for downstream publish.
2 tasks
ayushjhanwar-png
added a commit
that referenced
this pull request
Jun 15, 2026
Companion to #318. The replay handler still derived a session by recomputing deviceId from IP+UA, which overrode the SDK-provided session_id (and re-introduced NAT-collision behavior on the replay storage path even when the SDK was using the deviceId override correctly on the track path). With #318 shipped, the SDK's session_id is no longer self-generated: it comes from /track/device-id?deviceId=<our localStorage UUID or stable user id>, which is the same id the track flow uses to create sessions. So the SDK-supplied session_id is now a real row in the `sessions` table and trusting it is safe — which fixes the "Session not found" dashboard error from the earlier attempt (#315 / #317). Behavior: - payload.session_id present (modern SDKs): trust it. Chunks land under the same session row that the track flow created. Dashboard "session details" works. - payload.session_id missing (legacy SDKs): fall back to the IP+UA derivation that's been there all along. No regression.
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.
Reverts #315