From 7dff905c307895f1bef4585a7e993761afbb8f66 Mon Sep 17 00:00:00 2001 From: Krystof Woldrich Date: Wed, 11 Oct 2023 13:18:24 +0200 Subject: [PATCH 01/16] feat(profiling): Send debug information with Profiles --- CHANGELOG.md | 4 ++ src/js/profiling/convertHermesProfile.ts | 5 +- src/js/profiling/types.ts | 1 + src/js/profiling/utils.ts | 81 +++++++++++++++++++++++- 4 files changed, 88 insertions(+), 3 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index aef1e0fc31..b16ec9142d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,10 @@ ## Unreleased +### Features + +- Send debug information with Profiles for symbolicated stack traces ([#3338](https://github.com/getsentry/sentry-react-native/pull/3338)) + ### Fixes - Waif for `has-sourcemap-debugid` process to exit ([#3336](https://github.com/getsentry/sentry-react-native/pull/3336)) diff --git a/src/js/profiling/convertHermesProfile.ts b/src/js/profiling/convertHermesProfile.ts index 03809e15e8..aec524dbed 100644 --- a/src/js/profiling/convertHermesProfile.ts +++ b/src/js/profiling/convertHermesProfile.ts @@ -36,7 +36,7 @@ export function convertToSentryProfile(hermesProfile: Hermes.Profile): RawThread const { samples, hermesStacks, jsThreads } = mapSamples(hermesProfile.samples); - const { frames, hermesStackFrameIdToSentryFrameIdMap } = mapFrames(hermesProfile.stackFrames); + const { frames, hermesStackFrameIdToSentryFrameIdMap, resources } = mapFrames(hermesProfile.stackFrames); const { stacks, hermesStackToSentryStackMap } = mapStacks( hermesStacks, @@ -63,6 +63,7 @@ export function convertToSentryProfile(hermesProfile: Hermes.Profile): RawThread } return { + resources, samples, frames, stacks, @@ -124,6 +125,7 @@ export function mapSamples( function mapFrames(hermesStackFrames: Record): { frames: ThreadCpuFrame[]; hermesStackFrameIdToSentryFrameIdMap: Map; + resources: string[]; } { const frames: ThreadCpuFrame[] = []; const hermesStackFrameIdToSentryFrameIdMap = new Map(); @@ -147,6 +149,7 @@ function mapFrames(hermesStackFrames: Record>(); +/** + * Applies debug meta data to an event from a list of paths to resources (sourcemaps) + * https://github.com/getsentry/sentry-javascript/blob/ec9eebc5a09a0ce2e51cd4080e729a48c8b2fe97/packages/browser/src/profiling/utils.ts#L328 + */ +export function applyDebugMetadata(resource_paths: ReadonlyArray): DebugImage[] { + const debugIdMap = GLOBAL_OBJ._sentryDebugIds; + + if (!debugIdMap) { + return []; + } + + const hub = getCurrentHub(); + if (!hub) { + return []; + } + const client = hub.getClient(); + if (!client) { + return []; + } + const options = client.getOptions(); + if (!options) { + return []; + } + const stackParser = options.stackParser; + if (!stackParser) { + return []; + } + + let debugIdStackFramesCache: Map; + const cachedDebugIdStackFrameCache = debugIdStackParserCache.get(stackParser); + if (cachedDebugIdStackFrameCache) { + debugIdStackFramesCache = cachedDebugIdStackFrameCache; + } else { + debugIdStackFramesCache = new Map(); + debugIdStackParserCache.set(stackParser, debugIdStackFramesCache); + } + + // Build a map of filename -> debug_id + const filenameDebugIdMap = Object.keys(debugIdMap).reduce>((acc, debugIdStackTrace) => { + let parsedStack: StackFrame[]; + const cachedParsedStack = debugIdStackFramesCache.get(debugIdStackTrace); + if (cachedParsedStack) { + parsedStack = cachedParsedStack; + } else { + parsedStack = stackParser(debugIdStackTrace); + debugIdStackFramesCache.set(debugIdStackTrace, parsedStack); + } + + for (let i = parsedStack.length - 1; i >= 0; i--) { + const stackFrame = parsedStack[i]; + if (stackFrame.filename) { + acc[stackFrame.filename] = debugIdMap[debugIdStackTrace]; + break; + } + } + return acc; + }, {}); + + const images: DebugImage[] = []; + for (const path of resource_paths) { + if (path && filenameDebugIdMap[path]) { + images.push({ + type: 'sourcemap', + code_file: path, + debug_id: filenameDebugIdMap[path] as string, + }); + } + } + + return images; +} + /** * Adds items to envelope if they are not already present - mutates the envelope. * @param envelope From f147e933d9ea4a3db361f969a1dc8016fd75ad5d Mon Sep 17 00:00:00 2001 From: Krystof Woldrich Date: Wed, 11 Oct 2023 14:59:00 +0200 Subject: [PATCH 02/16] fix white space --- src/js/profiling/utils.ts | 1 - 1 file changed, 1 deletion(-) diff --git a/src/js/profiling/utils.ts b/src/js/profiling/utils.ts index 9f9aba247a..d2cd784994 100644 --- a/src/js/profiling/utils.ts +++ b/src/js/profiling/utils.ts @@ -181,7 +181,6 @@ const debugIdStackParserCache = new WeakMap): DebugImage[] { const debugIdMap = GLOBAL_OBJ._sentryDebugIds; - if (!debugIdMap) { return []; } From 5561dc7019be97b91d4dc2161f25c8a912aca89d Mon Sep 17 00:00:00 2001 From: Krystof Woldrich Date: Fri, 13 Oct 2023 14:24:42 +0200 Subject: [PATCH 03/16] Add support for Hermes bytecode frames --- src/js/profiling/convertHermesProfile.ts | 10 +--- src/js/profiling/hermes.ts | 61 ++++++++++++++++++--- test/profiling/convertHermesProfile.test.ts | 9 +-- test/profiling/hermes.test.ts | 50 +++++++++++++++-- 4 files changed, 103 insertions(+), 27 deletions(-) diff --git a/src/js/profiling/convertHermesProfile.ts b/src/js/profiling/convertHermesProfile.ts index 5ee0f9772e..58f1ff6952 100644 --- a/src/js/profiling/convertHermesProfile.ts +++ b/src/js/profiling/convertHermesProfile.ts @@ -136,15 +136,7 @@ function mapFrames(hermesStackFrames: Record; } +export interface ParsedHermesStackFrame { + function: string; + file?: string; + lineno?: number; + colno?: number; +} + +const DEFAULT_BUNDLE_NAME = Platform.OS === 'android' + ? ANDROID_DEFAULT_BUNDLE_NAME + : Platform.OS === 'ios' + ? IOS_DEFAULT_BUNDLE_NAME + : undefined; +const ANONYMOUS_FUNCTION_NAME = 'anonymous'; + /** - * Hermes Profile Stack Frame Name contains function name and file path. - * - * `foo(/path/to/file.js:1:2)` -> `foo` + * Parses Hermes StackFrame to Sentry StackFrame. + * For native frames only function name is returned, for Hermes bytecode the line and column are calculated. */ -export function parseHermesStackFrameFunctionName(hermesName: string): string { - const indexOfLeftParenthesis = hermesName.indexOf('('); - const name = indexOfLeftParenthesis !== -1 ? hermesName.substring(0, indexOfLeftParenthesis) : hermesName; - return name; +export function parseHermesJSStackFrame(frame: StackFrame): ParsedHermesStackFrame { + if (frame.category !== 'JavaScript') { + // Native + return { function: frame.name }; + } + + if (frame.funcVirtAddr !== undefined && frame.offset !== undefined) { + // Hermes Bytecode + return { + function: frame.name || ANONYMOUS_FUNCTION_NAME, + file: DEFAULT_BUNDLE_NAME, + // https://github.com/krystofwoldrich/metro/blob/417e6f276ff9422af6039fc4d1bce41fcf7d9f46/packages/metro-symbolicate/src/Symbolication.js#L298-L301 + // Hermes lineno is hardcoded 1, currently only one bundle symbolication is supported by metro-symbolicate and thus by us. + lineno: 1, + // Hermes colno is 0-based, while Sentry is 1-based + colno: Number(frame.funcVirtAddr) + Number(frame.offset) + 1, + }; + } + + // JavaScript + const indexOfLeftParenthesis = frame.name.indexOf('('); + return { + function: (indexOfLeftParenthesis !== -1 && (frame.name.substring(0, indexOfLeftParenthesis) || ANONYMOUS_FUNCTION_NAME)) || + frame.name, + file: DEFAULT_BUNDLE_NAME, + lineno: frame.line !== undefined ? Number(frame.line) : undefined, + colno: frame.column !== undefined ? Number(frame.column) : undefined, + }; } const MS_TO_NS: number = 1e6; diff --git a/test/profiling/convertHermesProfile.test.ts b/test/profiling/convertHermesProfile.test.ts index 547681e037..45fe3391f5 100644 --- a/test/profiling/convertHermesProfile.test.ts +++ b/test/profiling/convertHermesProfile.test.ts @@ -56,7 +56,7 @@ describe('convert hermes profile to sentry profile', () => { column: '33', funcLine: '1605', funcColumn: '14', - name: 'fooA(/absolute/path/app:///main.jsbundle:1610:33)', + name: 'fooA(app:///main.jsbundle:1610:33)', category: 'JavaScript', parent: 1, }, @@ -65,7 +65,7 @@ describe('convert hermes profile to sentry profile', () => { column: '21', funcLine: '1614', funcColumn: '14', - name: 'fooB(/absolute/path/app:///main.jsbundle:1616:21)', + name: 'fooB(http://localhost:8081/index.bundle//&platform=ios&dev=true&minify=false&modulesOnly=false&runModule=true&app=org.reactjs.native.example.sampleNewArchitecture:193430:4)', category: 'JavaScript', parent: 1, }, @@ -74,7 +74,7 @@ describe('convert hermes profile to sentry profile', () => { column: '18', funcLine: '1623', funcColumn: '16', - name: '(/absolute/path/app:///main.jsbundle:1627:18)', + name: '(/Users/distiller/react-native/packages/react-native/sdks/hermes/build_iphonesimulator/lib/InternalBytecode/InternalBytecode.js:139:27)', category: 'JavaScript', parent: 2, }, @@ -83,10 +83,7 @@ describe('convert hermes profile to sentry profile', () => { const expectedSentryProfile: RawThreadCpuProfile = { frames: [ { - colno: undefined, - file: undefined, function: '[root]', - lineno: undefined, }, { colno: 33, diff --git a/test/profiling/hermes.test.ts b/test/profiling/hermes.test.ts index f9fb9a6b4d..ebc6ebf2e8 100644 --- a/test/profiling/hermes.test.ts +++ b/test/profiling/hermes.test.ts @@ -1,18 +1,58 @@ -import { parseHermesStackFrameFunctionName } from '../../src/js/profiling/hermes'; +import type { ParsedHermesStackFrame} from '../../src/js/profiling/hermes'; +import { parseHermesJSStackFrame } from '../../src/js/profiling/hermes'; describe('hermes', () => { describe('parseHermesStackFrameName', () => { test('parses function name and file name', () => { - expect(parseHermesStackFrameFunctionName('fooA(/absolute/path/main.jsbundle:1610:33)')).toEqual('fooA'); + expect( + parseHermesJSStackFrame({ + name: 'fooA(/absolute/path/main.jsbundle:1610:33)', + line: '1610', + column: '33', + category: 'JavaScript', + }) + ).toEqual( { + function: 'fooA', + file: 'app:///main.jsbundle', + lineno: 1610, + colno: 33, + }); }); test('parse hermes root stack frame', () => { - expect(parseHermesStackFrameFunctionName('[root]')).toEqual('[root]'); + expect( + parseHermesJSStackFrame({ + name: '[root]', + category: 'root', + }) + ).toEqual(expect.objectContaining( { + function: '[root]', + })); }); test('parse only file name', () => { - expect(parseHermesStackFrameFunctionName('(/absolute/path/jsbundle:1610:33)')).toEqual(''); + expect( + parseHermesJSStackFrame({ + name: '(/absolute/path/main.jsbundle:1610:33)', + line: '1610', + column: '33', + category: 'JavaScript', + }) + ).toEqual( { + function: 'anonymous', + file: 'app:///main.jsbundle', + lineno: 1610, + colno: 33, + }); }); test('parse only function name', () => { - expect(parseHermesStackFrameFunctionName('fooA')).toEqual('fooA'); + expect( + parseHermesJSStackFrame({ + name: 'fooA', + category: 'JavaScript', + }) + ).toEqual(expect.objectContaining( { + function: 'fooA', + file: 'app:///main.jsbundle', + })); }); }); }); From fd0876e41da40a6d8ac4117730ba13ccf5168a2e Mon Sep 17 00:00:00 2001 From: Krystof Woldrich Date: Fri, 13 Oct 2023 15:55:31 +0200 Subject: [PATCH 04/16] Add changelog --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 7190fa9591..a46e380df5 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,6 +5,7 @@ ### Fixes - Add actual `activeThreadId` to Profiles ([#3338](https://github.com/getsentry/sentry-react-native/pull/3338)) +- Parse Hermes Profiling Bytecode Frames ([#3342](https://github.com/getsentry/sentry-react-native/pull/3342)) ## 5.11.1 From 7e964cee268fea015cac5f777f21e2135ddab02b Mon Sep 17 00:00:00 2001 From: Krystof Woldrich Date: Fri, 13 Oct 2023 14:25:23 +0200 Subject: [PATCH 05/16] Use bundle filename to lookup debugid --- src/js/profiling/convertHermesProfile.ts | 12 +----- src/js/profiling/types.ts | 1 - src/js/profiling/utils.ts | 53 +++++++++++++++++++++++- 3 files changed, 54 insertions(+), 12 deletions(-) diff --git a/src/js/profiling/convertHermesProfile.ts b/src/js/profiling/convertHermesProfile.ts index aec524dbed..b50881b2b3 100644 --- a/src/js/profiling/convertHermesProfile.ts +++ b/src/js/profiling/convertHermesProfile.ts @@ -1,21 +1,16 @@ import type { FrameId, StackId, ThreadCpuFrame, ThreadCpuSample, ThreadCpuStack, ThreadId } from '@sentry/types'; import { logger } from '@sentry/utils'; -import { Platform } from 'react-native'; -import { ANDROID_DEFAULT_BUNDLE_NAME, IOS_DEFAULT_BUNDLE_NAME } from '../integrations/rewriteframes'; import type * as Hermes from './hermes'; -import { parseHermesStackFrameFunctionName } from './hermes'; +import { parseHermesJSStackFrame } from './hermes'; import { MAX_PROFILE_DURATION_MS } from './integration'; import type { RawThreadCpuProfile } from './types'; const MS_TO_NS = 1e6; const MAX_PROFILE_DURATION_NS = MAX_PROFILE_DURATION_MS * MS_TO_NS; -const ANONYMOUS_FUNCTION_NAME = 'anonymous'; const UNKNOWN_STACK_ID = -1; const JS_THREAD_NAME = 'JavaScriptThread'; const JS_THREAD_PRIORITY = 1; -const DEFAULT_BUNDLE_NAME = - Platform.OS === 'android' ? ANDROID_DEFAULT_BUNDLE_NAME : Platform.OS === 'ios' ? IOS_DEFAULT_BUNDLE_NAME : undefined; /** * Converts a Hermes profile to a Sentry profile. @@ -36,7 +31,7 @@ export function convertToSentryProfile(hermesProfile: Hermes.Profile): RawThread const { samples, hermesStacks, jsThreads } = mapSamples(hermesProfile.samples); - const { frames, hermesStackFrameIdToSentryFrameIdMap, resources } = mapFrames(hermesProfile.stackFrames); + const { frames, hermesStackFrameIdToSentryFrameIdMap } = mapFrames(hermesProfile.stackFrames); const { stacks, hermesStackToSentryStackMap } = mapStacks( hermesStacks, @@ -63,7 +58,6 @@ export function convertToSentryProfile(hermesProfile: Hermes.Profile): RawThread } return { - resources, samples, frames, stacks, @@ -125,7 +119,6 @@ export function mapSamples( function mapFrames(hermesStackFrames: Record): { frames: ThreadCpuFrame[]; hermesStackFrameIdToSentryFrameIdMap: Map; - resources: string[]; } { const frames: ThreadCpuFrame[] = []; const hermesStackFrameIdToSentryFrameIdMap = new Map(); @@ -149,7 +142,6 @@ function mapFrames(hermesStackFrames: Record): Debug return images; } +let bundleFilenameCached: string | null | undefined; +/** + * This function assumes that the SDK is in the same bundle as the app code. + * @returns the bundle filename from an artificially created error + */ +function getBundleFilename(): string | null { + if (typeof bundleFilenameCached !== 'undefined') { + return bundleFilenameCached; + } + + const hub = getCurrentHub(); + if (!hub) { + bundleFilenameCached = null; + return null; + } + const client = hub.getClient(); + if (!client) { + bundleFilenameCached = null; + return null; + } + const options = client.getOptions(); + if (!options) { + bundleFilenameCached = null; + return null; + } + const stackParser = options.stackParser; + if (!stackParser) { + bundleFilenameCached = null; + return null; + } + + const error = new Error(); + if (!error.stack) { + bundleFilenameCached = null; + return null; + } + + const stack = stackParser(error.stack); + for (const frame of stack) { + if (frame.filename) { + bundleFilenameCached = frame.filename; + return bundleFilenameCached; + } + } + + bundleFilenameCached = null; + return null; +} + /** * Adds items to envelope if they are not already present - mutates the envelope. * @param envelope From 073450fea53e8efe495241ee3bc05c85955e9123 Mon Sep 17 00:00:00 2001 From: Krystof Woldrich Date: Fri, 13 Oct 2023 15:51:41 +0200 Subject: [PATCH 06/16] Remove unnecessary lookup logic as only one bundle is supported --- src/js/profiling/utils.ts | 130 ++++++-------------------------------- 1 file changed, 18 insertions(+), 112 deletions(-) diff --git a/src/js/profiling/utils.ts b/src/js/profiling/utils.ts index 382066c04b..9df26ec71b 100644 --- a/src/js/profiling/utils.ts +++ b/src/js/profiling/utils.ts @@ -1,7 +1,7 @@ -import { getCurrentHub } from '@sentry/core'; -import type { DebugImage, Envelope, Event, Profile, StackFrame, StackParser } from '@sentry/types'; +import type { DebugImage, Envelope, Event, Profile } from '@sentry/types'; import { forEachEnvelopeItem, GLOBAL_OBJ, logger } from '@sentry/utils'; +import { DEFAULT_BUNDLE_NAME } from './hermes'; import type { RawThreadCpuProfile } from './types'; const ACTIVE_THREAD_ID_STRING = '0'; @@ -136,8 +136,6 @@ function createProfilePayload( } } - const bundleFilename = getBundleFilename(); - const profile: Profile = { event_id: profile_id, timestamp: new Date(start_timestamp).toISOString(), @@ -169,132 +167,40 @@ function createProfilePayload( active_thread_id: ACTIVE_THREAD_ID_STRING, }, debug_meta: { - images: bundleFilename ? applyDebugMetadata([bundleFilename]) : [], + images: getDebugMetadata(), }, }; return profile; } -const debugIdStackParserCache = new WeakMap>(); /** - * Applies debug meta data to an event from a list of paths to resources (sourcemaps) - * https://github.com/getsentry/sentry-javascript/blob/ec9eebc5a09a0ce2e51cd4080e729a48c8b2fe97/packages/browser/src/profiling/utils.ts#L328 + * Returns debug meta images of the loaded bundle. */ -export function applyDebugMetadata(resource_paths: ReadonlyArray): DebugImage[] { - const debugIdMap = GLOBAL_OBJ._sentryDebugIds; - if (!debugIdMap) { +export function getDebugMetadata(): DebugImage[] { + if (!DEFAULT_BUNDLE_NAME) { return []; } - const hub = getCurrentHub(); - if (!hub) { - return []; - } - const client = hub.getClient(); - if (!client) { - return []; - } - const options = client.getOptions(); - if (!options) { - return []; - } - const stackParser = options.stackParser; - if (!stackParser) { + const debugIdMap = GLOBAL_OBJ._sentryDebugIds; + if (!debugIdMap) { return []; } - let debugIdStackFramesCache: Map; - const cachedDebugIdStackFrameCache = debugIdStackParserCache.get(stackParser); - if (cachedDebugIdStackFrameCache) { - debugIdStackFramesCache = cachedDebugIdStackFrameCache; - } else { - debugIdStackFramesCache = new Map(); - debugIdStackParserCache.set(stackParser, debugIdStackFramesCache); - } - - // Build a map of filename -> debug_id - const filenameDebugIdMap = Object.keys(debugIdMap).reduce>((acc, debugIdStackTrace) => { - let parsedStack: StackFrame[]; - const cachedParsedStack = debugIdStackFramesCache.get(debugIdStackTrace); - if (cachedParsedStack) { - parsedStack = cachedParsedStack; - } else { - parsedStack = stackParser(debugIdStackTrace); - debugIdStackFramesCache.set(debugIdStackTrace, parsedStack); - } - - for (let i = parsedStack.length - 1; i >= 0; i--) { - const stackFrame = parsedStack[i]; - if (stackFrame.filename) { - acc[stackFrame.filename] = debugIdMap[debugIdStackTrace]; - break; - } - } - return acc; - }, {}); - - const images: DebugImage[] = []; - for (const path of resource_paths) { - if (path && filenameDebugIdMap[path]) { - images.push({ - type: 'sourcemap', - code_file: path, - debug_id: filenameDebugIdMap[path] as string, - }); - } - } - - return images; -} - -let bundleFilenameCached: string | null | undefined; -/** - * This function assumes that the SDK is in the same bundle as the app code. - * @returns the bundle filename from an artificially created error - */ -function getBundleFilename(): string | null { - if (typeof bundleFilenameCached !== 'undefined') { - return bundleFilenameCached; - } - - const hub = getCurrentHub(); - if (!hub) { - bundleFilenameCached = null; - return null; - } - const client = hub.getClient(); - if (!client) { - bundleFilenameCached = null; - return null; - } - const options = client.getOptions(); - if (!options) { - bundleFilenameCached = null; - return null; - } - const stackParser = options.stackParser; - if (!stackParser) { - bundleFilenameCached = null; - return null; - } - - const error = new Error(); - if (!error.stack) { - bundleFilenameCached = null; - return null; + const debugIds = Object.values(debugIdMap); + if (!debugIds.length) { + return []; } - const stack = stackParser(error.stack); - for (const frame of stack) { - if (frame.filename) { - bundleFilenameCached = frame.filename; - return bundleFilenameCached; - } + if (debugIds.length > 1) { + logger.warn('[Profiling] Multiple debug images found, but only one one bundle is supported. Using the first one...'); } - bundleFilenameCached = null; - return null; + return [{ + code_file: DEFAULT_BUNDLE_NAME, + debug_id: debugIds[0], + type: 'sourcemap', + }]; } /** From affcee8c816a8d003a2dc1625aa75dde34409fdf Mon Sep 17 00:00:00 2001 From: Krystof Woldrich Date: Fri, 13 Oct 2023 16:00:15 +0200 Subject: [PATCH 07/16] Fix build --- src/js/profiling/convertHermesProfile.ts | 7 +------ 1 file changed, 1 insertion(+), 6 deletions(-) diff --git a/src/js/profiling/convertHermesProfile.ts b/src/js/profiling/convertHermesProfile.ts index 58f1ff6952..36daa2ba9b 100644 --- a/src/js/profiling/convertHermesProfile.ts +++ b/src/js/profiling/convertHermesProfile.ts @@ -1,22 +1,17 @@ import type { FrameId, StackId, ThreadCpuFrame, ThreadCpuSample, ThreadCpuStack, ThreadId } from '@sentry/types'; import { logger } from '@sentry/utils'; -import { Platform } from 'react-native'; -import { ANDROID_DEFAULT_BUNDLE_NAME, IOS_DEFAULT_BUNDLE_NAME } from '../integrations/rewriteframes'; import type * as Hermes from './hermes'; -import { parseHermesStackFrameFunctionName } from './hermes'; +import { parseHermesJSStackFrame } from './hermes'; import { MAX_PROFILE_DURATION_MS } from './integration'; import type { RawThreadCpuProfile } from './types'; const PLACEHOLDER_THREAD_ID_STRING = '0'; const MS_TO_NS = 1e6; const MAX_PROFILE_DURATION_NS = MAX_PROFILE_DURATION_MS * MS_TO_NS; -const ANONYMOUS_FUNCTION_NAME = 'anonymous'; const UNKNOWN_STACK_ID = -1; const JS_THREAD_NAME = 'JavaScriptThread'; const JS_THREAD_PRIORITY = 1; -const DEFAULT_BUNDLE_NAME = - Platform.OS === 'android' ? ANDROID_DEFAULT_BUNDLE_NAME : Platform.OS === 'ios' ? IOS_DEFAULT_BUNDLE_NAME : undefined; /** * Converts a Hermes profile to a Sentry profile. From 5dd4ea74cd576dc1562d049b828034f06ee84a88 Mon Sep 17 00:00:00 2001 From: Krystof Woldrich Date: Fri, 13 Oct 2023 16:05:27 +0200 Subject: [PATCH 08/16] Fix build --- src/js/profiling/hermes.ts | 2 +- src/js/profiling/utils.ts | 8 ++++---- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/src/js/profiling/hermes.ts b/src/js/profiling/hermes.ts index fb71a48f80..efa4ba177e 100644 --- a/src/js/profiling/hermes.ts +++ b/src/js/profiling/hermes.ts @@ -60,7 +60,7 @@ export interface ParsedHermesStackFrame { colno?: number; } -const DEFAULT_BUNDLE_NAME = Platform.OS === 'android' +export const DEFAULT_BUNDLE_NAME = Platform.OS === 'android' ? ANDROID_DEFAULT_BUNDLE_NAME : Platform.OS === 'ios' ? IOS_DEFAULT_BUNDLE_NAME diff --git a/src/js/profiling/utils.ts b/src/js/profiling/utils.ts index bc21d1a28d..580c738b22 100644 --- a/src/js/profiling/utils.ts +++ b/src/js/profiling/utils.ts @@ -185,18 +185,18 @@ export function getDebugMetadata(): DebugImage[] { return []; } - const debugIds = Object.values(debugIdMap); - if (!debugIds.length) { + const debugIdsKeys = Object.keys(debugIdMap); + if (!debugIdsKeys.length) { return []; } - if (debugIds.length > 1) { + if (debugIdsKeys.length > 1) { logger.warn('[Profiling] Multiple debug images found, but only one one bundle is supported. Using the first one...'); } return [{ code_file: DEFAULT_BUNDLE_NAME, - debug_id: debugIds[0], + debug_id: debugIdMap[debugIdsKeys[0]], type: 'sourcemap', }]; } From 81c2acb98661a33afc42d2b0c1e4f5c06c4ca22e Mon Sep 17 00:00:00 2001 From: Krystof Woldrich Date: Fri, 13 Oct 2023 16:51:52 +0200 Subject: [PATCH 09/16] fix lint --- src/js/profiling/hermes.ts | 14 ++++++-------- test/profiling/hermes.test.ts | 32 ++++++++++++++++++-------------- 2 files changed, 24 insertions(+), 22 deletions(-) diff --git a/src/js/profiling/hermes.ts b/src/js/profiling/hermes.ts index fb71a48f80..65c6577586 100644 --- a/src/js/profiling/hermes.ts +++ b/src/js/profiling/hermes.ts @@ -32,8 +32,8 @@ export interface Sample { export interface StackFrame { // Hermes Bytecode - funcVirtAddr?: string, - offset?: string, + funcVirtAddr?: string; + offset?: string; // JavaScript line?: string; @@ -60,11 +60,8 @@ export interface ParsedHermesStackFrame { colno?: number; } -const DEFAULT_BUNDLE_NAME = Platform.OS === 'android' - ? ANDROID_DEFAULT_BUNDLE_NAME - : Platform.OS === 'ios' - ? IOS_DEFAULT_BUNDLE_NAME - : undefined; +const DEFAULT_BUNDLE_NAME = + Platform.OS === 'android' ? ANDROID_DEFAULT_BUNDLE_NAME : Platform.OS === 'ios' ? IOS_DEFAULT_BUNDLE_NAME : undefined; const ANONYMOUS_FUNCTION_NAME = 'anonymous'; /** @@ -93,7 +90,8 @@ export function parseHermesJSStackFrame(frame: StackFrame): ParsedHermesStackFra // JavaScript const indexOfLeftParenthesis = frame.name.indexOf('('); return { - function: (indexOfLeftParenthesis !== -1 && (frame.name.substring(0, indexOfLeftParenthesis) || ANONYMOUS_FUNCTION_NAME)) || + function: + (indexOfLeftParenthesis !== -1 && (frame.name.substring(0, indexOfLeftParenthesis) || ANONYMOUS_FUNCTION_NAME)) || frame.name, file: DEFAULT_BUNDLE_NAME, lineno: frame.line !== undefined ? Number(frame.line) : undefined, diff --git a/test/profiling/hermes.test.ts b/test/profiling/hermes.test.ts index ebc6ebf2e8..63ba25b66a 100644 --- a/test/profiling/hermes.test.ts +++ b/test/profiling/hermes.test.ts @@ -1,4 +1,4 @@ -import type { ParsedHermesStackFrame} from '../../src/js/profiling/hermes'; +import type { ParsedHermesStackFrame } from '../../src/js/profiling/hermes'; import { parseHermesJSStackFrame } from '../../src/js/profiling/hermes'; describe('hermes', () => { @@ -10,8 +10,8 @@ describe('hermes', () => { line: '1610', column: '33', category: 'JavaScript', - }) - ).toEqual( { + }), + ).toEqual({ function: 'fooA', file: 'app:///main.jsbundle', lineno: 1610, @@ -23,10 +23,12 @@ describe('hermes', () => { parseHermesJSStackFrame({ name: '[root]', category: 'root', - }) - ).toEqual(expect.objectContaining( { - function: '[root]', - })); + }), + ).toEqual( + expect.objectContaining({ + function: '[root]', + }), + ); }); test('parse only file name', () => { expect( @@ -35,8 +37,8 @@ describe('hermes', () => { line: '1610', column: '33', category: 'JavaScript', - }) - ).toEqual( { + }), + ).toEqual({ function: 'anonymous', file: 'app:///main.jsbundle', lineno: 1610, @@ -48,11 +50,13 @@ describe('hermes', () => { parseHermesJSStackFrame({ name: 'fooA', category: 'JavaScript', - }) - ).toEqual(expect.objectContaining( { - function: 'fooA', - file: 'app:///main.jsbundle', - })); + }), + ).toEqual( + expect.objectContaining({ + function: 'fooA', + file: 'app:///main.jsbundle', + }), + ); }); }); }); From 729876e4d8e2aa2c3b65538ee71d7bda22ebfb6f Mon Sep 17 00:00:00 2001 From: Krystof Woldrich Date: Fri, 13 Oct 2023 16:54:29 +0200 Subject: [PATCH 10/16] fix lint --- src/js/profiling/hermes.ts | 7 ++----- src/js/profiling/utils.ts | 16 ++++++++++------ 2 files changed, 12 insertions(+), 11 deletions(-) diff --git a/src/js/profiling/hermes.ts b/src/js/profiling/hermes.ts index d5c81664c9..2b7ddfe4f7 100644 --- a/src/js/profiling/hermes.ts +++ b/src/js/profiling/hermes.ts @@ -60,11 +60,8 @@ export interface ParsedHermesStackFrame { colno?: number; } -export const DEFAULT_BUNDLE_NAME = Platform.OS === 'android' - ? ANDROID_DEFAULT_BUNDLE_NAME - : Platform.OS === 'ios' - ? IOS_DEFAULT_BUNDLE_NAME - : undefined; +export const DEFAULT_BUNDLE_NAME = + Platform.OS === 'android' ? ANDROID_DEFAULT_BUNDLE_NAME : Platform.OS === 'ios' ? IOS_DEFAULT_BUNDLE_NAME : undefined; const ANONYMOUS_FUNCTION_NAME = 'anonymous'; /** diff --git a/src/js/profiling/utils.ts b/src/js/profiling/utils.ts index 580c738b22..d1d73f3797 100644 --- a/src/js/profiling/utils.ts +++ b/src/js/profiling/utils.ts @@ -191,14 +191,18 @@ export function getDebugMetadata(): DebugImage[] { } if (debugIdsKeys.length > 1) { - logger.warn('[Profiling] Multiple debug images found, but only one one bundle is supported. Using the first one...'); + logger.warn( + '[Profiling] Multiple debug images found, but only one one bundle is supported. Using the first one...', + ); } - return [{ - code_file: DEFAULT_BUNDLE_NAME, - debug_id: debugIdMap[debugIdsKeys[0]], - type: 'sourcemap', - }]; + return [ + { + code_file: DEFAULT_BUNDLE_NAME, + debug_id: debugIdMap[debugIdsKeys[0]], + type: 'sourcemap', + }, + ]; } /** From 7d608fbfd8e91c2c519f5f3998723c58e3da250f Mon Sep 17 00:00:00 2001 From: Krystof Woldrich Date: Fri, 13 Oct 2023 18:47:49 +0200 Subject: [PATCH 11/16] change profile platform to javascript --- src/js/profiling/utils.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/js/profiling/utils.ts b/src/js/profiling/utils.ts index d1d73f3797..85230e4ef6 100644 --- a/src/js/profiling/utils.ts +++ b/src/js/profiling/utils.ts @@ -137,7 +137,7 @@ function createProfilePayload( const profile: Profile = { event_id: profile_id, timestamp: new Date(start_timestamp).toISOString(), - platform: 'node', + platform: 'javascript', version: '1', release: release, environment: environment, From 102f1f16c49bcb7ae4205895f1bee7336775a1e1 Mon Sep 17 00:00:00 2001 From: Krystof Woldrich Date: Fri, 13 Oct 2023 19:07:53 +0200 Subject: [PATCH 12/16] Mark root as in app false --- src/js/profiling/hermes.ts | 4 ++++ src/js/profiling/types.ts | 7 ++++++- test/profiling/convertHermesProfile.test.ts | 1 + 3 files changed, 11 insertions(+), 1 deletion(-) diff --git a/src/js/profiling/hermes.ts b/src/js/profiling/hermes.ts index 2b7ddfe4f7..0e9136e408 100644 --- a/src/js/profiling/hermes.ts +++ b/src/js/profiling/hermes.ts @@ -58,6 +58,7 @@ export interface ParsedHermesStackFrame { file?: string; lineno?: number; colno?: number; + in_app?: boolean; } export const DEFAULT_BUNDLE_NAME = @@ -71,6 +72,9 @@ const ANONYMOUS_FUNCTION_NAME = 'anonymous'; export function parseHermesJSStackFrame(frame: StackFrame): ParsedHermesStackFrame { if (frame.category !== 'JavaScript') { // Native + if (frame.name === '[root]') { + return { function: frame.name, in_app: false }; + } return { function: frame.name }; } diff --git a/src/js/profiling/types.ts b/src/js/profiling/types.ts index f03d8bcf6f..d2d669c013 100644 --- a/src/js/profiling/types.ts +++ b/src/js/profiling/types.ts @@ -1,6 +1,11 @@ -import type { ThreadCpuProfile } from '@sentry/types'; +import type { ThreadCpuFrame as SentryThreadCpuFrame, ThreadCpuProfile } from '@sentry/types'; + +export interface ThreadCpuFrame extends SentryThreadCpuFrame { + in_app?: boolean; +} export interface RawThreadCpuProfile extends ThreadCpuProfile { + frames: ThreadCpuFrame[]; profile_id?: string; active_thread_id: string; } diff --git a/test/profiling/convertHermesProfile.test.ts b/test/profiling/convertHermesProfile.test.ts index 45fe3391f5..655971b56a 100644 --- a/test/profiling/convertHermesProfile.test.ts +++ b/test/profiling/convertHermesProfile.test.ts @@ -84,6 +84,7 @@ describe('convert hermes profile to sentry profile', () => { frames: [ { function: '[root]', + in_app: false, }, { colno: 33, From 7e33cfad649ff8376d162beeb917cbb086e583b3 Mon Sep 17 00:00:00 2001 From: Krystof Woldrich Date: Wed, 18 Oct 2023 13:06:19 +0200 Subject: [PATCH 13/16] Use abs_path like error stacktrace frames --- src/js/profiling/convertHermesProfile.ts | 40 +++++++++++++++++++- src/js/profiling/hermes.ts | 47 ------------------------ 2 files changed, 39 insertions(+), 48 deletions(-) diff --git a/src/js/profiling/convertHermesProfile.ts b/src/js/profiling/convertHermesProfile.ts index 36daa2ba9b..bb2c8547ca 100644 --- a/src/js/profiling/convertHermesProfile.ts +++ b/src/js/profiling/convertHermesProfile.ts @@ -2,7 +2,7 @@ import type { FrameId, StackId, ThreadCpuFrame, ThreadCpuSample, ThreadCpuStack, import { logger } from '@sentry/utils'; import type * as Hermes from './hermes'; -import { parseHermesJSStackFrame } from './hermes'; +import { DEFAULT_BUNDLE_NAME } from './hermes'; import { MAX_PROFILE_DURATION_MS } from './integration'; import type { RawThreadCpuProfile } from './types'; @@ -173,3 +173,41 @@ function mapStacks( hermesStackToSentryStackMap, }; } + +/** + * Parses Hermes StackFrame to Sentry StackFrame. + * For native frames only function name is returned, for Hermes bytecode the line and column are calculated. + */ +export function parseHermesJSStackFrame(frame: Hermes.StackFrame): ThreadCpuFrame { + if (frame.category !== 'JavaScript') { + // Native + if (frame.name === '[root]') { + return { function: frame.name, in_app: false }; + } + return { function: frame.name }; + } + + if (frame.funcVirtAddr !== undefined && frame.offset !== undefined) { + // Hermes Bytecode + return { + function: frame.name, + abs_path: DEFAULT_BUNDLE_NAME, + // https://github.com/krystofwoldrich/metro/blob/417e6f276ff9422af6039fc4d1bce41fcf7d9f46/packages/metro-symbolicate/src/Symbolication.js#L298-L301 + // Hermes lineno is hardcoded 1, currently only one bundle symbolication is supported by metro-symbolicate and thus by us. + lineno: 1, + // Hermes colno is 0-based, while Sentry is 1-based + colno: Number(frame.funcVirtAddr) + Number(frame.offset) + 1, + }; + } + + // JavaScript + const indexOfLeftParenthesis = frame.name.indexOf('('); + return { + function: + indexOfLeftParenthesis !== -1 && frame.name.substring(0, indexOfLeftParenthesis) || + frame.name, + abs_path: DEFAULT_BUNDLE_NAME, + lineno: frame.line !== undefined ? Number(frame.line) : undefined, + colno: frame.column !== undefined ? Number(frame.column) : undefined, + }; +} diff --git a/src/js/profiling/hermes.ts b/src/js/profiling/hermes.ts index 0e9136e408..f9c98eb127 100644 --- a/src/js/profiling/hermes.ts +++ b/src/js/profiling/hermes.ts @@ -53,55 +53,8 @@ export interface Profile { stackFrames: Record; } -export interface ParsedHermesStackFrame { - function: string; - file?: string; - lineno?: number; - colno?: number; - in_app?: boolean; -} - export const DEFAULT_BUNDLE_NAME = Platform.OS === 'android' ? ANDROID_DEFAULT_BUNDLE_NAME : Platform.OS === 'ios' ? IOS_DEFAULT_BUNDLE_NAME : undefined; -const ANONYMOUS_FUNCTION_NAME = 'anonymous'; - -/** - * Parses Hermes StackFrame to Sentry StackFrame. - * For native frames only function name is returned, for Hermes bytecode the line and column are calculated. - */ -export function parseHermesJSStackFrame(frame: StackFrame): ParsedHermesStackFrame { - if (frame.category !== 'JavaScript') { - // Native - if (frame.name === '[root]') { - return { function: frame.name, in_app: false }; - } - return { function: frame.name }; - } - - if (frame.funcVirtAddr !== undefined && frame.offset !== undefined) { - // Hermes Bytecode - return { - function: frame.name || ANONYMOUS_FUNCTION_NAME, - file: DEFAULT_BUNDLE_NAME, - // https://github.com/krystofwoldrich/metro/blob/417e6f276ff9422af6039fc4d1bce41fcf7d9f46/packages/metro-symbolicate/src/Symbolication.js#L298-L301 - // Hermes lineno is hardcoded 1, currently only one bundle symbolication is supported by metro-symbolicate and thus by us. - lineno: 1, - // Hermes colno is 0-based, while Sentry is 1-based - colno: Number(frame.funcVirtAddr) + Number(frame.offset) + 1, - }; - } - - // JavaScript - const indexOfLeftParenthesis = frame.name.indexOf('('); - return { - function: - (indexOfLeftParenthesis !== -1 && (frame.name.substring(0, indexOfLeftParenthesis) || ANONYMOUS_FUNCTION_NAME)) || - frame.name, - file: DEFAULT_BUNDLE_NAME, - lineno: frame.line !== undefined ? Number(frame.line) : undefined, - colno: frame.column !== undefined ? Number(frame.column) : undefined, - }; -} const MS_TO_NS: number = 1e6; From 4b1c004884a21c73984b7d3e5a364c9bc3b5c57a Mon Sep 17 00:00:00 2001 From: Krystof Woldrich Date: Wed, 18 Oct 2023 13:26:50 +0200 Subject: [PATCH 14/16] Fix tests --- src/js/profiling/convertHermesProfile.ts | 4 +--- test/profiling/convertHermesProfile.test.ts | 8 ++++---- test/profiling/hermes.test.ts | 20 ++++++++++---------- 3 files changed, 15 insertions(+), 17 deletions(-) diff --git a/src/js/profiling/convertHermesProfile.ts b/src/js/profiling/convertHermesProfile.ts index bb2c8547ca..aa62146f00 100644 --- a/src/js/profiling/convertHermesProfile.ts +++ b/src/js/profiling/convertHermesProfile.ts @@ -203,9 +203,7 @@ export function parseHermesJSStackFrame(frame: Hermes.StackFrame): ThreadCpuFram // JavaScript const indexOfLeftParenthesis = frame.name.indexOf('('); return { - function: - indexOfLeftParenthesis !== -1 && frame.name.substring(0, indexOfLeftParenthesis) || - frame.name, + function: indexOfLeftParenthesis !== -1 ? frame.name.substring(0, indexOfLeftParenthesis) || undefined : frame.name, abs_path: DEFAULT_BUNDLE_NAME, lineno: frame.line !== undefined ? Number(frame.line) : undefined, colno: frame.column !== undefined ? Number(frame.column) : undefined, diff --git a/test/profiling/convertHermesProfile.test.ts b/test/profiling/convertHermesProfile.test.ts index 655971b56a..6e2ccfdd8e 100644 --- a/test/profiling/convertHermesProfile.test.ts +++ b/test/profiling/convertHermesProfile.test.ts @@ -88,20 +88,20 @@ describe('convert hermes profile to sentry profile', () => { }, { colno: 33, - file: 'app:///main.jsbundle', + abs_path: 'app:///main.jsbundle', function: 'fooA', lineno: 1610, }, { colno: 21, - file: 'app:///main.jsbundle', + abs_path: 'app:///main.jsbundle', function: 'fooB', lineno: 1616, }, { colno: 18, - file: 'app:///main.jsbundle', - function: 'anonymous', + abs_path: 'app:///main.jsbundle', + function: undefined, lineno: 1627, }, ], diff --git a/test/profiling/hermes.test.ts b/test/profiling/hermes.test.ts index 63ba25b66a..0d7852d4f0 100644 --- a/test/profiling/hermes.test.ts +++ b/test/profiling/hermes.test.ts @@ -1,5 +1,6 @@ -import type { ParsedHermesStackFrame } from '../../src/js/profiling/hermes'; -import { parseHermesJSStackFrame } from '../../src/js/profiling/hermes'; +import type { ThreadCpuFrame } from '@sentry/types'; + +import { parseHermesJSStackFrame } from '../../src/js/profiling/convertHermesProfile'; describe('hermes', () => { describe('parseHermesStackFrameName', () => { @@ -11,9 +12,9 @@ describe('hermes', () => { column: '33', category: 'JavaScript', }), - ).toEqual({ + ).toEqual({ function: 'fooA', - file: 'app:///main.jsbundle', + abs_path: 'app:///main.jsbundle', lineno: 1610, colno: 33, }); @@ -25,7 +26,7 @@ describe('hermes', () => { category: 'root', }), ).toEqual( - expect.objectContaining({ + expect.objectContaining({ function: '[root]', }), ); @@ -38,9 +39,8 @@ describe('hermes', () => { column: '33', category: 'JavaScript', }), - ).toEqual({ - function: 'anonymous', - file: 'app:///main.jsbundle', + ).toEqual({ + abs_path: 'app:///main.jsbundle', lineno: 1610, colno: 33, }); @@ -52,9 +52,9 @@ describe('hermes', () => { category: 'JavaScript', }), ).toEqual( - expect.objectContaining({ + expect.objectContaining({ function: 'fooA', - file: 'app:///main.jsbundle', + abs_path: 'app:///main.jsbundle', }), ); }); From facf3eda89bdb65ad87634908ab84a0ddd3a652b Mon Sep 17 00:00:00 2001 From: Krystof Woldrich Date: Thu, 19 Oct 2023 15:38:45 +0200 Subject: [PATCH 15/16] remove duplicate types --- src/js/profiling/types.ts | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/src/js/profiling/types.ts b/src/js/profiling/types.ts index d2d669c013..4880dd1f54 100644 --- a/src/js/profiling/types.ts +++ b/src/js/profiling/types.ts @@ -1,8 +1,4 @@ -import type { ThreadCpuFrame as SentryThreadCpuFrame, ThreadCpuProfile } from '@sentry/types'; - -export interface ThreadCpuFrame extends SentryThreadCpuFrame { - in_app?: boolean; -} +import type { ThreadCpuFrame, ThreadCpuProfile } from '@sentry/types'; export interface RawThreadCpuProfile extends ThreadCpuProfile { frames: ThreadCpuFrame[]; From 0dcaa07e9be55642978bd2782b910a3b82b37731 Mon Sep 17 00:00:00 2001 From: Krystof Woldrich Date: Fri, 27 Oct 2023 12:30:20 +0200 Subject: [PATCH 16/16] Update changelog --- CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index c699e3b42f..376a5214cf 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,7 +4,7 @@ ### Features -- Send debug information with Profiles for symbolicated stack traces ([#3338](https://github.com/getsentry/sentry-react-native/pull/3338)) +- Send Source Maps Debug ID for symbolicated Profiles ([#3343](https://github.com/getsentry/sentry-react-native/pull/3343)) ### Fixes