Skip to content

feat(suspect-commits): Remove suspect commit recalculation period (when all-frames is enabled) #58415

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

Merged
merged 1 commit into from
Oct 20, 2023

Conversation

malwilley
Copy link
Member

@malwilley malwilley commented Oct 18, 2023

Closes #57438

See ticket for more info, but we are no longer going to run the process_commit_context task when new events come in. To increase accuracy, we want to calculate the suspect commit only at the time the issue was created. Otherwise we might select commits that were made after the issue started.

There is a lot of cleanup that can be done after GA (around removing the debounce cache especially), but this is the first step.

Note: depends on #58307 being merged

@malwilley malwilley requested a review from a team as a code owner October 18, 2023 23:18
@github-actions github-actions bot added the Scope: Backend Automatically applied to PRs that change backend components label Oct 18, 2023
@codecov
Copy link

codecov bot commented Oct 18, 2023

Codecov Report

Merging #58415 (8e913c0) into master (3889396) will increase coverage by 2.60%.
Report is 4 commits behind head on master.
The diff coverage is 100.00%.

❗ Current head 8e913c0 differs from pull request most recent head 3322163. Consider uploading reports for the commit 3322163 to get more accurate results

@@            Coverage Diff             @@
##           master   #58415      +/-   ##
==========================================
+ Coverage   76.48%   79.09%   +2.60%     
==========================================
  Files        5140     5144       +4     
  Lines      224551   223980     -571     
  Branches    37843    37715     -128     
==========================================
+ Hits       171755   177149    +5394     
+ Misses      46987    41154    -5833     
+ Partials     5809     5677     -132     
Files Coverage Δ
src/sentry/tasks/post_process.py 91.06% <100.00%> (+19.89%) ⬆️

... and 723 files with indirect coverage changes

Copy link
Member

@wedamija wedamija left a comment

Choose a reason for hiding this comment

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

This seems ok but have we considered the merging case?

@malwilley
Copy link
Member Author

@wedamija we did consider merging, but have decided that merging non-related problems together is something we don't want to try to support since it makes the common case (each issue has one cause) more difficult to get right. If you have ideas on that front I'm all ears!

cc @armenzg

@malwilley malwilley force-pushed the malwilley/feat/suspect-commits-remove-recalc branch from 8e913c0 to 3322163 Compare October 19, 2023 21:04
@malwilley malwilley merged commit f421fee into master Oct 20, 2023
@malwilley malwilley deleted the malwilley/feat/suspect-commits-remove-recalc branch October 20, 2023 16:14
@github-actions github-actions bot locked and limited conversation to collaborators Nov 5, 2023
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.

Remove 7-day recalculation of suspect commits
3 participants