Skip to content

Commit 5414b4b

Browse files
antonislucas-zimerman
antonis
and
lucas-zimerman
authored
fix: Add frame data to all spans created through API (#5420)
* fix(ttid/ttfd): Add frame data to ttid/ttfd spans * Update changelog * Fix lint issues * Prevent memory leak * Fix test feedback * Seer review feedback * fix potential race condition * fix: Add frame data to all spans created through API * Add explanatory comments * Imrove log readability Co-authored-by: LucasZF <lucas-zimerman1@hotmail.com> * Update log for redability Co-authored-by: LucasZF <lucas-zimerman1@hotmail.com> --------- Co-authored-by: LucasZF <lucas-zimerman1@hotmail.com>
1 parent a699d13 commit 5414b4b

File tree

3 files changed

+191
-29
lines changed

3 files changed

+191
-29
lines changed

‎CHANGELOG.md‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@
1313
- Adds Metrics Beta ([#5402](https://github.com/getsentry/sentry-react-native/pull/5402))
1414
- Improves Expo Router integration to optionally include full paths to components instead of just component names ([#5414](https://github.com/getsentry/sentry-react-native/pull/5414))
1515
- Report slow and frozen frames as TTID/TTFD span data ([#5419](https://github.com/getsentry/sentry-react-native/pull/5419))
16+
- Report slow and frozen frames on spans created through the API ([#5420](https://github.com/getsentry/sentry-react-native/issues/5420))
1617

1718
### Fixes
1819

‎packages/core/src/js/tracing/integrations/nativeFrames.ts‎

Lines changed: 81 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -54,7 +54,8 @@ export const createNativeFramesIntegrations = (enable: boolean | undefined): Int
5454
};
5555

5656
/**
57-
* Instrumentation to add native slow/frozen frames measurements onto transactions.
57+
* Instrumentation to add native slow/frozen frames measurements onto transactions
58+
* and frame data (frames.total, frames.slow, frames.frozen) onto all spans.
5859
*/
5960
export const nativeFramesIntegration = (): Integration => {
6061
/** The native frames at the finish time of the most recent span. */
@@ -81,13 +82,11 @@ export const nativeFramesIntegration = (): Integration => {
8182
client.on('spanEnd', fetchEndFramesForSpan);
8283
};
8384

84-
const fetchStartFramesForSpan = (rootSpan: Span): void => {
85-
if (!isRootSpan(rootSpan)) {
86-
return;
87-
}
85+
const fetchStartFramesForSpan = (span: Span): void => {
86+
const spanId = span.spanContext().spanId;
87+
const spanType = isRootSpan(span) ? 'root' : 'child';
88+
debug.log(`[${INTEGRATION_NAME}] Fetching frames for ${spanType} span start (${spanId}).`);
8889

89-
const spanId = rootSpan.spanContext().spanId;
90-
debug.log(`[${INTEGRATION_NAME}] Fetching frames for root span start (${spanId}).`);
9190
_spanToNativeFramesAtStartMap.set(
9291
spanId,
9392
new Promise<NativeFramesResponse | null>(resolve => {
@@ -101,17 +100,26 @@ export const nativeFramesIntegration = (): Integration => {
101100
);
102101
};
103102

104-
const fetchEndFramesForSpan = (span: Span): void => {
103+
/**
104+
* Fetches end frames for a span and attaches frame data as span attributes.
105+
*
106+
* Note: This makes one native bridge call per span end. While this creates O(n) calls
107+
* for n spans, it's necessary for accuracy. Frame counts are cumulative and continuously
108+
* incrementing, so each span needs the exact frame count at its end time. Caching would
109+
* produce incorrect deltas. The native bridge calls are async and non-blocking.
110+
*/
111+
const fetchEndFramesForSpan = async (span: Span): Promise<void> => {
105112
const timestamp = timestampInSeconds();
106113
const spanId = span.spanContext().spanId;
114+
const hasStartFrames = _spanToNativeFramesAtStartMap.has(spanId);
107115

108-
if (isRootSpan(span)) {
109-
const hasStartFrames = _spanToNativeFramesAtStartMap.has(spanId);
110-
if (!hasStartFrames) {
111-
// We don't have start frames, won't be able to calculate the difference.
112-
return;
113-
}
116+
if (!hasStartFrames) {
117+
// We don't have start frames, won't be able to calculate the difference.
118+
return;
119+
}
114120

121+
if (isRootSpan(span)) {
122+
// Root spans: Store end frames for transaction measurements (backward compatibility)
115123
debug.log(`[${INTEGRATION_NAME}] Fetch frames for root span end (${spanId}).`);
116124
_spanToNativeFramesAtEndMap.set(
117125
spanId,
@@ -129,17 +137,45 @@ export const nativeFramesIntegration = (): Integration => {
129137
});
130138
}),
131139
);
132-
return undefined;
133-
} else {
134-
debug.log(`[${INTEGRATION_NAME}] Fetch frames for child span end (${spanId}).`);
135-
fetchNativeFrames()
136-
.then(frames => {
137-
_lastChildSpanEndFrames = {
138-
timestamp,
139-
nativeFrames: frames,
140-
};
141-
})
142-
.catch(error => debug.log(`[${INTEGRATION_NAME}] Error while fetching native frames.`, error));
140+
}
141+
142+
// All spans (root and child): Attach frame data as span attributes
143+
try {
144+
const startFrames = await _spanToNativeFramesAtStartMap.get(spanId);
145+
if (!startFrames) {
146+
debug.log(`[${INTEGRATION_NAME}] No start frames found for span ${spanId}, skipping frame data.`);
147+
return;
148+
}
149+
150+
// NOTE: For root spans, this is the second call to fetchNativeFrames() for the same span.
151+
// The calls are very close together (microseconds apart), so inconsistency is minimal.
152+
// Future optimization: reuse the first call's promise to avoid redundant native bridge call.
153+
const endFrames = await fetchNativeFrames();
154+
155+
// Calculate deltas
156+
const totalFrames = endFrames.totalFrames - startFrames.totalFrames;
157+
const slowFrames = endFrames.slowFrames - startFrames.slowFrames;
158+
const frozenFrames = endFrames.frozenFrames - startFrames.frozenFrames;
159+
160+
// Only attach if we have meaningful data
161+
if (totalFrames > 0 || slowFrames > 0 || frozenFrames > 0) {
162+
span.setAttribute('frames.total', totalFrames);
163+
span.setAttribute('frames.slow', slowFrames);
164+
span.setAttribute('frames.frozen', frozenFrames);
165+
debug.log(
166+
`[${INTEGRATION_NAME}] Attached frame data to span ${spanId}: total=${totalFrames}, slow=${slowFrames}, frozen=${frozenFrames}`,
167+
);
168+
}
169+
170+
// Update last child span end frames for root span fallback logic
171+
if (!isRootSpan(span)) {
172+
_lastChildSpanEndFrames = {
173+
timestamp,
174+
nativeFrames: endFrames,
175+
};
176+
}
177+
} catch (error) {
178+
debug.log(`[${INTEGRATION_NAME}] Error while capturing end frames for span ${spanId}.`, error);
143179
}
144180
};
145181

@@ -233,21 +269,37 @@ export const nativeFramesIntegration = (): Integration => {
233269

234270
function fetchNativeFrames(): Promise<NativeFramesResponse> {
235271
return new Promise<NativeFramesResponse>((resolve, reject) => {
272+
let settled = false;
273+
274+
const timeoutId = setTimeout(() => {
275+
if (!settled) {
276+
settled = true;
277+
reject('Fetching native frames took too long. Dropping frames.');
278+
}
279+
}, FETCH_FRAMES_TIMEOUT_MS);
280+
236281
NATIVE.fetchNativeFrames()
237282
.then(value => {
283+
if (settled) {
284+
return;
285+
}
286+
clearTimeout(timeoutId);
287+
settled = true;
288+
238289
if (!value) {
239290
reject('Native frames response is null.');
240291
return;
241292
}
242293
resolve(value);
243294
})
244295
.then(undefined, error => {
296+
if (settled) {
297+
return;
298+
}
299+
clearTimeout(timeoutId);
300+
settled = true;
245301
reject(error);
246302
});
247-
248-
setTimeout(() => {
249-
reject('Fetching native frames took too long. Dropping frames.');
250-
}, FETCH_FRAMES_TIMEOUT_MS);
251303
});
252304
}
253305

‎packages/core/test/tracing/integrations/nativeframes.test.ts‎

Lines changed: 109 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -273,4 +273,113 @@ describe('NativeFramesInstrumentation', () => {
273273
}),
274274
]);
275275
});
276+
277+
it('attaches frame data to child spans', async () => {
278+
const rootStartFrames = { totalFrames: 100, slowFrames: 10, frozenFrames: 5 };
279+
const childStartFrames = { totalFrames: 110, slowFrames: 11, frozenFrames: 6 };
280+
const childEndFrames = { totalFrames: 160, slowFrames: 16, frozenFrames: 8 };
281+
const rootEndFrames = { totalFrames: 200, slowFrames: 20, frozenFrames: 10 };
282+
283+
mockFunction(NATIVE.fetchNativeFrames)
284+
.mockResolvedValueOnce(rootStartFrames)
285+
.mockResolvedValueOnce(childStartFrames)
286+
.mockResolvedValueOnce(childEndFrames)
287+
.mockResolvedValueOnce(rootEndFrames);
288+
289+
await startSpan({ name: 'test' }, async () => {
290+
startSpan({ name: 'child-span' }, () => {});
291+
await Promise.resolve(); // Flush frame captures
292+
await Promise.resolve();
293+
await Promise.resolve();
294+
});
295+
296+
await client.flush();
297+
298+
expect(client.event).toBeDefined();
299+
const childSpan = client.event!.spans!.find(s => s.description === 'child-span');
300+
expect(childSpan).toBeDefined();
301+
expect(childSpan!.data).toEqual(
302+
expect.objectContaining({
303+
'frames.total': 50,
304+
'frames.slow': 5,
305+
'frames.frozen': 2,
306+
}),
307+
);
308+
});
309+
310+
it('does not attach frame data to child spans when deltas are zero', async () => {
311+
const frames = {
312+
totalFrames: 100,
313+
slowFrames: 10,
314+
frozenFrames: 5,
315+
};
316+
mockFunction(NATIVE.fetchNativeFrames).mockResolvedValue(frames); // Same frames = delta of 0
317+
318+
await startSpan({ name: 'test' }, async () => {
319+
startSpan({ name: 'child-span' }, () => {});
320+
await Promise.resolve(); // Flush frame captures
321+
await Promise.resolve();
322+
await Promise.resolve();
323+
});
324+
325+
await client.flush();
326+
327+
expect(client.event).toBeDefined();
328+
const childSpan = client.event!.spans!.find(s => s.description === 'child-span');
329+
expect(childSpan).toBeDefined();
330+
expect(childSpan!.data).not.toHaveProperty('frames.total');
331+
expect(childSpan!.data).not.toHaveProperty('frames.slow');
332+
expect(childSpan!.data).not.toHaveProperty('frames.frozen');
333+
});
334+
335+
it('attaches frame data to multiple child spans', async () => {
336+
const rootStartFrames = { totalFrames: 100, slowFrames: 10, frozenFrames: 5 };
337+
const child1StartFrames = { totalFrames: 100, slowFrames: 10, frozenFrames: 5 };
338+
const child2StartFrames = { totalFrames: 120, slowFrames: 12, frozenFrames: 6 };
339+
const child1EndFrames = { totalFrames: 120, slowFrames: 12, frozenFrames: 6 };
340+
const child2EndFrames = { totalFrames: 150, slowFrames: 15, frozenFrames: 8 };
341+
const rootEndFrames = { totalFrames: 200, slowFrames: 20, frozenFrames: 10 };
342+
343+
mockFunction(NATIVE.fetchNativeFrames)
344+
.mockResolvedValueOnce(rootStartFrames)
345+
.mockResolvedValueOnce(child1StartFrames)
346+
.mockResolvedValueOnce(child2StartFrames)
347+
.mockResolvedValueOnce(child1EndFrames)
348+
.mockResolvedValueOnce(child2EndFrames)
349+
.mockResolvedValueOnce(rootEndFrames);
350+
351+
await startSpan({ name: 'test' }, async () => {
352+
startSpan({ name: 'child-span-1' }, () => {});
353+
startSpan({ name: 'child-span-2' }, () => {});
354+
355+
await Promise.resolve(); // Flush all frame captures
356+
await Promise.resolve();
357+
await Promise.resolve();
358+
await Promise.resolve();
359+
});
360+
361+
await client.flush();
362+
363+
expect(client.event).toBeDefined();
364+
365+
const childSpan1 = client.event!.spans!.find(s => s.description === 'child-span-1');
366+
expect(childSpan1).toBeDefined();
367+
expect(childSpan1!.data).toEqual(
368+
expect.objectContaining({
369+
'frames.total': 20,
370+
'frames.slow': 2,
371+
'frames.frozen': 1,
372+
}),
373+
);
374+
375+
const childSpan2 = client.event!.spans!.find(s => s.description === 'child-span-2');
376+
expect(childSpan2).toBeDefined();
377+
expect(childSpan2!.data).toEqual(
378+
expect.objectContaining({
379+
'frames.total': 30,
380+
'frames.slow': 3,
381+
'frames.frozen': 2,
382+
}),
383+
);
384+
});
276385
});

0 commit comments

Comments
 (0)