1759682 - Move crash annotations into the minidump and remove the .extra file

Status: NEW

(Toolkit :: Crash Reporting, task)

Description

[Gabriele Svelto [:gsvelto]]

In the current system annotations are written out at crash time into a JSON file (with an .extra file prefix) and submitted alongside the minidump. A fully formed crash report requires both the minidump and the annotations but handling two files is complicated, causes a lot of code to be duplicated, increases the number of places where we can encounter an error and so on.

We should write the annotations into the minidump and get rid of the .extra file. This would simplify handling crash reports.

Crashpad already has a concept of an annotation stream (see here and we could consider reusing it to store the annotations. However crashpad's concept of annotations is different than ours. Specifically crashpad annotations consist of a per-process list and a number of per-module lists. The latter are attached to a module, with the pointer to the annotations being stored in the platform-specific module header.

Our annotation list is global ATM but we'd like to make it per-process instead so we might want to roll out our own stream instead to accommodate for this requirement.

Last but not least annotations in child processes are currently read from the crashed process within the exception handler and then sent to the main process for writing. In a fully OOP writer every process would need to do this, but this requires some sort of an IPC channel and I would like to avoid it. Given that we read from the process memory to write a minidump it would be best if we'd also read the annotations from the target process without involving its exception handler.

This is not strictly needed for bug 1620998 but we might consider implementing it prior to smooth our road towards fully out-of-process crash report generation.

Comment 1

[Will Kahn-Greene [:willkg] ET needinfo? me]

For planning purposes, this will require big changes in the Socorro collector and a rust-minidump-derived tool that can parse the annotations out of a minidump.

Comment 2

[Gabriele Svelto [:gsvelto]]

My goal is for the output of the rust-minidump stackwalker should stay the same. Basically I'd like that:

minidump-stackwalk <foo>.dmp --raw-json <foo>.extra

and

minidump-stackwalk <foo>.dmp

would give the same result given the same annotations irrespective of where they are stored.

Ingestion on the other hand would be dependent on being able to extract a subset of the information (e.g. version number, release channel, etc...) from the minidump without processing it fully as we had discussed. Basically we need all the information that is currently extracted from the .extra file during ingestion to be quickly available.

Comment 3

[Gabriele Svelto [:gsvelto]]

Removing this from the dependencies of bug 1620998 because it's not really needed. This is something we might consider doing down the line, after we've completed overhauling everything else.

Comment 4

[Gabriele Svelto [:gsvelto]]

I've got an update on this as work on bug 1776197 progresses. The easiest path forward is to store the annotations inside the minidump within the whole crash reporting flow, then spit them out into an .extra file only for submission purposes, so that no changes are required on Socorro's side. This would simplify the crash reporting flow without having any external impact.