Put abort messages into the signature

VERIFIED FIXED

Status

Socorro
Backend
P2
normal
VERIFIED FIXED
5 years ago
a year ago

People

(Reporter: Robert Kaiser, Assigned: adrian)

Tracking

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment, 1 obsolete attachment)

(Reporter)

Description

5 years ago
Aborts are a class of crashes that are extremely hard to bucket right now as there's a ton of different signatures with the same abort message, and at the same time different abort messages with the same signature, and from what I gather, all we are interested in there is actually the abort message and the classic signature is usually completely irrelevant in those cases.

We should look to bucket by that abort message and therefore replace the "classic" signature with it in some form. That message might be part of the app notes right now, we probably need a platform bug to get it into its own field as well before this really becomes actionable on the Socorro side - but I wanted this to be filed and gather input about it. :)

Benjamin, do you think this is a good idea?
Yes kinda. It seems likely that we want the abort text and the calling frame, not just the abort text. I think we should run experiments before flipping any switches though.
once you can identify the source of this "abort message", I can get going on this...
Assignee: nobody → lars
(Reporter)

Comment 3

4 years ago
We need bug 918389 for this, AFAIK, so that we have this in a separate field.
Depends on: 918389

Comment 4

3 years ago
comment 1
Flags: needinfo?(benjamin)
The abort message is in the AbortMessage field from bug 918389.

I think the format should be abort message first, then c++ signature. We'll probably need some truncation in order to get both:

Abort | AbortMessage[:80] | <C++Signature>
Flags: needinfo?(benjamin)
old and outdated - refile if this is still needed
Status: NEW → RESOLVED
Last Resolved: a year ago
Resolution: --- → WONTFIX
This is still valid as filed and rather important. Reopening.
Status: RESOLVED → REOPENED
Resolution: WONTFIX → ---
(Assignee)

Updated

a year ago
Assignee: lars → adrian
Priority: -- → P2
(Assignee)

Comment 8

a year ago
Created attachment 8738589 [details] [review]
Link to Github pull-request: https://github.com/mozilla/socorro/pull/3273

Here's my proposal for this. It strictly follows the pattern that Benjamin shared: Abort | <AbortMessage[0:80]> | <C signature>

Comment 9

a year ago
Commit pushed to master at https://github.com/mozilla/socorro

https://github.com/mozilla/socorro/commit/702ca6be292370575a993abedc26792fcc37b91d
Fixes bug 803779 - Added a rule for Abort signatures.

Added a new transform Rule to the processor to add the abort message to the signature of crashes that have an AbortMessage field. The format is "Abort | <AbortMessage[0:80]> | <C signature>".

Updated

a year ago
Status: REOPENED → RESOLVED
Last Resolved: a year agoa year ago
Resolution: --- → FIXED
(Reporter)

Comment 10

a year ago
So, looking at what this produces on stage: https://crash-stats.allizom.org/search/?product=Firefox&signature=%5EAbort&_facets=signature&_columns=date&_columns=signature&_columns=abort_message#facet-signature

It looks to me like since we crafted the bug and the spec, we added a string in [] at the front of the abort message that contains a number, which might be a process ID or so, which we don't want in a signature. When we are removing that, we can also remove the redundant start of the abort message, which is always the same.

I propose that before we add it to the signature, we remove /^\[.+\] ###!!! ABORT: / from the abort message.
(Assignee)

Comment 11

a year ago
Reverted for now, will do again with a better signature generation.
Status: RESOLVED → REOPENED
Resolution: FIXED → ---

Comment 12

a year ago
Commit pushed to master at https://github.com/mozilla/socorro

https://github.com/mozilla/socorro/commit/5e1b68de866ac053ea9dcf1bead3c3997b1de7dd
Revert "Fixes bug 803779 - Added a rule for Abort signatures."

This reverts commit 702ca6be292370575a993abedc26792fcc37b91d.

Updated

a year ago
Status: REOPENED → RESOLVED
Last Resolved: a year agoa year ago
Resolution: --- → FIXED
oops. When we reverted the git commit, it still had "fixes bug ..." in the commit message.
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
(Assignee)

Comment 14

a year ago
Created attachment 8755900 [details] [review]
Link to Github pull-request: https://github.com/mozilla/socorro/pull/3354

