feat(profiling): js self profiling #7273

JonasBa · 2023-02-23T16:09:02Z

This pr adds support for JS self profiling to @sentry/browser package. This is a POC stage and we want to test it on our sentry dashboard - the current approach is to take the js profiler output and convert it to our sample format. Due to overhead reasons I do not think this will be a good long term solution, but at least for now, it allows us to visualize the profiles and test the feasibility of the API without a significant investment.

To support automated browser instrumentation (route navigation), we had to make a small change to @sentry/tracing to support. Tldr, browser tracing uses idle transactions as opposed to regular transactions and our hubExtension patch was never being called. Thanks @k-fish for the help there.

~~I will be adding some more tests to cover the profile conversion logic~~ -> done

…G_BUILD__

github-actions · 2023-02-23T16:25:57Z

size-limit report 📦

Path	Size
@sentry/browser - ES5 CDN Bundle (gzipped + minified)	20.11 KB (-0.02% 🔽)
@sentry/browser - ES5 CDN Bundle (minified)	62.49 KB (0%)
@sentry/browser - ES6 CDN Bundle (gzipped + minified)	18.74 KB (-0.02% 🔽)
@sentry/browser - ES6 CDN Bundle (minified)	55.5 KB (0%)
@sentry/browser - Webpack (gzipped + minified)	20.48 KB (-0.01% 🔽)
@sentry/browser - Webpack (minified)	66.94 KB (0%)
@sentry/react - Webpack (gzipped + minified)	20.51 KB (-0.01% 🔽)
@sentry/nextjs Client - Webpack (gzipped + minified)	48.08 KB (+0.05% 🔺)
@sentry/browser + @sentry/tracing - ES5 CDN Bundle (gzipped + minified)	27.06 KB (+0.08% 🔺)
@sentry/browser + @sentry/tracing - ES6 CDN Bundle (gzipped + minified)	25.31 KB (+0.11% 🔺)
@sentry/replay ES6 CDN Bundle (gzipped + minified)	42.86 KB (-0.01% 🔽)
@sentry/replay - Webpack (gzipped + minified)	36.93 KB (0%)
@sentry/browser + @sentry/tracing + @sentry/replay - ES6 CDN Bundle (gzipped + minified)	60.46 KB (+0.04% 🔺)
@sentry/browser + @sentry/replay - ES6 CDN Bundle (gzipped + minified)	53.99 KB (-0.01% 🔽)

… causing long spans again

github-actions · 2023-02-24T00:21:16Z

Replay SDK metrics 🚀

		Plain	+Sentry			+Replay
	Revision	Value	Value	Diff	Ratio	Value	Diff	Ratio
LCP	This PR `4b95c04`	65.31 ms	95.62 ms	+30.31 ms	+46.40 %	127.81 ms	+62.50 ms	+95.69 %
LCP	Previous `4b95c04`	74.67 ms	100.14 ms	+25.47 ms	+34.11 %	132.23 ms	+57.56 ms	+77.09 %
CLS	This PR `4b95c04`	0.06 ms	0.06 ms	+0.00 ms	+0.05 %	0.06 ms	+0.00 ms	+0.13 %
CLS	Previous `4b95c04`	0.06 ms	0.06 ms	-0.00 ms	-0.79 %	0.06 ms	-0.00 ms	-0.47 %
CPU	This PR `4b95c04`	13.21 %	13.60 %	+0.38 pp	+2.91 %	18.64 %	+5.43 pp	+41.07 %
CPU	Previous `4b95c04`	12.32 %	12.48 %	+0.16 pp	+1.28 %	20.27 %	+7.94 pp	+64.46 %
JS heap avg	This PR `4b95c04`	1.94 MB	2 MB	+61.95 kB	+3.19 %	2.87 MB	+931.53 kB	+47.96 %
JS heap avg	Previous `4b95c04`	1.94 MB	1.99 MB	+43.74 kB	+2.25 %	2.86 MB	+920.88 kB	+47.38 %
JS heap max	This PR `4b95c04`	2.3 MB	2.56 MB	+257.23 kB	+11.17 %	3.34 MB	+1.04 MB	+45.11 %
JS heap max	Previous `4b95c04`	2.3 MB	2.55 MB	+249.86 kB	+10.85 %	3.35 MB	+1.05 MB	+45.58 %
netTx	This PR `4b95c04`	0 B	0 B	0 B	n/a	2.22 kB	+2.22 kB	n/a
netTx	Previous `4b95c04`	0 B	0 B	0 B	n/a	2.21 kB	+2.21 kB	n/a
netRx	This PR `4b95c04`	0 B	0 B	0 B	n/a	41 B	+41 B	n/a
netRx	Previous `4b95c04`	0 B	0 B	0 B	n/a	41 B	+41 B	n/a
netCount	This PR `4b95c04`	0	0	0	n/a	1	+1	n/a
netCount	Previous `4b95c04`	0	0	0	n/a	1	+1	n/a
netTime	This PR `4b95c04`	0.00 ms	0.00 ms	0.00 ms	n/a	64.22 ms	+64.22 ms	n/a
netTime	Previous `4b95c04`	0.00 ms	0.00 ms	0.00 ms	n/a	90.32 ms	+90.32 ms	n/a

