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

Status: RESOLVED FIXED

(Toolkit :: Printing, defect)

Description

[Mike Conley (:mconley) (:⚙️)]

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

Comment 1

[Mike Conley (:mconley) (:⚙️)]

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).

Comment 2

[Thauã Silveira]

Attachment: WIP1: Add telemetry probes

This patch adds the telemetry probes in order to keep tracking of printing data.

Comment 3

[Mike Conley (: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.

Comment 4

[Thauã Silveira]

Attachment: WIP2: Move telemetry probes to browser-content.js file

This patch moves telemetry probes to the browser-content.js file.

Comment 5

[Mike Conley (: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");
}

Comment 6

[Thauã Silveira]

Attachment: WIP3: Small changes

Small changes as mconley suggested.

Comment 7

[Mike Conley (:mconley) (:⚙️)]

Comment on attachment 8773473 [details] [diff] [review]
WIP3: Small changes

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

Looks good!

Comment 8

[Chenxia Liu [:liuche]]

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!

Comment 9

[Mike Conley (:mconley) (:⚙️)]

Comment on attachment 8773473 [details] [diff] [review]
WIP3: Small changes

Okay, we got the data-review+. Clearing r?. Thanks liuche!

Comment 10

[Mike Conley (:mconley) (:⚙️)]

Adding checkin-needed:

Author: Thauã Silveira <[e-mail]>
Bug number: 1287587
Commit message: Bug 1287587 - Add Telemetry probes for user printing activities. r=mconley, data-review=liuche.

Comment 11

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/400da2ae93a1
Add Telemetry probes for user printing activities. r=mconley, data-review=liuche

Comment 12

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/400da2ae93a1

Comment 13

[Thauã Silveira]

Attachment: Patch for aurora

This patch adds the telemetry for printing usage to Aurora.

Comment 14

[Mike Conley (: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!

Comment 15

[Mike Conley (:mconley) (:⚙️)]

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.

Comment 16

[Sylvestre Ledru [:Sylvestre]]

Comment on attachment 8774454 [details] [diff] [review]
Patch for aurora

moar telemetry, taking it

Comment 17

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-aurora/rev/bb3556aea128