Closed Bug 1274345 Opened 8 years ago Closed 8 years ago

Add support for skipping a dll in the signature

Categories

(Socorro :: Backend, task)

Product:
Socorro
Component:
Backend
Type:
task
Priority:
Not set
Severity:
normal

Tracking

(Not tracked)

Status:
RESOLVED FIXED

People

(Reporter: jrmuizel, Assigned: adrian)

References

Details

Attachments

(2 files)

Link to Github pull-request: https://github.com/mozilla/socorro/pull/3363
44 bytes, text/x-github-pull-request
Details | Review
Link to Github pull-request: https://github.com/mozilla/socorro/pull/3374
44 bytes, text/x-github-pull-request
Details | Review 
For example we'd like both of these reports to have the same signature:
https://crash-stats.mozilla.com/report/index/2f6372d4-299e-474b-a596-626ef2160513
https://crash-stats.mozilla.com/report/index/67d0b018-db42-4732-b43c-6fb8f2160513

I think the ideal signature for these reports would be:
'igdumdim32.dll | CreateDecodeDeviceLH'
It is not possible with our current algorithm to do what you're asking. However, we could skip the `igdumdim32.dll` frames entirely, and have the signature of the first crash report become `CreateDecodeDeviceLH`. Would that work for you? 

For the second report you linked to, the first frame is @0x0 which is something I think we want to keep in signatures. So what I suggested won't work, the signature would stay the same. We could either skip `@0x0` in signatures, and then it would work as the first report, or we would have to change our algorithm to be able to remove frames after we have started accumulating signatures (note, our algorithm is explained here: https://github.com/mozilla/socorro/tree/master/socorro/siglists#signature-generation-algorithm ). 

