Create a histogram serialization mechanism in components/metrics

components/metrics/chromeos/metrics_utils.cc

+// Copyright 2014 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#include "components/metrics/chromeos/metrics_utils.h"
+
+#include <sys/file.h>
+
+#include <string>
+#include <vector>
+
+#include "base/file_util.h"
+#include "base/files/file_path.h"
+#include "base/files/scoped_file.h"
+#include "base/logging.h"
+#include "base/memory/scoped_ptr.h"
+#include "base/memory/scoped_vector.h"
+#include "base/strings/string_split.h"
+#include "base/strings/string_util.h"
+#include "components/metrics/chromeos/metric_sample.h"
+
+#define READ_WRITE_ALL_FILE_FLAGS \
+  (S_IRUSR | S_IWUSR | S_IRGRP | S_IWGRP | S_IROTH | S_IWOTH)
+
+namespace metrics {
+
+scoped_ptr<MetricSample> MetricsUtils::ReadSample(const std::string& sample) {
+  std::vector<std::string> parts;
+  base::SplitString(sample, '\0', &parts);
+  // We should have two null terminated strings so split should produce
+  // three chunks.
+  if (parts.size() != 3) {
+    DLOG(ERROR) << "splitting message on \\0 produced " << parts.size()
+                << " parts (expected 3)";
+    return scoped_ptr<MetricSample>();
+  }
+  const std::string& name = parts[0];
+  const std::string& value = parts[1];
+
+  if (LowerCaseEqualsASCII(name, "crash")) {
+    return MetricSample::CrashSample(value);
+  } else if (LowerCaseEqualsASCII(name, "histogram")) {
+    return MetricSample::ReadHistogram(value);
+  } else if (LowerCaseEqualsASCII(name, "linearhistogram")) {
+    return MetricSample::ReadLinearHistogram(value);
+  } else if (LowerCaseEqualsASCII(name, "sparsehistogram")) {
+    return MetricSample::ReadSparseHistogram(value);
+  } else if (LowerCaseEqualsASCII(name, "useraction")) {
+    return MetricSample::UserActionSample(value);
+  } else {
+    DLOG(ERROR) << "invalid event type: " << name << ", value: " << value;
+  }
+  return scoped_ptr<MetricSample>();
+}
+
+bool MetricsUtils::WriteMetricToFile(const MetricSample* sample,
+                                     const std::string& filename) {
+  if (!sample->IsValid())
+    return false;
+
+  base::ScopedFD file_descriptor(open(filename.c_str(),
+                                      O_WRONLY | O_APPEND | O_CREAT,
+                                      READ_WRITE_ALL_FILE_FLAGS));
+
+  if (file_descriptor.get() < 0) {
+    DLOG(ERROR) << "error openning the file";
+    return false;
+  }
+
+  fchmod(file_descriptor.get(), READ_WRITE_ALL_FILE_FLAGS);
+  // Grab a lock to avoid chrome truncating the file
+  // underneath us. Keep the file locked as briefly as possible.
+  // Freeing file_descriptor will close the file and and remove the lock.
+  if (HANDLE_EINTR(flock(file_descriptor.get(), LOCK_EX)) < 0) {
+    DLOG(ERROR) << "error locking" << filename << " : " << errno;
+    return false;
+  }
+
+  std::string msg = sample->ToString();
+  int32_t size = msg.length() + sizeof(int32_t);

[Alexei Svitkine (slow), 2014/05/13 19:52:42]

Move |size| to right before it's used, i.e. line 85.

[bsimonnet, 2014/05/13 21:28:00]

Actually I should use size below.

+  if (msg.length() > size_t(kMessageMaxLength)) {
+    DLOG(ERROR) << "cannot write message: too long";
+    return false;
+  }
+
+  msg = std::string(reinterpret_cast<char*>(&size), sizeof(int32_t)) + msg;

[Alexei Svitkine (slow), 2014/05/13 19:52:42]

Can you use two WriteFileDescriptor() calls instead of doing this extra
temporary string construction?

Also, this code currently doesn't guarantee a byte order for |size|. You
probably want to ensure it's always in a given endianness (e.g. little endian)
via utils from "base/sys_byteorder.h". Same for the read code.

[bsimonnet, 2014/05/13 21:28:00]

Done
TL;DR; we do not have a way to make atomic change on chromeos and chrome.

Ideally we would have that but the current code (on chrome and chromeos) do not
guarantee the endianness either. If we make the endianness platform independant
here, we will have problem reading the metrics on a system where this library is
not used on both sides.

[Alexei Svitkine (slow), 2014/05/13 21:30:42]

I see. Are there actual big-endian ChromeOS devices currently? I was assuming
that there wasn't so this would be a safe change to do since it wouldn't break
anything - but will be futureproof if later there is a big-endian device.

If not, then I suggest adding a comment explicitly mentioning why it's not being
done.

[Luigi Semenzato, 2014/05/13 21:36:35]

The producer and the consumer of this data are running on the same device, so
they have the same endianness.

[bsimonnet, 2014/05/13 21:49:56]

Benchan mentioned that there are no big endian machines out there at the moment
but that might change in the future.
We can force the serialization to use little endian now without fearing a break.
It will be easier than enforcing a common endianness later on.

+
+  if (base::WriteFileDescriptor(file_descriptor.get(), msg.c_str(), size) !=
+      size) {
+    DLOG(ERROR) << "error writing to " << filename << " : " << errno;
+    return false;
+  }
+
+  return true;
+}
+
+void MetricsUtils::ReadAndTruncateMetricsFromFile(
+    const std::string& filename,
+    ScopedVector<MetricSample>* metrics) {
+  struct stat stat_buf;
+  int result;
+
+  result = stat(filename.c_str(), &stat_buf);
+  if (result < 0) {
+    if (errno != ENOENT) {
+      DPLOG(ERROR) << filename << ": bad metrics file stat";
+    }
+    // Nothing to collect---try later.
+    return;
+  }
+  if (stat_buf.st_size == 0) {
+    // Also nothing to collect.
+    return;
+  }
+  base::ScopedFD fd(open(filename.c_str(), O_RDWR));
+  if (fd.get() < 0) {
+    DPLOG(ERROR) << filename << ": cannot open";
+    return;
+  }
+  result = flock(fd.get(), LOCK_EX);
+  if (result < 0) {
+    DPLOG(ERROR) << filename << ": cannot lock";
+    return;
+  }
+  // This processes all messages in the log.  Each message starts with a 4-byte
+  // field containing the length of the entire message.  The length is followed
+  // by a name-value pair of null-terminated strings.  When all messages are
+  // read and processed, or an error occurs, truncate the file to zero size.
+  for (;;) {
+    int32 message_size;
+    result = HANDLE_EINTR(read(fd.get(), &message_size, sizeof(message_size)));
+    if (result < 0) {
+      DPLOG(ERROR) << "reading metrics message header";
+      break;
+    }
+    if (result == 0) {  // This indicates a normal EOF.
+      break;
+    }
+    if (result < static_cast<int>(sizeof(message_size))) {
+      DLOG(ERROR) << "bad read size " << result << ", expecting "
+                  << sizeof(message_size);
+      break;
+    }
+    // kMessageMaxLength applies to the entire message: the 4-byte
+    // length field and the two null-terminated strings.
+    if (message_size < 2 + static_cast<int>(sizeof(message_size))) {
+      DLOG(ERROR) << "bad message size " << message_size;
+      break;
+    }
+
+    if (message_size > kMessageMaxLength) {
+      DLOG(ERROR) << "message too long : " << message_size;
+      if (HANDLE_EINTR(lseek(fd.get(), message_size - 4, SEEK_CUR)) == -1) {
+        DLOG(ERROR) << "error while skipping message. abort";
+        break;
+      }
+      continue;
+    }
+
+    message_size -= sizeof(message_size);  // The message size includes itself.
+    char buffer[kMessageMaxLength];
+    result = HANDLE_EINTR(read(fd.get(), buffer, message_size));
+    if (result < 0) {
+      DPLOG(ERROR) << "reading metrics message body";
+      break;
+    }
+    if (result < message_size) {
+      DLOG(ERROR) << "message too short: length " << result << ", expected "
+                  << message_size;
+      break;
+    }
+
+    metrics->push_back(ReadSample(std::string(buffer, message_size)).release());
+  }
+
+  result = ftruncate(fd.get(), 0);
+  if (result < 0) {
+    DPLOG(ERROR) << "truncate metrics log";
+  }
+  result = flock(fd.get(), LOCK_UN);
+  if (result < 0) {
+    DPLOG(ERROR) << "unlock metrics log";
+  }
+}
+
+}  // namespace metrics