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: Add source map debug ids #7068

Merged
merged 14 commits into from Feb 8, 2023
Merged

feat: Add source map debug ids #7068

merged 14 commits into from Feb 8, 2023

Conversation

lforst
Copy link
Member

@lforst lforst commented Feb 6, 2023

Ref: getsentry/team-webplatform-meta#17

Most importantly this PR adds adding debug ids to individual stack frames that have previously been injected into the bundle.

Additionally, this PR adds a small lil' script to inject debug IDs into JS bundles.

@github-actions
Copy link
Contributor

github-actions bot commented Feb 6, 2023

size-limit report 📦

Path Size
@sentry/browser - ES5 CDN Bundle (gzipped + minified) 20.06 KB (+0.78% 🔺)
@sentry/browser - ES5 CDN Bundle (minified) 62.19 KB (+0.77% 🔺)
@sentry/browser - ES6 CDN Bundle (gzipped + minified) 18.69 KB (+0.56% 🔺)
@sentry/browser - ES6 CDN Bundle (minified) 55.33 KB (+0.55% 🔺)
@sentry/browser - Webpack (gzipped + minified) 20.42 KB (+0.51% 🔺)
@sentry/browser - Webpack (minified) 66.77 KB (+0.45% 🔺)
@sentry/react - Webpack (gzipped + minified) 20.45 KB (+0.51% 🔺)
@sentry/nextjs Client - Webpack (gzipped + minified) 47.74 KB (+0.22% 🔺)
@sentry/browser + @sentry/tracing - ES5 CDN Bundle (gzipped + minified) 26.97 KB (+0.59% 🔺)
@sentry/browser + @sentry/tracing - ES6 CDN Bundle (gzipped + minified) 25.23 KB (+0.43% 🔺)
@sentry/replay ES6 CDN Bundle (gzipped + minified) 43.85 KB (+0.01% 🔺)
@sentry/replay - Webpack (gzipped + minified) 38.76 KB (0%)
@sentry/browser + @sentry/tracing + @sentry/replay - ES6 CDN Bundle (gzipped + minified) 61.57 KB (+0.16% 🔺)

const INJECTOR_CODE =
'\n!function(){try{var e="undefined"!=typeof window?window:"undefined"!=typeof global?global:"undefined"!=typeof self?self:{},n=(new Error).stack;n&&(e._sentryDebugIds=e._sentryDebugIds||{},e._sentryDebugIds[n]="__DEBUG_ID__")}catch(e){}}()';

const DEBUG_COMMENT_CODE = '\n//# sentryDebugId=__DEBUG_ID__';
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

let's call this __SENTRY_DEBUG_ID__ to stay on the safe side.

Suggested change
const DEBUG_COMMENT_CODE = '\n//# sentryDebugId=__DEBUG_ID__';
const DEBUG_COMMENT_CODE = '\n//# sentryDebugId=__DEBUG_ID__';

exception.stacktrace?.frames?.forEach(eventFrame => {
let debugId: string | undefined;
Object.keys(GLOBAL_OBJ._sentryDebugIds!).forEach(sentryDebugIdStack => {
const stackFrames = stackParser(sentryDebugIdStack);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's cache this result.

@lforst lforst changed the title feat: Add debug ids feat: Add source map debug ids Feb 7, 2023
Comment on lines 27 to 28
"yargs": "^17.6.2",
"uuid": "^9.0.0"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

let's try and avoid adding these deps if we can!

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

uuid we can just generate ourselves, we don't need strict uuid4 (copy paste the implementation in Sentry you have to)

yargs is more painful, but just parse the args ourselves. We don't have a super complicated CLI atm, so no need to worry about ordering concerns or similar for now.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As discussed: Since this is only a poc I removed the CLI

@lforst lforst marked this pull request as ready for review February 7, 2023 13:35
let debugIdCache = debugIdParserCache.get(parser);
if (!debugIdCache) {
debugIdCache = new Map();
debugIdParserCache.set(parser, debugIdCache);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

m: This is unbounded, which is not a problem for browser js, but it is for node. We may have re-think this as a result, or add some crude eviction mechanism.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As discussed: The upper bound is the number of modules that have been injected with the debug id times the amount of stack trace parsers. I am gonna go ahead and say this is safely cachable.

@lforst lforst merged commit 026072e into develop Feb 8, 2023
@lforst lforst deleted the lforst-debug-ids-poc branch February 8, 2023 09:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants