Skip to content

Commit e8e70f9

Browse files
Abdkhan14Abdullah Khan
Abdkhan14
and
Abdullah Khan
authored
feat(trace-eap-waterfall): Preserving transaction structure in eap trace waterfall (#86999)
The purpose of this PR is to mimic the transaction hierarchy behaviour from non-eap traces. Expected behaviour: - For collapsed eap spans with `is_transaction: true` we render the embedded similar eap-transactions as visible children. - When we expand, we reparent the embedded node A, under under span B where `B.event_id === A.parent_span_id`. Non-eap trace: <img width="553" alt="Screenshot 2025-03-17 at 2 16 43 PM" src="https://github.com/user-attachments/assets/331b5755-9c42-4b79-9dc7-212883534e0c" /> EAP trace: <img width="521" alt="Screenshot 2025-03-17 at 2 17 18 PM" src="https://github.com/user-attachments/assets/3f4eaf76-8f84-48ad-aed3-04b57d9a349d" /> - Upon expanding <img width="505" alt="Screenshot 2025-03-17 at 2 17 51 PM" src="https://github.com/user-attachments/assets/f5f9270c-06e4-4d97-8766-0dc3705ce543" /> Added tests --------- Co-authored-by: Abdullah Khan <[email protected]>
1 parent 3e3b11b commit e8e70f9

File tree

4 files changed

+223
-32
lines changed

4 files changed

+223
-32
lines changed

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

+10
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,16 @@ export function isSpanNode(
2323
);
2424
}
2525

26+
export function isEAPTransaction(value: TraceTree.NodeValue): value is TraceTree.EAPSpan {
27+
return !!(value && 'is_transaction' in value && value.is_transaction);
28+
}
29+
30+
export function isEAPTransactionNode(
31+
node: TraceTreeNode<TraceTree.NodeValue>
32+
): node is TraceTreeNode<TraceTree.EAPSpan> {
33+
return isEAPTransaction(node.value);
34+
}
35+
2636
export function isEAPSpanNode(
2737
node: TraceTreeNode<TraceTree.NodeValue>
2838
): node is TraceTreeNode<TraceTree.EAPSpan> {

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

+17
Original file line numberDiff line numberDiff line change
@@ -69,6 +69,23 @@ eap trace root
6969
"
7070
`;
7171

72+
exports[`TraceTree eap trace correctly renders eap-transactions collapsed state 1`] = `
73+
"
74+
eap trace root
75+
span.op - span.description (eap-transaction)
76+
span.op - span.description (eap-transaction)
77+
"
78+
`;
79+
80+
exports[`TraceTree eap trace correctly renders eap-transactions expanded state 1`] = `
81+
"
82+
eap trace root
83+
span.op - span.description (eap-transaction)
84+
span.op - span.description
85+
span.op - span.description (eap-transaction)
86+
"
87+
`;
88+
7289
exports[`TraceTree expand collapsing a parent autogroup node shows tail chain 1`] = `
7390
"
7491
trace root

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

+74
Original file line numberDiff line numberDiff line change
@@ -533,6 +533,80 @@ describe('TraceTree', () => {
533533
// eap-span-2 is a span and should be expanded
534534
expect(findEAPSpanByEventId(tree, 'eap-span-2')?.expanded).toBe(true);
535535
});
536+
537+
it('correctly renders eap-transactions collapsed state', () => {
538+
const tree = TraceTree.FromTrace(
539+
makeEAPTrace([
540+
makeEAPSpan({
541+
event_id: 'eap-span-1',
542+
start_timestamp: start,
543+
end_timestamp: start + 2,
544+
is_transaction: true,
545+
parent_span_id: undefined,
546+
children: [
547+
makeEAPSpan({
548+
event_id: 'eap-span-2',
549+
start_timestamp: start + 1,
550+
end_timestamp: start + 4,
551+
is_transaction: false,
552+
parent_span_id: 'eap-span-1',
553+
children: [
554+
makeEAPSpan({
555+
event_id: 'eap-span-3',
556+
start_timestamp: start + 2,
557+
end_timestamp: start + 3,
558+
is_transaction: true,
559+
parent_span_id: 'eap-span-2',
560+
children: [],
561+
}),
562+
],
563+
}),
564+
],
565+
}),
566+
]),
567+
traceMetadata
568+
);
569+
expect(tree.build().serialize()).toMatchSnapshot();
570+
});
571+
572+
it('correctly renders eap-transactions expanded state', () => {
573+
const tree = TraceTree.FromTrace(
574+
makeEAPTrace([
575+
makeEAPSpan({
576+
event_id: 'eap-span-1',
577+
start_timestamp: start,
578+
end_timestamp: start + 2,
579+
is_transaction: true,
580+
parent_span_id: undefined,
581+
children: [
582+
makeEAPSpan({
583+
event_id: 'eap-span-2',
584+
start_timestamp: start + 1,
585+
end_timestamp: start + 4,
586+
is_transaction: false,
587+
parent_span_id: 'eap-span-1',
588+
children: [
589+
makeEAPSpan({
590+
event_id: 'eap-span-3',
591+
start_timestamp: start + 2,
592+
end_timestamp: start + 3,
593+
is_transaction: true,
594+
parent_span_id: 'eap-span-2',
595+
children: [],
596+
}),
597+
],
598+
}),
599+
],
600+
}),
601+
]),
602+
traceMetadata
603+
);
604+
605+
const eapTxn = findEAPSpanByEventId(tree, 'eap-span-1');
606+
tree.expand(eapTxn!, true);
607+
608+
expect(tree.build().serialize()).toMatchSnapshot();
609+
});
536610
});
537611

