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

Support .NET Source Context #725

Closed
Swatinem opened this issue Dec 1, 2022 · 4 comments
Closed

Support .NET Source Context #725

Swatinem opened this issue Dec 1, 2022 · 4 comments

Comments

@Swatinem
Copy link
Member

Swatinem commented Dec 1, 2022

Problem Statement

.NET Developers want to see source context in their Sentry Issues, just like with any other language ecosystem.

CC @vaind @bruno-garcia

We have two ways to achieve this.

Alternative 1: Using Portable PDB Embedded Sources

Portable PDBs have support for embedded sources, they are defined here:
https://github.com/dotnet/runtime/blob/main/docs/design/specs/PortablePdb-Metadata.md#embedded-source-c-and-vb-compilers

A .NET compiler will do so if instructed by using <EmbedAllSources>true</EmbedAllSources> in the build definition.

With this mechanism, we would need to extend our Portable PDB reader support, and cache in the following way:

  • The Portable PDB reader needs to be able to parse the embedded source
  • The Portable PDB Cache writer needs to put that source into the cache file. We can use a similar way to how we deal with source contents in JS SourceMaps, and also generate a list of line-splits for extremely quick line access.
  • Symbolicator would need to apply this embedded source in the symbolication step (not in the "apply source context" step afterwards).

Extending the Portable PDB Cache format for sources would need a new format version and backwards compatiblity to the version without sources.

Alternative 2: Using existing SourceBundles

In this approach, we would use sentry-cli to generate the existing sourcebundle format, and upload that as a separate file to Sentry.

For this, we would need the following:

  • A way to iterate over all the referenced files from a Portable PDB.
  • Once this iterator is hooked up, sentry-cli will look up those files from the local file system (not from within the Portable PDB)
  • The rest of the pipeline will have the ability to handle sources, with no extra effort.

Discussion

Approach 2 would be the simpler solution implementation-wise, as it would just need one additional change, iterating over the files referenced by the Portable PDB. It also does not require developers to embed sources into the Portable PDB in the first place.

However, profiling and benchmarks have shown that our SourceBundle format is not the most efficient. Zip handling and parsing the JSON manifest (for every usage of the file) shows up in profiles as an expensive operation.

Appoach 1 would need more implementation effort, but would have the following benefits:

  • You only need the .pdb file, and don’t need to have the original sources on the local file system to create a SourceBundle.
  • Only 1 file needed instead of 2.
  • Using a more efficient lookup format than existing SourceBundle.

Followup: Fixing SourceBundle

The inefficiency of the SourceBundle format comes from two downsides: usage of not-state-of-the-art zip/deflate, and usage of JSON manifests. We can fix both:

  • Use zstd compression instead of zip
  • Use a simple binary format instead of JSON which can be loaded instantaneously instead of needing to be parsed

The current SourceBundle format has the advantage of being "almost" a valid zip file, and it can be extracted by tools like unzip, though some other tools will reject it. This is a valueable property as it allows folks to inspect the contents easily without need for more sophisticated tools.

@vaind
Copy link
Collaborator

vaind commented Dec 1, 2022

As posted in the dotnet issue, here's an example app with a PDB that should contain sources: https://github.com/getsentry/sentry-dotnet/files/10124988/console-sample-pdb-with-sources.zip

@vaind
Copy link
Collaborator

vaind commented Dec 1, 2022

Use a simple binary format instead of JSON which can be loaded instantaneously instead of needing to be parsed

I have good experience with flatbuffers which is extremely fast and has backward-compatible schema updates.

@vaind
Copy link
Collaborator

vaind commented Dec 1, 2022

FYI: I've started looking into Alternative 2, i.e. support --include-sources in the CLI, which can work regardless of a future Alternative 1 implementation

@Swatinem
Copy link
Member Author

Swatinem commented Dec 2, 2022

👍🏻 I believe Alternative 2 is a good quick solution.

Eventually if we want to invest more effort into this, we can also take on alternative 1 later on.

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

No branches or pull requests

3 participants