search

getsentry / sentry-javascript

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

Replay: Mutation breadcrumbs can get spammy #8420

Closed bruno-garcia closed 1 year ago

bruno-garcia commented 1 year ago

On websites with lots of mutations this can be spammy:

image

Consider debouncing it

mydea commented 1 year ago

Hmm, interesting. What would be a reasonable way to debounce this from your POV? Just drop any such breadcrumb if there was one in the last second, or in a longer time period, e.g. 10 seconds?

bruno-garcia commented 1 year ago

Hmm, interesting. What would be a reasonable way to debounce this from your POV? Just drop any such breadcrumb if there was one in the last second, or in a longer time period, e.g. 10 seconds?

That sounds good to me. I don't have strong feelings on this one, just think this is a best effort in telling the dev the "site is heavy and can potentially have issues" but we shouldn't do that in a way that gets too noisy.

billyvg commented 1 year ago

I don't think we should drop breadcrumbs unless we sum up the mutations when debouncing, otherwise the significance of the mutation breadcrumbs will get lost.

bruno-garcia commented 1 year ago

Just saw this extreme case:

image

billyvg commented 1 year ago

We talked this over and came out with 2 potential solutions:

1) Handle this on the UI side - this keeps SDK simpler and also allows the UI to handle this more generically across other crumbs if we wanted.

2) On the SDK, we should consider flushing based on event buffer size instead of only time. This way a massive event-build up won't result in lost events due to segment size

billyvg commented 1 year ago

Closing in favor of https://github.com/getsentry/sentry/issues/54688