Add Installer.UpdatesAbandonedEventCount metric.
This patch adds a new metric similar to
Installer.UpdatesAbandonedCount that reports the number of abandoned
payloads every time a payload is abandoned. A payload is considered
"abandoned" when it was already attempted but not finished for any
reason, and a newer payload is avaibled. In this situation,
update_engine drops the previous payload and starts again with the
new one.
Every time a payload is abandoned, the new metric
Installer.UpdatesAbandonedEventCount is sent with the number of
consecutive different abandoned payloads since the last successful
update. A high number of entries greater than 1 in this metric will
imply that we are releasing new payloads too often, or that the users
are taking too long to download an update.
Finally, when a payload is successfully applied, the total number of
abandoned payloads since the previous successful update is sent as
the the metric Installer.UpdatesAbandonedCount.
BUG=chromium:261370
TEST=unittests
Change-Id: I47054b541c4700a86183461a5cf92830883ea4f3
Reviewed-on: https://gerrit.chromium.org/gerrit/65328
Commit-Queue: Alex Deymo <deymo@chromium.org>
Reviewed-by: Alex Deymo <deymo@chromium.org>
Tested-by: Alex Deymo <deymo@chromium.org>
diff --git a/payload_state.cc b/payload_state.cc
index 6d6791f..1d7e0fe 100644
--- a/payload_state.cc
+++ b/payload_state.cc
@@ -92,6 +92,7 @@
SetNumResponsesSeen(num_responses_seen_ + 1);
SetResponseSignature(new_response_signature);
ResetPersistedState();
+ ReportUpdatesAbandonedEventCountMetric();
return;
}
@@ -1045,6 +1046,24 @@
kNumDefaultUmaBuckets);
}
+void PayloadState::ReportUpdatesAbandonedEventCountMetric() {
+ string metric = "Installer.UpdatesAbandonedEventCount";
+ int value = num_responses_seen_ - 1;
+
+ // Do not send an "abandoned" event when 0 payloads were abandoned since the
+ // last successful update.
+ if (value == 0)
+ return;
+
+ LOG(INFO) << "Uploading " << value << " (count) for metric " << metric;
+ system_state_->metrics_lib()->SendToUMA(
+ metric,
+ value,
+ 0, // min value
+ 100, // max value
+ kNumDefaultUmaBuckets);
+}
+
void PayloadState::ComputeCandidateUrls() {
bool http_url_ok = false;
diff --git a/payload_state.h b/payload_state.h
index 64d134f..6689b8d 100644
--- a/payload_state.h
+++ b/payload_state.h
@@ -318,9 +318,17 @@
void LoadNumResponsesSeen();
// Reports metric conveying how many times updates were abandoned
- // before an update was applied.
+ // before an update was applied. This metric is reported when an update is
+ // successfully applied.
void ReportUpdatesAbandonedCountMetric();
+ // Reports metric conveying how many times updates were abandoned since
+ // the last update was applied. The difference between this metric and the
+ // previous ReportUpdatesAbandonedCountMetric() one is that this metric is
+ // reported every time an update is abandoned, as oposed to the mentioned
+ // metric that is reported once the new update was applied.
+ void ReportUpdatesAbandonedEventCountMetric();
+
// The global state of the system.
SystemState* system_state_;
diff --git a/payload_state_unittest.cc b/payload_state_unittest.cc
index 0eb1875..8fffab9 100644
--- a/payload_state_unittest.cc
+++ b/payload_state_unittest.cc
@@ -297,6 +297,10 @@
EXPECT_TRUE(payload_state.Initialize(&mock_system_state));
+ // The first response doesn't send an abandoned event.
+ EXPECT_CALL(*mock_system_state.mock_metrics_lib(), SendToUMA(
+ "Installer.UpdatesAbandonedEventCount", 0, _, _, _)).Times(0);
+
// Set the first response.
SetupPayloadStateWith2Urls("Hash5823", true, &payload_state, &response);
EXPECT_EQ(1, payload_state.GetNumResponsesSeen());
@@ -307,10 +311,25 @@
EXPECT_EQ("https://test", payload_state.GetCurrentUrl());
EXPECT_EQ(1, payload_state.GetUrlSwitchCount());
+ EXPECT_CALL(*mock_system_state.mock_metrics_lib(), SendToUMA(
+ "Installer.UpdatesAbandonedEventCount", 1, _, _, _));
+
// Now, slightly change the response and set it again.
SetupPayloadStateWith2Urls("Hash8225", true, &payload_state, &response);
EXPECT_EQ(2, payload_state.GetNumResponsesSeen());
+ // Fake an error again.
+ payload_state.UpdateFailed(error);
+ EXPECT_EQ("https://test", payload_state.GetCurrentUrl());
+ EXPECT_EQ(1, payload_state.GetUrlSwitchCount());
+
+ EXPECT_CALL(*mock_system_state.mock_metrics_lib(), SendToUMA(
+ "Installer.UpdatesAbandonedEventCount", 2, _, _, _));
+
+ // Return a third different response.
+ SetupPayloadStateWith2Urls("Hash9999", true, &payload_state, &response);
+ EXPECT_EQ(3, payload_state.GetNumResponsesSeen());
+
// Make sure the url index was reset to 0 because of the new response.
EXPECT_EQ("http://test", payload_state.GetCurrentUrl());
EXPECT_EQ(0, payload_state.GetUrlFailureCount());