Skip to content
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

feat(grouping): Group chunk load errors #60885

Merged
merged 7 commits into from
Dec 11, 2023
Merged

Conversation

jangjodi
Copy link
Member

@jangjodi jangjodi commented Nov 30, 2023

Group node ChunkLoadErrors based on their message

fixes #57458

@jangjodi jangjodi requested a review from a team as a code owner November 30, 2023 17:03
@jangjodi jangjodi requested a review from a team November 30, 2023 17:03
Copy link

sentry-io bot commented Nov 30, 2023

🚀 Sentry Issue Report

You modified these files in this pull request and we noticed these issues associated with them.

📄 src/sentry/event_manager.py

Issue Additional Info
‼️ HashDiscarded: Timeout when getting next_short_id issues.occurrence_consume... Handled: True Event Count: 69 Users: 0
‼️ HashDiscarded: Matches group tombstone 108268 Pro... Handled: True Event Count: 20 Users: 6
‼️ HashDiscarded: Timeout when getting next_short_id issues.occurrence_consume... Handled: True Event Count: 12 Users: 0
‼️ HashDiscarded: Timeout when getting next_short_id issues.occurrence_consume... Handled: True Event Count: 5 Users: 0

Did you find this useful? React with a 👍 or 👎 or let us know in #proj-github-pr-comments

@github-actions github-actions bot added the Scope: Backend Automatically applied to PRs that change backend components label Nov 30, 2023
Copy link

codecov bot commented Nov 30, 2023

Codecov Report

Merging #60885 (40c5410) into master (9d8b42f) will decrease coverage by 0.01%.
Report is 5 commits behind head on master.
The diff coverage is n/a.

Additional details and impacted files
@@            Coverage Diff             @@
##           master   #60885      +/-   ##
==========================================
- Coverage   81.13%   81.12%   -0.01%     
==========================================
  Files        5191     5193       +2     
  Lines      228146   228255     +109     
  Branches    38262    38277      +15     
==========================================
+ Hits       185103   185175      +72     
- Misses      37425    37464      +39     
+ Partials     5618     5616       -2     
Files Coverage Δ
static/app/components/sidebar/index.tsx 90.90% <ø> (-0.14%) ⬇️
static/app/constants/index.tsx 92.10% <ø> (ø)
static/app/utils/isActiveSuperuser.tsx 80.00% <ø> (-1.82%) ⬇️
static/app/utils/theme.tsx 100.00% <ø> (ø)

... and 56 files with indirect coverage changes

Base automatically changed from jodi/group-chunkloaderror-flag to master November 30, 2023 18:12
Copy link
Member

@lobsterkatie lobsterkatie left a comment

Choose a reason for hiding this comment

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

This is a good start! I've highlighted a bunch of small things, but overall I think the logic is sound. (It kinda sucks that we have to brute-force it this way, but until we decide on a more general way to handle weird cases like this, I think it's what we're left with.)

src/sentry/event_manager.py Outdated Show resolved Hide resolved
src/sentry/event_manager.py Outdated Show resolved Hide resolved
src/sentry/event_manager.py Outdated Show resolved Hide resolved
Comment on lines +2641 to +2642
# Only check for the flag after it is established if it's a ChunkLoadError to avoid
# unnecessary querying
Copy link
Member

Choose a reason for hiding this comment

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

Good thinking!

src/sentry/event_manager.py Outdated Show resolved Hide resolved
tests/sentry/event_manager/test_event_manager_grouping.py Outdated Show resolved Hide resolved
tests/sentry/event_manager/test_event_manager_grouping.py Outdated Show resolved Hide resolved
@jangjodi jangjodi merged commit 4b3d311 into master Dec 11, 2023
50 checks passed
@jangjodi jangjodi deleted the jodi/group-chunk-load-errors branch December 11, 2023 17:08
Copy link

sentry-io bot commented Dec 11, 2023

Suspect Issues

This pull request was deployed and Sentry observed the following issues:

  • ‼️ TypeError: 'NoneType' object is not subscriptable sentry.tasks.store.save_event View Issue

Did you find this useful? React with a 👍 or 👎

@jangjodi jangjodi added the Trigger: Revert Add to a merged PR to revert it (skips CI) label Dec 11, 2023
@getsentry-bot
Copy link
Contributor

PR reverted: b9c7b73

getsentry-bot added a commit that referenced this pull request Dec 11, 2023
@github-actions github-actions bot locked and limited conversation to collaborators Dec 27, 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 Trigger: Revert Add to a merged PR to revert it (skips CI)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Webpack ChunkLoadErrors Filtered out
3 participants