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
30 changes: 16 additions & 14 deletions src/js/tracing/nativeframes.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,12 @@ export interface FramesMeasurements extends Measurements {
frames_frozen: { value: number; unit: MeasurementUnit };
}

/** The listeners for each native frames response, keyed by traceId. This must be global to avoid closure issues and reading outdated values. */
const _framesListeners: Map<string, () => void> = new Map();

/** The native frames at the transaction finish time, keyed by traceId. This must be global to avoid closure issues and reading outdated values. */
const _finishFrames: Map<string, { timestamp: number; nativeFrames: NativeFramesResponse | null }> = new Map();

/**
* A margin of error of 50ms is allowed for the async native bridge call.
* Anything larger would reduce the accuracy of our frames measurements.
Expand All @@ -22,10 +28,6 @@ const MARGIN_OF_ERROR_SECONDS = 0.05;
* Instrumentation to add native slow/frozen frames measurements onto transactions.
*/
export class NativeFramesInstrumentation {
/** The native frames at the transaction finish time, keyed by traceId. */
private _finishFrames: Map<string, { timestamp: number; nativeFrames: NativeFramesResponse | null }> = new Map();
/** The listeners for each native frames response, keyed by traceId */
private _framesListeners: Map<string, () => void> = new Map();
/** The native frames at the finish time of the most recent span. */
private _lastSpanFinishFrames?: {
timestamp: number;
Expand Down Expand Up @@ -98,22 +100,22 @@ export class NativeFramesInstrumentation {
finalEndTimestamp: number,
startFrames: NativeFramesResponse,
): Promise<FramesMeasurements | null> {
if (this._finishFrames.has(traceId)) {
if (_finishFrames.has(traceId)) {
return this._prepareMeasurements(traceId, finalEndTimestamp, startFrames);
}

return new Promise(resolve => {
const timeout = setTimeout(() => {
this._framesListeners.delete(traceId);
_framesListeners.delete(traceId);

resolve(null);
}, 2000);

this._framesListeners.set(traceId, () => {
_framesListeners.set(traceId, () => {
resolve(this._prepareMeasurements(traceId, finalEndTimestamp, startFrames));

clearTimeout(timeout);
this._framesListeners.delete(traceId);
_framesListeners.delete(traceId);
});
});
}
Expand All @@ -128,7 +130,7 @@ export class NativeFramesInstrumentation {
): FramesMeasurements | null {
let finalFinishFrames: NativeFramesResponse | undefined;

const finish = this._finishFrames.get(traceId);
const finish = _finishFrames.get(traceId);
if (
finish &&
finish.nativeFrames &&
Expand Down Expand Up @@ -178,12 +180,12 @@ export class NativeFramesInstrumentation {
finishFrames = await NATIVE.fetchNativeFrames();
}

this._finishFrames.set(transaction.traceId, {
_finishFrames.set(transaction.traceId, {
nativeFrames: finishFrames,
timestamp,
});

this._framesListeners.get(transaction.traceId)?.();
_framesListeners.get(transaction.traceId)?.();

setTimeout(() => this._cancelFinishFrames(transaction), 2000);
}
Expand All @@ -192,8 +194,8 @@ export class NativeFramesInstrumentation {
* On a finish frames failure, we cancel the await.
*/
private _cancelFinishFrames(transaction: Transaction): void {
if (this._finishFrames.has(transaction.traceId)) {
this._finishFrames.delete(transaction.traceId);
if (_finishFrames.has(transaction.traceId)) {
_finishFrames.delete(transaction.traceId);

logger.log(
`[NativeFrames] Native frames timed out for ${transaction.op} transaction ${transaction.name}. Not adding native frames measurements.`,
Expand Down Expand Up @@ -243,7 +245,7 @@ export class NativeFramesInstrumentation {
...measurements,
};

this._finishFrames.delete(traceId);
_finishFrames.delete(traceId);
}

delete traceContext.data.__startFrames;
Expand Down
26 changes: 25 additions & 1 deletion test/mocks/client.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,12 @@
import { BaseClient, createTransport, initAndBind } from '@sentry/core';
import {
BaseClient,
createTransport,
getCurrentScope,
getGlobalScope,
getIsolationScope,
initAndBind,
setCurrentClient,
} from '@sentry/core';
import type {
ClientOptions,
Event,
Expand All @@ -11,6 +19,8 @@ import type {
} from '@sentry/types';
import { resolvedSyncPromise } from '@sentry/utils';

import { _addTracingExtensions } from '../../src/js/tracing/addTracingExtensions';

export function getDefaultTestClientOptions(options: Partial<TestClientOptions> = {}): TestClientOptions {
return {
dsn: 'https://1234@some-domain.com/4505526893805568',
Expand Down Expand Up @@ -103,3 +113,17 @@ export class TestClient extends BaseClient<TestClientOptions> {
export function init(options: TestClientOptions): void {
initAndBind(TestClient, options);
}

export function setupTestClient(options: Partial<TestClientOptions> = {}): TestClient {
_addTracingExtensions();

getCurrentScope().clear();
getIsolationScope().clear();
getGlobalScope().clear();

const finalOptions = getDefaultTestClientOptions({ tracesSampleRate: 1.0, ...options });
const client = new TestClient(finalOptions);
setCurrentClient(client);
client.init();
return client;
}
79 changes: 50 additions & 29 deletions test/tracing/addTracingExtensions.test.ts
Original file line number Diff line number Diff line change
@@ -1,58 +1,79 @@
import type { Carrier, Transaction } from '@sentry/core';
import { getCurrentHub, getMainCarrier } from '@sentry/core';
import type { Hub } from '@sentry/types';
import { getCurrentScope, spanToJSON, startSpanManual } from '@sentry/core';

import type { StartTransactionFunction } from '../../src/js/tracing/addTracingExtensions';
import { _addTracingExtensions } from '../../src/js/tracing/addTracingExtensions';
import { ReactNativeTracing } from '../../src/js';
import { type TestClient, setupTestClient } from '../mocks/client';

describe('Tracing extensions', () => {
let hub: Hub;
let carrier: Carrier;
let startTransaction: StartTransactionFunction | undefined;
let client: TestClient;

beforeEach(() => {
_addTracingExtensions();
hub = getCurrentHub();
carrier = getMainCarrier();
startTransaction = carrier.__SENTRY__?.extensions?.startTransaction as StartTransactionFunction | undefined;
client = setupTestClient({
integrations: [new ReactNativeTracing()],
});
});

test('transaction has default op', async () => {
const transaction: Transaction = startTransaction?.apply(hub, [{}]);
const transaction = startSpanManual({ name: 'parent' }, span => span);

expect(transaction).toEqual(expect.objectContaining({ op: 'default' }));
expect(spanToJSON(transaction!)).toEqual(
expect.objectContaining({
op: 'default',
}),
);
});

test('transaction does not overwrite custom op', async () => {
const transaction: Transaction = startTransaction?.apply(hub, [{ op: 'custom' }]);
const transaction = startSpanManual({ name: 'parent', op: 'custom' }, span => span);

expect(transaction).toEqual(expect.objectContaining({ op: 'custom' }));
expect(spanToJSON(transaction!)).toEqual(
expect.objectContaining({
op: 'custom',
}),
);
});

test('transaction start span creates default op', async () => {
const transaction: Transaction = startTransaction?.apply(hub, [{ op: 'custom' }]);
const span = transaction?.startChild();
// TODO: add event listener to spanStart and add default op if not set
startSpanManual({ name: 'parent', scope: getCurrentScope() }, () => {});
const span = startSpanManual({ name: 'child', scope: getCurrentScope() }, span => span);

expect(span).toEqual(expect.objectContaining({ op: 'default' }));
expect(spanToJSON(span!)).toEqual(
expect.objectContaining({
op: 'default',
}),
);
});

test('transaction start span keeps custom op', async () => {
const transaction: Transaction = startTransaction?.apply(hub, [{ op: 'custom' }]);
const span = transaction?.startChild({ op: 'custom' });
startSpanManual({ name: 'parent', op: 'custom', scope: getCurrentScope() }, () => {});
const span = startSpanManual({ name: 'child', op: 'custom', scope: getCurrentScope() }, span => span);

expect(span).toEqual(expect.objectContaining({ op: 'custom' }));
expect(spanToJSON(span!)).toEqual(
expect.objectContaining({
op: 'custom',
}),
);
});

test('transaction start span passes correct values to the child', async () => {
const transaction: Transaction = startTransaction?.apply(hub, [{ op: 'custom' }]);
const span = transaction?.startChild({ op: 'custom' });
const transaction = startSpanManual({ name: 'parent', op: 'custom', scope: getCurrentScope() }, span => span);
const span = startSpanManual({ name: 'child', scope: getCurrentScope() }, span => span);
span!.end();
transaction!.end();

expect(span).toEqual(
await client.flush();
expect(client.event).toEqual(
expect.objectContaining({
contexts: expect.objectContaining({
trace: expect.objectContaining({
trace_id: transaction!.spanContext().traceId,
}),
}),
}),
);
expect(spanToJSON(span!)).toEqual(
expect.objectContaining({
transaction,
parentSpanId: transaction.spanId,
sampled: transaction.sampled,
traceId: transaction.traceId,
parent_span_id: transaction!.spanContext().spanId,
}),
);
});
Expand Down
Loading