From 149a02ebf55699c39de4e8ddcbce9966c43339cd Mon Sep 17 00:00:00 2001 From: Roman Zavarnitsyn Date: Tue, 17 Sep 2024 14:23:09 +0200 Subject: [PATCH 1/5] Add flag to disable native profilers (#4094) --- CHANGELOG.md | 18 ++++++++++++ .../io/sentry/react/RNSentryModuleImpl.java | 29 ++++++++++++------- .../java/io/sentry/react/RNSentryModule.java | 4 +-- .../java/io/sentry/react/RNSentryModule.java | 4 +-- ios/RNSentry.mm | 10 +++++-- .../project.pbxproj | 2 ++ src/js/NativeRNSentry.ts | 2 +- src/js/profiling/integration.ts | 26 +++++++++++++---- src/js/wrapper.ts | 6 ++-- test/profiling/integration.test.ts | 19 ++++++++++++ test/wrapper.test.ts | 4 +-- 11 files changed, 95 insertions(+), 29 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index c2e7f8902a..cf04722700 100644 --- 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.24.1 ### Fixes diff --git a/android/src/main/java/io/sentry/react/RNSentryModuleImpl.java b/android/src/main/java/io/sentry/react/RNSentryModuleImpl.java index b73bf6a753..d715a40b5c 100644 --- a/android/src/main/java/io/sentry/react/RNSentryModuleImpl.java +++ b/android/src/main/java/io/sentry/react/RNSentryModuleImpl.java @@ -695,15 +695,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) { @@ -718,7 +720,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( @@ -730,14 +735,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 index 78dfa4fa58..eed8a0e064 100644 --- 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 index 1a11e85711..67c269af6a 100644 --- 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 index 8c774ddbb7..ddedbf45ea 100644 --- a/ios/RNSentry.mm +++ b/ios/RNSentry.mm @@ -614,18 +614,22 @@ - (NSDictionary*) fetchNativeStackFramesBy: (NSArray*)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 index b6a08b9eef..c0e1cee404 100644 --- a/samples/react-native/ios/sentryreactnativesample.xcodeproj/project.pbxproj +++ b/samples/react-native/ios/sentryreactnativesample.xcodeproj/project.pbxproj @@ -632,6 +632,7 @@ "-DFOLLY_NO_CONFIG", "-DFOLLY_MOBILE=1", "-DFOLLY_USE_LIBCPP=1", + "-DRN_FABRIC_ENABLED", ); OTHER_LDFLAGS = ( "$(inherited)", @@ -715,6 +716,7 @@ "-DFOLLY_NO_CONFIG", "-DFOLLY_MOBILE=1", "-DFOLLY_USE_LIBCPP=1", + "-DRN_FABRIC_ENABLED", ); OTHER_LDFLAGS = ( "$(inherited)", diff --git a/src/js/NativeRNSentry.ts b/src/js/NativeRNSentry.ts index 3af21faae6..058ba6d179 100644 --- a/src/js/NativeRNSentry.ts +++ b/src/js/NativeRNSentry.ts @@ -34,7 +34,7 @@ export interface Spec extends TurboModule { enableNativeFramesTracking(): void; fetchModules(): Promise; fetchViewHierarchy(): Promise; - 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 index 8fb6b7bcef..5cf0ab69eb 100644 --- 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'; @@ -30,12 +30,27 @@ 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 = { + 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; @@ -43,6 +58,7 @@ export const hermesProfilingIntegration: IntegrationFn = () => { } | undefined; let _currentProfileTimeout: number | undefined; + const usePlatformProfilers = initOptions.platformProfilers ?? true; const setupOnce = (): void => { if (!isHermesEnabled()) { @@ -134,7 +150,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; } @@ -223,8 +239,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 index 1d27a9d056..f4ec8fc1be 100644 --- a/src/js/wrapper.ts +++ b/src/js/wrapper.ts @@ -90,7 +90,7 @@ interface SentryNativeWrapper { fetchModules(): Promise | null>; fetchViewHierarchy(): PromiseLike; - startProfiling(): boolean; + startProfiling(platformProfilers: boolean): boolean; stopProfiling(): { hermesProfile: Hermes.Profile; nativeProfile?: NativeProfileEvent; @@ -521,7 +521,7 @@ export const NATIVE: SentryNativeWrapper = { return raw ? new Uint8Array(raw) : null; }, - startProfiling(): boolean { + startProfiling(platformProfilers: boolean): boolean { if (!this.enableNative) { throw this._DisabledNativeError; } @@ -529,7 +529,7 @@ export const NATIVE: SentryNativeWrapper = { 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 index b9acc58c3d..e7f165266c 100644 --- 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, }, @@ -374,6 +387,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 index 11ecc8065b..d433b153b3 100644 --- a/test/wrapper.test.ts +++ b/test/wrapper.test.ts @@ -574,13 +574,13 @@ describe('Tests Native Wrapper', () => { (RNSentry.startProfiling as jest.MockedFunction).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).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).mockReturnValue({ From 0efab4bb125d180cf4f97646c72247d410e24952 Mon Sep 17 00:00:00 2001 From: Roman Zavarnitsyn Date: Tue, 17 Sep 2024 15:57:44 +0200 Subject: [PATCH 2/5] Revert --- .../ios/sentryreactnativesample.xcodeproj/project.pbxproj | 1 - 1 file changed, 1 deletion(-) diff --git a/samples/react-native/ios/sentryreactnativesample.xcodeproj/project.pbxproj b/samples/react-native/ios/sentryreactnativesample.xcodeproj/project.pbxproj index c0e1cee404..2a251b0d88 100644 --- a/samples/react-native/ios/sentryreactnativesample.xcodeproj/project.pbxproj +++ b/samples/react-native/ios/sentryreactnativesample.xcodeproj/project.pbxproj @@ -716,7 +716,6 @@ "-DFOLLY_NO_CONFIG", "-DFOLLY_MOBILE=1", "-DFOLLY_USE_LIBCPP=1", - "-DRN_FABRIC_ENABLED", ); OTHER_LDFLAGS = ( "$(inherited)", From 788e2a256ff712619abacb759653e1120aab6fbc Mon Sep 17 00:00:00 2001 From: getsentry-bot Date: Tue, 17 Sep 2024 14:04:47 +0000 Subject: [PATCH 3/5] release: 5.24.2 --- CHANGELOG.md | 2 +- package.json | 2 +- samples/expo/app.json | 6 +++--- samples/expo/package.json | 2 +- samples/react-native/android/app/build.gradle | 4 ++-- samples/react-native/ios/sentryreactnativesample/Info.plist | 4 ++-- .../ios/sentryreactnativesampleTests/Info.plist | 4 ++-- samples/react-native/package.json | 2 +- src/js/version.ts | 2 +- 9 files changed, 14 insertions(+), 14 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index cf04722700..47dd941567 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,6 +1,6 @@ # Changelog -## Unreleased +## 5.24.2 ### Features diff --git a/package.json b/package.json index 11a1309f0e..4d3ad6a114 100644 --- a/package.json +++ b/package.json @@ -2,7 +2,7 @@ "name": "@sentry/react-native", "homepage": "https://github.com/getsentry/sentry-react-native", "repository": "https://github.com/getsentry/sentry-react-native", - "version": "5.24.1", + "version": "5.24.2", "description": "Official Sentry SDK for react-native", "typings": "dist/js/index.d.ts", "types": "dist/js/index.d.ts", diff --git a/samples/expo/app.json b/samples/expo/app.json index 7486e307e0..753a5826bc 100644 --- a/samples/expo/app.json +++ b/samples/expo/app.json @@ -4,7 +4,7 @@ "slug": "sentry-react-native-expo-sample", "jsEngine": "hermes", "scheme": "sentry-expo-sample", - "version": "5.24.1", + "version": "5.24.2", "orientation": "portrait", "icon": "./assets/icon.png", "userInterfaceStyle": "light", @@ -19,7 +19,7 @@ "ios": { "supportsTablet": true, "bundleIdentifier": "io.sentry.expo.sample", - "buildNumber": "11" + "buildNumber": "12" }, "android": { "adaptiveIcon": { @@ -27,7 +27,7 @@ "backgroundColor": "#ffffff" }, "package": "io.sentry.expo.sample", - "versionCode": 11 + "versionCode": 12 }, "web": { "bundler": "metro", diff --git a/samples/expo/package.json b/samples/expo/package.json index c5a3506800..58bf49bcaa 100644 --- a/samples/expo/package.json +++ b/samples/expo/package.json @@ -1,6 +1,6 @@ { "name": "sentry-react-native-expo-sample", - "version": "5.24.1", + "version": "5.24.2", "main": "expo-router/entry", "scripts": { "start": "expo start", diff --git a/samples/react-native/android/app/build.gradle b/samples/react-native/android/app/build.gradle index 3f8097f0a5..e953b419de 100644 --- a/samples/react-native/android/app/build.gradle +++ b/samples/react-native/android/app/build.gradle @@ -134,8 +134,8 @@ android { applicationId "io.sentry.reactnative.sample" minSdkVersion rootProject.ext.minSdkVersion targetSdkVersion rootProject.ext.targetSdkVersion - versionCode 18 - versionName "5.24.1" + versionCode 19 + versionName "5.24.2" } signingConfigs { diff --git a/samples/react-native/ios/sentryreactnativesample/Info.plist b/samples/react-native/ios/sentryreactnativesample/Info.plist index 7e2aa7d9d2..c5d05f02d9 100644 --- a/samples/react-native/ios/sentryreactnativesample/Info.plist +++ b/samples/react-native/ios/sentryreactnativesample/Info.plist @@ -17,11 +17,11 @@ CFBundlePackageType APPL CFBundleShortVersionString - 5.24.1 + 5.24.2 CFBundleSignature ???? CFBundleVersion - 18 + 19 LSRequiresIPhoneOS NSAppTransportSecurity diff --git a/samples/react-native/ios/sentryreactnativesampleTests/Info.plist b/samples/react-native/ios/sentryreactnativesampleTests/Info.plist index 795471a3bc..39923462f2 100644 --- a/samples/react-native/ios/sentryreactnativesampleTests/Info.plist +++ b/samples/react-native/ios/sentryreactnativesampleTests/Info.plist @@ -15,10 +15,10 @@ CFBundlePackageType BNDL CFBundleShortVersionString - 5.24.1 + 5.24.2 CFBundleSignature ???? CFBundleVersion - 18 + 19 diff --git a/samples/react-native/package.json b/samples/react-native/package.json index 1f58d11bc0..7596ecd3f0 100644 --- a/samples/react-native/package.json +++ b/samples/react-native/package.json @@ -1,6 +1,6 @@ { "name": "sentry-react-native-sample", - "version": "5.24.1", + "version": "5.24.2", "private": true, "scripts": { "postinstall": "patch-package", diff --git a/src/js/version.ts b/src/js/version.ts index 0a6b53b2ab..cda9382cbe 100644 --- a/src/js/version.ts +++ b/src/js/version.ts @@ -1,3 +1,3 @@ export const SDK_PACKAGE_NAME = 'npm:@sentry/react-native'; export const SDK_NAME = 'sentry.javascript.react-native'; -export const SDK_VERSION = '5.24.1'; +export const SDK_VERSION = '5.24.2'; From aabfe9b3eccfd559b4340dec570510f5dc57ece9 Mon Sep 17 00:00:00 2001 From: Roman Zavarnitsyn Date: Tue, 17 Sep 2024 16:45:01 +0200 Subject: [PATCH 4/5] Merge leftovers --- samples/expo/app.json | 4 ---- .../ios/sentryreactnativesampleTests/Info.plist | 8 -------- 2 files changed, 12 deletions(-) diff --git a/samples/expo/app.json b/samples/expo/app.json index e67b30bc7d..cd18790bba 100644 --- a/samples/expo/app.json +++ b/samples/expo/app.json @@ -4,11 +4,7 @@ "slug": "sentry-react-native-expo-sample", "jsEngine": "hermes", "scheme": "sentry-expo-sample", -<<<<<<< HEAD - "version": "5.24.2", -======= "version": "5.32.0", ->>>>>>> main "orientation": "portrait", "icon": "./assets/icon.png", "userInterfaceStyle": "light", diff --git a/samples/react-native/ios/sentryreactnativesampleTests/Info.plist b/samples/react-native/ios/sentryreactnativesampleTests/Info.plist index 931ba55308..038e6f32dc 100644 --- a/samples/react-native/ios/sentryreactnativesampleTests/Info.plist +++ b/samples/react-native/ios/sentryreactnativesampleTests/Info.plist @@ -15,18 +15,10 @@ CFBundlePackageType BNDL CFBundleShortVersionString -<<<<<<< HEAD - 5.24.2 - CFBundleSignature - ???? - CFBundleVersion - 19 -======= 5.32.0 CFBundleSignature ???? CFBundleVersion 28 ->>>>>>> main From d035a36c09467f2c17b4b80b5067fb2635e69da8 Mon Sep 17 00:00:00 2001 From: Roman Zavarnitsyn Date: Tue, 17 Sep 2024 16:50:15 +0200 Subject: [PATCH 5/5] revert --- samples/expo/app.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/samples/expo/app.json b/samples/expo/app.json index cd18790bba..52878a6e9e 100644 --- a/samples/expo/app.json +++ b/samples/expo/app.json @@ -59,4 +59,4 @@ ] ] } -} +} \ No newline at end of file