From c678628cc56d316c000aac96843570b0ce2f4a38 Mon Sep 17 00:00:00 2001 From: Tomislav Jovanovic Date: Tue, 28 Nov 2023 21:31:43 +0000 Subject: [PATCH] Bug 1820154 - Port extensions.data.* telemetry events to Glean, r=rpl Differential Revision: https://phabricator.services.mozilla.com/D194883 --- .../extensions/ExtensionStorageIDB.sys.mjs | 22 ++++- toolkit/components/extensions/metrics.yaml | 78 ++++++++++++++++++ .../test_ext_storage_idb_data_migration.js | 94 +++++++++++++--------- .../test/xpcshell/test_ext_storage_telemetry.js | 11 +++ 4 files changed, 163 insertions(+), 42 deletions(-) diff --git a/toolkit/components/extensions/ExtensionStorageIDB.sys.mjs b/toolkit/components/extensions/ExtensionStorageIDB.sys.mjs index 1be5bf26897f..8dfb034cce6f 100644 --- a/toolkit/components/extensions/ExtensionStorageIDB.sys.mjs +++ b/toolkit/components/extensions/ExtensionStorageIDB.sys.mjs @@ -136,13 +136,22 @@ var ErrorsTelemetry = { extra.error_name = this.getErrorName(error); } + let addon_id = lazy.getTrimmedString(extensionId); Services.telemetry.recordEvent( "extensions.data", "migrateResult", "storageLocal", - lazy.getTrimmedString(extensionId), + addon_id, extra ); + Glean.extensionsData.migrateResult.record({ + addon_id, + backend: extra.backend, + data_migrated: extra.data_migrated, + has_jsonfile: extra.has_jsonfile, + has_olddata: extra.has_olddata, + error_name: extra.error_name, + }); } catch (err) { // Report any telemetry error on the browser console, but // we treat it as a non-fatal error and we don't re-throw @@ -165,14 +174,21 @@ var ErrorsTelemetry = { */ recordStorageLocalError({ extensionId, storageMethod, error }) { this.lazyInit(); + let addon_id = lazy.getTrimmedString(extensionId); + let error_name = this.getErrorName(error); Services.telemetry.recordEvent( "extensions.data", "storageLocalError", storageMethod, - lazy.getTrimmedString(extensionId), - { error_name: this.getErrorName(error) } + addon_id, + { error_name } ); + Glean.extensionsData.storageLocalError.record({ + addon_id, + method: storageMethod, + error_name, + }); }, }; diff --git a/toolkit/components/extensions/metrics.yaml b/toolkit/components/extensions/metrics.yaml index 4d94b6b88d31..c12679d282c3 100644 --- a/toolkit/components/extensions/metrics.yaml +++ b/toolkit/components/extensions/metrics.yaml @@ -272,6 +272,84 @@ extensions.apis.dnr: - technical telemetry_mirror: EXTENSIONS_APIS_DNR_EVALUATE_RULES_COUNT_MAX +extensions.data: + + migrate_result: + type: event + description: | + These events are sent when an extension is migrating its data to the new + IndexedDB backend. + bugs: + - https://bugzilla.mozilla.org/1470213 + - https://bugzilla.mozilla.org/1553297 + - https://bugzilla.mozilla.org/1590736 + - https://bugzilla.mozilla.org/1630596 + - https://bugzilla.mozilla.org/1672570 + - https://bugzilla.mozilla.org/1714251 + - https://bugzilla.mozilla.org/1749878 + - https://bugzilla.mozilla.org/1781974 + - https://bugzilla.mozilla.org/1817100 + - https://bugzilla.mozilla.org/1861295 + data_reviews: + - https://bugzilla.mozilla.org/show_bug.cgi?id=1470213#c15 + notification_emails: + - addons-dev-internal@mozilla.com + extra_keys: + addon_id: + description: Id of the addon. + type: string + backend: + description: The selected backend ("JSONFile" / "IndexedDB"). + type: string + data_migrated: + description: The old extension data has been migrated ("y" / "n"). + type: string + error_name: + description: | + A DOMException error name if any ("OtherError" for unknown errors). + The error has been fatal if the `backend` extra key is "JSONFile", + otherwise it is a non fatal error which didn't prevented the + extension from switching to the IndexedDB backend. + type: string + has_jsonfile: + description: The extension has a JSONFile ("y" / "n"). + type: string + has_olddata: + description: Extension had some data stored in JSONFile ("y" / "n"). + type: string + expires: 132 + + storage_local_error: + type: event + description: | + These events are collected when an extension triggers an unexpected error + while running a storage.local API call (e.g. because of some underlying + QuotaManager and/or IndexedDB error). + bugs: + - https://bugzilla.mozilla.org/1606903 + - https://bugzilla.mozilla.org/1649948 + - https://bugzilla.mozilla.org/1689255 + - https://bugzilla.mozilla.org/1730038 + - https://bugzilla.mozilla.org/1763523 + - https://bugzilla.mozilla.org/1811148 + - https://bugzilla.mozilla.org/1861297 + data_reviews: + - https://bugzilla.mozilla.org/show_bug.cgi?id=1606903#c3 + notification_emails: + - addons-dev-internal@mozilla.com + extra_keys: + addon_id: + description: Id of the addon. + type: string + method: + description: The storage.local API method name. + type: string + error_name: + description: | + A DOMException error name if any ("OtherError" for unknown errors). + type: string + expires: 132 + extensions.quarantined_domains: listsize: diff --git 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 index 8a426c366957..8a6631f26b3a 100644 --- 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 @@ -50,6 +50,11 @@ const TELEMETRY_EVENTS_FILTER = { object: "storageLocal", }; +add_setup(async function setup() { + do_get_profile(); + Services.fog.initializeFOG(); +}); + async function createExtensionJSONFileWithData(extensionId, data) { await ExtensionStorage.set(extensionId, data); const jsonFile = await ExtensionStorage.getFile(extensionId); @@ -88,12 +93,29 @@ function assertMigrationHistogramCount(category, expectedCount) { ); } +// Note: for consistency with telemetry event format, this function also +// expects the addon_id to be passed in the event.value property. +function assertMigrateResultGleanEvents(expectedEvents) { + let glean = Glean.extensionsData.migrateResult.testGetValue() ?? []; + equal(glean.length, expectedEvents.length, "Correct number of events."); + + expectedEvents.forEach((expected, i) => + Assert.deepEqual( + glean[i].extra, + { addon_id: expected.value, ...expected.extra }, + "Correct addon_id and event extra properties." + ) + ); + Services.fog.testResetFOG(); +} + function assertTelemetryEvents(expectedEvents) { TelemetryTestUtils.assertEvents(expectedEvents, { category: EVENT_CATEGORY, method: EVENT_METHOD, object: EVENT_OBJECT, }); + assertMigrateResultGleanEvents(expectedEvents); } add_setup(async function setup() { @@ -156,6 +178,7 @@ add_task(async function test_no_migration_for_newly_installed_extensions() { // Verify that no data migration have been needed on the newly installed // extension, by asserting that no telemetry events has been collected. await TelemetryTestUtils.assertEvents([], TELEMETRY_EVENTS_FILTER); + assertMigrateResultGleanEvents([]); }); // Test that the data migration is still running for a newly installed extension @@ -196,21 +219,19 @@ add_task(async function test_data_migration_on_keep_storage_on_uninstall() { await extension.unload(); // Verify that the expected telemetry has been recorded. - await TelemetryTestUtils.assertEvents( - [ - { - method: "migrateResult", - value: EXTENSION_ID, - extra: { - backend: "IndexedDB", - data_migrated: "y", - has_jsonfile: "y", - has_olddata: "y", - }, - }, - ], - TELEMETRY_EVENTS_FILTER - ); + let expected = { + method: "migrateResult", + value: EXTENSION_ID, + extra: { + backend: "IndexedDB", + data_migrated: "y", + has_jsonfile: "y", + has_olddata: "y", + }, + }; + + await TelemetryTestUtils.assertEvents([expected], TELEMETRY_EVENTS_FILTER); + assertMigrateResultGleanEvents([expected]); Services.prefs.clearUserPref(LEAVE_STORAGE_PREF); }); @@ -687,18 +708,16 @@ add_task(async function test_migration_aborted_on_shutdown() { { backendEnabled: false }, "Expect migration to have been aborted" ); - TelemetryTestUtils.assertEvents( - [ - { - value: EXTENSION_ID, - extra: { - backend: "JSONFile", - error_name: "DataMigrationAbortedError", - }, - }, - ], - TELEMETRY_EVENTS_FILTER - ); + let expected = { + value: EXTENSION_ID, + extra: { + backend: "JSONFile", + error_name: "DataMigrationAbortedError", + }, + }; + + TelemetryTestUtils.assertEvents([expected], TELEMETRY_EVENTS_FILTER); + assertMigrateResultGleanEvents([expected]); }); add_task(async function test_storage_local_data_migration_clear_pref() { @@ -771,18 +790,15 @@ async function test_quota_exceeded_while_migrating_data() { ); await extension.unload(); - TelemetryTestUtils.assertEvents( - [ - { - value: EXT_ID, - extra: { - backend: "JSONFile", - error_name: "QuotaExceededError", - }, - }, - ], - TELEMETRY_EVENTS_FILTER - ); + let expected = { + value: EXT_ID, + extra: { + backend: "JSONFile", + error_name: "QuotaExceededError", + }, + }; + TelemetryTestUtils.assertEvents([expected], TELEMETRY_EVENTS_FILTER); + assertMigrateResultGleanEvents([expected]); Services.prefs.clearUserPref("dom.quotaManager.temporaryStorage.fixedLimit"); await promiseQuotaManagerServiceClear(); diff --git a/toolkit/components/extensions/test/xpcshell/test_ext_storage_telemetry.js b/toolkit/components/extensions/test/xpcshell/test_ext_storage_telemetry.js index 7ae12202efa7..d0448b7b2e97 100644 --- a/toolkit/components/extensions/test/xpcshell/test_ext_storage_telemetry.js +++ b/toolkit/components/extensions/test/xpcshell/test_ext_storage_telemetry.js @@ -444,4 +444,15 @@ add_task(async function test_telemetry_storage_local_unexpected_error() { category: "extensions.data", method: "storageLocalError", }); + + let glean = Glean.extensionsData.storageLocalError.testGetValue() ?? []; + equal(glean.length, expectedEvents.length, "Correct number of events."); + + for (let i = 0; i < expectedEvents.length; i++) { + let event = expectedEvents[i]; + equal(glean[i].extra.addon_id, event.value, "Correct addon_id."); + equal(glean[i].extra.method, event.object, "Correct method."); + equal(glean[i].extra.error_name, event.extra.error_name, "Correct error."); + } + Services.fog.testResetFOG(); }); -- 2.11.4.GIT