Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(profiling): Add Hermes JS Profiling (experimental) #3057

Merged
merged 50 commits into from
Jul 27, 2023
Merged
Show file tree
Hide file tree
Changes from 41 commits
Commits
Show all changes
50 commits
Select commit Hold shift + click to select a range
5d55591
Add profiling start and stop to RNSentry Android module
krystofwoldrich May 5, 2023
43d96af
Merge remote-tracking branch 'origin/main' into kw-profiling
krystofwoldrich May 8, 2023
c3f4157
WIP! Add Profiling Integration Interface
krystofwoldrich May 10, 2023
03e054f
Add convertToSentryProfile
krystofwoldrich May 11, 2023
1b84f7b
Add test for convert hermes to sentry profile
krystofwoldrich May 12, 2023
5a0dac7
Merge branch 'main' into kw-profiling
krystofwoldrich May 22, 2023
5ee59f1
Update vscode shared launch options
krystofwoldrich May 24, 2023
eafa576
!WIP JS profiling iOS
krystofwoldrich May 25, 2023
1240f05
Large clean up and profiles sample rate
krystofwoldrich May 25, 2023
0cf7665
Fix clean-ios
krystofwoldrich May 26, 2023
d11e846
Set SENTRY_PROFILING_ENABLED using env when installing RNSentry pod
krystofwoldrich May 26, 2023
38d31bf
Remove unused user defined xcode settings
krystofwoldrich May 26, 2023
3baae8a
Update Android profiling to use Hermes Profiler directly
krystofwoldrich May 26, 2023
a7ae896
Merge branch 'main' into kw-profiling
krystofwoldrich May 26, 2023
f7d55f6
Remove profiling test buttons and NATIVE tmp export
krystofwoldrich May 26, 2023
8d9613d
Remove rn sentry package unused changes
krystofwoldrich May 30, 2023
1cee74e
Use profiles sample rate to add profiling integration
krystofwoldrich May 30, 2023
f44b1a6
Add profiling integration tests
krystofwoldrich May 31, 2023
19679d7
Fix mocks paths
krystofwoldrich May 31, 2023
2e27a20
Add yalc add javascript
krystofwoldrich Jun 1, 2023
c8458cb
Add integrations test and use profiling types and cache from js sdk
krystofwoldrich Jun 1, 2023
89182dc
Merge remote-tracking branch 'origin/main' into kw-profiling
krystofwoldrich Jun 1, 2023
87bf20d
Close buffered reader
krystofwoldrich Jun 1, 2023
11ab737
Remove compiler flag and use dynamic flag based on hermes.h
krystofwoldrich Jun 1, 2023
918eef0
Add clear current profile timeout
krystofwoldrich Jun 19, 2023
49de5d2
Use try with resource for profile file read buffer
krystofwoldrich Jun 19, 2023
a47c012
Use plural profilesSampleRate
krystofwoldrich Jun 19, 2023
df586fe
Refactor convert hermes profiles to smaller functions and add js docs…
krystofwoldrich Jun 19, 2023
9a19971
hermesStackFrameIdToSentryFrameIdMap to ensure correct sentry frame id
krystofwoldrich Jun 19, 2023
bb2ff5d
Merge remote-tracking branch 'origin/main' into kw-profiling
krystofwoldrich Jun 20, 2023
7e45cc9
Update iOS Sentry Profiling disabled error message
krystofwoldrich Jun 20, 2023
65155ad
Correctly name anonymous function during Hermes Sentry Profile conver…
krystofwoldrich Jun 20, 2023
b44830c
Add debug raw profile file log
krystofwoldrich Jun 21, 2023
0c47065
Add profiling changelog
krystofwoldrich Jun 21, 2023
a4b7cee
Update changelog
krystofwoldrich Jun 21, 2023
bfe91bb
Add log when hermes stack id not found in the map when mapping to sen…
krystofwoldrich Jun 22, 2023
f56dac0
Remove bundle file name regex to speed up the profile conversion
krystofwoldrich Jun 28, 2023
0eb553e
Update to JS SDK 7.57.0-beta.0
krystofwoldrich Jun 28, 2023
1e55aaf
Fix lint
krystofwoldrich Jun 28, 2023
caa12e5
Merge remote-tracking branch 'origin/main' into kw-profiling
krystofwoldrich Jun 28, 2023
d90eb10
Add remove over duration samples tests
krystofwoldrich Jun 28, 2023
cc6765c
Merge branch 'main' into kw-profiling
krystofwoldrich Jul 3, 2023
ffe4aab
Remove debugEnabled helper var
krystofwoldrich Jul 3, 2023
91cb036
Delete profile file after reading it
krystofwoldrich Jul 3, 2023
c14d4a7
Fix ms to ns conversion
krystofwoldrich Jul 3, 2023
d636449
Delete profile file in debug and catch thrown errors during delete
krystofwoldrich Jul 4, 2023
80e6fac
Merge branch 'main' into kw-profiling
krystofwoldrich Jul 10, 2023
81889c6
Merge remote-tracking branch 'origin/main' into kw-profiling
krystofwoldrich Jul 19, 2023
a71b774
fix changelog
krystofwoldrich Jul 19, 2023
333b1e7
Merge remote-tracking branch 'origin/main' into kw-profiling
krystofwoldrich Jul 27, 2023
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
24 changes: 21 additions & 3 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -5,12 +5,30 @@
### Features

