Skip to content

Commit

Permalink
ref(replay): Refactor Replay Details>Network table to use new *Frame …
Browse files Browse the repository at this point in the history
…types (#53384)
  • Loading branch information
ryan953 committed Jul 24, 2023
1 parent 5cf7648 commit f23055d
Show file tree
Hide file tree
Showing 21 changed files with 434 additions and 425 deletions.
8 changes: 1 addition & 7 deletions static/app/components/replays/virtualizedGrid/headerCell.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -6,18 +6,12 @@ import {IconArrow, IconInfo} from 'sentry/icons';
import {space} from 'sentry/styles/space';
import type {Crumb} from 'sentry/types/breadcrumbs';
import type {BreadcrumbFrame, SpanFrame} from 'sentry/utils/replays/types';
import type {NetworkSpan} from 'sentry/views/replays/types';

interface SortCrumbs {
asc: boolean;
by: keyof Crumb | string;
getValue: (row: Crumb) => any;
}
interface SortSpans {
asc: boolean;
by: keyof NetworkSpan | string;
getValue: (row: NetworkSpan) => any;
}

interface SortBreadcrumbFrame {
asc: boolean;
Expand All @@ -34,7 +28,7 @@ type Props = {
field: string;
handleSort: (fieldName: string) => void;
label: string;
sortConfig: SortCrumbs | SortSpans | SortBreadcrumbFrame | SortSpanFrame;
sortConfig: SortCrumbs | SortBreadcrumbFrame | SortSpanFrame;
style: CSSProperties;
tooltipTitle: undefined | ReactNode;
};
Expand Down
9 changes: 4 additions & 5 deletions static/app/utils/replays/hooks/useCrumbHandlers.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@ import {BreadcrumbType, Crumb} from 'sentry/types/breadcrumbs';
import getFrameDetails from 'sentry/utils/replays/getFrameDetails';
import useActiveReplayTab from 'sentry/utils/replays/hooks/useActiveReplayTab';
import {ReplayFrame} from 'sentry/utils/replays/types';
import type {NetworkSpan} from 'sentry/views/replays/types';

function useCrumbHandlers(startTimestampMs: number = 0) {
const {
Expand All @@ -19,15 +18,15 @@ function useCrumbHandlers(startTimestampMs: number = 0) {
const {setActiveTab} = useActiveReplayTab();

const mouseEnterCallback = useRef<{
id: Crumb | NetworkSpan | ReplayFrame | null;
id: Crumb | ReplayFrame | null;
timeoutId: NodeJS.Timeout | null;
}>({
id: null,
timeoutId: null,
});

const handleMouseEnter = useCallback(
(item: Crumb | NetworkSpan | ReplayFrame) => {
(item: Crumb | ReplayFrame) => {
// this debounces the mouseEnter callback in unison with mouseLeave
// we ensure the pointer remains over the target element before dispatching state events in order to minimize unnecessary renders
// this helps during scrolling or mouse move events which would otherwise fire in rapid succession slowing down our app
Expand Down Expand Up @@ -56,7 +55,7 @@ function useCrumbHandlers(startTimestampMs: number = 0) {
);

const handleMouseLeave = useCallback(
(item: Crumb | NetworkSpan | ReplayFrame) => {
(item: Crumb | ReplayFrame) => {
// if there is a mouseEnter callback queued and we're leaving it we can just cancel the timeout
if (mouseEnterCallback.current.id === item) {
if (mouseEnterCallback.current.timeoutId) {
Expand All @@ -78,7 +77,7 @@ function useCrumbHandlers(startTimestampMs: number = 0) {
);

const handleClick = useCallback(
(crumb: Crumb | NetworkSpan | ReplayFrame) => {
(crumb: Crumb | ReplayFrame) => {
if ('offsetMs' in crumb) {
const frame = crumb; // Finding `offsetMs` means we have a frame, not a crumb or span

Expand Down
14 changes: 0 additions & 14 deletions static/app/utils/replays/replayReader.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@ import memoize from 'lodash/memoize';
import {duration} from 'moment';

import type {Crumb} from 'sentry/types/breadcrumbs';
import {BreadcrumbType} from 'sentry/types/breadcrumbs';
import domId from 'sentry/utils/domId';
import localStorageWrapper from 'sentry/utils/localStorage';
import extractDomNodes from 'sentry/utils/replays/extractDomNodes';
Expand Down Expand Up @@ -34,7 +33,6 @@ import type {
} from 'sentry/utils/replays/types';
import {isDeadClick, isDeadRageClick} from 'sentry/utils/replays/types';
import type {
NetworkSpan,
RecordingEvent,
ReplayCrumb,
ReplayError,
Expand Down Expand Up @@ -311,16 +309,4 @@ export default class ReplayReader {
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));
}

const isNetworkSpan = (span: ReplaySpan): span is NetworkSpan => {
return span.op?.startsWith('navigation.') || span.op?.startsWith('resource.');
};
42 changes: 42 additions & 0 deletions static/app/utils/replays/resourceFrame.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,42 @@
import type {RequestFrame, ResourceFrame, SpanFrame} from 'sentry/utils/replays/types';

export function isRequestFrame(frame: SpanFrame): frame is RequestFrame {
return ['resource.fetch', 'resource.xhr'].includes(frame.op);
}

function isResourceFrame(frame: SpanFrame): frame is ResourceFrame {
return [
'resource.css',
'resource.iframe',
'resource.img',
'resource.link',
'resource.other',
'resource.script',
].includes(frame.op);
}

export function getFrameMethod(frame: SpanFrame) {
return isRequestFrame(frame) ? frame.data.method ?? 'GET' : 'GET';
}

export function getFrameStatus(frame: SpanFrame) {
return isRequestFrame(frame)
? frame.data.statusCode
: isResourceFrame(frame)
? frame.data.statusCode
: undefined;
}

export function getResponseBodySize(frame: SpanFrame) {
if (isRequestFrame(frame)) {
// `data.responseBodySize` is from SDK version 7.44-7.45
return frame.data.response?.size ?? frame.data.responseBodySize;
}
if (isResourceFrame(frame)) {
// What about these?
// frame.data.decodedBodySize
// frame.data.encodedBodySize
return frame.data.size;
}
return undefined;
}
2 changes: 1 addition & 1 deletion static/app/views/replays/detail/layout/focusArea.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ function FocusArea({}: Props) {
return (
<NetworkList
isNetworkDetailsSetup={Boolean(replay?.isNetworkDetailsSetup())}
networkSpans={replay?.getNetworkSpans()}
networkFrames={replay?.getNetworkFrames()}
projectId={replay?.getReplay()?.project_id}
startTimestampMs={replay?.getReplay()?.started_at?.getTime() || 0}
/>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ export function SizeTooltip({children}: {children: ReactNode}) {
}

export function keyValueTableOrNotFound(
data: undefined | Record<string, string>,
data: undefined | Record<string, string | ReactNode>,
notFoundText: string
) {
return data ? (
Expand Down
49 changes: 38 additions & 11 deletions static/app/views/replays/detail/network/details/content.spec.tsx
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import {render, screen} from 'sentry-test/reactTestingLibrary';

import hydrateSpans from 'sentry/utils/replays/hydrateSpans';
import useProjectSdkNeedsUpdate from 'sentry/utils/useProjectSdkNeedsUpdate';
import NetworkDetailsContent from 'sentry/views/replays/detail/network/details/content';
import type {TabKey} from 'sentry/views/replays/detail/network/details/tabs';
Expand All @@ -14,21 +15,30 @@ function mockNeedsUpdate(needsUpdate: boolean) {
mockUseProjectSdkNeedsUpdate.mockReturnValue({isFetching: false, needsUpdate});
}

const mockItems = {
img: TestStubs.ReplaySpanPayload({
const [
img,
fetchNoDataObj,
fetchUrlSkipped,
fetchBodySkipped,
fetchWithHeaders,
fetchWithRespBody,
] = hydrateSpans(TestStubs.ReplayRecord(), [
TestStubs.Replay.ResourceFrame({
op: 'resource.img',
startTimestamp: new Date(),
endTimestamp: new Date(),
description: '/static/img/logo.png',
data: {
method: 'GET',
statusCode: 200,
},
}),
fetchNoDataObj: TestStubs.ReplaySpanPayload({
TestStubs.Replay.RequestFrame({
op: 'resource.fetch',
startTimestamp: new Date(),
endTimestamp: new Date(),
description: '/api/0/issues/1234',
}),
fetchUrlSkipped: TestStubs.ReplaySpanPayload({
TestStubs.Replay.RequestFrame({
op: 'resource.fetch',
startTimestamp: new Date(),
endTimestamp: new Date(),
description: '/api/0/issues/1234',
data: {
method: 'GET',
Expand All @@ -37,24 +47,30 @@ const mockItems = {
response: {_meta: {warnings: ['URL_SKIPPED']}, headers: {}},
},
}),
fetchBodySkipped: TestStubs.ReplaySpanPayload({
TestStubs.Replay.RequestFrame({
op: 'resource.fetch',
startTimestamp: new Date(),
endTimestamp: new Date(),
description: '/api/0/issues/1234',
data: {
method: 'GET',
statusCode: 200,
request: {
// @ts-expect-error
_meta: {warnings: ['BODY_SKIPPED']},
headers: {accept: 'application/json'},
},
response: {
// @ts-expect-error
_meta: {warnings: ['BODY_SKIPPED']},
headers: {'content-type': 'application/json'},
},
},
}),
fetchWithHeaders: TestStubs.ReplaySpanPayload({
TestStubs.Replay.RequestFrame({
op: 'resource.fetch',
startTimestamp: new Date(),
endTimestamp: new Date(),
description: '/api/0/issues/1234',
data: {
method: 'GET',
Expand All @@ -69,8 +85,10 @@ const mockItems = {
},
},
}),
fetchWithRespBody: TestStubs.ReplaySpanPayload({
TestStubs.Replay.RequestFrame({
op: 'resource.fetch',
startTimestamp: new Date(),
endTimestamp: new Date(),
description: '/api/0/issues/1234',
data: {
method: 'GET',
Expand All @@ -86,6 +104,15 @@ const mockItems = {
},
},
}),
]);

const mockItems = {
img,
fetchNoDataObj,
fetchUrlSkipped,
fetchBodySkipped,
fetchWithHeaders,
fetchWithRespBody,
};

function basicSectionProps() {
Expand Down
5 changes: 3 additions & 2 deletions static/app/views/replays/detail/network/details/content.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ import {useEffect} from 'react';
import styled from '@emotion/styled';

import {trackAnalytics} from 'sentry/utils/analytics';
import {getFrameMethod, getFrameStatus} from 'sentry/utils/replays/resourceFrame';
import useOrganization from 'sentry/utils/useOrganization';
import FluidHeight from 'sentry/views/replays/detail/layout/fluidHeight';
import getOutputType, {
Expand Down Expand Up @@ -34,8 +35,8 @@ export default function NetworkDetailsContent(props: Props) {
is_sdk_setup: isSetup,
organization,
output,
resource_method: item.data.method,
resource_status: item.data.statusCode,
resource_method: getFrameMethod(item),
resource_status: String(getFrameStatus(item)),
resource_type: item.op,
tab: visibleTab,
});
Expand Down
24 changes: 13 additions & 11 deletions static/app/views/replays/detail/network/details/getOutputType.tsx
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
import {isRequestFrame} from 'sentry/utils/replays/resourceFrame';
import type {SectionProps} from 'sentry/views/replays/detail/network/details/sections';
import type {TabKey} from 'sentry/views/replays/detail/network/details/tabs';

Expand All @@ -16,41 +17,42 @@ type Args = {
};

export default function getOutputType({isSetup, item, visibleTab}: Args): Output {
const isSupportedOp = ['resource.fetch', 'resource.xhr'].includes(item.op);
if (!isSupportedOp) {
if (!isRequestFrame(item)) {
return Output.UNSUPPORTED;
}

if (!isSetup) {
return Output.SETUP;
}

const request = item.data?.request ?? {};
const response = item.data?.response ?? {};
const request = item.data.request;
const response = item.data.response;

const hasHeaders =
Object.keys(request.headers || {}).length ||
Object.keys(response.headers || {}).length;
Object.keys(request?.headers ?? {}).length ||
Object.keys(response?.headers ?? {}).length;
if (hasHeaders && visibleTab === 'details') {
return Output.DATA;
}

const hasBody = request.body || response.body;
const hasBody = request?.body || response?.body;
if (hasBody && ['request', 'response'].includes(visibleTab)) {
return Output.DATA;
}

const reqWarnings = request._meta?.warnings ?? ['URL_SKIPPED'];
const respWarnings = response._meta?.warnings ?? ['URL_SKIPPED'];
const reqWarnings = request?._meta?.warnings ?? ['URL_SKIPPED'];
const respWarnings = response?._meta?.warnings ?? ['URL_SKIPPED'];
const isReqUrlSkipped = reqWarnings?.includes('URL_SKIPPED');
const isRespUrlSkipped = respWarnings?.includes('URL_SKIPPED');
if (isReqUrlSkipped || isRespUrlSkipped) {
return Output.URL_SKIPPED;
}

if (['request', 'response'].includes(visibleTab)) {
const isReqBodySkipped = reqWarnings?.includes('BODY_SKIPPED');
const isRespBodySkipped = respWarnings?.includes('BODY_SKIPPED');
// @ts-expect-error: is BODY_SKIPPED really emitted from the SDK?
const isReqBodySkipped = reqWarnings.includes('BODY_SKIPPED');
// @ts-expect-error: is BODY_SKIPPED really emitted from the SDK?
const isRespBodySkipped = respWarnings.includes('BODY_SKIPPED');
if (isReqBodySkipped || isRespBodySkipped) {
return Output.BODY_SKIPPED;
}
Expand Down
4 changes: 2 additions & 2 deletions static/app/views/replays/detail/network/details/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -6,18 +6,18 @@ import Stacked from 'sentry/components/replays/breadcrumbs/stacked';
import {IconClose} from 'sentry/icons';
import {t} from 'sentry/locale';
import {space} from 'sentry/styles/space';
import type {SpanFrame} from 'sentry/utils/replays/types';
import {useResizableDrawer} from 'sentry/utils/useResizableDrawer';
import useUrlParams from 'sentry/utils/useUrlParams';
import SplitDivider from 'sentry/views/replays/detail/layout/splitDivider';
import NetworkDetailsContent from 'sentry/views/replays/detail/network/details/content';
import NetworkDetailsTabs, {
TabKey,
} from 'sentry/views/replays/detail/network/details/tabs';
import type {NetworkSpan} from 'sentry/views/replays/types';

type Props = {
isSetup: boolean;
item: null | NetworkSpan;
item: null | SpanFrame;
onClose: () => void;
projectId: undefined | string;
startTimestampMs: number;
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import {render, screen} from 'sentry-test/reactTestingLibrary';
import {textWithMarkupMatcher} from 'sentry-test/utils';

import hydrateSpans from 'sentry/utils/replays/hydrateSpans';
import useProjectSdkNeedsUpdate from 'sentry/utils/useProjectSdkNeedsUpdate';
import {Output} from 'sentry/views/replays/detail/network/details/getOutputType';

Expand All @@ -12,10 +13,14 @@ const mockUseProjectSdkNeedsUpdate = useProjectSdkNeedsUpdate as jest.MockedFunc

import {Setup} from 'sentry/views/replays/detail/network/details/onboarding';

const MOCK_ITEM = TestStubs.ReplaySpanPayload({
op: 'resource.fetch',
description: '/api/0/issues/1234',
});
const [MOCK_ITEM] = hydrateSpans(TestStubs.ReplayRecord(), [
TestStubs.Replay.RequestFrame({
op: 'resource.fetch',
startTimestamp: new Date(),
endTimestamp: new Date(),
description: '/api/0/issues/1234',
}),
]);

describe('Setup', () => {
mockUseProjectSdkNeedsUpdate.mockReturnValue({isFetching: false, needsUpdate: false});
Expand Down
Loading

0 comments on commit f23055d

Please sign in to comment.