538612
describe('events', () => {

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

+122-32
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,8 @@ import {
2828
isCollapsedNode,
2929
isEAPSpanNode,
3030
isEAPTraceNode,
31+
isEAPTransaction,
32+
isEAPTransactionNode,
3133
isJavascriptSDKEvent,
3234
isMissingInstrumentationNode,
3335
isPageloadTransactionNode,
@@ -322,7 +324,16 @@ export class TraceTree extends TraceTreeEventDispatcher {
322324
slug: value.project_slug,
323325
});
324326

325-
const node = new TraceTreeNode(parent, value, {
327+
let parentNode;
328+
if (isEAPTransaction(value)) {
329+
// For collapsed eap-transactions we still render the embedded eap-transactions as visible children.
330+
// Mimics the behavior of non-eap traces, enabling a less noisy/summarized view of the trace.
331+
parentNode = TraceTree.ParentEAPTransaction(parent) ?? parent;
332+
} else {
333+
parentNode = parent;
334+
}
335+
336+
const node = new TraceTreeNode(parentNode, value, {
326337
spans: options.meta?.transaction_child_count_map[value.event_id] ?? 0,
327338
project_slug: value && 'project_slug' in value ? value.project_slug : undefined,
328339
event_id: value && 'event_id' in value ? value.event_id : undefined,
@@ -343,7 +354,7 @@ export class TraceTree extends TraceTreeEventDispatcher {
343354
}
344355
}
345356

346-
parent.children.push(node);
357+
parentNode.children.push(node);
347358

348359
if (node.value && 'children' in node.value) {
349360
// EAP spans are not sorted by default
@@ -1027,6 +1038,16 @@ export class TraceTree extends TraceTreeEventDispatcher {
10271038
return node.tail.children;
10281039
}
10291040

1041+
if (isEAPTransaction(node.value)) {
1042+
if (!node.expanded) {
1043+
// For collapsed eap-transactions we still render the embedded eap-transactions as visible children.
1044+
// Mimics the behavior of non-eap traces, enabling a less noisy/summarized view of the trace.
1045+
return node.children.filter(child => isEAPTransaction(child.value));
1046+
}
1047+
1048+
return node.children;
1049+
}
1050+
10301051
return node.children;
10311052
}
10321053

@@ -1036,7 +1057,7 @@ export class TraceTree extends TraceTreeEventDispatcher {
10361057
const queue: Array<TraceTreeNode<TraceTree.NodeValue>> = [];
10371058
const visibleChildren: Array<TraceTreeNode<TraceTree.NodeValue>> = [];
10381059

1039-
if (root.expanded || isParentAutogroupedNode(root)) {
1060+
if (root.expanded || isParentAutogroupedNode(root) || isEAPTransaction(root.value)) {
10401061
const children = TraceTree.DirectVisibleChildren(root);
10411062

10421063
for (let i = children.length - 1; i >= 0; i--) {
@@ -1050,7 +1071,11 @@ export class TraceTree extends TraceTreeEventDispatcher {
10501071
visibleChildren.push(node);
10511072

10521073
// iterate in reverse to ensure nodes are processed in order
1053-
if (node.expanded || isParentAutogroupedNode(node)) {
1074+
if (
1075+
node.expanded ||
1076+
isParentAutogroupedNode(node) ||
1077+
isEAPTransaction(node.value)
1078+
) {
10541079
const children = TraceTree.DirectVisibleChildren(node);
10551080

10561081
for (let i = children.length - 1; i >= 0; i--) {
@@ -1284,6 +1309,11 @@ export class TraceTree extends TraceTreeEventDispatcher {
12841309
}
12851310
}
12861311
}
1312+
if (isEAPSpanNode(n)) {
1313+
if (n.value.event_id === eventId) {
1314+
return true;
1315+
}
1316+
}
12871317
if (isTraceErrorNode(n)) {
12881318
return n.value.event_id === eventId;
12891319
}
@@ -1316,21 +1346,73 @@ export class TraceTree extends TraceTreeEventDispatcher {
13161346
});
13171347
}
13181348

1319-
static ParentTransaction(
1320-
node: TraceTreeNode<TraceTree.NodeValue>
1321-
): TraceTreeNode<TraceTree.Transaction> | null {
1349+
static ParentNode<T extends TraceTree.NodeValue>(
1350+
node: TraceTreeNode<TraceTree.NodeValue> | null,
1351+
predicate: (n: TraceTreeNode<TraceTree.NodeValue>) => boolean
1352+
): TraceTreeNode<T> | null {
1353+
if (!node) {
1354+
return null;
1355+
}
1356+
13221357
let next: TraceTreeNode<TraceTree.NodeValue> | null = node.parent;
13231358

13241359
while (next) {
1325-
if (isTransactionNode(next)) {
1326-
return next;
1360+
if (predicate(next)) {
1361+
return next as TraceTreeNode<T>;
13271362
}
13281363
next = next.parent;
13291364
}
13301365

13311366
return null;
13321367
}
13331368

1369+
static ParentTransaction(
1370+
node: TraceTreeNode<TraceTree.NodeValue> | null
1371+
): TraceTreeNode<TraceTree.Transaction> | null {
1372+
return TraceTree.ParentNode<TraceTree.Transaction>(node, isTransactionNode);
1373+
}
1374+
1375+
static ParentEAPTransaction(
1376+
node: TraceTreeNode<TraceTree.NodeValue> | null
1377+
): TraceTreeNode<TraceTree.EAPSpan> | null {
1378+
return TraceTree.ParentNode<TraceTree.EAPSpan>(node, isEAPTransactionNode);
1379+
}
1380+
1381+
static ReparentEAPTransactions(
1382+
node: TraceTreeNode<TraceTree.EAPSpan>,
1383+
findNewParent: (
1384+
t: TraceTreeNode<TraceTree.EAPSpan>
1385+
) => TraceTreeNode<TraceTree.NodeValue> | null
1386+
): void {
1387+
// Find all embedded eap-transactions, excluding the node itself
1388+
const eapTransactions = TraceTree.FindAll(
1389+
node,
1390+
n => isEAPTransactionNode(n) && n !== node
1391+
);
1392+
1393+
for (const t of eapTransactions) {
1394+
if (isEAPTransactionNode(t)) {
1395+
const newParent = findNewParent(t);
1396+
1397+
// If the transaction already has the correct parent, we can continue
1398+
if (newParent === t.parent) {
1399+
continue;
1400+
}
1401+
1402+
// If we have found a new parent to reparent the transaction under,
1403+
// remove it from its current parent's children and add it to the new parent
1404+
if (newParent) {
1405+
if (t.parent) {
1406+
t.parent.children = t.parent.children.filter(c => c !== t);
1407+
}
1408+
newParent.children.push(t);
1409+
t.parent = newParent;
1410+
t.parent.children.sort(traceChronologicalSort);
1411+
}
1412+
}
1413+
}
1414+
}
1415+
13341416
expand(node: TraceTreeNode<TraceTree.NodeValue>, expanded: boolean): boolean {
13351417
// Trace root nodes are not expandable or collapsable
13361418
if (isTraceNode(node)) {
@@ -1378,7 +1460,22 @@ export class TraceTree extends TraceTreeEventDispatcher {
13781460
}
13791461

13801462
if (expanded) {
1463+
if (isEAPTransactionNode(node)) {
1464+
const index = this.list.indexOf(node);
1465+
this.list.splice(index + 1, TraceTree.VisibleChildren(node).length);
1466+
}
1467+
1468+
// Flip expanded so that we can collect visible children
13811469
node.expanded = expanded;
1470+
1471+
// When eap-transaction nodes are expanded, we need to reparent the transactions under
1472+
// the eap-spans (by their parent_span_id) that were previously hidden.
1473+
if (isEAPTransactionNode(node)) {
1474+
TraceTree.ReparentEAPTransactions(node, t =>
1475+
TraceTree.FindByID(node, t.value.parent_span_id)
1476+
);
1477+
}
1478+
13821479
// Flip expanded so that we can collect visible children
13831480
const index = this.list.indexOf(node);
13841481
this.list.splice(index + 1, 0, ...TraceTree.VisibleChildren(node));
@@ -1387,8 +1484,18 @@ export class TraceTree extends TraceTreeEventDispatcher {
13871484
this.list.splice(index + 1, TraceTree.VisibleChildren(node).length);
13881485

13891486
node.expanded = expanded;
1487+
1488+
// When eap-transaction nodes are collapsed, they still render transactions as visible children.
1489+
// Reparent the transactions from under the eap-spans in the expanded state, to under the closest eap-transaction
1490+
// in the collapsed state.
1491+
if (isEAPTransactionNode(node)) {
1492+
TraceTree.ReparentEAPTransactions(node, t =>
1493+
TraceTree.ParentEAPTransaction(t.parent)
1494+
);
1495+
}
1496+
13901497
// When transaction nodes are collapsed, they still render child transactions
1391-
if (isTransactionNode(node)) {
1498+
if (isTransactionNode(node) || isEAPTransactionNode(node)) {
13921499
this.list.splice(index + 1, 0, ...TraceTree.VisibleChildren(node));
13931500
}
13941501
}
@@ -1672,30 +1779,12 @@ export class TraceTree extends TraceTreeEventDispatcher {
16721779
return false;
16731780
}
16741781

1675-
if (isParentAutogroupedNode(n.parent)) {
1676-
if (n.parent.expanded) {
1677-
// The autogrouped
1678-
return true;
1679-
}
1680-
return n.parent.tail.children[n.parent.tail.children.length - 1] === n;
1681-
}
1682-
1683-
return n.parent.children[n.parent.children.length - 1] === n;
1782+
const visibleChildren = TraceTree.DirectVisibleChildren(n.parent);
1783+
return visibleChildren[visibleChildren.length - 1] === n;
16841784
}
16851785

16861786
static HasVisibleChildren(node: TraceTreeNode<TraceTree.NodeValue>): boolean {
1687-
if (isParentAutogroupedNode(node)) {
1688-
if (node.expanded) {
1689-
return node.head.children.length > 0;
1690-
}
1691-
return node.tail.children.length > 0;
1692-
}
1693-
1694-
if (node.expanded) {
1695-
return node.children.length > 0;
1696-
}
1697-
1698-
return false;
1787+
return TraceTree.VisibleChildren(node).length > 0;
16991788
}
17001789

17011790
/**
@@ -2003,7 +2092,8 @@ function printTraceTreeNode(
20032092
padding +
20042093
(t.value.op || 'unknown span') +
20052094
' - ' +
2006-
(t.value.description || 'unknown description')
2095+
(t.value.description || 'unknown description') +
2096+
(isEAPTransactionNode(t) ? ` (eap-transaction)` : '')
20072097
);
20082098
}
20092099
if (isTransactionNode(t)) {

0 commit comments

Comments
 (0)