- Use `android.namespace` for AGP 8 and RN 0.73 ([#3133](https://github.com/getsentry/sentry-react-native/pull/3133))
- 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/).

### Dependencies

- Bump JavaScript SDK from v7.54.0 to v7.56.0 ([#3119](https://github.com/getsentry/sentry-react-native/pull/3119))
- [changelog](https://github.com/getsentry/sentry-javascript/blob/develop/CHANGELOG.md#7560)
- [diff](https://github.com/getsentry/sentry-javascript/compare/7.54.0...7.56.0)
- Bump JavaScript SDK from v7.54.0 to v7.57.0-beta.0 ([#3119](https://github.com/getsentry/sentry-react-native/pull/3119), [#3057](https://github.com/getsentry/sentry-react-native/pull/3057))
- [changelog](https://github.com/getsentry/sentry-javascript/blob/7.57.0-beta.0/CHANGELOG.md#7570-beta0)
- [diff](https://github.com/getsentry/sentry-javascript/compare/7.54.0...7.57.0-beta.0)
- Bump CLI from v2.18.1 to v2.19.2 ([#3124](https://github.com/getsentry/sentry-react-native/pull/3124))
- [changelog](https://github.com/getsentry/sentry-cli/blob/master/CHANGELOG.md#2192)
- [diff](https://github.com/getsentry/sentry-cli/compare/2.18.1...2.19.2)
Expand Down
11 changes: 8 additions & 3 deletions RNSentry.podspec
Original file line number Diff line number Diff line change
@@ -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'
Expand All @@ -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"
Expand Down
47 changes: 47 additions & 0 deletions android/src/main/java/io/sentry/react/RNSentryModuleImpl.java
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
import androidx.annotation.Nullable;
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;
Expand All @@ -25,9 +26,11 @@
import com.facebook.react.bridge.WritableNativeMap;

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;
Expand Down Expand Up @@ -79,6 +82,7 @@ public class RNSentryModuleImpl {
private final PackageInfo packageInfo;
private FrameMetricsAggregator frameMetricsAggregator = null;
private boolean androidXAvailable;
private boolean debugEnabled = false;
krystofwoldrich marked this conversation as resolved.
Show resolved Hide resolved

private static boolean didFetchAppStart;

Expand Down Expand Up @@ -118,6 +122,7 @@ public void initNativeSdk(final ReadableMap rnOptions, Promise promise) {

if (rnOptions.hasKey("debug") && rnOptions.getBoolean("debug")) {
options.setDebug(true);
this.debugEnabled = true;
}
if (rnOptions.hasKey("dsn") && rnOptions.getString("dsn") != null) {
String dsn = rnOptions.getString("dsn");
Expand Down Expand Up @@ -616,6 +621,48 @@ 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 WritableMap result = new WritableNativeMap();
try {
krystofwoldrich marked this conversation as resolved.
Show resolved Hide resolved
HermesSamplingProfiler.disable();

final File output = File.createTempFile(
"sampling-profiler-trace", ".cpuprofile", reactApplicationContext.getCacheDir());
krystofwoldrich marked this conversation as resolved.
Show resolved Hide resolved

if (this.debugEnabled) {
logger.log(SentryLevel.INFO, "Profile saved to: " + output.getAbsolutePath());
}

try (final BufferedReader br = new BufferedReader(new FileReader(output));) {
HermesSamplingProfiler.dumpSampledTraceToFile(output.getPath());
krystofwoldrich marked this conversation as resolved.
Show resolved Hide resolved

final StringBuilder text = new StringBuilder();
String line;
while ((line = br.readLine()) != null) {
text.append(line);
text.append('\n');
}

result.putString("profile", text.toString());
marandaneto marked this conversation as resolved.
Show resolved Hide resolved
}
} catch (Throwable e) {
result.putString("error", e.toString());
marandaneto marked this conversation as resolved.
Show resolved Hide resolved
}
return result;
}

private void setEventOriginTag(SentryEvent event) {
SdkVersion sdk = event.getSdk();
if (sdk != null) {
Expand Down
12 changes: 12 additions & 0 deletions android/src/newarch/java/io/sentry/react/RNSentryModule.java
Original file line number Diff line number Diff line change
Expand Up @@ -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 {

Expand Down Expand Up @@ -121,4 +123,14 @@ public void fetchNativeDeviceContexts(Promise promise) {
public void fetchNativeSdkInfo(Promise promise) {
// Not used on android
}

@Override
public WritableMap startProfiling() {
return this.impl.startProfiling();
}

@Override
public WritableMap stopProfiling() {
return this.impl.stopProfiling();
}
}
11 changes: 11 additions & 0 deletions android/src/oldarch/java/io/sentry/react/RNSentryModule.java
Original file line number Diff line number Diff line change
Expand Up @@ -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 {

Expand Down Expand Up @@ -121,4 +122,14 @@ public void fetchNativeDeviceContexts(Promise promise) {
public void fetchNativeSdkInfo(Promise promise) {
// Not used on android
}

@ReactMethod(isBlockingSynchronousMethod = true)
public WritableMap startProfiling() {
return this.impl.startProfiling();
}

@ReactMethod(isBlockingSynchronousMethod = true)
public WritableMap stopProfiling() {
return this.impl.stopProfiling();
}
}
62 changes: 62 additions & 0 deletions ios/RNSentry.mm
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,17 @@
#import <Sentry/SentryScreenFrames.h>
#import <Sentry/SentryOptions+HybridSDKs.h>

#if __has_include(<hermes/hermes.h>)
#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 <hermes/hermes.h>
#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"
Expand Down Expand Up @@ -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) {
krystofwoldrich marked this conversation as resolved.
Show resolved Hide resolved
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<facebook::react::TurboModule>)getTurboModule:
Expand Down
21 changes: 11 additions & 10 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand All @@ -55,18 +56,18 @@
"react-native": ">=0.65.0"
},
"dependencies": {
"@sentry/browser": "7.56.0",
"@sentry/browser": "7.57.0-beta.0",
"@sentry/cli": "2.19.2",
"@sentry/core": "7.56.0",
"@sentry/hub": "7.56.0",
"@sentry/integrations": "7.56.0",
"@sentry/react": "7.56.0",
"@sentry/types": "7.56.0",
"@sentry/utils": "7.56.0"
"@sentry/core": "7.57.0-beta.0",
"@sentry/hub": "7.57.0-beta.0",
"@sentry/integrations": "7.57.0-beta.0",
"@sentry/react": "7.57.0-beta.0",
"@sentry/types": "7.57.0-beta.0",
"@sentry/utils": "7.57.0-beta.0"
},
"devDependencies": {
"@sentry-internal/eslint-config-sdk": "7.56.0",
"@sentry-internal/eslint-plugin-sdk": "7.56.0",
"@sentry-internal/eslint-config-sdk": "7.57.0-beta.0",
"@sentry-internal/eslint-plugin-sdk": "7.57.0-beta.0",
"@sentry/typescript": "^5.20.1",
"@sentry/wizard": "3.2.3",
"@types/jest": "^29.2.5",
Expand Down
Original file line number Diff line number Diff line change
@@ -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;
Expand Down
3 changes: 2 additions & 1 deletion sample-new-architecture/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down
5 changes: 4 additions & 1 deletion sample-new-architecture/src/App.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -85,6 +85,9 @@ Sentry.init({
// otherwise they will not work.
// release: '[email protected]+1',
// dist: `1`,
_experiments: {
profilesSampleRate: 1,
},
});

const Stack = createStackNavigator();
Expand Down
1 change: 0 additions & 1 deletion sample-new-architecture/src/Screens/HomeScreen.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -128,7 +128,6 @@ const HomeScreen = (props: Props) => {
/>
)}
<Spacer />

<Sentry.ErrorBoundary fallback={errorBoundaryFallback}>
<Button
title="Activate Error Boundary"
Expand Down
Loading