Skip to content

Commit 84501f6

Browse files
authored
feat(trace): autogroup links (#68948)
The span-id, txn-id links we generate are sometimes not representative of the final tree because of features like autogrouping. This means that a span-id, txn-id link should really be span-id, ag-id, txn-id. When this happens, perform a fallback scan to try and find the node in the hidden part of the tree and expand it.
1 parent 530d898 commit 84501f6

File tree

3 files changed

+101
-21
lines changed

3 files changed

+101
-21
lines changed

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,7 @@ export function isSpanNode(
2525
export function isTransactionNode(
2626
node: TraceTreeNode<TraceTree.NodeValue>
2727
): node is TraceTreeNode<TraceTree.Transaction> {
28-
return !!(node.value && 'transaction' in node.value);
28+
return !!(node.value && 'transaction' in node.value) && !isAutogroupedNode(node);
2929
}
3030

3131
export function isParentAutogroupedNode(

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

Lines changed: 51 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -773,7 +773,15 @@ export class TraceTree {
773773
throw new Error('Parent node is missing, this should be unreachable code');
774774
}
775775

776+
const index = node.parent.children.indexOf(node);
777+
node.parent.children[index] = autoGroupedNode;
778+
779+
autoGroupedNode.head.parent = autoGroupedNode;
776780
autoGroupedNode.groupCount = groupMatchCount + 1;
781+
autoGroupedNode.space = [
782+
start * autoGroupedNode.multiplier,
783+
(end - start) * autoGroupedNode.multiplier,
784+
];
777785

778786
for (const error of errors) {
779787
autoGroupedNode.errors.add(error);
@@ -783,18 +791,10 @@ export class TraceTree {
783791
autoGroupedNode.performance_issues.add(performanceIssue);
784792
}
785793

786-
autoGroupedNode.space = [
787-
start * autoGroupedNode.multiplier,
788-
(end - start) * autoGroupedNode.multiplier,
789-
];
790-
791794
for (const c of tail.children) {
792795
c.parent = autoGroupedNode;
793796
queue.push(c);
794797
}
795-
796-
const index = node.parent.children.indexOf(node);
797-
node.parent.children[index] = autoGroupedNode;
798798
}
799799
}
800800

@@ -1144,7 +1144,34 @@ export class TraceTree {
11441144

11451145
// We are at the last path segment (the node that the user clicked on)
11461146
// and we should scroll the view to this node.
1147-
const index = tree.list.findIndex(node => node === current);
1147+
let index = current ? tree.list.findIndex(node => node === current) : -1;
1148+
1149+
// We have found the node, yet it is somehow not in the visible tree.
1150+
// This means that the path we were given did not match the current tree.
1151+
// This sometimes happens when we receive external links like span-x, txn-y
1152+
// however the resulting tree looks like span-x, autogroup, txn-y. In this case,
1153+
// we should expand the autogroup node and try to find the node again.
1154+
if (current && index === -1) {
1155+
let parent_node = current.parent;
1156+
while (parent_node) {
1157+
// Transactions break autogrouping chains, so we can stop here
1158+
if (isTransactionNode(parent_node)) {
1159+
break;
1160+
}
1161+
if (isAutogroupedNode(parent_node)) {
1162+
tree.expand(parent_node, true);
1163+
index = current ? tree.list.findIndex(node => node === current) : -1;
1164+
// This is very wasteful as it performs O(n^2) search each time we expand a node...
1165+
// In most cases though, we should be operating on a tree with sub 10k elements and hopefully
1166+
// a low autogrouped node count.
1167+
if (index !== -1) {
1168+
break;
1169+
}
1170+
}
1171+
parent_node = parent_node.parent;
1172+
}
1173+
}
1174+
11481175
if (index === -1) {
11491176
throw new Error(`Couldn't find node in list ${scrollQueue.join(',')}`);
11501177
}
@@ -1730,10 +1757,16 @@ export class TraceTreeNode<T extends TraceTree.NodeValue = TraceTree.NodeValue>
17301757
while (queue.length > 0) {
17311758
const next = queue.pop()!;
17321759

1733-
if (predicate(next)) return next;
1760+
if (predicate(next)) {
1761+
return next;
1762+
}
17341763

1735-
for (const child of next.children) {
1736-
queue.push(child);
1764+
if (isParentAutogroupedNode(next)) {
1765+
queue.push(next.head);
1766+
} else {
1767+
for (const child of next.children) {
1768+
queue.push(child);
1769+
}
17371770
}
17381771
}
17391772

@@ -1859,12 +1892,6 @@ export class NoDataNode extends TraceTreeNode<null> {
18591892

18601893
// Generates a ID of the tree node based on its type
18611894
function nodeToId(n: TraceTreeNode<TraceTree.NodeValue>): TraceTree.NodePath {
1862-
if (isTransactionNode(n)) {
1863-
return `txn-${n.value.event_id}`;
1864-
}
1865-
if (isSpanNode(n)) {
1866-
return `span-${n.value.span_id}`;
1867-
}
18681895
if (isAutogroupedNode(n)) {
18691896
if (isParentAutogroupedNode(n)) {
18701897
return `ag-${n.head.value.span_id}`;
@@ -1876,6 +1903,12 @@ function nodeToId(n: TraceTreeNode<TraceTree.NodeValue>): TraceTree.NodePath {
18761903
}
18771904
}
18781905
}
1906+
if (isTransactionNode(n)) {
1907+
return `txn-${n.value.event_id}`;
1908+
}
1909+
if (isSpanNode(n)) {
1910+
return `span-${n.value.span_id}`;
1911+
}
18791912
if (isTraceNode(n)) {
18801913
return `trace-root`;
18811914
}
@@ -2082,7 +2115,6 @@ function findInTreeFromSegment(
20822115
if (type === 'txn' && isTransactionNode(node)) {
20832116
return node.value.event_id === id;
20842117
}
2085-
20862118
if (type === 'span' && isSpanNode(node)) {
20872119
return node.value.span_id === id;
20882120
}

static/app/views/performance/newTraceDetails/traceRenderers/virtualizedViewManager.spec.tsx

Lines changed: 49 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -299,7 +299,7 @@ describe('VirtualizedViewManger', () => {
299299
});
300300
});
301301

302-
describe('scrollToPath', () => {
302+
describe('expandToPath', () => {
303303
const organization = OrganizationFixture();
304304
const api = new MockApiClient();
305305

@@ -692,5 +692,53 @@ describe('VirtualizedViewManger', () => {
692692

693693
expect(result?.node).toBe(tree.list[2]);
694694
});
695+
696+
describe('error handling', () => {
697+
it('scrolls to child span of sibling autogrouped node when path is missing autogrouped node', async () => {
698+
manager.list = makeList();
699+
const tree = makeSingleTransactionTree();
700+
701+
MockApiClient.addMockResponse({
702+
url: EVENT_REQUEST_URL,
703+
method: 'GET',
704+
body: makeEvent({}, makeSiblingAutogroupedSpans()),
705+
});
706+
707+
const result = await TraceTree.ExpandToPath(
708+
tree,
709+
['span-middle_span', 'txn-event_id'],
710+
() => void 0,
711+
{
712+
api: api,
713+
organization,
714+
}
715+
);
716+
717+
expect(result).toBeTruthy();
718+
});
719+
720+
it('scrolls to child span of parent autogrouped node when path is missing autogrouped node', async () => {
721+
manager.list = makeList();
722+
const tree = makeSingleTransactionTree();
723+
724+
MockApiClient.addMockResponse({
725+
url: EVENT_REQUEST_URL,
726+
method: 'GET',
727+
body: makeEvent({}, makeParentAutogroupSpans()),
728+
});
729+
730+
const result = await TraceTree.ExpandToPath(
731+
tree,
732+
['span-middle_span', 'txn-event_id'],
733+
() => void 0,
734+
{
735+
api: api,
736+
organization,
737+
}
738+
);
739+
740+
expect(result).toBeTruthy();
741+
});
742+
});
695743
});
696744
});

0 commit comments

Comments
 (0)