Bug 1451152 - Record the bookmarks mirror database size in telemetry. r?tcsc draft
authorKit Cambridge <kit@yakshaving.ninja>
Wed, 18 Apr 2018 16:44:47 -0700
changeset 785133 d6f470e80e57a577ac414cd042fc7d2931b789a9
parent 785132 74b21d27cc0e3a4e6217638f7f5d9211cd3db005
child 785134 3755ade4504fcd0e30bb423de3319138c7f129c3
push id107146
push userbmo:kit@mozilla.com
push dateThu, 19 Apr 2018 18:15:11 +0000
reviewerstcsc
bugs1451152
milestone61.0a1
Bug 1451152 - Record the bookmarks mirror database size in telemetry. r?tcsc MozReview-Commit-ID: JIQpzTc9Kuw
toolkit/components/places/SyncedBookmarksMirror.jsm
toolkit/components/places/tests/sync/head_sync.js
toolkit/components/places/tests/sync/test_bookmark_mirror_migration.js
--- a/toolkit/components/places/SyncedBookmarksMirror.jsm
+++ b/toolkit/components/places/SyncedBookmarksMirror.jsm
@@ -142,54 +142,64 @@ class SyncedBookmarksMirror {
   }
 
   /**
    * Sets up the mirror database connection and upgrades the mirror to the
    * newest schema version. Automatically recreates the mirror if it's corrupt;
    * throws on failure.
    *
    * @param  {String} options.path
-   *         The full path to the mirror database file.
+   *         The path to the mirror database file, either absolute or relative
+   *         to the profile path.
    * @param  {Function} options.recordTelemetryEvent
    *         A function with the signature `(object: String, method: String,
    *         value: String?, extra: Object?)`, used to emit telemetry events.
    * @param  {AsyncShutdown.Barrier} [options.finalizeAt]
    *         A shutdown phase, barrier, or barrier client that should
    *         automatically finalize the mirror when triggered. Exposed for
    *         testing.
    * @return {SyncedBookmarksMirror}
    *         A mirror ready for use.
    */
   static async open(options) {
     let db = await Sqlite.cloneStorageConnection({
       connection: PlacesUtils.history.DBConnection,
       readOnly: false,
     });
+    let path = OS.Path.join(OS.Constants.Path.profileDir, options.path);
     let whyFailed = "initialize";
     try {
       await db.execute(`PRAGMA foreign_keys = ON`);
       try {
-        await attachAndInitMirrorDatabase(db, options.path);
+        await attachAndInitMirrorDatabase(db, path);
       } catch (ex) {
         if (isDatabaseCorrupt(ex)) {
           MirrorLog.warn("Error attaching mirror to Places; removing and " +
                          "recreating mirror", ex);
           options.recordTelemetryEvent("mirror", "open", "retry",
                                        { why: "corrupt" });
 
           whyFailed = "remove";
-          await OS.File.remove(options.path);
+          await OS.File.remove(path);
 
           whyFailed = "replace";
-          await attachAndInitMirrorDatabase(db, options.path);
+          await attachAndInitMirrorDatabase(db, path);
         } else {
-          MirrorLog.warn("Unrecoverable error attaching mirror to Places", ex);
+          MirrorLog.error("Unrecoverable error attaching mirror to Places", ex);
           throw ex;
         }
       }
+      try {
+        let info = await OS.File.stat(path);
+        let fileSize = Math.floor(info.size / 1024);
+        options.recordTelemetryEvent("mirror", "open", "success",
+                                     { size: fileSize.toString(10) });
+      } catch (ex) {
+        MirrorLog.warn("Error recording stats for mirror database size", ex);
+      }
     } catch (ex) {
       options.recordTelemetryEvent("mirror", "open", "error",
                                    { why: whyFailed });
       await db.close();
       throw ex;
     }
     return new SyncedBookmarksMirror(db, options);
   }
--- a/toolkit/components/places/tests/sync/head_sync.js
+++ b/toolkit/components/places/tests/sync/head_sync.js
@@ -159,19 +159,18 @@ function shuffle(array) {
 
 async function fetchAllKeywords(info) {
   let entries = [];
   await PlacesUtils.keywords.fetch(info, entry => entries.push(entry));
   return entries;
 }
 
 async function openMirror(name, options = {}) {
-  let path = OS.Path.join(OS.Constants.Path.profileDir, `${name}_buf.sqlite`);
   let buf = await SyncedBookmarksMirror.open({
-    path,
+    path: `${name}_buf.sqlite`,
     recordTelemetryEvent(...args) {
       if (options.recordTelemetryEvent) {
         options.recordTelemetryEvent.call(this, ...args);
       }
     },
   });
   return buf;
 }
--- a/toolkit/components/places/tests/sync/test_bookmark_mirror_migration.js
+++ b/toolkit/components/places/tests/sync/test_bookmark_mirror_migration.js
@@ -6,39 +6,52 @@ add_task(async function test_migrate_fro
   let dbFile = do_get_file("mirror_v1.sqlite");
   let telemetryEvents = [];
   let buf = await SyncedBookmarksMirror.open({
     path: dbFile.path,
     recordTelemetryEvent(object, method, value, extra) {
       telemetryEvents.push({ object, method, value, extra });
     },
   });
+  let dbFileSize = Math.floor((await OS.File.stat(dbFile.path)).size / 1024);
   deepEqual(telemetryEvents, [{
     object: "mirror",
     method: "open",
     value: "retry",
     extra: { why: "corrupt" },
+  }, {
+    object: "mirror",
+    method: "open",
+    value: "success",
+    extra: { size: dbFileSize.toString(10) },
   }]);
   await buf.finalize();
 });
 
 add_task(async function test_database_corrupt() {
   let corruptFile = do_get_file("mirror_corrupt.sqlite");
   let telemetryEvents = [];
   let buf = await SyncedBookmarksMirror.open({
     path: corruptFile.path,
     recordTelemetryEvent(object, method, value, extra) {
       telemetryEvents.push({ object, method, value, extra });
     },
   });
+  let dbFileSize = Math.floor((
+    await OS.File.stat(corruptFile.path)).size / 1024);
   deepEqual(telemetryEvents, [{
     object: "mirror",
     method: "open",
     value: "retry",
     extra: { why: "corrupt" },
+  }, {
+    object: "mirror",
+    method: "open",
+    value: "success",
+    extra: { size: dbFileSize.toString(10) },
   }]);
   await buf.finalize();
 });
 
 add_task(async function test_database_readonly() {
   let dbFile = do_get_file("mirror_v1.sqlite");
   try {
     await OS.File.setPermissions(dbFile.path, {