Collect some disk statistics.

metrics_daemon.cc

Index: metrics_daemon.cc
diff --git a/metrics_daemon.cc b/metrics_daemon.cc
index a5059d9832bff092d4000a82746ba799f8759172..287555ad0c99c47391de6cf63019d5e68b7e0ad2 100644
--- a/metrics_daemon.cc
+++ b/metrics_daemon.cc
@@ -4,6 +4,7 @@
 
 #include "metrics_daemon.h"
 
+#include <fcntl.h>
 #include <string.h>
 
 #include <base/file_util.h>
@@ -84,6 +85,26 @@ const char MetricsDaemon::kMetricCrashFrequencyMin = 1;
 const char MetricsDaemon::kMetricCrashFrequencyMax = 100;
 const char MetricsDaemon::kMetricCrashFrequencyBuckets = 50;
 
+// disk stats metrics
+const char MetricsDaemon::kMetricReadSectorsLongName[] =
+    "Platform.ReadSectorsLong";

[petkov, 2011/02/11 06:42:30]

maybe add a comment about the units -- sectors/second? is a sector 512 bytes?

+const char MetricsDaemon::kMetricWriteSectorsLongName[] =
+    "Platform.WriteSectorsLong";
+const char MetricsDaemon::kMetricReadSectorsShortName[] =
+    "Platform.ReadSectorsShort";
+const char MetricsDaemon::kMetricWriteSectorsShortName[] =
+    "Platform.WriteSectorsShort";
+
+const int MetricsDaemon::kMetricDiskStatsShortInterval = 1;    // seconds

[petkov, 2011/02/11 06:42:30]

just two spaces before //

+const int MetricsDaemon::kMetricDiskStatsLongInterval = 30;    // seconds
+
+// Assume a max rate of 250Mb/s for reads, worse for writes.
+const int MetricsDaemon::kMetricSectorsIOMax = 500000;
+const int MetricsDaemon::kMetricSectorsBuckets = 50;
+
+// Path to disk stats.
+const char MetricsDaemon::kMetricsDiskStatsPath[] = "/sys/class/block/sda/stat";

[petkov, 2011/02/11 06:42:30]

x86 specific... is it easy to make this work for ARM too?

[Luigi Semenzato, 2011/02/12 00:33:02]

ARM uses the same path, at least on my machine.

+
 // persistent metrics path
 const char MetricsDaemon::kMetricsPath[] = "/var/log/metrics";
 
