Bug 1465129 - Collect telemetry data related to the storage.local "JSONFile to IDBBackend" data migrations. draft
authorLuca Greco <lgreco@mozilla.com>
Sun, 17 Jun 2018 13:38:02 +0200
changeset 808771 f5a4853e6bdbff55e0bac19ce953ced45bed5800
parent 808416 257c191e7903523a1132e04460a0b2460d950809
child 809245 6c0431652b4878ac5ad6bbc593db02e064bb8f8e
child 809512 9ca3f52a48dc21d172ff348f69c34e658a8463de
push id113482
push userluca.greco@alcacoop.it
push dateWed, 20 Jun 2018 16:44:43 +0000
bugs1465129
milestone62.0a1
Bug 1465129 - Collect telemetry data related to the storage.local "JSONFile to IDBBackend" data migrations. MozReview-Commit-ID: 3iGv5XkqeA3
toolkit/components/extensions/ExtensionStorageIDB.jsm
toolkit/components/extensions/test/xpcshell/test_ext_storage_idb_data_migration.js
toolkit/components/telemetry/Histograms.json
--- a/toolkit/components/extensions/ExtensionStorageIDB.jsm
+++ b/toolkit/components/extensions/ExtensionStorageIDB.jsm
@@ -22,16 +22,17 @@ XPCOMUtils.defineLazyModuleGetters(this,
 XPCOMUtils.defineLazyGetter(this, "WEBEXT_STORAGE_USER_CONTEXT_ID", () => {
   return ContextualIdentityService.getDefaultPrivateIdentity(
     "userContextIdInternal.webextStorageLocal").userContextId;
 });
 
 const IDB_NAME = "webExtensions-storage-local";
 const IDB_DATA_STORENAME = "storage-local-data";
 const IDB_VERSION = 1;
+const IDB_MIGRATE_RESULT_HISTOGRAM = "WEBEXT_STORAGE_LOCAL_IDB_MIGRATE_RESULT_COUNT";
 
 // Whether or not the installed extensions should be migrated to the storage.local IndexedDB backend.
 const BACKEND_ENABLED_PREF = "extensions.webextensions.ExtensionStorageIDB.enabled";
 
 class ExtensionStorageLocalIDB extends IndexedDB {
   onupgradeneeded(event) {
     if (event.oldVersion < 1) {
       this.createObjectStore(IDB_DATA_STORENAME);
@@ -233,16 +234,17 @@ class ExtensionStorageLocalIDB extends I
 async function migrateJSONFileData(extension, storagePrincipal) {
   let oldStoragePath;
   let oldStorageExists;
   let idbConn;
   let jsonFile;
   let hasEmptyIDB;
   let oldDataRead = false;
   let migrated = false;
+  let histogram = Services.telemetry.getHistogramById(IDB_MIGRATE_RESULT_HISTOGRAM);
 
   try {
     idbConn = await ExtensionStorageLocalIDB.openForPrincipal(storagePrincipal);
     hasEmptyIDB = await idbConn.isEmpty();
 
     if (!hasEmptyIDB) {
       // If the IDB backend is enabled and there is data already stored in the IDB backend,
       // there is no "going back": any data that has not been migrated will be still on disk
@@ -274,27 +276,32 @@ async function migrateJSONFileData(exten
   } catch (err) {
     extension.logWarning(`Error on migrating storage.local data: ${err.message}::${err.stack}`);
     if (oldDataRead) {
       // If the data has been read successfully and it has been failed to be stored
       // into the IndexedDB backend, then clear any partially stored data and reject
       // the data migration promise explicitly (which would prevent the new backend
       // from being enabled for this session).
       Services.qms.clearStoragesForPrincipal(storagePrincipal);
+
+      histogram.add("failure");
+
       throw err;
     }
   } finally {
     // Finalize the jsonFile and clear the jsonFilePromise cached by the ExtensionStorage
     // (so that the file can be immediatelly removed when we call OS.File.remove).
     ExtensionStorage.clearCachedFile(extension.id);
     if (jsonFile) {
       jsonFile.finalize();
     }
   }
 
+  histogram.add("success");
+
   // If the IDB backend has been enabled, try to remove the old storage.local data file,
   // but keep using the selected backend even if it fails to be removed.
   if (oldStorageExists && migrated) {
     try {
       await OS.File.remove(oldStoragePath);
     } catch (err) {
       extension.logWarning(err.message);
     }
@@ -302,16 +309,17 @@ async function migrateJSONFileData(exten
 }
 
 /**
  * This ExtensionStorage class implements a backend for the storage.local API which
  * uses IndexedDB to store the data.
  */
 this.ExtensionStorageIDB = {
   BACKEND_ENABLED_PREF,
+  IDB_MIGRATE_RESULT_HISTOGRAM,
 
   // Map<extension-id, Set<Function>>
   listeners: new Map(),
 
   // Keep track if the IDB backend has been selected or not for a running extension
   // (the selected backend should never change while the extension is running, even if the
   // related preference has been changed in the meantime):
   //
--- a/toolkit/components/extensions/test/xpcshell/test_ext_storage_idb_data_migration.js
+++ b/toolkit/components/extensions/test/xpcshell/test_ext_storage_idb_data_migration.js
@@ -1,33 +1,50 @@
 /* -*- Mode: indent-tabs-mode: nil; js-indent-level: 2 -*- */
 /* vim: set sts=2 sw=2 et tw=80: */
 "use strict";
 
 // This test file verifies various scenarios related to the data migration
 // from the JSONFile backend to the IDB backend.
 
 ChromeUtils.import("resource://gre/modules/XPCOMUtils.jsm");
+ChromeUtils.import("resource://gre/modules/ExtensionStorage.jsm");
+ChromeUtils.import("resource://gre/modules/ExtensionStorageIDB.jsm");
 
 XPCOMUtils.defineLazyModuleGetters(this, {
-  ExtensionStorage: "resource://gre/modules/ExtensionStorage.jsm",
-  ExtensionStorageIDB: "resource://gre/modules/ExtensionStorageIDB.jsm",
   OS: "resource://gre/modules/osfile.jsm",
 });
 
+const {IDB_MIGRATE_RESULT_HISTOGRAM} = ExtensionStorageIDB;
+const CATEGORIES = ["success", "failure"];
+
 async function createExtensionJSONFileWithData(extensionId, data) {
   await ExtensionStorage.set(extensionId, data);
   const jsonFile = await ExtensionStorage.getFile(extensionId);
   await jsonFile._save();
   const oldStorageFilename = ExtensionStorage.getStorageFile(extensionId);
   equal(await OS.File.exists(oldStorageFilename), true, "The old json file has been created");
 
   return {jsonFile, oldStorageFilename};
 }
 
+function clearMigrationHistogram() {
+  const histogram = Services.telemetry.getHistogramById(IDB_MIGRATE_RESULT_HISTOGRAM);
+  histogram.clear();
+  equal(histogram.snapshot().sum, 0,
+        `No data recorded for histogram ${IDB_MIGRATE_RESULT_HISTOGRAM}`);
+}
+
+function assertMigrationHistogramCount(category, expectedCount) {
+  const histogram = Services.telemetry.getHistogramById(IDB_MIGRATE_RESULT_HISTOGRAM);
+
+  equal(histogram.snapshot().counts[CATEGORIES.indexOf(category)], expectedCount,
+        `Got the expected count on category "${category}" for histogram ${IDB_MIGRATE_RESULT_HISTOGRAM}`);
+}
+
 add_task(async function setup() {
   Services.prefs.setBoolPref(ExtensionStorageIDB.BACKEND_ENABLED_PREF, true);
 });
 
 // Test that the old data is migrated successfully to the new storage backend
 // and that the original JSONFile is being removed.
 add_task(async function test_storage_local_data_migration() {
   const EXTENSION_ID = "extension-to-be-migrated@mozilla.org";
@@ -51,16 +68,18 @@ add_task(async function test_storage_loc
     browser.test.assertEq(1000, storedData.test_key_number,
                           "Got the expected data after the storage.local data migration");
     browser.test.assertEq(true, storedData.test_nested_data.nested_key,
                           "Got the expected data after the storage.local data migration");
 
     browser.test.sendMessage("storage-local-data-migrated");
   }
 
+  clearMigrationHistogram();
+
   let extension = ExtensionTestUtils.loadExtension({
     manifest: {
       permissions: ["storage"],
       applications: {
         gecko: {
           id: EXTENSION_ID,
         },
       },
@@ -77,16 +96,19 @@ add_task(async function test_storage_loc
   const idbConn = await ExtensionStorageIDB.open(storagePrincipal);
 
   equal(await idbConn.isEmpty(extension.extension), false,
         "Data stored in the ExtensionStorageIDB backend as expected");
 
   equal(await OS.File.exists(oldStorageFilename), false,
         "The old json storage file should have been removed");
 
+  assertMigrationHistogramCount("success", 1);
+  assertMigrationHistogramCount("failure", 0);
+
   await extension.unload();
 });
 
 // Test that if the old JSONFile data file is corrupted and the old data
 // can't be successfully migrated to the new storage backend, then:
 // - the new storage backend for that extension is still initialized and enabled
 // - any new data is being stored in the new backend
 // - the old file is being renamed (with the `.corrupted` suffix that JSONFile.jsm
@@ -105,23 +127,25 @@ add_task(async function test_storage_loc
   await OS.File.writeAtomic(oldStorageFilename, invalidData, {flush: true});
   equal(await OS.File.read(oldStorageFilename, {encoding: "utf-8"}),
         invalidData, "The old json file has been overwritten with invalid data");
 
   async function background() {
     const storedData = await browser.storage.local.get();
 
     browser.test.assertEq(Object.keys(storedData).length, 0,
-                          "No data should be found found on invalid data migration");
+                          "No data should be found on invalid data migration");
 
     await browser.storage.local.set({"test_key_string_on_IDBBackend": "expected-value"});
 
     browser.test.sendMessage("storage-local-data-migrated-and-set");
   }
 
+  clearMigrationHistogram();
+
   let extension = ExtensionTestUtils.loadExtension({
     manifest: {
       permissions: ["storage"],
       applications: {
         gecko: {
           id: EXTENSION_ID,
         },
       },
@@ -138,14 +162,19 @@ add_task(async function test_storage_loc
   const idbConn = await ExtensionStorageIDB.open(storagePrincipal);
 
   equal(await idbConn.isEmpty(extension.extension), false,
         "Data stored in the ExtensionStorageIDB backend as expected");
 
   equal(await OS.File.exists(`${oldStorageFilename}.corrupt`), true,
         "The old json storage should still be available if failed to be read");
 
+  // The extension is still migrated successfully to the new backend if the file from the
+  // original json file was corrupted.
+  assertMigrationHistogramCount("success", 1);
+  assertMigrationHistogramCount("failure", 0);
+
   await extension.unload();
 });
 
 add_task(function test_storage_local_data_migration_clear_pref() {
   Services.prefs.clearUserPref(ExtensionStorageIDB.BACKEND_ENABLED_PREF);
 });
--- a/toolkit/components/telemetry/Histograms.json
+++ b/toolkit/components/telemetry/Histograms.json
@@ -13434,17 +13434,17 @@
     "bug_numbers": [1371398],
     "expires_in_version": "67",
     "kind": "exponential",
     "releaseChannelCollection": "opt-out",
     "high": 50000,
     "n_buckets": 100,
     "description": "The amount of time it takes to perform a set via storage.local using the JSONFile backend."
   },
-    "WEBEXT_STORAGE_LOCAL_IDB_GET_MS": {
+  "WEBEXT_STORAGE_LOCAL_IDB_GET_MS": {
     "record_in_processes": ["main", "content"],
     "alert_emails": ["addons-dev-internal@mozilla.com"],
     "bug_numbers": [1465120],
     "expires_in_version": "67",
     "kind": "exponential",
     "releaseChannelCollection": "opt-out",
     "high": 50000,
     "n_buckets": 100,
@@ -13456,16 +13456,26 @@
     "bug_numbers": [1465120],
     "expires_in_version": "67",
     "kind": "exponential",
     "releaseChannelCollection": "opt-out",
     "high": 50000,
     "n_buckets": 100,
     "description": "The amount of time it takes to perform a set via storage.local using the IndexedDB backend."
   },
+  "WEBEXT_STORAGE_LOCAL_IDB_MIGRATE_RESULT_COUNT": {
+    "record_in_processes": ["main"],
+    "bug_numbers": [1465129],
+    "alert_emails": ["addons-dev-internal@mozilla.com"],
+    "expires_in_version": "67",
+    "kind": "categorical",
+    "labels": ["success", "failure"],
+    "releaseChannelCollection": "opt-out",
+    "description": "The number of times a storage.local backend data migration has been completed and results in one of the categories."
+  },
   "EXTENSION_UPDATE_TYPE": {
     "record_in_processes": ["main"],
     "alert_emails": ["addons-dev-internal@mozilla.com"],
     "bug_numbers": [1460336],
     "expires_in_version": "66",
     "kind": "categorical",
     "labels": ["JSON", "RDF"],
     "releaseChannelCollection": "opt-out",