ref(replays): Hydrate Replay Frame* types for a more typesafe ui #50707

ryan953 · 2023-06-11T19:53:26Z

The idea here is that we can have some standard base-types (BreadcrumbFrame, SpanFrame, and ErrorFrame) which we can return from our ReplayReader instance. With memoized returns we can reduce the amount of iterations that we run against the arrays whenever we're switching tabs in the Replay Details view, this will come at a cost of some more memory.

This PR focuses on adding the new hydrated type definitions, and doing the up-front work to insert missing fields to make things consistent and easy to use. More fields could be added over time too (like bringing back transformCrumbs() or some more specific version.)

To follow up we will need to go through each component and convert types into the new *Frame system. The only tough areas will be the Breadcrumb List and Timeline. Both of those expect arrays of type Breadcrumb, but not they'll be getting Breadcrumb, Spans, and Errors mixed together. It'll be a small matter of some if-statements and then using the correct field.

Related to #47991
Fixes #50590
Fixes #46130

ryan953 · 2023-06-12T13:40:38Z

static/app/utils/replays/replayReader.tsx

+    ...this._breadcrumbFrames.filter(frame =>
+      ['replay.init', 'ui.click', 'replay.mutations', 'ui.slowClickDetected'].includes(
+        frame.category
+      )
+    ),


This is going to filter out custom frames now, so like "Redux Action" for example won't have a home anymore.

We can put it back in here for sure, but maybe it's also a good time to ask where the best spots are for it to be.

Also this hides the different dead/slow/rage click events! #50590

I think we should leave custom breadcrumbs in instead of taking it away. I know some users use the redux crumbs and/or custom crumbs and expect them to be there. I know we have one request for crumbs to be searchable and in the timeline even.

I do agree we should start to think about how to treat them.

#50590 says we should have the dead/rage clicks visible for us too

ya. i sort of noticed that the custom ones were removed after the fact. fixing now!

billyvg · 2023-06-13T13:59:51Z

static/app/utils/replays/hydrateBreadcrumbs.tsx

