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

Add flag to disable native profilers #4094

Merged
merged 8 commits into from
Sep 17, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
18 changes: 18 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -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))

antonis marked this conversation as resolved.
Show resolved Hide resolved
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
Expand Down
29 changes: 18 additions & 11 deletions android/src/main/java/io/sentry/react/RNSentryModuleImpl.java
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand All @@ -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(
Expand All @@ -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 {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
10 changes: 7 additions & 3 deletions ios/RNSentry.mm
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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";
Expand Down Expand Up @@ -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";
Expand Down
2 changes: 1 addition & 1 deletion src/js/NativeRNSentry.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
26 changes: 21 additions & 5 deletions src/js/profiling/integration.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ import type {
Event,
Integration,
IntegrationClass,
IntegrationFn,
IntegrationFnResult,
ThreadCpuProfile,
Transaction,
} from '@sentry/types';
Expand All @@ -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()) {
Expand Down Expand Up @@ -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;
}
Expand Down Expand Up @@ -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;
}
Expand Down
6 changes: 3 additions & 3 deletions src/js/wrapper.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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 {
Expand Down
19 changes: 19 additions & 0 deletions test/profiling/integration.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand Down Expand Up @@ -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,
},
Expand All @@ -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: () => ({
Expand Down
4 changes: 2 additions & 2 deletions test/wrapper.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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({
Expand Down
Loading