Closed Bug 1275430 Opened 4 years ago Closed 4 years ago

Add telemetry and logging to record content process failures to start

Categories

(Core :: DOM: Content Processes, defect, P2)

defect

Tracking

()

RESOLVED FIXED
mozilla49
Tracking Status
firefox49 --- fixed

People

(Reporter: haik, Assigned: haik)

References

Details

(Whiteboard: btpp-active)

Attachments

(2 files, 2 obsolete files)

+++ This bug was initially created as a clone of Bug #1165945 +++

Splitting the telemetry and logging off from 1165945 so that 1165945 can be used for improving the user experience when a content process fails to launch. When I commented out the code to start a content process to test the telemetry code, Firefox hung at launch and no UI ever was displayed.

This bug will just be used for the added telemetry and logging.
No longer depends on: 1165945
See Also: → 1165945
bsmedberg provided data-collection review on 1165945.
Assignee: nobody → haftandilian
Attachment #8756130 - Flags: review?(wmccloskey)
Comment on attachment 8756130 [details] [diff] [review]
Adds a telemetry probe and console logging to record subprocess launch failures

Review of attachment 8756130 [details] [diff] [review]:
-----------------------------------------------------------------

::: ipc/glue/GeckoChildProcessHost.cpp
@@ +567,5 @@
> +    return true;
> +  } else {
> +    CHROMIUM_LOG(ERROR) << "Failed to launch " <<
> +      XRE_ChildProcessTypeToString(mProcessType) << " subprocess";
> +    Telemetry::Accumulate(Telemetry::SUBPROCESS_LAUNCH_FAILURE, mProcessType);

In other cases we've used keyed count histograms, which can be a little clearer. Here's an example:
http://searchfox.org/mozilla-central/rev/c416322cd4079ab9ef6b30203fedae11cf39fdc5/dom/ipc/ContentParent.cpp#2092

Rather than using "content", you should be able to use the result of XRE_ChildProcessTypeToString. You probably will have to use nsDependentCString as follows:
http://searchfox.org/mozilla-central/rev/c416322cd4079ab9ef6b30203fedae11cf39fdc5/ipc/glue/MessageChannel.cpp#761
Attachment #8756130 - Flags: review?(wmccloskey)
Whiteboard: btpp-active
Changed the histogram type per billm's recommendation.

Should values persist across restarts? In my testing, I found they don't.

Will add about:telemetry screenshot showing the new histograms.
Attachment #8756130 - Attachment is obsolete: true
Attachment #8758802 - Flags: review?(wmccloskey)
Attached image Histogram Screenshot
(In reply to Haik Aftandilian [:haik] from comment #3)
> Created attachment 8758802 [details] [diff] [review]
> Should values persist across restarts? In my testing, I found they don't.

I wasn't looking in the right place. The values are getting saved in the archived data sets.
Comment on attachment 8758802 [details] [diff] [review]
Adds a telemetry probe and console logging to record subprocess launch failures

Review of attachment 8758802 [details] [diff] [review]:
-----------------------------------------------------------------

Thanks.
Attachment #8758802 - Flags: review?(wmccloskey) → review+
Keywords: checkin-needed
Pushed by kcambridge@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/32a8d48c26ee
Add telemetry and logging to record content process failures to start; r=billm
Keywords: checkin-needed
https://hg.mozilla.org/mozilla-central/rev/32a8d48c26ee
Status: NEW → RESOLVED
Closed: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla49
You need to log in before you can comment on or make changes to this bug.