Bug 994707 - Introduce crash submission events and handle them. r=bsm…

…edberg

This introduces a new crash event type for submissions and handles
them in the Crash Manager. These crash submissions will also be counted
in FHR.

services/healthreport/docs/dataformat.rst

@@ -1073,6 +1073,35 @@ org.mozilla.crashes.crashes
 
 This measurement contains a historical record of application crashes.
 
+Version 4
+^^^^^^^^^
+
+This version follows up from version 3, adding submissions which are now
+tracked by the :ref:`crashes_crashmanager`.
+
+This measurement will be reported on each day there was a crash or crash
+submission. Records may contain the following fields, whose values indicate
+the number of crashes, hangs, or submissions that occurred on the given day:
+
+* main-crash
+* main-crash-submission-succeeded
+* main-crash-submission-failed
+* main-hang
+* main-hang-submission-succeeded
+* main-hang-submission-failed
+* content-crash
+* content-crash-submission-succeeded
+* content-crash-submission-failed
+* content-hang
+* content-hang-submission-succeeded
+* content-hang-submission-failed
+* plugin-crash
+* plugin-crash-submission-succeeded
+* plugin-crash-submission-failed
+* plugin-hang
+* plugin-hang-submission-succeeded
+* plugin-hang-submission-failed
+
 Version 3
 ^^^^^^^^^
 
@@ -1152,6 +1181,14 @@ Example
       "_v": 2,
       "mainCrash": 2
     }
+    "org.mozilla.crashes.crashes": {
+      "_v": 4,
+      "main-crash": 2,
+      "main-crash-submission-succeeded": 1,
+      "main-crash-submission-failed": 1,
+      "main-hang": 1,
+      "plugin-crash": 2
+    }
 
 org.mozilla.healthreport.submissions
 ------------------------------------

services/healthreport/providers.jsm

@@ -1047,6 +1047,38 @@ DailyCrashesMeasurement3.prototype = Object.freeze({
   },
 });
 
