Improve crash call stack data.#14555
Conversation
There was a problem hiding this comment.
Pull request overview
This PR adjusts how C/C++ crash telemetry is generated in the extension to make crash call stack data easier to read and analyze by removing the separate “offset/address” telemetry field and restructuring the emitted crash header/details.
Changes:
- Removes the separate
CrashingThreadCallStackOffsetstelemetry property and simplifies the telemetry logging helpers. - Changes the crash telemetry header/signal formatting (adds
si_code/si_addrinline) and alters how address/offset data is appended to the call stack text. - Updates the “C/C++ Crash Call Stacks” output formatting to include the new signal info.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| const isCppToolsSrv2: boolean = crashFile.startsWith("cpptools-srv2"); | ||
| const isCppToolsSrv: boolean = crashFile.startsWith("cpptools-srv"); | ||
| const telemetryHeader: string = (isCppToolsSrv2 ? "cpptools-srv2.txt" : isCppToolsSrv ? "cpptools-srv.txt" : crashFile) + "\n"; | ||
| const telemetryHeader: string = (isCppToolsSrv2 ? "cpptools-srv2 process" : isCppToolsSrv ? "cpptools-srv process" : "cpptools process") + "\n"; |
There was a problem hiding this comment.
I already have renaming rules in my data queries. Why do we need to change this?
There was a problem hiding this comment.
The ".txt" doesn't make any sense (Copilot doesn't understand it, neither do humans). It was a bad idea to use to begin with. The intention is " process".
I don't know what renaming rules you're referring to are.
There was a problem hiding this comment.
Regarding the Copilot comment, cpptools-wordexp isn't known to crash.
There was a problem hiding this comment.
The line used to mean "this is the file I got it from". In my PowerBI dashboard I have it renamed to remove the .txt for the crashes table. If you are going to do anything with this part of the telemetry, just delete the .txt and have it be the process name only. Adding the word "process" to this string doesn't provide any value for me and actually requires me to go add more renaming logic to my dashboard.
There was a problem hiding this comment.
And who else needs to understand the context? This is telemetry that only we see.
There was a problem hiding this comment.
Umm...if you really want I can remove " process" and hope the reader can infer that from the context, but keeping ".txt" is confusing.
There was a problem hiding this comment.
What reader? Are we logging this to the Output window or something? If the goal is to present this to users and have them understand it, wouldn't we add labels and such for fields in the logging? That would be more effective than printing out raw telemetry payloads don't you think?
There was a problem hiding this comment.
@bobbrow Copilot.
I answered that question earlier. #14555 (comment)
The previous crash call stack data was unnecessarily hard to analyze. The separate address data was not necessary.
Fixed by Copilot with Claude Opus 4.8 in VS Code.