Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Send Less Client Reports When Rate Limited #2370

Open
philipphofmann opened this issue Oct 23, 2024 · 2 comments · May be fixed by #2380
Open

Send Less Client Reports When Rate Limited #2370

philipphofmann opened this issue Oct 23, 2024 · 2 comments · May be fixed by #2380
Assignees

Comments

@philipphofmann
Copy link
Member

philipphofmann commented Oct 23, 2024

Description

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

Future<SentryId?> _attachClientReportsAndSend(SentryEnvelope envelope) {
final clientReport = _options.recorder.flush();
envelope.addClientReport(clientReport);
return _taskQueue.enqueue(
() => _options.transport.send(envelope),
SentryId.empty(),
);
}

In the http_trasnport, it then applies the rate limit

Future<SentryId?> send(SentryEnvelope envelope) async {
final filteredEnvelope = _rateLimiter.filter(envelope);
if (filteredEnvelope == null) {
return SentryId.empty();
}
filteredEnvelope.header.sentAt = _options.clock();

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 getsentry/sentry-cocoa#4468.

@philipphofmann philipphofmann changed the title Client Report Improvement Send Less Client Reports When Rate Limited Oct 23, 2024
@denrase
Copy link
Collaborator

denrase commented Oct 29, 2024

@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
Copy link
Member Author

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

@denrase denrase linked a pull request Oct 30, 2024 that will close this issue
8 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: Needs Review
Development

Successfully merging a pull request may close this issue.

2 participants