But I'm wondering, isn't the actual problem that we have no symbols for that file?
(In reply to Adrian Gaudebert [:adrian] from comment #1)
> It is not possible with our current algorithm to do what you're asking.
> However, we could skip the `igdumdim32.dll` frames entirely, and have the
> signature of the first crash report become `CreateDecodeDeviceLH`. Would
> that work for you? 

Nope, we need the igdumdim32.dll in the signature still.

> For the second report you linked to, the first frame is @0x0 which is
> something I think we want to keep in signatures. So what I suggested won't
> work, the signature would stay the same. We could either skip `@0x0` in
> signatures, and then it would work as the first report, or we would have to
> change our algorithm to be able to remove frames after we have started
> accumulating signatures (note, our algorithm is explained here:
> https://github.com/mozilla/socorro/tree/master/socorro/siglists#signature-
> generation-algorithm ). 
> 
> But I'm wondering, isn't the actual problem that we have no symbols for that
> file?

We don't have access to symbols for that file (and graphics drivers in general), so we need something that will work without symbols.
OK, so doing that will require changing our algorithm. I'm thinking we could merge all signatures missing symbols (so things like *.dll*) into just one fragment of the final signature. That means for your two examples above you would get ``igdumdim32.dll | CreateDecodeDeviceLH`` and ``@0x0 | igdumdim32.dll | CreateDecodeDeviceLH`` as signatures. 

Proposal: 

In our signature generation algorithm, merge all consecutive frames with no symbols and the same module name into just one frame using that module name. 

Example:

Signature for this crash: https://crash-stats.mozilla.com/report/index/67d0b018-db42-4732-b43c-6fb8f2160513 would become: ``@0x0 | igdumdim32.dll`` (or ``@0x0 | igdumdim32.dll | CreateDecodeDeviceLH`` if we add ``igdumdim32.dll`` to our prefix list). 

Should that apply to all cases, or just a defined list of modules like ``igdumdim32.dll``?
(In reply to Adrian Gaudebert [:adrian] from comment #3)
> OK, so doing that will require changing our algorithm. I'm thinking we could
> merge all signatures missing symbols (so things like *.dll*) into just one
> fragment of the final signature. That means for your two examples above you
> would get ``igdumdim32.dll | CreateDecodeDeviceLH`` and ``@0x0 |
> igdumdim32.dll | CreateDecodeDeviceLH`` as signatures. 

That sounds good to me.


> Should that apply to all cases, or just a defined list of modules like
> ``igdumdim32.dll``?

I think it would be good to start with a defined list of modules. If it works well we can expand it to everything.
Benjamin, what do you think about that plan? Anything I should be cautious about?
Flags: needinfo?(benjamin)
Assignee: nobody → adrian
If we're talking about the plan from comment 3, I think that makes sense, but we should prototype and test carefully before deploying (and notify dev-platform).

I expect this will work best for all modules without symbols, but you're welcome to do the safe whitelist approach first.
Flags: needinfo?(benjamin)
Any idea approximately when we can expect this change?
Flags: needinfo?(adrian)
Here's an example where this would have made a big difference - bug 1277626.  Instead of a crash with 454 reports, and an unrelated crash of 203 reports, we'd see a single crash with a total of 1665 reports.  That's have escalated it to our attention a lot sooner.
Starting to work on this now. Hopefully this can land in our staging site early next week.
Status: NEW → ASSIGNED
Flags: needinfo?(adrian)
Commit pushed to master at https://github.com/mozilla/socorro

https://github.com/mozilla/socorro/commit/f1b5a0aeed3c596a324076912ffa641276313bfd
Fixes bug 1274345 - Added support from triming signatures missing symbols. (#3363)

* Fixes bug 1274345 - Added support from triming signatures missing symbols.

* nits
Status: ASSIGNED → RESOLVED
Closed: 8 years ago
Resolution: --- → FIXED
(In reply to Adrian Gaudebert [:adrian] from comment #12)
> This is landed and working on stage! 
> 
> Should we add `igdumdim32.dll` to the prefix list, so that the next frame
> gets added to the signature?

Yes. I think that will be essential for this to work well.
It looks like this is working great on stage.

Can you add atiumdag.dll, atiumd6a.dll, aticfx32.dll, atiuxpag.dll, igd10umd32.dll, igdumd32.dll, atidxx32.dll, atiumdva.dll, nvoglnt.dll, nvwgf2um.dll, nvumdshim.dll, and nvd3dum.dll to the same same lists as igdumdim32.dll
and also atiu9pag.dll
Jeff, does this look good to you? Note that we have made it a lot easier to make this kind of changes yourself, see the documentation here: https://github.com/mozilla/socorro/tree/master/socorro/siglists
Looks good to me.
Commit pushed to master at https://github.com/mozilla/socorro

https://github.com/mozilla/socorro/commit/1a6ec0c5e28bd7606b21232b06ac9e1604ac5eef
Bug 1274345 - Added more DLLs to the siglists. (#3374)

r=willkg,jeff
When can we expect this to hit staging and master?
Flags: needinfo?(adrian)
Sadly, I'm very excited about this.
Commits pushed to master at https://github.com/mozilla/socorro

https://github.com/mozilla/socorro/commit/198d9a41ce39dcf9fa24fda9c3cbf9817d6888e7
Revert "Bug 1274345. Add some 64bit dlls to the siglists"

https://github.com/mozilla/socorro/commit/a92272ee56d0385f14d6984c03958b2c3ebb1414
Merge pull request #3386 from mozilla/revert-3383-master

Revert "Bug 1274345. Add some 64bit dlls to the siglists"
Commits pushed to master at https://github.com/mozilla/socorro

https://github.com/mozilla/socorro/commit/44169b69043c285547fe01d9a31b0186ca33acff
Revert "Revert "Bug 1274345. Add some 64bit dlls to the siglists""

https://github.com/mozilla/socorro/commit/6cc2a963a3232cee8cfc1e377e17300e90fa3d25
Merge pull request #3387 from mozilla/revert-3386-revert-3383-master

Revert "Revert "Bug 1274345. Add some 64bit dlls to the siglists""
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: