metricsd: Only collect metrics over a short period.

Instead of reporting the metrics over both a long and a short period,
collect and report only over a short period. This makes the code simpler
and the metrics easier to understand.

Also move the collection out of metrics_daemon and into a separate
collector to make it simpler to understand.

BUG: 24464945

Change-Id: I17e52536aaa75321a5e34f42ed488545c2c3efde

7 files changed, 412 insertions, 347 deletions

diff --git a/metricsd/Android.mk b/metricsd/Android.mk
index 03b5739c0..9fd27f527 100644
--- a/metricsd/Android.mk
+++ b/metricsd/Android.mk
@@ -26,6 +26,7 @@ metrics_client_sources := \
   metrics_client.cc
 
 metrics_daemon_sources := \
+  collectors/averaged_statistics_collector.cc \
   collectors/disk_usage_collector.cc \
   metrics_daemon.cc \
   metrics_daemon_main.cc \
@@ -40,6 +41,8 @@ metrics_daemon_sources := \
   serialization/serialization_utils.cc
 
 metrics_tests_sources := \
+  collectors/averaged_statistics_collector.cc \
+  collectors/averaged_statistics_collector_test.cc \
   collectors/disk_usage_collector.cc \
   metrics_daemon.cc \
   metrics_daemon_test.cc \
@@ -146,6 +149,7 @@ include $(BUILD_EXECUTABLE)
 # ========================================================
 include $(CLEAR_VARS)
 LOCAL_MODULE := metrics_tests
+LOCAL_CLANG := true
 LOCAL_CFLAGS := $(metrics_CFLAGS)
 LOCAL_CPP_EXTENSION := $(metrics_cpp_extension)
 LOCAL_CPPFLAGS := $(metrics_CPPFLAGS) -Wno-sign-compare
