Bug 1301289 - send accumulated sync pings earlier in the shutdown process. r=tcsc
MozReview-Commit-ID: Ls6HeieFi1H
--- 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());
});
}