feat(browser): Allow collecting of pageload profiles #9317
Conversation
JonasBa
force-pushed
the
jb/feat/pageload-profiling
branch
3 times, most recently
from
0e4d4bf to
330d1b1
Compare
JonasBa
force-pushed
the
jb/feat/pageload-profiling
branch
from
330d1b1 to
c551f0a
Compare
|
I added a test to this and noticed that my current approach is not feasible because profiler.stop is async. Since transactions seem to be delegated to the transporter immediately after finish is called (correct me if I'm wrong, but this is what I observed when adding a test), it means the profiler promise will always resolve after the transaction is already sent, breaking the requirement for profiles and transactions to be sent in the same envelope. As an alternative approach, I query for the current active transaction as soon as integration.setupOnce method is called and wrap it. The limitation of this approach is that if an intermediary transaction is created between pageload and our setupOnce method call which is active at the time of setupOnce call, we will no longer be able to wrap (as we may only query for the current active txn). There might be other situations where this might fail or better ways to approach this, I'm open to ideas. For this to work reliably, we would need to be able to enumerate all of the active txns at time of integration setup so we can patch the finish step. |
|
Revised implementation after meeting with the team (thanks @AbhiPrasad for the invite). Instead of relying on the profile to already exist and its reference to be stored on the main carrier, start the profile as part of the A blind spot of this approach (and the reason why I'm calling it partial page load profiling) is that any JS activity happening between the document response and our SDK init will be missed. A solution to that would be to provide users with a snippet that would start the profile earlier (this can be done as a progressive enhancement at a later point). I think this approach is much better than what I originally intended as it will provide an actual out-of-the-box experience for our users that doesn't require them to make any changes and is flexible enough so that it can be improved for more advanced users. |
|
@mydea @lforst @AbhiPrasad @Lms24 This is now ready for another review 🙏🏼 |
lforst
changed the title
size-limit report 📦
|
|
Let's get this merged, we can do another release with it in for tomorrow |
|
@AbhiPrasad will merge this asap as CI passed. I also have a small fix that I will open for adjusting the timing of the pageload profiles as well. |
Profiling is currently unable to profile page loads which creates a gap in the product as performance instrumentation seemingly can (via synthetic transactions)
In order to bridge this gap, the plan is to provide a small JS snippet that users can insert into their document which will initialize and store a reference to the pageload profile on the sentry carrier. By doing so we can hook back into our finishTransaction codepath and send a profile associated to a transaction.
Would love to hear early thoughts on this approach from SDK maintainers before I start adding test coverage.