This new version does every thing the previous one did, but also removes the irrelevant part at the beginning of the abort message (no more ``[xxx] ###!!! ABORT: `` in the signature).
Attachment #8738589 - Attachment is obsolete: true

Comment 15

a year ago
Commits pushed to master at https://github.com/mozilla/socorro

https://github.com/mozilla/socorro/commit/3422c7dec391d5ef9908669f6e92939743153711
Fixes bug 803779 - Added a rule for Abort signatures.

Added a new transform Rule to the processor to add the abort message to the signature of crashes that have an AbortMessage field. The format is "Abort | <AbortMessage[0:80]> | <C signature>".

https://github.com/mozilla/socorro/commit/11ccbaffb63dd9f96fea7d3ad6dacfdc7ea89191
Bug 803779 - Remove irrelevant part of the abort message.

https://github.com/mozilla/socorro/commit/16c85391d108a661697b1faee444a69e4cd6304d
Merge pull request #3354 from adngdb/803779-abort-signatures-again

Fixes bug 803779 - Added a rule for Abort signatures.

Updated

a year ago
Status: REOPENED → RESOLVED
Last Resolved: a year agoa year ago
Resolution: --- → FIXED
I didn't realize that the file/line# information would be in the abort message. That makes it likely that signatures will change across release (since line numbers change).

So for example at https://crash-stats.allizom.org/report/index/1d578a00-ed21-45aa-b884-1e0862160525 could we cut the "file X, line Y" bit out of the AbortMessage so that it's just "Abort | unsafe destruction | mozalloc_abort | NS_DebugBreak | mozilla::plugins::PluginModuleChromeParent::~PluginModuleChromeParent" ?

That does leave an open question what to do about things like https://crash-stats.allizom.org/report/index/dedb245c-ed2d-4d6f-9982-932b52160525 which at that point don't have any abort message at all. Perhaps if it's empty, we should just do Abort | <C++sig>
Flags: needinfo?(benjamin)
(Assignee)

Comment 17

a year ago
This is on stage now. Does it look good? 

https://crash-stats.allizom.org/search/?product=Firefox&signature=%5EAbort&_facets=signature&_columns=date&_columns=signature&_columns=product&_columns=version&_columns=build_id&_columns=platform#crash-reports
Flags: needinfo?(benjamin)
(Assignee)

Comment 18

a year ago
OK, I am reverting my change once again because I can't work on improving it now. I'll finish this up next week hopefully. :)
Status: RESOLVED → REOPENED
Resolution: FIXED → ---

Comment 19

a year ago
Commits pushed to master at https://github.com/mozilla/socorro

https://github.com/mozilla/socorro/commit/84180573ad1c2c1b5a485acbea0b6aa6fcfb04af
Revert "Fixes bug 803779 - Added a rule for Abort signatures."

https://github.com/mozilla/socorro/commit/c31747de951cbb349aedbfecfa9207da75074928
Merge pull request #3355 from mozilla/revert-3354-803779-abort-signatures-again

Revert "Fixes bug 803779 - Added a rule for Abort signatures."

Updated

a year ago
Status: REOPENED → RESOLVED
Last Resolved: a year agoa year ago
Resolution: --- → FIXED
(Assignee)

Updated

a year ago
Status: RESOLVED → REOPENED
Resolution: FIXED → ---

Comment 20

a year ago
Commit pushed to master at https://github.com/mozilla/socorro

https://github.com/mozilla/socorro/commit/1bffd9149d69dc890ecce844f4c009af260114bf
Fixes bug 803779 - Added a rule for Abort signatures. (#3367)

* Fixes bug 803779 - Added a rule for Abort signatures.

Added a new transform Rule to the processor to add the abort message to the signature of crashes that have an AbortMessage field. The format is "Abort | <AbortMessage[0:80]> | <C signature>".

* Bug 803779 - Remove irrelevant part of the abort message.

* Bug 803779 - Removed file name and line number from abort message in signatures.

Updated

a year ago
Status: REOPENED → RESOLVED
Last Resolved: a year agoa year ago
Resolution: --- → FIXED
(Assignee)

Comment 21

a year ago
Hopefully this is satisfying now! :)

https://crash-stats.allizom.org/search/?product=Firefox&signature=%5EAbort&_facets=signature&_columns=date&_columns=signature&_columns=product&_columns=version&_columns=build_id&_columns=platform#crash-reports
Flags: needinfo?(benjamin)
LGTM!
Flags: needinfo?(benjamin)
(Assignee)

Comment 23

a year ago
\o/
Status: RESOLVED → VERIFIED
(Reporter)

Comment 24

a year ago
Just as a question (I'm not someone to decide anything here any more), should this have a signature starting in "Abort" or in "OOM"?

https://crash-stats.allizom.org/signature/?product=Firefox&signature=Abort%20%7C%20OOM%20%7C%20mozalloc_abort%20%7C%20NS_DebugBreak%20%7C%20nsSameProcessAsyncMessageBase%3A%3AnsSameProcessAsyncMessageBase
You need to log in before you can comment on or make changes to this bug.