Skip to content

Commit 4295cdc

Browse files
fix(appStart): Attach App Start spans to the first created not the first processed root span (#4618) (#4644)
1 parent 4a7267c commit 4295cdc

File tree

5 files changed

+55
-8
lines changed

5 files changed

+55
-8
lines changed

CHANGELOG.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@
1111
### Fixes
1212

1313
- Fixes missing Cold Start measurements by bumping the Android SDK version to v7.22.1 ([#4643](https://github.com/getsentry/sentry-react-native/pull/4643))
14+
- Attach App Start spans to the first created not the first processed root span ([#4618](https://github.com/getsentry/sentry-react-native/pull/4618), [#4644](https://github.com/getsentry/sentry-react-native/pull/4644))
1415

1516
### Dependencies
1617

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

Lines changed: 42 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
1-
/* eslint-disable complexity */
2-
import type { Client, Event, Integration, SpanJSON, TransactionEvent } from '@sentry/core';
1+
/* eslint-disable complexity, max-lines */
2+
import type { Client, Event, Integration, Span, SpanJSON, TransactionEvent } from '@sentry/core';
33
import {
44
getCapturedScopesOnSpan,
55
getClient,
@@ -17,7 +17,7 @@ import {
1717
} from '../../measurements';
1818
import type { NativeAppStartResponse } from '../../NativeRNSentry';
1919
import type { ReactNativeClientOptions } from '../../options';
20-
import { convertSpanToTransaction, setEndTimeValue } from '../../utils/span';
20+
import { convertSpanToTransaction, isRootSpan, setEndTimeValue } from '../../utils/span';
2121
import { NATIVE } from '../../wrapper';
2222
import {
2323
APP_START_COLD as APP_START_COLD_OP,
@@ -136,16 +136,18 @@ export const appStartIntegration = ({
136136
let _client: Client | undefined = undefined;
137137
let isEnabled = true;
138138
let appStartDataFlushed = false;
139+
let firstStartedActiveRootSpanId: string | undefined = undefined;
139140

140141
const setup = (client: Client): void => {
141142
_client = client;
142-
const clientOptions = client.getOptions() as ReactNativeClientOptions;
143+
const { enableAppStartTracking } = client.getOptions() as ReactNativeClientOptions;
143144

144-
const { enableAppStartTracking } = clientOptions;
145145
if (!enableAppStartTracking) {
146146
isEnabled = false;
147147
logger.warn('[AppStart] App start tracking is disabled.');
148148
}
149+
150+
client.on('spanStart', recordFirstStartedActiveRootSpanId);
149151
};
150152

151153
const afterAllSetup = (_client: Client): void => {
@@ -167,6 +169,27 @@ export const appStartIntegration = ({
167169
return event;
168170
};
169171

172+
const recordFirstStartedActiveRootSpanId = (rootSpan: Span): void => {
173+
if (firstStartedActiveRootSpanId) {
174+
return;
175+
}
176+
177+
if (!isRootSpan(rootSpan)) {
178+
return;
179+
}
180+
181+
setFirstStartedActiveRootSpanId(rootSpan.spanContext().spanId);
182+
};
183+
184+
/**
185+
* For testing purposes only.
186+
* @private
187+
*/
188+
const setFirstStartedActiveRootSpanId = (spanId: string | undefined): void => {
189+
firstStartedActiveRootSpanId = spanId;
190+
logger.debug('[AppStart] First started active root span id recorded.', firstStartedActiveRootSpanId);
191+
};
192+
170193
async function captureStandaloneAppStart(): Promise<void> {
171194
if (!standalone) {
172195
logger.debug(
@@ -212,11 +235,23 @@ export const appStartIntegration = ({
212235
return;
213236
}
214237

238+
if (!firstStartedActiveRootSpanId) {
239+
logger.warn('[AppStart] No first started active root span id recorded. Can not attach app start.');
240+
return;
241+
}
242+
215243
if (!event.contexts || !event.contexts.trace) {
216244
logger.warn('[AppStart] Transaction event is missing trace context. Can not attach app start.');
217245
return;
218246
}
219247

248+
if (firstStartedActiveRootSpanId !== event.contexts.trace.span_id) {
249+
logger.warn(
250+
'[AppStart] First started active root span id does not match the transaction event span id. Can not attached app start.',
251+
);
252+
return;
253+
}
254+
220255
const appStart = await NATIVE.fetchNativeAppStart();
221256
if (!appStart) {
222257
logger.warn('[AppStart] Failed to retrieve the app start metrics from the native layer.');
@@ -332,7 +367,8 @@ export const appStartIntegration = ({
332367
afterAllSetup,
333368
processEvent,
334369
captureStandaloneAppStart,
335-
};
370+
setFirstStartedActiveRootSpanId,
371+
} as AppStartIntegration;
336372
};
337373

338374
function setSpanDurationAsMeasurementOnTransactionEvent(event: TransactionEvent, label: string, span: SpanJSON): void {

packages/core/test/profiling/integration.test.ts

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -262,8 +262,9 @@ describe('profiling integration', () => {
262262
const transaction1 = Sentry.startSpanManual({ name: 'test-name-1' }, span => span);
263263
const transaction2 = Sentry.startSpanManual({ name: 'test-name-2' }, span => span);
264264
transaction1.end();
265-
transaction2.end();
265+
jest.runOnlyPendingTimers();
266266

267+
transaction2.end();
267268
jest.runAllTimers();
268269

269270
expectEnvelopeToContainProfile(

packages/core/test/tracing/integrations/appStart.test.ts

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,10 @@ import { NATIVE } from '../../../src/js/wrapper';
3333
import { getDefaultTestClientOptions, TestClient } from '../../mocks/client';
3434
import { mockFunction } from '../../testutils';
3535

36+
type AppStartIntegrationTest = ReturnType<typeof appStartIntegration> & {
37+
setFirstStartedActiveRootSpanId: (spanId: string | undefined) => void;
38+
};
39+
3640
let dateNowSpy: jest.SpyInstance;
3741

3842
jest.mock('../../../src/js/wrapper', () => {
@@ -689,7 +693,10 @@ describe('App Start Integration', () => {
689693
const integration = appStartIntegration();
690694
const client = new TestClient(getDefaultTestClientOptions());
691695

692-
const actualEvent = await integration.processEvent(getMinimalTransactionEvent(), {}, client);
696+
const firstEvent = getMinimalTransactionEvent();
697+
(integration as AppStartIntegrationTest).setFirstStartedActiveRootSpanId(firstEvent.contexts?.trace?.span_id);
698+
699+
const actualEvent = await integration.processEvent(firstEvent, {}, client);
693700
expect(actualEvent).toEqual(
694701
expectEventWithAttachedColdAppStart({ timeOriginMilliseconds, appStartTimeMilliseconds }),
695702
);
@@ -722,6 +729,7 @@ describe('App Start Integration', () => {
722729

723730
function processEvent(event: Event): PromiseLike<Event | null> | Event | null {
724731
const integration = appStartIntegration();
732+
(integration as AppStartIntegrationTest).setFirstStartedActiveRootSpanId(event.contexts?.trace?.span_id);
725733
return integration.processEvent(event, {}, new TestClient(getDefaultTestClientOptions()));
726734
}
727735

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

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -359,6 +359,7 @@ describe('React Navigation - TTID', () => {
359359
});
360360

361361
test('idle transaction should cancel the ttid span if new frame not received', () => {
362+
jest.runOnlyPendingTimers(); // Flush app start transaction
362363
mockedNavigation.navigateToNewScreen();
363364
jest.runOnlyPendingTimers(); // Flush ttid transaction
364365

0 commit comments

Comments
 (0)