Skip to content

Commit a2aad5c

Browse files
authored
Merge branch 'main' into antonis/fix/ttid-fallback-nsarray
2 parents c6bc808 + 208bf09 commit a2aad5c

4 files changed

Lines changed: 44 additions & 1 deletion

File tree

CHANGELOG.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@
1515
### Fixes
1616

1717
- Fix iOS time-to-initial-display fallback spans reporting a spurious `deadline_exceeded` status and inflated duration ([#6438](https://github.com/getsentry/sentry-react-native/pull/6438))
18+
- Fix orphaned TTID/TTFD spans in the trace view ([#6437](https://github.com/getsentry/sentry-react-native/pull/6437))
1819

1920
## 8.18.0
2021

packages/core/src/js/tracing/integrations/timeToDisplayIntegration.ts

Lines changed: 26 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -40,6 +40,14 @@ export const timeToDisplayIntegration = (): Integration => {
4040
return event;
4141
}
4242

43+
// `trace_id` is a required field of a transaction's trace context and is
44+
// always present here alongside `rootSpanId`. Read it (instead of early
45+
// returning) so the TTID/TTFD spans inherit the transaction's trace and
46+
// nest correctly, and so we never skip the coordinator cleanup at the end
47+
// of `processEvent`. If it were ever absent, `createSpanJSON` keeps its
48+
// existing behavior of generating one.
49+
const traceId = event.contexts?.trace?.trace_id;
50+
4351
const transactionStartTimestampSeconds = event.start_timestamp;
4452
if (!transactionStartTimestampSeconds) {
4553
// This should never happen
@@ -53,10 +61,17 @@ export const timeToDisplayIntegration = (): Integration => {
5361
const ttidSpan = await addTimeToInitialDisplay({
5462
event,
5563
rootSpanId,
64+
traceId,
5665
transactionStartTimestampSeconds,
5766
enableTimeToInitialDisplayForPreloadedRoutes,
5867
});
59-
const ttfdSpan = await addTimeToFullDisplay({ event, rootSpanId, transactionStartTimestampSeconds, ttidSpan });
68+
const ttfdSpan = await addTimeToFullDisplay({
69+
event,
70+
rootSpanId,
71+
traceId,
72+
transactionStartTimestampSeconds,
73+
ttidSpan,
74+
});
6075

6176
const ttidDurationMs =
6277
ttidSpan?.start_timestamp && ttidSpan?.timestamp
@@ -109,11 +124,13 @@ export const timeToDisplayIntegration = (): Integration => {
109124
async function addTimeToInitialDisplay({
110125
event,
111126
rootSpanId,
127+
traceId,
112128
transactionStartTimestampSeconds,
113129
enableTimeToInitialDisplayForPreloadedRoutes,
114130
}: {
115131
event: Event;
116132
rootSpanId: string;
133+
traceId: string | undefined;
117134
transactionStartTimestampSeconds: number;
118135
enableTimeToInitialDisplayForPreloadedRoutes: boolean;
119136
}): Promise<SpanJSON | undefined> {
@@ -133,6 +150,7 @@ async function addTimeToInitialDisplay({
133150
return addAutomaticTimeToInitialDisplay({
134151
event,
135152
rootSpanId,
153+
traceId,
136154
transactionStartTimestampSeconds,
137155
enableTimeToInitialDisplayForPreloadedRoutes,
138156
});
@@ -156,6 +174,7 @@ async function addTimeToInitialDisplay({
156174
timestamp: ttidEndTimestampSeconds,
157175
origin: SPAN_ORIGIN_MANUAL_UI_TIME_TO_DISPLAY,
158176
parent_span_id: rootSpanId,
177+
trace_id: traceId,
159178
data: {
160179
[SPAN_THREAD_NAME]: SPAN_THREAD_NAME_JAVASCRIPT,
161180
},
@@ -168,11 +187,13 @@ async function addTimeToInitialDisplay({
168187
async function addAutomaticTimeToInitialDisplay({
169188
event,
170189
rootSpanId,
190+
traceId,
171191
transactionStartTimestampSeconds,
172192
enableTimeToInitialDisplayForPreloadedRoutes,
173193
}: {
174194
event: Event;
175195
rootSpanId: string;
196+
traceId: string | undefined;
176197
transactionStartTimestampSeconds: number;
177198
enableTimeToInitialDisplayForPreloadedRoutes: boolean;
178199
}): Promise<SpanJSON | undefined> {
@@ -205,6 +226,7 @@ async function addAutomaticTimeToInitialDisplay({
205226
timestamp: ttidTimestampSeconds,
206227
origin: SPAN_ORIGIN_AUTO_UI_TIME_TO_DISPLAY,
207228
parent_span_id: rootSpanId,
229+
trace_id: traceId,
208230
data: {
209231
[SPAN_THREAD_NAME]: SPAN_THREAD_NAME_JAVASCRIPT,
210232
},
@@ -217,11 +239,13 @@ async function addAutomaticTimeToInitialDisplay({
217239
async function addTimeToFullDisplay({
218240
event,
219241
rootSpanId,
242+
traceId,
220243
transactionStartTimestampSeconds,
221244
ttidSpan,
222245
}: {
223246
event: Event;
224247
rootSpanId: string;
248+
traceId: string | undefined;
225249
transactionStartTimestampSeconds: number;
226250
ttidSpan: SpanJSON | undefined;
227251
}): Promise<SpanJSON | undefined> {
@@ -271,6 +295,7 @@ async function addTimeToFullDisplay({
271295
timestamp: ttfdAdjustedEndTimestampSeconds,
272296
origin: SPAN_ORIGIN_MANUAL_UI_TIME_TO_DISPLAY,
273297
parent_span_id: rootSpanId,
298+
trace_id: traceId,
274299
data: {
275300
[SPAN_THREAD_NAME]: SPAN_THREAD_NAME_JAVASCRIPT,
276301
},

packages/core/test/tracing/reactnavigation.ttid.test.tsx

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -98,6 +98,20 @@ describe('React Navigation - TTID', () => {
9898
);
9999
});
100100

101+
test('automatic ttid span inherits trace_id and parent_span_id from the transaction', () => {
102+
jest.runOnlyPendingTimers(); // Flush app start transaction
103+
104+
mockedNavigation.navigateToNewScreen();
105+
mockAutomaticTimeToDisplay();
106+
jest.runOnlyPendingTimers(); // Flush ttid transaction
107+
108+
const transaction = getLastTransaction(transportSendMock);
109+
const ttidSpan = transaction.spans?.find(s => s.op === 'ui.load.initial_display');
110+
expect(ttidSpan).toBeDefined();
111+
expect(ttidSpan?.trace_id).toBe(transaction.contexts?.trace?.trace_id);
112+
expect(ttidSpan?.parent_span_id).toBe(transaction.contexts?.trace?.span_id);
113+
});
114+
101115
test('should end ttid with measurements even when active span was removed from the scope', () => {
102116
jest.runOnlyPendingTimers(); // Flush app start transaction
103117

packages/core/test/tracing/timetodisplay.test.tsx

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -438,6 +438,7 @@ function expectFinishedInitialDisplaySpan(event: Event) {
438438
description: 'Time To Initial Display',
439439
op: 'ui.load.initial_display',
440440
parent_span_id: event.contexts.trace.span_id,
441+
trace_id: event.contexts.trace.trace_id,
441442
start_timestamp: event.start_timestamp,
442443
status: 'ok',
443444
timestamp: expect.any(Number),
@@ -456,6 +457,7 @@ function expectFinishedFullDisplaySpan(event: Event) {
456457
description: 'Time To Full Display',
457458
op: 'ui.load.full_display',
458459
parent_span_id: event.contexts.trace.span_id,
460+
trace_id: event.contexts.trace.trace_id,
459461
start_timestamp: event.start_timestamp,
460462
status: 'ok',
461463
timestamp: expect.any(Number),
@@ -474,6 +476,7 @@ function expectDeadlineExceededFullDisplaySpan(event: Event) {
474476
description: 'Time To Full Display',
475477
op: 'ui.load.full_display',
476478
parent_span_id: event.contexts.trace.span_id,
479+
trace_id: event.contexts.trace.trace_id,
477480
start_timestamp: event.start_timestamp,
478481
status: 'deadline_exceeded',
479482
timestamp: expect.any(Number),

0 commit comments

Comments
 (0)