Add flag to disable native profilers (#4094)

getsentry · Sep 17, 2024 · 9a727cb · 9a727cb
1 parent 4f3eb7e
commit 9a727cb
Show file tree

Hide file tree

Showing 11 changed files with 97 additions and 31 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -1,5 +1,23 @@
 # Changelog
 
+## Unreleased
+
+### Features
+
+- Add an option to disable native (iOS and Android) profiling for the `HermesProfiling` integration ([#4094](https://github.com/getsentry/sentry-react-native/pull/4094))
+
+  To disable native profilers add the `hermesProfilingIntegration`.
+
+  ```js
+  import * as Sentry from '@sentry/react-native';
+
+  Sentry.init({
+    integrations: [
+      Sentry.hermesProfilingIntegration({ platformProfilers: false }),
+    ],
+  });
+  ```
+
 ## 5.32.0
 
 ### Features

diff --git a/android/src/main/java/io/sentry/react/RNSentryModuleImpl.java b/android/src/main/java/io/sentry/react/RNSentryModuleImpl.java
@@ -718,15 +718,17 @@ private void initializeAndroidProfiler() {
         );
     }
 
-    public WritableMap startProfiling() {
+    public WritableMap startProfiling(boolean platformProfilers) {
         final WritableMap result = new WritableNativeMap();
-        if (androidProfiler == null) {
+        if (androidProfiler == null && platformProfilers) {
             initializeAndroidProfiler();
         }
 
         try {
             HermesSamplingProfiler.enable();
-            androidProfiler.start();
+            if (androidProfiler != null) {
+                androidProfiler.start();
+            }
 
             result.putBoolean("started", true);
         } catch (Throwable e) {
@@ -741,7 +743,10 @@ public WritableMap stopProfiling() {
         final WritableMap result = new WritableNativeMap();
         File output = null;
         try {
-            AndroidProfiler.ProfileEndData end = androidProfiler.endAndCollect(false, null);
+            AndroidProfiler.ProfileEndData end = null;
+            if (androidProfiler != null) {
+                end = androidProfiler.endAndCollect(false, null);
+            }
             HermesSamplingProfiler.disable();
 
             output = File.createTempFile(
@@ -753,14 +758,16 @@ public WritableMap stopProfiling() {
             HermesSamplingProfiler.dumpSampledTraceToFile(output.getPath());
             result.putString("profile", readStringFromFile(output));
 
-            WritableMap androidProfile = new WritableNativeMap();
-            byte[] androidProfileBytes = FileUtils.readBytesFromFile(end.traceFile.getPath(), maxTraceFileSize);
-            String base64AndroidProfile = Base64.encodeToString(androidProfileBytes, NO_WRAP | NO_PADDING);
+            if (end != null) {
+                WritableMap androidProfile = new WritableNativeMap();
+                byte[] androidProfileBytes = FileUtils.readBytesFromFile(end.traceFile.getPath(), maxTraceFileSize);
+                String base64AndroidProfile = Base64.encodeToString(androidProfileBytes, NO_WRAP | NO_PADDING);
 
-            androidProfile.putString("sampled_profile", base64AndroidProfile);
-            androidProfile.putInt("android_api_level", buildInfo.getSdkInfoVersion());
-            androidProfile.putString("build_id", getProguardUuid());
-            result.putMap("androidProfile", androidProfile);
+                androidProfile.putString("sampled_profile", base64AndroidProfile);
+                androidProfile.putInt("android_api_level", buildInfo.getSdkInfoVersion());
+                androidProfile.putString("build_id", getProguardUuid());
+                result.putMap("androidProfile", androidProfile);
+            }
         } catch (Throwable e) {
             result.putString("error", e.toString());
         } finally {

diff --git a/android/src/newarch/java/io/sentry/react/RNSentryModule.java b/android/src/newarch/java/io/sentry/react/RNSentryModule.java
@@ -139,8 +139,8 @@ public void fetchNativeSdkInfo(Promise promise) {
     }
 
     @Override
-    public WritableMap startProfiling() {
-        return this.impl.startProfiling();
+    public WritableMap startProfiling(boolean platformProfilers) {
+        return this.impl.startProfiling(platformProfilers);
     }
 
     @Override

diff --git a/android/src/oldarch/java/io/sentry/react/RNSentryModule.java b/android/src/oldarch/java/io/sentry/react/RNSentryModule.java
@@ -139,8 +139,8 @@ public void fetchNativeSdkInfo(Promise promise) {
     }
 
     @ReactMethod(isBlockingSynchronousMethod = true)
-    public WritableMap startProfiling() {
-        return this.impl.startProfiling();
+    public WritableMap startProfiling(boolean platformProfilers) {
+        return this.impl.startProfiling(platformProfilers);
     }
 
     @ReactMethod(isBlockingSynchronousMethod = true)

diff --git a/ios/RNSentry.mm b/ios/RNSentry.mm
@@ -649,18 +649,22 @@ - (NSDictionary*) fetchNativeStackFramesBy: (NSArray<NSNumber*>*)instructionsAdd
 static SentryId* nativeProfileTraceId = nil;
 static uint64_t nativeProfileStartTime = 0;
 
-RCT_EXPORT_SYNCHRONOUS_TYPED_METHOD(NSDictionary *, startProfiling)
+RCT_EXPORT_SYNCHRONOUS_TYPED_METHOD(NSDictionary *, startProfiling: (BOOL)platformProfilers)
 {
 #if SENTRY_PROFILING_ENABLED
     try {
         facebook::hermes::HermesRuntime::enableSamplingProfiler();
-        if (nativeProfileTraceId == nil && nativeProfileStartTime == 0) {
+        if (nativeProfileTraceId == nil && nativeProfileStartTime == 0 && platformProfilers) {
 #if SENTRY_TARGET_PROFILING_SUPPORTED
             nativeProfileTraceId = [RNSentryId newId];
             nativeProfileStartTime = [PrivateSentrySDKOnly startProfilerForTrace: nativeProfileTraceId];
 #endif
         } else {
-            NSLog(@"Native profiling already in progress. Currently existing trace: %@", nativeProfileTraceId);
+            if (!platformProfilers) {
+                NSLog(@"Native profiling is disabled. Only starting Hermes profiling.");
+            } else {
+                NSLog(@"Native profiling already in progress. Currently existing trace: %@", nativeProfileTraceId);
+            }
         }
         return @{ @"started": @YES };
     } catch (const std::exception& ex) {

diff --git a/samples/react-native/ios/sentryreactnativesample.xcodeproj/project.pbxproj b/samples/react-native/ios/sentryreactnativesample.xcodeproj/project.pbxproj
@@ -632,13 +632,14 @@
 				ONLY_ACTIVE_ARCH = YES;
 				OTHER_CFLAGS = (
 					"$(inherited)",
-					" ",
+					"-DRN_FABRIC_ENABLED",
 				);
 				OTHER_CPLUSPLUSFLAGS = (
 					"$(OTHER_CFLAGS)",
 					"-DFOLLY_NO_CONFIG",
 					"-DFOLLY_MOBILE=1",
 					"-DFOLLY_USE_LIBCPP=1",
+					"-DRN_FABRIC_ENABLED",
 				);
 				OTHER_LDFLAGS = "$(inherited)";
 				REACT_NATIVE_PATH = "${PODS_ROOT}/../../node_modules/react-native";
@@ -714,13 +715,14 @@
 				MTL_ENABLE_DEBUG_INFO = NO;
 				OTHER_CFLAGS = (
 					"$(inherited)",
-					" ",
+					"-DRN_FABRIC_ENABLED",
 				);
 				OTHER_CPLUSPLUSFLAGS = (
 					"$(OTHER_CFLAGS)",
 					"-DFOLLY_NO_CONFIG",
 					"-DFOLLY_MOBILE=1",
 					"-DFOLLY_USE_LIBCPP=1",
+					"-DRN_FABRIC_ENABLED",
 				);
 				OTHER_LDFLAGS = "$(inherited)";
 				REACT_NATIVE_PATH = "${PODS_ROOT}/../../node_modules/react-native";

diff --git a/src/js/NativeRNSentry.ts b/src/js/NativeRNSentry.ts
@@ -34,7 +34,7 @@ export interface Spec extends TurboModule {
   enableNativeFramesTracking(): void;
   fetchModules(): Promise<string | undefined | null>;
   fetchViewHierarchy(): Promise<number[] | undefined | null>;
-  startProfiling(): { started?: boolean; error?: string };
+  startProfiling(platformProfilers: boolean): { started?: boolean; error?: string };
   stopProfiling(): {
     profile?: string;
     nativeProfile?: UnsafeObject;

diff --git a/src/js/profiling/integration.ts b/src/js/profiling/integration.ts
@@ -5,7 +5,7 @@ import type {
   Event,
   Integration,
   IntegrationClass,
-  IntegrationFn,
+  IntegrationFnResult,
   ThreadCpuProfile,
   Transaction,
 } from '@sentry/types';
@@ -31,19 +31,35 @@ const INTEGRATION_NAME = 'HermesProfiling';
 
 const MS_TO_NS: number = 1e6;
 
+export interface HermesProfilingOptions {
+  /**
+   * Enable or disable profiling of native (iOS and Android) threads
+   *
+   * @default true
+   */
+  platformProfilers?: boolean;
+}
+
+const defaultOptions: Required<HermesProfilingOptions> = {
+  platformProfilers: true,
+};
+
 /**
  * Profiling integration creates a profile for each transaction and adds it to the event envelope.
  *
  * @experimental
  */
-export const hermesProfilingIntegration: IntegrationFn = () => {
+export const hermesProfilingIntegration = (
+  initOptions: HermesProfilingOptions = defaultOptions,
+): IntegrationFnResult => {
   let _currentProfile:
     | {
         profile_id: string;
         startTimestampNs: number;
       }
     | undefined;
   let _currentProfileTimeout: number | undefined;
+  const usePlatformProfilers = initOptions.platformProfilers ?? true;
 
   const setupOnce = (): void => {
     if (!isHermesEnabled()) {
@@ -138,7 +154,7 @@ export const hermesProfilingIntegration: IntegrationFn = () => {
    * Starts a new profile and links it to the transaction.
    */
   const _startNewProfile = (transaction: Transaction): void => {
-    const profileStartTimestampNs = startProfiling();
+    const profileStartTimestampNs = startProfiling(usePlatformProfilers);
     if (!profileStartTimestampNs) {
       return;
     }
@@ -227,8 +243,8 @@ export const HermesProfiling = convertIntegrationFnToClass(
 /**
  * Starts Profilers and returns the timestamp when profiling started in nanoseconds.
  */
-export function startProfiling(): number | null {
-  const started = NATIVE.startProfiling();
+export function startProfiling(platformProfilers: boolean): number | null {
+  const started = NATIVE.startProfiling(platformProfilers);
   if (!started) {
     return null;
   }

diff --git a/src/js/wrapper.ts b/src/js/wrapper.ts
@@ -95,7 +95,7 @@ interface SentryNativeWrapper {
   fetchModules(): Promise<Record<string, string> | null>;
   fetchViewHierarchy(): PromiseLike<Uint8Array | null>;
 
-  startProfiling(): boolean;
+  startProfiling(platformProfilers: boolean): boolean;
   stopProfiling(): {
     hermesProfile: Hermes.Profile;
     nativeProfile?: NativeProfileEvent;
@@ -531,15 +531,15 @@ export const NATIVE: SentryNativeWrapper = {
     return raw ? new Uint8Array(raw) : null;
   },
 
-  startProfiling(): boolean {
+  startProfiling(platformProfilers: boolean): boolean {
     if (!this.enableNative) {
       throw this._DisabledNativeError;
     }
     if (!this._isModuleLoaded(RNSentry)) {
       throw this._NativeClientError;
     }
 
-    const { started, error } = RNSentry.startProfiling();
+    const { started, error } = RNSentry.startProfiling(platformProfilers);
     if (started) {
       logger.log('[NATIVE] Start Profiling');
     } else {

diff --git a/test/profiling/integration.test.ts b/test/profiling/integration.test.ts
@@ -9,6 +9,7 @@ import type { Envelope, Event, Profile, ThreadCpuProfile, Transaction, Transport
 import * as Sentry from '../../src/js';
 import type { NativeDeviceContextsResponse } from '../../src/js/NativeRNSentry';
 import { getDebugMetadata } from '../../src/js/profiling/debugid';
+import type { HermesProfilingOptions } from '../../src/js/profiling/integration';
 import { hermesProfilingIntegration } from '../../src/js/profiling/integration';
 import type { AndroidProfileEvent } from '../../src/js/profiling/types';
 import { getDefaultEnvironment, isHermesEnabled, notWeb } from '../../src/js/utils/environment';
@@ -351,12 +352,24 @@ describe('profiling integration', () => {
       jest.runAllTimers();
     });
   });
+
+  test('platformProviders flag passed down to native', () => {
+    mock = initTestClient({ withProfiling: true, hermesProfilingOptions: { platformProfilers: false } });
+    const transaction: Transaction = Sentry.startTransaction({
+      name: 'test-name',
+    });
+    transaction.finish();
+    jest.runAllTimers();
+
+    expect(mockWrapper.NATIVE.startProfiling).toBeCalledWith(false);
+  });
 });
 
 function initTestClient(
   testOptions: {
     withProfiling?: boolean;
     environment?: string;
+    hermesProfilingOptions?: HermesProfilingOptions;
   } = {
     withProfiling: true,
   },
@@ -372,6 +385,12 @@ function initTestClient(
       if (!testOptions.withProfiling) {
         return integrations.filter(i => i.name !== 'HermesProfiling');
       }
+      return integrations.map(integration => {
+        if (integration.name === 'HermesProfiling') {
+          return hermesProfilingIntegration(testOptions.hermesProfilingOptions ?? {});
+        }
+        return integration;
+      });
       return integrations;
     },
     transport: () => ({

diff --git a/test/wrapper.test.ts b/test/wrapper.test.ts
@@ -574,13 +574,13 @@ describe('Tests Native Wrapper', () => {
       (RNSentry.startProfiling as jest.MockedFunction<typeof RNSentry.startProfiling>).mockReturnValue({
         started: true,
       });
-      expect(NATIVE.startProfiling()).toBe(true);
+      expect(NATIVE.startProfiling(true)).toBe(true);
     });
     test('failed start profiling returns false', () => {
       (RNSentry.startProfiling as jest.MockedFunction<typeof RNSentry.startProfiling>).mockReturnValue({
         error: 'error',
       });
-      expect(NATIVE.startProfiling()).toBe(false);
+      expect(NATIVE.startProfiling(true)).toBe(false);
     });
     test('stop profiling returns hermes profile', () => {
       (RNSentry.stopProfiling as jest.MockedFunction<typeof RNSentry.stopProfiling>).mockReturnValue({