Repository navigation
Conversation
Read outcomes recorded by enforcement instead of reconstructing them from the limit plan. Preserve the existing expected categories and quantities. Co-authored-by: Codex <noreply@openai.com>
Carry rejection reasons through item retention and account for the data actually removed. Include feedback rejected with its parent error, avoid outcomes for retained data, and restore managed quantity checks. Remove the separate outcome generator and its extra-category bookkeeping. Co-authored-by: Codex <noreply@openai.com>
Check emitted quantities and Kafka output for legacy transactions and Python-shaped envelopes containing a transaction and separate spans. Cover zero quotas, indexed quotas, and cached limits after exhaustion. Co-authored-by: Codex <noreply@openai.com>
Co-authored-by: Codex <noreply@openai.com>
Move the transaction storage characterization to its own PR. Keep the mixed-envelope quota accounting regressions alongside the accounting fix. Co-authored-by: Codex <noreply@openai.com>
Assert that an attachment quota preserves a Nintendo dying message and accounts only for the ordinary attachment removed alongside it. The current implementation panics because it counts the dying message as rejected attachment bytes. Co-authored-by: Codex <noreply@openai.com>
Apply envelope quantity rules to retained, rate-limited crash items so Nintendo dying messages do not emit attachment outcomes. Count quantities before marking items as rate limited, preserving the shared event. Match ItemAction directly while keeping item and envelope rejection accounting together. Co-authored-by: Codex <noreply@openai.com>
Rejection quantities now come from retained or removed envelope items, leaving Enforcement.spans without a production reader. Remove the redundant field while preserving span quota checks and indexed enforcement. Cover silent event-carrier removal, Nintendo parent rejection, and already-rate-limited minidumps alongside newly rejected attachments. Co-authored-by: Codex <noreply@openai.com>
Name V1 user reports by their envelope item type and describe corrected accounting without claiming completeness. Co-authored-by: Codex <noreply@openai.com>
Share the mixed-envelope setup, separate zero and cached quota paths, and remove the case duplicated by cache warm-up. Assert accepted and rejected outcome totals and exact span counts without coupling accounting tests to embedded-span storage behavior. Reuse rate-limit assertions for V1 user reports and replace Rust retention bit masks with explicit input cases. Co-authored-by: Codex <noreply@openai.com>
Represent quota inputs and complete expected outputs with named Python cases. Use ItemAction directly in retention tables instead of boolean flags. Co-authored-by: Codex <noreply@openai.com>
Use Case.id for pytest identifiers and remove the pytest.param wrappers. Co-authored-by: Codex <noreply@openai.com>
Preserve the removal of unused envelope helpers while retaining rejection accounting in Envelope::retain_items. Co-authored-by: Codex <noreply@openai.com>
Dav1dde
left a comment
Dav1dde
left a comment
There was a problem hiding this comment.
Nice! Definitely an improvement over what we had before, though overall I am still not entirely happy with how this works, it all relies on some very hard accounting split across EnvelopeSummary and other code parts, where ideally we can eventually formalize accounting of an envelope and build up a proper set of tools to deal with it.
| self.items.retain(f) | ||
| /// Preserves item order. Ordinary kept items need no quantity calculation. Rejected quantities | ||
| /// follow envelope counting rules, including the single implicit event created by crash attachments. | ||
| pub fn retain_items( |
There was a problem hiding this comment.
I think this function should stay as it was, a very simple way to filter out items of the envelope.
The changes you made are very rate limiting specific (e.g. KeepRateLimited and having to make infer_event_category pub(crate) are good indicators) and require some attached Managed<> instance. So I would either inline this function into apply_to_managed or make it a helper in the rate limiting module.
| if let (None, Some((category, outcome))) = (surviving_event, event_rejection) { | ||
| records.reject_err(outcome.clone(), (category, 1)); | ||
| if let Some(indexed) = category.index_category() { | ||
| records.reject_err(outcome, (indexed, 1)); | ||
| } | ||
| } |
There was a problem hiding this comment.
This is becoming a re-occuring pattern, we really need a better way to deal with this. Removing the event creating item should always also mean the entire envelope is dropped if it was the only one.
For example, 1x Minidump ("creates event"), 1x other needs to be completely dropped if the minidump is dropped. Though of course there are always exceptions, if other is a standalone item (e.g. session) then that's fine again ...
| ), | ||
| Case( | ||
| id="span-fits", | ||
| quota=Quota(category="span", limit=3), |
There was a problem hiding this comment.
These tests here test a lot more than the code you modified. If you want to limit yourself to the code you changed, you can/should run a Relay without processing. The rate limiter you changed is only for the "fast path", any limit above 0 is not enforced there initially. So to test the fast path you can create simpler tests by just setting the quota to 0, with the upside of not requiring Kafka (= faster tests).
This branch has not been deployed
2 participants
Rejecting an error now emits the missing
user_report_v2outcome for its accompanying UserReport V1, using the error quota's reason. Rejecting feedback independently still preserves the error.Enforcement::retain_itemcarries the rejection reason intoEnvelope::retain_items, which records rejected quantities throughRecordKeeper. The same path fixes these tested combinations without pair-specific branches:Enforcement::apply_to_managed, then count rejectedExtractedSpansduring transaction processing (Python SDK envelope shape, integration tests).Refs RELAY-193