+function DailyCrashesMeasurement4() {
+  Metrics.Measurement.call(this);
+}
+
+DailyCrashesMeasurement4.prototype = Object.freeze({
+  __proto__: Metrics.Measurement.prototype,
+
+  name: "crashes",
+  version: 4,
+
+  fields: {
+    "main-crash": DAILY_LAST_NUMERIC_FIELD,
+    "main-crash-submission-succeeded": DAILY_LAST_NUMERIC_FIELD,
+    "main-crash-submission-failed": DAILY_LAST_NUMERIC_FIELD,
+    "main-hang": DAILY_LAST_NUMERIC_FIELD,
+    "main-hang-submission-succeeded": DAILY_LAST_NUMERIC_FIELD,
+    "main-hang-submission-failed": DAILY_LAST_NUMERIC_FIELD,
+    "content-crash": DAILY_LAST_NUMERIC_FIELD,
+    "content-crash-submission-succeeded": DAILY_LAST_NUMERIC_FIELD,
+    "content-crash-submission-failed": DAILY_LAST_NUMERIC_FIELD,
+    "content-hang": DAILY_LAST_NUMERIC_FIELD,
+    "content-hang-submission-succeeded": DAILY_LAST_NUMERIC_FIELD,
+    "content-hang-submission-failed": DAILY_LAST_NUMERIC_FIELD,
+    "plugin-crash": DAILY_LAST_NUMERIC_FIELD,
+    "plugin-crash-submission-succeeded": DAILY_LAST_NUMERIC_FIELD,
+    "plugin-crash-submission-failed": DAILY_LAST_NUMERIC_FIELD,
+    "plugin-hang": DAILY_LAST_NUMERIC_FIELD,
+    "plugin-hang-submission-succeeded": DAILY_LAST_NUMERIC_FIELD,
+    "plugin-hang-submission-failed": DAILY_LAST_NUMERIC_FIELD,
+  },
+});
+
 this.CrashesProvider = function () {
   Metrics.Provider.call(this);
 
@@ -1063,6 +1095,7 @@ CrashesProvider.prototype = Object.freeze({
     DailyCrashesMeasurement1,
     DailyCrashesMeasurement2,
     DailyCrashesMeasurement3,
+    DailyCrashesMeasurement4,
   ],
 
   pullOnly: true,
@@ -1075,8 +1108,8 @@ CrashesProvider.prototype = Object.freeze({
     this._log.info("Grabbing crash counts from crash manager.");
     let crashCounts = yield this._manager.getCrashCountsByDay();
 
-    let m = this.getMeasurement("crashes", 3);
-    let fields = DailyCrashesMeasurement3.prototype.fields;
+    let m = this.getMeasurement("crashes", 4);
+    let fields = DailyCrashesMeasurement4.prototype.fields;
 
     for (let [day, types] of crashCounts) {
       let date = Metrics.daysToDate(day);

services/healthreport/tests/xpcshell/test_provider_crashes.js

@@ -50,9 +50,17 @@ add_task(function* test_collect() {
   yield manager.addCrash(manager.PROCESS_TYPE_MAIN,
                          manager.CRASH_TYPE_CRASH,
                          "mc1", day1);
+  yield manager.addSubmission(manager.PROCESS_TYPE_MAIN,
+                              manager.CRASH_TYPE_CRASH,
+                              true,
+                              "mc1", day1)
   yield manager.addCrash(manager.PROCESS_TYPE_MAIN,
                          manager.CRASH_TYPE_CRASH,
                          "mc2", day1);
+  yield manager.addSubmission(manager.PROCESS_TYPE_MAIN,
+                              manager.CRASH_TYPE_CRASH,
+                              false,
+                              "mc2", day1)
   yield manager.addCrash(manager.PROCESS_TYPE_CONTENT,
                          manager.CRASH_TYPE_HANG,
                          "ch", day1);
@@ -66,13 +74,17 @@ add_task(function* test_collect() {
   yield manager.addCrash(manager.PROCESS_TYPE_CONTENT,
                          manager.CRASH_TYPE_CRASH,
                          "cc", day2);
+  yield manager.addSubmission(manager.PROCESS_TYPE_CONTENT,
+                              manager.CRASH_TYPE_CRASH,
+                              true,
+                              "cc", day2)
   yield manager.addCrash(manager.PROCESS_TYPE_PLUGIN,
                          manager.CRASH_TYPE_HANG,
                          "ph", day2);
 
   yield provider.collectDailyData();
 
-  let m = provider.getMeasurement("crashes", 3);
+  let m = provider.getMeasurement("crashes", 4);
   let values = yield m.getValues();
   do_check_eq(values.days.size, 2);
   do_check_true(values.days.hasDay(day1));
@@ -81,6 +93,10 @@ add_task(function* test_collect() {
   let value = values.days.getDay(day1);
   do_check_true(value.has("main-crash"));
   do_check_eq(value.get("main-crash"), 2);
+  do_check_true(value.has("main-crash-submission-succeeded"));
+  do_check_eq(value.get("main-crash-submission-succeeded"), 1);
+  do_check_true(value.has("main-crash-submission-failed"));
+  do_check_eq(value.get("main-crash-submission-failed"), 1);
   do_check_true(value.has("content-hang"));
   do_check_eq(value.get("content-hang"), 1);
   do_check_true(value.has("plugin-crash"));
@@ -91,6 +107,8 @@ add_task(function* test_collect() {
   do_check_eq(value.get("main-hang"), 1);
   do_check_true(value.has("content-crash"));
   do_check_eq(value.get("content-crash"), 1);
+  do_check_true(value.has("content-crash-submission-succeeded"));
+  do_check_eq(value.get("content-crash-submission-succeeded"), 1);
   do_check_true(value.has("plugin-hang"));
   do_check_eq(value.get("plugin-hang"), 1);
 

toolkit/components/crashes/CrashManager.jsm

@@ -130,12 +130,21 @@ this.CrashManager.prototype = Object.freeze({
   // A crash in a plugin process.
   PROCESS_TYPE_PLUGIN: "plugin",
 
+  // A submission of a crash.
+  PROCESS_TYPE_SUBMISSION: "submission",
+
   // A real crash.
   CRASH_TYPE_CRASH: "crash",
 
   // A hang.
   CRASH_TYPE_HANG: "hang",
 
+  // A successful submission.
+  SUBMISSION_TYPE_SUCCEEDED: "succeeded",
+
+  // A failed submission.
+  SUBMISSION_TYPE_FAILED: "failed",
+
   DUMP_REGEX: /^([0-9a-f]{8}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{12})\.dmp$/i,
   SUBMITTED_REGEX: /^bp-(?:hr-)?([0-9a-f]{8}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{12})\.txt$/i,
   ALL_REGEX: /^(.*)$/,
@@ -360,6 +369,38 @@ this.CrashManager.prototype = Object.freeze({
     }.bind(this));
   },
 
+  /**
+   * Record the occurrence of a crash submission.
+   *
+   * @param processType (string) One of the PROCESS_TYPE constants.
+   * @param crashType (string) One of the CRASH_TYPE constants.
+   * @param succeeded (boolean) Whether the submission succeeded.
+   * @param id (string) Crash ID. Likely a UUID.
+   * @param date (Date) When the crash occurred.
+   *
+   * @return boolean True if the crash submission was recorded and false if not.
+   */
+  addSubmission: function (processType, crashType, succeeded, id, date) {
+    return Task.spawn(function* () {
+      let store = yield this._getStore();
+      if (this._addSubmissionAsCrash(store, processType, crashType, succeeded,
+                                     id, date)) {
+        yield store.save();
+      }
+    }.bind(this));
+  },
+
+  _addSubmissionAsCrash: function (store, processType, crashType, succeeded,
+                                   id, date) {
+    let id = id + "-" + this.PROCESS_TYPE_SUBMISSION;
+    let process = processType + "-" + crashType + "-" +
+                  this.PROCESS_TYPE_SUBMISSION;
+    let submission_type = (
+      succeeded ? this.SUBMISSION_TYPE_SUCCEEDED : this.SUBMISSION_TYPE_FAILED);
+
+    return store.addCrash(process, submission_type, id, date);
+  },
+
   /**
    * Obtain the paths of all unprocessed events files.
    *
@@ -425,27 +466,35 @@ this.CrashManager.prototype = Object.freeze({
       // 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.
+      // DO NOT ADD NEW TYPES WITHOUT DOCUMENTING!
+      let lines = payload.split("\n");
+
+      switch (type) {
+        case "crash.main.1":
+          if (lines.length > 1) {
+            this._log.warn("Multiple lines unexpected in payload for " +
+                           entry.path);
+            return this.EVENT_FILE_ERROR_MALFORMED;
+          }
+          store.addCrash(this.PROCESS_TYPE_MAIN, this.CRASH_TYPE_CRASH,
+                         payload, date);
+          break;
+
+        case "crash.submission.1":
+          if (lines.length == 3) {
+            this._addSubmissionAsCrash(store, this.PROCESS_TYPE_MAIN,
+                                       this.CRASH_TYPE_CRASH,
+                                       lines[1] === "true", lines[0], date);
+          } else {
+            return this.EVENT_FILE_ERROR_MALFORMED;
+          }
+          break;
 
-      // type in event file => [processType, crashType]
-      let eventMap = {
-        "crash.main.1": ["main", "crash"],
-      };
-
-      if (type in eventMap) {
-        let lines = payload.split("\n");
-        if (lines.length > 1) {
-          this._log.warn("Multiple lines unexpected in payload for " +
-                         entry.path);
-          return this.EVENT_FILE_ERROR_MALFORMED;
-        }
-
-        store.addCrash(...eventMap[type], payload, date);
-        return this.EVENT_FILE_SUCCESS;
+        default:
+          return this.EVENT_FILE_ERROR_UNKNOWN_EVENT;
       }
 
-      // DO NOT ADD NEW TYPES WITHOUT DOCUMENTING!
-
-      return this.EVENT_FILE_ERROR_UNKNOWN_EVENT;
+      return this.EVENT_FILE_SUCCESS;
   },
 
   /**

toolkit/components/crashes/docs/crash-events.rst

@@ -74,6 +74,18 @@ The payload of this event is the string crash ID, very likely a UUID.
 There should be ``UUID.dmp`` and ``UUID.extra`` files on disk, saved by
 Breakpad.
 
+crash.submission.1
+^^^^^^^^^^^^
+
+This event is produced when a crash is submitted.
+
+The payload of this event is delimited by UNIX newlines (*\n*) and contains the
+following fields:
+
+* The crash ID string
+* "true" if the submission succeeded or "false" otherwise
+* The remote crash ID string if the submission succeeded
+
 Aggregated Event Log
 ====================
 

toolkit/components/crashes/tests/xpcshell/test_crash_manager.js

@@ -304,3 +304,42 @@ add_task(function* test_addCrash() {
   Assert.equal(crash.type, m.PROCESS_TYPE_PLUGIN + "-" + m.CRASH_TYPE_HANG);
   Assert.ok(crash.isOfType(m.PROCESS_TYPE_PLUGIN, m.CRASH_TYPE_HANG));
 });
+
+add_task(function* test_addSubmission() {
+  let m = yield getManager();
+
+  let crashes = yield m.getCrashes();
+  Assert.equal(crashes.length, 0);
+
+  yield m.addSubmission(m.PROCESS_TYPE_MAIN, m.CRASH_TYPE_CRASH, true,
+                        "success", DUMMY_DATE);
+  yield m.addSubmission(m.PROCESS_TYPE_MAIN, m.CRASH_TYPE_CRASH, false,
+                        "failure", DUMMY_DATE);
+
+  crashes = yield m.getCrashes();
+  Assert.equal(crashes.length, 2);
+
+  let map = new Map(crashes.map(crash => [crash.id, crash]));
+
+  let crash = map.get("success-submission");
+  Assert.ok(!!crash);
+  Assert.equal(crash.crashDate, DUMMY_DATE);
+  Assert.equal(crash.type,
+               m.PROCESS_TYPE_MAIN + "-" + m.CRASH_TYPE_CRASH + "-" +
+               m.PROCESS_TYPE_SUBMISSION + "-" + m.SUBMISSION_TYPE_SUCCEEDED);
+  Assert.ok(
+    crash.isOfType(m.PROCESS_TYPE_MAIN + "-" + m.CRASH_TYPE_CRASH + "-" +
+                   m.PROCESS_TYPE_SUBMISSION, m.SUBMISSION_TYPE_SUCCEEDED));
+
+  let crash = map.get("failure-submission");
+  Assert.ok(!!crash);
+  Assert.equal(crash.crashDate, DUMMY_DATE);
+  Assert.equal(crash.type,
+               m.PROCESS_TYPE_MAIN + "-" + m.CRASH_TYPE_CRASH + "-" +
+               m.PROCESS_TYPE_SUBMISSION + "-" + m.SUBMISSION_TYPE_FAILED);
+  Assert.ok(
+    crash.isOfType(m.PROCESS_TYPE_MAIN + "-" + m.CRASH_TYPE_CRASH + "-" +
+                   m.PROCESS_TYPE_SUBMISSION, m.SUBMISSION_TYPE_FAILED));
+
+});
+

toolkit/components/crashes/tests/xpcshell/test_crash_store.js

@@ -17,8 +17,11 @@ const {
   PROCESS_TYPE_MAIN,
   PROCESS_TYPE_CONTENT,
   PROCESS_TYPE_PLUGIN,
+  PROCESS_TYPE_SUBMISSION,
   CRASH_TYPE_CRASH,
   CRASH_TYPE_HANG,
+  SUBMISSION_TYPE_SUCCEEDED,
+  SUBMISSION_TYPE_FAILED,
 } = CrashManager.prototype;
 
 const CrashStore = bsp.CrashStore;
@@ -273,6 +276,44 @@ add_task(function* test_add_plugin_hang() {
   Assert.equal(crashes.length, 2);
 });
 
+add_task(function* test_add_submission() {
+  let s = yield getStore();
+
+  Assert.ok(
+    s.addCrash(PROCESS_TYPE_MAIN + "-" + CRASH_TYPE_CRASH + "-" +
+               PROCESS_TYPE_SUBMISSION, SUBMISSION_TYPE_SUCCEEDED,
+               "id1", new Date())
+  );
+  Assert.equal(s.crashesCount, 1);
+
+  let c = s.crashes[0];
+  Assert.ok(c.crashDate);
+  Assert.equal(c.type, PROCESS_TYPE_MAIN + "-" + CRASH_TYPE_CRASH + "-" +
+               PROCESS_TYPE_SUBMISSION + "-" + SUBMISSION_TYPE_SUCCEEDED);
+  Assert.ok(c.isOfType(PROCESS_TYPE_MAIN + "-" + CRASH_TYPE_CRASH + "-" +
+                       PROCESS_TYPE_SUBMISSION, SUBMISSION_TYPE_SUCCEEDED));
+
+  Assert.ok(
+    s.addCrash(PROCESS_TYPE_MAIN + "-" + CRASH_TYPE_CRASH + "-" +
+               PROCESS_TYPE_SUBMISSION, SUBMISSION_TYPE_FAILED,
+               "id2", new Date())
+  );
+  Assert.equal(s.crashesCount, 2);
+
+  // Duplicate.
+  Assert.ok(
+    s.addCrash(PROCESS_TYPE_MAIN + "-" + CRASH_TYPE_CRASH + "-" +
+               PROCESS_TYPE_SUBMISSION, SUBMISSION_TYPE_SUCCEEDED,
+               "id1", new Date())
+  );
+  Assert.equal(s.crashesCount, 2);
+
+  let crashes = s.getCrashesOfType(PROCESS_TYPE_MAIN + "-" + CRASH_TYPE_CRASH +
+                                   "-" + PROCESS_TYPE_SUBMISSION,
+                                   SUBMISSION_TYPE_SUCCEEDED);
+  Assert.equal(crashes.length, 1);
+});
+
 add_task(function* test_add_mixed_types() {
   let s = yield getStore();