Skip to content

Conversation

@lobsterkatie
Copy link
Member

In our current grouping enhancements code, we designate actions (and thereby rules containing said actions) as "modifiers" and/or "updaters," and we provide the ability to split a rule which is both into a modifier version and an updater version. There are two problems with that: 1) Updating and modifying are effectively synonymous concepts, and so it's not at all clear what it means to be one or the other. 2) We actually shouldn't even have this code anymore, because it's a relic of both the pre-rust-enhancements era and the hierarchical grouping era, and is no longer used.

Rather than remove the code, though, we can actually repurpose it. During ingest, we run the stacktrace through the rules twice, once to assign in_app and category values to frames, and once to set contributes values on both the frames and the overall stacktrace. Being able to split a given set of rules into ones useful for the first task and ones useful for the second task would actually be helpful, because at each stage it would allow us to avoid checking rules we know won't apply. It also would solve the problem of "marked in-app by rule x" hints clobbering "ignored by rule x" hints (and vice versa) in cases where a single rule has both effects.

This PR therefore changes the existing designations from is_modifier and is_updater to is_classifier and sets_contributes (for actions) and has_classifier_actions and has_contributes_actions (for rules). Further changes to use this information will come in follow-up PRs.

Note to reviewers: I super don't love the non-parallel naming with is_classifier and sets_contributes, but I struggled to find anything with parallel structure that wasn't horribly ungrammatical. Definitely open to ideas here.

@github-actions github-actions bot added the Scope: Backend Automatically applied to PRs that change backend components label Mar 21, 2025
@codecov
Copy link

codecov bot commented Mar 21, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

✅ All tests successful. No failed tests found.

Additional details and impacted files
@@             Coverage Diff             @@
##           master   #87655       +/-   ##
===========================================
+ Coverage   51.76%   87.74%   +35.98%     
===========================================
  Files        9882     9893       +11     
  Lines      561062   561575      +513     
  Branches    22134    22134               
===========================================
+ Hits       290427   492780   +202353     
+ Misses     270234    68394   -201840     
  Partials      401      401               

@lobsterkatie lobsterkatie force-pushed the kmclb-repurpose-updater-modifier-action-and-rule-designations branch from 7ca7a9d to 735431c Compare March 21, 2025 23:04
@lobsterkatie lobsterkatie marked this pull request as ready for review March 24, 2025 17:27
@lobsterkatie lobsterkatie requested a review from a team as a code owner March 24, 2025 17:27
Copy link
Member

@armenzg armenzg left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Very nice PR!

@lobsterkatie lobsterkatie merged commit 107ddf3 into master Mar 24, 2025
48 checks passed
@lobsterkatie lobsterkatie deleted the kmclb-repurpose-updater-modifier-action-and-rule-designations branch March 24, 2025 21:42
andrewshie-sentry pushed a commit that referenced this pull request Mar 27, 2025
…87655)

In our current grouping enhancements code, we designate actions (and thereby rules containing said actions) as "modifiers" and/or "updaters," and we provide the ability to split a rule which is both into a modifier version and an updater version. There are two problems with that: 1) Updating and modifying are effectively synonymous concepts, and so it's not at all clear what it means to be one or the other. 2) We actually shouldn't even have this code anymore, because it's a relic of both the pre-rust-enhancements era and the hierarchical grouping era, and is no longer used.

Rather than remove the code, though, we can actually repurpose it. During ingest, we run the stacktrace through the rules twice, once to assign `in_app` and `category` values to frames, and once to set `contributes` values on both the frames and the overall stacktrace. Being able to split a given set of rules into ones useful for the first task and ones useful for the second task would actually be helpful, because at each stage it would allow us to avoid checking rules we know won't apply. It also would solve the problem of "marked in-app by rule x" hints clobbering "ignored by rule x" hints (and vice versa) in cases where a single rule has both effects.

This PR therefore changes the existing designations from `is_modifier` and `is_updater` to `is_classifier` and `sets_contributes` (for actions) and `has_classifier_actions` and `has_contributes_actions` (for rules). Further changes to use this information will come in follow-up PRs.
@github-actions github-actions bot locked and limited conversation to collaborators Apr 9, 2025
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

Scope: Backend Automatically applied to PRs that change backend components

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants