Skip to content

Commit

Permalink
fix(ttid): End and measure TTID regardless current active span
Browse files Browse the repository at this point in the history
  • Loading branch information
krystofwoldrich committed Aug 12, 2024
1 parent 27126e9 commit cae8232
Show file tree
Hide file tree
Showing 4 changed files with 113 additions and 41 deletions.
5 changes: 5 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,11 @@

## Unreleased

### Fixed

- fix(ttid): End and measure TTID regardless current active span ([#4019](https://github.com/getsentry/sentry-react-native/pull/4019))
- Fixes possible missing TTID measurements and spans

### Dependencies

- Bump JavaScript SDK from v7.117.0 to v7.118.0 ([#4018](https://github.com/getsentry/sentry-react-native/pull/4018))
Expand Down
47 changes: 14 additions & 33 deletions src/js/tracing/reactnavigation.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/* eslint-disable max-lines */
import { getActiveSpan, setMeasurement, spanToJSON, startInactiveSpan } from '@sentry/core';
import { getActiveSpan, startInactiveSpan } from '@sentry/core';
import type { Span, Transaction as TransactionType, TransactionContext } from '@sentry/types';
import { logger, timestampInSeconds } from '@sentry/utils';

Expand All @@ -11,7 +11,12 @@ import type { OnConfirmRoute, TransactionCreator } from './routingInstrumentatio
import { InternalRoutingInstrumentation } from './routingInstrumentation';
import { manualInitialDisplaySpans, startTimeToInitialDisplaySpan } from './timetodisplay';
import type { BeforeNavigate, ReactNavigationTransactionContext, RouteChangeContextData } from './types';
import { customTransactionSource, defaultTransactionSource, getBlankTransactionContext } from './utils';
import {
customTransactionSource,
defaultTransactionSource,
getBlankTransactionContext,
setSpanDurationAsMeasurementOnTransaction,
} from './utils';

export interface NavigationRoute {
name: string;
Expand Down Expand Up @@ -224,6 +229,7 @@ export class ReactNavigationInstrumentation extends InternalRoutingInstrumentati
if (this._latestTransaction) {
if (!previousRoute || previousRoute.key !== route.key) {
const routeHasBeenSeen = this._recentRouteKeys.includes(route.key);
const latestTransaction = this._latestTransaction;
const latestTtidSpan =
!routeHasBeenSeen &&
this._options.enableTimeToInitialDisplay &&
Expand All @@ -233,46 +239,21 @@ export class ReactNavigationInstrumentation extends InternalRoutingInstrumentati
});

!routeHasBeenSeen &&
latestTtidSpan &&
this._newScreenFrameEventEmitter?.once(
NewFrameEventName,
({ newFrameTimestampInSeconds }: NewFrameEvent) => {
const activeSpan = getActiveSpan();
if (!activeSpan) {
logger.warn(
'[ReactNavigationInstrumentation] No active span found to attach ui.load.initial_display to.',
);
return;
}

if (manualInitialDisplaySpans.has(activeSpan)) {
if (activeSpan && manualInitialDisplaySpans.has(activeSpan)) {
logger.warn(
'[ReactNavigationInstrumentation] Detected manual instrumentation for the current active span.',
);
return;
}

if (!latestTtidSpan) {
return;
}

if (spanToJSON(latestTtidSpan).parent_span_id !== getActiveSpan()?.spanContext().spanId) {
logger.warn(
'[ReactNavigationInstrumentation] Currently Active Span changed before the new frame was rendered, _latestTtidSpan is not a child of the currently active span.',
);
return;
}

latestTtidSpan.setStatus('ok');
latestTtidSpan.end(newFrameTimestampInSeconds);
const ttidSpan = spanToJSON(latestTtidSpan);

const ttidSpanEnd = ttidSpan.timestamp;
const ttidSpanStart = ttidSpan.start_timestamp;
if (!ttidSpanEnd || !ttidSpanStart) {
return;
}

setMeasurement('time_to_initial_display', (ttidSpanEnd - ttidSpanStart) * 1000, 'millisecond');
setSpanDurationAsMeasurementOnTransaction(latestTransaction, 'time_to_initial_display', latestTtidSpan);
},
);

Expand All @@ -281,7 +262,7 @@ export class ReactNavigationInstrumentation extends InternalRoutingInstrumentati
this._navigationProcessingSpan?.end(stateChangedTimestamp);
this._navigationProcessingSpan = undefined;

const originalContext = this._latestTransaction.toContext() as typeof BLANK_TRANSACTION_CONTEXT;
const originalContext = latestTransaction.toContext() as typeof BLANK_TRANSACTION_CONTEXT;

const data: RouteChangeContextData = {
...originalContext.data,
Expand Down Expand Up @@ -313,10 +294,10 @@ export class ReactNavigationInstrumentation extends InternalRoutingInstrumentati
};

const finalContext = this._prepareFinalContext(updatedContext);
this._latestTransaction.updateWithContext(finalContext);
latestTransaction.updateWithContext(finalContext);

const isCustomName = updatedContext.name !== finalContext.name;
this._latestTransaction.setName(
latestTransaction.setName(
finalContext.name,
isCustomName ? customTransactionSource : defaultTransactionSource,
);
Expand Down
29 changes: 21 additions & 8 deletions src/js/tracing/utils.ts
Original file line number Diff line number Diff line change
@@ -1,11 +1,6 @@
import {
type IdleTransaction,
type Span as SpanClass,
type Transaction,
setMeasurement,
spanToJSON,
} from '@sentry/core';
import type { Span, TransactionContext, TransactionSource } from '@sentry/types';
import type { Transaction } from '@sentry/core';
import { type IdleTransaction, type Span as SpanClass, setMeasurement, spanToJSON } from '@sentry/core';
import type { Span, Transaction as TransactionType, TransactionContext, TransactionSource } from '@sentry/types';
import { logger, timestampInSeconds } from '@sentry/utils';

import { RN_GLOBAL_OBJ } from '../utils/worldwide';
Expand Down Expand Up @@ -114,6 +109,24 @@ export function setSpanDurationAsMeasurement(name: string, span: Span): void {
setMeasurement(name, (spanEnd - spanStart) * 1000, 'millisecond');
}

/**
* Sets the duration of the span as a measurement.
* Uses `setMeasurement` function from @sentry/core.
*/
export function setSpanDurationAsMeasurementOnTransaction(
transaction: TransactionType,
name: string,
span: Span,
): void {
const spanEnd = spanToJSON(span).timestamp;
const spanStart = spanToJSON(span).start_timestamp;
if (!spanEnd || !spanStart) {
return;
}

transaction.setMeasurement(name, (spanEnd - spanStart) * 1000, 'millisecond');
}

/**
* Returns unix timestamp in ms of the bundle start time.
*
Expand Down
73 changes: 73 additions & 0 deletions test/tracing/reactnavigation.ttid.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ jest.mock('../../src/js/utils/environment');
jest.mock('../../src/js/utils/sentryeventemitter', () => mockedSentryEventEmitter);
jest.mock('../../src/js/tracing/timetodisplaynative', () => mockedtimetodisplaynative);

import { Span } from '@sentry/core';
import type { SpanJSON, TransactionEvent, Transport } from '@sentry/types';
import { timestampInSeconds } from '@sentry/utils';
import React from "react";
Expand Down Expand Up @@ -84,6 +85,78 @@ describe('React Navigation - TTID', () => {
);
});

test('should end ttid with measurements even when active span was removed from the scope', () => {
jest.runOnlyPendingTimers(); // Flush app start transaction

mockedNavigation.navigateToNewScreen();
Sentry.getCurrentScope().setSpan(undefined);
mockedEventEmitter.emitNewFrameEvent();
jest.runOnlyPendingTimers(); // Flush transaction

const transaction = getLastTransaction(transportSendMock);
expect(transaction).toEqual(
expect.objectContaining<TransactionEvent>({
type: 'transaction',
spans: expect.arrayContaining([
expect.objectContaining<Partial<SpanJSON>>({
data: {
'sentry.op': 'ui.load.initial_display',
'sentry.origin': 'manual',
},
description: 'New Screen initial display',
op: 'ui.load.initial_display',
origin: 'manual',
status: 'ok',
start_timestamp: transaction.start_timestamp,
timestamp: expect.any(Number),
}),
]),
measurements: expect.objectContaining<Required<TransactionEvent>['measurements']>({
time_to_initial_display: {
value: expect.any(Number),
unit: 'millisecond',
},
}),
}),
);
});

test('should end ttid with measurements even when active span on the scope changed', () => {
jest.runOnlyPendingTimers(); // Flush app start transaction

mockedNavigation.navigateToNewScreen();
Sentry.getCurrentScope().setSpan(new Span());
mockedEventEmitter.emitNewFrameEvent();
jest.runOnlyPendingTimers(); // Flush transaction

const transaction = getLastTransaction(transportSendMock);
expect(transaction).toEqual(
expect.objectContaining<TransactionEvent>({
type: 'transaction',
spans: expect.arrayContaining([
expect.objectContaining<Partial<SpanJSON>>({
data: {
'sentry.op': 'ui.load.initial_display',
'sentry.origin': 'manual',
},
description: 'New Screen initial display',
op: 'ui.load.initial_display',
origin: 'manual',
status: 'ok',
start_timestamp: transaction.start_timestamp,
timestamp: expect.any(Number),
}),
]),
measurements: expect.objectContaining<Required<TransactionEvent>['measurements']>({
time_to_initial_display: {
value: expect.any(Number),
unit: 'millisecond',
},
}),
}),
);
});

test('should add ttid measurement', () => {
jest.runOnlyPendingTimers(); // Flush app start transaction

Expand Down

0 comments on commit cae8232

Please sign in to comment.