update_engine: Update repeated update metrics
Increment consecutive update metric after every update.
The lastFp pref will always be empty going into an update so it
can no longer be used to determine whether an update was a
consecutive update or not, `kPrefsConsecutiveUpdateCount` can be used
instead.
BUG=b:161259884
TEST=FEATURES=test update_engine
Change-Id: Ief11ebaa551453ba203ad30c767d614d5c0c813b
Reviewed-on: https://chromium-review.googlesource.com/c/aosp/platform/system/update_engine/+/2902631
Tested-by: Vyshu Khota <vyshu@chromium.org>
Reviewed-by: Amin Hassani <ahassani@chromium.org>
Commit-Queue: Amin Hassani <ahassani@chromium.org>
(cherry picked from commit fe2d9a95ed65218d4997e38849ad1115a98ee114)
Reviewed-on: https://chromium-review.googlesource.com/c/aosp/platform/system/update_engine/+/2920492
Reviewed-by: Jae Hoon Kim <kimjae@chromium.org>
Commit-Queue: Vyshu Khota <vyshu@chromium.org>
diff --git a/cros/update_attempter.cc b/cros/update_attempter.cc
index 3ea048c..197f7a1 100644
--- a/cros/update_attempter.cc
+++ b/cros/update_attempter.cc
@@ -1220,15 +1220,12 @@
// |install_plan_| is null during rollback operations, and the stats don't
// make much sense then anyway.
if (install_plan_) {
- if (SystemState::Get()->prefs()->Exists(kPrefsLastFp)) {
int64_t num_consecutive_updates = 0;
SystemState::Get()->prefs()->GetInt64(kPrefsConsecutiveUpdateCount,
&num_consecutive_updates);
- // If last fingerprint exists, then this is a consecutive update.
- // Increment pref.
+ // Increment pref after every update.
SystemState::Get()->prefs()->SetInt64(kPrefsConsecutiveUpdateCount,
++num_consecutive_updates);
- }
// Generate an unique payload identifier.
string target_version_uid;
for (const auto& payload : install_plan_->payloads) {
@@ -1378,9 +1375,12 @@
case UpdateStatus::CLEANUP_PREVIOUS_UPDATE:
MarkDeltaUpdateFailure();
// Errored out after partition was marked unbootable.
- if (SystemState::Get()->prefs()->Exists(kPrefsLastFp)) {
- // Last fingerprint exists so this is a consecutive update that
- // failed. Send Metric.
+ int64_t num_consecutive_updates = 0;
+ SystemState::Get()->prefs()->GetInt64(kPrefsConsecutiveUpdateCount,
+ &num_consecutive_updates);
+ if (num_consecutive_updates >= 1) {
+ // There has already been at least 1 update, so this is a
+ // consecutive update that failed. Send Metric.
SystemState::Get()
->metrics_reporter()
->ReportFailedConsecutiveUpdate();
diff --git a/cros/update_attempter_unittest.cc b/cros/update_attempter_unittest.cc
index ed714f7..6caca93 100644
--- a/cros/update_attempter_unittest.cc
+++ b/cros/update_attempter_unittest.cc
@@ -2426,22 +2426,6 @@
EXPECT_EQ("3.75", dlc_app_params.last_fp);
}
-TEST_F(UpdateAttempterTest, FirstUpdateBeforeReboot) {
- attempter_.is_install_ = false;
- attempter_.install_plan_.reset(new InstallPlan);
- attempter_.install_plan_->payloads.push_back(
- {.size = 1234ULL, .type = InstallPayloadType::kFull});
-
- // Call |ProcessingDoneUpdate|.
- pd_params_.should_update_completed_be_called = true;
- pd_params_.expected_exit_status = UpdateStatus::UPDATED_NEED_REBOOT;
- TestProcessingDone();
-
- // Consecutive update metric should not be updated on the first update.
- EXPECT_FALSE(
- FakeSystemState::Get()->prefs()->Exists(kPrefsConsecutiveUpdateCount));
-}
-
TEST_F(UpdateAttempterTest, ConsecutiveUpdateBeforeRebootSuccess) {
FakeSystemState::Get()->prefs()->SetString(kPrefsLastFp, "3.75");
attempter_.is_install_ = false;
@@ -2463,7 +2447,8 @@
TEST_F(UpdateAttempterTest, ConsecutiveUpdateFailureMetric) {
MockAction action;
- FakeSystemState::Get()->prefs()->SetString(kPrefsLastFp, "3.75");
+ // Two previous consecutive updates.
+ FakeSystemState::Get()->prefs()->SetInt64(kPrefsConsecutiveUpdateCount, 2);
// Fail an update.
EXPECT_CALL(action, Type()).WillRepeatedly(Return("MockAction"));