metrics: Initialize "last update time".
Before this change, when metrics_daemon started, it initialized its
"last update time" to 0 ticks and used this as the basis for computing
daily usage time statistics. If metrics daemon starts right after boot
(and ticks start counting from 0 on boot) this is reasonably accurate,
but if metrics daemon restarts for some reason (e.g. it crashes),
counting from 0 will provide a measure of total system uptime rather
than recent usage.
To fix this, initialize the "last update time" to the current number of
ticks when metrics daemon is initialized.
BUG=chromium:1159236
TEST=unit tests
Change-Id: I31151c00865d34708a83d6cd045e1c6e05e70912
Reviewed-on: https://chromium-review.googlesource.com/c/chromiumos/platform2/+/2593717
Tested-by: Miriam Zimmerman <mutexlox@chromium.org>
Reviewed-by: Ian Barkley-Yeung <iby@chromium.org>
Commit-Queue: Miriam Zimmerman <mutexlox@chromium.org>
diff --git a/metrics/metrics_daemon.cc b/metrics/metrics_daemon.cc
index f221813..a4c2e0a 100644
--- a/metrics/metrics_daemon.cc
+++ b/metrics/metrics_daemon.cc
@@ -403,6 +403,20 @@
vmstats_daily_success = VmStatsReadStats(&vmstats_daily_start);
+ // Start the "last update" time at the time of metrics daemon starting.
+ // This isn't entirely accurate -- in general, the tick counter will start
+ // counting when the kernel starts executing, while metrics_daemon will only
+ // start up once boot has mostly completed.
+ // However, we need to initialize because:
+ // 1. metrics_daemon might crash and restart. In this case, if we did not
+ // initialize last_update_stats_time_, the first values we calculate based on
+ // it will be based on the overall uptime of the device, rather than the
+ // actual interval between updates.
+ // 2. TimeTicks doesn't guarantee anything about when the value it returns
+ // will be zero -- only that the value is monotonically nondecreasing. In
+ // principle, on some platforms, it could start at a large number on boot.
+ last_update_stats_time_ = TimeTicks::Now();
+
// If testing, initialize Stats Reporter without connecting DBus
if (testing_)
StatsReporterInit();
diff --git a/metrics/metrics_daemon.h b/metrics/metrics_daemon.h
index 400d66c..2b4cbaf 100644
--- a/metrics/metrics_daemon.h
+++ b/metrics/metrics_daemon.h
@@ -135,6 +135,7 @@
FRIEND_TEST(MetricsDaemonTest, SendZramMetricsOld);
FRIEND_TEST(MetricsDaemonTest, SendZramMetricsWithIncompressiblePageStats);
FRIEND_TEST(MetricsDaemonTest, GetDetachableBaseTimes);
+ FRIEND_TEST(MetricsDaemonTest, UpdateUsageStats);
// State for disk stats collector callback.
enum StatsState {
diff --git a/metrics/metrics_daemon_test.cc b/metrics/metrics_daemon_test.cc
index 4b3e4be..8ae825c 100644
--- a/metrics/metrics_daemon_test.cc
+++ b/metrics/metrics_daemon_test.cc
@@ -32,6 +32,7 @@
using ::testing::_;
using ::testing::AnyNumber;
using ::testing::AtLeast;
+using ::testing::Le;
using ::testing::Return;
using ::testing::StrictMock;
@@ -72,6 +73,8 @@
CHECK(persistent_integer_backing_dir_.CreateUniqueTempDir());
base::FilePath backing_dir_path = persistent_integer_backing_dir_.GetPath();
+ test_start_ = base::TimeTicks::Now();
+
daemon_.Init(true, false, &metrics_lib_, kFakeDiskStatsName,
kFakeVmStatsName, kFakeScalingMaxFreqPath,
kFakeCpuinfoMaxFreqPath, base::TimeDelta::FromMinutes(30),
@@ -104,16 +107,16 @@
EXPECT_EQ(0, unlink(kFakeCpuinfoMaxFreqPath));
}
- // Adds active use aggregation counters update expectations that the
- // specified count will be added.
- void ExpectActiveUseUpdate(int count) {
- EXPECT_CALL(*daily_active_use_mock_, Add(count))
+ // Adds active use aggregation counters update expectations that a count no
+ // larger than the specified upper bound will be added.
+ void ExpectActiveUseUpdate(int upper_bound) {
+ EXPECT_CALL(*daily_active_use_mock_, Add(Le(upper_bound)))
.Times(1)
.RetiresOnSaturation();
- EXPECT_CALL(*kernel_crash_interval_mock_, Add(count))
+ EXPECT_CALL(*kernel_crash_interval_mock_, Add(Le(upper_bound)))
.Times(1)
.RetiresOnSaturation();
- EXPECT_CALL(*user_crash_interval_mock_, Add(count))
+ EXPECT_CALL(*user_crash_interval_mock_, Add(Le(upper_bound)))
.Times(1)
.RetiresOnSaturation();
}
@@ -208,6 +211,8 @@
// The MetricsDaemon under test.
MetricsDaemon daemon_;
+ // The system time just before the daemon was initialized.
+ base::TimeTicks test_start_;
// The temporary directory for backing files.
base::ScopedTempDir persistent_integer_backing_dir_;
@@ -662,6 +667,21 @@
EXPECT_EQ(suspended_time, 20);
}
+TEST_F(MetricsDaemonTest, UpdateUsageStats) {
+ // Ignore calls to SendToUMA.
+ EXPECT_CALL(metrics_lib_, SendToUMA(_, _, _, _, _)).Times(AnyNumber());
+
+ // Add an arbitrary amount to the test start.
+ const int elapsed_seconds = 42;
+ base::TimeTicks end =
+ test_start_ + base::TimeDelta::FromSeconds(elapsed_seconds);
+ ASSERT_EQ(elapsed_seconds, (end - test_start_).InSeconds());
+
+ ExpectActiveUseUpdate(elapsed_seconds);
+
+ daemon_.UpdateStats(end, base::Time::Now());
+}
+
} // namespace chromeos_metrics
int main(int argc, char** argv) {
