Bug 1301289 - send accumulated sync pings earlier in the shutdown process. r=tcsc
authorMark Hammond <mhammond@skippinet.com.au>
Thu, 08 Sep 2016 15:05:11 +1000
changeset 354549 1a47959fb817af04e872fc0db67bf7fe08038645
parent 354548 c82d3d5d08864a2d437db3c1ae9a669249d36575
child 354550 cb23a7c310c8184550a84a3eef40a3adbef77781
push id6570
push userraliiev@mozilla.com
push dateMon, 14 Nov 2016 12:26:13 +0000
treeherdermozilla-beta@f455459b2ae5 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerstcsc
bugs1301289
milestone51.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1301289 - send accumulated sync pings earlier in the shutdown process. r=tcsc MozReview-Commit-ID: Ls6HeieFi1H
services/sync/modules/telemetry.js
services/sync/tests/unit/head_helpers.js
--- a/services/sync/modules/telemetry.js
+++ b/services/sync/modules/telemetry.js
@@ -28,17 +28,17 @@ Cu.import("resource://gre/modules/FxAcco
 
 XPCOMUtils.defineLazyServiceGetter(this, "Telemetry",
                                    "@mozilla.org/base/telemetry;1",
                                    "nsITelemetry");
 
 const log = Log.repository.getLogger("Sync.Telemetry");
 
 const TOPICS = [
-  "xpcom-shutdown",
+  "profile-before-change",
   "weave:service:sync:start",
   "weave:service:sync:finish",
   "weave:service:sync:error",
 
   "weave:engine:sync:start",
   "weave:engine:sync:finish",
   "weave:engine:sync:error",
   "weave:engine:sync:applied",
@@ -356,16 +356,17 @@ class SyncTelemetryImpl {
       Observers.remove(topic, this, this);
     }
   }
 
   submit(record) {
     // We still call submit() with possibly illegal payloads so that tests can
     // know that the ping was built. We don't end up submitting them, however.
     if (record.syncs.length) {
+      log.trace(`submitting ${record.syncs.length} sync record(s) to telemetry`);
       TelemetryController.submitExternalPing("sync", record);
     }
   }
 
 
   onSyncStarted() {
     if (this.current) {
       log.warn("Observed weave:service:sync:start, but we're already recording a sync!");
@@ -400,17 +401,17 @@ class SyncTelemetryImpl {
       this.lastSubmissionTime = Telemetry.msSinceProcessStart();
     }
   }
 
   observe(subject, topic, data) {
     log.trace(`observed ${topic} ${data}`);
 
     switch (topic) {
-      case "xpcom-shutdown":
+      case "profile-before-change":
         this.shutdown();
         break;
 
       /* sync itself state changes */
       case "weave:service:sync:start":
         this.onSyncStarted();
         break;
 
--- a/services/sync/tests/unit/head_helpers.js
+++ b/services/sync/tests/unit/head_helpers.js
@@ -247,17 +247,21 @@ function get_sync_test_telemetry() {
   for (let engineName of testEngines) {
     ns.SyncTelemetry.allowedEngines.add(engineName);
   }
   ns.SyncTelemetry.submissionInterval = -1;
   return ns.SyncTelemetry;
 }
 
 function assert_valid_ping(record) {
-  if (record) {
+  // This is called as the test harness tears down due to shutdown. This
+  // will typically have no recorded syncs, and the validator complains about
+  // it. So ignore such records (but only ignore when *both* shutdown and
+  // no Syncs - either of them not being true might be an actual problem)
+  if (record && (record.why != "shutdown" || record.syncs.length != 0)) {
     if (!SyncPingValidator(record)) {
       deepEqual([], SyncPingValidator.errors, "Sync telemetry ping validation failed");
     }
     equal(record.version, 1);
     record.syncs.forEach(p => {
       lessOrEqual(p.when, Date.now());
     });
   }