Skip to content

Commit

Permalink
fix(stalltracking): Ignore FrameStalling when app goes to background (#…
Browse files Browse the repository at this point in the history
…3211)

Co-authored-by: Kryštof Woldřich <[email protected]>
  • Loading branch information
lucas-zimerman and krystofwoldrich authored Aug 9, 2023
1 parent a76d821 commit 43dbc9e
Show file tree
Hide file tree
Showing 5 changed files with 126 additions and 1 deletion.
1 change: 1 addition & 0 deletions .prettierignore
Original file line number Diff line number Diff line change
@@ -1,2 +1,3 @@
/test/react-native/versions
CHANGELOG.md

1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@

### Fixes

- Stall Time is no longer counted when App is in Background. ([#3211](https://github.com/getsentry/sentry-react-native/pull/3211))
- Use application variant instead of variant output to hook to correct package task for modules cleanup ([#3161](https://github.com/getsentry/sentry-react-native/pull/3161))
- Fix `isNativeAvailable` after SDK reinitialization ([#3200](https://github.com/getsentry/sentry-react-native/pull/3200))

Expand Down
29 changes: 28 additions & 1 deletion src/js/tracing/stalltracking.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,8 @@
import type { IdleTransaction, Span, Transaction } from '@sentry/core';
import type { Measurements, MeasurementUnit } from '@sentry/types';
import { logger, timestampInSeconds } from '@sentry/utils';
import type { AppStateStatus } from 'react-native';
import { AppState } from 'react-native';

import { STALL_COUNT, STALL_LONGEST_TIME, STALL_TOTAL_TIME } from '../measurements';

Expand Down Expand Up @@ -47,6 +49,8 @@ export class StallTrackingInstrumentation {
private _lastIntervalMs: number = 0;
private _timeout: ReturnType<typeof setTimeout> | null = null;

private _isBackground: boolean = false;

private _statsByTransaction: Map<
Transaction,
{
Expand All @@ -61,6 +65,13 @@ export class StallTrackingInstrumentation {

public constructor(options: StallTrackingOptions = { minimumStallThreshold: 50 }) {
this._minimumStallThreshold = options.minimumStallThreshold;

this._backgroundEventListener = this._backgroundEventListener.bind(this);
// Avoids throwing any error if using React Native on a environment that doesn't implement AppState.
if (AppState?.isAvailable) {
// eslint-disable-next-line @typescript-eslint/unbound-method
AppState.addEventListener('change', this._backgroundEventListener);
}
}

/**
Expand Down Expand Up @@ -215,6 +226,22 @@ export class StallTrackingInstrumentation {
);
}

/**
* Switch that enables the iteraction once app moves from background to foreground.
*/
private _backgroundEventListener(state: AppStateStatus): void {
if (state === ('active' as AppStateStatus)) {
this._isBackground = false;
if (this._timeout != null) {
this._lastIntervalMs = timestampInSeconds() * 1000;
this._iteration();
}
} else {
this._isBackground = true;
this._timeout !== null && clearTimeout(this._timeout);
}
}

/**
* Logs the finish time of the span for use in `trimEnd: true` transactions.
*/
Expand Down Expand Up @@ -329,7 +356,7 @@ export class StallTrackingInstrumentation {

this._lastIntervalMs = now;

if (this.isTracking) {
if (this.isTracking && !this._isBackground) {
this._timeout = setTimeout(this._iteration.bind(this), LOOP_TIMEOUT_INTERVAL_MS);
}
}
Expand Down
46 changes: 46 additions & 0 deletions test/tracing/stalltracking.background.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,46 @@
import type { AppStateStatus } from 'react-native';

import { StallTrackingInstrumentation } from '../../src/js/tracing/stalltracking';

describe('BackgroundEventListener', () => {
it('Stall tracking should set _isBackground to false, update _lastIntervalMs, and call _iteration when state is active and _timeout is not null', () => {
const stallTracking = new StallTrackingInstrumentation();
const LOOP_TIMEOUT_INTERVAL_MS = 500; // Change this value based on your actual interval value
const currentTime = Date.now();
stallTracking['_lastIntervalMs'] = currentTime;
stallTracking['_timeout'] = setTimeout(() => {}, LOOP_TIMEOUT_INTERVAL_MS); // Create a fake timeout to simulate a running interval
stallTracking['_isBackground'] = true;
jest.useFakeTimers(); // Enable fake timers to control timeouts
stallTracking['_backgroundEventListener']('active' as AppStateStatus);
// Check if _isBackground is set to false and _lastIntervalMs is updated correctly
expect(stallTracking['_isBackground']).toBe(false);
expect(stallTracking['_lastIntervalMs']).toBeGreaterThanOrEqual(currentTime);
jest.runOnlyPendingTimers(); // Fast-forward the timer to execute the timeout function
});
it('Stall tracking should set _isBackground to true when state is not active', () => {
const stallTracking = new StallTrackingInstrumentation();
stallTracking['_isBackground'] = false;
stallTracking['_backgroundEventListener']('background' as AppStateStatus);
// Check if _isBackground is set to true
expect(stallTracking['_isBackground']).toBe(true);
});
it('Stall tracking should not call _iteration when state is active but _timeout is null', () => {
const stallTracking = new StallTrackingInstrumentation();
stallTracking['_timeout'] = null;
// Mock _iteration
stallTracking['_iteration'] = jest.fn();
jest.useFakeTimers(); // Enable fake timers to control timeouts
stallTracking['_backgroundEventListener']('active' as AppStateStatus);

expect(stallTracking['_iteration']).not.toBeCalled();
});
it('Stall tracking should call _iteration when state is active and _timeout is defined', () => {
const stallTracking = new StallTrackingInstrumentation();
stallTracking['_timeout'] = setTimeout(() => {}, 500);
// Mock _iteration
stallTracking['_iteration'] = jest.fn(); // Create a fake timeout to simulate a running interval
jest.useFakeTimers(); // Enable fake timers to control timeouts
stallTracking['_backgroundEventListener']('active' as AppStateStatus);
expect(stallTracking['_iteration']).toBeCalled();
});
});
50 changes: 50 additions & 0 deletions test/tracing/stalltracking.iteration.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,50 @@
import { StallTrackingInstrumentation } from '../../src/js/tracing/stalltracking';

describe('Iteration', () => {
it('Stall tracking does not set _timeout when isTracking is false', () => {
const stallTracking = new StallTrackingInstrumentation();
stallTracking['isTracking'] = false;
stallTracking['_isBackground'] = false;
stallTracking['_lastIntervalMs'] = Date.now() - 1000; // Force a timeout
jest.useFakeTimers();
// Invokes the private _interaction function.
stallTracking['_iteration']();
expect(stallTracking['_timeout']).toBeNull();
});
it('Stall tracking does not set _timeout when isBackground is true', () => {
const stallTracking = new StallTrackingInstrumentation();
stallTracking['isTracking'] = true;
stallTracking['_isBackground'] = true;
stallTracking['_lastIntervalMs'] = Date.now() - 1000; // Force a timeout
jest.useFakeTimers();
// Invokes the private _interaction function.
stallTracking['_iteration']();
expect(stallTracking['_timeout']).toBeNull();
});
it('Stall tracking should set _timeout when isTracking is true and isBackground false', () => {
const stallTracking = new StallTrackingInstrumentation();
stallTracking['isTracking'] = true;
stallTracking['_isBackground'] = false;
jest.useFakeTimers();
stallTracking['_lastIntervalMs'] = Date.now(); // Force a timeout
// Invokes the private _interaction function.
stallTracking['_iteration']();
expect(stallTracking['_timeout']).toBeDefined();
});
it('Stall tracking should update _stallCount and _totalStallTime when timeout condition is met', () => {
const stallTracking = new StallTrackingInstrumentation();
const LOOP_TIMEOUT_INTERVAL_MS = 50;
const _minimumStallThreshold = 100;
// Call _iteration with totalTimeTaken >= LOOP_TIMEOUT_INTERVAL_MS + _minimumStallThreshold
const totalTimeTaken = LOOP_TIMEOUT_INTERVAL_MS + _minimumStallThreshold;
jest.useFakeTimers();
stallTracking['_lastIntervalMs'] = Date.now() - totalTimeTaken;
stallTracking['_statsByTransaction'] = new Map();
stallTracking['_iteration']();
// Check if _stallCount and _totalStallTime have been updated as expected.
expect(stallTracking['_stallCount']).toBe(1);
expect(stallTracking['_totalStallTime']).toBeGreaterThanOrEqual(
Math.round(totalTimeTaken - LOOP_TIMEOUT_INTERVAL_MS),
);
});
});

0 comments on commit 43dbc9e

Please sign in to comment.