Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix: App start time span no longer created if too long #3299

Merged
merged 10 commits into from
Oct 9, 2023
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,7 @@

### Fixes

- App start time span no longer created if too long ([#3299](https://github.com/getsentry/sentry-react-native/pull/3299))
- Change log output to show what paths are considered when collecting modules ([#3316](https://github.com/getsentry/sentry-react-native/pull/3316))

### Dependencies
Expand Down
39 changes: 23 additions & 16 deletions src/js/tracing/reactnativetracing.ts
Original file line number Diff line number Diff line change
Expand Up @@ -328,6 +328,17 @@ export class ReactNativeTracing implements Integration {
return this._inflightInteractionTransaction;
}

/**
* Returns the App Start Duration in Milliseconds. Also returns undefined if not able do
* define the duration.
*/
private _getAppStartDurationMilliseconds(appStart: NativeAppStartResponse): number | undefined {
if (!this._appStartFinishTimestamp) {
return undefined;
}
return this._appStartFinishTimestamp * 1000 - appStart.appStartTime;
}

/**
* Instruments the app start measurements on the first route transaction.
* Starts a route transaction if there isn't routing instrumentation.
Expand All @@ -350,12 +361,9 @@ export class ReactNativeTracing implements Integration {
if (this.options.routingInstrumentation) {
this._awaitingAppStartData = appStart;
} else {
const appStartTimeSeconds = appStart.appStartTime / 1000;

const idleTransaction = this._createRouteTransaction({
name: 'App Start',
op: UI_LOAD,
startTimestamp: appStartTimeSeconds,
});

if (idleTransaction) {
Expand All @@ -368,13 +376,23 @@ export class ReactNativeTracing implements Integration {
* Adds app start measurements and starts a child span on a transaction.
*/
private _addAppStartData(transaction: IdleTransaction, appStart: NativeAppStartResponse): void {
if (!this._appStartFinishTimestamp) {
const appStartDurationMilliseconds = this._getAppStartDurationMilliseconds(appStart);
if (!appStartDurationMilliseconds) {
logger.warn('App start was never finished.');
return;
}

// we filter out app start more than 60s.
// this could be due to many different reasons.
// we've seen app starts with hours, days and even months.
if (appStartDurationMilliseconds >= ReactNativeTracing._maxAppStart) {
return;
}

const appStartTimeSeconds = appStart.appStartTime / 1000;

lucas-zimerman marked this conversation as resolved.
Show resolved Hide resolved
transaction.startTimestamp = appStartTimeSeconds;

const op = appStart.isColdStart ? APP_START_COLD_OP : APP_START_WARM_OP;
transaction.startChild({
description: appStart.isColdStart ? 'Cold App Start' : 'Warm App Start',
Expand All @@ -383,15 +401,6 @@ export class ReactNativeTracing implements Integration {
endTimestamp: this._appStartFinishTimestamp,
});

const appStartDurationMilliseconds = this._appStartFinishTimestamp * 1000 - appStart.appStartTime;

// we filter out app start more than 60s.
// this could be due to many different reasons.
// we've seen app starts with hours, days and even months.
if (appStartDurationMilliseconds >= ReactNativeTracing._maxAppStart) {
return;
}

const measurement = appStart.isColdStart ? APP_START_COLD : APP_START_WARM;
transaction.setMeasurement(measurement, appStartDurationMilliseconds, 'millisecond');
}
Expand Down Expand Up @@ -462,9 +471,7 @@ export class ReactNativeTracing implements Integration {

idleTransaction.registerBeforeFinishCallback(transaction => {
if (this.options.enableAppStartTracking && this._awaitingAppStartData) {
transaction.startTimestamp = this._awaitingAppStartData.appStartTime / 1000;
transaction.op = 'ui.load';

transaction.op = UI_LOAD;
this._addAppStartData(transaction, this._awaitingAppStartData);

this._awaitingAppStartData = undefined;
Expand Down
38 changes: 38 additions & 0 deletions test/tracing/reactnativetracing.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -93,6 +93,7 @@ import {
APP_START_WARM as APP_START_WARM_OP,
UI_LOAD,
} from '../../src/js/tracing';
import { APP_START_WARM as APP_SPAN_START_WARM } from '../../src/js/tracing/ops';
import { ReactNativeTracing } from '../../src/js/tracing/reactnativetracing';
import { getTimeOriginMilliseconds } from '../../src/js/tracing/utils';
import { NATIVE } from '../../src/js/wrapper';
Expand Down Expand Up @@ -233,6 +234,43 @@ describe('ReactNativeTracing', () => {
}
});

it('Does not add app start span if more than 60s', async () => {
const integration = new ReactNativeTracing();

const timeOriginMilliseconds = Date.now();
const appStartTimeMilliseconds = timeOriginMilliseconds - 65000;
const mockAppStartResponse: NativeAppStartResponse = {
isColdStart: false,
appStartTime: appStartTimeMilliseconds,
didFetchAppStart: false,
};

mockFunction(getTimeOriginMilliseconds).mockReturnValue(timeOriginMilliseconds);
mockFunction(NATIVE.fetchNativeAppStart).mockResolvedValue(mockAppStartResponse);

const mockHub = getMockHub();
integration.setupOnce(addGlobalEventProcessor, () => mockHub);

await jest.advanceTimersByTimeAsync(500);

const transaction = mockHub.getScope()?.getTransaction();

expect(transaction).toBeDefined();

if (transaction) {
expect(
// @ts-expect-error access private for test
transaction.spanRecorder,
).toBeDefined();

expect(
// @ts-expect-error access private for test
transaction.spanRecorder.spans.some(span => span.op == APP_SPAN_START_WARM),
).toBe(false);
expect(transaction.startTimestamp).toBeGreaterThanOrEqual(timeOriginMilliseconds / 1000);
}
lucas-zimerman marked this conversation as resolved.
Show resolved Hide resolved
});

it('Does not create app start transaction if didFetchAppStart == true', async () => {
const integration = new ReactNativeTracing();

Expand Down
Loading