-
-
Notifications
You must be signed in to change notification settings - Fork 4.4k
feat(issues): Implement suspect flag heuristics for feature flags aggregated inside issue details #93076
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
🚨 Warning: This pull request contains Frontend and Backend changes! It's discouraged to make changes to Sentry's Frontend and Backend in a single pull request. The Frontend and Backend are not atomically deployed. If the changes are interdependent of each other, they must be separated into two pull requests and be made forward or backwards compatible, such that the Backend or Frontend can be safely deployed independently. Have questions? Please ask in the |
2a32413
to
6a7527f
Compare
distribution: { | ||
baseline: Record<string, number>; | ||
outliers: Record<string, number>; | ||
}; |
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.
updated to match #92801
a388e29
to
3d900bf
Compare
Codecov ReportAll modified and coverable lines are covered by tests ✅ ✅ All tests successful. No failed tests found. Additional details and impacted files@@ Coverage Diff @@
## master #93076 +/- ##
=======================================
Coverage 87.82% 87.83%
=======================================
Files 10284 10284
Lines 590331 590285 -46
Branches 22950 22941 -9
=======================================
- Hits 518468 518457 -11
+ Misses 71416 71381 -35
Partials 447 447 |
I approve 👍 |
…regated inside issue details (#93076) This isn't a final UI for everything, still behind it's own FF for employees right now only. If this set of heuristics turns out to be decent for a range of issues, then we'll have to iterate and move a bunch of logic into the backend directly. Right now it's fun and easy to experiment in the frontend, and render data related to what's used in the heuristics (ie: we use distribution, so render that too). I'm happier with the results of this algorithm in this iteration. What i've found by spot-checking a bunch of issues is that the flags which are surfaced seem related to the issue i'm looking at. more cross-cutting issues are showing more generic flags, like the `trace-view-v1` flag in the example. But I'm happy because it's trace related and the problem happened on the trace page. What the new heuristic is focused on a few things types of flags: 1. Flags that are 100% on within the issue. This means that the flag has to be one of the most recently checked flags, so that it's still in the SDK's buffer when the error happens. 2. Flags that are not 100% enabled for every user. We're doing an approximation of this, because we can only see flags when an issue happens (we're only look at the issue data set right now). There were some examples of flags that I assumed were rolled out to 100%, so like 18,000 examples where `foo=true` but had some single-digit cases where `foo=false`. These are tricky! 3. Flags where every error within the issue includes the flag. Similar to the 1st condition above, we only want to focus on flags where each error event includes a check for that issue. If the issue has 100 errors, spread across 100 users, then we want to have seen the flag 100 times as well within the issue. 4. Flags where the definition changed recently. This is poorly implemented right now. In this PR we do sort of a best-effort attempt to load a list of 100 changes for the list of flags identified in 1 thru 3 above. Then we keep the flags where a change was detected. This is another way to filter down flags to expose those which are in flux. It seems to be working well right now for new issues where firstSeen is less than 90d ago. We need to iterate on this and test more for issues that are older than 90d. Or... it could be the case that for older issues we don't need to worry about sus flags too much. | Desc | Img | | --- | --- | | Loading | <img width="501" alt="loading suspects" src="https://github.com/user-attachments/assets/ce2dd12e-e658-4337-a6af-1b46fb8054ff" /> | Nothing found | <img width="502" alt="no suspects" src="https://github.com/user-attachments/assets/685c2b9c-aed8-444b-9e99-16c8aab53fe9" /> | Single Item | <img width="500" alt="single suspect" src="https://github.com/user-attachments/assets/429000a6-6692-4b40-9341-a51f339e63fd" /> | Some Items | <img width="908" alt="flag details" src="https://github.com/user-attachments/assets/0f3c053b-e8da-44bd-92fd-25d3726ad37f" /> | Flag Details | <img width="502" alt="some suspects" src="https://github.com/user-attachments/assets/da775b29-baf7-4786-a054-800971cce15b" />
This isn't a final UI for everything, still behind it's own FF for employees right now only.
If this set of heuristics turns out to be decent for a range of issues, then we'll have to iterate and move a bunch of logic into the backend directly. Right now it's fun and easy to experiment in the frontend, and render data related to what's used in the heuristics (ie: we use distribution, so render that too).
I'm happier with the results of this algorithm in this iteration. What i've found by spot-checking a bunch of issues is that the flags which are surfaced seem related to the issue i'm looking at. more cross-cutting issues are showing more generic flags, like the
trace-view-v1
flag in the example. But I'm happy because it's trace related and the problem happened on the trace page.What the new heuristic is focused on a few things types of flags:
foo=true
but had some single-digit cases wherefoo=false
. These are tricky!