-
-
Notifications
You must be signed in to change notification settings - Fork 58
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: Browser profiling #799
Conversation
0763706
to
bf83a65
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
awesome
const { | ||
addTracingExtensions, | ||
init, | ||
BrowserProfilingIntegration, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We need both RendererProfiling
and BrowserProfilingIntegration
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
RendererProfiling
is only for the main node process and enables the header injection. BrowserProfilingIntegration
does the actual profiling in the browser.
I added some more detail to the top post to show how this would be used by users |
No blocking comments, just wondering if we can reduce the config point down to a single entry |
Electron has a "preload" context where you can inject code but it's
We can inject code into the browser context via either the debugger interface or via intercepting the requests for existing assets (html, js, etc). The debugger is a bit heavy handed and likely won't happen before Assuming the browser front end is loading from a If we could determine a reliable way to do this, we could inject all the browser code automatically, not just additional features like this! Due to (optional but common) bundling of the main node process code, any code we inject would need to be inlined into the main process SDK source and we'd need add this in a way that doesn't impact app size for those not using this feature. |
I think it's very reasonable to ask people to do two different configuration changes in this context for now, the isolation seems unreliable to work around, but it could be a nicer DX improvement later on if we can find a way to do it reliably. |
Closes #792
Adds a new
RendererProfiling
integration:enableRendererProfiling
option'Document-Policy': 'js-profiling'
headers to all renderer requestsbeforeEnvelope
to re-add the profiles to the correct envelopesThis PR also adds a test to ensure a profile is sent with the transaction.
Enabling Browser Profiling
From a users perspective, to enable profiling in the renderers, you'll need to pass
enableRendererProfiling: true
in the main process:Then in the renderer follow the regular browser profiling instructions: