817545 - TBPL should fall back to searching for the top frame for crashes, if no other bug suggestions found

Status: RESOLVED FIXED

(Tree Management Graveyard :: TBPL, defect)

Description

[Ed Morley [:emorley]]

Now that bug 813650 means we have the top frame of crashes in the annotated summary, eg:
PROCESS-CRASH | Shutdown | application crashed [@ nssCertificate_Destroy]
 PROCESS-CRASH | /home/cltbld/talos-slave/test/build/xpcshell/tests/toolkit/components/places/tests/queries/test_tags.js | application crashed [@ libc-2.11.so + 0x80f9c]

...we can use it to supplement the bug suggestions.

The main cases that cause irritation are:
1) Shutdown crashes, since AnnotatedSummaryGenerator.php's getBugsForTestFailure() deliberately blacklists "Shutdown" as a search term, since there are too many false positives.
2) GC type crashes that occur in a vast number of tests, so playing the clone bug and add more test names game becomes impractical.

As such, we should:
* Try to find bug suggestions as normal
* If that returns 0 suggestions (all the time on Shutdown crashes; likely for GC crashes as long as that test doesn't have unrelated bugs filed for it), call getBugsForTestFailure() again, but this time with searchTerm as the top frame.

Comment 1

[Ed Morley [:emorley]]

Note: An alternative to the plan in comment 0, would be to check if the failure was a crash, and do a custom bzapi search using searchTerm as (testname OR topFrame). However I imagine for some 'crashes', where there is a moz_abort (or whatever) on the top frame, this would result in many false positives & tbh it's not worth the effort for small number of additional failures this would let us catch (GC crashes whose crashing test has existing bugs filed).

Comment 2

[Ed Morley [:emorley]]

Attachment: Part 1: Move search term sanitisation into getBugsForTestFailure()

* We already do rudimentary search term validation/blacklisting in getBugsForTestFailure(), so it makes more sense there.
* Avoids duplication in part 2.
* Removes the "if ($searchTerm === '')" check, since redundant with "if (strlen($searchTerm) < 5)".

Comment 3

[(not currently active) Ted Mielczarek]

Just a few notes:
a) The top frame isn't going to match the crash signature for crash bugs all the time (Socorro has an algorithm for generating signatures.)
b) There's a crash signature field in bugzilla now, but I don't know what it's called offhand.

Comment 4

[Ed Morley [:emorley]]

(In reply to Ted Mielczarek [:ted.mielczarek] from comment #3)
> a) The top frame isn't going to match the crash signature for crash bugs all
> the time (Socorro has an algorithm for generating signatures.)

Yeah I agree, but for bugs we've specifically filed for intermittent failures, we use [@ topFrame-maybe-irrelevant | nextFrame | ...] , which will still match most.

> b) There's a crash signature field in bugzilla now, but I don't know what
> it's called offhand.

Yes, unfortunately it's not always populated and uses a slightly different format from that output in minidump stackwalk. Whilst we could adjust the format of the topFrame output by automationutils.py, it doesn't seem worth it, if it means we'll catch less bugs.

The primary motivation for this bug at the moment is bug 761987 (which has been driving myself and other sheriffs nuts), which the simpler solution here will catch :-)

Long-term when we re-write TBPL, I agree it would be good to use Socorro's libraries & search specifically for the crash field (after mass-fixing missing metadata).

Comment 5

[Ed Morley [:emorley]]

Attachment: Part 2: Fallback if no suggestions and is a crash

Tested in Vagrant with php error logging enabled; seems to work well :-)

Comment 6

[Ed Morley [:emorley]]

Thank you for the reviews :-)

https://hg.mozilla.org/users/mstange_themasta.com/tinderboxpushlog/rev/058184d5f62f
https://hg.mozilla.org/users/mstange_themasta.com/tinderboxpushlog/rev/ecb321ede67d

Comment 7

[Ed Morley [:emorley]]

In production \o/

Comment 8

[Ed Morley [:emorley]]

(In reply to Ed Morley [UTC+0; email:edmorley@moco] from comment #7)
> In production \o/

(And with great glee have starred several of bug 761987 already. Now to just get some traction on that bug...)