metrics: Emit kAbnormalTermination if last update attempt failed.
This allows us to track how many update attempts terminates
abnormally, for example if the device is rebooted in the middle of an
update or if update_engine crashes. We use a marker state variable
(aka a "prefs" file) to keep track of this.
Note that we don't currently report any of the other metrics in the
UpdateEngine.Attempt.* namespace. If necessary, this can be changed in
the future - I left a TODO item in the code with more details.
BUG=chromium:357676
TEST=New unit tests + Unit tests pass + Manual tests.
Change-Id: I83fe284c7c46917c0c55b92314c58098e2fd1789
Reviewed-on: https://chromium-review.googlesource.com/197175
Reviewed-by: Alex Deymo <deymo@chromium.org>
Tested-by: David Zeuthen <zeuthen@chromium.org>
Commit-Queue: David Zeuthen <zeuthen@chromium.org>
diff --git a/payload_state_unittest.cc b/payload_state_unittest.cc
index 24aa070..3071a6b 100644
--- a/payload_state_unittest.cc
+++ b/payload_state_unittest.cc
@@ -13,6 +13,7 @@
#include "update_engine/constants.h"
#include "update_engine/fake_clock.h"
#include "update_engine/fake_hardware.h"
+#include "update_engine/fake_prefs.h"
#include "update_engine/fake_system_state.h"
#include "update_engine/omaha_request_action.h"
#include "update_engine/payload_state.h"
@@ -1221,7 +1222,7 @@
EXPECT_TRUE(payload_state.Initialize(&fake_system_state));
- // No prefs should be used after a crash.
+ // Only the |kPrefsAttemptInProgress| state variable should be read.
EXPECT_CALL(*prefs, Exists(_)).Times(0);
EXPECT_CALL(*prefs, SetString(_, _)).Times(0);
EXPECT_CALL(*prefs, SetInt64(_, _)).Times(0);
@@ -1229,6 +1230,7 @@
EXPECT_CALL(*prefs, GetString(_, _)).Times(0);
EXPECT_CALL(*prefs, GetInt64(_, _)).Times(0);
EXPECT_CALL(*prefs, GetBoolean(_, _)).Times(0);
+ EXPECT_CALL(*prefs, GetBoolean(kPrefsAttemptInProgress, _));
// No metrics are reported after a crash.
EXPECT_CALL(*fake_system_state.mock_metrics_lib(),
@@ -1240,6 +1242,77 @@
payload_state.UpdateEngineStarted();
}
+TEST(PayloadStateTest, AbnormalTerminationAttemptMetricsNoReporting) {
+ PayloadState payload_state;
+ FakeSystemState fake_system_state;
+
+ // If there's no marker at startup, ensure we don't report a metric.
+ EXPECT_TRUE(payload_state.Initialize(&fake_system_state));
+ EXPECT_CALL(*fake_system_state.mock_metrics_lib(),
+ SendEnumToUMA(
+ metrics::kMetricAttemptResult,
+ static_cast<int>(metrics::AttemptResult::kAbnormalTermination),
+ _)).Times(0);
+ payload_state.UpdateEngineStarted();
+}
+
+TEST(PayloadStateTest, AbnormalTerminationAttemptMetricsReported) {
+ PayloadState payload_state;
+ FakeSystemState fake_system_state;
+ FakePrefs prefs;
+
+ // If we have a marker at startup, ensure it's reported and the
+ // marker is then cleared.
+ fake_system_state.set_prefs(&prefs);
+ prefs.SetBoolean(kPrefsAttemptInProgress, true);
+
+ EXPECT_TRUE(payload_state.Initialize(&fake_system_state));
+
+ EXPECT_CALL(*fake_system_state.mock_metrics_lib(),
+ SendEnumToUMA(
+ metrics::kMetricAttemptResult,
+ static_cast<int>(metrics::AttemptResult::kAbnormalTermination),
+ _)).Times(1);
+ payload_state.UpdateEngineStarted();
+
+ EXPECT_FALSE(prefs.Exists(kPrefsAttemptInProgress));
+}
+
+TEST(PayloadStateTest, AbnormalTerminationAttemptMetricsClearedOnSucceess) {
+ PayloadState payload_state;
+ FakeSystemState fake_system_state;
+ FakePrefs prefs;
+
+ // Make sure the marker is written and cleared during an attempt and
+ // also that we DO NOT emit the metric (since the attempt didn't end
+ // abnormally).
+ fake_system_state.set_prefs(&prefs);
+ EXPECT_TRUE(payload_state.Initialize(&fake_system_state));
+
+ EXPECT_CALL(*fake_system_state.mock_metrics_lib(), SendToUMA(_, _, _, _, _))
+ .Times(AnyNumber());
+ EXPECT_CALL(*fake_system_state.mock_metrics_lib(), SendEnumToUMA(_, _, _))
+ .Times(AnyNumber());
+ EXPECT_CALL(*fake_system_state.mock_metrics_lib(),
+ SendEnumToUMA(
+ metrics::kMetricAttemptResult,
+ static_cast<int>(metrics::AttemptResult::kAbnormalTermination),
+ _)).Times(0);
+
+ // Attempt not in progress, should be clear.
+ EXPECT_FALSE(prefs.Exists(kPrefsAttemptInProgress));
+
+ payload_state.UpdateRestarted();
+
+ // Attempt not in progress, should be set.
+ EXPECT_TRUE(prefs.Exists(kPrefsAttemptInProgress));
+
+ payload_state.UpdateSucceeded();
+
+ // Attempt not in progress, should be clear.
+ EXPECT_FALSE(prefs.Exists(kPrefsAttemptInProgress));
+}
+
TEST(PayloadStateTest, CandidateUrlsComputedCorrectly) {
OmahaResponse response;
FakeSystemState fake_system_state;