Repository navigation
Introduce internal event "quality score" for outgoing events #10882
Description
Problem Statement
Javascript errors/events can be very noisy - and our SDK today send a lot of data.
We have simple mechanisms in place like InboundFilters
to remove specific errors that and not worth sending.
Tho this is not enough and still a lot of clutter reaches Sentry, from weird chrome extension errors to other edge cases of low quality events that are just noise (no good message / no stacktrace)
Solution Brainstorm
What if we add a very simple internal "Quality Score" for an error/event in the SDK and only if it reaches a certain value, we actually send it.
For example:
new Error(undefined) & no message & no stack trace
-> Drop
No exception, no stack trace, no message
-> Drop
Weird edge cases we identified ourselves by looking at Sentry Events
-> Drop
A stack trace with no message and only one single frame with <anonymous>
-> Drop
It could operate on the payload/JSON level to make it relatable to what enters Sentry.
Activity
antonpirker commentedon Mar 1, 2024
Could this be done in
relayso it works for all SDKs?HazAT commentedon Mar 1, 2024
In theory, yes BUT
A lot of things depend on errors in the SDK.
For example: We trigger Replays for errors in SDKs - if it's just a worthless error, we also shouldn't trigger a Replay.
The same is true for transactions; we set the status to
interal_errorin transactions if an error happens - and again, if the error is not actually a relevant error, we mark perfectly fine transactions as "errored".So this could be technically done in Relay to then also drop a Replay/transaction when there is a crappy error - tho it's super complex and not easy to implement and might even require tail-based sampling.
Lms24 commentedon Mar 5, 2024
I think this is a good idea and something we should explore a bit more.
Big +1, probably easy to do in the SDK with little code (and yes probably SDK specific how such low quality errors can be identified) 👍
Generally yes but I'd say these depend a lot on how we'd identify such individual edge cases. For instance, only by error message? we should consider adding them as a server-side inbound filter. checking against strings (or RegExp) could become bundle size intensive depending on how many such cases we handle. IIRC we already handle the infamous
Script Errorthis way. Might be worth exploring if we could move this to the server.If there's a deeper pattern (I dunno think something like, only anonymous stack frames or a certain amount of frames in combination with another factor) it probably makes sense to keep this in the SDK.
HazAT commentedon Mar 5, 2024
Yeah there is a balance to strike - but the goal is not to send them from the SDK because so much already is connected to errors (what I mentioned, Replays/Transactions etc.)
scefali commentedon Mar 5, 2024
I may not have standing here, but the SDK seems like the wrong place to do it, especially for browser JS since this code will add to bundle size. We also require folks to update SDKs to get these changes (obviously) which means that doing this in replay is a higher leverage change by the number of users impacted immediately. I see there are some downsides doing this in Relay but I think we should do more investigation before we decide.
mydea commentedon Mar 6, 2024
So just to add a perspective for Replay: Replay's error sampling is based on a 200 response when an error event is sent. So in order to keep this working "as expected", either we need to drop errors in the SDK, or they need to be dropped early enough that we actually get a non-200 status code. If errors are dropped anywhere later (e.g. as currently for inbound filters) the SDK does not know about it and thus replay sampling cannot take it into consideration.
HazAT commentedon Apr 2, 2024
Sentry Examples
We should not report these
https://sentry.sentry.io/issues/4934549560/?project=1267915&query=error.unhandled%3Atrue+is%3Aunresolved&referrer=issue-stream&sort=freq&statsPeriod=14d&stream_index=18
https://sentry.sentry.io/issues/4814129167/?project=1267915&query=error.unhandled%3Atrue+is%3Aunresolved&referrer=issue-stream&sort=freq&statsPeriod=14d&stream_index=6
Lms24 commentedon Apr 3, 2024
The googletag error should already be filtered out since
7.108/8.0.0-alpha.5(via #11210 and #11208).The other one (
Cannot read properties of null (reading 'content')) is very generic. It has a stack trace, a generic filename and an error message that's similar to actual "valid" errors. We'd probably need to check for error kind + message + 1 stack frame fromapp:///popup.jsto avoid filtering false positves.HazAT commentedon Apr 3, 2024
app:///popup.jsindicates that is is probably happening within a Chrome extension and for some reason our SDKs picks it up.HazAT commentedon Apr 16, 2024
Another one
Permission denied to access property "correspondingUseElement"ref: bitwarden/clients#7837
HazAT commentedon Apr 18, 2024
Consider: getsentry/sentry#69096
ref(browser-extensions): Add new regex for permission denied to inbou…
Lms24 commentedon Apr 18, 2024
RE getsentry/sentry#69096 and related issues where filtering on the message isn't enough. I think in theses cases we want to make filtering decisions on the original stack trace, meaning ideally, before our SDK integrations and event processors come in and potentially modify the stack frames (e.g.
rewriteFramesIntegration). Perhaps we can use the newpreprocessEventintegration hook to filter which should run before other event processors.lforst commentedon Apr 18, 2024
BIG BRAIN idea we just had: What if users would be able to tell Sentry "this is a list of all the static JS files I am deploying, please ignore everything that doesn't come from those files".
We have all the building blocks already:
sentry-cli sourcemaps inject, bundler plugins, ...HazAT commentedon May 28, 2024
lforst commentedon May 29, 2024
Dumping a few thoughts for posterity:
captureMessagewhich often don't have a stack trace?myVeryWellKnownFunction is not a function, and you would instantly know that you are accessing that function even though it's not defined. So we likely shouldn't throw those errors away (?).Immediate plan of action based on my thoughts and research: Since the vast majority of noisy issues in the internal Sentry Sentry project or the Sentry Docs project come from browser extensions or similar, it makes most sense to a) work on getting the third party code filter integration out b) improve documentation around
allowUrlsand make it more prominent in the docs.Next, we can filter out:
lforst commentedon Jun 3, 2024
Thread to revisit when we ship: https://www.reddit.com/r/nextjs/comments/1d5fxx8/anyone_using_sentry_while_not_getting_overwhelmed/
lforst commentedon Jun 25, 2024
Summing up the changes we did: