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

Project: Metrics Rate Limits #77

Closed
14 tasks done
Tracked by #64045
stephanie-anderson opened this issue Mar 28, 2024 · 3 comments
Closed
14 tasks done
Tracked by #64045

Project: Metrics Rate Limits #77

stephanie-anderson opened this issue Mar 28, 2024 · 3 comments

Comments

@stephanie-anderson
Copy link
Collaborator

stephanie-anderson commented Mar 28, 2024

Description

In order to protect our infrastructure and also customers money, Relay is now able to propagate the limits from processing to PoP Relays and further to our SDKs. We need to make sure that all SDKs can stop sending metrics for some orgs (e.g. above quota, abusing Sentry).

The develop specs for this can be found here:
https://develop.sentry.dev/sdk/rate-limiting/

The tracking issue for the Ingest team is:
https://github.com/getsentry/team-ingest/issues/303

A quick TL;DR: How to Implement

Add a new data category to the existing rate limiter code, called metric_bucket.
A new parameter called namespaces might be returned for the metric_bucket category.
If the returned namespaces contain custom, back off transmitting metrics from the SDK.
If the returned namespaces exclusively contain namespaces other than custom, do not back off transmitting metrics from the SDK.
If the namespaces are empty (2700:metric_bucket:organization:quota_exceeded:) or omitted (2700:metric_bucket:organization:quota_exceeded) in the response header, back off transmitting metrics from the SDK.
While we do not support setting metric namespaces in the SDK yet, Relay could return a X-Sentry-Rate-Limits header that contains a different metric namespace only, e.g. transaction.
In case your SDK supports client outcomes, report rate limited metrics as the metric_bucket category.

Get inspired by Python SDK PR #1 or PHP PR #1 and PHP PR #2.

Why should we be doing this?

We need to protect our infrastructure and respect communicated rate limits.

Why now?

We didn't plan this for the metrics beta, but metrics is adopted pretty well.

Slack-Channel

#discuss-ingest

Stakeholder(s)

Ingest team (esp. Oleksandr & Jan)

Team(s)

Mobile, Web Frontend, Web Backend

Implementation Status

Web Backend SDKs

  1. cleptric
  2. Platform: Android Platform: Java
    stefanosiano
  3. metrics
    sl0thentr0py
  4. jamescrosswell

Web Frontend SDKs

  1. Feature: Metrics
    s1gr1d

Mobile SDKs

  1. Platform: Dart
  2. Platform: Cocoa Platform: iOS
    philipphofmann
@philipphofmann
Copy link
Member

A new parameter called namespaces might be returned for the metric_bucket category.
If the returned namespaces contain custom, back off transmitting metrics from the SDK.
If the namespace is omitted in the response header, back off transmitting metrics from the SDK.

I had to double-check the code to ensure I got this right. @cleptric, please correct me if I'm wrong. When the SDK receives a rate limits header with the category metric_bucket and a namespace other than custom e.g. transaction, the SDK must not apply a rate limit for metrics.

@cleptric
Copy link
Member

cleptric commented Apr 8, 2024

If the returned namespaces exclusively contain namespaces other than custom, do not back off transmitting metrics from the SDK.

Updated the description.

@krystofwoldrich
Copy link
Member

React Native SDK https://github.com/getsentry/sentry-react-native/releases/tag/5.22.0 includes Rate Limits impl from sentry-cocoa and sentry-android.

@stephanie-anderson stephanie-anderson added this to the Metrics milestone Jul 10, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

5 participants