chore(server): Remove GenerateBillingOutcome flag usage#6228
Conversation
797ceb7 to
69b1fb4
Compare
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Want reviews to match your repository better? Bugbot Learning can learn team-specific rules from PR activity. A team admin can enable Learning in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit 69b1fb4. Configure here.
| /// Enable relay billing outcome generation. | ||
| #[serde(rename = "organizations:relay-generate-billing-outcome")] | ||
| GenerateBillingOutcome, | ||
|
|
There was a problem hiding this comment.
Graduated billing flag not preserved
Medium Severity
Removing GenerateBillingOutcome without a deprecated serde variant or GRADUATED_FEATURE_FLAGS entry means organizations:relay-generate-billing-outcome deserializes as Unknown and is dropped from FeatureSet. Upstream Relays no longer retain or inject that flag in forwarded project configs, so older processing Relays that still gate track_accepted_outcome on it can stop emitting metric billing outcomes.
Reviewed by Cursor Bugbot for commit 69b1fb4. Configure here.
![sentry[bot]](https://avatars.githubusercontent.com/in/12637?s=60&v=4){kind=small}
|
|
||
| outcomes.sort(key=lambda o: o["outcome"]) | ||
|
|
||
| # skip billing outcomes | ||
| assert outcomes[0]["outcome"] == 0 | ||
| assert outcomes[1]["outcome"] == 0 | ||
|
|
||
| o = outcomes[2] | ||
| assert [(o["category"], o["outcome"], o["reason"])] == [ | ||
| (9, 1, "discarded"), # TransactionIndexed, Filtered | ||
| ] |
There was a problem hiding this comment.
Bug: The test test_discard_transaction hardcodes an expectation of 3 outcomes, but 4 are now generated, causing an assertion failure in poll_many.
Severity: LOW
Suggested Fix
The test should be made more robust. Instead of fetching a hardcoded number of outcomes with get_outcomes(n=3), it should fetch all available outcomes without specifying n. Then, it should filter the results to find the specific outcome being tested or dynamically verify the total count. Sorting should also use a more stable key, like the outcome category, to avoid flakiness.
Prompt for AI Agent
Review the code at the location below. A potential bug has been identified by an AI
agent. Verify if this is a real issue. If it is, propose a fix; if not, explain why it's
not valid.
Location: tests/integration/test_spans.py#L1103-L1114
Potential issue: The test `test_discard_transaction` hardcodes an expectation for
exactly 3 outcomes by calling `outcomes_consumer.get_outcomes(n=3)`. However, due to the
removal of the `GenerateBillingOutcome` feature flag, `track_accepted_outcome` is now
always called. For the `envelope_with_transaction_and_spans` fixture, this change causes
4 outcomes to be generated: one for a non-segment child span, two for the segment span
(one `Span` and one `Transaction`), and one `TransactionIndexed` filtered outcome. The
`poll_many` method asserts that the number of received messages matches the requested
number `n`, which will fail with an error like "Expected 3 messages, only got 4".
Did we get this right? 👍 / 👎 to inform future reviews.
3 participants
No description provided.