diff --git a/toolkit/components/telemetry/Histograms.json b/toolkit/components/telemetry/Histograms.json index ea2986462d0a..ff4d9b2780a5 100644 --- a/toolkit/components/telemetry/Histograms.json +++ b/toolkit/components/telemetry/Histograms.json @@ -8428,6 +8428,22 @@ "n_buckets": 100, "description": "Time taken (in ms) to update the inspector during a page reload, starting from new-root event." }, + "MEDIA_GMP_UPDATE_XML_FETCH_RESULT": { + "record_in_processes": ["main"], + "products": ["firefox"], + "alert_emails": ["media-alerts@mozilla.com"], + "expires_in_version": "never", + "releaseChannelCollection": "opt-out", + "kind": "categorical", + "labels": [ + "cert_pinning_ok", + "cert_pinning_fail", + "content_sig_ok", + "content_sig_fail" + ], + "description": "Reports if a Firefox succeeded or failed in fetching the GMP update.xml from balrog, and which method was used to verify the download.", + "bug_numbers": [1714621] + }, "MEDIA_MP4_PARSE_SAMPLE_DESCRIPTION_ENTRIES_HAVE_MULTIPLE_CODECS": { "record_in_processes": ["main", "content"], "products": ["firefox", "fennec"], diff --git a/toolkit/modules/GMPInstallManager.sys.mjs b/toolkit/modules/GMPInstallManager.sys.mjs index 2299f3b9c3b5..c73bd65df919 100644 --- a/toolkit/modules/GMPInstallManager.sys.mjs +++ b/toolkit/modules/GMPInstallManager.sys.mjs @@ -229,11 +229,19 @@ GMPInstallManager.prototype = { "GMPInstallManager.recordUpdateXmlTelemetryForContentSignature" ); try { + let updateResultHistogram = Services.telemetry.getHistogramById( + "MEDIA_GMP_UPDATE_XML_FETCH_RESULT" + ); + + // The non-glean telemetry used here will be removed in future and just + // the glean data will be gathered. if (didGetAddonList) { + updateResultHistogram.add("content_sig_ok"); Glean.gmp.updateXmlFetchResult.content_sig_success.add(1); return; } // All remaining cases are failure cases. + updateResultHistogram.add("content_sig_fail"); if (!err?.addonCheckerErr) { // Unknown error case. If this is happening we should audit error paths // to identify why we're not getting an error, or not getting it @@ -283,11 +291,19 @@ GMPInstallManager.prototype = { "GMPInstallManager.recordUpdateXmlTelemetryForCertPinning" ); try { + let updateResultHistogram = Services.telemetry.getHistogramById( + "MEDIA_GMP_UPDATE_XML_FETCH_RESULT" + ); + + // The non-glean telemetry used here will be removed in future and just + // the glean data will be gathered. if (didGetAddonList) { + updateResultHistogram.add("cert_pinning_ok"); Glean.gmp.updateXmlFetchResult.cert_pin_success.add(1); return; } // All remaining cases are failure cases. + updateResultHistogram.add("cert_pinning_fail"); if (!err?.addonCheckerErr) { // Unknown error case. If this is happening we should audit error paths // to identify why we're not getting an error, or not getting it diff --git a/toolkit/modules/tests/xpcshell/test_GMPInstallManager.js b/toolkit/modules/tests/xpcshell/test_GMPInstallManager.js index ee09781ac058..21a15c4c917f 100644 --- a/toolkit/modules/tests/xpcshell/test_GMPInstallManager.js +++ b/toolkit/modules/tests/xpcshell/test_GMPInstallManager.js @@ -20,6 +20,9 @@ const { HttpServer } = ChromeUtils.importESModule( const { Preferences } = ChromeUtils.importESModule( "resource://gre/modules/Preferences.sys.mjs" ); +const { TelemetryTestUtils } = ChromeUtils.importESModule( + "resource://testing-common/TelemetryTestUtils.sys.mjs" +); const { UpdateUtils } = ChromeUtils.importESModule( "resource://gre/modules/UpdateUtils.sys.mjs" ); @@ -561,7 +564,7 @@ add_task(async function test_checkForAddons_updatesWithAddons() { add_task(async function test_checkForAddons_contentSignatureSuccess() { const previousUrlOverride = setupContentSigTestPrefs(); - Services.fog.testResetFOG(); + const xmlFetchResultHistogram = resetGmpTelemetryAndGetHistogram(); const testServerInfo = getTestServerForContentSignatureTests(); Preferences.set(GMPPrefs.KEY_URL_OVERRIDE, testServerInfo.validUpdateUri); @@ -606,6 +609,8 @@ add_task(async function test_checkForAddons_contentSignatureSuccess() { Assert.ok(false, "checkForAddons should succeed"); } + // # Ok content sig fetches should be 1, all others should be 0. + TelemetryTestUtils.assertHistogram(xmlFetchResultHistogram, 2, 1); // Test that glean has 1 success for content sig and no other metrics. const expectedGleanValues = { cert_pin_success: 0, @@ -637,7 +642,7 @@ add_task(async function test_checkForAddons_contentSignatureSuccess() { add_task(async function test_checkForAddons_contentSignatureFailure() { const previousUrlOverride = setupContentSigTestPrefs(); - Services.fog.testResetFOG(); + const xmlFetchResultHistogram = resetGmpTelemetryAndGetHistogram(); const testServerInfo = getTestServerForContentSignatureTests(); Preferences.set( @@ -692,6 +697,8 @@ add_task(async function test_checkForAddons_contentSignatureFailure() { Assert.ok(false, "checkForAddons should succeed"); } + // # Failed content sig fetches should be 1, all others should be 0. + TelemetryTestUtils.assertHistogram(xmlFetchResultHistogram, 3, 1); // Glean values should reflect the content sig algo failed. Assert.equal( Glean.gmp.updateXmlFetchResult.content_sig_missing_data.testGetValue(), @@ -704,8 +711,10 @@ add_task(async function test_checkForAddons_contentSignatureFailure() { // Fail due to bad content signature. Preferences.set(GMPPrefs.KEY_URL_OVERRIDE, testServerInfo.badContentSigUri); await installManager.checkForAddons(); - // Should have another failure and it should be due to the signature being bad, - // which causes verification to fail. + // Should have another failure... + TelemetryTestUtils.assertHistogram(xmlFetchResultHistogram, 3, 2); + // ... and it should be due to the signature being bad, which causes + // verification to fail. Assert.equal( Glean.gmp.updateXmlFetchResult.content_sig_failed.testGetValue(), 1 @@ -717,7 +726,9 @@ add_task(async function test_checkForAddons_contentSignatureFailure() { testServerInfo.invalidContentSigUri ); await installManager.checkForAddons(); - // Should have another failure and it should be due to the signature being invalid. + // Should have another failure... + TelemetryTestUtils.assertHistogram(xmlFetchResultHistogram, 3, 3); + // ... and it should be due to the signature being invalid. Assert.equal( Glean.gmp.updateXmlFetchResult.content_sig_invalid.testGetValue(), 1 @@ -729,7 +740,9 @@ add_task(async function test_checkForAddons_contentSignatureFailure() { "https://this.url.doesnt/go/anywhere" ); await installManager.checkForAddons(); - // Should have another failure and it should be due to a bad request. + // Should have another failure... + TelemetryTestUtils.assertHistogram(xmlFetchResultHistogram, 3, 4); + // ... and it should be due to a bad request. Assert.equal( Glean.gmp.updateXmlFetchResult.content_sig_net_request_error.testGetValue(), 1 @@ -746,7 +759,8 @@ add_task(async function test_checkForAddons_contentSignatureFailure() { overriddenServiceRequest, () => installManager.checkForAddons() ); - // Should have another failure and it should be due to a timeout. + TelemetryTestUtils.assertHistogram(xmlFetchResultHistogram, 3, 5); + // ... and it should be due to a timeout. Assert.equal( Glean.gmp.updateXmlFetchResult.content_sig_net_timeout.testGetValue(), 1 @@ -766,7 +780,9 @@ add_task(async function test_checkForAddons_contentSignatureFailure() { overriddenServiceRequest.abort(); }, 100); await promise; - // Should have another failure and it should be due to an abort. + // Should have another failure... + TelemetryTestUtils.assertHistogram(xmlFetchResultHistogram, 3, 6); + // ... and it should be due to an abort. Assert.equal( Glean.gmp.updateXmlFetchResult.content_sig_abort.testGetValue(), 1 @@ -774,8 +790,9 @@ add_task(async function test_checkForAddons_contentSignatureFailure() { Preferences.set(GMPPrefs.KEY_URL_OVERRIDE, testServerInfo.badXmlUri); await installManager.checkForAddons(); - // Should have another failure and it should be due to the xml response being - // unrecognized. + // Should have another failure... + TelemetryTestUtils.assertHistogram(xmlFetchResultHistogram, 3, 7); + // ... and it should be due to the xml response being unrecognized. Assert.equal( Glean.gmp.updateXmlFetchResult.content_sig_xml_parse_error.testGetValue(), 1 @@ -784,7 +801,9 @@ add_task(async function test_checkForAddons_contentSignatureFailure() { // Fail via bad request during the x5u look up. Preferences.set(GMPPrefs.KEY_URL_OVERRIDE, testServerInfo.badX5uRequestUri); await installManager.checkForAddons(); - // Should have another failure and it should be due to a bad request. + // Should have another failure... + TelemetryTestUtils.assertHistogram(xmlFetchResultHistogram, 3, 8); + // ... and it should be due to a bad request. Assert.equal( Glean.gmp.updateXmlFetchResult.content_sig_net_request_error.testGetValue(), 2 @@ -801,7 +820,9 @@ add_task(async function test_checkForAddons_contentSignatureFailure() { await testServerInfo.promiseHolder.serverPromise; delete testServerInfo.promiseHolder.installPromise; delete testServerInfo.promiseHolder.serverPromise; - // Should have another failure and it should be due to a timeout. + // Should have another failure... + TelemetryTestUtils.assertHistogram(xmlFetchResultHistogram, 3, 9); + // ... and it should be due to a timeout. Assert.equal( Glean.gmp.updateXmlFetchResult.content_sig_net_timeout.testGetValue(), 2 @@ -818,7 +839,9 @@ add_task(async function test_checkForAddons_contentSignatureFailure() { await testServerInfo.promiseHolder.serverPromise; delete testServerInfo.promiseHolder.installPromise; delete testServerInfo.promiseHolder.serverPromise; - // Should have another failure and it should be due to an abort. + // Should have another failure... + TelemetryTestUtils.assertHistogram(xmlFetchResultHistogram, 3, 10); + // ... and it should be due to an abort. Assert.equal( Glean.gmp.updateXmlFetchResult.content_sig_abort.testGetValue(), 2 @@ -915,7 +938,7 @@ add_task(async function test_checkForAddons_telemetry_certPinning() { // Grab state so we can restore it at the end of the test. const previousUrlOverride = Preferences.get(GMPPrefs.KEY_URL_OVERRIDE, ""); - Services.fog.testResetFOG(); + let xmlFetchResultHistogram = resetGmpTelemetryAndGetHistogram(); // Re-use the content-sig test server config. We're not going to need any of // the content signature specific config but this gives us a server to get @@ -936,19 +959,27 @@ add_task(async function test_checkForAddons_telemetry_certPinning() { Assert.ok(false, "checkForAddons should succeed"); } - // Glean values should reflect 1 successful pin fetch. + // # Ok cert pin fetches should be 1, all others should be 0. + TelemetryTestUtils.assertHistogram(xmlFetchResultHistogram, 0, 1); + // Glean values should reflect the same. Assert.equal( Glean.gmp.updateXmlFetchResult.cert_pin_success.testGetValue(), 1 ); + // Reset the histogram because we want to check a different index. + xmlFetchResultHistogram = TelemetryTestUtils.getAndClearHistogram( + "MEDIA_GMP_UPDATE_XML_FETCH_RESULT" + ); // Fail by pointing to a bad URL. Preferences.set( GMPPrefs.KEY_URL_OVERRIDE, "https://this.url.doesnt/go/anywhere" ); await installManager.checkForAddons(); - // Should have another failure and it should be due to a bad request. + // Should have another failure... + TelemetryTestUtils.assertHistogram(xmlFetchResultHistogram, 1, 1); + // ... and it should be due to a bad request. Assert.equal( Glean.gmp.updateXmlFetchResult.cert_pin_net_request_error.testGetValue(), 1 @@ -965,7 +996,8 @@ add_task(async function test_checkForAddons_telemetry_certPinning() { overriddenServiceRequest, () => installManager.checkForAddons() ); - // Should have another failure and it should be due to a timeout. + TelemetryTestUtils.assertHistogram(xmlFetchResultHistogram, 1, 2); + // ... and it should be due to a timeout. Assert.equal( Glean.gmp.updateXmlFetchResult.cert_pin_net_timeout.testGetValue(), 1 @@ -985,7 +1017,9 @@ add_task(async function test_checkForAddons_telemetry_certPinning() { overriddenServiceRequest.abort(); }, 100); await promise; - // Should have another failure and it should be due to an abort. + // Should have another failure... + TelemetryTestUtils.assertHistogram(xmlFetchResultHistogram, 1, 3); + // ... and it should be due to an abort. Assert.equal(Glean.gmp.updateXmlFetchResult.cert_pin_abort.testGetValue(), 1); // Check all glean metrics have expected values at test end. @@ -1918,6 +1952,19 @@ function revertContentSigTestPrefs(previousUrlOverride) { Preferences.set("media.gmp-manager.checkContentSignature", false); } +/*** + * Reset telemetry data related to gmp updates, and get the histogram + * associated with MEDIA_GMP_UPDATE_XML_FETCH_RESULT. + * + * @returns The freshly cleared MEDIA_GMP_UPDATE_XML_FETCH_RESULT histogram. + */ +function resetGmpTelemetryAndGetHistogram() { + Services.fog.testResetFOG(); + return TelemetryTestUtils.getAndClearHistogram( + "MEDIA_GMP_UPDATE_XML_FETCH_RESULT" + ); +} + /*** * A helper to check that glean metrics have expected counts. * @param expectedGleanValues a object that has properties with names set to glean metrics to be checked