Add DownloadSourcesUsed and DownloadOverheadPercentage metrics.
DownloadSourcesUsed: This metric will tell us the various combinations of
the protocols and servers that were used in completing the download for
each successful update attempt.
DownloadOverheadPercentage: This metric will indicate how efficient our
download mechanisms are by calculating the overhead we incurred as a percentage
of the number of bytes that were actually needed to do the update
successfully.
BUG=chromium:225953
TEST=Unit Tests added, chrome://histograms shows new metrics correctly.
Change-Id: Ic1e9547a9a27e1aad53f7e30c70d822820d2c60f
Reviewed-on: https://gerrit.chromium.org/gerrit/48856
Commit-Queue: Jay Srinivasan <[email protected]>
Reviewed-by: Jay Srinivasan <[email protected]>
Tested-by: Jay Srinivasan <[email protected]>
diff --git a/payload_state.cc b/payload_state.cc
index 31c4e42..5ab6028 100644
--- a/payload_state.cc
+++ b/payload_state.cc
@@ -404,14 +404,24 @@
// Report metrics collected from all known download sources to UMA.
// The reported data is in Megabytes in order to represent a larger
// sample range.
+ int download_sources_used = 0;
+ string metric;
+ uint64_t successful_mbs = 0;
+ uint64_t total_mbs = 0;
for (int i = 0; i < kNumDownloadSources; i++) {
DownloadSource source = static_cast<DownloadSource>(i);
const int kMaxMiBs = 10240; // Anything above 10GB goes in the last bucket.
- string metric = "Installer.SuccessfulMBsDownloadedFrom" +
- utils::ToString(source);
+ metric = "Installer.SuccessfulMBsDownloadedFrom" + utils::ToString(source);
uint64_t mbs = GetCurrentBytesDownloaded(source) / kNumBytesInOneMiB;
- LOG(INFO) << "Uploading " << mbs << " (MBs) for metric " << metric;
+
+ // Count this download source as having been used if we downloaded any
+ // bytes that contributed to the final success of the update.
+ if (mbs)
+ download_sources_used |= (1 << source);
+
+ successful_mbs += mbs;
+ LOG(INFO) << "Uploading " << mbs << " (MBs) for metric " << metric;
system_state_->metrics_lib()->SendToUMA(metric,
mbs,
0, // min
@@ -421,7 +431,8 @@
metric = "Installer.TotalMBsDownloadedFrom" + utils::ToString(source);
mbs = GetTotalBytesDownloaded(source) / kNumBytesInOneMiB;
- LOG(INFO) << "Uploading " << mbs << " (MBs) for metric " << metric;
+ total_mbs += mbs;
+ LOG(INFO) << "Uploading " << mbs << " (MBs) for metric " << metric;
system_state_->metrics_lib()->SendToUMA(metric,
mbs,
0, // min
@@ -430,6 +441,27 @@
SetTotalBytesDownloaded(source, 0, true);
}
+
+ metric = "Installer.DownloadSourcesUsed";
+ LOG(INFO) << "Uploading 0x" << std::hex << download_sources_used
+ << " (bit flags) for metric " << metric;
+ int num_buckets = std::min(1 << kNumDownloadSources, kNumDefaultUmaBuckets);
+ system_state_->metrics_lib()->SendToUMA(metric,
+ download_sources_used,
+ 0, // min
+ 1 << kNumDownloadSources,
+ num_buckets);
+
+ if (successful_mbs) {
+ metric = "Installer.DownloadOverheadPercentage";
+ int percent_overhead = (total_mbs - successful_mbs) * 100 / successful_mbs;
+ LOG(INFO) << "Uploading " << percent_overhead << "% for metric " << metric;
+ system_state_->metrics_lib()->SendToUMA(metric,
+ percent_overhead,
+ 0, // min: 0% overhead
+ 1000, // max: 1000% overhead
+ kNumDefaultUmaBuckets);
+ }
}
void PayloadState::ReportUpdateUrlSwitchesMetric() {
diff --git a/payload_state_unittest.cc b/payload_state_unittest.cc
index f7688c2..440d8b7 100644
--- a/payload_state_unittest.cc
+++ b/payload_state_unittest.cc
@@ -28,6 +28,7 @@
using testing::Return;
using testing::SetArgumentPointee;
using testing::AtLeast;
+using testing::AnyNumber;
namespace chromeos_update_engine {
@@ -88,7 +89,7 @@
OmahaResponse response;
MockSystemState mock_system_state;
NiceMock<PrefsMock>* prefs = mock_system_state.mock_prefs();
- EXPECT_CALL(*prefs, SetInt64(_,_)).Times(AtLeast(0));
+ EXPECT_CALL(*prefs, SetInt64(_,_)).Times(AnyNumber());
EXPECT_CALL(*prefs, SetInt64(kPrefsPayloadAttemptNumber, 0))
.Times(AtLeast(1));
EXPECT_CALL(*prefs, SetInt64(kPrefsBackoffExpiryTime, 0)).Times(AtLeast(1));
@@ -131,7 +132,7 @@
response.metadata_signature = "msign";
MockSystemState mock_system_state;
NiceMock<PrefsMock>* prefs = mock_system_state.mock_prefs();
- EXPECT_CALL(*prefs, SetInt64(_,_)).Times(AtLeast(0));
+ EXPECT_CALL(*prefs, SetInt64(_,_)).Times(AnyNumber());
EXPECT_CALL(*prefs, SetInt64(kPrefsPayloadAttemptNumber, 0))
.Times(AtLeast(1));
EXPECT_CALL(*prefs, SetInt64(kPrefsBackoffExpiryTime, 0))
@@ -179,7 +180,7 @@
response.metadata_signature = "metasign";
MockSystemState mock_system_state;
NiceMock<PrefsMock>* prefs = mock_system_state.mock_prefs();
- EXPECT_CALL(*prefs, SetInt64(_,_)).Times(AtLeast(0));
+ EXPECT_CALL(*prefs, SetInt64(_,_)).Times(AnyNumber());
EXPECT_CALL(*prefs, SetInt64(kPrefsPayloadAttemptNumber, 0))
.Times(AtLeast(1));
EXPECT_CALL(*prefs, SetInt64(kPrefsBackoffExpiryTime, 0))
@@ -220,7 +221,7 @@
NiceMock<PrefsMock>* prefs = mock_system_state.mock_prefs();
PayloadState payload_state;
- EXPECT_CALL(*prefs, SetInt64(_,_)).Times(AtLeast(0));
+ EXPECT_CALL(*prefs, SetInt64(_,_)).Times(AnyNumber());
// Payload attempt should start with 0 and then advance to 1.
EXPECT_CALL(*prefs, SetInt64(kPrefsPayloadAttemptNumber, 0))
.Times(AtLeast(1));
@@ -304,7 +305,7 @@
int progress_bytes = 100;
NiceMock<PrefsMock>* prefs = mock_system_state.mock_prefs();
- EXPECT_CALL(*prefs, SetInt64(_,_)).Times(AtLeast(0));
+ EXPECT_CALL(*prefs, SetInt64(_,_)).Times(AnyNumber());
EXPECT_CALL(*prefs, SetInt64(kPrefsPayloadAttemptNumber, 0))
.Times(AtLeast(2));
EXPECT_CALL(*prefs, SetInt64(kPrefsPayloadAttemptNumber, 1))
@@ -428,7 +429,7 @@
MockSystemState mock_system_state;
NiceMock<PrefsMock>* prefs = mock_system_state.mock_prefs();
- EXPECT_CALL(*prefs, SetInt64(_,_)).Times(AtLeast(0));
+ EXPECT_CALL(*prefs, SetInt64(_,_)).Times(AnyNumber());
EXPECT_CALL(*prefs, SetInt64(kPrefsPayloadAttemptNumber, 0))
.Times(AtLeast(1));
EXPECT_CALL(*prefs, SetInt64(kPrefsPayloadAttemptNumber, 1))
@@ -602,8 +603,23 @@
EXPECT_TRUE(payload_state.Initialize(&mock_system_state));
SetupPayloadStateWith2Urls("Hash3286", &payload_state, &response);
- // Simulate a successful download and see that we are ready to download
- // again without any backoff.
+ // Simulate a previous attempt with in order to set an initial non-zero value
+ // for the total bytes downloaded for HTTP.
+ int prev_chunk = 323456789;
+ http_total += prev_chunk;
+ payload_state.DownloadProgress(prev_chunk);
+
+ // Ensure that the initial values for HTTP reflect this attempt.
+ EXPECT_EQ(prev_chunk,
+ payload_state.GetCurrentBytesDownloaded(kDownloadSourceHttpServer));
+ EXPECT_EQ(http_total,
+ payload_state.GetTotalBytesDownloaded(kDownloadSourceHttpServer));
+
+ // Change the response hash so as to simulate a new response which will
+ // reset the current bytes downloaded, but not the total bytes downloaded.
+ SetupPayloadStateWith2Urls("Hash9904", &payload_state, &response);
+
+ // First, simulate successful download of a few bytes over HTTP.
int first_chunk = 5000000;
http_total += first_chunk;
payload_state.DownloadProgress(first_chunk);
@@ -645,16 +661,15 @@
// Test that third chunk is again back on HTTP. HTTPS remains on second chunk.
EXPECT_EQ(http_chunk,
payload_state.GetCurrentBytesDownloaded(kDownloadSourceHttpServer));
- EXPECT_EQ(http_chunk,
+ EXPECT_EQ(http_total,
payload_state.GetTotalBytesDownloaded(kDownloadSourceHttpServer));
EXPECT_EQ(second_chunk, payload_state.GetCurrentBytesDownloaded(
kDownloadSourceHttpsServer));
EXPECT_EQ(https_total,
payload_state.GetTotalBytesDownloaded(kDownloadSourceHttpsServer));
- // Don't care about other metrics in this test.
- EXPECT_CALL(*mock_system_state.mock_metrics_lib(), SendToUMA(
- _,_,_,_,_)).Times(AtLeast(0));
+ EXPECT_CALL(*mock_system_state.mock_metrics_lib(), SendToUMA(_, _, _, _, _))
+ .Times(AnyNumber());
EXPECT_CALL(*mock_system_state.mock_metrics_lib(), SendToUMA(
"Installer.SuccessfulMBsDownloadedFromHttpServer",
http_chunk / kNumBytesInOneMiB, _, _, _));
@@ -676,6 +691,10 @@
EXPECT_CALL(*mock_system_state.mock_metrics_lib(), SendToUMA(
"Installer.UpdateDurationUptimeMinutes",
_, _, _, _));
+ EXPECT_CALL(*mock_system_state.mock_metrics_lib(), SendToUMA(
+ "Installer.DownloadSourcesUsed", 3, _, _, _));
+ EXPECT_CALL(*mock_system_state.mock_metrics_lib(), SendToUMA(
+ "Installer.DownloadOverheadPercentage", 542, _, _, _));
payload_state.UpdateSucceeded();