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

replay: trimming network responses results block main thread #9426

Closed
3 tasks done
JonasBa opened this issue Nov 1, 2023 · 3 comments
Closed
3 tasks done

replay: trimming network responses results block main thread #9426

JonasBa opened this issue Nov 1, 2023 · 3 comments
Labels
Package: react Issues related to the Sentry React SDK Package: replay Issues related to the Sentry Replay SDK Type: Bug

Comments

@JonasBa
Copy link
Member

JonasBa commented Nov 1, 2023

Is there an existing issue for this?

How do you use Sentry?

Sentry Saas (sentry.io)

Which SDK are you using?

@sentry/react

SDK Version

7.75.0

Framework Version

No response

Link to Sentry event

No response

SDK Setup

No response

Steps to Reproduce

Set up env using replay, enable request body logging, and fetch a response exceeding max body length.

CleanShot 2023-11-01 at 09 24 10@2x

I did a very surface investigation of this, but the issue comes from the fact that replay defines max response body size as 150kB, trims any responses that exceed it, and rebuilds the JSON to again be valid. This seems to all be done synchronously on the client side. This means that the parser rebuilding the JSON will run 150k iterations as it iterates over each char and maintains the matching bracket stack, which is probably enough to cause jank on its own, but I suspect a large part of why this is so slow is because _evaluateJsonPos runs a regexp on each char.

We should look into doing this more efficiently by either workerizing it or reconstructing the JSON on the server.

Expected Result

Replay should not cause jank

Actual Result

Jank

@github-actions github-actions bot added the Package: react Issues related to the Sentry React SDK label Nov 1, 2023
@AbhiPrasad AbhiPrasad added the Package: replay Issues related to the Sentry Replay SDK label Nov 1, 2023
@mydea
Copy link
Member

mydea commented Nov 2, 2023

Hey, thanks for the investigation!

Thinking about this some more, I think it would make sense to push this into the Sentry UI. WDYT @billyvg @ryan953 , I would propose the following:

  • In the SDK if the payload is too large, we just concat it. If we think (from the SDK) it may be a JSON that we truncate, we add a new meta warning type MAYBE_JSON_TRUNCATED.
  • The replay UI, if it detects this warning for a request/response, it basically does exactly what we do right now in the SDK to try to reconstruct the payload.

We can literally move the code over from replay. This has a few additional benefits:

  1. Reduced bundle size for the SDK - this is quite a few bytes!
  2. It may also help with issues like Request getting filtered by Replay integration.  #9339 that seem to be related to bodies - not sure if it is actually because of a parsing issue, as we actually should (?) be handling that, but it would reduce complexity in the SDK considerably.

@JonasBa
Copy link
Member Author

JonasBa commented Nov 2, 2023

Yeah, agree that truncating in the UI makes sense, especially as that makes it on-demand, meaning it will be only be done when necessary so we don't end up wasting cycles in the SDK or the server.

mydea added a commit that referenced this issue Nov 7, 2023
This is potentially performance intensive, so we should just do this in
the UI instead.

Part of #9426

This was implemented in Sentry in getsentry/sentry#59266
@mydea
Copy link
Member

mydea commented Nov 8, 2023

This should be "fixed" in v7.78.0. Let's see if we need to do some follow up work for PII stripping! Thanks for spotting this Jonas 🚀

@mydea mydea closed this as completed Nov 8, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Package: react Issues related to the Sentry React SDK Package: replay Issues related to the Sentry Replay SDK Type: Bug
Projects
None yet
Development

No branches or pull requests

3 participants