search

getsentry / sentry-dart

Sentry SDK for Dart and Flutter
https://sentry.io/for/flutter/
MIT License
766 stars 239 forks source link

Send Less Client Reports When Rate Limited #2370

Closed philipphofmann closed 3 weeks ago

philipphofmann commented 1 month ago

Description

The Dart SDK attaches the client report before applying the rate limit in the client.

https://github.com/getsentry/sentry-dart/blob/0880a97f2be69b0bbd4e9fb8460afd39a73b5033/dart/lib/src/sentry_client.dart#L630-L637

In the http_trasnport, it then applies the rate limit https://github.com/getsentry/sentry-dart/blob/fe6dcac6041d41f2b6ff38f507a75470ea94b2f4/dart/lib/src/transport/http_transport.dart#L37-L42

Because clients can assume that client reports never get rate limited (develop docs), the Dart SDK continues to send an envelope with a client report even when the SDK is fully rate-limited and should drop all envelope items. Instead, the Dart SDK should only add the client report envelope item if the envelope has items to send.

The downside of this is that when the SDK is fully rate-limited for a long time or the user drops all events in beforeSend, the SDK may never send a client report. This can be addressed by checking in the HTTP transport if the ClientReports has more than x discarded events (I guess 10 is a good start), and then send an envelope with only the client report. This is related to https://github.com/getsentry/sentry-cocoa/issues/4468.

denrase commented 1 month ago

@philipphofmann This is only relevant for the http transport on the flutter side, right? Because when on iOS/Andorid, we don't rate limit on the flutter layer, but rather just send the envelope downstream to the native SDKs. In that case, we can't know if we should attach the client reports, right?

philipphofmann commented 1 month ago

Yes, this is only relevant to the HTTPTrasnport. The native SDKs handle client reports and rate limits for the passed down envelopes themselves.