bug-1926077: remove enclosing square bracket(s) from module name in signature generation

[biancadanforth]

Because:

• Some module names have extra square brackets, in particular one at the end (bug-1926561).
• This causes Crash Stats report Bugzilla tab not to list related bugs.

This commit:

• Removes enclosing square bracket(s) from module names in stack frames.

I tested this locally by processing the crash mentioned in the bug. The extra closing bracket gets removed as desired:

Before

Bugzilla tab does not list the expected bug, Bug-1923773:

After

I also verified that the crash bug that wasn't originally showing up in the reports view Bugzilla tab is now showing up:

[willkg]

This looks good--sorry for misleading you as to where I thought the fix should go.

I think I would switch the code to use strip() because it'll remove any number of prefix [ and postfix ], but there's no evidence signature generation would encounter cases with multiple brackets so it's not important.

One thing I do after making signature generation changes is to test them. There's documentation here:

https://socorro.readthedocs.io/en/latest/signaturegeneration.html#signaturegeneration-chapter-module

app@socorro:/app$ socorro-cmd signature 4c36561c-ef1e-4a90-aea7-c79b60241020
Crash id: 4c36561c-ef1e-4a90-aea7-c79b60241020
Original: libart.so | base.art]
New:      libart.so | base.art
Same?:    False

We can go further and see how it affects signatures for other crash reports. I'm using supersearch (part of crashstats-tools) outside of the container:

> supersearch --signature='~]' --_columns=uuid --_columns=signature --num=10
82663a46-f539-4314-985d-3da090241025	objc_msgSend | -[_NSTrackingAreaAKManager _updateActiveTrackingAreasForWindowLocation:modifierFlags:]
1cdaea34-4c2e-48ef-94ff-257740241025	mozilla::detail::InvalidArrayIndex_CRASH | nsTArray_Impl<T>::ElementAt | nsTArray_Impl<T>::operator[] | ClusterIterator::ClusterIterator
6cf0d698-7704-47f6-b47a-bca0d0241025	-[__NSSetM addObject:]
7f052acf-d101-4163-8ec3-0cca20241025	boot-framework.art]
80cd4a7c-cc4d-4e6d-9426-39fe70241025	objc_msgSend | -[_NSTrackingAreaAKManager _updateActiveTrackingAreasForWindowLocation:modifierFlags:]
cd454d92-745c-4972-8c22-8770a0241025	mozilla::detail::InvalidArrayIndex_CRASH | nsTArray_Impl<T>::ElementAt | nsTArray_Impl<T>::operator[] | ClusterIterator::ClusterIterator
b511bfd1-23e3-4f41-a945-671e50241025	libc.so | libart.so | boot-framework.art]
75b7616b-f24a-4dcd-b537-ce7fc0241025	<usize as core::slice::index::SliceIndex<[T]>>::index
7697fd08-5f82-42b6-a78d-db6830241025	mozilla::detail::InvalidArrayIndex_CRASH | nsTArray_Impl<T>::ElementAt | nsTArray_Impl<T>::operator[] | mozilla::safebrowsing::HashStore::ReadAddCompletes
e0c6aafc-85fc-4b88-98fc-8264e0241025	objc_msgSend | -[_NSTrackingAreaAKManager _updateActiveTrackingAreasForWindowLocation:modifierFlags:]

There are other variations of this bug with other module names. Looks like ] is used in function symbols. I forget what the [ ... ] means in that context.

We can drop the signature column and create a crashids.txt file and then run signature generation on those crash ids:

Outside the container:

> supersearch --signature='~]' --_columns=uuid --num=10 > crashids.txt

Inside the container:

> cat crashids.txt | socorro signature
app@socorro:/app$ cat crashids.txt | socorro-cmd signature
Crash id: 82663a46-f539-4314-985d-3da090241025
Original: objc_msgSend | -[_NSTrackingAreaAKManager _updateActiveTrackingAreasForWindowLocation:modifierFlags:]
New:      objc_msgSend | -[_NSTrackingAreaAKManager _updateActiveTrackingAreasForWindowLocation:modifierFlags:]
Same?:    True

Crash id: 1cdaea34-4c2e-48ef-94ff-257740241025
Original: mozilla::detail::InvalidArrayIndex_CRASH | nsTArray_Impl<T>::ElementAt | nsTArray_Impl<T>::operator[] | ClusterIterator::ClusterIterator
New:      mozilla::detail::InvalidArrayIndex_CRASH | nsTArray_Impl<T>::ElementAt | nsTArray_Impl<T>::operator[] | ClusterIterator::ClusterIterator
Same?:    True