+    const time = new Date(frame.timestamp * 1000);
+    return {
+      ...frame,
+      offsetMS: Math.abs(time.getTime() - startTimestampMs),


We generally treat abbreviations as a word, but there's no documentation for it (nor can I find an eslint rule for it), but it's also inconsistently used here.

Suggested change

offsetMS: Math.abs(time.getTime() - startTimestampMs),

offsetMs: Math.abs(time.getTime() - startTimestampMs),

billyvg · 2023-06-13T14:01:09Z

static/app/utils/replays/hydrateRRWebRecordingFrames.tsx

+export function recordingEndFrame(replayRecord: ReplayRecord): RecordingFrame {
+  return {
+    type: EventType.Custom,
+    timestamp: replayRecord.finished_at.getTime(),
+    data: {
+      tag: 'replay.end',
+      payload: {},
+    },
+  };
+}


Does this actually get used? I can't recall ever seeing it in a replay.

This is a slight tweak on what we were doing before.

Before: the code would amend the timestamp of the first rrweb event to be earlier. This is so that rrweb aligned with the earliest crumb&span&error data we got.
There was also a replay-init breadcrumb created that was inconsistently displayed in the timeline & breadcrumb list.
The code would insert a replay-end rrweb event, so the rrweb player would have an end timestamp that matches the last crumb&span&error data.

Thinking about this fresh:

I still do think it's nice to have the replay-init breadcrumb, and we should display it.

It's important to have the rrweb events match crumb&span&error data so that timeline rendering matches with the player. This could accounts for some drift between the sdk/serverside 'duration' being different from the calculated one; but i don't think it's the whole story (duration on the index page is not calculated, and can be days long)

billyvg · 2023-06-13T14:04:02Z

static/app/utils/replays/replayReader.tsx

+      // TODO(replays): We should get correct timestamps from the backend instead
+      // of having to fix them up here.


Can we track how often these two timestamps are still mismatched? It's more of a SDK issue than backend.

ya good call. i'll throw it into amplitude so we can see the good vs. bad counts and maybe be able to group by unique replayid

static/app/utils/replays/replayReader.tsx

billyvg · 2023-06-13T14:20:50Z

static/app/utils/replays/replayReader.tsx

+    ...this._breadcrumbFrames.filter(frame =>
+      ['replay.init', 'ui.click', 'replay.mutations', 'ui.slowClickDetected'].includes(
+        frame.category
+      )
+    ),


I think we should leave custom breadcrumbs in instead of taking it away. I know some users use the redux crumbs and/or custom crumbs and expect them to be there. I know we have one request for crumbs to be searchable and in the timeline even.

I do agree we should start to think about how to treat them.

#50590 says we should have the dead/rage clicks visible for us too

…ut no where else

…manually sort later (#50901) This is a followup from #50707 Fixes #50891

ryan953 requested a review from a team as a code owner June 11, 2023 19:53

github-actions bot added the Scope: Frontend Automatically applied to PRs that change frontend components label Jun 11, 2023

vercel bot deployed to Preview June 11, 2023 19:56 View deployment

ryan953 force-pushed the ryan953/47991-hydrate-replay-frame-types branch from b937961 to a557240 Compare June 11, 2023 19:56

vercel bot deployed to Preview June 11, 2023 19:59 View deployment

ryan953 force-pushed the ryan953/47991-hydrate-replay-frame-types branch from a557240 to f1ce45e Compare June 11, 2023 22:25

vercel bot deployed to Preview June 11, 2023 22:27 View deployment

ryan953 force-pushed the ryan953/47991-hydrate-replay-frame-types branch from f1ce45e to b45d507 Compare June 11, 2023 22:53

vercel bot deployed to Preview June 11, 2023 22:56 View deployment

ryan953 commented Jun 12, 2023

View reviewed changes

ryan953 requested review from billyvg and eliashussary June 12, 2023 15:30

billyvg reviewed Jun 13, 2023

View reviewed changes

ryan953 requested a review from billyvg June 13, 2023 21:46

vercel bot deployed to Preview June 13, 2023 21:48 View deployment

vercel bot deployed to Preview June 13, 2023 21:54 View deployment

billyvg approved these changes Jun 13, 2023

View reviewed changes

ryan953 added 7 commits June 13, 2023 15:46

ref(replays): Hydrate Replay Frame* types for a more typesafe ui

da9c432

offsetMs and timestampMs, also comment the fields

51e192d

allow unknown breadcrumb types in the breadcrumb list

19e952a

log the delta in started_at and finished_at when we do the frontend calc

17965b3

note about why we sort attachments

20827cf

test that user-generated breadcrumbs appear in the breadcrumb list, b…

a3cf67b

…ut no where else

also track replayId so we can dedupe unique objects

003ddb4

ryan953 force-pushed the ryan953/47991-hydrate-replay-frame-types branch from 9d45aab to 003ddb4 Compare June 13, 2023 22:51

vercel bot deployed to Preview June 13, 2023 22:54 View deployment

ryan953 mentioned this pull request Jun 13, 2023

ref(replay): Accept replay attachments in-order so we do not have to manually sort later #50901

Merged

Merge branch 'master' into ryan953/47991-hydrate-replay-frame-types

0696a04

vercel bot deployed to Preview June 13, 2023 23:44 View deployment

lint

921ada3

vercel bot deployed to Preview June 14, 2023 20:29 View deployment

fix exported type name

fdabd1b

vercel bot deployed to Preview June 16, 2023 16:40 View deployment

ryan953 merged commit cb68d30 into master Jun 16, 2023

ryan953 deleted the ryan953/47991-hydrate-replay-frame-types branch June 16, 2023 17:25

ryan953 added a commit that referenced this pull request Jun 16, 2023

ref(replay): Accept replay attachments in-order so we do not have to …

e4abf10

…manually sort later (#50901) This is a followup from #50707 Fixes #50891

github-actions bot locked and limited conversation to collaborators Jul 2, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Uh oh!

ref(replays): Hydrate Replay Frame* types for a more typesafe ui #50707

ref(replays): Hydrate Replay Frame* types for a more typesafe ui #50707

Uh oh!

ryan953 commented Jun 11, 2023 •

edited

Loading

Uh oh!

ryan953 Jun 12, 2023

Uh oh!

ryan953 Jun 12, 2023

Uh oh!

billyvg Jun 13, 2023

Uh oh!

ryan953 Jun 13, 2023

Uh oh!

billyvg Jun 13, 2023

Uh oh!

billyvg Jun 13, 2023

Uh oh!

ryan953 Jun 13, 2023

Uh oh!

billyvg Jun 13, 2023

Uh oh!

ryan953 Jun 13, 2023

Uh oh!

Uh oh!

billyvg Jun 13, 2023

Uh oh!

Uh oh!

	offsetMS: Math.abs(time.getTime() - startTimestampMs),
	offsetMs: Math.abs(time.getTime() - startTimestampMs),

		// TODO(replays): We should get correct timestamps from the backend instead
		// of having to fix them up here.

Uh oh!

ref(replays): Hydrate Replay Frame* types for a more typesafe ui #50707

ref(replays): Hydrate Replay Frame* types for a more typesafe ui #50707

Uh oh!

Conversation

ryan953 commented Jun 11, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

ryan953 commented Jun 11, 2023 •

edited

Loading