fix(sdk): Rename app start measurements to match other SDKs (#2855)

getsentry · Mar 2, 2023 · 982b55d · 982b55d
1 parent 2416f09
commit 982b55d
Show file tree

Hide file tree

Showing 7 changed files with 64 additions and 29 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -6,6 +6,12 @@
 
 - Add App Context `in_foreground` ([#2826](https://github.com/getsentry/sentry-react-native/pull/2826))
 
+### Fixes
+
+- Match app start measurements naming with other SDKs ([#2855](https://github.com/getsentry/sentry-react-native/pull/2855))
+  - `app.start.cold` to `app_start_cold`
+  - `app.start.warm` to `app_start_warm`
+
 ### Dependencies
 
 - Bump Cocoa SDK from v8.0.0 to v8.2.0 ([#2776](https://github.com/getsentry/sentry-react-native/pull/2776))

diff --git a/src/js/measurements.ts b/src/js/measurements.ts
@@ -3,9 +3,14 @@ import { getCurrentHub, getMainCarrier } from '@sentry/core';
 import type { Transaction } from '@sentry/tracing';
 import type { CustomSamplingContext, Span, SpanContext, TransactionContext } from '@sentry/types';
 
-import { ReactNativeTracing } from './tracing';
+import { DEFAULT,ReactNativeTracing } from './tracing';
 
-const SPAN_OP_DEFAULT = 'default';
+export const APP_START_WARM = 'app_start_warm';
+export const APP_START_COLD = 'app_start_cold';
+
+export const STALL_COUNT = 'stall_count';
+export const STALL_TOTAL_TIME = 'stall_total_time';
+export const STALL_LONGEST_TIME = 'stall_longest_time';
 
 /**
  * Adds React Native's extensions. Needs to be called after @sentry/tracing's extension methods are added
@@ -53,7 +58,7 @@ const _patchStartTransaction = (
   ): Transaction {
     // Native SDKs require op to be set - for JS Relay sets `default`
     if (!transactionContext.op) {
-      transactionContext.op = SPAN_OP_DEFAULT;
+      transactionContext.op = DEFAULT;
     }
 
     const transaction: Transaction = originalStartTransaction.apply(this, [
@@ -67,7 +72,7 @@ const _patchStartTransaction = (
       return originalStartChild({
         ...spanContext,
         // Native SDKs require op to be set
-        op: spanContext?.op || SPAN_OP_DEFAULT,
+        op: spanContext?.op || DEFAULT,
       });
     };
 

diff --git a/src/js/tracing/index.ts b/src/js/tracing/index.ts
@@ -20,3 +20,5 @@ export {
 } from './types';
 
 export { ReactNativeProfiler } from './reactnativeprofiler';
+
+export * from './ops';
diff --git a/src/js/tracing/ops.ts b/src/js/tracing/ops.ts
@@ -0,0 +1,7 @@
+
+export const DEFAULT = 'default';
+export const UI_LOAD = 'ui.load';
+export const NAVIGATION = 'navigation';
+
+export const APP_START_COLD = 'app.start.cold';
+export const APP_START_WARM = 'app.start.warm';
diff --git a/src/js/tracing/reactnativetracing.ts b/src/js/tracing/reactnativetracing.ts
@@ -18,10 +18,16 @@ import type {
 } from '@sentry/types';
 import { logger } from '@sentry/utils';
 
+import { APP_START_COLD, APP_START_WARM } from '../measurements';
 import type { NativeAppStartResponse } from '../NativeRNSentry';
 import type { RoutingInstrumentationInstance } from '../tracing/routingInstrumentation';
 import { NATIVE } from '../wrapper';
 import { NativeFramesInstrumentation } from './nativeframes';
+import {
+  APP_START_COLD as APP_START_COLD_OP,
+  APP_START_WARM as APP_START_WARM_OP,
+  UI_LOAD,
+} from './ops';
 import { StallTrackingInstrumentation } from './stalltracking';
 import type { BeforeNavigate, RouteChangeContextData } from './types';
 import {
@@ -291,7 +297,7 @@ export class ReactNativeTracing implements Integration {
 
       const idleTransaction = this._createRouteTransaction({
         name: 'App Start',
-        op: 'ui.load',
+        op: UI_LOAD,
         startTimestamp: appStartTimeSeconds,
       });
 
@@ -315,10 +321,10 @@ export class ReactNativeTracing implements Integration {
 
     const appStartTimeSeconds = appStart.appStartTime / 1000;
 
-    const appStartMode = appStart.isColdStart ? 'app.start.cold' : 'app.start.warm';
+    const op = appStart.isColdStart ? APP_START_COLD_OP : APP_START_WARM_OP;
     transaction.startChild({
       description: appStart.isColdStart ? 'Cold App Start' : 'Warm App Start',
-      op: appStartMode,
+      op,
       startTimestamp: appStartTimeSeconds,
       endTimestamp: this._appStartFinishTimestamp,
     });
@@ -333,7 +339,8 @@ export class ReactNativeTracing implements Integration {
       return;
     }
 
-    transaction.setMeasurement(appStartMode, appStartDurationMilliseconds, 'millisecond');
+    const measurement = appStart.isColdStart ? APP_START_COLD : APP_START_WARM;
+    transaction.setMeasurement(measurement, appStartDurationMilliseconds, 'millisecond');
   }
 
   /** To be called when the route changes, but BEFORE the components of the new route mount. */

diff --git a/src/js/tracing/stalltracking.ts b/src/js/tracing/stalltracking.ts
@@ -3,10 +3,12 @@ import type { IdleTransaction, Span, Transaction } from '@sentry/tracing';
 import type { Measurements, MeasurementUnit } from '@sentry/types';
 import { logger, timestampInSeconds } from '@sentry/utils';
 
+import { STALL_COUNT, STALL_LONGEST_TIME, STALL_TOTAL_TIME } from '../measurements';
+
 export interface StallMeasurements extends Measurements {
-  'stall_count': { value: number, unit: MeasurementUnit };
-  'stall_total_time': { value: number, unit: MeasurementUnit };
-  'stall_longest_time': { value: number, unit: MeasurementUnit };
+  [STALL_COUNT]: { value: number, unit: MeasurementUnit };
+  [STALL_TOTAL_TIME]: { value: number, unit: MeasurementUnit };
+  [STALL_LONGEST_TIME]: { value: number, unit: MeasurementUnit };
 }
 
 export type StallTrackingOptions = {
@@ -211,17 +213,17 @@ export class StallTrackingInstrumentation {
       return;
     }
 
-    transaction.setMeasurement('stall_count',
+    transaction.setMeasurement(STALL_COUNT,
       statsOnFinish.stall_count.value -
       transactionStats.atStart.stall_count.value,
       transactionStats.atStart.stall_count.unit);
 
-    transaction.setMeasurement('stall_total_time',
+    transaction.setMeasurement(STALL_TOTAL_TIME,
       statsOnFinish.stall_total_time.value -
       transactionStats.atStart.stall_total_time.value,
       transactionStats.atStart.stall_total_time.unit);
 
-    transaction.setMeasurement('stall_longest_time',
+    transaction.setMeasurement(STALL_LONGEST_TIME,
       statsOnFinish.stall_longest_time.value,
       statsOnFinish.stall_longest_time.unit);
   }

diff --git a/test/tracing/reactnativetracing.test.ts b/test/tracing/reactnativetracing.test.ts
@@ -57,6 +57,12 @@ const getMockHub = () => {
 
 import type { BrowserClientOptions } from '@sentry/browser/types/client';
 
+import { APP_START_COLD, APP_START_WARM } from '../../src/js/measurements';
+import {
+  APP_START_COLD as APP_START_COLD_OP,
+  APP_START_WARM as APP_START_WARM_OP,
+  UI_LOAD,
+} from '../../src/js/tracing';
 import { ReactNativeTracing } from '../../src/js/tracing/reactnativetracing';
 import { getTimeOriginMilliseconds } from '../../src/js/tracing/utils';
 import { NATIVE } from '../../src/js/wrapper';
@@ -110,15 +116,15 @@ describe('ReactNativeTracing', () => {
             expect(transaction.startTimestamp).toBe(
               appStartTimeMilliseconds / 1000
             );
-            expect(transaction.op).toBe('ui.load');
+            expect(transaction.op).toBe(UI_LOAD);
 
             expect(
               // @ts-ignore access private for test
-              transaction._measurements['app.start.cold'].value
+              transaction._measurements[APP_START_COLD].value
             ).toEqual(timeOriginMilliseconds - appStartTimeMilliseconds);
             expect(
               // @ts-ignore access private for test
-              transaction._measurements['app.start.cold'].unit).toBe('millisecond');
+              transaction._measurements[APP_START_COLD].unit).toBe('millisecond');
 
             done();
           }
@@ -157,15 +163,15 @@ describe('ReactNativeTracing', () => {
             expect(transaction.startTimestamp).toBe(
               appStartTimeMilliseconds / 1000
             );
-            expect(transaction.op).toBe('ui.load');
+            expect(transaction.op).toBe(UI_LOAD);
 
             expect(
               // @ts-ignore access private for test
-              transaction._measurements['app.start.warm'].value
+              transaction._measurements[APP_START_WARM].value
             ).toEqual(timeOriginMilliseconds - appStartTimeMilliseconds);
             expect(
               // @ts-ignore access private for test
-              transaction._measurements['app.start.warm'].unit).toBe('millisecond');
+              transaction._measurements[APP_START_WARM].unit).toBe('millisecond');
 
             done();
           }
@@ -203,12 +209,12 @@ describe('ReactNativeTracing', () => {
           if (transaction) {
             expect(
               // @ts-ignore access private for test
-              transaction._measurements['app.start.warm']
+              transaction._measurements[APP_START_WARM]
             ).toBeUndefined();
 
             expect(
               // @ts-ignore access private for test
-              transaction._measurements['app.start.cold']
+              transaction._measurements[APP_START_COLD]
             ).toBeUndefined();
 
             done();
@@ -301,11 +307,11 @@ describe('ReactNativeTracing', () => {
         jest.advanceTimersByTime(DEFAULT_IDLE_TIMEOUT);
 
         // @ts-ignore access private for test
-        expect(routeTransaction._measurements['app.start.cold'].value).toBe(
+        expect(routeTransaction._measurements[APP_START_COLD].value).toBe(
           timeOriginMilliseconds - appStartTimeMilliseconds
         );
 
-        expect(routeTransaction.op).toBe('ui.load');
+        expect(routeTransaction.op).toBe(UI_LOAD);
         expect(routeTransaction.startTimestamp).toBe(
           appStartTimeMilliseconds / 1000
         );
@@ -316,7 +322,7 @@ describe('ReactNativeTracing', () => {
 
         const span = spanRecorder?.spans[spanRecorder?.spans.length - 1];
 
-        expect(span?.op).toBe('app.start.cold');
+        expect(span?.op).toBe(APP_START_COLD_OP);
         expect(span?.description).toBe('Cold App Start');
         expect(span?.startTimestamp).toBe(appStartTimeMilliseconds / 1000);
         expect(span?.endTimestamp).toBe(timeOriginMilliseconds / 1000);
@@ -364,11 +370,11 @@ describe('ReactNativeTracing', () => {
         jest.advanceTimersByTime(DEFAULT_IDLE_TIMEOUT);
 
         // @ts-ignore access private for test
-        expect(routeTransaction._measurements['app.start.warm'].value).toBe(
+        expect(routeTransaction._measurements[APP_START_WARM].value).toBe(
           timeOriginMilliseconds - appStartTimeMilliseconds
         );
 
-        expect(routeTransaction.op).toBe('ui.load');
+        expect(routeTransaction.op).toBe(UI_LOAD);
         expect(routeTransaction.startTimestamp).toBe(
           appStartTimeMilliseconds / 1000
         );
@@ -379,7 +385,7 @@ describe('ReactNativeTracing', () => {
 
         const span = spanRecorder?.spans[spanRecorder?.spans.length - 1];
 
-        expect(span?.op).toBe('app.start.warm');
+        expect(span?.op).toBe(APP_START_WARM_OP);
         expect(span?.description).toBe('Warm App Start');
         expect(span?.startTimestamp).toBe(appStartTimeMilliseconds / 1000);
         expect(span?.endTimestamp).toBe(timeOriginMilliseconds / 1000);
@@ -429,7 +435,7 @@ describe('ReactNativeTracing', () => {
         // @ts-ignore access private for test
         expect(routeTransaction._measurements).toMatchObject({});
 
-        expect(routeTransaction.op).not.toBe('ui.load');
+        expect(routeTransaction.op).not.toBe(UI_LOAD);
         expect(routeTransaction.startTimestamp).not.toBe(
           appStartTimeMilliseconds / 1000
         );