Add more telemetry probes for gathering data on the Simplify Page feature for Print Preview

RESOLVED FIXED in Firefox 49

Status

()

defect
RESOLVED FIXED
3 years ago
3 years ago

People

(Reporter: mconley, Assigned: thaua)

Tracking

unspecified
mozilla50
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox49 fixed, firefox50 fixed)

Details

Attachments

(2 attachments, 2 obsolete attachments)

I spoke to Ilana Segall from metrics, and laid out the questions that HP wants to investigate:

1) What percentage of the Firefox user population tends to print a document at least once per session?
2) What percentage of the Firefox user population enters print preview at least once per session?
3) What percentage of the Firefox user population prints a document from within the print preview UI?
4) What percentage of the Firefox user population enters print preview and simplifies their print preview at least once per session?
5) How often is simplifying print preview not possible due to the way the document is constructed?
6) How often are simplified prints printed in comparison to normal prints?

We discussed these and decided that the following probes will be sufficient:

1) When the user initiates a print. This is a keyed histogram[1], with the following keys: printing from outside print preview, printing from within print preview, and printing a simplified document
2) The user enters print preview. This is a normal count histogram, and was landed in bug 1275570.
3) The user simplifies a print preview. This is a normal count histogram and was landed in bug 1275570.
4) The user enters print preview, but simplification is deemed not possible

So this bug is about adding (1) and (4).

[1]: https://developer.mozilla.org/en-US/docs/Mozilla/Performance/Adding_a_new_Telemetry_probe#Keyed_Histograms
We probably want a "before" snapshot without the Simplify Page feature enabled to do a comparison, so we'll probably want a second patch to land on Aurora with probe 2 and a modified version of probe 1 (without the printing a simplified document key, naturally).
Assignee: nobody → thaua
Posted patch WIP1: Add telemetry probes (obsolete) — Splinter Review
This patch adds the telemetry probes in order to keep tracking of printing data.
Attachment #8773065 - Flags: review?(mconley)
Comment on attachment 8773065 [details] [diff] [review]
WIP1: Add telemetry probes

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

Hey thaua,

The "PRINT_PREVIEW_SIMPLIFY_PAGE_UNAVAILABLE_COUNT" measurement looks good.

For the printing measurements, I actually think it might be safer to do the measurements down in browser-content.js, instead of setting up another message listener to wait for a result in the parent.

Inside the print function inside browser-content.js:
http://searchfox.org/mozilla-central/rev/868b17897f7a7fcd7f6f67fd8185a7370db46604/toolkit/content/browser-content.js#634

We already know if we're in simplified mode, since it's passed to us. I think we can also check to see if we're in print preview mode by checking the value of:

print.doingPrintPreview

After print (implementing the nsIWebBrowserPrint interface) is defined on line 646 in the above snapshot.

That should be enough to figure out what to add to the histograms.

::: toolkit/components/printing/content/printUtils.js
@@ +126,5 @@
> +        this.logKeyedTelemetry("PRINT_COUNT", "WITHOUT_PREVIEW");
> +      }
> +    };
> +
> +    mm.addMessageListener("Printing:Done", onDone);

In the event of an error, this Printing:Done message will never be fired from the content process, which means that the message listener will never be removed.
Attachment #8773065 - Flags: review?(mconley) → review-
This patch moves telemetry probes to the browser-content.js file.
Attachment #8773065 - Attachment is obsolete: true
Attachment #8773451 - Flags: review?(mconley)
Comment on attachment 8773451 [details] [diff] [review]
WIP2: Move telemetry probes to browser-content.js file

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

This looks great - I just have one small suggestion (see below).

Also requesting review from a data stewardship peer. liuche, does this look okay to gather?

::: toolkit/content/browser-content.js
@@ +648,5 @@
>        print.print(printSettings, null);
> +
> +      if (print.doingPrintPreview) {
> +        if (simplifiedMode) {
> +          Services.telemetry.getKeyedHistogramById("PRINT_COUNT").add("SIMPLIFIED");

Might as well pull this out, like:

let histogram = Services.telemetry.getKeyedHistogramById("PRINT_COUNT");
if (print.doingPrintPreview) {
  if (simplifiedMode) {
    histogram.add("SIMPLIFIED");
  } else {
    histogram.add("WITH_PREVIEW");
  }
} else {
  histogram.add("WITHOUT_PREVIEW");
}
Attachment #8773451 - Flags: review?(mconley)
Attachment #8773451 - Flags: review?(liuche)
Attachment #8773451 - Flags: review-
Small changes as mconley suggested.
Attachment #8773451 - Attachment is obsolete: true
Attachment #8773451 - Flags: review?(liuche)
Attachment #8773473 - Flags: review?(mconley)
Attachment #8773473 - Flags: review?(liuche)
Comment on attachment 8773473 [details] [diff] [review]
WIP3: Small changes

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

Looks good!
Attachment #8773473 - Flags: review?(mconley) → review+
Comment on attachment 8773451 [details] [diff] [review]
WIP2: Move telemetry probes to browser-content.js file

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

data-review r+

Looking at the previous patches, this follows the same pattern and details what specifically is collected. Figuring out when simplification fails and also counting the total number of prints seems necessary to figure out how users use printing. So this looks good to me from a data-review perspective!
Attachment #8773451 - Attachment is obsolete: false
Comment on attachment 8773473 [details] [diff] [review]
WIP3: Small changes

Okay, we got the data-review+. Clearing r?. Thanks liuche!
Attachment #8773473 - Flags: review?(liuche)
Attachment #8773451 - Attachment is obsolete: true
Adding checkin-needed:

Author: Thauã Silveira <thaua@hp.com>
Bug number: 1287587
Commit message: Bug 1287587 - Add Telemetry probes for user printing activities. r=mconley, data-review=liuche.
Keywords: checkin-needed
Pushed by ryanvm@gmail.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/400da2ae93a1
Add Telemetry probes for user printing activities. r=mconley, data-review=liuche
Keywords: checkin-needed
https://hg.mozilla.org/mozilla-central/rev/400da2ae93a1
Status: NEW → RESOLVED
Closed: 3 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla50
This patch adds the telemetry for printing usage to Aurora.
Attachment #8774454 - Flags: review?(mconley)
Comment on attachment 8774454 [details] [diff] [review]
Patch for aurora

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

Looks good to me! I assume because this is the same data that liuche's data-review still applies.

Thanks for your work!
Attachment #8774454 - Flags: review?(mconley) → review+
Comment on attachment 8774454 [details] [diff] [review]
Patch for aurora

Approval Request Comment
[Feature/regressing bug #]:

Simplify Print Preview (bug 962433)

[User impact if declined]:

None. This adds some Telemetry probes that we're going to use as a baseline to study how the Simplify Print Preview feature affects printing behaviour (if at all).

[Describe test coverage new/current, TreeHerder]:

None.

[Risks and why]: 

Very low risk - just adding some probes.

[String/UUID change made/needed]:

None.
Attachment #8774454 - Flags: approval-mozilla-aurora?
Comment on attachment 8774454 [details] [diff] [review]
Patch for aurora

moar telemetry, taking it
Attachment #8774454 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
You need to log in before you can comment on or make changes to this bug.