test(tracing): Update performance tests for JS Core V8 (#3807)

getsentry · May 13, 2024 · 8df8f60 · 8df8f60
1 parent 9eddef8
commit 8df8f60
Show file tree

Hide file tree

Showing 12 changed files with 1,269 additions and 1,544 deletions.
diff --git a/src/js/tracing/nativeframes.ts b/src/js/tracing/nativeframes.ts
@@ -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.
@@ -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;
@@ -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);
       });
     });
   }
@@ -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 &&
@@ -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);
   }
@@ -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.`,
@@ -243,7 +245,7 @@ export class NativeFramesInstrumentation {
             ...measurements,
           };
 
-          this._finishFrames.delete(traceId);
+          _finishFrames.delete(traceId);
         }
 
         delete traceContext.data.__startFrames;

diff --git a/test/mocks/client.ts b/test/mocks/client.ts
@@ -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,
@@ -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',
@@ -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;
+}
diff --git a/test/tracing/addTracingExtensions.test.ts b/test/tracing/addTracingExtensions.test.ts
@@ -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,
       }),
     );
   });