1910438 - Stack traces are missing from almost all Glean crash pings from the windows crash reporter client

Status: RESOLVED FIXED

(Toolkit :: Crash Reporting, defect)

Description

[Alex Franchuk [:afranchuk]]

Attachment: Crash pings with stack traces

As shown in the plot, the orange line (Windows Glean crash pings with stack traces) is way, way lower than expected (though oddly also non-zero).

Comment 1

[Alex Franchuk [:afranchuk]]

There's no logic specific to the Glean implementation that would result in this behavior, as any error to convert a value results in the entire ping being discarded (as opposed to only the field). However, if the stack traces were simply missing, I would expect legacy Telemetry to look the same. At the moment, my theory is that Glean itself may be internally discarding the field for some reason.

Comment 2

[Alex Franchuk [:afranchuk]]

I have confirmed in the logs that we encounter:

[WARN glean_core::error_recording] crash.stack_traces: Value did not match predefined schema

Comment 3

[Alex Franchuk [:afranchuk]]

This is due to windows having an extra unloaded_modules field. I will change my approach to copy all data to avoid extra fields breaking the expected schema.

Comment 4

[Alex Franchuk [:afranchuk]]

Attachment: Bug 1910438 pt1 - Improve the population of glean object metrics r=gsvelto

This ensures that we always match the metric schema.

Comment 5

[Alex Franchuk [:afranchuk]]

Attachment: Bug 1910438 pt2 - Improve test behavior r=gsvelto

Having multiple glean tests exposed some issues and races (since glean
is a global value, it must be tested serially). This changes mock data
to be an Arc as we can't depend on glean deterministically accessing
the data we pass to it, unfortunately.

Comment 6

[Alex Franchuk [:afranchuk]]

Attachment: Bug 1910438 pt3 - Matching changes to the CrashManager stack traces impl r=gsvelto

Comment 7

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/7ef5ebb06679
pt1 - Improve the population of glean object metrics r=gsvelto
https://hg.mozilla.org/integration/autoland/rev/5a587f1aa35d
pt2 - Improve test behavior r=gsvelto
https://hg.mozilla.org/integration/autoland/rev/00f2c12f3a27
pt3 - Matching changes to the CrashManager stack traces impl r=gsvelto

Comment 8

[Cristina Horotan [:chorotan]]

https://hg.mozilla.org/mozilla-central/rev/7ef5ebb06679
https://hg.mozilla.org/mozilla-central/rev/5a587f1aa35d
https://hg.mozilla.org/mozilla-central/rev/00f2c12f3a27

Comment 9

[BugBot [:suhaib / :marco/ :calixte]]

The patch landed in nightly and beta is affected.
:afranchuk, is this bug important enough to require an uplift?

• If yes, please nominate the patch for beta approval.
• If no, please set status-firefox130 to wontfix.

For more information, please visit BugBot documentation.

Comment 10

[Alex Franchuk [:afranchuk]]

Attachment: Bug 1910438 pt1 - Improve the population of glean object metrics

This ensures that we always match the metric schema.

Original Revision: https://phabricator.services.mozilla.com/D218215

Comment 11

[Alex Franchuk [:afranchuk]]

Attachment: Bug 1910438 pt2 - Improve test behavior

Having multiple glean tests exposed some issues and races (since glean
is a global value, it must be tested serially). This changes mock data
to be an Arc as we can't depend on glean deterministically accessing
the data we pass to it, unfortunately.

Original Revision: https://phabricator.services.mozilla.com/D218216

Comment 12

[Alex Franchuk [:afranchuk]]

Attachment: Bug 1910438 pt3 - Matching changes to the CrashManager stack traces impl

Original Revision: https://phabricator.services.mozilla.com/D218217

Comment 13

[Phabricator Automation]

beta Uplift Approval Request

• User impact if declined: No stack traces in most windows crash reports, and potentially other platforms (more difficult for us to fix user bugs)
• Code covered by automated testing: yes
• Fix verified in Nightly: yes
• Needs manual QE test: no
• Steps to reproduce for manual QE testing: N/A
• Risk associated with taking this patch: Low
• Explanation of risk level: The changes fix a bug in stack trace glean metric recording. They do not affect any other code.
• String changes made/needed: None
• Is Android affected?: yes

Comment 14

[Pulsebot]

https://hg.mozilla.org/releases/mozilla-beta/rev/ee8dad006dc3
https://hg.mozilla.org/releases/mozilla-beta/rev/5c97fe492bbe
https://hg.mozilla.org/releases/mozilla-beta/rev/a89ea5629027