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: Capture Debug Images and StackFrame fields for Portable PDB symbolication #1785

Closed
wants to merge 1 commit into from

Conversation

Swatinem
Copy link
Member

@Swatinem Swatinem commented Jul 19, 2022

This adds the new event properties defined in RFC 0013, namely InstructionAddress / FunctionId + Relative Indexing, and a list of pe_dotnet DebugImages.

The new information allows symbolicating .NET stack traces on symbolicator, using uploaded Portable PDB debug files.

Example events that were captured with this PR and symbolicated serverside:

The second event might be an example where it could be possible to load debug symbols from the Microsoft Symbol Server which is not possible yet, but we might add this later.


I open this PR with the hope of either delegating it to the .NET team, or at the very least get some dedicated mentoring to finish it up.
The problem I see right now are:

  • The PR depends on default interface methods which I believe we can’t use because of backwards compatibility?
  • The way that stack frame factories and event / exception processors are implemented means that we currently have no way to process frames while at the same time have "atomic" access to the list of debug images. The relative indexing that we need depends on having full control over the index of the referenced debug images in the DebugImages list. Currently the different factories and processors can change that list as they please, which makes it quite fragile.

@github-actions
Copy link
Contributor

github-actions bot commented Jul 19, 2022

Fails
🚫 Please consider adding a changelog entry for the next release.
Messages
📖 Do not forget to update Sentry-docs with your feature once the pull request gets approved.

Instructions and example for changelog

Please add an entry to CHANGELOG.md to the "Unreleased" section. Make sure the entry includes this PR's number.

Example:

## Unreleased

- Capture Debug Images and StackFrame fields for Portable PDB symbolication ([#1785](https://github.com/getsentry/sentry-dotnet/pull/1785))

If none of the above apply, you can opt out of this check by adding #skip-changelog to the PR description.

Generated by 🚫 dangerJS against 3324668

@Swatinem Swatinem changed the title add a small test program to get il offset + metadata tokens [IGNORE for now] add a small test program to get il offset + metadata tokens Jul 19, 2022
@Swatinem Swatinem force-pushed the feat/fn-idx branch 2 times, most recently from df29039 to ddddffd Compare October 11, 2022 10:10
@Swatinem Swatinem changed the title [IGNORE for now] add a small test program to get il offset + metadata tokens feat: Capture Debug Images and StackFrame fields for Portable PDB symbolication Oct 19, 2022
@Swatinem Swatinem marked this pull request as ready for review October 19, 2022 10:00
@mattjohnsonpint mattjohnsonpint self-assigned this Oct 19, 2022
@mattjohnsonpint
Copy link
Contributor

Thanks for all your hard work on this! I've merged in main to catch up this branch. I'll review in further detail and update failing tests, etc. as soon as time allows. 👍

This adds new protocol fields to StackFrame and DebugImage that are needed for server side Portable PDB symbolication.

The StackTraceFactory is also extended in such a way that it collects referenced DebugImages which are then later added to the event.
@vaind
Copy link
Collaborator

vaind commented Nov 15, 2022

I've rebased this PR but in the end decided to replace the PR as the approach may end up different, due to the lack of default interface method support this PR relied on. Also, want to keep the new PR as a draft to avoid spamming everyone before it's ready.

replaced by #2050

@vaind vaind closed this Nov 15, 2022
@bruno-garcia bruno-garcia deleted the feat/fn-idx branch October 27, 2023 02:13
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.

None yet

3 participants