Skip to content

Commit 1a93e14

Browse files
Abdkhan14Abdullah Khan
and
Abdullah Khan
authored
feat(trace-eap-waterfall): Rendering errors associated with spans (#88060)
The goal of this PR is to only render error markers for errors that ARE correlated with a span in the trace. Will put up a separate PR to render orphan errors in the waterfall. To summarize the changes, we added a new `EAPError` type and added `errors: EAPError[]` to `EAPSpan` type. Added tests <img width="1254" alt="Screenshot 2025-03-27 at 2 26 53 PM" src="https://github.com/user-attachments/assets/d4bd0d15-8f54-47dc-8d0a-fb68ef48adbf" /> --------- Co-authored-by: Abdullah Khan <[email protected]>
1 parent 6d9c2c9 commit 1a93e14

File tree

15 files changed

+160
-55
lines changed

15 files changed

+160
-55
lines changed

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

Lines changed: 8 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,9 @@ import useOrganization from 'sentry/utils/useOrganization';
1919
import useProjects from 'sentry/utils/useProjects';
2020
import {IssueTraceWaterfallOverlay} from 'sentry/views/performance/newTraceDetails/issuesTraceWaterfallOverlay';
2121
import {
22+
isEAPSpanNode,
23+
isEAPTransactionNode,
24+
isNonTransactionEAPSpanNode,
2225
isSpanNode,
2326
isTraceErrorNode,
2427
isTransactionNode,
@@ -200,8 +203,9 @@ export function IssuesTraceWaterfall(props: IssuesTraceWaterfallProps) {
200203
}
201204
}
202205
}
203-
if (isSpanNode(n)) {
204-
if (n.value.span_id === props.event.eventID) {
206+
if (isSpanNode(n) || isEAPSpanNode(n)) {
207+
const spanId = 'span_id' in n.value ? n.value.span_id : n.value.event_id;
208+
if (spanId === props.event.eventID) {
205209
return true;
206210
}
207211
for (const e of n.errors) {
@@ -223,8 +227,8 @@ export function IssuesTraceWaterfall(props: IssuesTraceWaterfallProps) {
223227
// the error may have been attributed to, otherwise we look at the transaction.
224228
const node =
225229
nodes?.find(n => isTraceErrorNode(n)) ||
226-
nodes?.find(n => isSpanNode(n)) ||
227-
nodes?.find(n => isTransactionNode(n));
230+
nodes?.find(n => isSpanNode(n) || isNonTransactionEAPSpanNode(n)) ||
231+
nodes?.find(n => isTransactionNode(n) || isEAPTransactionNode(n));
228232

229233
const index = node ? props.tree.list.indexOf(node) : -1;
230234

static/app/views/performance/newTraceDetails/traceDrawer/details/issues/issues.tsx

Lines changed: 9 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -19,11 +19,7 @@ import {t, tct, tn} from 'sentry/locale';
1919
import {space} from 'sentry/styles/space';
2020
import type {Group} from 'sentry/types/group';
2121
import type {Organization} from 'sentry/types/organization';
22-
import type {
23-
TraceError,
24-
TraceErrorOrIssue,
25-
TracePerformanceIssue,
26-
} from 'sentry/utils/performance/quickTrace/types';
22+
import type {TracePerformanceIssue} from 'sentry/utils/performance/quickTrace/types';
2723
import {useApiQuery} from 'sentry/utils/queryClient';
2824
import type RequestError from 'sentry/utils/requestError/requestError';
2925
import useOrganization from 'sentry/utils/useOrganization';
@@ -40,7 +36,7 @@ import {TraceDrawerComponents} from '../styles';
4036
import {IssueSummary} from './issueSummary';
4137

4238
type IssueProps = {
43-
issue: TraceErrorOrIssue;
39+
issue: TraceTree.TraceIssue;
4440
organization: Organization;
4541
};
4642

@@ -63,7 +59,10 @@ const issueOrderPriority: Record<keyof Theme['level'], number> = {
6359
unknown: 6,
6460
};
6561

66-
function sortIssuesByLevel(a: TraceError, b: TraceError): number {
62+
function sortIssuesByLevel(
63+
a: TraceTree.TraceErrorIssue,
64+
b: TraceTree.TraceErrorIssue
65+
): number {
6766
// If the level is not defined in the priority map, default to unknown
6867
const aPriority = issueOrderPriority[a.level] ?? issueOrderPriority.unknown;
6968
const bPriority = issueOrderPriority[b.level] ?? issueOrderPriority.unknown;
@@ -352,15 +351,15 @@ function LegacyIssue(
352351
) : null;
353352
}
354353
type IssueListProps = {
355-
issues: TraceErrorOrIssue[];
354+
issues: TraceTree.TraceIssue[];
356355
node: TraceTreeNode<TraceTree.NodeValue>;
357356
organization: Organization;
358357
};
359358

360359
export function IssueList({issues, node, organization}: IssueListProps) {
361360
const hasTraceNewUi = useHasTraceNewUi();
362361
const uniqueErrorIssues = useMemo(() => {
363-
const unique: TraceError[] = [];
362+
const unique: TraceTree.TraceErrorIssue[] = [];
364363

365364
const seenIssues: Set<number> = new Set();
366365

@@ -451,7 +450,7 @@ function IssueListHeader({
451450
errorIssues,
452451
performanceIssues,
453452
}: {
454-
errorIssues: TraceError[];
453+
errorIssues: TraceTree.TraceErrorIssue[];
455454
node: TraceTreeNode<TraceTree.NodeValue>;
456455
performanceIssues: TracePerformanceIssue[];
457456
}) {

static/app/views/performance/newTraceDetails/traceDrawer/details/span/index.tsx

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -308,8 +308,10 @@ export function SpanNodeDetails({
308308
analyticsPageSource={LogsAnalyticsPageSource.TRACE_DETAILS}
309309
>
310310
<LogsPageDataProvider>
311+
{issues.length > 0 ? (
312+
<IssueList organization={organization} issues={issues} node={node} />
313+
) : null}
311314
<LogDetails />
312-
<div>TO DO: EAP Span Details</div>
313315
</LogsPageDataProvider>
314316
</LogsPageParamsProvider>
315317
</TraceDrawerComponents.BodyContainer>

static/app/views/performance/newTraceDetails/traceDrawer/tabs/trace/generalInfo.tsx

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,6 @@ import {t, tn} from 'sentry/locale';
77
import type {EventTransaction} from 'sentry/types/event';
88
import type {Organization} from 'sentry/types/organization';
99
import getDuration from 'sentry/utils/duration/getDuration';
10-
import type {TraceErrorOrIssue} from 'sentry/utils/performance/quickTrace/types';
1110
import type {UseApiQueryResult} from 'sentry/utils/queryClient';
1211
import type RequestError from 'sentry/utils/requestError/requestError';
1312
import {useParams} from 'sentry/utils/useParams';
@@ -38,7 +37,7 @@ export function GeneralInfo(props: GeneralInfoProps) {
3837
return [];
3938
}
4039

41-
const unique: TraceErrorOrIssue[] = [];
40+
const unique: TraceTree.TraceErrorIssue[] = [];
4241
const seenIssues: Set<number> = new Set();
4342

4443
for (const issue of traceNode.errors) {
@@ -57,7 +56,7 @@ export function GeneralInfo(props: GeneralInfoProps) {
5756
return [];
5857
}
5958

60-
const unique: TraceErrorOrIssue[] = [];
59+
const unique: TraceTree.TracePerformanceIssue[] = [];
6160
const seenIssues: Set<number> = new Set();
6261

6362
for (const issue of traceNode.performance_issues) {

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

Lines changed: 22 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -39,6 +39,12 @@ export function isEAPSpanNode(
3939
return !!(node.value && 'is_transaction' in node.value);
4040
}
4141

42+
export function isNonTransactionEAPSpanNode(
43+
node: TraceTreeNode<TraceTree.NodeValue>
44+
): node is TraceTreeNode<TraceTree.EAPSpan> {
45+
return isEAPSpanNode(node) && !isEAPTransactionNode(node);
46+
}
47+
4248
export function isTransactionNode(
4349
node: TraceTreeNode<TraceTree.NodeValue>
4450
): node is TraceTreeNode<TraceTree.Transaction> {
@@ -49,6 +55,21 @@ export function isTransactionNode(
4955
);
5056
}
5157

58+
export function isEAPError(value: TraceTree.NodeValue): value is TraceTree.EAPError {
59+
return !!(
60+
value &&
61+
'event_type' in value &&
62+
value.event_type === 'error' &&
63+
!('message' in value) // a bit gross, but we won't need this soon as we remove the legacy error type
64+
);
65+
}
66+
67+
export function isEAPErrorNode(
68+
node: TraceTreeNode<TraceTree.NodeValue>
69+
): node is TraceTreeNode<TraceTree.EAPError> {
70+
return isEAPError(node.value);
71+
}
72+
5273
export function isParentAutogroupedNode(
5374
node: TraceTreeNode<TraceTree.NodeValue>
5475
): node is ParentAutogroupNode {
@@ -178,7 +199,7 @@ export function getPageloadTransactionChildCount(
178199
}
179200

180201
export function isTracePerformanceIssue(
181-
issue: TraceTree.TraceError | TraceTree.TracePerformanceIssue
202+
issue: TraceTree.TraceIssue
182203
): issue is TraceTree.TracePerformanceIssue {
183204
return 'suspect_spans' in issue;
184205
}

static/app/views/performance/newTraceDetails/traceHeader/meta.tsx

Lines changed: 3 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -8,10 +8,7 @@ import {space} from 'sentry/styles/space';
88
import type {EventTransaction} from 'sentry/types/event';
99
import type {Organization} from 'sentry/types/organization';
1010
import getDuration from 'sentry/utils/duration/getDuration';
11-
import type {
12-
TraceErrorOrIssue,
13-
TraceMeta,
14-
} from 'sentry/utils/performance/quickTrace/types';
11+
import type {TraceMeta} from 'sentry/utils/performance/quickTrace/types';
1512
import type {UseApiQueryResult} from 'sentry/utils/queryClient';
1613
import type RequestError from 'sentry/utils/requestError/requestError';
1714

@@ -65,7 +62,7 @@ export function Meta(props: MetaProps) {
6562
return [];
6663
}
6764

68-
const unique: TraceErrorOrIssue[] = [];
65+
const unique: TraceTree.TraceErrorIssue[] = [];
6966
const seenIssues: Set<number> = new Set();
7067

7168
for (const issue of traceNode.errors) {
@@ -84,7 +81,7 @@ export function Meta(props: MetaProps) {
8481
return [];
8582
}
8683

87-
const unique: TraceErrorOrIssue[] = [];
84+
const unique: TraceTree.TracePerformanceIssue[] = [];
8885
const seenIssues: Set<number> = new Set();
8986

9087
for (const issue of traceNode.performance_issues) {

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

Lines changed: 1 addition & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1,12 +1,7 @@
1-
import type {
2-
TraceError,
3-
TracePerformanceIssue,
4-
} from 'sentry/utils/performance/quickTrace/types';
5-
61
import type {TraceTree} from './traceModels/traceTree';
72

83
interface EventTypeIconProps {
9-
event: TracePerformanceIssue | TraceError | TraceTree.Profile;
4+
event: TraceTree.TraceIssue | TraceTree.Profile;
105
}
116

127
function EventTypeIcon(props: EventTypeIconProps) {

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

Lines changed: 13 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
import type {Client} from 'sentry/api';
22
import type {Organization} from 'sentry/types/organization';
33
import {
4+
isEAPSpanNode,
45
isTraceErrorNode,
56
isTransactionNode,
67
} from 'sentry/views/performance/newTraceDetails/traceGuards';
@@ -65,7 +66,7 @@ export class IssuesTraceTree extends TraceTree {
6566
if (isTraceErrorNode(n)) {
6667
return n.value.event_id === eventId;
6768
}
68-
if (isTransactionNode(n)) {
69+
if (isTransactionNode(n) || isEAPSpanNode(n)) {
6970
if (n.value.event_id === eventId) {
7071
return true;
7172
}
@@ -85,8 +86,14 @@ export class IssuesTraceTree extends TraceTree {
8586
return false;
8687
});
8788

88-
if (node && isTransactionNode(node)) {
89-
return tree.zoom(node, true, options).then(() => {});
89+
if (node) {
90+
if (isTransactionNode(node)) {
91+
return tree.zoom(node, true, options).then(() => {});
92+
}
93+
94+
if (isEAPSpanNode(node)) {
95+
tree.expand(node, true);
96+
}
9097
}
9198

9299
return Promise.resolve();
@@ -106,7 +113,9 @@ export class IssuesTraceTree extends TraceTree {
106113
const preserveNodes = new Set(preserveLeafNodes);
107114

108115
for (const node of preserveLeafNodes) {
109-
const parentTransaction = TraceTree.ParentTransaction(node);
116+
const parentTransaction = isEAPSpanNode(node)
117+
? TraceTree.ParentEAPTransaction(node)
118+
: TraceTree.ParentTransaction(node);
110119
if (parentTransaction) {
111120
preserveNodes.add(parentTransaction);
112121
}

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

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@ import type {ParentAutogroupNode} from './parentAutogroupNode';
1717
import {TraceTree} from './traceTree';
1818
import {
1919
assertTransactionNode,
20+
makeEAPError,
2021
makeEAPSpan,
2122
makeEAPTrace,
2223
makeEventTransaction,
@@ -170,6 +171,21 @@ const parentAutogroupSpansWithTailChildren = [
170171
}),
171172
];
172173

174+
const eapTraceWithErrors = makeEAPTrace([
175+
makeEAPSpan({
176+
event_id: 'eap-span-1',
177+
is_transaction: true,
178+
errors: [],
179+
children: [
180+
makeEAPSpan({
181+
event_id: 'eap-span-2',
182+
is_transaction: false,
183+
errors: [makeEAPError()],
184+
}),
185+
],
186+
}),
187+
]);
188+
173189
function findTransactionByEventId(tree: TraceTree, eventId: string) {
174190
return TraceTree.Find(
175191
tree.root,
@@ -536,6 +552,18 @@ describe('TraceTree', () => {
536552
expect(tree.build().serialize()).toMatchSnapshot();
537553
});
538554

555+
it('adds eap errors to tree nodes', () => {
556+
const tree = TraceTree.FromTrace(eapTraceWithErrors, traceMetadata);
557+
558+
expect(tree.root.children[0]!.errors.size).toBe(1);
559+
560+
const eapTransaction = findEAPSpanByEventId(tree, 'eap-span-1');
561+
const eapSpan = findEAPSpanByEventId(tree, 'eap-span-2');
562+
563+
expect(eapTransaction?.errors.size).toBe(1);
564+
expect(eapSpan?.errors.size).toBe(1);
565+
});
566+
539567
it('initializes expanded based on is_transaction property', () => {
540568
const tree = TraceTree.FromTrace(
541569
makeEAPTrace([

0 commit comments

Comments
 (0)