Skip to content

Commit

Permalink
Record metrics for enterprise reporting request size
Browse files Browse the repository at this point in the history
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 <jwd@chromium.org>
Reviewed-by: Julian Pastarmov <pastarmovj@chromium.org>
Commit-Queue: Owen Min <zmin@chromium.org>
Cr-Commit-Position: refs/heads/master@{#696159}
  • Loading branch information
Owen Min authored and Commit Bot committed Sep 12, 2019
1 parent 6800bdf commit 63fb580
Show file tree
Hide file tree
Showing 3 changed files with 44 additions and 7 deletions.
33 changes: 26 additions & 7 deletions chrome/browser/enterprise_reporting/report_generator.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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<em::ChromeDesktopReportRequest>(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(
Expand Down Expand Up @@ -197,6 +214,8 @@ void ReportGenerator::OnBasicRequestReady() {
index++) {
GenerateProfileReportWithIndex(index);
}
base::UmaHistogramMemoryKB(kRequestSizeMetricsName,
requests_.back()->ByteSizeLong() / 1024);
Response();
}

Expand Down
10 changes: 10 additions & 0 deletions chrome/browser/enterprise_reporting/report_generator_unittest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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_;
Expand Down Expand Up @@ -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) {
Expand All @@ -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) {
Expand Down Expand Up @@ -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) {
Expand All @@ -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
Expand Down
8 changes: 8 additions & 0 deletions tools/metrics/histograms/histograms.xml
Original file line number Diff line number Diff line change
Expand Up @@ -35677,6 +35677,14 @@ uploading your change for review.
</summary>
</histogram>

<histogram name="Enterprise.CloudReportingRequestSize" units="KB">
<owner>zmin@chromium.org</owner>
<summary>
The proto size of each Chrome browser cloud management reporting request. It
includes the requests which are more than 5MB and discarded.
</summary>
</histogram>

<histogram name="Enterprise.DevicePolicyInvalidations"
enum="EnterprisePolicyInvalidations">
<owner>bartfab@chromium.org</owner>
Expand Down

0 comments on commit 63fb580

Please sign in to comment.