--- a/browser/app/profile/firefox.js
+++ b/browser/app/profile/firefox.js
@@ -1521,8 +1521,14 @@ pref("browser.crashReports.unsubmittedCh
#endif
// chancesUntilSuppress is how many times we'll show the unsubmitted
// crash report notification across different days and shutdown
// without a user choice before we suppress the notification for
// some number of days.
pref("browser.crashReports.unsubmittedCheck.chancesUntilSuppress", 4);
pref("browser.crashReports.unsubmittedCheck.autoSubmit", false);
+
+#ifdef NIGHTLY_BUILD
+// Enable the (fairly costly) client/server validation on nightly only. The other prefs
+// controlling validation are located in /services/sync/services-sync.js
+pref("services.sync.validation.enabled", true);
+#endif
--- a/services/sync/modules/bookmark_validator.js
+++ b/services/sync/modules/bookmark_validator.js
@@ -2,16 +2,17 @@
* License, v. 2.0. If a copy of the MPL was not distributed with this file,
* You can obtain one at http://mozilla.org/MPL/2.0/. */
"use strict";
const Cu = Components.utils;
Cu.import("resource://gre/modules/PlacesUtils.jsm");
+Cu.import("resource://gre/modules/Task.jsm");
Cu.import("resource://services-sync/util.js");
Cu.import("resource://services-sync/bookmark_utils.js");
this.EXPORTED_SYMBOLS = ["BookmarkValidator", "BookmarkProblemData"];
/**
* Result of bookmark validation. Contains the following fields which describe
* server-side problems unless otherwise specified.
@@ -77,24 +78,44 @@ class BookmarkProblemData {
this.serverMissing = [];
this.serverDeleted = [];
this.serverUnexpected = [];
this.differences = [];
this.structuralDifferences = [];
}
/**
+ * Convert ("difference", [{ differences: ["tags", "name"] }, { differences: ["name"] }]) into
+ * [{ name: "difference:tags", count: 1}, { name: "difference:name", count: 2 }], etc.
+ */
+ _summarizeDifferences(prefix, diffs) {
+ let diffCounts = new Map();
+ for (let { differences } of diffs) {
+ for (let type of differences) {
+ let name = prefix + ":" + type;
+ let count = diffCounts.get(name) || 0;
+ diffCounts.set(name, count + 1);
+ }
+ }
+ return [...diffCounts].map(([name, count]) => ({ name, count }));
+ }
+
+ /**
* Produce a list summarizing problems found. Each entry contains {name, count},
* where name is the field name for the problem, and count is the number of times
* the problem was encountered.
*
* Validation has failed if all counts are not 0.
+ *
+ * If the `full` argument is truthy, we also include information about which
+ * properties we saw structural differences in. Currently, this means either
+ * "sdiff:parentid" and "sdiff:childGUIDS" may be present.
*/
- getSummary() {
- return [
+ getSummary(full) {
+ let result = [
{ name: "clientMissing", count: this.clientMissing.length },
{ name: "serverMissing", count: this.serverMissing.length },
{ name: "serverDeleted", count: this.serverDeleted.length },
{ name: "serverUnexpected", count: this.serverUnexpected.length },
{ name: "structuralDifferences", count: this.structuralDifferences.length },
{ name: "differences", count: this.differences.length },
@@ -110,16 +131,21 @@ class BookmarkProblemData {
{ name: "deletedChildren", count: this.deletedChildren.length },
{ name: "multipleParents", count: this.multipleParents.length },
{ name: "deletedParents", count: this.deletedParents.length },
{ name: "childrenOnNonFolder", count: this.childrenOnNonFolder.length },
{ name: "duplicateChildren", count: this.duplicateChildren.length },
{ name: "parentNotFolder", count: this.parentNotFolder.length },
{ name: "wrongParentName", count: this.wrongParentName.length },
];
+ if (full) {
+ let structural = this._summarizeDifferences("sdiff", this.structuralDifferences);
+ result.push.apply(result, structural);
+ }
+ return result;
}
}
class BookmarkValidator {
_followQueries(recordMap) {
for (let [guid, entry] of recordMap) {
if (entry.type !== "query" && (!entry.bmkUri || !entry.bmkUri.startsWith("place:"))) {
@@ -688,12 +714,46 @@ class BookmarkValidator {
problemData.differences.push({id, differences});
}
if (structuralDifferences.length) {
problemData.structuralDifferences.push({ id, differences: structuralDifferences });
}
}
return inspectionInfo;
}
+
+ _getServerState(engine) {
+ let collection = engine.itemSource();
+ let collectionKey = engine.service.collectionKeys.keyForCollection(engine.name);
+ collection.full = true;
+ let items = [];
+ collection.recordHandler = function(item) {
+ item.decrypt(collectionKey);
+ items.push(item.cleartext);
+ };
+ collection.get();
+ return items;
+ }
+
+ validate(engine) {
+ let self = this;
+ return Task.spawn(function*() {
+ let start = Date.now();
+ let clientTree = yield PlacesUtils.promiseBookmarksTree("", {
+ includeItemIds: true
+ });
+ let serverState = self._getServerState(engine);
+ let serverRecordCount = serverState.length;
+ let result = self.compareServerWithClient(serverState, clientTree);
+ let end = Date.now();
+ let duration = end-start;
+ return {
+ duration,
+ problems: result.problemData,
+ recordCount: serverRecordCount
+ };
+ });
+ }
+
};
--- a/services/sync/modules/engines.js
+++ b/services/sync/modules/engines.js
@@ -706,16 +706,25 @@ Engine.prototype = {
this._tracker.ignoreAll = true;
this._store.wipe();
this._tracker.ignoreAll = false;
this._tracker.clearChangedIDs();
},
wipeClient: function () {
this._notify("wipe-client", this.name, this._wipeClient)();
+ },
+
+ /**
+ * If one exists, initialize and return a validator for this engine (which
+ * must have a `validate(engine)` method that returns a promise to an object
+ * with a getSummary method). Otherwise return null.
+ */
+ getValidator: function () {
+ return null;
}
};
this.SyncEngine = function SyncEngine(name, service) {
Engine.call(this, name || "SyncEngine", service);
this.loadToFetch();
this.loadPreviousFailed();
@@ -956,34 +965,34 @@ SyncEngine.prototype = {
// Keep track of what to delete at the end of sync
this._delete = {};
},
/**
* A tiny abstraction to make it easier to test incoming record
* application.
*/
- _itemSource: function () {
+ itemSource: function () {
return new Collection(this.engineURL, this._recordObj, this.service);
},
/**
* Process incoming records.
* In the most awful and untestable way possible.
* This now accepts something that makes testing vaguely less impossible.
*/
_processIncoming: function (newitems) {
this._log.trace("Downloading & applying server changes");
// Figure out how many total items to fetch this sync; do less on mobile.
let batchSize = this.downloadLimit || Infinity;
let isMobile = (Svc.Prefs.get("client.type") == "mobile");
if (!newitems) {
- newitems = this._itemSource();
+ newitems = this.itemSource();
}
if (this._defaultSort) {
newitems.sort = this._defaultSort;
}
if (isMobile) {
batchSize = MOBILE_BATCH_SIZE;
--- a/services/sync/modules/engines/bookmarks.js
+++ b/services/sync/modules/engines/bookmarks.js
@@ -16,16 +16,18 @@ Cu.import("resource://gre/modules/XPCOMU
Cu.import("resource://services-common/async.js");
Cu.import("resource://services-sync/constants.js");
Cu.import("resource://services-sync/engines.js");
Cu.import("resource://services-sync/record.js");
Cu.import("resource://services-sync/util.js");
Cu.import("resource://services-sync/bookmark_utils.js");
Cu.import("resource://gre/modules/Task.jsm");
Cu.import("resource://gre/modules/PlacesBackups.jsm");
+XPCOMUtils.defineLazyModuleGetter(this, "BookmarkValidator",
+ "resource://services-sync/bookmark_validator.js");
const ANNOS_TO_TRACK = [BookmarkAnnos.DESCRIPTION_ANNO, BookmarkAnnos.SIDEBAR_ANNO,
PlacesUtils.LMANNO_FEEDURI, PlacesUtils.LMANNO_SITEURI];
const SERVICE_NOT_SUPPORTED = "Service not supported on this platform";
const FOLDER_SORTINDEX = 1000000;
const {
SOURCE_SYNC,
@@ -555,16 +557,19 @@ BookmarksEngine.prototype = {
}
}
// The local, duplicate ID is always deleted on the server - but for
// bookmarks it is a logical delete.
// Simply adding this (now non-existing) ID to the tracker is enough.
this._modified.set(localDupeGUID, { modified: now, deleted: true });
},
+ getValidator() {
+ return new BookmarkValidator();
+ }
};
function BookmarksStore(name, engine) {
Store.call(this, name, engine);
// Explicitly nullify our references to our cached services so we don't leak
Svc.Obs.add("places-shutdown", function() {
for (let query in this._stmts) {
--- a/services/sync/modules/stages/enginesync.js
+++ b/services/sync/modules/stages/enginesync.js
@@ -10,16 +10,19 @@ this.EXPORTED_SYMBOLS = ["EngineSynchron
var {utils: Cu} = Components;
Cu.import("resource://gre/modules/Log.jsm");
Cu.import("resource://services-sync/constants.js");
Cu.import("resource://services-sync/engines.js");
Cu.import("resource://services-sync/policies.js");
Cu.import("resource://services-sync/util.js");
+Cu.import("resource://services-common/observers.js");
+Cu.import("resource://services-common/async.js");
+Cu.import("resource://gre/modules/Task.jsm");
/**
* Perform synchronization of engines.
*
* This was originally split out of service.js. The API needs lots of love.
*/
this.EngineSynchronizer = function EngineSynchronizer(service) {
this._log = Log.repository.getLogger("Sync.Synchronizer");
@@ -153,22 +156,25 @@ EngineSynchronizer.prototype = {
if (allowEnginesHint && engineNamesToSync) {
this._log.info("Syncing specified engines", engineNamesToSync);
enginesToSync = engineManager.get(engineNamesToSync).filter(e => e.enabled);
} else {
this._log.info("Syncing all enabled engines.");
enginesToSync = engineManager.getEnabled();
}
try {
+ // We don't bother validating engines that failed to sync.
+ let enginesToValidate = [];
for (let engine of enginesToSync) {
// If there's any problems with syncing the engine, report the failure
if (!(this._syncEngine(engine)) || this.service.status.enforceBackoff) {
this._log.info("Aborting sync for failure in " + engine.name);
break;
}
+ enginesToValidate.push(engine);
}
// If _syncEngine fails for a 401, we might not have a cluster URL here.
// If that's the case, break out of this immediately, rather than
// throwing an exception when trying to fetch metaURL.
if (!this.service.clusterURL) {
this._log.debug("Aborting sync, no cluster URL: " +
"not uploading new meta/global.");
@@ -184,16 +190,18 @@ EngineSynchronizer.prototype = {
this.service.uploadMetaGlobal(meta);
delete meta.isNew;
delete meta.changed;
} catch (error) {
this._log.error("Unable to upload meta/global. Leaving marked as new.");
}
}
+ Async.promiseSpinningly(this._tryValidateEngines(enginesToValidate));
+
// If there were no sync engine failures
if (this.service.status.service != SYNC_FAILED_PARTIAL) {
Svc.Prefs.set("lastSync", new Date().toString());
this.service.status.sync = SYNC_SUCCEEDED;
}
} finally {
Svc.Prefs.reset("firstSync");
@@ -201,16 +209,116 @@ EngineSynchronizer.prototype = {
let dateStr = new Date().toLocaleFormat(LOG_DATE_FORMAT);
this._log.info("Sync completed at " + dateStr
+ " after " + syncTime + " secs.");
}
this.onComplete(null);
},
+ _tryValidateEngines: Task.async(function* (recentlySyncedEngines) {
+ if (!Services.telemetry.canRecordBase || !Svc.Prefs.get("validation.enabled", false)) {
+ this._log.info("Skipping validation: validation or telemetry reporting is disabled");
+ return;
+ }
+
+ let lastValidation = Svc.Prefs.get("validation.lastTime", 0);
+ let validationInterval = Svc.Prefs.get("validation.interval");
+ let nowSeconds = Math.floor(Date.now() / 1000);
+
+ if (nowSeconds - lastValidation < validationInterval) {
+ this._log.info("Skipping validation: too recent since last validation attempt");
+ return;
+ }
+ // Update the time now, even if we may return false still. We don't want to
+ // check the rest of these more frequently than once a day.
+ Svc.Prefs.set("validation.lastTime", nowSeconds);
+
+ // Validation only occurs a certain percentage of the time.
+ let validationProbability = Svc.Prefs.get("validation.percentageChance", 0) / 100.0;
+ if (validationProbability < Math.random()) {
+ this._log.info("Skipping validation: Probability threshold not met");
+ return;
+ }
+ let maxRecords = Svc.Prefs.get("validation.maxRecords");
+ if (!maxRecords) {
+ // Don't bother asking the server for the counts if we know validation
+ // won't happen anyway.
+ return;
+ }
+
+ // maxRecords of -1 means "any number", so we can skip asking the server.
+ // Used for tests.
+ let info;
+ if (maxRecords < 0) {
+ info = {};
+ for (let e of recentlySyncedEngines) {
+ info[e.name] = 1; // needs to be < maxRecords
+ }
+ maxRecords = 2;
+ } else {
+
+ let collectionCountsURL = this.service.userBaseURL + "info/collection_counts";
+ try {
+ let infoResp = this.service._fetchInfo(collectionCountsURL);
+ if (!infoResp.success) {
+ this._log.error("Can't run validation: request to info/collection_counts responded with "
+ + resp.status);
+ return;
+ }
+ info = infoResp.obj; // might throw because obj is a getter which parses json.
+ } catch (e) {
+ // Not running validation is totally fine, so we just write an error log and return.
+ this._log.error("Can't run validation: Caught error when fetching counts", e);
+ return;
+ }
+ }
+
+ if (!info) {
+ return;
+ }
+
+ let engineLookup = new Map(recentlySyncedEngines.map(e => [e.name, e]));
+ let toRun = [];
+ for (let [engineName, recordCount] of Object.entries(info)) {
+ let engine = engineLookup.get(engineName);
+ if (recordCount > maxRecords || !engine) {
+ this._log.debug(`Skipping validation for ${engineName} because it's not an engine or ` +
+ `the number of records (${recordCount}) is greater than the maximum allowed (${maxRecords}).`);
+ continue;
+ }
+ let validator = engine.getValidator();
+ if (!validator) {
+ continue;
+ }
+ // Put this in an array so that we know how many we're going to do, so we
+ // don't tell users we're going to run some validators when we aren't.
+ toRun.push({ engine, validator });
+ }
+
+ if (!toRun.length) {
+ return;
+ }
+ Services.console.logStringMessage(
+ "Sync is about to run a consistency check. This may be slow, and " +
+ "can be controlled using the pref \"services.sync.validation.enabled\".\n" +
+ "If you encounter any problems because of this, please file a bug.");
+ for (let { validator, engine } of toRun) {
+ try {
+ let result = yield validator.validate(engine);
+ Observers.notify("weave:engine:validate:finish", result, engine.name);
+ } catch (e) {
+ this._log.error(`Failed to run validation on ${engine.name}!`, e);
+ Observers.notify("weave:engine:validate:error", e, engine.name)
+ // Keep validating -- there's no reason to think that a failure for one
+ // validator would mean the others will fail.
+ }
+ }
+ }),
+
// Returns true if sync should proceed.
// false / no return value means sync should be aborted.
_syncEngine: function _syncEngine(engine) {
try {
engine.sync();
}
catch(e) {
if (e.status == 401) {
--- a/services/sync/modules/telemetry.js
+++ b/services/sync/modules/telemetry.js
@@ -38,16 +38,18 @@ const TOPICS = [
"weave:service:sync:finish",
"weave:service:sync:error",
"weave:engine:sync:start",
"weave:engine:sync:finish",
"weave:engine:sync:error",
"weave:engine:sync:applied",
"weave:engine:sync:uploaded",
+ "weave:engine:validate:finish",
+ "weave:engine:validate:error",
];
const PING_FORMAT_VERSION = 1;
// The set of engines we record telemetry for - any other engines are ignored.
const ENGINES = new Set(["addons", "bookmarks", "clients", "forms", "history",
"passwords", "prefs", "tabs"]);
@@ -161,16 +163,46 @@ class EngineRecord {
for (let property of properties) {
if (counts[property]) {
incomingData[property] = counts[property];
this.incoming = incomingData;
}
}
}
+ recordValidation(validationResult) {
+ if (this.validation) {
+ log.error(`Multiple validations occurred for engine ${this.name}!`);
+ return;
+ }
+ let { problems, duration, recordCount } = validationResult;
+ let validation = {
+ checked: recordCount || 0,
+ };
+ if (duration > 0) {
+ validation.took = Math.round(duration);
+ }
+ let summarized = problems.getSummary(true).filter(({count}) => count > 0);
+ if (summarized.length) {
+ validation.problems = summarized;
+ }
+ this.validation = validation;
+ }
+
+ recordValidationError(e) {
+ if (this.validation) {
+ log.error(`Multiple validations occurred for engine ${this.name}!`);
+ return;
+ }
+
+ this.validation = {
+ failureReason: transformError(e)
+ };
+ }
+
recordUploaded(counts) {
if (counts.sent || counts.failed) {
if (!this.outgoing) {
this.outgoing = [];
}
this.outgoing.push({
sent: counts.sent || undefined,
failed: counts.failed || undefined,
@@ -293,16 +325,46 @@ class TelemetryRecord {
onEngineApplied(engineName, counts) {
if (this._shouldIgnoreEngine(engineName)) {
return;
}
this.currentEngine.recordApplied(counts);
}
+ onEngineValidated(engineName, validationData) {
+ if (this._shouldIgnoreEngine(engineName, false)) {
+ return;
+ }
+ let engine = this.engines.find(e => e.name === engineName);
+ if (!engine && this.currentEngine && engineName === this.currentEngine.name) {
+ engine = this.currentEngine;
+ }
+ if (engine) {
+ engine.recordValidation(validationData);
+ } else {
+ log.warn(`Validation event triggered for engine ${engineName}, which hasn't been synced!`);
+ }
+ }
+
+ onEngineValidateError(engineName, error) {
+ if (this._shouldIgnoreEngine(engineName, false)) {
+ return;
+ }
+ let engine = this.engines.find(e => e.name === engineName);
+ if (!engine && this.currentEngine && engineName === this.currentEngine.name) {
+ engine = this.currentEngine;
+ }
+ if (engine) {
+ engine.recordValidationError(error);
+ } else {
+ log.warn(`Validation failure event triggered for engine ${engineName}, which hasn't been synced!`);
+ }
+ }
+
onEngineUploaded(engineName, counts) {
if (this._shouldIgnoreEngine(engineName)) {
return;
}
this.currentEngine.recordUploaded(counts);
}
_shouldIgnoreEngine(engineName, shouldBeCurrent = true) {
@@ -462,16 +524,28 @@ class SyncTelemetryImpl {
break;
case "weave:engine:sync:uploaded":
if (this._checkCurrent(topic)) {
this.current.onEngineUploaded(data, subject);
}
break;
+ case "weave:engine:validate:finish":
+ if (this._checkCurrent(topic)) {
+ this.current.onEngineValidated(data, subject);
+ }
+ break;
+
+ case "weave:engine:validate:error":
+ if (this._checkCurrent(topic)) {
+ this.current.onEngineValidateError(data, subject || "Unknown");
+ }
+ break;
+
default:
log.warn(`unexpected observer topic ${topic}`);
break;
}
}
}
this.SyncTelemetry = new SyncTelemetryImpl(ENGINES);
--- a/services/sync/services-sync.js
+++ b/services/sync/services-sync.js
@@ -73,8 +73,22 @@ pref("services.sync.log.logger.identity"
pref("services.sync.log.logger.userapi", "Debug");
pref("services.sync.log.cryptoDebug", false);
pref("services.sync.fxa.termsURL", "https://accounts.firefox.com/legal/terms");
pref("services.sync.fxa.privacyURL", "https://accounts.firefox.com/legal/privacy");
pref("services.sync.telemetry.submissionInterval", 43200); // 12 hours in seconds
pref("services.sync.telemetry.maxPayloadCount", 500);
+
+// Note that services.sync.validation.enabled is located in browser/app/profile/firefox.js
+
+// We consider validation this frequently. After considering validation, even
+// if we don't end up validating, we won't try again unless this much time has passed.
+pref("services.sync.validation.interval", 86400); // 24 hours in seconds
+
+// We only run validation `services.sync.validation.percentageChance` percent of
+// the time, even if it's been the right amount of time since the last validation,
+// and you meet the maxRecord checks.
+pref("services.sync.validation.percentageChance", 10);
+
+// We won't validate an engine if it has more than this many records on the server.
+pref("services.sync.validation.maxRecords", 100);
--- a/services/sync/tests/unit/head_appinfo.js
+++ b/services/sync/tests/unit/head_appinfo.js
@@ -12,16 +12,22 @@ gSyncProfile = do_get_profile();
// Init FormHistoryStartup and pretend we opened a profile.
var fhs = Cc["@mozilla.org/satchel/form-history-startup;1"]
.getService(Ci.nsIObserver);
fhs.observe(null, "profile-after-change", null);
// An app is going to have some prefs set which xpcshell tests don't.
Services.prefs.setCharPref("identity.sync.tokenserver.uri", "http://token-server");
+// Set the validation prefs to attempt validation every time to avoid non-determinism.
+Services.prefs.setIntPref("services.sync.validation.interval", 0);
+Services.prefs.setIntPref("services.sync.validation.percentageChance", 100);
+Services.prefs.setIntPref("services.sync.validation.maxRecords", -1);
+Services.prefs.setBoolPref("services.sync.validation.enabled", true);
+
// Make sure to provide the right OS so crypto loads the right binaries
function getOS() {
switch (mozinfo.os) {
case "win":
return "WINNT";
case "mac":
return "Darwin";
default:
--- a/services/sync/tests/unit/head_helpers.js
+++ b/services/sync/tests/unit/head_helpers.js
@@ -273,16 +273,20 @@ function assert_success_ping(ping) {
assert_valid_ping(ping);
ping.syncs.forEach(record => {
ok(!record.failureReason);
equal(undefined, record.status);
greater(record.engines.length, 0);
for (let e of record.engines) {
ok(!e.failureReason);
equal(undefined, e.status);
+ if (e.validation) {
+ equal(undefined, e.validation.problems);
+ equal(undefined, e.validation.failureReason);
+ }
if (e.outgoing) {
for (let o of e.outgoing) {
equal(undefined, o.failed);
notEqual(undefined, o.sent);
}
}
if (e.incoming) {
equal(undefined, e.incoming.failed);
--- a/services/sync/tests/unit/sync_ping_schema.json
+++ b/services/sync/tests/unit/sync_ping_schema.json
@@ -77,19 +77,32 @@
}
},
"outgoing": {
"type": "array",
"minItems": 1,
"items": { "$ref": "#/definitions/outgoingBatch" }
},
"validation": {
- "type": "array",
- "minItems": 1,
- "$ref": "#/definitions/validationProblem"
+ "type": "object",
+ "additionalProperties": false,
+ "anyOf": [
+ { "required": ["checked"] },
+ { "required": ["failureReason"] }
+ ],
+ "properties": {
+ "checked": { "type": "integer", "minimum": 0 },
+ "failureReason": { "$ref": "#/definitions/error" },
+ "took": { "type": "integer" },
+ "problems": {
+ "type": "array",
+ "minItems": 1,
+ "$ref": "#/definitions/validationProblem"
+ }
+ }
}
}
},
"outgoingBatch": {
"type": "object",
"additionalProperties": false,
"anyOf": [
{"required": ["sent"]},
--- a/services/sync/tests/unit/test_bookmark_validator.js
+++ b/services/sync/tests/unit/test_bookmark_validator.js
@@ -1,12 +1,13 @@
/* Any copyright is dedicated to the Public Domain.
http://creativecommons.org/publicdomain/zero/1.0/ */
Components.utils.import("resource://services-sync/bookmark_validator.js");
+Components.utils.import("resource://services-sync/util.js");
function inspectServerRecords(data) {
return new BookmarkValidator().inspectServerRecords(data);
}
add_test(function test_isr_rootOnServer() {
let c = inspectServerRecords([{
id: 'places',
@@ -242,11 +243,49 @@ add_test(function test_cswc_differences(
let c = new BookmarkValidator().compareServerWithClient(server, client).problemData;
equal(c.clientMissing.length, 0);
equal(c.serverMissing.length, 0);
deepEqual(c.differences, [{id: 'cccccccccccc', differences: ['type']}]);
}
run_next_test();
});
+function validationPing(server, client, duration) {
+ return wait_for_ping(function() {
+ // fake this entirely
+ Svc.Obs.notify("weave:service:sync:start");
+ Svc.Obs.notify("weave:engine:sync:start", null, "bookmarks");
+ Svc.Obs.notify("weave:engine:sync:finish", null, "bookmarks");
+ let data = {
+ duration, // We fake this just so that we can verify it's passed through.
+ recordCount: server.length,
+ problems: new BookmarkValidator().compareServerWithClient(server, client).problemData,
+ };
+ Svc.Obs.notify("weave:engine:validate:finish", data, "bookmarks");
+ Svc.Obs.notify("weave:service:sync:finish");
+ }, true); // Allow "failing" pings, since having validation info indicates failure.
+}
+
+add_task(function *test_telemetry_integration() {
+ let {server, client} = getDummyServerAndClient();
+ // remove "c"
+ server.pop();
+ server[0].children.pop();
+ const duration = 50;
+ let ping = yield validationPing(server, client, duration);
+ ok(ping.engines);
+ let bme = ping.engines.find(e => e.name === "bookmarks");
+ ok(bme);
+ ok(bme.validation);
+ ok(bme.validation.problems)
+ equal(bme.validation.checked, server.length);
+ equal(bme.validation.took, duration);
+ bme.validation.problems.sort((a, b) => String.localeCompare(a.name, b.name));
+ deepEqual(bme.validation.problems, [
+ { name: "sdiff:childGUIDs", count: 1 },
+ { name: "serverMissing", count: 1 },
+ { name: "structuralDifferences", count: 1 },
+ ]);
+});
+
function run_test() {
run_next_test();
}
--- a/toolkit/components/telemetry/docs/data/sync-ping.rst
+++ b/toolkit/components/telemetry/docs/data/sync-ping.rst
@@ -65,24 +65,31 @@ Structure:
sent: <integer>, // Number of outgoing records sent. Zero values are omitted.
failed: <integer>, // Number that failed to send. Zero values are omitted.
}
],
// Optional, excluded if there were no errors
failureReason: { ... }, // Same as above.
// Optional, excluded if it would be empty or if the engine cannot
- // or did not run validation on itself. Entries with a count of 0
- // are excluded.
- validation: [
- {
- name: <string>, // The problem identified.
- count: <integer>, // Number of times it occurred.
- }
- ]
+ // or did not run validation on itself.
+ validation: {
+ checked: <integer>,
+ took: <non-monotonic integer duration in milliseconds>,
+ // Entries with a count of 0 are excluded, the array is excluded if no problems are found.
+ problems: [
+ {
+ name: <string>, // The problem identified.
+ count: <integer>, // Number of times it occurred.
+ }
+ ],
+ // Format is same as above, this is only included if we tried and failed
+ // to run validation, and if it's present, all other fields in this object are optional.
+ failureReason: { ... },
+ }
}
]
}]
}
}
info
----
@@ -145,12 +152,12 @@ Stores error information, if any is pres
- ``error``: The message provided by the error.
syncs.engine.name
~~~~~~~~~~~~~~~~~
Third-party engines are not reported, so only the following values are allowed: ``addons``, ``bookmarks``, ``clients``, ``forms``, ``history``, ``passwords``, ``prefs``, and ``tabs``.
-syncs.engine.validation
-~~~~~~~~~~~~~~~~~~~~~~~
+syncs.engine.validation.problems
+~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
For engines that can run validation on themselves, an array of objects describing validation errors that have occurred. Items that would have a count of 0 are excluded. Each engine will have its own set of items that it might put in the ``name`` field, but there are a finite number. See ``BookmarkProblemData.getSummary`` in `services/sync/modules/bookmark\_validator.js <https://dxr.mozilla.org/mozilla-central/source/services/sync/modules/bookmark_validator.js>`_ for an example.