1740397 - fix CrashingThreadRule to set crashing_thread [may 2022]

Status: RESOLVED FIXED

(Socorro :: General, task, P3)

Description

[Will Kahn-Greene [:willkg] ET needinfo? me]

There's a CrashingThreadRule in the processor that sets crashedThread in the processed crash. However, that field isn't used anywhere.

Instead, every place that goes to find the crashing thread index pulls it out of json_dump.crash_info.crashing_thread.

That's kind of silly. We should have a rule that normalizes the value and sets it in the processed crash and then retrieve that field everywhere.

Comment 1

[Will Kahn-Greene [:willkg] ET needinfo? me]

Attachment: pr 5893: bug 1740397: rework CrashingThreadInfoRule

Comment 2

[Will Kahn-Greene [:willkg] ET needinfo? me]

willkg merged PR #5893: "bug 1740397: rework CrashingThreadInfoRule" in b5a8b61.

That covers the processor side of this. We also use crashing_thread a ton in the webapp, but that uses the processed data and so I want to wait 6 months after deploying this to prod so that the data that's missing the field expires.

Comment 3

[Will Kahn-Greene [:willkg] ET needinfo? me]

I deployed this to prod in bug #1744108.

Comment 4

[Will Kahn-Greene [:willkg] ET needinfo? me]

Setting this to get revisited in May and unassigning myself.

Comment 5

[Calixte Denizet (:calixte)]

I was using crashedThread... (see https://github.com/mozilla/crash-clouseau/blob/e10d33d05a4d4bf900074047b172e1b128559a88/crashclouseau/inspector.py#L99).
Is it safe to just change into crashing_thread in my code ?
And fyi, there is still an occurrence: https://github.com/mozilla-services/socorro/blob/46e29902d6ad7c9a118dbf8a64d4deede1db049a/webapp-django/crashstats/crashstats/models.py#L627.

Comment 6

[Will Kahn-Greene [:willkg] ET needinfo? me]

Whoops! It's hard to know if anyone uses things, but I should have notified stability and crash-reporting-wg lists. I'll do that today. Sorry about that.

If crashclouseau/inspector is just looking at new crash data, then you could switch the code to:

        N = data.get('crashing_thread')

If it's not just looking at new crash data or you don't know, I would do something like this:

        # FIXME: may 2022, drop the crashedThread part
        N = data.get('crashing_thread', data.get('crashedThread'))

That'll look for the new field and if it's not there, try the old field, and if that's not there, return None like it did before.

Regarding your FYI--yes, it's still used in the webapp because there's processed crash data that doesn't have the new field, yet. The rest of the usage of the old field will get removed in May 2022.

Comment 7

[Will Kahn-Greene [:willkg] ET needinfo? me]

Attachment: pr 6125: bug 1740397: switch webapp to pull crashing_thread from processed_crash

Comment 8

[Will Kahn-Greene [:willkg] ET needinfo? me]

willkg merged PR #6125: "bug 1740397: switch webapp to pull crashing_thread from processed_crash" in 293e285.

This updates the webapp code to use processed crash crashing_thread which is normalized and validated.

Comment 9

[Will Kahn-Greene [:willkg] ET needinfo? me]

I deployed this in bug #1782791 just now. Marking as FIXED.