Introduce internal event "quality score" for outgoing events

[HazAT]

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

sentry-javascript/packages/core/src/integrations/inboundfilters.ts

Lines 9 to 13 in 652b621

	const DEFAULT_IGNORE_ERRORS = [
	/^Script error\.?$/,
	/^Javascript error: Script error\.? on line 0$/,
	/^ResizeObserver loop completed with undelivered notifications.$/,
	];

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.

[antonpirker]

Could this be done in relay so it works for all SDKs?

[HazAT]

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_error in 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]

I think this is a good idea and something we should explore a bit more.

new Error(undefined) & no message & no stack trace
-> Drop

No exception, no stack trace, no message
-> Drop

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) 👍

Weird edge cases we identified ourselves by looking at Sentry Events
-> Drop

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 Error this 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]

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]

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]

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]

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]

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 from app:///popup.js to avoid filtering false positves.

[HazAT]

app:///popup.js indicates that is is probably happening within a Chrome extension and for some reason our SDKs picks it up.

[HazAT]

Another one

Permission denied to access property "correspondingUseElement"
ref: bitwarden/clients#7837

[HazAT]

Consider: getsentry/sentry#69096