Bug 1310703 - Use the ID generated by the crashreporter client when sending a crash ping so that server-side deduplication works correctly; r?Dexter
MozReview-Commit-ID: JvqY0Lu75HQ
--- a/toolkit/components/crashes/CrashManager.jsm
+++ b/toolkit/components/crashes/CrashManager.jsm
@@ -50,17 +50,19 @@ function dateToDays(date) {
*
* @returns {Object|String} the parsed object or the raw string
*/
function parseAndRemoveField(obj, field, parseAsJson = true) {
let value = null;
if (field in obj) {
if (!parseAsJson) {
- value = obj[field];
+ // We split extra files on LF characters but Windows-generated ones might
+ // contain trailing CR characters so trim them here.
+ value = obj[field].trim();
} else {
try {
value = JSON.parse(obj[field]);
} catch (e) {
Cu.reportError(e);
}
}
@@ -633,16 +635,19 @@ this.CrashManager.prototype = Object.fre
// If we have a saved environment, use it. Otherwise report
// the current environment.
let reportMeta = Cu.cloneInto(metadata, myScope);
let crashEnvironment = parseAndRemoveField(reportMeta,
"TelemetryEnvironment");
let sessionId = parseAndRemoveField(reportMeta, "TelemetrySessionId",
/* parseAsJson */ false);
let stackTraces = parseAndRemoveField(reportMeta, "StackTraces");
+ // If CrashPingUUID is not present then Telemetry will generate a new UUID
+ let pingId = parseAndRemoveField(reportMeta, "CrashPingUUID",
+ /* parseAsJson */ false);
// Filter the remaining annotations to remove privacy-sensitive ones
reportMeta = this._filterAnnotations(reportMeta);
this._pingPromise = TelemetryController.submitExternalPing("crash",
{
version: 1,
crashDate: date.toISOString().slice(0, 10), // YYYY-MM-DD
@@ -652,16 +657,17 @@ this.CrashManager.prototype = Object.fre
stackTraces,
metadata: reportMeta,
hasCrashEnvironment: (crashEnvironment !== null),
},
{
addClientId: true,
addEnvironment: true,
overrideEnvironment: crashEnvironment,
+ overridePingId: pingId
}
);
},
_handleEventFilePayload(store, entry, type, date, payload) {
// The payload types and formats are documented in docs/crash-events.rst.
// Do not change the format of an existing type. Instead, invent a new
// type.
--- a/toolkit/components/telemetry/TelemetryController.jsm
+++ b/toolkit/components/telemetry/TelemetryController.jsm
@@ -192,16 +192,18 @@ 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 {String} [aOptions.overridePingId=null] set to override the
+ * generated ping id.
* @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;
return Impl.submitExternalPing(aType, aPayload, aOptions);
},
@@ -224,16 +226,18 @@ this.TelemetryController = Object.freeze
* @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 {Boolean} [aOptions.overwrite=false] true overwrites a ping with the same name,
* if found.
* @param {Object} [aOptions.overrideEnvironment=null] set to override the environment data.
+ * @param {String} [aOptions.overridePingId=null] set to override the
+ * generated ping id.
*
* @returns {Promise} A promise that resolves with the ping id when the ping is saved to
* disk.
*/
addPendingPing(aType, aPayload, aOptions = {}) {
let options = aOptions;
options.addClientId = aOptions.addClientId || false;
options.addEnvironment = aOptions.addEnvironment || false;
@@ -281,16 +285,18 @@ this.TelemetryController = Object.freeze
* @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 {Boolean} [aOptions.overwrite=false] true overwrites a ping with the same name,
* if found.
* @param {Object} [aOptions.overrideEnvironment=null] set to override the environment data.
+ * @param {String} [aOptions.overridePingId=null] set to override the
+ * generated ping id.
*
* @returns {Promise} A promise that resolves with the ping id when the ping is saved to
* disk.
*/
savePing(aType, aPayload, aFilePath, aOptions = {}) {
let options = aOptions;
options.addClientId = aOptions.addClientId || false;
options.addEnvironment = aOptions.addEnvironment || false;
@@ -390,31 +396,33 @@ 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 true if the ping should contain the client
* id, false otherwise.
* @param {Boolean} aOptions.addEnvironment true if the ping should contain the
* environment data.
* @param {Object} [aOptions.overrideEnvironment=null] set to override the environment data.
+ * @param {String} [aOptions.overridePingId=null] set to override the
+ * generated ping id.
*
* @returns {Object} An object that contains the assembled ping data.
*/
assemblePing: function assemblePing(aType, aPayload, aOptions = {}) {
this._log.trace("assemblePing - Type " + aType + ", aOptions " + JSON.stringify(aOptions));
// Clone the payload data so we don't race against unexpected changes in subobjects that are
// still referenced by other code.
// We can't trust all callers to do this properly on their own.
let payload = Cu.cloneInto(aPayload, myScope);
// Fill the common ping fields.
let pingData = {
type: aType,
- id: Policy.generatePingId(),
+ id: aOptions.overridePingId || Policy.generatePingId(),
creationDate: (Policy.now()).toISOString(),
version: PING_FORMAT_VERSION,
application: this._getApplicationSection(),
payload,
};
if (aOptions.addClientId) {
pingData.clientId = this._clientID;
@@ -446,16 +454,18 @@ 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 {String} [aOptions.overridePingId=null] set to override the
+ * generated ping id.
* @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();
@@ -484,16 +494,18 @@ 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 {String} [aOptions.overridePingId=null] set to override the
+ * generated ping id.
* @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;
@@ -529,16 +541,18 @@ var Impl = {
* @param {Object} aPayload The actual data payload for the ping.
* @param {Object} aOptions Options object.
* @param {Boolean} aOptions.addClientId true if the ping should contain the client id,
* false otherwise.
* @param {Boolean} aOptions.addEnvironment true if the ping should contain the
* environment data.
* @param {Boolean} aOptions.overwrite true overwrites a ping with the same name, if found.
* @param {Object} [aOptions.overrideEnvironment=null] set to override the environment data.
+ * @param {String} [aOptions.overridePingId=null] set to override the
+ * generated ping id.
*
* @returns {Promise} A promise that resolves with the ping id when the ping is saved to
* disk.
*/
addPendingPing: function addPendingPing(aType, aPayload, aOptions) {
this._log.trace("addPendingPing - Type " + aType + ", aOptions " + JSON.stringify(aOptions));
let pingData = this.assemblePing(aType, aPayload, aOptions);
@@ -565,16 +579,18 @@ var Impl = {
* @param {String} aFilePath The path to save the ping to.
* @param {Object} aOptions Options object.
* @param {Boolean} aOptions.addClientId true if the ping should contain the client id,
* false otherwise.
* @param {Boolean} aOptions.addEnvironment true if the ping should contain the
* environment data.
* @param {Boolean} aOptions.overwrite true overwrites a ping with the same name, if found.
* @param {Object} [aOptions.overrideEnvironment=null] set to override the environment data.
+ * @param {String} [aOptions.overridePingId=null] set to override the
+ * generated ping id.
*
* @returns {Promise} A promise that resolves with the ping id when the ping is saved to
* disk.
*/
savePing: function savePing(aType, aPayload, aFilePath, aOptions) {
this._log.trace("savePing - Type " + aType + ", File Path " + aFilePath +
", aOptions " + JSON.stringify(aOptions));
let pingData = this.assemblePing(aType, aPayload, aOptions);
--- a/toolkit/components/telemetry/tests/unit/head.js
+++ b/toolkit/components/telemetry/tests/unit/head.js
@@ -291,19 +291,28 @@ function getSnapshot(histogramId) {
}
// Helper for setting an empty list of Environment preferences to watch.
function setEmptyPrefWatchlist() {
let TelemetryEnvironment =
Cu.import("resource://gre/modules/TelemetryEnvironment.jsm").TelemetryEnvironment;
return TelemetryEnvironment.onInitialized().then(() => {
TelemetryEnvironment.testWatchPreferences(new Map());
+
});
}
+// Generate a UUID, used for the ping ID
+function generateUUID() {
+ let str = Cc["@mozilla.org/uuid-generator;1"]
+ .getService(Ci.nsIUUIDGenerator).generateUUID().toString();
+ // strip {}
+ return str.substring(1, str.length - 1);
+}
+
if (runningInParent) {
// Set logging preferences for all the tests.
Services.prefs.setCharPref("toolkit.telemetry.log.level", "Trace");
// Telemetry archiving should be on.
Services.prefs.setBoolPref("toolkit.telemetry.archive.enabled", true);
// Telemetry xpcshell tests cannot show the infobar.
Services.prefs.setBoolPref("datareporting.policy.dataSubmissionPolicyBypassNotification", true);
// FHR uploads should be enabled.
--- a/toolkit/components/telemetry/tests/unit/test_TelemetryController.js
+++ b/toolkit/components/telemetry/tests/unit/test_TelemetryController.js
@@ -30,26 +30,27 @@ const APP_NAME = "XPCShell";
const PREF_BRANCH = "toolkit.telemetry.";
const PREF_ENABLED = PREF_BRANCH + "enabled";
const PREF_ARCHIVE_ENABLED = PREF_BRANCH + "archive.enabled";
const PREF_FHR_UPLOAD_ENABLED = "datareporting.healthreport.uploadEnabled";
const PREF_UNIFIED = PREF_BRANCH + "unified";
var gClientID = null;
-function sendPing(aSendClientId, aSendEnvironment) {
+function sendPing(aSendClientId, aSendEnvironment, aOverridePingId) {
if (PingServer.started) {
TelemetrySend.setServer("http://localhost:" + PingServer.port);
} else {
TelemetrySend.setServer("http://doesnotexist");
}
let options = {
addClientId: aSendClientId,
addEnvironment: aSendEnvironment,
+ overridePingId: aOverridePingId || null,
};
return TelemetryController.submitExternalPing(TEST_PING_TYPE, {}, options);
}
function checkPingFormat(aPing, aType, aHasClientId, aHasEnvironment) {
const MANDATORY_PING_FIELDS = [
"type", "id", "creationDate", "version", "application", "payload"
];
@@ -281,16 +282,28 @@ add_task(function* test_pingHasEnvironme
checkPingFormat(ping, TEST_PING_TYPE, true, true);
// Test a field in the environment build section.
Assert.equal(ping.application.buildId, ping.environment.build.buildId);
// Test that we have the correct clientId.
Assert.equal(ping.clientId, gClientID, "The correct clientId must be reported.");
});
+add_task(function* test_pingIdCanBeOverridden() {
+ // Send a ping with an overridden id
+ const myPingId = generateUUID();
+ yield sendPing(/* aSendClientId */ false,
+ /* aSendEnvironment */ false,
+ myPingId);
+ let ping = yield PingServer.promiseNextPing();
+ checkPingFormat(ping, TEST_PING_TYPE, false, false);
+
+ Assert.equal(ping.id, myPingId, "The ping id must be the one we provided.");
+});
+
add_task(function* test_archivePings() {
let now = new Date(2009, 10, 18, 12, 0, 0);
fakeNow(now);
// Disable ping upload so that pings don't get sent.
// With unified telemetry the FHR upload pref controls this,
// with non-unified telemetry the Telemetry enabled pref.
const isUnified = Preferences.get(PREF_UNIFIED, false);
--- a/toolkit/components/telemetry/tests/unit/test_TelemetrySession.js
+++ b/toolkit/components/telemetry/tests/unit/test_TelemetrySession.js
@@ -69,22 +69,16 @@ const ABORTED_SESSION_UPDATE_INTERVAL_MS
XPCOMUtils.defineLazyGetter(this, "DATAREPORTING_PATH", function() {
return OS.Path.join(OS.Constants.Path.profileDir, DATAREPORTING_DIR);
});
var gClientID = null;
var gMonotonicNow = 0;
-function generateUUID() {
- let str = Cc["@mozilla.org/uuid-generator;1"].getService(Ci.nsIUUIDGenerator).generateUUID().toString();
- // strip {}
- return str.substring(1, str.length - 1);
-}
-
function truncateDateToDays(date) {
return new Date(date.getFullYear(),
date.getMonth(),
date.getDate(),
0, 0, 0, 0);
}
function sendPing() {