-
-
Notifications
You must be signed in to change notification settings - Fork 1.5k
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
ref(stats): Introduce discard outcome & reason descriptions #10585
ref(stats): Introduce discard outcome & reason descriptions #10585
Conversation
The latest updates on your projects. Learn more about Vercel for Git ↗︎
1 Skipped Deployment
|
docs/product/stats/index.mdx
Outdated
|
||
Events and attachments discarded by the system due to rate limits, quotas, spike protection, [size limits](/concepts/data-management/size-limits), or invalid data. You can see the common reasons below: | ||
|
||
| Reason | Meaning | |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I was uncertain about adding this table here since it can get quite lengthy. However, I didn't want to place it elsewhere, as that might make it feel disconnected from the usage stats section. Thoughts?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would not list those at all. Those outcome reasons are not stable and meant for internal use. Apart from that, they refer to internals of the system that are confusing to a user.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I understand those outcome reasons are intended for internal use and may confuse users. However, if users can see this information, we should provide at least a brief explanation to ensure clarity. Are you sure it’s better to not add an explanation for these reasons?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let's expose these reasons in a simplified and grouped way that makes it easier for users to comprehend. I will post an update here with mapped reasons and improved descriptions.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Here is a list that groups all discard reasons so that they can be understood by users. I've also added more relatable descriptions that point at docs to make this more actionable.
https://gist.github.com/jan-auer/2d29e2ee6334bbcf32514b12b42b31ea
As for the different kinds of outcomes, please show them separately in the following combination at least in charts:
- Accepted (0)
- Filtered (1)
- Rate limited (2) + Abuse (4) + Cardinality Limited (6) -- currently shown as "Dropped" in the UI
- Invalid (3)
- Client Discard (5)
Bundle ReportChanges will increase total bundle size by 24 bytes ⬆️
|
docs/product/stats/index.mdx
Outdated
|
||
#### Discarded | ||
|
||
Events and attachments discarded on the client side due to the sampling configuration in the SDK or other factors. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
"or other factors" is a bit unspecific. I would either remove this part of the sentence or list more reasons. They are documented here under "discarded_events
": https://develop.sentry.dev/sdk/client-reports/#envelope-item-payload
docs/product/stats/index.mdx
Outdated
|
||
#### Dropped | ||
|
||
Events and attachments discarded by the system due to rate limits, quotas, spike protection, [size limits](/concepts/data-management/size-limits), or invalid data. You can see the common reasons below: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This combines the two outcomes RATE_LIMITED
and INVALID
. Do you really want to mix those?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yes, because we are combining them in the code https://github.com/getsentry/sentry/blob/566f4a0a04d9a9dd6f1a56d13808ea715b913e8b/static/app/views/organizationStats/usageStatsOrg.tsx#L391-L396. So they are listed under dropped
docs/product/stats/index.mdx
Outdated
|
||
Events and attachments discarded by the system due to rate limits, quotas, spike protection, [size limits](/concepts/data-management/size-limits), or invalid data. You can see the common reasons below: | ||
|
||
| Reason | Meaning | |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would not list those at all. Those outcome reasons are not stable and meant for internal use. Apart from that, they refer to internals of the system that are confusing to a user.
Co-authored-by: Jan Michael Auer <mail@jauer.org>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks so much for making these updates @priscilawebdev! I left a few suggestions :)
docs/product/stats/index.mdx
Outdated
### Rate Limited | ||
Events and attachments discarded due to rate limits or quota. The following reasons are currently defined: | ||
|
||
- **Abuse**: Activated when Sentry is misused. | ||
- **Cardinality Limited**: Exceeded internally defined cardinality limit. | ||
- **Rate Limited**: Exceeded allowed event rate. | ||
- **Spike Protection**: Activated to protect from any sudden quota-depleting spike. See [Spike Protection](/pricing/quotas/spike-protection/) for more information. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@jan-auer can you help me with this list? I have noticed that we are displaying "Spike protection" and "Project Abuse Limit" for example
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Here's a suggestion for how to structure rate limits. Please expand this with links to relevant documentation as appropriate:
- **Quota**: The monthly quota of your subscription was depleted and there is no on-demand budget left.
- **Key Limit**: Traffic exceeded a rate limit defined on the client key (DSN).
- **Spike Protection**: Activated to protect from a sudden spike in data volume. See [Spike Protection](/pricing/quotas/spike-protection/) for more information.
- **Internal Limit**: A rate limit for excessive volume was enforced by Sentry. These limits are not configurable.
Reason codes for the above are:
- quota:
*_usage_exceeded
(everything that ends with"_usage_exceeded"
) - key limit:
key_quota
- spike protection:
smart_rate_limit
- internal: (everything else)
Therefore, please include the "abuse" and "cardinality" limit outcomes under "Internal Limits" for now.
docs/product/stats/index.mdx
Outdated
|
||
- **Quota**: The monthly quota of your subscription was depleted and there is no on-demand budget left. See [Adjusting your Quota](/pricing/quotas/#adjusting-your-quota) for more information. | ||
- **Key Limit**: Traffic exceeded a rate limit defined on the client key (DSN). See [Rate Limits](/pricing/quotas/#rate-limits) for more information. | ||
- **Spike Protection**: Activated to protect from a sudden spike in data volume. See [Spike Protection](/pricing/quotas/spike-protection/) for more information. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
suggestion: "event volume" instead of "data volume"?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@jan-auer what do you think?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I originally suggested "data" instead of "event", since spike protection could also apply to non-events. Today, that's not the case, so let's go with "event volume" 👍
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Descriptions look good from my end, thank you!
Goal
We are currently expanding the range of outcomes visible on the stats page, as well as refining how they are presented. This pull request aims to update the documentation to reflect these changes and provides details on the reasons behind displaying each outcome.
closes getsentry/sentry#73811