803779 - Put abort messages into the signature

Status: VERIFIED FIXED

(Socorro :: Backend, task, P2)

Description

[Robert Kaiser]

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]

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]]

once you can identify the source of this "abort message", I can get going on this...

Comment 3

[Robert Kaiser]

We need bug 918389 for this, AFAIK, so that we have this in a separate field.

Comment 4

[Lonnen :lonnen]

comment 1

Comment 5

[Benjamin Smedberg]

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]]

old and outdated - refile if this is still needed

Comment 7

[Benjamin Smedberg]

This is still valid as filed and rather important. Reopening.

Comment 8

[[DEACTIVATED] Adrian Gaudebert]

Attachment: 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]]

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]

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

[[DEACTIVATED] Adrian Gaudebert]

Reverted for now, will do again with a better signature generation.

Comment 12

[[github robot]]

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]]

oops. When we reverted the git commit, it still had "fixes bug ..." in the commit message.

Comment 14

[[DEACTIVATED] Adrian Gaudebert]

Attachment: 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]]

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]

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 17

[[DEACTIVATED] Adrian Gaudebert]

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

Comment 18

[[DEACTIVATED] Adrian Gaudebert]

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]]

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]]

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 21

[[DEACTIVATED] Adrian Gaudebert]

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

Comment 22

[Benjamin Smedberg]

LGTM!

Comment 23

[[DEACTIVATED] Adrian Gaudebert]

\o/

Comment 24

[Robert Kaiser]

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