@@ -218,6 +239,8 @@ void MetricsDaemon::Init(bool testing, MetricsLibraryInterface* metrics_lib) {
   ConfigureCrashFrequencyReporter(kMetricUserCrashesDailyName);
   ConfigureCrashFrequencyReporter(kMetricUserCrashesWeeklyName);
 
+  DiskStatsReporterInit();
+
   // Don't setup D-Bus and GLib in test mode.
   if (testing)
     return;
@@ -494,6 +517,97 @@ void MetricsDaemon::UnscheduleUseMonitor() {
   usemon_interval_ = 0;
 }
 
+void MetricsDaemon::DiskStatsReporterInit() {
+  diskstats_source_ = NULL;
+  DiskStatsReadStats(&read_sectors_, &write_sectors_);
+  // The first time around just run the long stat, so we don't delay boot.
+  diskstats_state_ = kDiskStatsLong;
+  ScheduleDiskStatsCallback(kMetricDiskStatsLongInterval);
+}
+
+void MetricsDaemon::ScheduleDiskStatsCallback(int wait) {
+  diskstats_source_ = g_timeout_source_new_seconds(wait);

[petkov, 2011/02/11 06:42:30]

You don't really need to store the source for this sort of timeout scheduling
because you're never destroying the source directly. So, the 3 lines can be
collapsed into a single line:

g_timeout_add_seconds(wait, DiskStatsCallbackStatic, this);

[Luigi Semenzato, 2011/02/12 00:33:02]

Splendid, thanks!

+  g_source_set_callback(diskstats_source_, DiskStatsCallbackStatic, this, NULL);
+  g_source_attach(diskstats_source_, NULL);
+}
+
+void MetricsDaemon::DiskStatsReadStats(long int* read_sectors,
+                                       long int* write_sectors) {
+  int nchars;
+  int nitems;
+  char line[200];
+  int file = open(kMetricsDiskStatsPath, O_RDONLY);

[petkov, 2011/02/11 06:42:30]

want to use HANDLE_EINTR macro here and below for the syscalls? see counter.cc.

[Luigi Semenzato, 2011/02/12 00:33:02]

Thanks, good suggestion.

+  LOG_IF(FATAL, file < 0) << "cannot open " << kMetricsDiskStatsPath << ": "

[petkov, 2011/02/11 06:42:30]

FATAL? This would cause the process to crash every 30 seconds in case this
fails... Actually, right now it means the process will crash every 30 seconds on
ARM.

[Luigi Semenzato, 2011/02/12 00:33:02]

You are completely right.  Making non-fatal.

+                          << strerror(errno);
+  nchars = read(file, line, sizeof(line));
+  LOG_IF(FATAL, nchars == sizeof(line)) << "line too long in "
+                                        << kMetricsDiskStatsPath;
+  line[nchars] = '\0';
+  nitems = sscanf(line, "%*d %*d %ld %*d %*d %*d %ld",
+                  read_sectors, write_sectors);
+  LOG_IF(FATAL, nitems != 2) << "found " << nitems << " in "
+                             << kMetricsDiskStatsPath << ", expected 2";
+  close(file);
+}
+
+// static
+gboolean MetricsDaemon::DiskStatsCallbackStatic(void* handle) {
+  (static_cast<MetricsDaemon*>(handle))->DiskStatsCallback();
+  return false;         // destroy this source instance

[petkov, 2011/02/11 06:42:30]

just 2 spaces before //

+}
+
+void MetricsDaemon::DiskStatsCallback() {
+  long int read_sectors_now, write_sectors_now;
+  DiskStatsReadStats(&read_sectors_now, &write_sectors_now);
+
+  switch (diskstats_state_) {
+
+  case kDiskStatsShort:

[petkov, 2011/02/11 06:42:30]

per style, case needs to be 2-space indented relative to switch; case body
2-space indented relative to case. don't you use emacs with google style
configuration? :-)

[Luigi Semenzato, 2011/02/12 00:33:02]

I was overriding some indentation variables by mistake.  It works now, thanks.

+  SendMetric(kMetricReadSectorsShortName,
+             (int) (read_sectors_now - read_sectors_) /
+             kMetricDiskStatsShortInterval,
+             0,
+             kMetricSectorsIOMax,
+             kMetricSectorsBuckets);
+  SendMetric(kMetricWriteSectorsShortName,
+             (int) (write_sectors_now - write_sectors_) /
+             kMetricDiskStatsShortInterval,
+             0,
+             kMetricSectorsIOMax,
+             kMetricSectorsBuckets);
+  // Schedule long callback.
+  diskstats_state_ = kDiskStatsLong;
+  ScheduleDiskStatsCallback(kMetricDiskStatsLongInterval -
+                            kMetricDiskStatsShortInterval);
+  break;
+
+  case kDiskStatsLong:
+  SendMetric(kMetricReadSectorsLongName,
+             (int) (read_sectors_now - read_sectors_) /
+             kMetricDiskStatsLongInterval,
+             0,
+             kMetricSectorsIOMax,
+             kMetricSectorsBuckets);
+  SendMetric(kMetricWriteSectorsLongName,
+             (int) (write_sectors_now - write_sectors_) /
+             kMetricDiskStatsLongInterval,
+             0,
+             kMetricSectorsIOMax,
+             kMetricSectorsBuckets);
+  // Reset sector counters
+  read_sectors_ = read_sectors_now;
+  write_sectors_ = write_sectors_now;
+
+  // Schedule short callback.
+  diskstats_state_ = kDiskStatsShort;
+  ScheduleDiskStatsCallback(kMetricDiskStatsShortInterval);
+  break;
+
+  default:
+    LOG(FATAL) << "Invalid disk stats state";
+  }
+}
+
 // static
 void MetricsDaemon::ReportDailyUse(void* handle, int tag, int count) {
   if (count <= 0)