diff --git a/metricsd/collectors/averaged_statistics_collector.cc b/metricsd/collectors/averaged_statistics_collector.cc
new file mode 100644
index 000000000..0931e7bd5
--- /dev/null
+++ b/metricsd/collectors/averaged_statistics_collector.cc
@@ -0,0 +1,216 @@
+/*
+ * Copyright (C) 2015 The Android Open Source Project
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+#include "averaged_statistics_collector.h"
+
+#include <base/files/file_util.h>
+#include <base/files/file_path.h>
+#include <base/strings/string_number_conversions.h>
+#include <base/strings/string_split.h>
+
+#include "metrics_daemon.h"
+
+namespace {
+
+// disk stats metrics
+
+// The {Read,Write}Sectors numbers are in sectors/second.
+// A sector is usually 512 bytes.
+const char kReadSectorsHistogramName[] = "Platform.ReadSectors";
+const char kWriteSectorsHistogramName[] = "Platform.WriteSectors";
+const int kDiskMetricsStatItemCount = 11;
+
+// Assume a max rate of 250Mb/s for reads (worse for writes) and 512 byte
+// sectors.
+const int kSectorsIOMax = 500000;  // sectors/second
+const int kSectorsBuckets = 50;    // buckets
+
+// Page size is 4k, sector size is 0.5k.  We're not interested in page fault
+// rates that the disk cannot sustain.
+const int kPageFaultsMax = kSectorsIOMax / 8;  // Page faults/second
+const int kPageFaultsBuckets = 50;
+
+// Major page faults, i.e. the ones that require data to be read from disk.
+const char kPageFaultsHistogramName[] = "Platform.PageFaults";
+
+// Swap in and Swap out
+const char kSwapInHistogramName[] = "Platform.SwapIn";
+const char kSwapOutHistogramName[] = "Platform.SwapOut";
+
+const int kIntervalBetweenCollection = 60;  // seconds
+const int kCollectionDuration = 1;  // seconds
+
+}  // namespace
+
+AveragedStatisticsCollector::AveragedStatisticsCollector(
+    MetricsLibraryInterface* metrics_library,
+    const std::string& diskstats_path,
+    const std::string& vmstats_path) :
+  metrics_lib_(metrics_library),
+  diskstats_path_(diskstats_path),
+  vmstats_path_(vmstats_path) {
+}
+
+void AveragedStatisticsCollector::ScheduleWait() {
+  base::MessageLoop::current()->PostDelayedTask(FROM_HERE,
+      base::Bind(&AveragedStatisticsCollector::WaitCallback,
+                 base::Unretained(this)),
+      base::TimeDelta::FromSeconds(
+          kIntervalBetweenCollection - kCollectionDuration));
+}
+
+void AveragedStatisticsCollector::ScheduleCollect() {
+  base::MessageLoop::current()->PostDelayedTask(FROM_HERE,
+      base::Bind(&AveragedStatisticsCollector::CollectCallback,
+                 base::Unretained(this)),
+      base::TimeDelta::FromSeconds(kCollectionDuration));
+}
+
+void AveragedStatisticsCollector::WaitCallback() {
+  ReadInitialValues();
+  ScheduleCollect();
+}
+
+void AveragedStatisticsCollector::CollectCallback() {
+  Collect();
+  ScheduleWait();
+}
+
+void AveragedStatisticsCollector::ReadInitialValues() {
+  stats_start_time_ = MetricsDaemon::GetActiveTime();
+  DiskStatsReadStats(&read_sectors_, &write_sectors_);
+  VmStatsReadStats(&vmstats_);
+}
+
+bool AveragedStatisticsCollector::DiskStatsReadStats(
+    uint64_t* read_sectors, uint64_t* write_sectors) {
+  CHECK(read_sectors);
+  CHECK(write_sectors);
+  std::string line;
+  if (diskstats_path_.empty()) {
+    return false;
+  }
+
+  if (!base::ReadFileToString(base::FilePath(diskstats_path_), &line)) {
+    PLOG(WARNING) << "Could not read disk stats from "
+                  << diskstats_path_.value();
+    return false;
+  }
+
+  std::vector<std::string> parts = base::SplitString(
+      line, " ", base::TRIM_WHITESPACE, base::SPLIT_WANT_NONEMPTY);
+  if (parts.size() != kDiskMetricsStatItemCount) {
+    LOG(ERROR) << "Could not parse disk stat correctly. Expected "
+               << kDiskMetricsStatItemCount << " elements but got "
+               << parts.size();
+    return false;
+  }
+  if (!base::StringToUint64(parts[2], read_sectors)) {
+    LOG(ERROR) << "Couldn't convert read sectors " << parts[2] << " to uint64";
+    return false;
+  }
+  if (!base::StringToUint64(parts[6], write_sectors)) {
+    LOG(ERROR) << "Couldn't convert write sectors " << parts[6] << " to uint64";
+    return false;
+  }
+
+  return true;
+}
+
+bool AveragedStatisticsCollector::VmStatsParseStats(
+    const char* stats, struct VmstatRecord* record) {
+  CHECK(stats);
+  CHECK(record);
+  base::StringPairs pairs;
+  base::SplitStringIntoKeyValuePairs(stats, ' ', '\n', &pairs);
+
+  for (base::StringPairs::iterator it = pairs.begin();
+       it != pairs.end(); ++it) {
+    if (it->first == "pgmajfault" &&
+        !base::StringToUint64(it->second, &record->page_faults)) {
+      return false;
+    }
+    if (it->first == "pswpin" &&
+        !base::StringToUint64(it->second, &record->swap_in)) {
+      return false;
+    }
+    if (it->first == "pswpout" &&
+        !base::StringToUint64(it->second, &record->swap_out)) {
+      return false;
+    }
+  }
+  return true;
+}
+
+bool AveragedStatisticsCollector::VmStatsReadStats(struct VmstatRecord* stats) {
+  CHECK(stats);
+  std::string value_string;
+  if (!base::ReadFileToString(vmstats_path_, &value_string)) {
+    LOG(WARNING) << "cannot read " << vmstats_path_.value();
+    return false;
+  }
+  return VmStatsParseStats(value_string.c_str(), stats);
+}
+
+void AveragedStatisticsCollector::Collect() {
+  uint64_t read_sectors_now, write_sectors_now;
+  struct VmstatRecord vmstats_now;
+  double time_now = MetricsDaemon::GetActiveTime();
+  double delta_time = time_now - stats_start_time_;
+  bool diskstats_success = DiskStatsReadStats(&read_sectors_now,
+                                              &write_sectors_now);
+
+  int delta_read = read_sectors_now - read_sectors_;
+  int delta_write = write_sectors_now - write_sectors_;
+  int read_sectors_per_second = delta_read / delta_time;
+  int write_sectors_per_second = delta_write / delta_time;
+  bool vmstats_success = VmStatsReadStats(&vmstats_now);
+  uint64_t delta_faults = vmstats_now.page_faults - vmstats_.page_faults;
+  uint64_t delta_swap_in = vmstats_now.swap_in - vmstats_.swap_in;
+  uint64_t delta_swap_out = vmstats_now.swap_out - vmstats_.swap_out;
+  uint64_t page_faults_per_second = delta_faults / delta_time;
+  uint64_t swap_in_per_second = delta_swap_in / delta_time;
+  uint64_t swap_out_per_second = delta_swap_out / delta_time;
+  if (diskstats_success) {
+    metrics_lib_->SendToUMA(kReadSectorsHistogramName,
+                            read_sectors_per_second,
+                            1,
+                            kSectorsIOMax,
+                            kSectorsBuckets);
+    metrics_lib_->SendToUMA(kWriteSectorsHistogramName,
+                            write_sectors_per_second,
+                            1,
+                            kSectorsIOMax,
+                            kSectorsBuckets);
+  }
+  if (vmstats_success) {
+    metrics_lib_->SendToUMA(kPageFaultsHistogramName,
+                            page_faults_per_second,
+                            1,
+                            kPageFaultsMax,
+                            kPageFaultsBuckets);
+    metrics_lib_->SendToUMA(kSwapInHistogramName,
+                            swap_in_per_second,
+                            1,
+                            kPageFaultsMax,
+                            kPageFaultsBuckets);
+    metrics_lib_->SendToUMA(kSwapOutHistogramName,
+                            swap_out_per_second,
+                            1,
+                            kPageFaultsMax,
+                            kPageFaultsBuckets);
+  }
+}
diff --git a/metricsd/collectors/averaged_statistics_collector.h b/metricsd/collectors/averaged_statistics_collector.h
new file mode 100644
index 000000000..753f70c7d
--- /dev/null
+++ b/metricsd/collectors/averaged_statistics_collector.h
@@ -0,0 +1,79 @@
+/*
+ * Copyright (C) 2015 The Android Open Source Project
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+#ifndef METRICSD_COLLECTORS_AVERAGED_STATISTICS_COLLECTOR_H_
+#define METRICSD_COLLECTORS_AVERAGED_STATISTICS_COLLECTOR_H_
+
+#include "metrics/metrics_library.h"
+
+class AveragedStatisticsCollector {
+ public:
+  AveragedStatisticsCollector(MetricsLibraryInterface* metrics_library,
+                              const std::string& diskstats_path,
+                              const std::string& vmstat_path);
+
+  // Schedule a wait period.
+  void ScheduleWait();
+
+  // Schedule a collection period.
+  void ScheduleCollect();
+
+  // Callback used by the main loop.
+  void CollectCallback();
+
+  // Callback used by the main loop.
+  void WaitCallback();
+
+  // Read and store the initial values at the beginning of a collection cycle.
+  void ReadInitialValues();
+
+  // Collect the disk usage statistics and report them.
+  void Collect();
+
+ private:
+  friend class AveragedStatisticsTest;
+  FRIEND_TEST(AveragedStatisticsTest, ParseDiskStats);
+  FRIEND_TEST(AveragedStatisticsTest, ParseVmStats);
+
+  // Record for retrieving and reporting values from /proc/vmstat
+  struct VmstatRecord {
+    uint64_t page_faults;    // major faults
+    uint64_t swap_in;        // pages swapped in
+    uint64_t swap_out;       // pages swapped out
+  };
+
+  // Read the disk read/write statistics for the main disk.
+  bool DiskStatsReadStats(uint64_t* read_sectors, uint64_t* write_sectors);
+
+  // Parse the content of the vmstats file into |record|.
+  bool VmStatsParseStats(const char* stats, struct VmstatRecord* record);
+
+  // Read the vmstats into |stats|.
+  bool VmStatsReadStats(struct VmstatRecord* stats);
+
+  MetricsLibraryInterface* metrics_lib_;
+  base::FilePath diskstats_path_;
+  base::FilePath vmstats_path_;
+
+  // Values observed at the beginning of the collection period.
+  uint64_t read_sectors_;
+  uint64_t write_sectors_;
+  struct VmstatRecord vmstats_;
+
+  double stats_start_time_;
+};
+
+#endif  // METRICSD_COLLECTORS_AVERAGED_STATISTICS_COLLECTOR_H_
diff --git a/metricsd/collectors/averaged_statistics_collector_test.cc b/metricsd/collectors/averaged_statistics_collector_test.cc
new file mode 100644
index 000000000..9c97f0011
--- /dev/null
+++ b/metricsd/collectors/averaged_statistics_collector_test.cc
@@ -0,0 +1,99 @@
+/*
+ * Copyright (C) 2015 The Android Open Source Project
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+#include "averaged_statistics_collector.h"
+
+#include <inttypes.h>
+
+#include <base/files/file_util.h>
+#include <base/files/scoped_temp_dir.h>
+#include <base/memory/scoped_ptr.h>
+#include <base/strings/stringprintf.h>
+#include <gtest/gtest.h>
+
+
+static const char kFakeDiskStatsFormat[] =
+    "    1793     1788    %" PRIu64 "   105580    "
+    "    196      175     %" PRIu64 "    30290    "
+    "    0    44060   135850\n";
+static const uint64_t kFakeReadSectors[] = {80000, 100000};
+static const uint64_t kFakeWriteSectors[] = {3000, 4000};
+
+
+class AveragedStatisticsTest : public testing::Test {
+ protected:
+  std::string kFakeDiskStats0;
+  std::string kFakeDiskStats1;
+
+  virtual void SetUp() {
+    ASSERT_TRUE(temp_dir_.CreateUniqueTempDir());
+    disk_stats_path_ = temp_dir_.path().Append("disk_stats");
+    collector_.reset(new AveragedStatisticsCollector(
+        &metrics_lib_, disk_stats_path_.value(), ""));
+
+    kFakeDiskStats0 = base::StringPrintf(kFakeDiskStatsFormat,
+                                         kFakeReadSectors[0],
+                                         kFakeWriteSectors[0]);
+    kFakeDiskStats1 = base::StringPrintf(kFakeDiskStatsFormat,
+                                         kFakeReadSectors[1],
+                                         kFakeWriteSectors[1]);
+
+    CreateFakeDiskStatsFile(kFakeDiskStats0);
+  }
+
+  // Creates or overwrites an input file containing fake disk stats.
+  void CreateFakeDiskStatsFile(const std::string& fake_stats) {
+    EXPECT_EQ(base::WriteFile(disk_stats_path_,
+                              fake_stats.data(), fake_stats.size()),
+              fake_stats.size());
+  }
+
+  // Collector used for tests.
+  scoped_ptr<AveragedStatisticsCollector> collector_;
+
+  // Temporary directory used for tests.
+  base::ScopedTempDir temp_dir_;
+
+  // Path for the fake files.
+  base::FilePath disk_stats_path_;
+
+  MetricsLibrary metrics_lib_;
+};
+
+TEST_F(AveragedStatisticsTest, ParseDiskStats) {
+  uint64_t read_sectors_now, write_sectors_now;
+  CreateFakeDiskStatsFile(kFakeDiskStats0);
+  ASSERT_TRUE(collector_->DiskStatsReadStats(&read_sectors_now,
+                                             &write_sectors_now));
+  EXPECT_EQ(read_sectors_now, kFakeReadSectors[0]);
+  EXPECT_EQ(write_sectors_now, kFakeWriteSectors[0]);
+
+  CreateFakeDiskStatsFile(kFakeDiskStats1);
+  ASSERT_TRUE(collector_->DiskStatsReadStats(&read_sectors_now,
+                                             &write_sectors_now));
+  EXPECT_EQ(read_sectors_now, kFakeReadSectors[1]);
+  EXPECT_EQ(write_sectors_now, kFakeWriteSectors[1]);
+}
+
+TEST_F(AveragedStatisticsTest, ParseVmStats) {
+  static char kVmStats[] = "pswpin 1345\npswpout 8896\n"
+    "foo 100\nbar 200\npgmajfault 42\netcetc 300\n";
+  struct AveragedStatisticsCollector::VmstatRecord stats;
+  EXPECT_TRUE(collector_->VmStatsParseStats(kVmStats, &stats));
+  EXPECT_EQ(stats.page_faults, 42);
+  EXPECT_EQ(stats.swap_in, 1345);
+  EXPECT_EQ(stats.swap_out, 8896);
+}
diff --git a/metricsd/metrics_daemon.cc b/metricsd/metrics_daemon.cc
index 3b81cf824..9eb680213 100644
--- a/metricsd/metrics_daemon.cc
+++ b/metricsd/metrics_daemon.cc
@@ -16,10 +16,6 @@
 
 #include "metrics_daemon.h"
 
-#include <fcntl.h>
-#include <inttypes.h>
-#include <math.h>
-#include <string.h>
 #include <sysexits.h>
 #include <time.h>
 
@@ -71,48 +67,12 @@ const char kKernelCrashDetectedFile[] = "/var/run/kernel-crash-detected";
 const char kUncleanShutdownDetectedFile[] =
     "/var/run/unclean-shutdown-detected";
 
-// disk stats metrics
-
-// The {Read,Write}Sectors numbers are in sectors/second.
-// A sector is usually 512 bytes.
-
-const char kMetricReadSectorsLongName[] = "Platform.ReadSectors.PerMinute";
-const char kMetricWriteSectorsLongName[] = "Platform.WriteSectors.PerMinute";
-const char kMetricReadSectorsShortName[] = "Platform.ReadSectors.PerSecond";
-const char kMetricWriteSectorsShortName[] = "Platform.WriteSectors.PerSecond";
-
-const int kMetricStatsShortInterval = 1;  // seconds
-const int kMetricStatsLongInterval = 60;  // seconds
-
 const int kMetricMeminfoInterval = 30;    // seconds
 
-// Assume a max rate of 250Mb/s for reads (worse for writes) and 512 byte
-// sectors.
-const int kMetricSectorsIOMax = 500000;  // sectors/second
-const int kMetricSectorsBuckets = 50;    // buckets
-// Page size is 4k, sector size is 0.5k.  We're not interested in page fault
-// rates that the disk cannot sustain.
-const int kMetricPageFaultsMax = kMetricSectorsIOMax / 8;
-const int kMetricPageFaultsBuckets = 50;
-
-// Major page faults, i.e. the ones that require data to be read from disk.
-
-const char kMetricPageFaultsLongName[] = "Platform.PageFaults.PerMinute";
-const char kMetricPageFaultsShortName[] = "Platform.PageFaults.PerSecond";
-
-// Swap in and Swap out
-
-const char kMetricSwapInLongName[] = "Platform.SwapIn.PerMinute";
-const char kMetricSwapInShortName[] = "Platform.SwapIn.PerSecond";
-
-const char kMetricSwapOutLongName[] = "Platform.SwapOut.PerMinute";
-const char kMetricSwapOutShortName[] = "Platform.SwapOut.PerSecond";
-
 const char kMetricsProcStatFileName[] = "/proc/stat";
-const char kVmStatFileName[] = "/proc/vmstat";
 const char kMeminfoFileName[] = "/proc/meminfo";
+const char kVmStatFileName[] = "/proc/vmstat";
 const int kMetricsProcStatFirstLineItemsCount = 11;
-const int kDiskMetricsStatItemCount = 11;
 
 // Thermal CPU throttling.
 
@@ -142,17 +102,13 @@ static const int kMemuseIntervals[] = {
 MetricsDaemon::MetricsDaemon()
     : memuse_final_time_(0),
       memuse_interval_index_(0),
-      read_sectors_(0),
-      write_sectors_(0),
-      vmstats_(),
-      stats_state_(kStatsShort),
-      stats_initial_time_(0),
       ticks_per_second_(0),
       latest_cpu_use_ticks_(0) {}
 
 MetricsDaemon::~MetricsDaemon() {
 }
 
+// static
 double MetricsDaemon::GetActiveTime() {
   struct timespec ts;
   int r = clock_gettime(CLOCK_MONOTONIC, &ts);
@@ -275,14 +231,12 @@ void MetricsDaemon::Init(bool testing,
   weekly_cycle_.reset(new PersistentInteger("weekly.cycle"));
   version_cycle_.reset(new PersistentInteger("version.cycle"));
 
-  diskstats_path_ = diskstats_path;
   scaling_max_freq_path_ = scaling_max_freq_path;
   cpuinfo_max_freq_path_ = cpuinfo_max_freq_path;
   disk_usage_collector_.reset(new DiskUsageCollector(metrics_lib_));
-
-  // If testing, initialize Stats Reporter without connecting DBus
-  if (testing_)
-    StatsReporterInit();
+  averaged_stats_collector_.reset(
+      new AveragedStatisticsCollector(metrics_lib_, diskstats_path,
+                                      kVmStatFileName));
 }
 
 int MetricsDaemon::OnInit() {
@@ -494,94 +448,12 @@ bool MetricsDaemon::CheckSystemCrash(const string& crash_file) {
 
 void MetricsDaemon::StatsReporterInit() {
   disk_usage_collector_->Schedule();
-  DiskStatsReadStats(&read_sectors_, &write_sectors_);
-  VmStatsReadStats(&vmstats_);
-  // The first time around just run the long stat, so we don't delay boot.
-  stats_state_ = kStatsLong;
-  stats_initial_time_ = GetActiveTime();
-  if (stats_initial_time_ < 0) {
-    LOG(WARNING) << "not collecting disk stats";
-  } else {
-    ScheduleStatsCallback(kMetricStatsLongInterval);
-  }
-}
 
-void MetricsDaemon::ScheduleStatsCallback(int wait) {
-  if (testing_) {
-    return;
-  }
-  base::MessageLoop::current()->PostDelayedTask(FROM_HERE,
-      base::Bind(&MetricsDaemon::StatsCallback, base::Unretained(this)),
-      base::TimeDelta::FromSeconds(wait));
+  // Don't start a collection cycle during the first run to avoid delaying the
+  // boot.
+  averaged_stats_collector_->ScheduleWait();
 }
 
-bool MetricsDaemon::DiskStatsReadStats(uint64_t* read_sectors,
-                                       uint64_t* write_sectors) {
-  CHECK(read_sectors);
-  CHECK(write_sectors);
-  std::string line;
-  if (diskstats_path_.empty()) {
-    return false;
-  }
-
-  if (!base::ReadFileToString(base::FilePath(diskstats_path_), &line)) {
-    PLOG(WARNING) << "Could not read disk stats from " << diskstats_path_;
-    return false;
-  }
-
-  std::vector<std::string> parts = base::SplitString(
-      line, " ", base::TRIM_WHITESPACE, base::SPLIT_WANT_NONEMPTY);
-  if (parts.size() != kDiskMetricsStatItemCount) {
-    LOG(ERROR) << "Could not parse disk stat correctly. Expected "
-               << kDiskMetricsStatItemCount << " elements but got "
-               << parts.size();
-    return false;
-  }
-  if (!base::StringToUint64(parts[2], read_sectors)) {
-    LOG(ERROR) << "Couldn't convert read sectors " << parts[2] << " to uint64";
-    return false;
-  }
-  if (!base::StringToUint64(parts[6], write_sectors)) {
-    LOG(ERROR) << "Couldn't convert write sectors " << parts[6] << " to uint64";
-    return false;
-  }
-
-  return true;
-}
-
-bool MetricsDaemon::VmStatsParseStats(const char* stats,
-                                      struct VmstatRecord* record) {
-  CHECK(stats);
-  CHECK(record);
-  base::StringPairs pairs;
-  base::SplitStringIntoKeyValuePairs(stats, ' ', '\n', &pairs);
-
-  for (base::StringPairs::iterator it = pairs.begin(); it != pairs.end(); ++it) {
-    if (it->first == "pgmajfault" &&
-        !base::StringToUint64(it->second, &record->page_faults_)) {
-      return false;
-    }
-    if (it->first == "pswpin" &&
-        !base::StringToUint64(it->second, &record->swap_in_)) {
-      return false;
-    }
-    if (it->first == "pswpout" &&
-        !base::StringToUint64(it->second, &record->swap_out_)) {
-      return false;
-    }
-  }
-  return true;
-}
-
-bool MetricsDaemon::VmStatsReadStats(struct VmstatRecord* stats) {
-  CHECK(stats);
-  string value_string;
-  if (!base::ReadFileToString(base::FilePath(kVmStatFileName), &value_string)) {
-    LOG(WARNING) << "cannot read " << kVmStatFileName;
-    return false;
-  }
-  return VmStatsParseStats(value_string.c_str(), stats);
-}
 
 bool MetricsDaemon::ReadFreqToInt(const string& sysfs_file_name, int* value) {
   const FilePath sysfs_path(sysfs_file_name);
@@ -639,115 +511,6 @@ void MetricsDaemon::SendCpuThrottleMetrics() {
   SendLinearSample(kMetricScaledCpuFrequencyName, percent, 101, 102);
 }
 
-// Collects disk and vm stats alternating over a short and a long interval.
-
-void MetricsDaemon::StatsCallback() {
-  uint64_t read_sectors_now, write_sectors_now;
-  struct VmstatRecord vmstats_now;
-  double time_now = GetActiveTime();
-  double delta_time = time_now - stats_initial_time_;
-  if (testing_) {
-    // Fake the time when testing.
-    delta_time = stats_state_ == kStatsShort ?
-        kMetricStatsShortInterval : kMetricStatsLongInterval;
-  }
-  bool diskstats_success = DiskStatsReadStats(&read_sectors_now,
-                                              &write_sectors_now);
-  int delta_read = read_sectors_now - read_sectors_;
-  int delta_write = write_sectors_now - write_sectors_;
-  int read_sectors_per_second = delta_read / delta_time;
-  int write_sectors_per_second = delta_write / delta_time;
-  bool vmstats_success = VmStatsReadStats(&vmstats_now);
-  uint64_t delta_faults = vmstats_now.page_faults_ - vmstats_.page_faults_;
-  uint64_t delta_swap_in = vmstats_now.swap_in_ - vmstats_.swap_in_;
-  uint64_t delta_swap_out = vmstats_now.swap_out_ - vmstats_.swap_out_;
-  uint64_t page_faults_per_second = delta_faults / delta_time;
-  uint64_t swap_in_per_second = delta_swap_in / delta_time;
-  uint64_t swap_out_per_second = delta_swap_out / delta_time;
-
-  switch (stats_state_) {
-    case kStatsShort:
-      if (diskstats_success) {
-        SendSample(kMetricReadSectorsShortName,
-                   read_sectors_per_second,
-                   1,
-                   kMetricSectorsIOMax,
-                   kMetricSectorsBuckets);
-        SendSample(kMetricWriteSectorsShortName,
-                   write_sectors_per_second,
-                   1,
-                   kMetricSectorsIOMax,
-                   kMetricSectorsBuckets);
-      }
-      if (vmstats_success) {
-        SendSample(kMetricPageFaultsShortName,
-                   page_faults_per_second,
-                   1,
-                   kMetricPageFaultsMax,
-                   kMetricPageFaultsBuckets);
-        SendSample(kMetricSwapInShortName,
-                   swap_in_per_second,
-                   1,
-                   kMetricPageFaultsMax,
-                   kMetricPageFaultsBuckets);
-        SendSample(kMetricSwapOutShortName,
-                   swap_out_per_second,
-                   1,
-                   kMetricPageFaultsMax,
-                   kMetricPageFaultsBuckets);
-      }
-      // Schedule long callback.
-      stats_state_ = kStatsLong;
-      ScheduleStatsCallback(kMetricStatsLongInterval -
-                            kMetricStatsShortInterval);
-      break;
-    case kStatsLong:
-      if (diskstats_success) {
-        SendSample(kMetricReadSectorsLongName,
-                   read_sectors_per_second,
-                   1,
-                   kMetricSectorsIOMax,
-                   kMetricSectorsBuckets);
-        SendSample(kMetricWriteSectorsLongName,
-                   write_sectors_per_second,
-                   1,
-                   kMetricSectorsIOMax,
-                   kMetricSectorsBuckets);
-        // Reset sector counters.
-        read_sectors_ = read_sectors_now;
-        write_sectors_ = write_sectors_now;
-      }
-      if (vmstats_success) {
-        SendSample(kMetricPageFaultsLongName,
-                   page_faults_per_second,
-                   1,
-                   kMetricPageFaultsMax,
-                   kMetricPageFaultsBuckets);
-        SendSample(kMetricSwapInLongName,
-                   swap_in_per_second,
-                   1,
-                   kMetricPageFaultsMax,
-                   kMetricPageFaultsBuckets);
-        SendSample(kMetricSwapOutLongName,
-                   swap_out_per_second,
-                   1,
-                   kMetricPageFaultsMax,
-                   kMetricPageFaultsBuckets);
-
-        vmstats_ = vmstats_now;
-      }
-      SendCpuThrottleMetrics();
-      // Set start time for new cycle.
-      stats_initial_time_ = time_now;
-      // Schedule short callback.
-      stats_state_ = kStatsShort;
-      ScheduleStatsCallback(kMetricStatsShortInterval);
-      break;
-    default:
-      LOG(FATAL) << "Invalid stats state";
-  }
-}
-
 void MetricsDaemon::ScheduleMeminfoCallback(int wait) {
   if (testing_) {
     return;
diff --git a/metricsd/metrics_daemon.h b/metricsd/metrics_daemon.h
index eaa82192b..612dfe209 100644
--- a/metricsd/metrics_daemon.h
+++ b/metricsd/metrics_daemon.h
@@ -29,6 +29,7 @@
 #include <chromeos/daemons/dbus_daemon.h>
 #include <gtest/gtest_prod.h>  // for FRIEND_TEST
 
+#include "collectors/averaged_statistics_collector.h"
 #include "collectors/disk_usage_collector.h"
 #include "metrics/metrics_library.h"
 #include "persistent_integer.h"
@@ -65,6 +66,9 @@ class MetricsDaemon : public chromeos::DBusDaemon {
   // Triggers an upload event and exit. (Used to test UploadService)
   void RunUploaderTest();
 
+  // Returns the active time since boot (uptime minus sleep time) in seconds.
+  static double GetActiveTime();
+
  protected:
   // Used also by the unit tests.
   static const char kComprDataSizeName[];
@@ -79,8 +83,6 @@ class MetricsDaemon : public chromeos::DBusDaemon {
   FRIEND_TEST(MetricsDaemonTest, GetHistogramPath);
   FRIEND_TEST(MetricsDaemonTest, IsNewEpoch);
   FRIEND_TEST(MetricsDaemonTest, MessageFilter);
-  FRIEND_TEST(MetricsDaemonTest, ParseDiskStats);
-  FRIEND_TEST(MetricsDaemonTest, ParseVmStats);
   FRIEND_TEST(MetricsDaemonTest, ProcessKernelCrash);
   FRIEND_TEST(MetricsDaemonTest, ProcessMeminfo);
   FRIEND_TEST(MetricsDaemonTest, ProcessMeminfo2);
@@ -95,12 +97,6 @@ class MetricsDaemon : public chromeos::DBusDaemon {
   FRIEND_TEST(MetricsDaemonTest, SendCpuThrottleMetrics);
   FRIEND_TEST(MetricsDaemonTest, SendZramMetrics);
 
-  // State for disk stats collector callback.
-  enum StatsState {
-    kStatsShort,    // short wait before short interval collection
-    kStatsLong,     // final wait before new collection
-  };
-
   // Type of scale to use for meminfo histograms.  For most of them we use
   // percent of total RAM, but for some we use absolute numbers, usually in
   // megabytes, on a log scale from 0 to 4000, and 0 to 8000 for compressed
@@ -120,16 +116,6 @@ class MetricsDaemon : public chromeos::DBusDaemon {
     int value;               // value from /proc/meminfo
   };
 
-  // Record for retrieving and reporting values from /proc/vmstat
-  struct VmstatRecord {
-    uint64_t page_faults_;    // major faults
-    uint64_t swap_in_;        // pages swapped in
-    uint64_t swap_out_;       // pages swapped out
-  };
-
-  // Returns the active time since boot (uptime minus sleep time) in seconds.
-  double GetActiveTime();
-
   // D-Bus filter callback.
   static DBusHandlerResult MessageFilter(DBusConnection* connection,
                                          DBusMessage* message,
@@ -189,21 +175,6 @@ class MetricsDaemon : public chromeos::DBusDaemon {
   // Initializes vm and disk stats reporting.
   void StatsReporterInit();
 
-  // Schedules a callback for the next vm and disk stats collection.
-  void ScheduleStatsCallback(int wait);
-
-  // Reads cumulative disk statistics from sysfs.  Returns true for success.
-  bool DiskStatsReadStats(uint64_t* read_sectors, uint64_t* write_sectors);
-
-  // Reads cumulative vm statistics from procfs.  Returns true for success.
-  bool VmStatsReadStats(struct VmstatRecord* stats);
-
-  // Parse cumulative vm statistics from a C string.  Returns true for success.
-  bool VmStatsParseStats(const char* stats, struct VmstatRecord* record);
-
-  // Reports disk and vm statistics.
-  void StatsCallback();
-
   // Schedules meminfo collection callback.
   void ScheduleMeminfoCallback(int wait);
 
@@ -286,14 +257,6 @@ class MetricsDaemon : public chromeos::DBusDaemon {
   // Selects the wait time for the next memory use callback.
   unsigned int memuse_interval_index_;
 
-  // Contain the most recent disk and vm cumulative stats.
-  uint64_t read_sectors_;
-  uint64_t write_sectors_;
-  struct VmstatRecord vmstats_;
-
-  StatsState stats_state_;
-  double stats_initial_time_;
-
   // The system "HZ", or frequency of ticks.  Some system data uses ticks as a
   // unit, and this is used to convert to standard time units.
   uint32_t ticks_per_second_;
@@ -329,8 +292,8 @@ class MetricsDaemon : public chromeos::DBusDaemon {
   scoped_ptr<PersistentInteger> unclean_shutdowns_daily_count_;
   scoped_ptr<PersistentInteger> unclean_shutdowns_weekly_count_;
   scoped_ptr<DiskUsageCollector> disk_usage_collector_;
+  scoped_ptr<AveragedStatisticsCollector> averaged_stats_collector_;
 
-  std::string diskstats_path_;
   std::string scaling_max_freq_path_;
   std::string cpuinfo_max_freq_path_;
 
diff --git a/metricsd/metrics_daemon_test.cc b/metricsd/metrics_daemon_test.cc
index cc00cc27e..3a8fc3a2d 100644
--- a/metricsd/metrics_daemon_test.cc
+++ b/metricsd/metrics_daemon_test.cc
@@ -14,17 +14,12 @@
  * limitations under the License.
  */
 
