Skip to content

ref(replay): Refactor the "Next Breadcrumb" button to use *Frame types #52931

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 3 commits into from
Jul 17, 2023
Merged

ref(replay): Refactor the "Next Breadcrumb" button to use *Frame types #52931

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
1b98bd0
ref(replay): Refactor the "Next Breadcrumb" button to use *Frame types
ryan953 Jul 14, 2023
606ea48
also cleanup some more of the extra reader methods
ryan953 Jul 14, 2023
b7b5129
Merge branch 'master' into ryan953/replay-next-breadcrumb-uses-frames
ryan953 Jul 17, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
17 changes: 8 additions & 9 deletions static/app/components/replays/replayController.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ import {CompositeSelect} from 'sentry/components/compactSelect/composite';
import {PlayerScrubber} from 'sentry/components/replays/player/scrubber';
import useScrubberMouseTracking from 'sentry/components/replays/player/useScrubberMouseTracking';
import {useReplayContext} from 'sentry/components/replays/replayContext';
import {formatTime, relativeTimeInMs} from 'sentry/components/replays/utils';
import {formatTime} from 'sentry/components/replays/utils';
import {
IconContract,
IconExpand,
Expand All @@ -24,7 +24,7 @@ import ConfigStore from 'sentry/stores/configStore';
import {useLegacyStore} from 'sentry/stores/useLegacyStore';
import {space} from 'sentry/styles/space';
import {trackAnalytics} from 'sentry/utils/analytics';
import {getNextReplayEvent} from 'sentry/utils/replays/getReplayEvent';
import {getNextReplayFrame} from 'sentry/utils/replays/getReplayEvent';
import useFullscreen from 'sentry/utils/replays/hooks/useFullscreen';
import useOrganization from 'sentry/utils/useOrganization';

Expand Down Expand Up @@ -79,17 +79,16 @@ function ReplayPlayPauseBar() {
title={t('Next breadcrumb')}
icon={<IconNext size="sm" />}
onClick={() => {
const startTimestampMs = replay?.getReplay().started_at?.getTime();
if (!startTimestampMs) {
if (!replay) {
return;
}
const next = getNextReplayEvent({
items: replay?.getUserActionCrumbs() || [],
targetTimestampMs: startTimestampMs + currentTime,
const next = getNextReplayFrame({
frames: replay.getChapterFrames(),
targetOffsetMs: currentTime,
});

if (startTimestampMs !== undefined && next?.timestamp) {
setCurrentTime(relativeTimeInMs(next.timestamp, startTimestampMs));
if (next) {
setCurrentTime(next.offsetMs);
}
}}
aria-label={t('Fast-forward to next breadcrumb')}
Expand Down
69 changes: 37 additions & 32 deletions static/app/utils/replays/getReplayEvent.spec.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,8 +1,9 @@
import {BreadcrumbLevelType, BreadcrumbType, Crumb} from 'sentry/types/breadcrumbs';
import {
getNextReplayEvent,
getNextReplayFrame,
getPrevReplayEvent,
} from 'sentry/utils/replays/getReplayEvent';
import hydrateBreadcrumbs from 'sentry/utils/replays/hydrateBreadcrumbs';

const START_TIMESTAMP_SEC = 1651693622.951;
const CURRENT_TIME_MS = 15000;
Expand Down Expand Up @@ -73,67 +74,71 @@ function createCrumbs(): Crumb[] {
];
}

describe('getNextReplayEvent', () => {
describe('getNextReplayFrame', () => {
const frames = hydrateBreadcrumbs(TestStubs.ReplayRecord(), [
TestStubs.Replay.ClickFrame({timestamp: new Date('2022-05-11T22:41:32.002Z')}),
TestStubs.Replay.ClickFrame({timestamp: new Date('2022-05-04T19:47:08.085000Z')}),
TestStubs.Replay.ClickFrame({timestamp: new Date('2022-05-04T19:47:11.086000Z')}),
TestStubs.Replay.ClickFrame({timestamp: new Date('2022-05-04T19:47:52.915000Z')}),
TestStubs.Replay.ClickFrame({timestamp: new Date('2022-05-04T19:47:59.915000Z')}),
]);

TestStubs.Replay.ClickEvent;
it('should return the next crumb', () => {
const crumbs = createCrumbs();
const results = getNextReplayEvent({
items: crumbs,
targetTimestampMs: START_TIMESTAMP_SEC * 1000 + CURRENT_TIME_MS,
const results = getNextReplayFrame({
frames,
targetOffsetMs: CURRENT_TIME_MS,
});

expect(results?.id).toEqual(20);
expect(results).toEqual(frames[1]);
});

it('should return the next crumb when the the list is not sorted', () => {
const [one, two, three, four, five] = createCrumbs();
const results = getNextReplayEvent({
items: [one, four, five, three, two],
targetTimestampMs: START_TIMESTAMP_SEC * 1000 + CURRENT_TIME_MS,
const [one, two, three, four, five] = frames;
const results = getNextReplayFrame({
frames: [one, four, five, three, two],
targetOffsetMs: CURRENT_TIME_MS,
});

expect(results?.id).toEqual(20);
expect(results).toEqual(frames[1]);
});

it('should return undefined when there are no crumbs', () => {
const crumbs = [];
const results = getNextReplayEvent({
items: crumbs,
targetTimestampMs: START_TIMESTAMP_SEC * 1000 + CURRENT_TIME_MS,
const results = getNextReplayFrame({
frames: [],
targetOffsetMs: CURRENT_TIME_MS,
});

expect(results).toBeUndefined();
});

it('should return undefined when the timestamp is later than any crumbs', () => {
const crumbs = createCrumbs();
const results = getNextReplayEvent({
items: crumbs,
targetTimestampMs: START_TIMESTAMP_SEC * 1000 + 99999999999,
const results = getNextReplayFrame({
frames,
targetOffsetMs: 99999999999,
});

expect(results).toBeUndefined();
});

it('should return the crumb after when a timestamp exactly matches', () => {
const crumbs = createCrumbs();
const exactCrumbTime = 8135;
const results = getNextReplayEvent({
items: crumbs,
targetTimestampMs: START_TIMESTAMP_SEC * 1000 + exactCrumbTime,
const exactTime = 8135;
const results = getNextReplayFrame({
frames,
targetOffsetMs: exactTime,
});

expect(results?.id).toEqual(20);
expect(results).toEqual(frames[1]);
});

it('should return the crumb if timestamps exactly match and allowMatch is enabled', () => {
const crumbs = createCrumbs();
const exactCrumbTime = 8135;
const results = getNextReplayEvent({
items: crumbs,
targetTimestampMs: START_TIMESTAMP_SEC * 1000 + exactCrumbTime,
const exactTime = 8135;
const results = getNextReplayFrame({
frames,
targetOffsetMs: exactTime,
});

expect(results?.id).toEqual(20);
expect(results).toEqual(frames[1]);
});
});

Expand Down
21 changes: 10 additions & 11 deletions static/app/utils/replays/getReplayEvent.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import sortedIndexBy from 'lodash/sortedIndexBy';

import type {Crumb} from 'sentry/types/breadcrumbs';
import type {ReplayFrame} from 'sentry/utils/replays/types';
import type {ReplaySpan} from 'sentry/views/replays/types';

export function getPrevReplayEvent<T extends ReplaySpan | Crumb>({
Expand All @@ -27,25 +28,23 @@ export function getPrevReplayEvent<T extends ReplaySpan | Crumb>({
return undefined;
}

export function getNextReplayEvent<T extends ReplaySpan | Crumb>({
items,
targetTimestampMs,
export function getNextReplayFrame({
frames,
targetOffsetMs,
allowExact = false,
}: {
items: T[];
targetTimestampMs: number;
frames: ReplayFrame[];
targetOffsetMs: number;
allowExact?: boolean;
}) {
return items.reduce<T | undefined>((found, item) => {
const itemTimestampMS = +new Date(item.timestamp || '');

return frames.reduce<ReplayFrame | undefined>((found, item) => {
if (
itemTimestampMS < targetTimestampMs ||
(!allowExact && itemTimestampMS === targetTimestampMs)
item.offsetMs < targetOffsetMs ||
(!allowExact && item.offsetMs === targetOffsetMs)
) {
return found;
}
if (!found || itemTimestampMS < +new Date(found.timestamp || '')) {
if (!found || item.timestampMs < found.timestampMs) {
return item;
}
return found;
Expand Down
28 changes: 0 additions & 28 deletions static/app/utils/replays/replayReader.tsx
View file
Open in desktop
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Were these all unused?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yes. i've done a bad job removing them after converting things over.

Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,6 @@ import type {
SpanFrame,
} from 'sentry/utils/replays/types';
import type {
MemorySpan,
NetworkSpan,
RecordingEvent,
ReplayCrumb,
Expand Down Expand Up @@ -309,33 +308,12 @@ export default class ReplayReader {
/*********************/
/** OLD STUFF BELOW **/
/*********************/
getCrumbsWithRRWebNodes = memoize(() =>
this.breadcrumbs.filter(
crumb => crumb.data && typeof crumb.data === 'object' && 'nodeId' in crumb.data
)
);

getUserActionCrumbs = memoize(() => {
const USER_ACTIONS = [
BreadcrumbType.ERROR,
BreadcrumbType.INIT,
BreadcrumbType.NAVIGATION,
BreadcrumbType.UI,
BreadcrumbType.USER,
];
return this.breadcrumbs.filter(crumb => USER_ACTIONS.includes(crumb.type));
});

getConsoleCrumbs = memoize(() =>
this.breadcrumbs.filter(crumb => crumb.category === 'console')
);

getRawErrors = memoize(() => this.rawErrors);

getIssueCrumbs = memoize(() =>
this.breadcrumbs.filter(crumb => crumb.category === 'issue')
);

getNonConsoleCrumbs = memoize(() =>
this.breadcrumbs.filter(crumb => crumb.category !== 'console')
);
Expand All @@ -347,14 +325,8 @@ export default class ReplayReader {
);

getNetworkSpans = memoize(() => this.sortedSpans.filter(isNetworkSpan));

getMemorySpans = memoize(() => this.sortedSpans.filter(isMemorySpan));
}

const isMemorySpan = (span: ReplaySpan): span is MemorySpan => {
return span.op === 'memory';
};

const isNetworkSpan = (span: ReplaySpan): span is NetworkSpan => {
return span.op?.startsWith('navigation.') || span.op?.startsWith('resource.');
};