-
-
Notifications
You must be signed in to change notification settings - Fork 4.5k
feat(billing): add transaction retention to config as span #102095
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
Conversation
| DataCategory.LOG_BYTE: "log", | ||
| DataCategory.TRANSACTION: "span", | ||
| DataCategory.SPAN: "span", | ||
| } |
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.
Bug: Duplicate Keys Cause Configuration Conflicts
The RETENTIONS_CONFIG_MAPPING maps DataCategory.TRANSACTION and DataCategory.SPAN to the same 'span' key. If both categories are present in retention data, dictionary comprehensions using this mapping will silently overwrite one value with the other. This results in unpredictable retention policies and potentially incorrect configuration sent to Relay.
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.
A customer can have either DataCategory.TRANSACTION or DataCategory.SPAN, but not both.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #102095 +/- ##
=========================================
Coverage 80.97% 80.97%
=========================================
Files 8733 8733
Lines 388590 388701 +111
Branches 24654 24654
=========================================
+ Hits 314653 314751 +98
- Misses 73577 73590 +13
Partials 360 360 |
For customers with
DataCategory.TRANSACTIONinstead ofDataCategory.SPANin their subscription plan, we want to provide the retention asspanin the project config.