Skip to content

Commit 950918b

Browse files
scttcperandrewshie-sentry
authored andcommitted
feat(issues): Support performance issue trace previews (#87466)
1 parent 2809859 commit 950918b

File tree

4 files changed

+94
-7
lines changed

4 files changed

+94
-7
lines changed

static/app/views/performance/newTraceDetails/issuesTraceWaterfall.tsx

Lines changed: 43 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@ import {
1010
import styled from '@emotion/styled';
1111
import * as Sentry from '@sentry/react';
1212

13+
import {getProblemSpansForSpanTree} from 'sentry/components/events/interfaces/performance/utils';
1314
import type {Event} from 'sentry/types/event';
1415
import type {Project} from 'sentry/types/project';
1516
import {trackAnalytics} from 'sentry/utils/analytics';
@@ -64,6 +65,22 @@ export function IssuesTraceWaterfall(props: IssuesTraceWaterfallProps) {
6465

6566
const traceView = useMemo(() => new TraceViewModel(), []);
6667
const traceScheduler = useMemo(() => new TraceScheduler(), []);
68+
const problemSpans = useMemo((): ReturnType<typeof getProblemSpansForSpanTree> => {
69+
if (props.event.type === 'transaction') {
70+
const result = getProblemSpansForSpanTree(props.event);
71+
if (result.affectedSpanIds.length > 4) {
72+
// Too many spans to focus on, instead let them click into the preview
73+
// n+1 will have hundreds of affected spans
74+
return {
75+
affectedSpanIds: result.affectedSpanIds.slice(0, 4),
76+
focusedSpanIds: result.focusedSpanIds.slice(0, 4),
77+
};
78+
}
79+
return result;
80+
}
81+
82+
return {affectedSpanIds: [], focusedSpanIds: []};
83+
}, [props.event]);
6784

6885
const projectsRef = useRef<Project[]>(projects);
6986
projectsRef.current = projects;
@@ -235,7 +252,28 @@ export function IssuesTraceWaterfall(props: IssuesTraceWaterfallProps) {
235252
}
236253
}
237254

238-
props.tree.collapseList(preserveNodes);
255+
start = 0;
256+
// Preserve affectedSpanIds
257+
while (start < props.tree.list.length) {
258+
const currentNode = props.tree.list[start]!;
259+
if (
260+
currentNode.value &&
261+
'span_id' in currentNode.value &&
262+
// Not already in the preserveNodes array
263+
!preserveNodes.some(n => n === currentNode) &&
264+
problemSpans.affectedSpanIds.includes(currentNode.value.span_id)
265+
) {
266+
preserveNodes.push(currentNode);
267+
}
268+
start++;
269+
}
270+
271+
// Performance issues are tighter to focus on the suspect spans (of which there may be many)
272+
const numSurroundingNodes =
273+
props.event.type === 'transaction'
274+
? PERFORMANCE_ISSUE_SURROUNDING_NODES
275+
: TRACE_PREVIEW_SURROUNDING_NODES;
276+
props.tree.collapseList(preserveNodes, numSurroundingNodes);
239277
}
240278

241279
if (index === -1 || !node) {
@@ -283,6 +321,7 @@ export function IssuesTraceWaterfall(props: IssuesTraceWaterfallProps) {
283321
props.event,
284322
isLoadingSubscriptionDetails,
285323
hasExceededPerformanceUsageLimit,
324+
problemSpans.affectedSpanIds,
286325
]);
287326

288327
useTraceTimelineChangeSync({
@@ -368,8 +407,10 @@ const IssuesPointerDisabled = styled('div')`
368407
const ROW_HEIGHT = 24;
369408
const MIN_ROW_COUNT = 1;
370409
const HEADER_HEIGHT = 38;
371-
const MAX_HEIGHT = 12 * ROW_HEIGHT + HEADER_HEIGHT;
410+
const MAX_HEIGHT = 24 * ROW_HEIGHT + HEADER_HEIGHT;
372411
const MAX_ROW_COUNT = Math.floor(MAX_HEIGHT / ROW_HEIGHT);
412+
const PERFORMANCE_ISSUE_SURROUNDING_NODES = 2;
413+
const TRACE_PREVIEW_SURROUNDING_NODES = 3;
373414

374415
const IssuesTraceGrid = styled(TraceGrid)<{
375416
layout: 'drawer bottom' | 'drawer left' | 'drawer right';

static/app/views/performance/newTraceDetails/traceModels/__snapshots__/issuesTraceTree.spec.tsx.snap

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -51,3 +51,24 @@ trace root
5151
transaction 4 - transaction.op
5252
"
5353
`;
54+
55+
exports[`IssuesTraceTree respects numSurroundingNodes parameter: custom surrounding nodes (2) 1`] = `
56+
"
57+
collapsed
58+
transaction 2 - transaction.op
59+
transaction 3 - transaction.op
60+
transaction 4 - transaction.op
61+
collapsed
62+
"
63+
`;
64+
65+
exports[`IssuesTraceTree respects numSurroundingNodes parameter: default surrounding nodes (3) 1`] = `
66+
"
67+
collapsed
68+
transaction 1 - transaction.op
69+
transaction 2 - transaction.op
70+
transaction 3 - transaction.op
71+
transaction 4 - transaction.op
72+
transaction 5 - transaction.op
73+
"
74+
`;

static/app/views/performance/newTraceDetails/traceModels/issuesTraceTree.spec.tsx

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -107,6 +107,26 @@ describe('IssuesTraceTree', () => {
107107
expect(tree.build().collapseList(errors).serialize()).toMatchSnapshot();
108108
});
109109

110+
it('respects numSurroundingNodes parameter', () => {
111+
const tree = IssuesTraceTree.FromTrace(traceWithErrorInMiddle, {
112+
meta: null,
113+
replay: null,
114+
});
115+
116+
const issues = IssuesTraceTree.FindAll(tree.root, hasErrors);
117+
118+
// Test with default value (3)
119+
const defaultCollapsed = tree.build().collapseList(issues).serialize();
120+
121+
// Test with custom value (2)
122+
const customCollapsed = tree.build().collapseList(issues, 2).serialize();
123+
124+
expect(defaultCollapsed).toMatchSnapshot('default surrounding nodes (3)');
125+
expect(customCollapsed).toMatchSnapshot('custom surrounding nodes (2)');
126+
127+
expect(defaultCollapsed).not.toEqual(customCollapsed);
128+
});
129+
110130
describe('FromSpans', () => {
111131
const traceWithSpans = makeTrace({
112132
transactions: [

static/app/views/performance/newTraceDetails/traceModels/issuesTraceTree.tsx

Lines changed: 10 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -92,7 +92,12 @@ export class IssuesTraceTree extends TraceTree {
9292
return Promise.resolve();
9393
}
9494

95-
collapseList(preserveLeafNodes: TraceTreeNode[]) {
95+
/**
96+
* Collapse the list of nodes to only include the preserveLeafNodes and the surrounding nodes.
97+
* @param preserveLeafNodes - The nodes to preserve.
98+
* @param numSurroundingNodes - The number of surrounding nodes to preserve.
99+
*/
100+
collapseList(preserveLeafNodes: TraceTreeNode[], numSurroundingNodes = 3) {
96101
const preserveNodes = new Set(preserveLeafNodes);
97102

98103
for (const node of preserveLeafNodes) {
@@ -108,18 +113,18 @@ export class IssuesTraceTree extends TraceTree {
108113
continue;
109114
}
110115

111-
// Preserve the previous 2 nodes
116+
// Preserve the previous n nodes
112117
let i = Math.max(index - 1, 0);
113-
while (i > index - 3) {
118+
while (i > index - numSurroundingNodes) {
114119
if (this.list[i]) {
115120
preserveNodes.add(this.list[i]!);
116121
}
117122
i--;
118123
}
119124

120-
// Preserve the next 2 nodes
125+
// Preserve the next n nodes
121126
let j = Math.min(index + 1, this.list.length - 1);
122-
while (j < index + 3) {
127+
while (j < index + numSurroundingNodes) {
123128
if (this.list[j]) {
124129
preserveNodes.add(this.list[j]!);
125130
}

0 commit comments

Comments
 (0)