feat(replay): Add traces_by_timestamp to replay event#18048
Open
feat(replay): Add traces_by_timestamp to replay event#18048
traces_by_timestamp to replay event#18048Conversation
Contributor
size-limit report 📦
|
Contributor
node-overhead report 🧳Note: This is a synthetic benchmark with a minimal express app and does not necessarily reflect the real-world performance impact in an application.
|
Member
Author
|
@sentry review |
![sentry[bot]](https://avatars.githubusercontent.com/in/12637?s=60&v=4){kind=small}
packages/replay-internal/src/coreHandlers/handleAfterSendEvent.ts
Outdated
Show resolved
Hide resolved
packages/replay-internal/test/integration/coreHandlers/handleAfterSendEvent.test.ts
Show resolved
Hide resolved
Member
Author
|
@sentry review |
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
chargome
reviewed
Nov 10, 2025
| if (this._context.traceIds.length === 0) { | ||
| const currentTraceId = getCurrentScope().getPropagationContext().traceId; | ||
| if (currentTraceId) { | ||
| this._context.traceIds.push([-1, currentTraceId]); |
Member
There was a problem hiding this comment.
Could we put a comment in here explaining the -1 ?
billyvg
force-pushed
the
billy/replay-802-replay-sdk-needs-to-send-timestamps-w-trace-ids
branch
from
1985467 to
84b12da
Compare
Contributor
Codecov Results 📊Generated by Codecov Action |
In order to support moving our custom rrweb events to EAP, we need to send timestamps w/ trace ids so that we can identify which trace the event belongs to. In order to avoid breaking changes, we should not change the current type of trace_ids field in the replay event, instead we add a new field traces_by_timestamp
andreiborza
force-pushed
the
billy/replay-802-replay-sdk-needs-to-send-timestamps-w-trace-ids
branch
from
84b12da to
f50a1cb
Compare
andreiborza
approved these changes
Feb 18, 2026
andreiborza
added a commit
that referenced
this pull request
Feb 24, 2026
This is a temporary branch to cut a beta release for #18048.
…-timestamps-w-trace-ids
…-timestamps-w-trace-ids
| initialUrl: this._context.initialUrl, | ||
| errorIds: Array.from(this._context.errorIds), | ||
| traceIds: Array.from(this._context.traceIds), | ||
| traceIds: this._context.traceIds, |
There was a problem hiding this comment.
Bug: A race condition exists because _popEventContext returns a direct reference to the traceIds array, allowing it to be mutated during the async sendReplay operation.
Severity: MEDIUM
Suggested Fix
In _popEventContext, return a shallow copy of the traceIds array instead of a direct reference. This can be achieved by using the spread syntax [...this._context.traceIds] or the slice() method.
Prompt for AI Agent
Review the code at the location below. A potential bug has been identified by an AI
agent.
Verify if this is a real issue. If it is, propose a fix; if not, explain why it's not
valid.
Location: packages/replay-internal/src/replay.ts#L1159
Potential issue: The `_popEventContext` method returns a context object containing a
direct reference to the internal `_context.traceIds` array, not a copy. When a segment
flush is triggered with one or zero trace IDs, the subsequent call to `_clearContext`
does not reassign this array. This creates a race condition: during the `await` for the
asynchronous `sendReplay` operation, a new transaction event can trigger
`handleAfterSendEvent`, which pushes a new trace ID onto the shared array. As a result,
the replay segment being sent is mutated to include a trace ID that was captured after
the segment was finalized, leading to incorrect trace linkage.
Did we get this right? 👍 / 👎 to inform future reviews.
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
Bugbot Autofix is OFF. To automatically fix reported issues with Cloud Agents, enable autofix in the Cursor dashboard.
| initialUrl: this._context.initialUrl, | ||
| errorIds: Array.from(this._context.errorIds), | ||
| traceIds: Array.from(this._context.traceIds), | ||
| traceIds: this._context.traceIds, |
There was a problem hiding this comment.
Shared mutable array reference causes race condition during flush
Medium Severity
In _popEventContext, traceIds is assigned as a direct reference (traceIds: this._context.traceIds) instead of a copy. The old code used Array.from(this._context.traceIds). When _clearContext runs and traceIds.length <= 1, the array is not reassigned — so the returned context and this._context share the same mutable array. In _runFlush, there's an await this.eventBuffer.finish() between _popEventContext() and sendReplay(), during which new transaction events can .push() into the shared array, corrupting the data being sent.
Additional Locations (1)
…-timestamps-w-trace-ids
billyvg
added a commit
to getsentry/sentry
that referenced
this pull request
Feb 26, 2026
Upgrades Sentry SDKs to 10.41.0-beta.0 to test out the new traces by timestamp for replay events for EAP trace association. getsentry/sentry-javascript#18048
billyvg
added a commit
to getsentry/sentry
that referenced
this pull request
Feb 26, 2026
Upgrades Sentry SDKs to 10.41.0-beta.0 to test out the new traces by timestamp for replay events for EAP trace association. getsentry/sentry-javascript#18048
billyvg
added a commit
to getsentry/sentry-docs
that referenced
this pull request
Feb 26, 2026
Document the new `traces_by_timestamp` field added in getsentry/sentry-javascript#18048. This field provides timestamped trace associations as `[timestamp, trace_id]` pairs, enabling Sentry to correlate replay events with the correct trace by temporal ordering. The existing `trace_ids` field is preserved for backwards compatibility. Co-Authored-By: Claude <noreply@anthropic.com>
6 tasks
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
3 participants
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.
In order to support moving our custom rrweb events to EAP, we need to send timestamps w/ trace ids so that we can identify which trace the event belongs to.
In order to avoid breaking changes, we should not change the current type of
trace_idsfield in the replay event, instead we add a new fieldtraces_by_timestampw/ type[transaction.start_timestamp, traceId][].Previously, we would clear all trace ids after each replay segment. so if there is not a new transaction, segments would not have a trace id associated at all. I've changed this so that we always keep the most recent trace id in between segments.
Also previously, in order to associate a replay with a trace, we wait until after a
type: transactionevent is sent successfully. it's possible that the replay integration is loaded after this event is sent and never gets associated with any trace ids. in this case, I've changed it so that we take the trace id from current scope and propagation context, and I use -1 for the timestamp here, but could also change to use current ts.Closes #19201 (added automatically)