-#include <inttypes.h>
-#include <utime.h>
-
-#include <string>
 #include <vector>
 
 #include <base/at_exit.h>
 #include <base/files/file_util.h>
 #include <base/files/scoped_temp_dir.h>
 #include <base/strings/string_number_conversions.h>
-#include <base/strings/stringprintf.h>
 #include <chromeos/flag_helper.h>
 #include <gtest/gtest.h>
 
@@ -34,10 +29,7 @@
 #include "persistent_integer_mock.h"
 
 using base::FilePath;
-using base::StringPrintf;
-using base::Time;
 using base::TimeDelta;
-using base::TimeTicks;
 using std::string;
 using std::vector;
 using ::testing::_;
@@ -47,34 +39,15 @@ using ::testing::Return;
 using ::testing::StrictMock;
 using chromeos_metrics::PersistentIntegerMock;
 
-static const char kFakeDiskStatsFormat[] =
-    "    1793     1788    %" PRIu64 "   105580    "
-    "    196      175     %" PRIu64 "    30290    "
-    "    0    44060   135850\n";
-static const uint64_t kFakeReadSectors[] = {80000, 100000};
-static const uint64_t kFakeWriteSectors[] = {3000, 4000};
-
 
 class MetricsDaemonTest : public testing::Test {
  protected:
-  std::string kFakeDiskStats0;
-  std::string kFakeDiskStats1;
-
   virtual void SetUp() {
     chromeos::FlagHelper::Init(0, nullptr, "");
     EXPECT_TRUE(temp_dir_.CreateUniqueTempDir());
     scaling_max_freq_path_ = temp_dir_.path().Append("scaling_max");
     cpu_max_freq_path_ = temp_dir_.path().Append("cpu_freq_max");
-    disk_stats_path_ = temp_dir_.path().Append("disk_stats");
 
-    kFakeDiskStats0 = base::StringPrintf(kFakeDiskStatsFormat,
-                                           kFakeReadSectors[0],
-                                           kFakeWriteSectors[0]);
-    kFakeDiskStats1 = base::StringPrintf(kFakeDiskStatsFormat,
-                                           kFakeReadSectors[1],
-                                           kFakeWriteSectors[1]);
-
-    CreateFakeDiskStatsFile(kFakeDiskStats0);
     CreateUint64ValueFile(cpu_max_freq_path_, 10000000);
     CreateUint64ValueFile(scaling_max_freq_path_, 10000000);
 
@@ -84,7 +57,7 @@ class MetricsDaemonTest : public testing::Test {
                  false,
                  true,
                  &metrics_lib_,
-                 disk_stats_path_.value(),
+                 "",
                  scaling_max_freq_path_.value(),
                  cpu_max_freq_path_.value(),
                  base::TimeDelta::FromMinutes(30),
@@ -131,12 +104,6 @@ class MetricsDaemonTest : public testing::Test {
     dbus_message_unref(msg);
   }
 
-  // Creates or overwrites an input file containing fake disk stats.
-  void CreateFakeDiskStatsFile(const string& fake_stats) {
-    EXPECT_EQ(base::WriteFile(disk_stats_path_,
-                              fake_stats.data(), fake_stats.size()),
-              fake_stats.size());
-  }
 
   // Creates or overwrites the file in |path| so that it contains the printable
   // representation of |value|.
@@ -156,7 +123,6 @@ class MetricsDaemonTest : public testing::Test {
   // Path for the fake files.
   base::FilePath scaling_max_freq_path_;
   base::FilePath cpu_max_freq_path_;
-  base::FilePath disk_stats_path_;
 
   // Mocks. They are strict mock so that all unexpected
   // calls are marked as failures.
@@ -200,21 +166,6 @@ TEST_F(MetricsDaemonTest, SendSample) {
                      /* min */ 1, /* max */ 100, /* buckets */ 50);
 }
 
-TEST_F(MetricsDaemonTest, ParseDiskStats) {
-  uint64_t read_sectors_now, write_sectors_now;
-  CreateFakeDiskStatsFile(kFakeDiskStats0);
-  ASSERT_TRUE(daemon_.DiskStatsReadStats(&read_sectors_now,
-                                         &write_sectors_now));
-  EXPECT_EQ(read_sectors_now, kFakeReadSectors[0]);
-  EXPECT_EQ(write_sectors_now, kFakeWriteSectors[0]);
-
-  CreateFakeDiskStatsFile(kFakeDiskStats1);
-  ASSERT_TRUE(daemon_.DiskStatsReadStats(&read_sectors_now,
-                                         &write_sectors_now));
-  EXPECT_EQ(read_sectors_now, kFakeReadSectors[1]);
-  EXPECT_EQ(write_sectors_now, kFakeWriteSectors[1]);
-}
-
 TEST_F(MetricsDaemonTest, ProcessMeminfo) {
   string meminfo =
       "MemTotal:        2000000 kB\nMemFree:          500000 kB\n"
@@ -258,16 +209,6 @@ TEST_F(MetricsDaemonTest, ProcessMeminfo2) {
   EXPECT_FALSE(daemon_.ProcessMeminfo(meminfo));
 }
 
-TEST_F(MetricsDaemonTest, ParseVmStats) {
-  static char kVmStats[] = "pswpin 1345\npswpout 8896\n"
-    "foo 100\nbar 200\npgmajfault 42\netcetc 300\n";
-  struct MetricsDaemon::VmstatRecord stats;
-  EXPECT_TRUE(daemon_.VmStatsParseStats(kVmStats, &stats));
-  EXPECT_EQ(stats.page_faults_, 42);
-  EXPECT_EQ(stats.swap_in_, 1345);
-  EXPECT_EQ(stats.swap_out_, 8896);
-}
-
 TEST_F(MetricsDaemonTest, ReadFreqToInt) {
   const int fake_scaled_freq = 1666999;
   const int fake_max_freq = 2000000;