Skip to content

Commit c74b4c2

Browse files
fix(tracing): Drop app start data older than one minute (#3974)
1 parent 2d978f0 commit c74b4c2

File tree

3 files changed

+101
-1
lines changed

3 files changed

+101
-1
lines changed

CHANGELOG.md

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,10 @@
22

33
## Unreleased
44

5+
### Fixes
6+
7+
- Drop app start data older than one minute ([#3974](https://github.com/getsentry/sentry-react-native/pull/3974))
8+
59
### Dependencies
610

711
- Bump Android SDK from v7.12.0 to v7.12.1 ([#3970](https://github.com/getsentry/sentry-react-native/pull/3970))

src/js/tracing/reactnativetracing.ts

Lines changed: 19 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@
22
import type { RequestInstrumentationOptions } from '@sentry/browser';
33
import { defaultRequestInstrumentationOptions, instrumentOutgoingRequests } from '@sentry/browser';
44
import type { Hub, IdleTransaction, Transaction } from '@sentry/core';
5-
import { getActiveTransaction, getCurrentHub, startIdleTransaction } from '@sentry/core';
5+
import { getActiveTransaction, getCurrentHub, spanToJSON, startIdleTransaction } from '@sentry/core';
66
import type {
77
Event,
88
EventProcessor,
@@ -134,6 +134,8 @@ export class ReactNativeTracing implements Integration {
134134
public static id: string = 'ReactNativeTracing';
135135
/** We filter out App starts more than 60s */
136136
private static _maxAppStart: number = 60000;
137+
/** We filter out App starts which timestamp is 60s and more before the transaction start */
138+
private static _maxAppStartBeforeTransactionMs: number = 60000;
137139
/**
138140
* @inheritDoc
139141
*/
@@ -477,6 +479,14 @@ export class ReactNativeTracing implements Integration {
477479
return;
478480
}
479481

482+
const isAppStartWithinBounds =
483+
appStartTimestampMs >=
484+
getTransactionStartTimestampMs(transaction) - ReactNativeTracing._maxAppStartBeforeTransactionMs;
485+
if (!__DEV__ && !isAppStartWithinBounds) {
486+
logger.warn('[ReactNativeTracing] App start timestamp is too far in the past to be used for app start span.');
487+
return;
488+
}
489+
480490
const appStartDurationMilliseconds = this._getAppStartDurationMilliseconds(appStartTimestampMs);
481491
if (!appStartDurationMilliseconds) {
482492
logger.warn('[ReactNativeTracing] App start end has not been recorded, not adding app start span.');
@@ -711,3 +721,11 @@ export class ReactNativeTracing implements Integration {
711721
return tx;
712722
}
713723
}
724+
725+
/**
726+
* Returns transaction start timestamp in milliseconds.
727+
* If start timestamp is not available, returns 0.
728+
*/
729+
function getTransactionStartTimestampMs(transaction: Transaction): number {
730+
return (spanToJSON(transaction).start_timestamp || 0) * 1000;
731+
}

test/tracing/reactnativetracing.test.ts

Lines changed: 78 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,15 @@ jest.mock('../../src/js/tracing/utils', () => {
2626
};
2727
});
2828

29+
jest.mock('@sentry/utils', () => {
30+
const originalUtils = jest.requireActual('@sentry/utils');
31+
32+
return {
33+
...originalUtils,
34+
timestampInSeconds: jest.fn(originalUtils.timestampInSeconds),
35+
};
36+
});
37+
2938
type MockAppState = {
3039
setState: (state: AppStateStatus) => void;
3140
listener: (newState: AppStateStatus) => void;
@@ -51,6 +60,7 @@ jest.mock('react-native/Libraries/AppState/AppState', () => mockedAppState);
5160

5261
import { getActiveSpan, startSpanManual } from '@sentry/browser';
5362
import { addGlobalEventProcessor, getCurrentHub, getCurrentScope, spanToJSON, startInactiveSpan } from '@sentry/core';
63+
import { timestampInSeconds } from '@sentry/utils';
5464
import type { AppState, AppStateStatus } from 'react-native';
5565

5666
import { APP_START_COLD, APP_START_WARM } from '../../src/js/measurements';
@@ -70,6 +80,8 @@ import { mockFunction } from '../testutils';
7080
import type { MockedRoutingInstrumentation } from './mockedrountinginstrumention';
7181
import { createMockedRoutingInstrumentation } from './mockedrountinginstrumention';
7282

83+
const originalTimestampInSeconds = mockFunction(timestampInSeconds).getMockImplementation();
84+
7385
const DEFAULT_IDLE_TIMEOUT = 1000;
7486

7587
describe('ReactNativeTracing', () => {
@@ -317,6 +329,65 @@ describe('ReactNativeTracing', () => {
317329
expect(transaction?.start_timestamp).toBeGreaterThanOrEqual(timeOriginMilliseconds / 1000);
318330
});
319331

332+
describe('old app starts', () => {
333+
let integration: ReactNativeTracing;
334+
let timeOriginMilliseconds: number;
335+
336+
beforeEach(() => {
337+
integration = new ReactNativeTracing();
338+
339+
timeOriginMilliseconds = Date.now();
340+
const appStartTimeMilliseconds = timeOriginMilliseconds - 65000;
341+
const mockAppStartResponse: NativeAppStartResponse = {
342+
type: 'warm',
343+
app_start_timestamp_ms: appStartTimeMilliseconds,
344+
has_fetched: false,
345+
spans: [],
346+
};
347+
348+
// App start finish timestamp
349+
mockFunction(getTimeOriginMilliseconds).mockReturnValue(timeOriginMilliseconds - 64000);
350+
mockFunction(NATIVE.fetchNativeAppStart).mockResolvedValue(mockAppStartResponse);
351+
// Transaction start timestamp
352+
mockFunction(timestampInSeconds).mockReturnValue(timeOriginMilliseconds / 1000 + 65);
353+
});
354+
355+
afterEach(() => {
356+
mockFunction(timestampInSeconds).mockReset().mockImplementation(originalTimestampInSeconds);
357+
set__DEV__(true);
358+
});
359+
360+
it('Does not add app start span older than than 60s in production', async () => {
361+
set__DEV__(false);
362+
363+
setup(integration);
364+
365+
await jest.advanceTimersByTimeAsync(500);
366+
await jest.runOnlyPendingTimersAsync();
367+
368+
const transaction = client.event;
369+
370+
expect(transaction).toBeDefined();
371+
expect(transaction?.spans?.some(span => span.op == APP_SPAN_START_WARM)).toBeFalse();
372+
expect(transaction?.start_timestamp).toBeGreaterThanOrEqual(timeOriginMilliseconds / 1000);
373+
});
374+
375+
it('Does add app start span older than than 60s in development builds', async () => {
376+
set__DEV__(true);
377+
378+
setup(integration);
379+
380+
await jest.advanceTimersByTimeAsync(500);
381+
await jest.runOnlyPendingTimersAsync();
382+
383+
const transaction = client.event;
384+
385+
expect(transaction).toBeDefined();
386+
expect(transaction?.spans?.some(span => span.op == APP_SPAN_START_WARM)).toBeTrue();
387+
expect(transaction?.start_timestamp).toBeGreaterThanOrEqual((timeOriginMilliseconds - 65000) / 1000);
388+
});
389+
});
390+
320391
it('Does not create app start transaction if has_fetched == true', async () => {
321392
const integration = new ReactNativeTracing();
322393

@@ -1195,3 +1266,10 @@ function clearReactNativeBundleExecutionStartTimestamp() {
11951266
delete RN_GLOBAL_OBJ.nativePerformanceNow;
11961267
delete RN_GLOBAL_OBJ.__BUNDLE_START_TIME__;
11971268
}
1269+
1270+
function set__DEV__(value: boolean) {
1271+
Object.defineProperty(globalThis, '__DEV__', {
1272+
value,
1273+
writable: true,
1274+
});
1275+
}

0 commit comments

Comments
 (0)