Closed Bug 708123 Opened 8 years ago Closed 8 years ago

Add a telemetry probe for update status

Categories

(Toolkit :: Application Update, defect)

defect
Not set

Tracking

()

RESOLVED FIXED
mozilla11

People

(Reporter: ehsan, Assigned: ehsan)

References

Details

Attachments

(2 files, 5 obsolete files)

Attached patch Patch (v1) (obsolete) — Splinter Review
No description provided.
Attachment #579507 - Flags: review?(robert.bugzilla)
Blocks: 481815
Assignee: nobody → ehsan
Attached patch Patch (v1) (obsolete) — Splinter Review
Adjusted the values based on the new error codes Brian has used.
Attachment #579507 - Attachment is obsolete: true
Attachment #579507 - Flags: review?(robert.bugzilla)
Attachment #579690 - Flags: review?(robert.bugzilla)
Attached patch Patch (v2) (obsolete) — Splinter Review
Fixed a bug caught by bbondy
Attachment #579690 - Attachment is obsolete: true
Attachment #579690 - Flags: review?(robert.bugzilla)
Attachment #579695 - Flags: review?(robert.bugzilla)
Attached patch Patch (v3) (obsolete) — Splinter Review
Even more fixes!
Attachment #579695 - Attachment is obsolete: true
Attachment #579695 - Flags: review?(robert.bugzilla)
Attachment #579703 - Flags: review?(robert.bugzilla)
OS: Mac OS X → All
Hardware: x86 → All
Comment on attachment 579703 [details] [diff] [review]
Patch (v3)

>diff --git a/toolkit/components/telemetry/TelemetryHistograms.h b/toolkit/components/telemetry/TelemetryHistograms.h
>index e4aa8cd..1b8dd4e 100644
>--- a/toolkit/components/telemetry/TelemetryHistograms.h
>+++ b/toolkit/components/telemetry/TelemetryHistograms.h
>@@ -239,6 +239,11 @@ HISTOGRAM(PLACES_EXPIRATION_STEPS_TO_CLEAN, 1, 10, 10, LINEAR, "PLACES: Expirati
> HISTOGRAM(PLACES_AUTOCOMPLETE_1ST_RESULT_TIME_MS, 50, 500, 10, EXPONENTIAL, "PLACES: Time for first autocomplete result if > 50ms (ms)")
> 
> /**
>+ * Updater telemetry.
>+ */
>+HISTOGRAM(UPDATE_STATUS, 0, 16004, 18, LINEAR, "Updater: the status of the latest update performed")
>+
>+/**
>  * Thunderbird-specific telemetry.
>  */
> #ifdef MOZ_THUNDERBIRD
>diff --git a/toolkit/mozapps/update/nsUpdateService.js b/toolkit/mozapps/update/nsUpdateService.js
>index 2209fe9..74d85b5 100644
>--- a/toolkit/mozapps/update/nsUpdateService.js
>+++ b/toolkit/mozapps/update/nsUpdateService.js
>@@ -1363,6 +1363,7 @@ UpdateService.prototype = {
>                    createInstance(Ci.nsIUpdatePrompt);
> 
>     update.state = status;
>+    this._submitTelemetryPing(status);
>     if (status == STATE_SUCCEEDED) {
>       update.statusText = gUpdateBundle.GetStringFromName("installSuccess");
> 
>@@ -1427,6 +1428,26 @@ UpdateService.prototype = {
>   },
> 
>   /**
>+   * Submit the results of applying the update via telemetry.
>+   *
>+   * @param  status
>+   *         The status of the update as read from the update.status file
>+   */
>+  _submitTelemetryPing: function AUS__submitTelemetryPing(status) {
>+    let parts = status.split(":");
>+    if ((parts.length == 1 && status != STATE_SUCCEEDED) ||
>+        (parts.length > 1  && parts[0] != STATE_FAILED)) {
>+      // we only want to report successes or failures
nit: "success or failure" since this only reports one status

>+      return;
>+    }
>+    let result = 0; // 0 means success
>+    if (parts.length > 1) {
>+      result = parseInt(parts[1]);
Since crazy things can happen here (QA has been known to manually modify this file without an error number for example) please try catch this. Would be nice if we reported failure of this as well but I think I'm ok with it just not reporting this condition.

>+    }
>+    Services.telemetry.getHistogramById("UPDATE_STATUS").add(result);
>+  },
>+
>+  /**
>    * Notified when a timer fires
>    * @param   timer
>    *          The timer that fired
Attachment #579703 - Flags: review?(robert.bugzilla) → review-
Attached patch Patch (v1) (obsolete) — Splinter Review
Attachment #579703 - Attachment is obsolete: true
Attachment #580116 - Flags: review?(robert.bugzilla)
Attached patch Patch (v4)Splinter Review
Attachment #580116 - Attachment is obsolete: true
Attachment #580116 - Flags: review?(robert.bugzilla)
Attachment #580117 - Flags: review?(robert.bugzilla)
https://hg.mozilla.org/integration/mozilla-inbound/rev/6c252e670af9
Target Milestone: --- → mozilla11
https://hg.mozilla.org/mozilla-central/rev/6c252e670af9
Status: NEW → RESOLVED
Closed: 8 years ago
Resolution: --- → FIXED
Depends on: 711195
Attached patch Patch v1.Splinter Review
Attachment #585418 - Flags: review?(robert.bugzilla)
Comment on attachment 585418 [details] [diff] [review]
Patch v1.

r=me.  For the record, it's usually a bad idea to attach a patch to a resolved bug.
Attachment #585418 - Flags: review?(robert.bugzilla) → review+
Pushed to mozilla-inbound:
http://hg.mozilla.org/integration/mozilla-inbound/rev/683a56961090

> r=me.  For the record, it's usually a bad idea to attach a patch to a 
> resolved bug.

Understood and agreed, will do that next time even for a trivial related change.
(In reply to Ehsan Akhgari [:ehsan] from comment #10)
> r=me.  For the record, it's usually a bad idea to attach a patch to a
> resolved bug.

especially when it gets pushed with the wrong bug number in the check-in comment, so that I spend a bunch of time looking for an open bug with that changeset :p

https://hg.mozilla.org/mozilla-central/rev/683a56961090
:( sorry about that.
Blocks: 731901
You need to log in before you can comment on or make changes to this bug.