Skip to content

Commit de53277

Browse files
authored
fix(waterfall): Prefer specific root events in trace root lookup (#104050)
This patch fixes two issues: 1. If we look at a trace in the waterfall where the first root span is not a navigation/pageload span but e.g. a web vital span, we can't look up the trace's `previous_trace` link. Therefore, we can only show the disabled "previous trace" button in the UI, although there actually is a previous trace. 2. For EAP-based traces, it seems like we always took the first transaction event as the title of the current trace. This sometimes led to a bit sub optimal titles in my opinion. I noticed we had a more selective logic for none-EAP events, where we preferred the `pageload`, `navigation` and `ui.load` events over the "first" event. To fix both of these issues, this patch ports the none-EAP logic of selecting a trace root all root events (so also to EAP traces).
1 parent 5bcb618 commit de53277

File tree

1 file changed

+23
-17
lines changed
  • static/app/views/performance/newTraceDetails/traceApi

1 file changed

+23
-17
lines changed

static/app/views/performance/newTraceDetails/traceApi/utils.tsx

Lines changed: 23 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,6 @@ import type {OurLogsResponseItem} from 'sentry/views/explore/logs/types';
33
import type {TraceRootEventQueryResults} from 'sentry/views/performance/newTraceDetails/traceApi/useTraceRootEvent';
44
import {
55
isEAPTraceNode,
6-
isEAPTransaction,
76
isRootEvent,
87
isTraceNode,
98
isTraceSplitResult,
@@ -57,7 +56,8 @@ export const getRepresentativeTraceEvent = (
5756
return {type: 'uptime_check', event: traceChild.value};
5857
}
5958

60-
let rootEvent: TraceTree.TraceEvent | null = null;
59+
let preferredRootEvent: TraceTree.TraceEvent | null = null;
60+
let firstRootEvent: TraceTree.TraceEvent | null = null;
6161
let candidateEvent: TraceTree.TraceEvent | null = null;
6262
let firstEvent: TraceTree.TraceEvent | null = null;
6363

@@ -67,15 +67,12 @@ export const getRepresentativeTraceEvent = (
6767
: [...traceNode.value.transactions, ...traceNode.value.orphan_errors];
6868
for (const event of events) {
6969
if (isRootEvent(event)) {
70-
rootEvent = event;
71-
72-
if (!isEAP) {
73-
// For non-EAP traces, we return the first root event.
74-
break;
70+
if (!firstRootEvent) {
71+
firstRootEvent = event;
7572
}
7673

77-
if (isEAPTransaction(event)) {
78-
// If we find a root EAP transaction, we can stop looking and use it for the title.
74+
if (hasPreferredOp(event)) {
75+
preferredRootEvent = event;
7976
break;
8077
}
8178
// Otherwise we keep looking for a root eap transaction. If we don't find one, we use other roots, like standalone spans.
@@ -85,13 +82,7 @@ export const getRepresentativeTraceEvent = (
8582
// with an op that we care about, we can use it for the title. We keep looking for
8683
// a root.
8784
!candidateEvent &&
88-
CANDIDATE_TRACE_TITLE_OPS.includes(
89-
'transaction.op' in event
90-
? event['transaction.op']
91-
: 'op' in event
92-
? event.op
93-
: ''
94-
)
85+
hasPreferredOp(event)
9586
) {
9687
candidateEvent = event;
9788
continue;
@@ -103,7 +94,7 @@ export const getRepresentativeTraceEvent = (
10394
}
10495

10596
return {
106-
event: rootEvent ?? candidateEvent ?? firstEvent,
97+
event: preferredRootEvent ?? firstRootEvent ?? candidateEvent ?? firstEvent,
10798
type: 'span',
10899
};
109100
};
@@ -119,3 +110,18 @@ export const isValidEventUUID = (id: string): boolean => {
119110
/^[0-9a-f]{8}[0-9a-f]{4}[1-5][0-9a-f]{3}[89ab][0-9a-f]{3}[0-9a-f]{12}$/i;
120111
return uuidRegex.test(id);
121112
};
113+
114+
/**
115+
* Prefer "special" root events over generic root events when generating a title
116+
* for the waterfall view. Picking these improves contextual navigation for linked
117+
* traces, resulting in more meaningful waterfall titles.
118+
*/
119+
function hasPreferredOp(event: TraceTree.TraceEvent): boolean {
120+
const op =
121+
'op' in event
122+
? event.op
123+
: 'transaction.op' in event
124+
? event['transaction.op']
125+
: undefined;
126+
return !!op && CANDIDATE_TRACE_TITLE_OPS.includes(op);
127+
}

0 commit comments

Comments
 (0)