Skip to content

GitLab

T
tangled
Commit 42c0d706 authored by Minh X. Nguyen's avatar Minh X. Nguyen Committed by Commit Bot
Browse files
Options

[Extensions]: Add more UMA histograms to help investigate Finch experiment. 

The Finch experiment is showing that the experiment group has much more
extensions than the control group. Adding these new histograms may help us
understand more about what's happening.

Bug: 722942
Change-Id: Ia3308e61efeeb533c31638ca0f0ecf500c7e0f9c
Reviewed-on: https://chromium-review.googlesource.com/1091695Reviewed-by: default avatarDevlin <rdevlin.cronin@chromium.org>
Reviewed-by: default avatarRobert Kaplow <rkaplow@chromium.org>
Commit-Queue: Minh Nguyen <mxnguyen@chromium.org>
Cr-Commit-Position: refs/heads/master@{#566924}
parent d158b83c
Hide whitespace changes
Inline Side-by-side
Showing with 119 additions and 72 deletions
chrome/browser/extensions/updater/extension_updater.cc
View file @ 42c0d706
......@@ -408,6 +408,10 @@ void ExtensionUpdater::CheckNow(CheckParams params) {
}
}
UMA_HISTOGRAM_COUNTS_100(
"Extensions.ExtensionUpdaterRawUpdateCalls",
request.in_progress_ids_.size() + update_check_params.update_info.size());
// StartAllPending() might call OnExtensionDownloadFailed/Finished before
// it returns, which would cause NotifyIfFinished to incorrectly try to
// send out a notification. So check before we call StartAllPending if any
......
chrome/browser/extensions/updater/update_service_browsertest.cc
View file @ 42c0d706
......@@ -79,6 +79,9 @@ IN_PROC_BROWSER_TEST_F(UpdateServiceTest, NoUpdate) {
WaitOnComponentUpdaterCompleteEvent(kExtensionId));
content::FetchHistogramsFromChildProcesses();
EXPECT_THAT(histogram_tester.GetAllSamples(
"Extensions.ExtensionUpdaterRawUpdateCalls"),
testing::ElementsAre(base::Bucket(1, 1)));
EXPECT_THAT(
histogram_tester.GetAllSamples("Extensions.ExtensionUpdaterUpdateCalls"),
testing::ElementsAre(base::Bucket(1, 1)));
......@@ -134,6 +137,9 @@ IN_PROC_BROWSER_TEST_F(UpdateServiceTest, UpdateCheckError) {
EXPECT_THAT(
histogram_tester.GetAllSamples("Extensions.ExtensionUpdaterUpdateCalls"),
testing::ElementsAre(base::Bucket(1, 1)));
EXPECT_THAT(histogram_tester.GetAllSamples(
"Extensions.ExtensionUpdaterRawUpdateCalls"),
testing::ElementsAre(base::Bucket(1, 1)));
EXPECT_THAT(
histogram_tester.GetAllSamples(
"Extensions.ExtensionUpdaterUpdateResults"),
......@@ -195,6 +201,9 @@ IN_PROC_BROWSER_TEST_F(UpdateServiceTest, TwoUpdateCheckErrors) {
run_loop2.Run();
content::FetchHistogramsFromChildProcesses();
EXPECT_THAT(histogram_tester.GetAllSamples(
"Extensions.ExtensionUpdaterRawUpdateCalls"),
testing::ElementsAre(base::Bucket(1, 1), base::Bucket(2, 1)));
EXPECT_THAT(
histogram_tester.GetAllSamples("Extensions.ExtensionUpdaterUpdateCalls"),
testing::ElementsAre(base::Bucket(1, 1), base::Bucket(2, 1)));
......@@ -255,6 +264,9 @@ IN_PROC_BROWSER_TEST_F(UpdateServiceTest, SuccessfulUpdate) {
run_loop.Run();
content::FetchHistogramsFromChildProcesses();
EXPECT_THAT(histogram_tester.GetAllSamples(
"Extensions.ExtensionUpdaterRawUpdateCalls"),
testing::ElementsAre(base::Bucket(1, 1)));
EXPECT_THAT(
histogram_tester.GetAllSamples("Extensions.ExtensionUpdaterUpdateCalls"),
testing::ElementsAre(base::Bucket(1, 1)));
......
extensions/browser/updater/extension_downloader.cc
View file @ 42c0d706
......@@ -457,18 +457,13 @@ void ExtensionDownloader::StartUpdateCheck(
return;
}
const std::set<std::string>& id_set(fetch_data->extension_ids());
if (!ExtensionsBrowserClient::Get()->IsBackgroundUpdateAllowed()) {
NotifyExtensionsDownloadFailed(id_set,
NotifyExtensionsDownloadFailed(fetch_data->extension_ids(),
fetch_data->request_ids(),
ExtensionDownloaderDelegate::DISABLED);
return;
}
UMA_HISTOGRAM_COUNTS_100("Extensions.ExtensionUpdaterUpdateCalls",
id_set.size());
RequestQueue<ManifestFetchData>::iterator i;
for (i = manifests_queue_.begin(); i != manifests_queue_.end(); ++i) {
if (fetch_data->full_url() == i->full_url()) {
......@@ -482,6 +477,8 @@ void ExtensionDownloader::StartUpdateCheck(
manifests_queue_.active_request()->full_url() == fetch_data->full_url()) {
manifests_queue_.active_request()->Merge(*fetch_data);
} else {
UMA_HISTOGRAM_COUNTS_100("Extensions.ExtensionUpdaterUpdateCalls",
fetch_data->extension_ids().size());
UMA_HISTOGRAM_COUNTS(
"Extensions.UpdateCheckUrlLength",
fetch_data->full_url().possibly_invalid_spec().length());
......
extensions/browser/updater/safe_manifest_parser.cc
View file @ 42c0d706
......@@ -7,6 +7,7 @@
#include <memory>
#include "base/bind.h"
#include "base/metrics/histogram_macros.h"
#include "base/optional.h"
#include "base/strings/string_number_conversions.h"
#include "base/values.h"
......@@ -39,7 +40,8 @@ void ReportError(ParseUpdateManifestCallback callback,
bool ParseSingleAppTag(const base::Value& app_element,
const std::string& xml_namespace,
UpdateManifestResult* result,
std::string* error_detail) {
std::string* error_detail,
int* prodversionmin_count) {
// Read the extension id.
result->extension_id = GetXmlElementAttribute(app_element, "appid");
if (result->extension_id.empty()) {
......@@ -65,6 +67,20 @@ bool ParseSingleAppTag(const base::Value& app_element,
if (GetXmlElementAttribute(*updatecheck, "status") == "noupdate")
return true;
// Get the optional minimum browser version.
result->browser_min_version =
GetXmlElementAttribute(*updatecheck, "prodversionmin");
if (!result->browser_min_version.empty()) {
*prodversionmin_count += 1;
base::Version browser_min_version(result->browser_min_version);
if (!browser_min_version.IsValid()) {
*error_detail = "Invalid prodversionmin: '";
*error_detail += result->browser_min_version;
*error_detail += "'.";
return false;
}
}
// Find the url to the crx file.
result->crx_url = GURL(GetXmlElementAttribute(*updatecheck, "codebase"));
if (!result->crx_url.is_valid()) {
......@@ -88,19 +104,6 @@ bool ParseSingleAppTag(const base::Value& app_element,
return false;
}
// Get the optional minimum browser version.
result->browser_min_version =
GetXmlElementAttribute(*updatecheck, "prodversionmin");
if (!result->browser_min_version.empty()) {
base::Version browser_min_version(result->browser_min_version);
if (!browser_min_version.IsValid()) {
*error_detail = "Invalid prodversionmin: '";
*error_detail += result->browser_min_version;
*error_detail += "'.";
return false;
}
}
// package_hash is optional. It is a sha256 hash of the package in hex format.
result->package_hash = GetXmlElementAttribute(*updatecheck, "hash_sha256");
......@@ -182,10 +185,12 @@ void ParseXmlDone(ParseUpdateManifestCallback callback,
data_decoder::GetAllXmlElementChildrenWithTag(
*root, GetXmlQualifiedName(gupdate_ns, "app"), &apps);
std::string error_msg;
int prodversionmin_count = 0;
for (const auto* app : apps) {
UpdateManifestResult result;
std::string app_error;
if (!ParseSingleAppTag(*app, gupdate_ns, &result, &app_error)) {
if (!ParseSingleAppTag(*app, gupdate_ns, &result, &app_error,
&prodversionmin_count)) {
if (!error_msg.empty())
error_msg += "\r\n"; // Should we have an OS specific EOL?
error_msg += app_error;
......@@ -194,6 +199,9 @@ void ParseXmlDone(ParseUpdateManifestCallback callback,
}
}
UMA_HISTOGRAM_COUNTS_100("Extensions.UpdateManifestHasProdVersionMinCounts",
prodversionmin_count);
std::move(callback).Run(
results->list.empty() ? nullptr : std::move(results),
error_msg.empty() ? base::Optional<std::string>() : error_msg);
......
extensions/browser/updater/update_service.cc
View file @ 42c0d706
......@@ -31,12 +31,29 @@ namespace extensions {
namespace {
// 98% of update checks have 23 or less extensions
// (see Extensions.UpdateCheckExtension histogram).
constexpr size_t kMaxExtensionsPerUpdate = 23;
// 98% of update checks have 22 or less extensions.
constexpr size_t kMaxExtensionsPerUpdate = 22;
void SendUninstallPingCompleteCallback(update_client::Error error) {}
void ReportUpdateCheckResult(ExtensionUpdaterUpdateResult update_result,
int error_code) {
UMA_HISTOGRAM_ENUMERATION("Extensions.ExtensionUpdaterUpdateResults",
update_result,
ExtensionUpdaterUpdateResult::UPDATE_RESULT_COUNT);
// This UMA histogram measures update check results of the unified extension
// updater.
UMA_HISTOGRAM_ENUMERATION("Extensions.UnifiedExtensionUpdaterUpdateResults",
update_result,
ExtensionUpdaterUpdateResult::UPDATE_RESULT_COUNT);
if (update_result == ExtensionUpdaterUpdateResult::UPDATE_CHECK_ERROR)
base::UmaHistogramSparse(
"Extensions.UnifiedExtensionUpdaterUpdateCheckErrors", error_code);
else if (update_result == ExtensionUpdaterUpdateResult::UPDATE_DOWNLOAD_ERROR)
base::UmaHistogramSparse("Extensions.UnifiedExtensionUpdaterDownloadErrors",
error_code);
}
} // namespace
UpdateService::InProgressUpdate::InProgressUpdate(base::OnceClosure cb)
......@@ -111,10 +128,7 @@ void UpdateService::OnEvent(Events event, const std::string& extension_id) {
switch (event) {
case Events::COMPONENT_UPDATED:
complete_event = true;
UMA_HISTOGRAM_ENUMERATION(
"Extensions.ExtensionUpdaterUpdateResults",
ExtensionUpdaterUpdateResult::UPDATE_SUCCESS,
ExtensionUpdaterUpdateResult::UPDATE_RESULT_COUNT);
ReportUpdateCheckResult(ExtensionUpdaterUpdateResult::UPDATE_SUCCESS, 0);
break;
case Events::COMPONENT_UPDATE_ERROR:
complete_event = true;
......@@ -125,46 +139,31 @@ void UpdateService::OnEvent(Events event, const std::string& extension_id) {
}
switch (update_item.error_category) {
case update_client::ErrorCategory::kUpdateCheck:
UMA_HISTOGRAM_ENUMERATION(
"Extensions.ExtensionUpdaterUpdateResults",
ReportUpdateCheckResult(
ExtensionUpdaterUpdateResult::UPDATE_CHECK_ERROR,
ExtensionUpdaterUpdateResult::UPDATE_RESULT_COUNT);
base::UmaHistogramSparse(
"Extensions.UnifiedExtensionUpdaterUpdateCheckErrors",
update_item.error_code);
break;
case update_client::ErrorCategory::kDownload:
UMA_HISTOGRAM_ENUMERATION(
"Extensions.ExtensionUpdaterUpdateResults",
ReportUpdateCheckResult(
ExtensionUpdaterUpdateResult::UPDATE_DOWNLOAD_ERROR,
ExtensionUpdaterUpdateResult::UPDATE_RESULT_COUNT);
base::UmaHistogramSparse(
"Extensions.UnifiedExtensionUpdaterDownloadErrors",
update_item.error_code);
break;
case update_client::ErrorCategory::kUnpack:
case update_client::ErrorCategory::kInstall:
UMA_HISTOGRAM_ENUMERATION(
"Extensions.ExtensionUpdaterUpdateResults",
ExtensionUpdaterUpdateResult::UPDATE_INSTALL_ERROR,
ExtensionUpdaterUpdateResult::UPDATE_RESULT_COUNT);
ReportUpdateCheckResult(
ExtensionUpdaterUpdateResult::UPDATE_INSTALL_ERROR, 0);
break;
case update_client::ErrorCategory::kNone:
case update_client::ErrorCategory::kService:
UMA_HISTOGRAM_ENUMERATION(
"Extensions.ExtensionUpdaterUpdateResults",
ExtensionUpdaterUpdateResult::UPDATE_SERVICE_ERROR,
ExtensionUpdaterUpdateResult::UPDATE_RESULT_COUNT);
ReportUpdateCheckResult(
ExtensionUpdaterUpdateResult::UPDATE_SERVICE_ERROR, 0);
break;
}
}
break;
case Events::COMPONENT_NOT_UPDATED:
complete_event = true;
UMA_HISTOGRAM_ENUMERATION(
"Extensions.ExtensionUpdaterUpdateResults",
ExtensionUpdaterUpdateResult::NO_UPDATE,
ExtensionUpdaterUpdateResult::UPDATE_RESULT_COUNT);
ReportUpdateCheckResult(ExtensionUpdaterUpdateResult::NO_UPDATE, 0);
// When no update is found, a previous update check might have queued an
// update for this extension because it was in use at the time. We should
// ask for the install of the queued update now if it's ready.
......
extensions/browser/updater/update_service_unittest.cc
View file @ 42c0d706
......@@ -809,23 +809,23 @@ TEST_F(UpdateServiceTest, InProgressUpdate_Batch) {
testing::ElementsAre("A00", "A01", "A02", "A03", "A04", "A05",
"A06", "A07", "A08", "A09", "A10", "A11",
"A12", "A13", "A14", "A15", "A16", "A17",
"A18", "A19", "A20", "A21", "A22"));
"A18", "A19", "A20", "A21"));
EXPECT_THAT(request2.extension_ids,
testing::ElementsAre("A23", "A24", "A25", "A26", "A27", "A28",
"A29", "A30", "A31", "A32", "A33", "A34",
"A35", "A36", "A37", "A38", "A39", "A40",
"A41", "A42", "A43", "A44", "A45"));
EXPECT_THAT(
request3.extension_ids,
testing::ElementsAre("A46", "A47", "A48", "A49", "A50", "A51", "A52",
"A53", "A54", "A55", "A56", "A57", "A58", "A59"));
testing::ElementsAre("A22", "A23", "A24", "A25", "A26", "A27",
"A28", "A29", "A30", "A31", "A32", "A33",
"A34", "A35", "A36", "A37", "A38", "A39",
"A40", "A41", "A42", "A43"));
EXPECT_THAT(request3.extension_ids,
testing::ElementsAre("A44", "A45", "A46", "A47", "A48", "A49",
"A50", "A51", "A52", "A53", "A54", "A55",
"A56", "A57", "A58", "A59"));
EXPECT_THAT(
histogram_tester.GetAllSamples("Extensions.ExtensionUpdaterUpdateCalls"),
testing::ElementsAre(base::Bucket(14, 1), base::Bucket(23, 2)));
testing::ElementsAre(base::Bucket(16, 1), base::Bucket(22, 2)));
EXPECT_THAT(histogram_tester.GetAllSamples(
"Extensions.UnifiedExtensionUpdaterUpdateCalls"),
testing::ElementsAre(base::Bucket(14, 1), base::Bucket(23, 2)));
testing::ElementsAre(base::Bucket(16, 1), base::Bucket(22, 2)));
update_client()->RunDelayedUpdate(0);
EXPECT_FALSE(executed);
......@@ -879,24 +879,24 @@ TEST_F(UpdateServiceTest, InProgressUpdate_NoBatchAndBatch) {
testing::ElementsAre("A00", "A01", "A02", "A03", "A04", "A05",
"A06", "A07", "A08", "A09", "A10", "A11",
"A12", "A13", "A14", "A15", "A16", "A17",
"A18", "A19", "A20", "A21", "A22"));
"A18", "A19", "A20", "A21"));
EXPECT_THAT(request3.extension_ids,
testing::ElementsAre("A23", "A24", "A25", "A26", "A27", "A28",
"A29", "A30", "A31", "A32", "A33", "A34",
"A35", "A36", "A37", "A38", "A39", "A40",
"A41", "A42", "A43", "A44", "A45"));
testing::ElementsAre("A22", "A23", "A24", "A25", "A26", "A27",
"A28", "A29", "A30", "A31", "A32", "A33",
"A34", "A35", "A36", "A37", "A38", "A39",
"A40", "A41", "A42", "A43"));
EXPECT_THAT(request4.extension_ids,
testing::ElementsAre("A46", "A47", "A48", "A49", "A50", "A51",
"A52", "A53", "A54"));
testing::ElementsAre("A44", "A45", "A46", "A47", "A48", "A49",
"A50", "A51", "A52", "A53", "A54"));
EXPECT_THAT(
histogram_tester.GetAllSamples("Extensions.ExtensionUpdaterUpdateCalls"),
testing::ElementsAre(base::Bucket(4, 1), base::Bucket(9, 1),
base::Bucket(23, 2)));
testing::ElementsAre(base::Bucket(4, 1), base::Bucket(11, 1),
base::Bucket(22, 2)));
EXPECT_THAT(histogram_tester.GetAllSamples(
"Extensions.UnifiedExtensionUpdaterUpdateCalls"),
testing::ElementsAre(base::Bucket(4, 1), base::Bucket(9, 1),
base::Bucket(23, 2)));
testing::ElementsAre(base::Bucket(4, 1), base::Bucket(11, 1),
base::Bucket(22, 2)));
update_client()->RunDelayedUpdate(0);
EXPECT_TRUE(executed1);
......
tools/metrics/histograms/histograms.xml
View file @ 42c0d706
......@@ -27138,6 +27138,15 @@ uploading your change for review.
<summary>An extension has been uninstalled.</summary>
</histogram>
<histogram name="Extensions.ExtensionUpdaterRawUpdateCalls" units="extensions">
<owner>mxnguyen@chromium.org</owner>
<summary>
The number of extensions that are checked for update. This number is emitted
right before the extensions are split between the current extension updater
and the unified extension updater (if used).
</summary>
</histogram>
<histogram name="Extensions.ExtensionUpdaterUpdateCalls" units="extensions">
<owner>mxnguyen@chromium.org</owner>
<summary>
......@@ -28875,6 +28884,15 @@ uploading your change for review.
</summary>
</histogram>
<histogram name="Extensions.UnifiedExtensionUpdaterUpdateResults"
enum="ExtensionUpdaterUpdateResult">
<owner>mxnguyen@chromium.org</owner>
<summary>
Records the update results of extensions in an unified extension updater
session, grouped by ExtensionUpdaterUpdateResult.
</summary>
</histogram>
<histogram name="Extensions.UninstallDialogAction"
enum="ExtensionUninstallDialogAction">
<owner>rdevlin.cronin@chromium.org</owner>
......@@ -29003,6 +29021,15 @@ uploading your change for review.
</summary>
</histogram>
<histogram name="Extensions.UpdateManifestHasProdVersionMinCounts"
units="extensions">
<owner>mxnguyen@chromium.org</owner>
<summary>
The number of prodversionmin attributes appearing in an update manifest of
an update check request.
</summary>
</histogram>
<histogram name="Extensions.UpdateOnLoad">
<owner>Please list the metric's owners. Add more owner tags as needed.</owner>
<summary>
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment