Get Debug ID for Full PDB format on Windows #2222
Conversation
mattjohnsonpint
requested review from
vaind,
SeanFeldman and
bruno-garcia
and removed request for
bitsandfoxes
| DebugFile: mscorlib.pdb, | ||
| CodeId: ______________, | ||
| CodeFile: .../mscorlib.dll | ||
| }, |
There was a problem hiding this comment.
Hey look - now that are collecting it, we also get a debug id for mscorlib on .NET Framework. (This should light up some framework symbolication.)
mattjohnsonpint
force-pushed
the
fix/full-pdb
branch
from
8c8e345
to
a2f6d05
Compare
| return idx; | ||
| } | ||
|
|
||
| internal static DebugImage? GetDebugImage(Module module, SentryOptions options) |
There was a problem hiding this comment.
Note this is also just refactoring. The AddDebugImage method now is entirely responsible for managing the cache, while GetDebugImage can focus on the hard parts. (We can later add some tests on GetDebugImage.)
mattjohnsonpint
force-pushed
the
fix/full-pdb
branch
from
a2f6d05
to
69539dc
Compare
| else | ||
| { | ||
| // Full PDB Format (Windows only) | ||
| // Version Major=0, Minor=0 | ||
| debugId = $"{codeView.Guid}-{codeView.Age}"; | ||
| } |
There was a problem hiding this comment.
This is the format we did not previously have. Everything else in the PR is refactoring and optimization.
Sounds great though it would break uses using sentry-cli manually. |
|
Another problem is, we'd have to write the attribute to the assembly during compilation before the upload was attempted. So if the upload fails, it's to late to change the attribute. I'll keep thinking about it though. It might be ok. Either way, it's a separate concern from this PR. This one is specifically about getting the |
I would be surprised if people are using sentry cli manually. Is there a reason we need to support that? I feel the trade off here (vast majority will not opt in to this feature) indicates it's best to avoid the overhead of adding all this debug id's and extra debug_meta to events unless we know we need server side symbolication or source context. We could also get source context client side if sources are embedded in the pdb and released with the app (server use cases) avoiding the need to upload anything. |
|
Let's keep this issue focused on the full-pdb debug id. Moving discussion about whether debug images should be sent or not to #2227. Thanks. |
Fixes #2221.
Also applied some refactoring and cleanup.