Last Comment Bug 803779 - Put abort messages into the signature
: Put abort messages into the signature
Status: VERIFIED FIXED
:
Product: Socorro
Classification: Server Software
Component: Backend (show other bugs)
: unspecified
: All All
: P2 normal (vote)
: ---
Assigned To: Adrian Gaudebert [:adrian]
:
Mentors:
Depends on: 918389
Blocks:
  Show dependency treegraph
 
Reported: 2012-10-19 20:48 PDT by Robert Kaiser
Modified: 2016-06-22 04:46 PDT (History)
6 users (show)
See Also:
QA Whiteboard:
Iteration: ---
Points: ---


Attachments
Link to Github pull-request: https://github.com/mozilla/socorro/pull/3273 (44 bytes, text/x-github-pull-request)
2016-04-06 08:30 PDT, Adrian Gaudebert [:adrian]
no flags Details | Review | Splinter Review
Link to Github pull-request: https://github.com/mozilla/socorro/pull/3354 (44 bytes, text/x-github-pull-request)
2016-05-24 15:46 PDT, Adrian Gaudebert [:adrian]
no flags Details | Review | Splinter Review

Description Robert Kaiser 2012-10-19 20:48:19 PDT
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?
Comment 1 Benjamin Smedberg AWAY UNTIL 2-AUG-2016 [:bsmedberg] 2012-10-22 09:10:36 PDT
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.
Comment 2 K Lars Lohn [:lars] [:klohn] 2013-10-24 12:54:57 PDT
once you can identify the source of this "abort message", I can get going on this...
Comment 3 Robert Kaiser 2013-10-24 14:38:23 PDT
We need bug 918389 for this, AFAIK, so that we have this in a separate field.
Comment 4 Chris Lonnen :lonnen 2014-06-27 09:37:58 PDT
comment 1
Comment 5 Benjamin Smedberg AWAY UNTIL 2-AUG-2016 [:bsmedberg] 2014-06-27 09:59:31 PDT
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>
Comment 6 K Lars Lohn [:lars] [:klohn] 2016-02-29 08:51:48 PST
old and outdated - refile if this is still needed
Comment 7 Benjamin Smedberg AWAY UNTIL 2-AUG-2016 [:bsmedberg] 2016-02-29 10:14:20 PST
This is still valid as filed and rather important. Reopening.
Comment 8 Adrian Gaudebert [:adrian] 2016-04-06 08:30:38 PDT
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 [github robot] 2016-04-06 12:53:44 PDT
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>".
Comment 10 Robert Kaiser 2016-04-07 04:33:06 PDT
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.
Comment 11 Adrian Gaudebert [:adrian] 2016-04-07 04:37:24 PDT
Reverted for now, will do again with a better signature generation.
Comment 12 [github robot] 2016-04-07 05:23:39 PDT
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.
Comment 13 Peter Bengtsson [:peterbe] 2016-04-07 05:49:41 PDT
oops. When we reverted the git commit, it still had "fixes bug ..." in the commit message.
Comment 14 Adrian Gaudebert [:adrian] 2016-05-24 15:46:18 PDT
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).
Comment 15 [github robot] 2016-05-24 18:11:28 PDT
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.
Comment 16 Benjamin Smedberg AWAY UNTIL 2-AUG-2016 [:bsmedberg] 2016-05-25 08:17:17 PDT
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>
Comment 18 Adrian Gaudebert [:adrian] 2016-05-25 10:58:04 PDT
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. :)
Comment 19 [github robot] 2016-05-25 11:16:36 PDT
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."
Comment 20 [github robot] 2016-06-15 12:22:27 PDT
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.
Comment 22 Benjamin Smedberg AWAY UNTIL 2-AUG-2016 [:bsmedberg] 2016-06-21 12:26:59 PDT
LGTM!
Comment 23 Adrian Gaudebert [:adrian] 2016-06-22 01:36:49 PDT
\o/
Comment 24 Robert Kaiser 2016-06-22 04:46:07 PDT
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

Note You need to log in before you can comment on or make changes to this bug.