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: source context links support #1103

Merged
merged 6 commits into from
Mar 20, 2023
Merged

Conversation

vaind
Copy link
Contributor

@vaind vaind commented Mar 16, 2023

@vaind vaind mentioned this pull request Mar 16, 2023
1 task
@codecov
Copy link

codecov bot commented Mar 17, 2023

Codecov Report

Merging #1103 (8d16cfa) into master (be02c6e) will decrease coverage by 3.19%.
The diff coverage is 92.68%.

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #1103      +/-   ##
==========================================
- Coverage   74.14%   70.95%   -3.19%     
==========================================
  Files          86       86              
  Lines       12476    12517      +41     
==========================================
- Hits         9250     8882     -368     
- Misses       3226     3635     +409     

@vaind vaind marked this pull request as ready for review March 17, 2023 13:38
@vaind vaind requested a review from a team March 17, 2023 13:38
Co-authored-by: Arpad Borsos <swatinem@swatinem.de>
@Swatinem Swatinem enabled auto-merge (squash) March 20, 2023 08:29
Comment on lines +157 to +158
// Only resolve source context from URLs if the frame is "in-app".
if frame.raw.in_app.unwrap_or(false) {
Copy link
Member

Choose a reason for hiding this comment

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

This also needs changes in the sentry processor to enable this, as it current does not send any in_app attributes.

The other problems with this that come to mind:

  • in_app is being modified/set by grouping, which runs after symbolication.
  • minidumps never set in_app at all. neither do some SDKs like Native. However those debug files do not (yet?) have support for source links.

I’m okay with merging this as is, but this might need to be revisited.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

in .NET, in_app is being sent from the SDK. What would need to change for that to get propagated here?

@Swatinem Swatinem merged commit 6fca4fe into getsentry:master Mar 20, 2023
@vaind vaind deleted the feat/source-links branch March 20, 2023 08:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants