Skip to content

ref(replays): Refactor breadcrumb accessors to be memoized and central in ReplayReader #48745

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.

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
May 9, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
11 changes: 1 addition & 10 deletions static/app/components/replays/breadcrumbs/replayTimeline.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -14,17 +14,9 @@ import {TimelineScrubber} from 'sentry/components/replays/player/scrubber';
import useScrubberMouseTracking from 'sentry/components/replays/player/useScrubberMouseTracking';
import {useReplayContext} from 'sentry/components/replays/replayContext';
import {Resizeable} from 'sentry/components/replays/resizeable';
import {BreadcrumbType} from 'sentry/types/breadcrumbs';

type Props = {};

const USER_ACTIONS = [
BreadcrumbType.ERROR,
BreadcrumbType.NAVIGATION,
BreadcrumbType.UI,
BreadcrumbType.USER,
];

function ReplayTimeline({}: Props) {
const {replay} = useReplayContext();

Expand All @@ -37,8 +29,7 @@ function ReplayTimeline({}: Props) {

const durationMs = replay.getDurationMs();
const startTimestampMs = replay.getReplay().started_at.getTime();
const crumbs = replay.getRawCrumbs();
const userCrumbs = crumbs.filter(crumb => USER_ACTIONS.includes(crumb.type));
const userCrumbs = replay.getUserActionCrumbs();
const networkSpans = replay.getNetworkSpans();

return (
Expand Down
12 changes: 1 addition & 11 deletions static/app/components/replays/replayController.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,6 @@ import {t} from 'sentry/locale';
import ConfigStore from 'sentry/stores/configStore';
import {useLegacyStore} from 'sentry/stores/useLegacyStore';
import {space} from 'sentry/styles/space';
import {BreadcrumbType} from 'sentry/types/breadcrumbs';
import {trackAnalytics} from 'sentry/utils/analytics';
import {getNextReplayEvent} from 'sentry/utils/replays/getReplayEvent';
import useFullscreen from 'sentry/utils/replays/hooks/useFullscreen';
Expand All @@ -33,14 +32,6 @@ const SECOND = 1000;

const COMPACT_WIDTH_BREAKPOINT = 500;

const USER_ACTIONS = [
BreadcrumbType.ERROR,
BreadcrumbType.INIT,
BreadcrumbType.NAVIGATION,
BreadcrumbType.UI,
BreadcrumbType.USER,
];

interface Props {
speedOptions?: number[];
toggleFullscreen?: () => void;
Expand Down Expand Up @@ -92,9 +83,8 @@ function ReplayPlayPauseBar() {
if (!startTimestampMs) {
return;
}
const transformedCrumbs = replay?.getRawCrumbs() || [];
const next = getNextReplayEvent({
items: transformedCrumbs.filter(crumb => USER_ACTIONS.includes(crumb.type)),
items: replay?.getUserActionCrumbs() || [],
targetTimestampMs: startTimestampMs + currentTime,
});

Expand Down
2 changes: 1 addition & 1 deletion static/app/components/replays/replayCurrentUrl.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ function ReplayCurrentUrl() {
}

const replayRecord = replay.getReplay();
const crumbs = replay.getRawCrumbs();
const crumbs = replay.getNavCrumbs();

return (
<TextCopyInput size="sm">
Expand Down
12 changes: 0 additions & 12 deletions static/app/components/replays/walker/urlWalker.spec.tsx
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
import {render, screen} from 'sentry-test/reactTestingLibrary';

import type {Crumb} from 'sentry/types/breadcrumbs';
import {BreadcrumbType} from 'sentry/types/breadcrumbs';

import {CrumbWalker, StringWalker} from './urlWalker';

Expand Down Expand Up @@ -72,16 +71,5 @@ describe('UrlWalker', () => {
)
).toBeInTheDocument();
});

it('should filter out non-navigation crumbs', () => {
const ERROR_CRUMB = TestStubs.Breadcrumb({
type: BreadcrumbType.ERROR,
});

const crumbs = [ERROR_CRUMB];

render(<CrumbWalker crumbs={crumbs} replayRecord={replayRecord} />);
expect(screen.getByText('0 Pages')).toBeInTheDocument();
});
});
});
6 changes: 1 addition & 5 deletions static/app/components/replays/walker/urlWalker.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -19,14 +19,10 @@ export const CrumbWalker = memo(function CrumbWalker({crumbs, replayRecord}: Cru
const startTimestampMs = replayRecord.started_at.getTime();
const {handleClick} = useCrumbHandlers(startTimestampMs);

const navCrumbs = crumbs.filter(crumb =>
[BreadcrumbType.INIT, BreadcrumbType.NAVIGATION].includes(crumb.type)
) as Crumb[];

return (
<ChevronDividedList
items={splitCrumbs({
crumbs: navCrumbs,
crumbs,
startTimestampMs,
onClick: handleClick,
})}
Expand Down
14 changes: 6 additions & 8 deletions static/app/utils/replays/getCurrentUrl.tsx
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
import last from 'lodash/last';

import type {Crumb} from 'sentry/types/breadcrumbs';
import {BreadcrumbType, BreadcrumbTypeNavigation} from 'sentry/types/breadcrumbs';
import type {ReplayRecord} from 'sentry/views/replays/types';

function parseUrl(url: string) {
Expand All @@ -20,16 +19,15 @@ function getCurrentUrl(
const startTimestampMs = replayRecord.started_at.getTime();
const currentTimeMs = startTimestampMs + Math.floor(currentOffsetMS);

const navigationCrumbs = crumbs.filter(
crumb => crumb.type === BreadcrumbType.NAVIGATION
) as BreadcrumbTypeNavigation[];

const initialUrl = replayRecord.urls[0];
const origin = parseUrl(initialUrl)?.origin || initialUrl;

const mostRecentNavigation = last(
navigationCrumbs.filter(({timestamp}) => +new Date(timestamp || 0) <= currentTimeMs)
)?.data?.to;
const navigationCrumbs = crumbs.filter(
({timestamp}) => +new Date(timestamp || 0) <= currentTimeMs
);

// @ts-expect-error: Crumb types are not strongly defined in Replay
const mostRecentNavigation = last(navigationCrumbs)?.data?.to;

if (!mostRecentNavigation) {
return origin;
Expand Down
8 changes: 1 addition & 7 deletions static/app/utils/replays/hooks/useExtractedCrumbHtml.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -67,13 +67,7 @@ function useExtractedCrumbHtml({replay}: HookOpts) {

// Get a list of the breadcrumbs that relate directly to the DOM, for each
// crumb we will extract the referenced HTML.
const crumbs = replay
.getRawCrumbs()
.filter(
crumb =>
crumb.data && typeof crumb.data === 'object' && 'nodeId' in crumb.data
);

const crumbs = replay.getCrumbsWithRRWebNodes();
const rrwebEvents = replay.getRRWebEvents();

// Grab the last event, but skip the synthetic `replay-end` event that the
Expand Down
12 changes: 1 addition & 11 deletions static/app/utils/replays/replayDataUtils.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -10,11 +10,7 @@ import type {
Crumb,
RawCrumb,
} from 'sentry/types/breadcrumbs';
import {
BreadcrumbLevelType,
BreadcrumbType,
isBreadcrumbTypeDefault,
} from 'sentry/types/breadcrumbs';
import {BreadcrumbLevelType, BreadcrumbType} from 'sentry/types/breadcrumbs';
import type {
MemorySpanType,
RecordingEvent,
Expand Down Expand Up @@ -66,12 +62,6 @@ export const isNetworkSpan = (span: ReplaySpan) => {
return span.op.startsWith('navigation.') || span.op.startsWith('resource.');
};

export const getBreadcrumbsByCategory = (breadcrumbs: Crumb[], categories: string[]) => {
return breadcrumbs
.filter(isBreadcrumbTypeDefault)
.filter(breadcrumb => categories.includes(breadcrumb.category || ''));
};

export function mapResponseToReplayRecord(apiResponse: any): ReplayRecord {
// Marshal special fields into tags
const user = Object.fromEntries(
Expand Down
90 changes: 51 additions & 39 deletions static/app/utils/replays/replayReader.tsx
Original file line number Diff line number Diff line change
@@ -1,10 +1,11 @@
import * as Sentry from '@sentry/react';
import memoize from 'lodash/memoize';
import {duration} from 'moment';

import type {Crumb} from 'sentry/types/breadcrumbs';
import {BreadcrumbType} from 'sentry/types/breadcrumbs';
import {
breadcrumbFactory,
getBreadcrumbsByCategory,
isMemorySpan,
isNetworkSpan,
mapRRWebAttachments,
Expand All @@ -13,7 +14,6 @@ import {
spansFactory,
} from 'sentry/utils/replays/replayDataUtils';
import type {
MemorySpanType,
RecordingEvent,
ReplayError,
ReplayRecord,
Expand Down Expand Up @@ -84,26 +84,22 @@ export default class ReplayReader {
replayRecord.finished_at.getTime() - replayRecord.started_at.getTime()
);

const sortedSpans = spansFactory(spans);
this.networkSpans = sortedSpans.filter(isNetworkSpan);
this.memorySpans = sortedSpans.filter(isMemorySpan);

this.breadcrumbs = breadcrumbFactory(replayRecord, errors, breadcrumbs, sortedSpans);
this.consoleCrumbs = getBreadcrumbsByCategory(this.breadcrumbs, ['console', 'issue']);

this.sortedSpans = spansFactory(spans);
this.breadcrumbs = breadcrumbFactory(
replayRecord,
errors,
breadcrumbs,
this.sortedSpans
);
this.rrwebEvents = rrwebEventListFactory(replayRecord, rrwebEvents);

this.replayRecord = replayRecord;
}

private sortedSpans: ReplaySpan[];
private replayRecord: ReplayRecord;
private rrwebEvents: RecordingEvent[];
private breadcrumbs: Crumb[];
private consoleCrumbs: ReturnType<typeof getBreadcrumbsByCategory>;
private networkSpans: ReplaySpan[];
private memorySpans: MemorySpanType[];

private _isDetailsSetup: undefined | boolean;

/**
* @returns Duration of Replay (milliseonds)
Expand All @@ -120,30 +116,46 @@ export default class ReplayReader {
return this.rrwebEvents;
};

getRawCrumbs = () => {
return this.breadcrumbs;
};

getConsoleCrumbs = () => {
return this.consoleCrumbs;
};

getNetworkSpans = () => {
return this.networkSpans;
};

getMemorySpans = () => {
return this.memorySpans;
};

isNetworkDetailsSetup = () => {
if (this._isDetailsSetup === undefined) {
// TODO(replay): there must be a better way
const hasHeaders = span =>
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 => ['console', 'issue'].includes(crumb.category || ''))
);

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

getNavCrumbs = memoize(() =>
this.breadcrumbs.filter(crumb =>
[BreadcrumbType.INIT, BreadcrumbType.NAVIGATION].includes(crumb.type)
)
);

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

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

isNetworkDetailsSetup = memoize(() =>
this.getNetworkSpans().some(
span =>
Object.keys(span.data.request?.headers || {}).length ||
Object.keys(span.data.response?.headers || {}).length;
this._isDetailsSetup = this.networkSpans.some(hasHeaders);
}
return this._isDetailsSetup;
};
Object.keys(span.data.response?.headers || {}).length
)
);
}
12 changes: 4 additions & 8 deletions static/app/views/replays/detail/breadcrumbs/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -34,11 +34,7 @@ const cellMeasurer = {

function Breadcrumbs({breadcrumbs, startTimestampMs}: Props) {
const {currentTime, currentHoverTime} = useReplayContext();
const items = useMemo(
() =>
(breadcrumbs || []).filter(crumb => !['console'].includes(crumb.category || '')),
[breadcrumbs]
);

const listRef = useRef<ReactVirtualizedList>(null);
// Keep a reference of object paths that are expanded (via <ObjectInspector>)
// by log row, so they they can be restored as the Console pane is scrolling.
Expand Down Expand Up @@ -82,7 +78,7 @@ function Breadcrumbs({breadcrumbs, startTimestampMs}: Props) {
[itemLookup, breadcrumbs, currentHoverTime, startTimestampMs]
);

const deps = useMemo(() => [items], [items]);
const deps = useMemo(() => [breadcrumbs], [breadcrumbs]);
const {cache, updateList} = useVirtualizedList({
cellMeasurer,
ref: listRef,
Expand All @@ -101,7 +97,7 @@ function Breadcrumbs({breadcrumbs, startTimestampMs}: Props) {
});

const renderRow = ({index, key, style, parent}: ListRowProps) => {
const item = items[index];
const item = (breadcrumbs || [])[index];

return (
<CellMeasurer
Expand Down Expand Up @@ -141,7 +137,7 @@ function Breadcrumbs({breadcrumbs, startTimestampMs}: Props) {
)}
overscanRowCount={5}
ref={listRef}
rowCount={items.length}
rowCount={breadcrumbs.length}
rowHeight={cache.rowHeight}
rowRenderer={renderRow}
width={width}
Expand Down
4 changes: 2 additions & 2 deletions static/app/views/replays/detail/console/consoleFilters.tsx
Original file line number Diff line number Diff line change
@@ -1,12 +1,12 @@
import {CompactSelect} from 'sentry/components/compactSelect';
import SearchBar from 'sentry/components/searchBar';
import {t} from 'sentry/locale';
import type {BreadcrumbTypeDefault, Crumb} from 'sentry/types/breadcrumbs';
import type {Crumb} from 'sentry/types/breadcrumbs';
import useConsoleFilters from 'sentry/views/replays/detail/console/useConsoleFilters';
import FiltersGrid from 'sentry/views/replays/detail/filtersGrid';

type Props = {
breadcrumbs: undefined | Extract<Crumb, BreadcrumbTypeDefault>[];
breadcrumbs: undefined | Crumb[];
} & ReturnType<typeof useConsoleFilters>;

function Filters({
Expand Down
4 changes: 2 additions & 2 deletions static/app/views/replays/detail/console/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ import {
import Placeholder from 'sentry/components/placeholder';
import {useReplayContext} from 'sentry/components/replays/replayContext';
import {t} from 'sentry/locale';
import type {BreadcrumbTypeDefault, Crumb} from 'sentry/types/breadcrumbs';
import type {Crumb} from 'sentry/types/breadcrumbs';
import ConsoleFilters from 'sentry/views/replays/detail/console/consoleFilters';
import ConsoleLogRow from 'sentry/views/replays/detail/console/consoleLogRow';
import useConsoleFilters from 'sentry/views/replays/detail/console/useConsoleFilters';
Expand All @@ -21,7 +21,7 @@ import useVirtualizedList from 'sentry/views/replays/detail/useVirtualizedList';
import useVirtualizedInspector from '../useVirtualizedInspector';

interface Props {
breadcrumbs: undefined | Extract<Crumb, BreadcrumbTypeDefault>[];
breadcrumbs: undefined | Crumb[];
startTimestampMs: number;
}

Expand Down
2 changes: 1 addition & 1 deletion static/app/views/replays/detail/layout/sidebarArea.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ function SidebarArea() {
default:
return (
<Breadcrumbs
breadcrumbs={replay?.getRawCrumbs()}
breadcrumbs={replay?.getNonConsoleCrumbs()}
startTimestampMs={replay?.getReplay()?.started_at?.getTime() || 0}
/>
);
Expand Down
Loading