Previous results on branch: `develop`

Revision	LCP	CLS	CPU	JS heap avg	JS heap max	netTx	netRx	netCount	netTime
`4b95c04`	+57.56 ms	-0.00 ms	+7.94 pp	+920.88 kB	+1.05 MB	+2.21 kB	+41 B	+1	+90.32 ms
`e60cd02`	+56.25 ms	-0.00 ms	+6.32 pp	+927.44 kB	+1.06 MB	+2.21 kB	+41 B	+1	+117.55 ms
`e25c067`	+48.34 ms	+0.00 ms	+5.59 pp	+926.37 kB	+1.05 MB	+2.22 kB	+41 B	+1	+65.23 ms
`b1b249b`	+43.88 ms	+0.00 ms	+4.80 pp	+937.99 kB	+1.05 MB	+2.22 kB	+41 B	+1	+111.56 ms
`12e34d4`	+28.57 ms	+0.00 ms	+5.77 pp	+930.12 kB	+1.04 MB	+2.26 kB	+41 B	+1	+109.67 ms
`c46c56c`	+65.45 ms	-0.00 ms	+5.38 pp	+930.26 kB	+1.07 MB	+2.21 kB	+41 B	+1	+91.29 ms
`7f4c4ec`	+56.64 ms	-0.00 ms	+5.57 pp	+927.42 kB	+1.06 MB	+2.21 kB	+41 B	+1	+110.83 ms
`00d2360`	+55.18 ms	+0.00 ms	+2.23 pp	+934.14 kB	+1.05 MB	+2.22 kB	+41 B	+1	+71.65 ms

*) pp - percentage points - an absolute difference between two percentages.
Last updated: Mon, 27 Feb 2023 00:20:25 GMT

…rumentation

JonasBa · 2023-02-28T15:36:08Z

Are those replay metrics accurate? I did not make any changes to replay and unless profiling is installed and enabled, the metrics should not be impacted 🤔

lforst · 2023-02-28T15:40:47Z

Are those replay metrics accurate? I did not make any changes to replay and unless profiling is installed and enabled, the metrics should not be impacted 🤔

@JonasBa honestly I kinda ignore them but from what I've seen they're pretty fluctuating.

JonasBa · 2023-02-28T16:23:26Z

Since this is a large PR to review, some comments on how it works:

TLDR:

Sentry.startTransaction -> create transaction -> start profiler
transaction.finish -> finish transaction -> profiling integration inserts it to cache and forwards it -> txn is sent (this chain is synchronous)
                              ------> async profiler.stop -> attempt to pull processed txn event from profiling integration cache -> construct envelope -> profile is sent

Longer explanation:
Profiling package works by patching the startTransaction call on the main carrier (same as profiling-node) - by doing so, we can hook ourselves into the transaction lifecycle. That said, browser profiling packages differ slightly from node as calls to profiler.stop are async - this presents a problem because calling transaction.finish will immediately process the transaction event and send it to sentry, meaning by the time our profiler.stop method has resolved, the transaction had already been processed and our reference is "stale" and attaching a profile to the transaction metadata then is too late as the transaction will not be processed again (unless we were to send it twice)

