doc(server): Document desired SDK rate limiting behavior per data category #5188
Conversation
| Default = 0, | ||
| /// Error events and Events with an `event_type` not explicitly listed below. | ||
| /// | ||
| /// SDK rate limiting behavior: apply to envelope items of type `event`. |
There was a problem hiding this comment.
Not just events, but the entire envelope. Which is implied by the protocol, but maybe not obvious to everyone?
| Error = 1, | ||
| /// Transaction events. | ||
| /// | ||
| /// SDK rate limiting behavior: apply to envelope items of type `transaction`. |
| Uptime = 21, | ||
| /// Counts the number of individual attachments, as opposed to the number of bytes in an attachment. | ||
| /// | ||
| /// SDK rate limiting behavior: apply to attachments. |
There was a problem hiding this comment.
Maybe for attachments, also profiles Relay should do the transformation from Item or Duration to just a single category?
| pub enum DataCategory { | ||
| /// Reserved and unused. | ||
| /// | ||
| /// SDK rate limiting behavior: ignore. |
There was a problem hiding this comment.
Or should this be "apply to all"?
| Transaction = 2, | ||
| /// Events with an event type of `csp`, `hpkp`, `expectct` and `expectstaple`. | ||
| /// | ||
| /// SDK rate limiting behavior: ignore (?). |
There was a problem hiding this comment.
See https://develop.sentry.dev/sdk/data-model/envelope-items/#reserved-types
| /// SDK rate limiting behavior: ignore (?). | |
| /// SDK rate limiting behavior: ignore. |
| /// This is the category for transaction payloads that were accepted and stored in full. In | ||
| /// contrast, `transaction` only guarantees that metrics have been accepted for the transaction. | ||
| /// | ||
| /// SDK rate limiting behavior: ignore. |
There was a problem hiding this comment.
Data may still be ingested even though we do not index it.
| /// | ||
| /// This is the category for spans from which we extracted metrics from. | ||
| /// | ||
| /// SDK rate limiting behavior: apply to spans that are not sent in a transaction. |
There was a problem hiding this comment.
@Dav1dde or should the SDK already limit transactions here? See https://develop.sentry.dev/ingestion/relay/transaction-span-ratelimits/#enforcement.
IMO no, it is up to the server (relay + sentry) to propagate a rate limit for both if we don't want either spans nor transactions.
There was a problem hiding this comment.
No, Relay should propagate a TRANSACTION rate limit also to SPANS (if that's the desired behaviour).
| /// | ||
| /// See also: [`Self::ProfileDuration`] | ||
| /// | ||
| /// SDK rate limiting behavior: ignore. |
There was a problem hiding this comment.
Is this correct? cc @getsentry/profiling
There was a problem hiding this comment.
Can you explain what exactly this comment means? I'm not sure I understand.
But in general, ProfileDurationUi should behave the same as ProfileDuration and ProfileChunkUi should behave the same as ProfileChunk as they're just used for different platforms.
There was a problem hiding this comment.
@Zylphrex if an SDK receives a rate limit header with this data category, should it stop sending profiling data to sentry? Or is it an internal data category that SDKs should not worry about?
jjbayer
requested review from
cleptric and
a team
and removed request for
a team
So far, dev docs have been used to inform SDK developers on which data categories to apply to which envelope items. It has been brought up that this list is not always complete, and that developers sometimes check the
DataCategorydefinition in Relay for a more complete picture, since it is the source of truth of data categories used across Sentry. This however raises new questions, as it is not always clear what relevance the categories have per SDK.Add a line to each doc comment for clarification.
INGEST-573.