From 63fb5800277fbf91bf937310c3a0222149f69602 Mon Sep 17 00:00:00 2001 From: Owen Min Date: Thu, 12 Sep 2019 20:52:12 +0000 Subject: [PATCH] Record metrics for enterprise reporting request size This also includes the requests that are discarded due to their size. Also update the way of calculating the estimated report size. Using incremental size for better result. Bug: 956237 Change-Id: I873cd5a0f5463e111da12fab98a50b8f1640ed0d Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1799343 Reviewed-by: Jesse Doherty Reviewed-by: Julian Pastarmov Commit-Queue: Owen Min Cr-Commit-Position: refs/heads/master@{#696159} --- .../enterprise_reporting/report_generator.cc | 33 +++++++++++++++---- .../report_generator_unittest.cc | 10 ++++++ tools/metrics/histograms/histograms.xml | 8 +++++ 3 files changed, 44 insertions(+), 7 deletions(-) diff --git a/chrome/browser/enterprise_reporting/report_generator.cc b/chrome/browser/enterprise_reporting/report_generator.cc index 1555a02258b24c..49da1e239f13e1 100644 --- a/chrome/browser/enterprise_reporting/report_generator.cc +++ b/chrome/browser/enterprise_reporting/report_generator.cc @@ -34,8 +34,11 @@ namespace { const size_t kMaximumReportSize = 5000000; // The report size limitation is 5mb. + constexpr char kRequestCountMetricsName[] = "Enterprise.CloudReportingRequestCount"; +constexpr char kRequestSizeMetricsName[] = + "Enterprise.CloudReportingRequestSize"; // Because server only stores 20 profiles for each report and when report is // separated into requests, there is at least one profile per request. It means @@ -142,29 +145,43 @@ void ReportGenerator::GenerateProfileReportWithIndex(int profile_index) { return; } - size_t profile_report_size = profile_report->ByteSizeLong(); + // Use size diff to calculate estimated request size after full profile report + // is added. There are still few bytes difference but close enough. + size_t profile_report_incremental_size = + profile_report->ByteSizeLong() - + basic_request_.browser_report() + .chrome_user_profile_infos(profile_index) + .ByteSizeLong(); size_t current_request_size = requests_.back()->ByteSizeLong(); - if (current_request_size + profile_report_size <= maximum_report_size_) { + if (current_request_size + profile_report_incremental_size <= + maximum_report_size_) { // The new full Profile report can be appended into the current request. requests_.back() ->mutable_browser_report() ->mutable_chrome_user_profile_infos(profile_index) ->Swap(profile_report.get()); - } else if (basic_request_size_ + profile_report_size <= + } else if (basic_request_size_ + profile_report_incremental_size <= maximum_report_size_) { // The new full Profile report is too big to be appended into the current - // request, move it to the next request if possible. + // request, move it to the next request if possible. Record metrics for the + // current request's size. + base::UmaHistogramMemoryKB(kRequestSizeMetricsName, + requests_.back()->ByteSizeLong() / 1024); requests_.push( std::make_unique(basic_request_)); requests_.back() ->mutable_browser_report() ->mutable_chrome_user_profile_infos(profile_index) ->Swap(profile_report.get()); + } else { + // The new full Profile report is too big to be uploaded, skip this + // Profile report. But we still add the report size into metrics so + // that we could understand the situation better. + base::UmaHistogramMemoryKB( + kRequestSizeMetricsName, + (basic_request_size_ + profile_report_incremental_size) / 1024); } - // Else: The new full Profile report is too big to be uploaded, skip this - // Profile report. - // TODO(crbug.com/956237): Record this event with UMA metrics. } void ReportGenerator::OnPluginsReady( @@ -197,6 +214,8 @@ void ReportGenerator::OnBasicRequestReady() { index++) { GenerateProfileReportWithIndex(index); } + base::UmaHistogramMemoryKB(kRequestSizeMetricsName, + requests_.back()->ByteSizeLong() / 1024); Response(); } diff --git a/chrome/browser/enterprise_reporting/report_generator_unittest.cc b/chrome/browser/enterprise_reporting/report_generator_unittest.cc index 82ef91f1ee3942..5241fa6218c300 100644 --- a/chrome/browser/enterprise_reporting/report_generator_unittest.cc +++ b/chrome/browser/enterprise_reporting/report_generator_unittest.cc @@ -201,6 +201,7 @@ class ReportGeneratorTest : public ::testing::Test { TestingProfileManager* profile_manager() { return &profile_manager_; } ReportGenerator* generator() { return &generator_; } + base::HistogramTester* histogram_tester() { return histogram_tester_.get(); } private: ReportGenerator generator_; @@ -257,6 +258,9 @@ TEST_F(ReportGeneratorTest, GenerateActiveProfiles) { VerifyProfileReport(active_profiles_names, inactive_profiles_names, requests[0]->browser_report()); + + histogram_tester()->ExpectBucketCount("Enterprise.CloudReportingRequestSize", + /*report size floor to KB*/ 0, 1); } TEST_F(ReportGeneratorTest, BasicReportIsTooBig) { @@ -268,6 +272,8 @@ TEST_F(ReportGeneratorTest, BasicReportIsTooBig) { // Because the limitation is so small, no request can be created. auto requests = GenerateRequests(); EXPECT_EQ(0u, requests.size()); + histogram_tester()->ExpectTotalCount("Enterprise.CloudReportingRequestSize", + 0); } TEST_F(ReportGeneratorTest, ReportSeparation) { @@ -295,6 +301,8 @@ TEST_F(ReportGeneratorTest, ReportSeparation) { requests[0]->browser_report()); VerifyProfileReport(second_request_profiles, first_request_profiles, requests[1]->browser_report()); + histogram_tester()->ExpectBucketCount("Enterprise.CloudReportingRequestSize", + /*report size floor to KB*/ 0, 2); } TEST_F(ReportGeneratorTest, ProfileReportIsTooBig) { @@ -317,6 +325,8 @@ TEST_F(ReportGeneratorTest, ProfileReportIsTooBig) { // reported. VerifyProfileReport(second_profile_name, first_profile_name, requests[0]->browser_report()); + histogram_tester()->ExpectBucketCount("Enterprise.CloudReportingRequestSize", + /*report size floor to KB*/ 0, 2); } #endif diff --git a/tools/metrics/histograms/histograms.xml b/tools/metrics/histograms/histograms.xml index eb7defc5acbcb8..a88cc4185bcbfe 100644 --- a/tools/metrics/histograms/histograms.xml +++ b/tools/metrics/histograms/histograms.xml @@ -35677,6 +35677,14 @@ uploading your change for review. + + zmin@chromium.org + + The proto size of each Chrome browser cloud management reporting request. It + includes the requests which are more than 5MB and discarded. + + + bartfab@chromium.org