A naive solution would be to await profiler.stop and block transaction.finish which would delay transaction.finish as well as block the event loop (from some testing, that can be in the range of hundreds of ms) which makes it prohibitive.

The second solution and the one I opted for was to introduce an event cache in our profiling integration which stores events that are carrying the profile context - this removes the need for awaiting profiler.stop at the expense of possibly dropping profiles (if the corresponding transaction event cannot be pulled from the cache, we cannot construct a profiling envelope). We would be dropping profiles in the event that max active transactions exceeds N (where N is the cache size = 20 = magic number. I'm open to increasing/decreasing that number, but my senses tell me that it is probably unlikely that more than 20 transactions would be active in the browser).

Supporting browser tracing
Til, but route tracing does not call startTransactions and instead uses idleTransactions which is why we expose the change in tracing which wraps the calls to startTransaction.

JonasBa · 2023-02-28T16:25:58Z

packages/profiling-browser/src/hubextensions.ts

@@ -0,0 +1,254 @@
+import { WINDOW } from '@sentry/browser';


exported code here is responsible for providing both the implementation of the patched startTransaction functionality (wrapTransactionWithProfiling) and the code that performs the patching addExtensionMethods

JonasBa · 2023-02-28T16:26:20Z

packages/profiling-browser/src/index.ts

@@ -0,0 +1,10 @@
+import { addExtensionMethods, wrapTransactionWithProfiling } from './hubextensions';


reexports the integration and patches the carrier

JonasBa · 2023-02-28T16:26:49Z

packages/profiling-browser/src/integration.ts

+ * rely on being able to pull the event from the cache when we need to construct the envelope. This makes the
+ * integration less reliable as we might be dropping profiles when the cache is full.
+ */
+export class BrowserProfilingIntegration implements Integration {


Listens for processed events with profile context and stores them in the cache

JonasBa · 2023-02-28T16:27:58Z

packages/profiling-browser/src/utils.ts

@@ -0,0 +1,400 @@
+import { WINDOW } from '@sentry/browser';
+import type {


All of this is mostly about constructing the profile envelope and converting from JS sampled format to the one we expect in Sentry

packages/profiling-browser/package.json

packages/profiling-browser/src/hubextensions.ts

packages/profiling-browser/src/utils.ts

packages/profiling-browser/test/integration.test.ts

packages/profiling-browser/test/utils.test.ts

packages/profiling-browser/package.json

…saction

JonasBa · 2023-03-02T14:29:14Z

@lforst profiling logic has been moved under the browser package so this is now ready for another review :)

lforst

Super clean PR 👌 Just some minor stuff.

packages/browser/src/profiling/hubextensions.ts

packages/browser/src/profiling/integration.ts

Co-authored-by: Luca Forstner <luca.forstner@sentry.io>

packages/browser/src/profiling/integration.ts

JonasBa added 7 commits February 17, 2023 10:04

feat(profiling): add boilerplate

2e1d6ef

feat(profiling): add files and remove isDebugBuild in favor of __DEBU…

1db7355

…G_BUILD__

ref(lint): fix linter errors on most files

38a0def

ref(profile): convert half the profile

9c5b80e

deps(sentry): start profiler

e38c5a1

ref(profiling): cleanup warnings and add os/device info

98e39b1

rollback yarn change

b552688

JonasBa and others added 2 commits February 23, 2023 16:37

feat(hub): move start transaction

9e3bd71

Remove span.start for profiler.stop so that we don't have idletimeout…

e134125

… causing long spans again

JonasBa added 4 commits February 24, 2023 08:35

feat(build): build esm/cjs and types

9b8ddd1

ref(profiling): update txn to txn|undefined type

a3be009

test(profiling): add utils and tests for browser profiling integration

360c1b8

test(tracing): assert onStartRouteTransaction is called by route inst…

0b2ab3d

…rumentation

JonasBa marked this pull request as ready for review February 27, 2023 16:55

JonasBa added 2 commits February 27, 2023 12:06

fix(verdaccio): add pkg to verdaccio config

c7ba372

eslint: run --fix

3ec2c90

JonasBa requested review from Lms24, AbhiPrasad, lforst and a team February 27, 2023 17:12

JonasBa commented Feb 28, 2023

View reviewed changes

lforst self-assigned this Feb 28, 2023

lforst reviewed Mar 1, 2023

View reviewed changes

fix(profile): change platform to javascript

3f9220c

JonasBa force-pushed the jb/boilerplate/js-self-profiling branch 3 times, most recently from f292041 to 42a9639 Compare March 1, 2023 17:07

ref(profiling): move profiling under browser package

d62e3aa

JonasBa force-pushed the jb/boilerplate/js-self-profiling branch from 42a9639 to d62e3aa Compare March 1, 2023 17:07

JonasBa added 7 commits March 1, 2023 12:20

ref(profiling): remove undefined from txn type union in wrapStartTran…

f3ce17e

…saction

fix(profiling): fix test

b3583e3

ref(profiling): rename profile and move it under types

70550a4

chore(profiling): run linters

bb12550

ref(profiling): split sendProfile to avoid a circular ref

979862d

ref(profiling): split cache

060424f

chore(profiling): sort imports

b1f1158

lforst reviewed Mar 2, 2023

View reviewed changes

packages/browser/src/profiling/hubextensions.ts Show resolved Hide resolved

packages/browser/src/profiling/hubextensions.ts Show resolved Hide resolved

packages/browser/src/profiling/integration.ts Outdated Show resolved Hide resolved

JonasBa and others added 2 commits March 2, 2023 11:16

Update packages/browser/src/profiling/hubextensions.ts

c23936d

Co-authored-by: Luca Forstner <luca.forstner@sentry.io>

Update packages/browser/src/profiling/integration.ts

3863f34

Co-authored-by: Luca Forstner <luca.forstner@sentry.io>

lforst reviewed Mar 2, 2023

View reviewed changes

packages/browser/src/profiling/integration.ts Show resolved Hide resolved

Update packages/browser/src/profiling/integration.ts

58c7828

lforst approved these changes Mar 2, 2023

View reviewed changes

lint

f3048df

JonasBa merged commit 45158f4 into develop Mar 2, 2023

JonasBa deleted the jb/boilerplate/js-self-profiling branch March 2, 2023 21:40

JonasBa mentioned this pull request Mar 8, 2023

Support and feedback from Sentry WICG/js-self-profiling#78

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(profiling): js self profiling #7273

feat(profiling): js self profiling #7273

JonasBa commented Feb 23, 2023 •

edited

github-actions bot commented Feb 23, 2023 •

edited

github-actions bot commented Feb 24, 2023 •

edited

Previous results on branch: `develop`

JonasBa commented Feb 28, 2023

lforst commented Feb 28, 2023

JonasBa commented Feb 28, 2023

JonasBa Feb 28, 2023

JonasBa Feb 28, 2023

JonasBa Feb 28, 2023

JonasBa Feb 28, 2023

JonasBa commented Mar 2, 2023

lforst left a comment

		@@ -0,0 +1,10 @@
		import { addExtensionMethods, wrapTransactionWithProfiling } from './hubextensions';

		@@ -0,0 +1,400 @@
		import { WINDOW } from '@sentry/browser';
		import type {

feat(profiling): js self profiling #7273

feat(profiling): js self profiling #7273

Conversation

JonasBa commented Feb 23, 2023 • edited

github-actions bot commented Feb 23, 2023 • edited

size-limit report 📦

github-actions bot commented Feb 24, 2023 • edited

Replay SDK metrics 🚀

Previous results on branch: develop

JonasBa commented Feb 28, 2023

lforst commented Feb 28, 2023

JonasBa commented Feb 28, 2023

JonasBa Feb 28, 2023

Choose a reason for hiding this comment

JonasBa Feb 28, 2023

Choose a reason for hiding this comment

JonasBa Feb 28, 2023

Choose a reason for hiding this comment

JonasBa Feb 28, 2023

Choose a reason for hiding this comment

JonasBa commented Mar 2, 2023

lforst left a comment

Choose a reason for hiding this comment

JonasBa commented Feb 23, 2023 •

edited

github-actions bot commented Feb 23, 2023 •

edited

github-actions bot commented Feb 24, 2023 •

edited

Previous results on branch: `develop`