Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@
### Fixes

- Preserves interaction span context during app restart to allow proper replay capture ([#5386](https://github.com/getsentry/sentry-react-native/pull/5386))
- Discard empty Route Change transactions ([#5387](https://github.com/getsentry/sentry-react-native/issues/5387))

### Dependencies

Expand Down
94 changes: 77 additions & 17 deletions packages/core/src/js/tracing/onSpanEndUtils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,28 @@ export const adjustTransactionDuration = (client: Client, span: Span, maxDuratio
});
};

export const ignoreEmptyBackNavigation = (client: Client | undefined, span: Span | undefined): void => {
/**
* Helper function to filter out auto-instrumentation child spans.
*/
function getMeaningfulChildSpans(span: Span): Span[] {
const children = getSpanDescendants(span);
return children.filter(
child =>
child.spanContext().spanId !== span.spanContext().spanId &&
spanToJSON(child).op !== 'ui.load.initial_display' &&
spanToJSON(child).op !== 'navigation.processing',
);
}

/**
* Generic helper to discard empty navigation spans based on a condition.
*/
function discardEmptyNavigationSpan(
client: Client | undefined,
span: Span | undefined,
shouldDiscardFn: (span: Span) => boolean,
onDiscardFn: (span: Span) => void,
): void {
if (!client) {
debug.warn('Could not hook on spanEnd event because client is not defined.');
return;
Expand All @@ -55,7 +76,7 @@ export const ignoreEmptyBackNavigation = (client: Client | undefined, span: Span
}

if (!isRootSpan(span) || !isSentrySpan(span)) {
debug.warn('Not sampling empty back spans only works for Sentry Transactions (Root Spans).');
debug.warn('Not sampling empty navigation spans only works for Sentry Transactions (Root Spans).');
return;
}

Expand All @@ -64,27 +85,66 @@ export const ignoreEmptyBackNavigation = (client: Client | undefined, span: Span
return;
}

if (!spanToJSON(span).data?.['route.has_been_seen']) {
if (!shouldDiscardFn(span)) {
return;
}

const children = getSpanDescendants(span);
const filtered = children.filter(
child =>
child.spanContext().spanId !== span.spanContext().spanId &&
spanToJSON(child).op !== 'ui.load.initial_display' &&
spanToJSON(child).op !== 'navigation.processing',
);

if (filtered.length <= 0) {
// filter children must include at least one span not created by the navigation automatic instrumentation
debug.log(
'Not sampling transaction as route has been seen before. Pass ignoreEmptyBackNavigationTransactions = false to disable this feature.',
);
// Route has been seen before and has no child spans.
const meaningfulChildren = getMeaningfulChildSpans(span);
if (meaningfulChildren.length <= 0) {
onDiscardFn(span);
span['_sampled'] = false;
}
});
}

export const ignoreEmptyBackNavigation = (client: Client | undefined, span: Span | undefined): void => {
discardEmptyNavigationSpan(
client,
span,
// Only discard if route has been seen before
span => spanToJSON(span).data?.['route.has_been_seen'] === true,
// Log message when discarding
() => {
debug.log(
'Not sampling transaction as route has been seen before. Pass ignoreEmptyBackNavigationTransactions = false to disable this feature.',
);
},
);
};

/**
* Discards empty "Route Change" transactions that never received route information.
* This happens when navigation library emits a route change event but getCurrentRoute() returns undefined.
* Such transactions don't contain any useful information and should not be sent to Sentry.
*
* This function must be called with a reference tracker function that can check if the span
* was cleared from the integration's tracking (indicating it went through the state listener).
*/
export const ignoreEmptyRouteChangeTransactions = (
client: Client | undefined,
span: Span | undefined,
defaultNavigationSpanName: string,
isSpanStillTracked: () => boolean,
): void => {
discardEmptyNavigationSpan(
client,
span,
// Only discard if:
// 1. Still has default name
// 2. No route information was set
// 3. Still being tracked (state listener never called)
span => {
const spanJSON = spanToJSON(span);
return (
spanJSON.description === defaultNavigationSpanName && !spanJSON.data?.['route.name'] && isSpanStillTracked()
);
},
// Log and record dropped event
_span => {
debug.log(`Discarding empty "${defaultNavigationSpanName}" transaction that never received route information.`);
client?.recordDroppedEvent('sample_rate', 'transaction');
},
);
};

/**
Expand Down
10 changes: 9 additions & 1 deletion packages/core/src/js/tracing/reactnativenavigation.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ import {
} from '@sentry/core';
import type { EmitterSubscription } from '../utils/rnlibrariesinterface';
import { isSentrySpan } from '../utils/span';
import { ignoreEmptyBackNavigation } from './onSpanEndUtils';
import { ignoreEmptyBackNavigation, ignoreEmptyRouteChangeTransactions } from './onSpanEndUtils';
import { SPAN_ORIGIN_AUTO_NAVIGATION_REACT_NATIVE_NAVIGATION } from './origin';
import type { ReactNativeTracingIntegration } from './reactnativetracing';
import { getReactNativeTracingIntegration } from './reactnativetracing';
Expand Down Expand Up @@ -140,6 +140,14 @@ export const reactNativeNavigationIntegration = ({
if (ignoreEmptyBackNavigationTransactions) {
ignoreEmptyBackNavigation(getClient(), latestNavigationSpan);
}
// Always discard transactions that never receive route information
const spanToCheck = latestNavigationSpan;
ignoreEmptyRouteChangeTransactions(
getClient(),
spanToCheck,
DEFAULT_NAVIGATION_SPAN_NAME,
() => latestNavigationSpan === spanToCheck,
);

stateChangeTimeout = setTimeout(discardLatestNavigationSpan.bind(this), routeChangeTimeoutMs);
};
Expand Down
10 changes: 9 additions & 1 deletion packages/core/src/js/tracing/reactnavigation.ts
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ import { isSentrySpan } from '../utils/span';
import { RN_GLOBAL_OBJ } from '../utils/worldwide';
import type { UnsafeAction } from '../vendor/react-navigation/types';
import { NATIVE } from '../wrapper';
import { ignoreEmptyBackNavigation } from './onSpanEndUtils';
import { ignoreEmptyBackNavigation, ignoreEmptyRouteChangeTransactions } from './onSpanEndUtils';
import { SPAN_ORIGIN_AUTO_NAVIGATION_REACT_NAVIGATION } from './origin';
import type { ReactNativeTracingIntegration } from './reactnativetracing';
import { getReactNativeTracingIntegration } from './reactnativetracing';
Expand Down Expand Up @@ -255,6 +255,14 @@ export const reactNavigationIntegration = ({
if (ignoreEmptyBackNavigationTransactions) {
ignoreEmptyBackNavigation(getClient(), latestNavigationSpan);
}
// Always discard transactions that never receive route information
const spanToCheck = latestNavigationSpan;
ignoreEmptyRouteChangeTransactions(
getClient(),
spanToCheck,
DEFAULT_NAVIGATION_SPAN_NAME,
() => latestNavigationSpan === spanToCheck,
);

if (enableTimeToInitialDisplay && latestNavigationSpan) {
NATIVE.setActiveSpanId(latestNavigationSpan.spanContext().spanId);
Expand Down
56 changes: 56 additions & 0 deletions packages/core/test/tracing/reactnavigation.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -329,6 +329,62 @@ describe('ReactNavigationInstrumentation', () => {
);
});

test('empty Route Change transaction is not sent when route is undefined', async () => {
setupTestClient();
jest.runOnlyPendingTimers(); // Flush the init transaction

mockNavigation.emitNavigationWithUndefinedRoute();
jest.runOnlyPendingTimers(); // Flush the empty route change transaction

await client.flush();

// Only the initial transaction should be sent
expect(client.eventQueue.length).toBe(1);
expect(client.event).toEqual(
expect.objectContaining({
type: 'transaction',
transaction: 'Initial Screen',
}),
);
});

test('empty Route Change transaction is recorded as dropped', async () => {
const mockRecordDroppedEvent = jest.fn();
setupTestClient();
client.recordDroppedEvent = mockRecordDroppedEvent;
jest.runOnlyPendingTimers(); // Flush the init transaction

mockNavigation.emitNavigationWithUndefinedRoute();
jest.runOnlyPendingTimers(); // Flush the empty route change transaction

await client.flush();

// Should have recorded a dropped transaction
expect(mockRecordDroppedEvent).toHaveBeenCalledWith('sample_rate', 'transaction');
});

test('empty Route Change transaction is not sent after multiple undefined routes', async () => {
setupTestClient();
jest.runOnlyPendingTimers(); // Flush the init transaction

mockNavigation.emitNavigationWithUndefinedRoute();
jest.runOnlyPendingTimers(); // Flush the first empty route change transaction

mockNavigation.emitNavigationWithUndefinedRoute();
jest.runOnlyPendingTimers(); // Flush the second empty route change transaction

await client.flush();

// Only the initial transaction should be sent
expect(client.eventQueue.length).toBe(1);
expect(client.event).toEqual(
expect.objectContaining({
type: 'transaction',
transaction: 'Initial Screen',
}),
);
});

describe('navigation container registration', () => {
test('registers navigation container object ref', () => {
const instrumentation = reactNavigationIntegration();
Expand Down
7 changes: 7 additions & 0 deletions packages/core/test/tracing/reactnavigationutils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -68,6 +68,13 @@ export function createMockNavigationAndAttachTo(sut: ReturnType<typeof reactNavi
// this object is not used by the instrumentation
});
},
emitNavigationWithUndefinedRoute: () => {
mockedNavigationContained.listeners['__unsafe_action__'](navigationAction);
mockedNavigationContained.currentRoute = undefined as any;
mockedNavigationContained.listeners['state']({
// this object is not used by the instrumentation
});
},
};
sut.registerNavigationContainer(mockRef(mockedNavigationContained));

Expand Down
Loading