Closed Bug 1453766 Opened 3 years ago Closed 3 years ago

Update crash ping documentation to mention StartupCrash

Categories

(Toolkit :: Crash Reporting, defect)

defect
Not set
normal
Points:
1

Tracking

()

RESOLVED FIXED
mozilla61
Tracking Status
firefox61 --- fixed

People

(Reporter: akomar, Assigned: gsvelto)

References

Details

Attachments

(1 file)

The crash ping documentation is missing metadata.StartupCrash field.
Gabriele, is this something you can take a look at?
Component: Telemetry → Crash Reporting
Flags: needinfo?(gsvelto)
Sure, taking this.
Assignee: nobody → gsvelto
Status: NEW → ASSIGNED
Flags: needinfo?(gsvelto)
I took the liberty of updating a few more fields to clarify what they do.
Comment on attachment 8967759 [details]
Bug 1453766 - Improve the documentation of the crash ping's metadata fields;

https://reviewboard.mozilla.org/r/236464/#review242234

Thanks!
Attachment #8967759 - Flags: review?(gfritzsche) → review+
Arkadiusz, does that cover your needs?
Flags: needinfo?(akomarzewski)
That's great, thanks!
Status: ASSIGNED → RESOLVED
Closed: 3 years ago
Flags: needinfo?(akomarzewski)
Resolution: --- → FIXED
We still have to land this :)
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Pushed by gsvelto@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/dc31fa8322c2
Improve the documentation of the crash ping's metadata fields; r=gfritzsche
https://hg.mozilla.org/mozilla-central/rev/dc31fa8322c2
Status: REOPENED → RESOLVED
Closed: 3 years ago3 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla61
Gabriele, I think documented type of `StartupCrash` is not quite right - it's actually String, not an Int.
Flags: needinfo?(gsvelto)
(In reply to akomarzewski from comment #11)
> Gabriele, I think documented type of `StartupCrash` is not quite right -
> it's actually String, not an Int.

You mean in the actual ping? I believe the way it is assembled all the fields under metadata are encoded as strings but boolean fields like StartupCrash will always be set to "1" for true or "0" for false (or absent which usually means false). Since these come from crash annotations I decided to stick with the annotation's type because that's how it should be interpreted.
Flags: needinfo?(gsvelto)
(In reply to Gabriele Svelto [:gsvelto] from comment #12)
> You mean in the actual ping? I believe the way it is assembled all the
> fields under metadata are encoded as strings but boolean fields like
> StartupCrash will always be set to "1" for true or "0" for false (or absent
> which usually means false). Since these come from crash annotations I
> decided to stick with the annotation's type because that's how it should be
> interpreted.

I see, looks like it's consistent with other fields after all.
Sorry for the false alarm (and thanks again!).
(In reply to Gabriele Svelto [:gsvelto] from comment #12)
> (In reply to akomarzewski from comment #11)
> > Gabriele, I think documented type of `StartupCrash` is not quite right -
> > it's actually String, not an Int.
> 
> You mean in the actual ping? I believe the way it is assembled all the
> fields under metadata are encoded as strings but boolean fields like
> StartupCrash will always be set to "1" for true or "0" for false (or absent
> which usually means false). Since these come from crash annotations I
> decided to stick with the annotation's type because that's how it should be
> interpreted.

Gabriele, it sounds like we should include in the documentation the casting of all metadata fields to strings. It's  a bit surprising on the analysis side.
Flags: needinfo?(gsvelto)
Good point, I'll file another bug for that.
Flags: needinfo?(gsvelto)
Blocks: 1455073
You need to log in before you can comment on or make changes to this bug.