Crash id: 6cf0d698-7704-47f6-b47a-bca0d0241025
Original: -[__NSSetM addObject:]
New:      -[__NSSetM addObject:]
Same?:    True

Crash id: 7f052acf-d101-4163-8ec3-0cca20241025
Original: boot-framework.art]
New:      boot-framework.art
Same?:    False

Crash id: 80cd4a7c-cc4d-4e6d-9426-39fe70241025
Original: objc_msgSend | -[_NSTrackingAreaAKManager _updateActiveTrackingAreasForWindowLocation:modifierFlags:]
New:      objc_msgSend | -[_NSTrackingAreaAKManager _updateActiveTrackingAreasForWindowLocation:modifierFlags:]
Same?:    True

Crash id: cd454d92-745c-4972-8c22-8770a0241025
Original: mozilla::detail::InvalidArrayIndex_CRASH | nsTArray_Impl<T>::ElementAt | nsTArray_Impl<T>::operator[] | ClusterIterator::ClusterIterator
New:      mozilla::detail::InvalidArrayIndex_CRASH | nsTArray_Impl<T>::ElementAt | nsTArray_Impl<T>::operator[] | ClusterIterator::ClusterIterator
Same?:    True

Crash id: b511bfd1-23e3-4f41-a945-671e50241025
Original: libc.so | libart.so | boot-framework.art]
New:      libc.so | libart.so | boot-framework.art
Same?:    False

Crash id: 75b7616b-f24a-4dcd-b537-ce7fc0241025
Original: <usize as core::slice::index::SliceIndex<[T]>>::index
New:      <usize as core::slice::index::SliceIndex<[T]>>::index
Same?:    True

Crash id: 7697fd08-5f82-42b6-a78d-db6830241025
Original: mozilla::detail::InvalidArrayIndex_CRASH | nsTArray_Impl<T>::ElementAt | nsTArray_Impl<T>::operator[] | mozilla::safebrowsing::HashStore::ReadAddCompletes
New:      mozilla::detail::InvalidArrayIndex_CRASH | nsTArray_Impl<T>::ElementAt | nsTArray_Impl<T>::operator[] | mozilla::safebrowsing::HashStore::ReadAddCompletes
Same?:    True

Crash id: e0c6aafc-85fc-4b88-98fc-8264e0241025
Original: objc_msgSend | -[_NSTrackingAreaAKManager _updateActiveTrackingAreasForWindowLocation:modifierFlags:]
New:      objc_msgSend | -[_NSTrackingAreaAKManager _updateActiveTrackingAreasForWindowLocation:modifierFlags:]
Same?:    True

From that we see a lot of signatures with a ], but everything looks good.

Sometimes if we're looking at a lot of signature generation output, it's better to isolate it to just what's changed:

app@socorro:/app$ cat crashids.txt | socorro-cmd signature --different-only

Crash id: 7f052acf-d101-4163-8ec3-0cca20241025
Original: boot-framework.art]
New:      boot-framework.art
Same?:    False

Crash id: b511bfd1-23e3-4f41-a945-671e50241025
Original: libc.so | libart.so | boot-framework.art]
New:      libc.so | libart.so | boot-framework.art
Same?:    False

The bug association workflow is complicated. The Django table keeps track of which bugs had which cf_crash_signature values and is updated by the bugassociations cron job. It looks at bugs based on when the bug was last updated. Since the bug wasn't updated, the data in the table wouldn't change. The test you did probably doesn't tell us anything. When signatures change, someone will update the cf_crash_signature field in the bug to add/remove relevant signatures. The next time bugassociations runs, it'll adjust the Django table accordingly and that'll be reflected in the "Bugzilla" tab.

Hope that's helpful context!

[biancadanforth]

The bug association workflow is complicated. The Django table keeps track of which bugs had which cf_crash_signature values and is updated by the bugassociations cron job. It looks at bugs based on when the bug was last updated. Since the bug wasn't updated, the data in the table wouldn't change. The test you did probably doesn't tell us anything. When signatures change, someone will update the cf_crash_signature field in the bug to add/remove relevant signatures. The next time bugassociations runs, it'll adjust the Django table accordingly and that'll be reflected in the "Bugzilla" tab.

IIUC in the case of Bug-1923773, cpeterson manually updated the cf_crash_signature field three days ago to include the signature without the extra bracket, [@ libart.so | base.art]. Locally, I ran the bugassociations Django command for updates in the last 4 days which would include this field update. Wouldn't that mean that the prod table would correctly associate this bug with this updated signature once it's deployed to prod?

[willkg]

Maybe? It depends on a lot of things that aren't explicitly stated here. My point was more that it'll figure itself out and that we don't need to test this as part of signature generation change workflow.