address code review

Author: billyvg

packages/core/src/types-hoist/replay.ts

@@ -9,6 +9,7 @@ export interface ReplayEvent extends Event {
   replay_start_timestamp?: number;
   error_ids: string[];
   trace_ids: string[];
+  // Data expected is [timestamp (seconds), trace id]
   traces_by_timestamp: [number, string][];
   replay_id: string;
   segment_id: number;

packages/replay-internal/src/replay.ts

@@ -1140,7 +1140,16 @@ export class ReplayContainer implements ReplayContainerInterface {
     if (this._context.traceIds.length === 0) {
       const currentTraceId = getCurrentScope().getPropagationContext().traceId;
       if (currentTraceId) {
-        this._context.traceIds.push([-1, currentTraceId]);
+        // Previously, in order to associate a replay with a trace, sdk waits
+        // until after a `type: transaction` event is sent successfully. it's
+        // possible that the replay integration is loaded after this event is
+        // sent and never gets associated with any trace. in this case, use the
+        // trace from current scope and propagation context. We associate the
+        // current trace w/ the earliest timestamp in event buffer.
+        //
+        // This is in seconds to be consistent with how we normally collect
+        // trace ids from the SDK hook event
+        this._context.traceIds.push([this._context.initialTimestamp / 1000, currentTraceId]);
       }
     }
     const _context = {

packages/replay-internal/src/util/sendReplayRequest.ts

@@ -44,9 +44,7 @@ export async function sendReplayRequest({
     timestamp: timestamp / 1000,
     error_ids: errorIds,
     trace_ids: uniqueTraceIds,
-    traces_by_timestamp: traceIds
-      .filter(([_ts, traceId]) => uniqueTraceIds.includes(traceId))
-      .map(([ts, traceId]) => [ts, traceId]),
+    traces_by_timestamp: traceIds.map(([ts, traceId]) => [ts, traceId]),
     urls,
     replay_id: replayId,
     segment_id,

packages/replay-internal/test/integration/coreHandlers/handleAfterSendEvent.test.ts

@@ -53,7 +53,7 @@ describe('Integration | coreHandlers | handleAfterSendEvent', () => {
     handler(error4, { statusCode: undefined });
 
     expect(Array.from(replay.getContext().errorIds)).toEqual(['err2']);
-    expect(Array.from(replay.getContext().traceIds)).toEqual([[-1, expect.any(String)]]);
+    expect(Array.from(replay.getContext().traceIds)).toEqual([[expect.any(Number), expect.any(String)]]);
   });
 
   it('records traceIds from sent transaction events', async () => {
@@ -122,7 +122,7 @@ describe('Integration | coreHandlers | handleAfterSendEvent', () => {
         .fill(undefined)
         .map((_, i) => `err-${i}`),
     );
-    expect(replay.getContext().traceIds).toEqual([[-1, expect.any(String)]]);
+    expect(replay.getContext().traceIds).toEqual([[expect.any(Number), expect.any(String)]]);
   });
 
   it('limits traceIds to max. 100', async () => {

packages/replay-internal/test/integration/errorSampleRate.test.ts

@@ -723,7 +723,7 @@ describe('Integration | errorSampleRate', () => {
           timestamp: (BASE_TIMESTAMP + DEFAULT_FLUSH_MIN_DELAY + DEFAULT_FLUSH_MIN_DELAY) / 1000,
           error_ids: [expect.any(String)],
           trace_ids: [expect.any(String)],
-          traces_by_timestamp: [[-1, expect.any(String)]],
+          traces_by_timestamp: [[expect.any(Number), expect.any(String)]],
           urls: ['http://localhost:3000/'],
           replay_id: expect.any(String),
         }),
@@ -932,7 +932,7 @@ describe('Integration | errorSampleRate', () => {
           replay_start_timestamp: (BASE_TIMESTAMP + stepDuration * steps) / 1000,
           error_ids: [expect.any(String)],
           trace_ids: [expect.any(String)],
-          traces_by_timestamp: [[-1, expect.any(String)]],
+          traces_by_timestamp: [[expect.any(Number), expect.any(String)]],
           urls: ['http://localhost:3000/'],
           replay_id: expect.any(String),
         }),

packages/replay-internal/test/integration/sendReplayEvent.test.ts

@@ -341,7 +341,7 @@ describe('Integration | sendReplayEvent', () => {
         // timestamp is set on first try, after 5s flush
         timestamp: (BASE_TIMESTAMP + 5000) / 1000,
         trace_ids: [expect.any(String)],
-        traces_by_timestamp: [[-1, expect.any(String)]],
+        traces_by_timestamp: [[expect.any(Number), expect.any(String)]],
         urls: ['http://localhost:3000/'],
       }),
       recordingPayloadHeader: { segment_id: 0 },

packages/replay-internal/test/integration/session.test.ts

@@ -252,7 +252,7 @@ describe('Integration | session', () => {
       initialTimestamp: newTimestamp - DEFAULT_FLUSH_MIN_DELAY,
       urls: [],
       errorIds: new Set(),
-      traceIds: [[-1, expect.any(String)]],
+      traceIds: [[expect.any(Number), expect.any(String)]],
     });
   });