metrics: Round down 24h5m of daily usage to 24h.
Since the metrics_daemon only updates usage statistics every 5 minutes,
we'll often report slightly more than 24 hours of usage for devices that
are active for exactly 24 hours in a day (e.g. kiosk devices). Work
around this by rounding down from 24h5m to 24h.
We don't round down any larger numbers because they could be due to
unrelated bugs, and we don't want to mask them.
BUG=chromium:1159236
TEST=unit tests
Change-Id: I9d55b5df476dee93d0842feda8fe24f09d032d4f
Reviewed-on: https://chromium-review.googlesource.com/c/chromiumos/platform2/+/2601298
Tested-by: Miriam Zimmerman <mutexlox@chromium.org>
Commit-Queue: Miriam Zimmerman <mutexlox@chromium.org>
Reviewed-by: Michael Irani <michaelirani@google.com>
diff --git a/metrics/metrics_daemon.cc b/metrics/metrics_daemon.cc
index 9e94ecf..00c1531 100644
--- a/metrics/metrics_daemon.cc
+++ b/metrics/metrics_daemon.cc
@@ -61,6 +61,7 @@
// See chromite/scripts/cros_set_lsb_release.py.
const char kOfficialBuild[] = "Official Build";
+const int kMillisPerSecond = 1000;
const int kSecondsPerMinute = 60;
const int kMinutesPerHour = 60;
const int kHoursPerDay = 24;
@@ -1532,7 +1533,23 @@
}
void MetricsDaemon::SendAndResetDailyUseSample() {
- SendSample(kDailyUseTimeName, daily_active_use_->GetAndClear(),
+ // Since metrics_daemon only updates statistics every kUpdateStatsIntervalMs,
+ // we will often report devices that are active for exactly 24 hours as being
+ // active for slightly more than 24 hours. Round down in such cases to exactly
+ // 24 hours, since we cannot be active for more than 24 hours in a day. Do
+ // *not* round down times more than that, because they could be due to
+ // unrelated bugs that we don't want to mask.
+ int64_t dau_seconds = daily_active_use_->GetAndClear();
+ if (dau_seconds > kSecondsPerDay &&
+ dau_seconds <=
+ kSecondsPerDay + (kUpdateStatsIntervalMs / kMillisPerSecond)) {
+ // Shift the extra over to the current day.
+ daily_active_use_->Add(dau_seconds - kSecondsPerDay);
+
+ // Then record only the maximum daily amount today.
+ dau_seconds = kSecondsPerDay;
+ }
+ SendSample(kDailyUseTimeName, dau_seconds,
1, // value of first bucket
kSecondsPerDay, // value of last bucket
50); // number of buckets
diff --git a/metrics/metrics_daemon.h b/metrics/metrics_daemon.h
index 2b4cbaf..050032a 100644
--- a/metrics/metrics_daemon.h
+++ b/metrics/metrics_daemon.h
@@ -136,6 +136,7 @@
FRIEND_TEST(MetricsDaemonTest, SendZramMetricsWithIncompressiblePageStats);
FRIEND_TEST(MetricsDaemonTest, GetDetachableBaseTimes);
FRIEND_TEST(MetricsDaemonTest, UpdateUsageStats);
+ FRIEND_TEST(MetricsDaemonTest, RoundsDailyUseIfJustOverOneDay);
// State for disk stats collector callback.
enum StatsState {
diff --git a/metrics/metrics_daemon_test.cc b/metrics/metrics_daemon_test.cc
index 8ae825c..338d885 100644
--- a/metrics/metrics_daemon_test.cc
+++ b/metrics/metrics_daemon_test.cc
@@ -110,6 +110,9 @@
// 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_, GetAndClear())
+ .Times(1)
+ .RetiresOnSaturation();
EXPECT_CALL(*daily_active_use_mock_, Add(Le(upper_bound)))
.Times(1)
.RetiresOnSaturation();
@@ -124,6 +127,9 @@
// As above, but ignore values of counter updates.
void IgnoreActiveUseUpdate() {
EXPECT_CALL(*daily_active_use_mock_, Add(_)).Times(1).RetiresOnSaturation();
+ EXPECT_CALL(*daily_active_use_mock_, GetAndClear())
+ .Times(1)
+ .RetiresOnSaturation();
EXPECT_CALL(*kernel_crash_interval_mock_, Add(_))
.Times(1)
.RetiresOnSaturation();
@@ -251,6 +257,10 @@
DeleteDBusMessage(msg);
IgnoreActiveUseUpdate();
+ EXPECT_CALL(*user_crash_interval_mock_, GetAndClear())
+ .Times(1)
+ .RetiresOnSaturation();
+
vector<string> signal_args;
msg = NewDBusSignalString("/", "org.chromium.CrashReporter", "UserCrash",
signal_args);
@@ -682,6 +692,29 @@
daemon_.UpdateStats(end, base::Time::Now());
}
+TEST_F(MetricsDaemonTest, RoundsDailyUseIfJustOverOneDay) {
+ // Should round down daily use within 5 minutes of 24 hours...
+ const int kSecondsPerDay = 24 * 60 * 60;
+ const int kFiveMinutes = 5 * 60;
+ EXPECT_CALL(*daily_active_use_mock_, GetAndClear())
+ .WillOnce(Return(kSecondsPerDay + kFiveMinutes));
+ EXPECT_CALL(*daily_active_use_mock_, Add(kFiveMinutes)).Times(1);
+ ExpectSample("Platform.DailyUseTime", kSecondsPerDay);
+ daemon_.SendAndResetDailyUseSample();
+
+ // ... but not round down daily use above that...
+ EXPECT_CALL(*daily_active_use_mock_, GetAndClear())
+ .WillOnce(Return(kSecondsPerDay + kFiveMinutes + 1));
+ ExpectSample("Platform.DailyUseTime", kSecondsPerDay + kFiveMinutes + 1);
+ daemon_.SendAndResetDailyUseSample();
+
+ // .. and not change times below that.
+ EXPECT_CALL(*daily_active_use_mock_, GetAndClear())
+ .WillOnce(Return(kSecondsPerDay - 1));
+ ExpectSample("Platform.DailyUseTime", kSecondsPerDay - 1);
+ daemon_.SendAndResetDailyUseSample();
+}
+
} // namespace chromeos_metrics
int main(int argc, char** argv) {
diff --git a/metrics/persistent_integer.h b/metrics/persistent_integer.h
index cb0f7e0..ccd762d 100644
--- a/metrics/persistent_integer.h
+++ b/metrics/persistent_integer.h
@@ -38,7 +38,8 @@
int64_t Get();
// Convenience function for Get() followed by Set(0).
- int64_t GetAndClear();
+ // Virtual only because of mock.
+ virtual int64_t GetAndClear();
// Convenience function for v = Get, Set(v + x).
// Virtual only because of mock.
diff --git a/metrics/persistent_integer_mock.h b/metrics/persistent_integer_mock.h
index 9f049c2..00185e1 100644
--- a/metrics/persistent_integer_mock.h
+++ b/metrics/persistent_integer_mock.h
@@ -19,6 +19,7 @@
explicit PersistentIntegerMock(const base::FilePath& backing_file_path)
: PersistentInteger(backing_file_path) {}
MOCK_METHOD(void, Add, (int64_t), (override));
+ MOCK_METHOD(int64_t, GetAndClear, (), (override));
};
} // namespace chromeos_metrics
