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): Exclude sentry-java sdk frames from group and inapp #60691

Merged
merged 2 commits into from
Dec 1, 2023

Conversation

romtsn
Copy link
Member

@romtsn romtsn commented Nov 28, 2023

This is necessary for the sentry-java sdk to not mess up grouping, because we'll start sending sdk frames for fatals from the next major version (relevant pr: getsentry/sentry-java#3021)

Also changed io.sentry to com.example in stacktrace-collapse-recursion snapshot so it's not affected by this change

@romtsn romtsn requested a review from a team November 28, 2023 16:26
@romtsn romtsn requested a review from a team as a code owner November 28, 2023 16:26
@github-actions github-actions bot added the Scope: Backend Automatically applied to PRs that change backend components label Nov 28, 2023
Copy link

codecov bot commented Nov 28, 2023

Codecov Report

Merging #60691 (949e716) into master (984e550) will increase coverage by 0.12%.
Report is 20 commits behind head on master.
The diff coverage is n/a.

Additional details and impacted files
@@            Coverage Diff             @@
##           master   #60691      +/-   ##
==========================================
+ Coverage   80.76%   80.88%   +0.12%     
==========================================
  Files        5185     5185              
  Lines      227509   227514       +5     
  Branches    38244    38247       +3     
==========================================
+ Hits       183742   184029     +287     
+ Misses      38143    37861     -282     
  Partials     5624     5624              

see 77 files with indirect coverage changes

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.

So, if I understand correctly, this doesn't affect current issues, just ones which will come from the new version of the SDK. Is that right?

@romtsn
Copy link
Member Author

romtsn commented Nov 28, 2023

So, if I understand correctly, this doesn't affect current issues, just ones which will come from the new version of the SDK. Is that right?

yes, that's right!

@romtsn
Copy link
Member Author

romtsn commented Nov 30, 2023

@lobsterkatie can we get this approved, or do you see any problems with that? :)

@kahest
Copy link
Member

kahest commented Nov 30, 2023

@lobsterkatie also just to be 100% sure - once this is merged, the new rule is applied immediately for new events, correct?

@lobsterkatie
Copy link
Member

Yes, it will be applied immediately. But I checked out getsentry/sentry-java#3021 and it seems like currently, no events have sentry frames, correct? If that's true, then this is fine to merge.

@romtsn
Copy link
Member Author

romtsn commented Dec 1, 2023

@lobsterkatie yep, correct. Could you please give an approval then?

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.

Yup! 🚢

@romtsn romtsn merged commit 7e66009 into master Dec 1, 2023
@romtsn romtsn deleted the rz/feat/grouping-java-sdk-frames branch December 1, 2023 22:38
@github-actions github-actions bot locked and limited conversation to collaborators Dec 17, 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.

3 participants