-
-
Notifications
You must be signed in to change notification settings - Fork 4.5k
feat(seer): Add separate scanner acknowledgement function with rollout rate #103496
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
…t rate - Add `get_seer_org_acknowledgement_for_scanner` function that uses deterministic rollout via `in_rollout_group` instead of random.random() - Add `seer.scanner_no_consent.rollout_rate` option for controlling scanner rollout - Update `is_issue_eligible_for_seer_automation` to use new function - Add comprehensive unit tests for `get_seer_org_acknowledgement_for_scanner` - Add unit tests for `is_issue_eligible_for_seer_automation` - Fix duplicate return statements in `has_seer_access_with_detail` The new function allows independent control of scanner behavior with a deterministic rollout based on organization ID, separate from the general Seer acknowledgement logic.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #103496 +/- ##
===========================================
- Coverage 80.68% 80.68% -0.01%
===========================================
Files 9246 9246
Lines 394964 394968 +4
Branches 25182 25182
===========================================
Hits 318686 318686
- Misses 75847 75851 +4
Partials 431 431 |
src/sentry/seer/seer_setup.py
Outdated
| if features.has("organizations:gen-ai-consent-flow-removal", organization) and in_rollout_group( | ||
| "seer.scanner_no_consent.rollout_rate", organization.id | ||
| ): | ||
| return True | ||
|
|
||
| return False |
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.
nit: can replace w/ return features.has(...) and ...?
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.
yup, can follow up.
- Simplify get_seer_org_acknowledgement_for_scanner to use a single return statement - Update all test patches from get_seer_org_acknowledgement to get_seer_org_acknowledgement_for_scanner - Fix failing tests in test_post_process.py
| ) | ||
|
|
||
| register( | ||
| "seer.scanner_no_consent.rollout_rate", |
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.
nit but i believe we prefer to use hyphens rather than underscores in options
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.
really? i see both in the file.
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.
ah yeah, that's not surprising. kind of a convention that's hard to stick to/enforce. we document it here but i couldn't tell you why 🤷♂️
shashjar
left a comment
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.
left a nit
This PR adds a new
get_seer_org_acknowledgement_for_scannerfunction that provides independent control over scanner behavior with respect to AI consent. We were seeing increased volume from scanner after enabling theno_ai_consentflag, and we'd like to roll this increased volume out separately to the in-UI AI changes.We do this by adding a new option which will allow us to control the rollout rate of scanner separately to the rest of our AI features.