Browse Source

ref(replay): Refactor ReplayReader.isNetworkDetailsSetup to use getSDKOptions internally (#51928)

I was also able to remove some dead code along the way.
Ryan Albrecht 1 year ago
parent
commit
a746bc47b1

+ 1 - 51
static/app/utils/replays/replayDataUtils.spec.tsx

@@ -1,8 +1,4 @@
-import {
-  breadcrumbFactory,
-  rrwebEventListFactory,
-} from 'sentry/utils/replays/replayDataUtils';
-import type {ReplayRecord} from 'sentry/views/replays/types';
+import {breadcrumbFactory} from 'sentry/utils/replays/replayDataUtils';
 
 const fooSpan = TestStubs.ReplaySpanPayload({
   op: 'foo',
@@ -142,49 +138,3 @@ describe('breadcrumbFactory', () => {
     expect(toTime(results[2].timestamp)).toBeLessThan(toTime(results[3].timestamp));
   });
 });
-
-describe('rrwebEventListFactory', () => {
-  it('returns a list of replay events for highlights', function () {
-    const replayRecord = {
-      started_at: new Date(13),
-      finished_at: new Date(213),
-    } as ReplayRecord;
-
-    const results = rrwebEventListFactory(replayRecord, []);
-
-    expect(results).toMatchInlineSnapshot(`
-      [
-        {
-          "data": {
-            "tag": "replay-end",
-          },
-          "timestamp": 13,
-          "type": 5,
-        },
-      ]
-    `);
-  });
-
-  it('merges and sorts rrweb-events and span data', function () {
-    const startTimestampMs = 0;
-    const endTimestampMs = 10_000;
-
-    const replayRecord = {
-      started_at: new Date(startTimestampMs),
-      finished_at: new Date(endTimestampMs),
-    } as ReplayRecord;
-
-    expect(
-      rrwebEventListFactory(replayRecord, [
-        {type: 0, timestamp: 5_000, data: {}},
-        {type: 1, timestamp: 1_000, data: {}},
-        {type: 2, timestamp: 3_000, data: {} as any},
-      ])
-    ).toEqual([
-      {type: 1, timestamp: 0, data: {}},
-      {type: 2, timestamp: 3_000, data: {}},
-      {type: 0, timestamp: 5_000, data: {}},
-      {type: 5, timestamp: 10_000, data: {tag: 'replay-end'}},
-    ]);
-  });
-});

+ 0 - 22
static/app/utils/replays/replayDataUtils.tsx

@@ -1,5 +1,4 @@
 import invariant from 'invariant';
-import first from 'lodash/first';
 import {duration} from 'moment';
 
 import {transformCrumbs} from 'sentry/components/events/interfaces/breadcrumbs/utils';
@@ -15,7 +14,6 @@ import {BreadcrumbLevelType, BreadcrumbType} from 'sentry/types/breadcrumbs';
 import isValidDate from 'sentry/utils/date/isValidDate';
 import getMinMax from 'sentry/utils/getMinMax';
 import type {
-  RecordingEvent,
   ReplayCrumb,
   ReplayError,
   ReplayRecord,
@@ -74,26 +72,6 @@ export function mapResponseToReplayRecord(apiResponse: any): ReplayRecord {
   };
 }
 
-export function rrwebEventListFactory(
-  replayRecord: ReplayRecord,
-  rrwebEvents: RecordingEvent[]
-) {
-  const events = ([] as RecordingEvent[]).concat(rrwebEvents).concat({
-    type: 5, // EventType.Custom,
-    timestamp: replayRecord.finished_at.getTime(),
-    data: {
-      tag: 'replay-end',
-    },
-  } as RecordingEvent);
-
-  events.sort((a, b) => a.timestamp - b.timestamp);
-
-  const firstRRWebEvent = first(events) as RecordingEvent;
-  firstRRWebEvent.timestamp = replayRecord.started_at.getTime();
-
-  return events;
-}
-
 export function breadcrumbFactory(
   replayRecord: ReplayRecord,
   errors: ReplayError[],

+ 1 - 1
static/app/utils/replays/replayReader.spec.tsx

@@ -198,7 +198,7 @@ describe('ReplayReader', () => {
       replayRecord,
     });
 
-    expect(replay?.sdkConfig()).toBe(optionsFrame);
+    expect(replay?.getSDKOptions()).toBe(optionsFrame);
   });
 
   describe('isNetworkDetailsSetup', () => {

+ 19 - 32
static/app/utils/replays/replayReader.tsx

@@ -20,7 +20,6 @@ import hydrateSpans from 'sentry/utils/replays/hydrateSpans';
 import {
   breadcrumbFactory,
   replayTimestamps,
-  rrwebEventListFactory,
   spansFactory,
 } from 'sentry/utils/replays/replayDataUtils';
 import splitAttachmentsByType from 'sentry/utils/replays/splitAttachmentsByType';
@@ -32,12 +31,11 @@ import type {
   RecordingFrame,
   SpanFrame,
 } from 'sentry/utils/replays/types';
-import {BreadcrumbCategories, EventType} from 'sentry/utils/replays/types';
+import {BreadcrumbCategories} from 'sentry/utils/replays/types';
 import type {
   MemorySpan,
   NetworkSpan,
   RecordingEvent,
-  RecordingOptions,
   ReplayCrumb,
   ReplayError,
   ReplayRecord,
@@ -183,10 +181,6 @@ export default class ReplayReader {
       rawBreadcrumbs as ReplayCrumb[],
       this.sortedSpans
     );
-    this.rrwebEvents = rrwebEventListFactory(
-      replayRecord,
-      rawRRWebEvents as RecordingEvent[]
-    );
 
     this.replayRecord = replayRecord;
   }
@@ -203,7 +197,6 @@ export default class ReplayReader {
   private rawErrors: ReplayError[];
   private sortedSpans: ReplaySpan[];
   private replayRecord: ReplayRecord;
-  private rrwebEvents: RecordingEvent[];
   private breadcrumbs: Crumb[];
 
   toJSON = () => this._cacheKey;
@@ -291,7 +284,24 @@ export default class ReplayReader {
 
   getSDKOptions = () => this._optionFrame;
 
-  // TODO: move isNetworkDetailsSetup() up here? or extract it
+  isNetworkDetailsSetup = memoize(() => {
+    const sdkOptions = this.getSDKOptions();
+    if (sdkOptions) {
+      return sdkOptions.networkDetailHasUrls;
+    }
+
+    // Network data was added in JS SDK 7.50.0 while sdkConfig was added in v7.51.1
+    // So even if we don't have the config object, we should still fallback and
+    // look for spans with network data, as that means things are setup!
+    return this.getNetworkFrames().some(
+      frame =>
+        // We'd need to `filter()` before calling `some()` in order for TS to be happy
+        // @ts-expect-error
+        Object.keys(frame?.data?.request?.headers ?? {}).length ||
+        // @ts-expect-error
+        Object.keys(frame?.data?.response?.headers ?? {}).length
+    );
+  });
 
   /*********************/
   /** OLD STUFF BELOW **/
@@ -336,29 +346,6 @@ export default class ReplayReader {
   getNetworkSpans = memoize(() => this.sortedSpans.filter(isNetworkSpan));
 
   getMemorySpans = memoize(() => this.sortedSpans.filter(isMemorySpan));
-
-  sdkConfig = memoize(() => {
-    const found = this.rrwebEvents.find(
-      event => event.type === EventType.Custom && event.data.tag === 'options'
-    ) as undefined | RecordingOptions;
-    return found?.data?.payload;
-  });
-
-  isNetworkDetailsSetup = memoize(() => {
-    const config = this.sdkConfig();
-    if (config) {
-      return this.sdkConfig()?.networkDetailHasUrls;
-    }
-
-    // Network data was added in JS SDK 7.50.0 while sdkConfig was added in v7.51.1
-    // So even if we don't have the config object, we should still fallback and
-    // look for spans with network data, as that means things are setup!
-    return this.getNetworkSpans().some(
-      span =>
-        Object.keys(span.data.request?.headers || {}).length ||
-        Object.keys(span.data.response?.headers || {}).length
-    );
-  });
 }
 
 const isMemorySpan = (span: ReplaySpan): span is MemorySpan => {

+ 1 - 14
static/app/views/replays/types.tsx

@@ -1,4 +1,4 @@
-import type {customEvent, eventWithTime} from '@sentry-internal/rrweb/typings/types';
+import type {eventWithTime} from '@sentry-internal/rrweb/typings/types';
 import type {Duration} from 'moment';
 
 import type {RawCrumb} from 'sentry/types/breadcrumbs';
@@ -157,19 +157,6 @@ export interface Highlight {
 }
 
 export type RecordingEvent = eventWithTime;
-export type RecordingOptions = customEvent<{
-  blockAllMedia: boolean;
-  errorSampleRate: number;
-  maskAllInputs: boolean;
-  maskAllText: boolean;
-  networkCaptureBodies: boolean;
-  networkDetailHasUrls: boolean;
-  networkRequestHasHeaders: boolean;
-  networkResponseHasHeaders: boolean;
-  sessionSampleRate: number;
-  useCompression: boolean;
-  useCompressionOption: boolean;
-}>;
 
 export interface ReplaySpan<T = Record<string, any>> {
   data: T;