-
-
Notifications
You must be signed in to change notification settings - Fork 4.4k
feat(eap): add downsampled event retention to config #96818
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
feat(eap): add downsampled event retention to config #96818
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #96818 +/- ##
===========================================
+ Coverage 62.08% 80.16% +18.07%
===========================================
Files 8562 8513 -49
Lines 377263 378879 +1616
Branches 24577 24140 -437
===========================================
+ Hits 234234 303729 +69495
+ Misses 140572 74860 -65712
+ Partials 2457 290 -2167 |
) Adding `downsampled_event_retention` to project configs as part of the [retention changes project](https://www.notion.so/sentry/Retention-Changes-Summary-Jul-2025-2238b10e4b5d80e4b846d055178f334e?source=copy_link) See getsentry/sentry#96818 and getsentry/getsentry#18058 for how it'll be set.
) Adding `downsampled_event_retention` to project configs as part of the [retention changes project](https://www.notion.so/sentry/Retention-Changes-Summary-Jul-2025-2238b10e4b5d80e4b846d055178f334e?source=copy_link) See getsentry/sentry#96818 and getsentry/getsentry#18058 for how it'll be set.
) Adding `downsampled_event_retention` to project configs as part of the [retention changes project](https://www.notion.so/sentry/Retention-Changes-Summary-Jul-2025-2238b10e4b5d80e4b846d055178f334e?source=copy_link) See getsentry/sentry#96818 and getsentry/getsentry#18058 for how it'll be set. --------- Co-authored-by: Pierre Massat <[email protected]>
Returns the retention for downsampled events in the given organization in days. | ||
Returns ``None`` if downsampled events are to be stored indefinitely. | ||
""" | ||
return 0 |
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.
Comment says it returns None
, this is actually 0
.
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.
Ok I updated the docstring here fc37c5d
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.
Would there ever be a need to set retention to 0?
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.
No, we wouldn't. If you want a better default here, we can return get_event_retention()
scrubData: true | ||
scrubDefaults: true | ||
sensitiveFields: [] | ||
downsampledEventRetention: 0 |
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.
That seems bad, if we were to store 0?
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.
Returns the retention for downsampled events in the given organization in days. | ||
Returns ``None`` if downsampled events are to be stored indefinitely. | ||
""" | ||
return 0 |
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 suppose this will read a value linked to the plan at some point?
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.
depends on getsentry/relay#5013 This PR is the second change needed to resolve https://linear.app/getsentry/issue/EAP-160/modify-the-org-setting-to-reflect-the-retention-change-based-on-the.
depends on getsentry/relay#5013 This PR is the second change needed to resolve https://linear.app/getsentry/issue/EAP-160/modify-the-org-setting-to-reflect-the-retention-change-based-on-the.
depends on getsentry/relay#5013
This PR is the second change needed to resolve https://linear.app/getsentry/issue/EAP-160/modify-the-org-setting-to-reflect-the-retention-change-based-on-the.