763011 - Report telemetry data for whether or not updates are enabled

Status: RESOLVED FIXED

(Toolkit :: Application Update, defect)

Description

[Brian R. Bondy [:bbondy]]

We need to create a telemetry histogram for UPDATE_ENABLED.  It should be a boolean histogram with values 0, 1 which correspond with the pref app.update.enabled.

Comment 1

[Brian R. Bondy [:bbondy]]

Attachment: Patch v1 - Not yet tested

Comment 2

[Brian R. Bondy [:bbondy]]

Attachment: Patch v1' - Tested

Comment 3

[Robert Strong (they/them - no direct email)]

Comment on attachment 631608 [details] [diff] [review]
Patch v1' - Tested

>diff --git a/toolkit/mozapps/update/nsUpdateService.js b/toolkit/mozapps/update/nsUpdateService.js
>--- a/toolkit/mozapps/update/nsUpdateService.js
>+++ b/toolkit/mozapps/update/nsUpdateService.js
>@@ -1582,18 +1582,23 @@ UpdateService.prototype = {
> 
>     if (!update)
>       update = new Update(null);
> 
>     var prompter = Cc["@mozilla.org/updates/update-prompt;1"].
>                    createInstance(Ci.nsIUpdatePrompt);
> 
>     update.state = status;
>-    this._submitTelemetryPing(status);
>+    this._sendStatusCodeTelemetryPing(status);
>+
>     if (status == STATE_SUCCEEDED) {
>+      // Report telemetry that we want after each successful update
Please expand the comment to include why it is only reported after a successful update.

>+      this._sendBoolPrefTelemetryPing(PREF_APP_UPDATE_ENABLED,
>+                                      "UPDATER_UPDATES_ENABLED");
>+
>       update.statusText = gUpdateBundle.GetStringFromName("installSuccess");
> 
>       // Update the patch's metadata.
>       um.activeUpdate = update;
>       Services.prefs.setBoolPref(PREF_APP_UPDATE_POSTUPDATE, true);
>       prompter.showUpdateInstalled();
> 
>       // Done with this update. Clean it up.
>@@ -1619,22 +1624,40 @@ UpdateService.prototype = {
>       // Something went wrong with the patch application process.
>       handleFallbackToCompleteUpdate(update);
> 
>       prompter.showUpdateError(update);
>     }
>   },
> 
>   /**
>+   * Submit a telemetry ping with the boolean value of a pref for a histogram
>+   *
>+   * @param  pref
>+   *         The preference to report
>+   * @param histogram
>+   *         The histogram ID to report to
alignment

>+   */
>+  _sendBoolPrefTelemetryPing: function AUS__boolTelemetryPing(pref, histogram) {
>+    try {
>+      let val = getPref("getBoolPref", pref, 0);
>+      Services.telemetry.getHistogramById(histogram).add(+val);
>+    } catch(e) {
>+      // Don't allow any exception to be propagated.
>+      Components.utils.reportError(e);
>+    }
Add the same comment regarding getPref that I asked you about in bug 763022.

Comment 4

[Brian R. Bondy [:bbondy]]

Attachment: Patch v2

Implemented review nits and carrying forward r+.

Comment 5

[Brian R. Bondy [:bbondy]]

Attachment: Patch v3

The getPref call had a default value of 0 for a boolean pref.  It was fine but replaced it with false instead.

Comment 6

[Brian R. Bondy [:bbondy]]

http://hg.mozilla.org/mozilla-central/rev/08dd31cffebc