From e4212f489a8e1e0e264e7bee4878cba750bef380 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Kry=C5=A1tof=20Wold=C5=99ich?= <31292499+krystofwoldrich@users.noreply.github.com> Date: Thu, 27 Jul 2023 11:23:03 +0200 Subject: [PATCH] feat(profiling): Add Hermes JS Profiling (experimental) (#3057) --- CHANGELOG.md | 21 ++ RNSentry.podspec | 11 +- .../io/sentry/react/RNSentryModuleImpl.java | 60 +++++ .../java/io/sentry/react/RNSentryModule.java | 12 + .../java/io/sentry/react/RNSentryModule.java | 11 + ios/RNSentry.mm | 62 +++++ package.json | 3 +- .../MainApplication.java | 8 +- sample-new-architecture/package.json | 3 +- sample-new-architecture/src/App.tsx | 5 +- .../src/Screens/HomeScreen.tsx | 1 - scripts/hermes-profile-transformer.js | 26 ++ src/js/NativeRNSentry.ts | 2 + src/js/client.ts | 2 + src/js/integrations/index.ts | 1 + src/js/integrations/rewriteframes.ts | 4 +- src/js/profiling/cache.ts | 5 + src/js/profiling/convertHermesProfile.ts | 185 +++++++++++++ src/js/profiling/hermes.ts | 81 ++++++ src/js/profiling/integration.ts | 191 +++++++++++++ src/js/profiling/types.ts | 5 + src/js/profiling/utils.ts | 187 +++++++++++++ src/js/sdk.tsx | 4 + src/js/wrapper.ts | 44 +++ test/mockDsn.ts | 1 + test/mockWrapper.ts | 71 +++++ test/profiling/convertHermesProfile.test.ts | 205 ++++++++++++++ test/profiling/hermes.test.ts | 18 ++ test/profiling/integration.test.ts | 255 ++++++++++++++++++ test/sdk.test.ts | 24 ++ test/testutils.ts | 4 + test/wrapper.test.ts | 35 +++ 32 files changed, 1537 insertions(+), 10 deletions(-) create mode 100644 scripts/hermes-profile-transformer.js create mode 100644 src/js/profiling/cache.ts create mode 100644 src/js/profiling/convertHermesProfile.ts create mode 100644 src/js/profiling/hermes.ts create mode 100644 src/js/profiling/integration.ts create mode 100644 src/js/profiling/types.ts create mode 100644 src/js/profiling/utils.ts create mode 100644 test/mockDsn.ts create mode 100644 test/mockWrapper.ts create mode 100644 test/profiling/convertHermesProfile.test.ts create mode 100644 test/profiling/hermes.test.ts create mode 100644 test/profiling/integration.test.ts diff --git a/CHANGELOG.md b/CHANGELOG.md index 4ea215a28..0bba93e21 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,27 @@ ## Unreleased +### Features + +- Alpha support for Hermes JavaScript Profiling ([#3057](https://github.com/getsentry/sentry-react-native/pull/3057)) + + Profiling is disabled by default. To enable it, configure both + `tracesSampleRate` and `profilesSampleRate` when initializing the SDK: + + ```javascript + Sentry.init({ + dsn: '__DSN__', + tracesSampleRate: 1.0, + _experiments: { + // The sampling rate for profiling is relative to TracesSampleRate. + // In this case, we'll capture profiles for 100% of transactions. + profilesSampleRate: 1.0, + }, + }); + ``` + + More documentation on profiling and current limitations [can be found here](https://docs.sentry.io/platforms/react-native/profiling/). + ### Fixes - Warn users about multiple versions of `promise` package which can cause unexpected behavior like undefined `Promise.allSettled` ([#3162](https://github.com/getsentry/sentry-react-native/pull/3162)) diff --git a/RNSentry.podspec b/RNSentry.podspec index 6073d3ffd..24fed7ec5 100644 --- a/RNSentry.podspec +++ b/RNSentry.podspec @@ -1,7 +1,12 @@ require 'json' version = JSON.parse(File.read('package.json'))["version"] -folly_compiler_flags = '-DFOLLY_NO_CONFIG -DFOLLY_MOBILE=1 -DFOLLY_USE_LIBCPP=1 -Wno-comma -Wno-shorten-64-to-32' +folly_flags = ' -DFOLLY_NO_CONFIG -DFOLLY_MOBILE=1 -DFOLLY_USE_LIBCPP=1' +folly_compiler_flags = folly_flags + ' ' + '-Wno-comma -Wno-shorten-64-to-32' + +is_new_arch_enabled = ENV["RCT_NEW_ARCH_ENABLED"] == "1" +new_arch_enabled_flag = (is_new_arch_enabled ? folly_compiler_flags + " -DRCT_NEW_ARCH_ENABLED" : "") +other_cflags = "$(inherited)" + new_arch_enabled_flag Pod::Spec.new do |s| s.name = 'RNSentry' @@ -24,9 +29,9 @@ Pod::Spec.new do |s| s.source_files = 'ios/**/*.{h,mm}' s.public_header_files = 'ios/RNSentry.h' + s.compiler_flags = other_cflags # This guard prevent to install the dependencies when we run `pod install` in the old architecture. - if ENV['RCT_NEW_ARCH_ENABLED'] == '1' then - s.compiler_flags = folly_compiler_flags + " -DRCT_NEW_ARCH_ENABLED=1" + if is_new_arch_enabled then s.pod_target_xcconfig = { "HEADER_SEARCH_PATHS" => "\"$(PODS_ROOT)/boost\"", "CLANG_CXX_LANGUAGE_STANDARD" => "c++17" diff --git a/android/src/main/java/io/sentry/react/RNSentryModuleImpl.java b/android/src/main/java/io/sentry/react/RNSentryModuleImpl.java index 5092969d0..845fc76ec 100644 --- a/android/src/main/java/io/sentry/react/RNSentryModuleImpl.java +++ b/android/src/main/java/io/sentry/react/RNSentryModuleImpl.java @@ -11,6 +11,7 @@ import androidx.core.app.FrameMetricsAggregator; +import com.facebook.hermes.instrumentation.HermesSamplingProfiler; import com.facebook.react.bridge.Arguments; import com.facebook.react.bridge.Promise; import com.facebook.react.bridge.ReactApplicationContext; @@ -27,7 +28,11 @@ import org.jetbrains.annotations.Nullable; import java.io.BufferedInputStream; +import java.io.BufferedReader; +import java.io.File; import java.io.FileNotFoundException; +import java.io.FileOutputStream; +import java.io.FileReader; import java.io.InputStream; import java.nio.charset.Charset; import java.util.HashMap; @@ -610,6 +615,61 @@ public void disableNativeFramesTracking() { } } + public WritableMap startProfiling() { + final WritableMap result = new WritableNativeMap(); + try { + HermesSamplingProfiler.enable(); + result.putBoolean("started", true); + } catch (Throwable e) { + result.putBoolean("started", false); + result.putString("error", e.toString()); + } + return result; + } + + public WritableMap stopProfiling() { + final boolean isDebug = HubAdapter.getInstance().getOptions().isDebug(); + final WritableMap result = new WritableNativeMap(); + File output = null; + try { + HermesSamplingProfiler.disable(); + + output = File.createTempFile( + "sampling-profiler-trace", ".cpuprofile", reactApplicationContext.getCacheDir()); + + if (isDebug) { + logger.log(SentryLevel.INFO, "Profile saved to: " + output.getAbsolutePath()); + } + + try (final BufferedReader br = new BufferedReader(new FileReader(output));) { + HermesSamplingProfiler.dumpSampledTraceToFile(output.getPath()); + + final StringBuilder text = new StringBuilder(); + String line; + while ((line = br.readLine()) != null) { + text.append(line); + text.append('\n'); + } + + result.putString("profile", text.toString()); + } + } catch (Throwable e) { + result.putString("error", e.toString()); + } finally { + if (output != null) { + try { + final boolean wasProfileSuccessfullyDeleted = output.delete(); + if (!wasProfileSuccessfullyDeleted) { + logger.log(SentryLevel.WARNING, "Profile not deleted from:" + output.getAbsolutePath()); + } + } catch (Throwable e) { + logger.log(SentryLevel.WARNING, "Profile not deleted from:" + output.getAbsolutePath()); + } + } + } + return result; + } + public void fetchNativeDeviceContexts(Promise promise) { final @NotNull SentryOptions options = HubAdapter.getInstance().getOptions(); if (!(options instanceof SentryAndroidOptions)) { diff --git a/android/src/newarch/java/io/sentry/react/RNSentryModule.java b/android/src/newarch/java/io/sentry/react/RNSentryModule.java index e167de797..0310f537a 100644 --- a/android/src/newarch/java/io/sentry/react/RNSentryModule.java +++ b/android/src/newarch/java/io/sentry/react/RNSentryModule.java @@ -2,10 +2,12 @@ import androidx.annotation.NonNull; +import com.facebook.react.bridge.JavaScriptExecutorFactory; import com.facebook.react.bridge.ReactApplicationContext; import com.facebook.react.bridge.ReadableArray; import com.facebook.react.bridge.ReadableMap; import com.facebook.react.bridge.Promise; +import com.facebook.react.bridge.WritableMap; public class RNSentryModule extends NativeRNSentrySpec { @@ -121,4 +123,14 @@ public void fetchNativeDeviceContexts(Promise promise) { public void fetchNativeSdkInfo(Promise promise) { this.impl.fetchNativeSdkInfo(promise); } + + @Override + public WritableMap startProfiling() { + return this.impl.startProfiling(); + } + + @Override + public WritableMap stopProfiling() { + return this.impl.stopProfiling(); + } } diff --git a/android/src/oldarch/java/io/sentry/react/RNSentryModule.java b/android/src/oldarch/java/io/sentry/react/RNSentryModule.java index f6aa57802..3f0d70863 100644 --- a/android/src/oldarch/java/io/sentry/react/RNSentryModule.java +++ b/android/src/oldarch/java/io/sentry/react/RNSentryModule.java @@ -7,6 +7,7 @@ import com.facebook.react.bridge.ReactContext; import com.facebook.react.bridge.ReactContextBaseJavaModule; import com.facebook.react.bridge.ReactMethod; +import com.facebook.react.bridge.WritableMap; public class RNSentryModule extends ReactContextBaseJavaModule { @@ -121,4 +122,14 @@ public void fetchNativeDeviceContexts(Promise promise) { public void fetchNativeSdkInfo(Promise promise) { this.impl.fetchNativeSdkInfo(promise); } + + @ReactMethod(isBlockingSynchronousMethod = true) + public WritableMap startProfiling() { + return this.impl.startProfiling(); + } + + @ReactMethod(isBlockingSynchronousMethod = true) + public WritableMap stopProfiling() { + return this.impl.stopProfiling(); + } } diff --git a/ios/RNSentry.mm b/ios/RNSentry.mm index c2bd9cf69..8afe998f4 100644 --- a/ios/RNSentry.mm +++ b/ios/RNSentry.mm @@ -11,6 +11,17 @@ #import #import +#if __has_include() +#define SENTRY_PROFILING_ENABLED 1 +#else +#define SENTRY_PROFILING_ENABLED 0 +#endif + +// This guard prevents importing Hermes in JSC apps +#if SENTRY_PROFILING_ENABLED +#import +#endif + // Thanks to this guard, we won't import this header when we build for the old architecture. #ifdef RCT_NEW_ARCH_ENABLED #import "RNSentrySpec.h" @@ -498,6 +509,57 @@ - (void)setEventEnvironmentTag:(SentryEvent *)event // the 'tracesSampleRate' or 'tracesSampler' option. } +static NSString* const enabledProfilingMessage = @"Enable Hermes to use Sentry Profiling."; + +RCT_EXPORT_SYNCHRONOUS_TYPED_METHOD(NSDictionary *, startProfiling) +{ +#if SENTRY_PROFILING_ENABLED + try { + facebook::hermes::HermesRuntime::enableSamplingProfiler(); + return @{ @"started": @YES }; + } catch (const std::exception& ex) { + return @{ @"error": [NSString stringWithCString: ex.what() encoding:[NSString defaultCStringEncoding]] }; + } catch (...) { + return @{ @"error": @"Failed to start profiling" }; + } +#else + return @{ @"error": enabledProfilingMessage }; +#endif +} + +RCT_EXPORT_SYNCHRONOUS_TYPED_METHOD(NSDictionary *, stopProfiling) +{ +#if SENTRY_PROFILING_ENABLED + try { + facebook::hermes::HermesRuntime::disableSamplingProfiler(); + std::stringstream ss; + facebook::hermes::HermesRuntime::dumpSampledTraceToStream(ss); + + std::string s = ss.str(); + NSString *data = [NSString stringWithCString:s.c_str() encoding:[NSString defaultCStringEncoding]]; + +#if SENTRY_PROFILING_DEBUG_ENABLED + NSString *rawProfileFileName = @"hermes.profile"; + NSError *error = nil; + NSString *rawProfileFilePath = [NSTemporaryDirectory() stringByAppendingPathComponent:rawProfileFileName]; + if (![data writeToFile:rawProfileFilePath atomically:YES encoding:NSUTF8StringEncoding error:&error]) { + NSLog(@"Error writing Raw Hermes Profile to %@: %@", rawProfileFilePath, error); + } else { + NSLog(@"Raw Hermes Profile saved to %@", rawProfileFilePath); + } +#endif + + return @{ @"profile": data }; + } catch (const std::exception& ex) { + return @{ @"error": [NSString stringWithCString: ex.what() encoding:[NSString defaultCStringEncoding]] }; + } catch (...) { + return @{ @"error": @"Failed to stop profiling" }; + } +#else + return @{ @"error": enabledProfilingMessage }; +#endif +} + // Thanks to this guard, we won't compile this code when we build for the old architecture. #ifdef RCT_NEW_ARCH_ENABLED - (std::shared_ptr)getTurboModule: diff --git a/package.json b/package.json index 5a5186983..e6308c253 100644 --- a/package.json +++ b/package.json @@ -33,7 +33,8 @@ "lint:prettier": "prettier --check \"{src,test,scripts}/**/**.ts\"", "test:watch": "jest --watch", "run-ios": "cd sample-new-architecture && yarn react-native run-ios", - "run-android": "cd sample-new-architecture && yarn react-native run-android" + "run-android": "cd sample-new-architecture && yarn react-native run-android", + "yalc:add:sentry-javascript": "yalc add @sentry/browser @sentry/core @sentry/hub @sentry/integrations @sentry/react @sentry/types @sentry/utils" }, "keywords": [ "react-native", diff --git a/sample-new-architecture/android/app/src/main/java/com/samplenewarchitecture/MainApplication.java b/sample-new-architecture/android/app/src/main/java/com/samplenewarchitecture/MainApplication.java index cae11eb38..ca8e4259f 100644 --- a/sample-new-architecture/android/app/src/main/java/com/samplenewarchitecture/MainApplication.java +++ b/sample-new-architecture/android/app/src/main/java/com/samplenewarchitecture/MainApplication.java @@ -1,21 +1,27 @@ package com.samplenewarchitecture; import android.app.Application; + +import com.facebook.hermes.reactexecutor.HermesExecutorFactory; import com.facebook.react.PackageList; import com.facebook.react.ReactApplication; import com.facebook.react.ReactNativeHost; import com.facebook.react.ReactPackage; +import com.facebook.react.bridge.JavaScriptExecutor; +import com.facebook.react.bridge.JavaScriptExecutorFactory; +import com.facebook.react.common.JavascriptException; import com.facebook.react.defaults.DefaultNewArchitectureEntryPoint; import com.facebook.react.defaults.DefaultReactNativeHost; import com.facebook.soloader.SoLoader; import java.util.List; +import io.sentry.react.RNSentryModuleImpl; import io.sentry.react.RNSentryPackage; public class MainApplication extends Application implements ReactApplication { - private final ReactNativeHost mReactNativeHost = new DefaultReactNativeHost(this) { + @Override public boolean getUseDeveloperSupport() { return BuildConfig.DEBUG; diff --git a/sample-new-architecture/package.json b/sample-new-architecture/package.json index 727db2a13..762e9aac9 100644 --- a/sample-new-architecture/package.json +++ b/sample-new-architecture/package.json @@ -11,7 +11,8 @@ "pod-install": "cd ios; RCT_NEW_ARCH_ENABLED=1 bundle exec pod install; cd ..", "pod-install-production": "cd ios; PRODUCTION=1 RCT_NEW_ARCH_ENABLED=1 bundle exec pod install; cd ..", "pod-install-legacy": "cd ios; bundle exec pod install; cd ..", - "pod-install-legacy-production": "cd ios; PRODUCTION=1 bundle exec pod install; cd .." + "pod-install-legacy-production": "cd ios; PRODUCTION=1 bundle exec pod install; cd ..", + "clean-ios": "cd ios; rm -rf Podfile.lock Pods build; cd .." }, "dependencies": { "@react-navigation/native": "^6.1.7", diff --git a/sample-new-architecture/src/App.tsx b/sample-new-architecture/src/App.tsx index 5ebd06949..6c52b98ed 100644 --- a/sample-new-architecture/src/App.tsx +++ b/sample-new-architecture/src/App.tsx @@ -31,7 +31,7 @@ Sentry.init({ dsn: SENTRY_INTERNAL_DSN, debug: true, beforeSend: (event: Sentry.Event) => { - console.log('Event beforeSend:', event); + console.log('Event beforeSend:', event.event_id); return event; }, // This will be called with a boolean `didCallNativeInit` when the native SDK has been contacted. @@ -85,6 +85,9 @@ Sentry.init({ // otherwise they will not work. // release: 'myapp@1.2.3+1', // dist: `1`, + _experiments: { + profilesSampleRate: 1, + }, }); const Stack = createStackNavigator(); diff --git a/sample-new-architecture/src/Screens/HomeScreen.tsx b/sample-new-architecture/src/Screens/HomeScreen.tsx index 31c5f72cd..0d57b5191 100644 --- a/sample-new-architecture/src/Screens/HomeScreen.tsx +++ b/sample-new-architecture/src/Screens/HomeScreen.tsx @@ -128,7 +128,6 @@ const HomeScreen = (props: Props) => { /> )} -