From 4612689b187a4eb7fbf3662c7159e361bcc97302 Mon Sep 17 00:00:00 2001 From: Neil Deakin Date: Wed, 14 May 2014 08:01:45 -0400 Subject: [PATCH] Bug 893098, record update starts, stops and errors in the health report, r=rnewman,rstrong --- .../mozapps/update/UpdaterHealthProvider.jsm | 69 +++++++++ toolkit/mozapps/update/moz.build | 6 +- toolkit/mozapps/update/nsUpdateService.js | 137 +++++++++++++----- .../mozapps/update/nsUpdateService.manifest | 3 + .../unit_aus_update/updateHealthReport.js | 91 ++++++++++++ .../update/tests/unit_aus_update/xpcshell.ini | 2 + 6 files changed, 271 insertions(+), 37 deletions(-) create mode 100644 toolkit/mozapps/update/UpdaterHealthProvider.jsm create mode 100644 toolkit/mozapps/update/tests/unit_aus_update/updateHealthReport.js diff --git a/toolkit/mozapps/update/UpdaterHealthProvider.jsm b/toolkit/mozapps/update/UpdaterHealthProvider.jsm new file mode 100644 index 000000000000..d15e0cf86d90 --- /dev/null +++ b/toolkit/mozapps/update/UpdaterHealthProvider.jsm @@ -0,0 +1,69 @@ +/* This Source Code Form is subject to the terms of the Mozilla Public + * License, v. 2.0. If a copy of the MPL was not distributed with this + * file, You can obtain one at http://mozilla.org/MPL/2.0/. */ + +"use strict"; + +this.EXPORTED_SYMBOLS = [ + "UpdateProvider", +]; + +const {classes: Cc, interfaces: Ci, utils: Cu} = Components; + +Cu.import("resource://gre/modules/Metrics.jsm", this); +Cu.import("resource://gre/modules/Task.jsm", this); + +const DAILY_COUNTER_FIELD = {type: Metrics.Storage.FIELD_DAILY_COUNTER}; +const DAILY_DISCRETE_NUMERIC_FIELD = {type: Metrics.Storage.FIELD_DAILY_DISCRETE_NUMERIC}; + +function UpdateMeasurement1() { + Metrics.Measurement.call(this); +} + +UpdateMeasurement1.prototype = Object.freeze({ + __proto__: Metrics.Measurement.prototype, + + name: "update", + version: 1, + + fields: { + updateCheckStartCount: DAILY_COUNTER_FIELD, + updateCheckSuccessCount: DAILY_COUNTER_FIELD, + updateCheckFailedCount: DAILY_COUNTER_FIELD, + updateCheckFailedStatuses: DAILY_DISCRETE_NUMERIC_FIELD, + completeUpdateStartCount: DAILY_COUNTER_FIELD, + partialUpdateStartCount: DAILY_COUNTER_FIELD, + completeUpdateSuccessCount: DAILY_COUNTER_FIELD, + partialUpdateSuccessCount: DAILY_COUNTER_FIELD, + updateFailedCount: DAILY_COUNTER_FIELD, + updateFailedStatuses: DAILY_DISCRETE_NUMERIC_FIELD, + }, +}); + +this.UpdateProvider = function () { + Metrics.Provider.call(this); +}; +UpdateProvider.prototype = Object.freeze({ + __proto__: Metrics.Provider.prototype, + + name: "org.mozilla.update", + + measurementTypes: [ + UpdateMeasurement1, + ], + + recordUpdate: function (field, status) { + let m = this.getMeasurement(UpdateMeasurement1.prototype.name, + UpdateMeasurement1.prototype.version); + + return this.enqueueStorageOperation(function recordUpdateFields() { + return Task.spawn(function recordUpdateFieldsTask() { + yield m.incrementDailyCounter(field + "Count"); + + if ((field == "updateFailed" || field == "updateCheckFailed") && status) { + yield m.addDailyDiscreteNumeric(field + "Statuses", status); + } + }.bind(this)); + }.bind(this)); + }, +}); diff --git a/toolkit/mozapps/update/moz.build b/toolkit/mozapps/update/moz.build index 2862c961a996..136b3c91e4d1 100644 --- a/toolkit/mozapps/update/moz.build +++ b/toolkit/mozapps/update/moz.build @@ -46,4 +46,8 @@ if CONFIG['MOZ_UPDATER']: 'nsUpdateServiceStub.js', ] -JAR_MANIFESTS += ['jar.mn'] \ No newline at end of file + EXTRA_JS_MODULES += [ + 'UpdaterHealthProvider.jsm' + ] + +JAR_MANIFESTS += ['jar.mn'] diff --git a/toolkit/mozapps/update/nsUpdateService.js b/toolkit/mozapps/update/nsUpdateService.js index 5e25bd97f85f..c23654778c8b 100644 --- a/toolkit/mozapps/update/nsUpdateService.js +++ b/toolkit/mozapps/update/nsUpdateService.js @@ -12,6 +12,7 @@ Components.utils.import("resource://gre/modules/FileUtils.jsm"); Components.utils.import("resource://gre/modules/AddonManager.jsm"); Components.utils.import("resource://gre/modules/Services.jsm"); Components.utils.import("resource://gre/modules/ctypes.jsm"); +Components.utils.import("resource://gre/modules/UpdaterHealthProvider.jsm"); const Cc = Components.classes; const Ci = Components.interfaces; @@ -258,6 +259,18 @@ const PING_BGUC_ADDON_UPDATES_FOR_INCOMPAT = 29; // Incompatible add-ons found (update notification) const PING_BGUC_ADDON_HAVE_INCOMPAT = 30; +// Health report field names +const UpdaterHealthReportFields = { + CHECK_START: "updateCheckStart", + CHECK_SUCCESS: "updateCheckSuccess", + CHECK_FAILED: "updateCheckFailed", + COMPLETE_START: "completeUpdateStart", + PARTIAL_START: "partialUpdateStart", + COMPLETE_SUCCESS: "completeUpdateSuccess", + PARTIAL_SUCCESS: "partialUpdateSuccess", + FAILED: "updateFailed" +}; + var gLocale = null; var gUpdateMutexHandle = null; @@ -935,6 +948,37 @@ function getStatusTextFromCode(code, defaultCode) { return reason; } +/** + * Record count in the health report. + * @param field + * The field name to record + * @param status + * Status code for errors, 0 otherwise + */ +function recordInHealthReport(field, status) { +#ifdef MOZ_SERVICES_HEALTHREPORT + try { + LOG("recordInHealthReport - " + field + " - " + status); + + let reporter = Cc["@mozilla.org/datareporting/service;1"] + .getService().wrappedJSObject.healthReporter; + + if (reporter) { + reporter.onInit().then(function recordUpdateInHealthReport() { + try { + reporter.getProvider("org.mozilla.update").recordUpdate(field, status); + } catch (ex) { + Cu.reportError(ex); + } + }); + } + // If getting the heath reporter service fails, don't fail updating. + } catch (ex) { + LOG("recordInHealthReport - could not initialize health reporter"); + } +#endif +} + /** * Get the Active Updates directory * @return The active updates directory, as a nsIFile object @@ -3600,6 +3644,8 @@ Checker.prototype = { if (!url || (!this.enabled && !force)) return; + recordInHealthReport(UpdaterHealthReportFields.CHECK_START, 0); + this._request = Cc["@mozilla.org/xmlextras/xmlhttprequest;1"]. createInstance(Ci.nsISupports); // This is here to let unit test code override XHR @@ -3723,6 +3769,8 @@ Checker.prototype = { if (Services.prefs.prefHasUserValue(PREF_APP_UPDATE_BACKGROUNDERRORS)) Services.prefs.clearUserPref(PREF_APP_UPDATE_BACKGROUNDERRORS); + recordInHealthReport(UpdaterHealthReportFields.CHECK_SUCCESS, 0); + // Tell the callback about the updates this._callback.onCheckComplete(event.target, updates, updates.length); } @@ -3743,6 +3791,9 @@ Checker.prototype = { update.errorCode = updates[0] ? CERT_ATTR_CHECK_FAILED_HAS_UPDATE : CERT_ATTR_CHECK_FAILED_NO_UPDATE; } + + recordInHealthReport(UpdaterHealthReportFields.CHECK_FAILED, update.errorCode); + this._callback.onError(request, update); } @@ -3774,6 +3825,8 @@ Checker.prototype = { update.errorCode = HTTP_ERROR_OFFSET + status; } + recordInHealthReport(UpdaterHealthReportFields.CHECK_FAILED, update.errorCode); + this._callback.onError(request, update); this._request = null; @@ -4088,6 +4141,10 @@ Downloader.prototype = { } this.isCompleteUpdate = this._patch.type == "complete"; + recordInHealthReport( + this.isCompleteUpdate ? UpdaterHealthReportFields.COMPLETE_START : + UpdaterHealthReportFields.PARTIAL_START, 0); + var patchFile = null; #ifdef MOZ_WIDGET_GONK @@ -4343,6 +4400,10 @@ Downloader.prototype = { "current fail: " + this.updateService._consecutiveSocketErrors + ", " + "max fail: " + maxFail + ", " + "retryTimeout: " + retryTimeout); if (Components.isSuccessCode(status)) { + recordInHealthReport( + this.isCompleteUpdate ? UpdaterHealthReportFields.COMPLETE_SUCCESS : + UpdaterHealthReportFields.PARTIAL_SUCCESS, 0); + if (this._verifyDownload()) { state = shouldUseService() ? STATE_PENDING_SVC : STATE_PENDING; if (this.background) { @@ -4371,49 +4432,53 @@ Downloader.prototype = { // Destroy the updates directory, since we're done with it. cleanUpUpdatesDir(); } - } else if (status == Cr.NS_ERROR_OFFLINE) { - // Register an online observer to try again. - // The online observer will continue the incremental download by - // calling downloadUpdate on the active update which continues - // downloading the file from where it was. - LOG("Downloader:onStopRequest - offline, register online observer: true"); - shouldRegisterOnlineObserver = true; - deleteActiveUpdate = false; - // Each of NS_ERROR_NET_TIMEOUT, ERROR_CONNECTION_REFUSED, and - // NS_ERROR_NET_RESET can be returned when disconnecting the internet while - // a download of a MAR is in progress. There may be others but I have not - // encountered them during testing. - } else if ((status == Cr.NS_ERROR_NET_TIMEOUT || - status == Cr.NS_ERROR_CONNECTION_REFUSED || - status == Cr.NS_ERROR_NET_RESET) && - this.updateService._consecutiveSocketErrors < maxFail) { - LOG("Downloader:onStopRequest - socket error, shouldRetrySoon: true"); - shouldRetrySoon = true; - deleteActiveUpdate = false; - } else if (status != Cr.NS_BINDING_ABORTED && - status != Cr.NS_ERROR_ABORT && - status != Cr.NS_ERROR_DOCUMENT_NOT_CACHED) { - LOG("Downloader:onStopRequest - non-verification failure"); - // Some sort of other failure, log this in the |statusText| property - state = STATE_DOWNLOAD_FAILED; + } else { + recordInHealthReport(UpdaterHealthReportFields.FAILED, status); - // XXXben - if |request| (The Incremental Download) provided a means - // for accessing the http channel we could do more here. + if (status == Cr.NS_ERROR_OFFLINE) { + // Register an online observer to try again. + // The online observer will continue the incremental download by + // calling downloadUpdate on the active update which continues + // downloading the file from where it was. + LOG("Downloader:onStopRequest - offline, register online observer: true"); + shouldRegisterOnlineObserver = true; + deleteActiveUpdate = false; + // Each of NS_ERROR_NET_TIMEOUT, ERROR_CONNECTION_REFUSED, and + // NS_ERROR_NET_RESET can be returned when disconnecting the internet while + // a download of a MAR is in progress. There may be others but I have not + // encountered them during testing. + } else if ((status == Cr.NS_ERROR_NET_TIMEOUT || + status == Cr.NS_ERROR_CONNECTION_REFUSED || + status == Cr.NS_ERROR_NET_RESET) && + this.updateService._consecutiveSocketErrors < maxFail) { + LOG("Downloader:onStopRequest - socket error, shouldRetrySoon: true"); + shouldRetrySoon = true; + deleteActiveUpdate = false; + } else if (status != Cr.NS_BINDING_ABORTED && + status != Cr.NS_ERROR_ABORT && + status != Cr.NS_ERROR_DOCUMENT_NOT_CACHED) { + LOG("Downloader:onStopRequest - non-verification failure"); + // Some sort of other failure, log this in the |statusText| property + state = STATE_DOWNLOAD_FAILED; - this._update.statusText = getStatusTextFromCode(status, - Cr.NS_BINDING_FAILED); + // XXXben - if |request| (The Incremental Download) provided a means + // for accessing the http channel we could do more here. + + this._update.statusText = getStatusTextFromCode(status, + Cr.NS_BINDING_FAILED); #ifdef MOZ_WIDGET_GONK - // bug891009: On FirefoxOS, manaully retry OTA download will reuse - // the Update object. We need to remove selected patch so that download - // can be triggered again successfully. - this._update.selectedPatch.selected = false; + // bug891009: On FirefoxOS, manaully retry OTA download will reuse + // the Update object. We need to remove selected patch so that download + // can be triggered again successfully. + this._update.selectedPatch.selected = false; #endif - // Destroy the updates directory, since we're done with it. - cleanUpUpdatesDir(); + // Destroy the updates directory, since we're done with it. + cleanUpUpdatesDir(); - deleteActiveUpdate = true; + deleteActiveUpdate = true; + } } LOG("Downloader:onStopRequest - setting state to: " + state); this._patch.state = state; diff --git a/toolkit/mozapps/update/nsUpdateService.manifest b/toolkit/mozapps/update/nsUpdateService.manifest index df5e77fbd8f5..dc27ee26757d 100644 --- a/toolkit/mozapps/update/nsUpdateService.manifest +++ b/toolkit/mozapps/update/nsUpdateService.manifest @@ -10,3 +10,6 @@ contract @mozilla.org/updates/update-prompt;1 {27ABA825-35B5-4018-9FDD-F99250A0E component {e43b0010-04ba-4da6-b523-1f92580bc150} nsUpdateServiceStub.js contract @mozilla.org/updates/update-service-stub;1 {e43b0010-04ba-4da6-b523-1f92580bc150} category profile-after-change nsUpdateServiceStub @mozilla.org/updates/update-service-stub;1 +#ifdef MOZ_SERVICES_HEALTHREPORT +category healthreport-js-provider-default UpdateProvider resource://gre/modules/UpdaterHealthProvider.jsm +#endif diff --git a/toolkit/mozapps/update/tests/unit_aus_update/updateHealthReport.js b/toolkit/mozapps/update/tests/unit_aus_update/updateHealthReport.js new file mode 100644 index 000000000000..b1fd4ce5acbf --- /dev/null +++ b/toolkit/mozapps/update/tests/unit_aus_update/updateHealthReport.js @@ -0,0 +1,91 @@ +/* Any copyright is dedicated to the Public Domain. + * http://creativecommons.org/publicdomain/zero/1.0/ */ + +"use strict"; + +const {utils: Cu, classes: Cc, interfaces: Ci} = Components; + + +Cu.import("resource://gre/modules/Metrics.jsm"); +Cu.import("resource://gre/modules/UpdaterHealthProvider.jsm"); + +// Create a profile +let gProfile = do_get_profile(); + +function run_test() { + run_next_test(); +} + +add_test(function test_constructor() { + let provider = new UpdateProvider(); + run_next_test(); +}); + +add_task(function test_init() { + let storage = yield Metrics.Storage("init"); + let provider = new UpdateProvider(); + yield provider.init(storage); + yield provider.shutdown(); + + yield storage.close(); +}); + +add_task(function test_collect() { + let storage = yield Metrics.Storage("collect"); + let provider = new UpdateProvider(); + yield provider.init(storage); + + let now = new Date(); + + let m = provider.getMeasurement("update", 1); + + let fieldcount = 0; + for (let field of ["updateCheckStart", + "updateCheckSuccess", + "completeUpdateStart", + "partialUpdateStart", + "completeUpdateSuccess", + "partialUpdateSuccess"]) { + fieldcount++; // One new day per iteration + + yield provider.recordUpdate(field, 0); + + let data = yield m.getValues(); + do_check_eq(data.days.size, 1); + + let day = data.days.getDay(now); + do_check_eq(day.size, fieldcount); + do_check_eq(day.get(field + "Count"), 1); + + yield provider.recordUpdate(field, 0); + + data = yield m.getValues(); + day = data.days.getDay(now); + do_check_eq(day.size, fieldcount); + do_check_eq(day.get(field + "Count"), 2); + } + + for (let field of ["updateCheckFailed", "updateFailed"]) { + fieldcount += 2; // Two fields added per iteration + + yield provider.recordUpdate(field, 500); + + let data = yield m.getValues(); + let day = data.days.getDay(now); + do_check_eq(day.size, fieldcount); + + do_check_eq(day.get(field + "Statuses"), 500); + + yield provider.recordUpdate(field, 800); + + data = yield m.getValues(); + day = data.days.getDay(now); + do_check_eq(day.size, fieldcount); + do_check_eq(day.get(field + "Statuses")[0], 500); + do_check_eq(day.get(field + "Statuses")[1], 800); + } + + yield provider.shutdown(); + yield storage.close(); +}); + diff --git a/toolkit/mozapps/update/tests/unit_aus_update/xpcshell.ini b/toolkit/mozapps/update/tests/unit_aus_update/xpcshell.ini index 104fd13b4f11..02f47c2e4e61 100644 --- a/toolkit/mozapps/update/tests/unit_aus_update/xpcshell.ini +++ b/toolkit/mozapps/update/tests/unit_aus_update/xpcshell.ini @@ -39,3 +39,5 @@ skip-if = toolkit == 'gonk' reason = custom nsIUpdatePrompt [updateRootDirMigration_win.js] run-if = os == 'win' +[updateHealthReport.js] +skip-if = ! healthreport