getsentry / sentry-javascript

Official Sentry SDKs for JavaScript
https://sentry.io
MIT License
7.78k stars 1.53k forks source link

Support remote asset caching in Replays #7007

Open billyvg opened 1 year ago

billyvg commented 1 year ago

Problem Statement

Session Replay handles static assets differently based on the type of asset. They are either inlined into the recording or the source URL is preserved and fetched during playback-time. There are pros and cons to both of these approaches that will be explained below.

Inline

Currently only CSS is inlined in the recordings. The benefits of this are:

Remote

The benefits of inlining assets are conversely the downsides of remotely fetching the asset. However, the reason to remotely fetch static assets is to reduce the file size of recordings. This affects how much data the recording clients send, as well as the amount of storage required . Generally these assets (fonts, images, etc) do not change often, so replays can contain a lot of redundant data as well.

In our Session Replays, fonts and images are loaded at playback-time and can face cross-origin issues. By default we block images due to privacy concerns, but have had a handful of tickets from users who choose to unblock and not see their images get loaded during playback due to CORS. This leads them to think our product is broken.

Solution Brainstorm

It would require a bit of complexity, but we can have the best of both worlds if we were to proxy requests to all static assets to a version that is saved on a server we control. This means we are not inling anything so recording file sizes should be reduced (which means our customer's customers are sending less data). We should face less CORS issues as we can add our asset server as an allowed origin.

One thing this approach may not be able to handle however is if an asset requires authentication to access.

mydea commented 1 year ago

To be clear, this is mostly not a js-sdk issue I think, but a server side issue? Or do you see us rewriting urls or similar in the SDK?

billyvg commented 1 year ago

Yes I think this could potentially require SDK work to rewrite URLs, but implementation hasn't really been discussed yet.

Lms24 commented 1 year ago

Just for cross-reference: #7020 surfaces an issue around SVGs that use the <use> tag, where asset caching would be the only way to get the SVGs to show up (other than inlining, which we abandoned before) .

dwd commented 1 year ago

I was expecting the proposal to include the Release Artifacts as an option - I'd be absolutely fine with relying on an uploaded asset for the particular release. It puts the asset management effort somewhat onto me, but since I'm already uploading sourcemaps (etc) for the release it's not much effort.

That said, my focus is very much on static assets like CSS - I absolutely don't want any images loaded in my case, and they're all explicitly blocked.

lucas-zimerman commented 1 year ago

maybe the images could be uploaded as an release artifact and the path of images could be replaced with something like

<img src="/{dsn}/{release}/{dist}/http://www.imageurl.com/support.gif">

If the image isn't found, a placeholder image could be shown just to not alter the website layout with sizes.

One disadvantage of it is it would only cover images that are statically stored on the app/website

github-actions[bot] commented 1 year ago

This issue has gone three weeks without activity. In another week, I will close it.

But! If you comment or otherwise update it, I will reset the clock, and if you label it Status: Backlog or Status: In Progress, I will leave it alone ... forever!


"A weed is but an unloved flower." ― Ella Wheeler Wilcox 🥀

futzlarson commented 1 year ago

Bump

iqzas commented 7 months ago

is this issue going to be fixed anywhere in the near future? I am also facing the same issue. Chrome straightaway blocks the loading of icon images. I am not able to see any of material design icons on the replay videos. I am using browser javascript bundle using below: https://browser.sentry-cdn.com/7.91.0/bundle.tracing.replay.min.js

lforst commented 7 months ago

is this issue going to be fixed anywhere in the near future? I am also facing the same issue.

@iqzas

@billyvg Correct me if I am wrong, but I believe this is not the highest of priorities for us right now. Also this is not trivial to implement so it might still take a while.

simonflk commented 2 months ago

Somewhat related to this issue is this question I asked over on the discord:

My team finds Sentry Replay an invaluable tool for fixing bugs and making our product better.

Our app is a React webapp running in a mobile webview using Capacitor JS. We have some images and fonts which are bundled in the app and served locally e.g. (http://localhost/images/getting-started.webp)

When we view sessions in replay, the images do not load and instead we see the "alt" text. This is suboptimal, because it looks like the images are broken, and also the "alt" text breaks the layout.

It would be fantastic if we could provide configuration for how to resolve localhost requests. E.g. Something like the following:

Sentry.replayIntegration({
  rewriteUrl: {
    'http://localhost/': 'https://my-deployed-app.example.com/'
  }
})

Something like this would allow sentry to fetch assets from a web-accessible URL, that otherwise would be inaccessible (localhost)

futzlarson commented 2 months ago

@simonflk Personally, I disabled Sentry when developing locally since I knew what I did to cause an error so logging to Sentry felt unnecessary. My two cents.

simonflk commented 2 months ago

@futzlarson to be clear, this isn't local - it's a deployed production app using CapacitorJS

Internally capacitor opens a webview configured to look a localhost which is serving static files from the filesystem.

It uses localhost because the app is locally installed on the device. But the users and their devices are remote and distributed