Bug 1336360 - Use the PingSender for sending pings when the browser shuts down. r=chutten
MozReview-Commit-ID: 6bNQs87UQ9m
--- a/toolkit/components/telemetry/TelemetryController.jsm
+++ b/toolkit/components/telemetry/TelemetryController.jsm
@@ -192,21 +192,23 @@ this.TelemetryController = Object.freeze
* @param {String} aType The type of the ping.
* @param {Object} aPayload The actual data payload for the ping.
* @param {Object} [aOptions] Options object.
* @param {Boolean} [aOptions.addClientId=false] true if the ping should contain the client
* id, false otherwise.
* @param {Boolean} [aOptions.addEnvironment=false] true if the ping should contain the
* environment data.
* @param {Object} [aOptions.overrideEnvironment=null] set to override the environment data.
+ * @param {Boolean} [aOptions.usePingSender=false] if true, send the ping using the PingSender.
* @returns {Promise} Test-only - a promise that resolves with the ping id once the ping is stored or sent.
*/
submitExternalPing(aType, aPayload, aOptions = {}) {
aOptions.addClientId = aOptions.addClientId || false;
aOptions.addEnvironment = aOptions.addEnvironment || false;
+ aOptions.usePingSender = aOptions.usePingSender || false;
return Impl.submitExternalPing(aType, aPayload, aOptions);
},
/**
* Get the current session ping data as it would be sent out or stored.
*
* @param {bool} aSubsession Whether to get subsession data. Optional, defaults to false.
@@ -446,16 +448,17 @@ var Impl = {
* @param {String} aType The type of the ping.
* @param {Object} aPayload The actual data payload for the ping.
* @param {Object} [aOptions] Options object.
* @param {Boolean} [aOptions.addClientId=false] true if the ping should contain the client
* id, false otherwise.
* @param {Boolean} [aOptions.addEnvironment=false] true if the ping should contain the
* environment data.
* @param {Object} [aOptions.overrideEnvironment=null] set to override the environment data.
+ * @param {Boolean} [aOptions.usePingSender=false] if true, send the ping using the PingSender.
* @returns {Promise} Test-only - a promise that is resolved with the ping id once the ping is stored or sent.
*/
_submitPingLogic: Task.async(function* (aType, aPayload, aOptions) {
// Make sure to have a clientId if we need one. This cover the case of submitting
// a ping early during startup, before Telemetry is initialized, if no client id was
// cached.
if (!this._clientID && aOptions.addClientId) {
Telemetry.getHistogramById("TELEMETRY_PING_SUBMISSION_WAITING_CLIENTID").add();
@@ -468,32 +471,33 @@ var Impl = {
this._log.trace("submitExternalPing - ping assembled, id: " + pingData.id);
// Always persist the pings if we are allowed to. We should not yield on any of the
// following operations to keep this function synchronous for the majority of the calls.
let archivePromise = TelemetryArchive.promiseArchivePing(pingData)
.catch(e => this._log.error("submitExternalPing - Failed to archive ping " + pingData.id, e));
let p = [ archivePromise ];
- p.push(TelemetrySend.submitPing(pingData));
+ p.push(TelemetrySend.submitPing(pingData, {usePingSender: aOptions.usePingSender}));
return Promise.all(p).then(() => pingData.id);
}),
/**
* Submit ping payloads to Telemetry.
*
* @param {String} aType The type of the ping.
* @param {Object} aPayload The actual data payload for the ping.
* @param {Object} [aOptions] Options object.
* @param {Boolean} [aOptions.addClientId=false] true if the ping should contain the client
* id, false otherwise.
* @param {Boolean} [aOptions.addEnvironment=false] true if the ping should contain the
* environment data.
* @param {Object} [aOptions.overrideEnvironment=null] set to override the environment data.
+ * @param {Boolean} [aOptions.usePingSender=false] if true, send the ping using the PingSender.
* @returns {Promise} Test-only - a promise that is resolved with the ping id once the ping is stored or sent.
*/
submitExternalPing: function send(aType, aPayload, aOptions) {
this._log.trace("submitExternalPing - type: " + aType + ", aOptions: " + JSON.stringify(aOptions));
// Reject pings sent after shutdown.
if (this._shuttingDown) {
const errorMessage = "submitExternalPing - Submission is not allowed after shutdown, discarding ping of type: " + aType;
--- a/toolkit/components/telemetry/TelemetrySend.jsm
+++ b/toolkit/components/telemetry/TelemetrySend.jsm
@@ -29,16 +29,18 @@ Cu.import("resource://gre/modules/Teleme
Cu.import("resource://gre/modules/Timer.jsm", this);
XPCOMUtils.defineLazyModuleGetter(this, "AsyncShutdown",
"resource://gre/modules/AsyncShutdown.jsm");
XPCOMUtils.defineLazyModuleGetter(this, "TelemetryStorage",
"resource://gre/modules/TelemetryStorage.jsm");
XPCOMUtils.defineLazyModuleGetter(this, "TelemetryReportingPolicy",
"resource://gre/modules/TelemetryReportingPolicy.jsm");
+XPCOMUtils.defineLazyModuleGetter(this, "OS",
+ "resource://gre/modules/osfile.jsm");
XPCOMUtils.defineLazyServiceGetter(this, "Telemetry",
"@mozilla.org/base/telemetry;1",
"nsITelemetry");
const Utils = TelemetryUtils;
const LOGGER_NAME = "Toolkit.Telemetry";
const LOGGER_PREFIX = "TelemetrySend::";
@@ -208,20 +210,23 @@ this.TelemetrySend = {
},
/**
* Submit a ping for sending. This will:
* - send the ping right away if possible or
* - save the ping to disk and send it at the next opportunity
*
* @param {Object} ping The ping data to send, must be serializable to JSON.
+ * @param {Object} [aOptions] Options object.
+ * @param {Boolean} [options.usePingSender=false] if true, send the ping using the PingSender.
* @return {Promise} Test-only - a promise that is resolved when the ping is sent or saved.
*/
- submitPing(ping) {
- return TelemetrySendImpl.submitPing(ping);
+ submitPing(ping, options = {}) {
+ options.usePingSender = options.usePingSender || false;
+ return TelemetrySendImpl.submitPing(ping, options);
},
/**
* Count of pending pings that were found to be overdue at startup.
*/
get overduePingsCount() {
return TelemetrySendImpl.overduePingsCount;
},
@@ -757,24 +762,56 @@ var TelemetrySendImpl = {
observe(subject, topic, data) {
switch (topic) {
case TOPIC_IDLE_DAILY:
SendScheduler.triggerSendingPings(true);
break;
}
},
- submitPing(ping) {
- this._log.trace("submitPing - ping id: " + ping.id);
+ /**
+ * Spawn the PingSender process that sends a ping. This function does
+ * not return an error or throw, it only logs an error.
+ *
+ * Even if the function doesn't fail, it doesn't mean that the ping was
+ * successfully sent, as we have no control over the spawned process. If it,
+ * succeeds, the ping is eventually removed from the disk to prevent duplicated
+ * submissions.
+ *
+ * @param {String} pingId The id of the ping to send.
+ * @param {String} submissionURL The complete Telemetry-compliant URL for the ping.
+ */
+ _sendWithPingSender(pingId, submissionURL) {
+ this._log.trace("_sendWithPingSender - sending " + pingId + " to " + submissionURL);
+ try {
+ const pingPath = OS.Path.join(TelemetryStorage.pingDirectoryPath, pingId);
+ Telemetry.runPingSender(submissionURL, pingPath);
+ } catch (e) {
+ this._log.error("_sendWithPingSender - failed to submit shutdown ping", e);
+ }
+ },
+
+ submitPing(ping, options) {
+ this._log.trace("submitPing - ping id: " + ping.id + ", options: " + JSON.stringify(options));
if (!this.sendingEnabled(ping)) {
this._log.trace("submitPing - Telemetry is not allowed to send pings.");
return Promise.resolve();
}
+ // Send the ping using the PingSender, if requested and the user was
+ // notified of our policy.
+ if (options.usePingSender && TelemetryReportingPolicy.canUpload()) {
+ const url = this._buildSubmissionURL(ping);
+ // Serialize the ping to the disk and spawn the PingSender.
+ let savePromise = savePing(ping);
+ savePromise.then(() => this._sendWithPingSender(ping.id, url));
+ return savePromise;
+ }
+
if (!this.canSendNow) {
// Sending is disabled or throttled, add this to the persisted pending pings.
this._log.trace("submitPing - can't send ping now, persisting to disk - " +
"canSendNow: " + this.canSendNow);
return savePing(ping);
}
// Let the scheduler trigger sending pings if possible.
@@ -924,16 +961,21 @@ var TelemetrySendImpl = {
if (TelemetryStorage.isDeletionPing(id)) {
return TelemetryStorage.removeDeletionPing();
}
return TelemetryStorage.removePendingPing(id);
}
return Promise.resolve();
},
+ _buildSubmissionURL(ping) {
+ const version = isV4PingFormat(ping) ? PING_FORMAT_VERSION : 1;
+ return this._server + this._getSubmissionPath(ping) + "?v=" + version;
+ },
+
_getSubmissionPath(ping) {
// The new ping format contains an "application" section, the old one doesn't.
let pathComponents;
if (isV4PingFormat(ping)) {
// We insert the Ping id in the URL to simplify server handling of duplicated
// pings.
let app = ping.application;
pathComponents = [
@@ -967,19 +1009,17 @@ var TelemetrySendImpl = {
// We can't send the pings to the server, so don't try to.
this._log.trace("_doPing - Can't send ping " + ping.id);
return Promise.resolve();
}
this._log.trace("_doPing - server: " + this._server + ", persisted: " + isPersisted +
", id: " + id);
- const isNewPing = isV4PingFormat(ping);
- const version = isNewPing ? PING_FORMAT_VERSION : 1;
- const url = this._server + this._getSubmissionPath(ping) + "?v=" + version;
+ const url = this._buildSubmissionURL(ping);
let request = new ServiceRequest();
request.mozBackgroundRequest = true;
request.timeout = PING_SUBMIT_TIMEOUT_MS;
request.open("POST", url, true);
request.overrideMimeType("text/plain");
request.setRequestHeader("Content-Type", "application/json; charset=UTF-8");
@@ -1045,17 +1085,17 @@ var TelemetrySendImpl = {
this._log.error("_doPing - error submitting to " + url + ", status: " + status
+ ", type: " + event.type);
}
onRequestFinished(success, event);
};
// If that's a legacy ping format, just send its payload.
- let networkPayload = isNewPing ? ping : ping.payload;
+ let networkPayload = isV4PingFormat(ping) ? ping : ping.payload;
request.setRequestHeader("Content-Encoding", "gzip");
let converter = Cc["@mozilla.org/intl/scriptableunicodeconverter"]
.createInstance(Ci.nsIScriptableUnicodeConverter);
converter.charset = "UTF-8";
let startTime = new Date();
let utf8Payload = converter.ConvertFromUnicode(JSON.stringify(networkPayload));
utf8Payload += converter.Finish();
Telemetry.getHistogramById("TELEMETRY_STRINGIFY").add(new Date() - startTime);
--- a/toolkit/components/telemetry/TelemetrySession.jsm
+++ b/toolkit/components/telemetry/TelemetrySession.jsm
@@ -60,16 +60,17 @@ const MIN_SUBSESSION_LENGTH_MS = Prefere
const LOGGER_NAME = "Toolkit.Telemetry";
const LOGGER_PREFIX = "TelemetrySession" + (Utils.isContentProcess ? "#content::" : "::");
const PREF_BRANCH = "toolkit.telemetry.";
const PREF_PREVIOUS_BUILDID = PREF_BRANCH + "previousBuildID";
const PREF_FHR_UPLOAD_ENABLED = "datareporting.healthreport.uploadEnabled";
const PREF_ASYNC_PLUGIN_INIT = "dom.ipc.plugins.asyncInit.enabled";
const PREF_UNIFIED = PREF_BRANCH + "unified";
+const PREF_SHUTDOWN_PINGSENDER = PREF_BRANCH + "shutdownPingSender.enabled";
const MESSAGE_TELEMETRY_PAYLOAD = "Telemetry:Payload";
const MESSAGE_TELEMETRY_THREAD_HANGS = "Telemetry:ChildThreadHangs";
const MESSAGE_TELEMETRY_GET_CHILD_THREAD_HANGS = "Telemetry:GetChildThreadHangs";
const MESSAGE_TELEMETRY_USS = "Telemetry:USS";
const MESSAGE_TELEMETRY_GET_CHILD_USS = "Telemetry:GetChildUSS";
const DATAREPORTING_DIRECTORY = "datareporting";
@@ -1732,16 +1733,17 @@ var Impl = {
let p = [];
if (IS_UNIFIED_TELEMETRY) {
let shutdownPayload = this.getSessionPayload(REASON_SHUTDOWN, false);
let options = {
addClientId: true,
addEnvironment: true,
+ usePingSender: Preferences.get(PREF_SHUTDOWN_PINGSENDER, false),
};
p.push(TelemetryController.submitExternalPing(getPingType(shutdownPayload), shutdownPayload, options)
.catch(e => this._log.error("saveShutdownPings - failed to submit shutdown ping", e)));
}
// As a temporary measure, we want to submit saved-session too if extended Telemetry is enabled
// to keep existing performance analysis working.
if (Telemetry.canRecordExtended) {
--- a/toolkit/components/telemetry/docs/internals/preferences.rst
+++ b/toolkit/components/telemetry/docs/internals/preferences.rst
@@ -45,16 +45,20 @@ Preferences
This sets the Telemetry logging verbosity per ``Log.jsm``, with ``Trace`` or ``0`` being the most verbose and the default being ``Warn``.
By default logging goes only the console service.
``toolkit.telemetry.log.dump``
Sets whether to dump Telemetry log messages to ``stdout`` too.
+``toolkit.telemetry.shutdownPingSender.enabled``
+
+ Allow the ``shutdown`` ping to be sent when the browser shuts down, instead of the next restart, using the :doc:`ping sender <pingsender>`.
+
Data-choices notification
-------------------------
``toolkit.telemetry.reportingpolicy.firstRun``
This preference is not present until the first run. After, its value is set to false. This is used to show the infobar with a more aggressive timeout if it wasn't shown yet.
``datareporting.policy.firstRunURL``
--- a/toolkit/components/telemetry/tests/unit/test_TelemetrySession.js
+++ b/toolkit/components/telemetry/tests/unit/test_TelemetrySession.js
@@ -58,16 +58,18 @@ const NUMBER_OF_THREADS_TO_LAUNCH = 30;
var gNumberOfThreadsLaunched = 0;
const MS_IN_ONE_HOUR = 60 * 60 * 1000;
const MS_IN_ONE_DAY = 24 * MS_IN_ONE_HOUR;
const PREF_BRANCH = "toolkit.telemetry.";
const PREF_SERVER = PREF_BRANCH + "server";
const PREF_FHR_UPLOAD_ENABLED = "datareporting.healthreport.uploadEnabled";
+const PREF_BYPASS_NOTIFICATION = "datareporting.policy.dataSubmissionPolicyBypassNotification";
+const PREF_SHUTDOWN_PINGSENDER = "toolkit.telemetry.shutdownPingSender.enabled";
const DATAREPORTING_DIR = "datareporting";
const ABORTED_PING_FILE_NAME = "aborted-session-ping";
const ABORTED_SESSION_UPDATE_INTERVAL_MS = 5 * 60 * 1000;
XPCOMUtils.defineLazyGetter(this, "DATAREPORTING_PATH", function() {
return OS.Path.join(OS.Constants.Path.profileDir, DATAREPORTING_DIR);
});
@@ -1366,16 +1368,66 @@ add_task(function* test_savedPingsOnShut
(ping.type == PING_TYPE_SAVED_SESSION) ? REASON_SAVED_SESSION : REASON_SHUTDOWN;
checkPingFormat(ping, ping.type, true, true);
Assert.equal(ping.payload.info.reason, expectedReason);
Assert.equal(ping.clientId, gClientID);
}
});
+add_task(function* test_sendShutdownPing() {
+ if (gIsAndroid ||
+ (AppConstants.platform == "linux" && OS.Constants.Sys.bits == 32)) {
+ // We don't support the pingsender on Android, yet, see bug 1335917.
+ // We also don't suppor the pingsender testing on Treeherder for
+ // Linux 32 bit (due to missing libraries). So skip it there too.
+ // See bug 1310703 comment 78.
+ return;
+ }
+
+ Preferences.set(PREF_SHUTDOWN_PINGSENDER, true);
+ PingServer.clearRequests();
+
+ // Shutdown telemetry and wait for an incoming ping.
+ let nextPing = PingServer.promiseNextPing();
+ yield TelemetryController.testShutdown();
+ const ping = yield nextPing;
+
+ // Check that we received a shutdown ping.
+ checkPingFormat(ping, ping.type, true, true);
+ Assert.equal(ping.payload.info.reason, REASON_SHUTDOWN);
+ Assert.equal(ping.clientId, gClientID);
+
+ // Try again, this time disable ping upload. The PingSender
+ // should not be sending any ping!
+ PingServer.registerPingHandler(() => Assert.ok(false, "Telemetry must not send pings if not allowed to."));
+ Preferences.set(PREF_FHR_UPLOAD_ENABLED, false);
+ yield TelemetryController.testReset();
+ yield TelemetryController.testShutdown();
+
+ // Make sure we have no pending pings between the runs.
+ yield TelemetryStorage.testClearPendingPings();
+
+ // Enable ping upload and disable the "submission policy".
+ // The shutdown ping must not be sent.
+ Preferences.set(PREF_FHR_UPLOAD_ENABLED, true);
+ Preferences.set(PREF_BYPASS_NOTIFICATION, false);
+ yield TelemetryController.testReset();
+ yield TelemetryController.testShutdown();
+
+
+ // Reset the pref and restart Telemetry.
+ Preferences.reset(PREF_SHUTDOWN_PINGSENDER);
+ // We cannot reset PREF_BYPASS_NOTIFICATION, as we need it to be
+ // |true| in tests.
+ Preferences.set(PREF_BYPASS_NOTIFICATION, true);
+ PingServer.resetPingHandler();
+ yield TelemetryController.testReset();
+});
+
add_task(function* test_savedSessionData() {
// Create the directory which will contain the data file, if it doesn't already
// exist.
yield OS.File.makeDir(DATAREPORTING_PATH);
getHistogram("TELEMETRY_SESSIONDATA_FAILED_LOAD").clear();
getHistogram("TELEMETRY_SESSIONDATA_FAILED_PARSE").clear();
getHistogram("TELEMETRY_SESSIONDATA_FAILED_VALIDATION").clear();