metrics: Use the new serialization mechanism in libmetrics
Convert libmetrics to the new serialization mechanism living in
//components/metrics in the chrome repo.
BUG=chromium:374368
TEST=FEATURES=test emerge-amd64-generic metrics
Change-Id: Ia8cf128d04fedd9672fb47096dc6fd87d6a9043d
Reviewed-on: https://chromium-review.googlesource.com/207237
Reviewed-by: Luigi Semenzato <semenzato@chromium.org>
Commit-Queue: Bertrand Simonnet <bsimonnet@chromium.org>
Tested-by: Bertrand Simonnet <bsimonnet@chromium.org>
diff --git a/metrics/libmetrics.gypi b/metrics/libmetrics.gypi
index 1753830..65de6f5 100644
--- a/metrics/libmetrics.gypi
+++ b/metrics/libmetrics.gypi
@@ -24,7 +24,10 @@
'c_metrics_library.cc',
'metrics_library.cc',
'timer.cc',
+ 'components/metrics/chromeos/metric_sample.cc',
+ 'components/metrics/chromeos/serialization_utils.cc',
],
+ 'include_dirs': ['.'],
},
],
}
diff --git a/metrics/metrics.gyp b/metrics/metrics.gyp
index 044ff66..7a4338b 100644
--- a/metrics/metrics.gyp
+++ b/metrics/metrics.gyp
@@ -55,7 +55,8 @@
'target_name': 'libupload_service',
'type': 'static_library',
'dependencies': [
- 'metrics_proto'
+ 'metrics_proto',
+ '../metrics/libmetrics-<(libbase_ver).gyp:libmetrics-<(libbase_ver)',
],
'link_settings': {
'libraries': [
@@ -83,8 +84,6 @@
'uploader/metrics_log.cc',
'uploader/system_profile_cache.cc',
'uploader/curl_sender.cc',
- 'components/metrics/chromeos/metric_sample.cc',
- 'components/metrics/chromeos/serialization_utils.cc',
'components/metrics/metrics_log_base.cc',
'components/metrics/metrics_log_manager.cc',
'components/metrics/metrics_hashes.cc',
diff --git a/metrics/metrics_library.cc b/metrics/metrics_library.cc
index 4fbc1d8..c418864 100644
--- a/metrics/metrics_library.cc
+++ b/metrics/metrics_library.cc
@@ -2,7 +2,7 @@
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.
-#include "metrics_library.h"
+#include "metrics/metrics_library.h"
#include <base/logging.h>
#include <base/strings/stringprintf.h>
@@ -10,12 +10,11 @@
#include <sys/file.h>
#include <sys/stat.h>
-#include <cstdarg>
#include <cstdio>
#include <cstring>
-// HANDLE_EINTR macro, no libbase required.
-#include <base/posix/eintr_wrapper.h>
+#include "components/metrics/chromeos/metric_sample.h"
+#include "components/metrics/chromeos/serialization_utils.h"
#include "policy/device_policy.h"
@@ -51,22 +50,6 @@
time_t MetricsLibrary::cached_enabled_time_ = 0;
bool MetricsLibrary::cached_enabled_ = false;
-// Copied from libbase to avoid pulling in all of libbase just for libmetrics.
-static int WriteFileDescriptor(const int fd, const char* data, int size) {
- // Allow for partial writes.
- ssize_t bytes_written_total = 0;
- for (ssize_t bytes_written_partial = 0; bytes_written_total < size;
- bytes_written_total += bytes_written_partial) {
- bytes_written_partial =
- HANDLE_EINTR(write(fd, data + bytes_written_total,
- size - bytes_written_total));
- if (bytes_written_partial < 0)
- return -1;
- }
-
- return bytes_written_total;
-}
-
MetricsLibrary::MetricsLibrary() : consent_file_(kConsentFile) {}
MetricsLibrary::~MetricsLibrary() {}
@@ -169,55 +152,6 @@
return cached_enabled_;
}
-bool MetricsLibrary::SendMessageToChrome(const std::string& message) {
- int size = static_cast<int>(message.size());
- if (size > kBufferSize) {
- LOG(ERROR) << "chrome message too big (" << size << " bytes)";
- return false;
- }
- // Use libc here instead of chromium base classes because we need a UNIX fd
- // for flock. |uma_events_file_| must exist already.
- int chrome_fd = HANDLE_EINTR(open(uma_events_file_.c_str(),
- O_WRONLY | O_APPEND));
- // If we failed to open it, return.
- if (chrome_fd < 0) {
- PLOG(ERROR) << uma_events_file_ << ": open";
- return false;
- }
-
- // Grab an exclusive lock to protect Chrome from truncating
- // underneath us.
- if (HANDLE_EINTR(flock(chrome_fd, LOCK_EX)) < 0) {
- PLOG(ERROR) << uma_events_file_ << ": flock";
- IGNORE_EINTR(close(chrome_fd));
- return false;
- }
-
- bool success = true;
- if (WriteFileDescriptor(chrome_fd, message.c_str(), size) != size) {
- PLOG(ERROR) << uma_events_file_ << ": write";
- success = false;
- }
-
- // Close the file and release the lock.
- IGNORE_EINTR(close(chrome_fd));
- return success;
-}
-
-const std::string MetricsLibrary::FormatChromeMessage(
- const std::string& name,
- const std::string& value) {
- uint32 message_length =
- sizeof(message_length) + name.size() + 1 + value.size() + 1;
- std::string result;
- result.reserve(message_length);
- // Marshal the total message length in the native byte order.
- result.assign(reinterpret_cast<char*>(&message_length),
- sizeof(message_length));
- result += name + '\0' + value + '\0';
- return result;
-}
-
void MetricsLibrary::Init() {
uma_events_file_ = kUMAEventsPath;
}
@@ -239,43 +173,33 @@
int min,
int max,
int nbuckets) {
- // Format the message.
- std::string value = base::StringPrintf("%s %d %d %d %d",
- name.c_str(), sample, min, max, nbuckets);
- std::string message = FormatChromeMessage("histogram", value);
- // Send the message.
- return SendMessageToChrome(message);
+ return metrics::SerializationUtils::WriteMetricToFile(
+ *metrics::MetricSample::HistogramSample(name, sample, min, max, nbuckets)
+ .get(),
+ kUMAEventsPath);
}
bool MetricsLibrary::SendEnumToUMA(const std::string& name, int sample,
int max) {
- // Format the message.
- std::string value = base::StringPrintf("%s %d %d", name.c_str(), sample, max);
- std::string message = FormatChromeMessage("linearhistogram", value);
- // Send the message.
- return SendMessageToChrome(message);
+ return metrics::SerializationUtils::WriteMetricToFile(
+ *metrics::MetricSample::LinearHistogramSample(name, sample, max).get(),
+ kUMAEventsPath);
}
bool MetricsLibrary::SendSparseToUMA(const std::string& name, int sample) {
- // Format the message.
- std::string value = base::StringPrintf("%s %d", name.c_str(), sample);
- std::string message = FormatChromeMessage("sparsehistogram", value);
- // Send the message.
- return SendMessageToChrome(message);
+ return metrics::SerializationUtils::WriteMetricToFile(
+ *metrics::MetricSample::SparseHistogramSample(name, sample).get(),
+ kUMAEventsPath);
}
bool MetricsLibrary::SendUserActionToUMA(const std::string& action) {
- // Format the message.
- std::string message = FormatChromeMessage("useraction", action);
- // Send the message.
- return SendMessageToChrome(message);
+ return metrics::SerializationUtils::WriteMetricToFile(
+ *metrics::MetricSample::UserActionSample(action).get(), kUMAEventsPath);
}
bool MetricsLibrary::SendCrashToUMA(const char *crash_kind) {
- // Format the message.
- std::string message = FormatChromeMessage("crash", crash_kind);
- // Send the message.
- return SendMessageToChrome(message);
+ return metrics::SerializationUtils::WriteMetricToFile(
+ *metrics::MetricSample::CrashSample(crash_kind).get(), kUMAEventsPath);
}
void MetricsLibrary::SetPolicyProvider(policy::PolicyProvider* provider) {
diff --git a/metrics/metrics_library.h b/metrics/metrics_library.h
index e34ff4a..9e4be89 100644
--- a/metrics/metrics_library.h
+++ b/metrics/metrics_library.h
@@ -2,8 +2,8 @@
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.
-#ifndef METRICS_LIBRARY_H_
-#define METRICS_LIBRARY_H_
+#ifndef METRICS_METRICS_LIBRARY_H_
+#define METRICS_METRICS_LIBRARY_H_
#include <sys/types.h>
#include <string>
@@ -131,18 +131,6 @@
char* buffer, int buffer_size,
bool* result);
- // Sends message to Chrome for transport to UMA and returns true on success.
- bool SendMessageToChrome(const std::string& message);
-
- // Serializes a name/value pair into a message buffer.
- //
- // The serialized format is: | LENGTH | NAME | \0 | VALUE | \0 |
- //
- // where LENGTH is a 32-bit integer in native endianness, and NAME and VALUE
- // are null-terminated strings (the zero bytes are explicitly shown above).
- const std::string FormatChromeMessage(const std::string& name,
- const std::string& value);
-
// This function is used by tests only to mock the device policies.
void SetPolicyProvider(policy::PolicyProvider* provider);
@@ -160,4 +148,4 @@
DISALLOW_COPY_AND_ASSIGN(MetricsLibrary);
};
-#endif // METRICS_LIBRARY_H_
+#endif // METRICS_METRICS_LIBRARY_H_
diff --git a/metrics/metrics_library_test.cc b/metrics/metrics_library_test.cc
index 82418ed..ccd4129 100644
--- a/metrics/metrics_library_test.cc
+++ b/metrics/metrics_library_test.cc
@@ -10,8 +10,8 @@
#include <policy/mock_device_policy.h>
#include <policy/libpolicy.h>
-#include "c_metrics_library.h"
-#include "metrics_library.h"
+#include "metrics/c_metrics_library.h"
+#include "metrics/metrics_library.h"
using base::FilePath;
using ::testing::_;
@@ -165,89 +165,6 @@
VerifyEnabledCacheEviction(true);
}
-TEST_F(MetricsLibraryTest, FormatChromeMessage) {
- char kLen = 10;
- char exp[] = { kLen, 0, 0, 0, 'f', 'o', 'o', 0, '1', 0 };
- // The message is composed by a 4-byte length field, followed
- // by two null-terminated strings (name/value).
- EXPECT_EQ(sizeof(exp), kLen);
- EXPECT_EQ(std::string(exp, kLen), lib_.FormatChromeMessage("foo", "1"));
-}
-
-TEST_F(MetricsLibraryTest, SendEnumToUMA) {
- char buf[100];
- const int kLen = 40;
- EXPECT_TRUE(lib_.SendEnumToUMA("Test.EnumMetric", 1, 3));
- EXPECT_EQ(kLen, base::ReadFile(kTestUMAEventsFile, buf, 100));
-
- char exp[kLen];
- sprintf(exp, "%c%c%c%clinearhistogram%cTest.EnumMetric 1 3",
- kLen, 0, 0, 0, 0);
- EXPECT_EQ(0, memcmp(exp, buf, kLen));
-}
-
-TEST_F(MetricsLibraryTest, SendMessageToChrome) {
- EXPECT_TRUE(lib_.SendMessageToChrome(std::string("test")));
- EXPECT_TRUE(lib_.SendMessageToChrome(std::string("content")));
- std::string uma_events;
- EXPECT_TRUE(base::ReadFileToString(kTestUMAEventsFile, &uma_events));
- EXPECT_EQ("testcontent", uma_events);
-}
-
-TEST_F(MetricsLibraryTest, SendMessageToChromeUMAEventsBadFileLocation) {
- // Checks that the library doesn't die badly if the file can't be
- // created.
- static const char kDoesNotExistFile[] = "/does/not/exist";
- lib_.uma_events_file_ = kDoesNotExistFile;
- const std::string kDummyMessage = "Dummy Message";
- EXPECT_FALSE(lib_.SendMessageToChrome(kDummyMessage));
- base::DeleteFile(FilePath(kDoesNotExistFile), false);
-}
-
-TEST_F(MetricsLibraryTest, SendToUMA) {
- char buf[100];
- const int kLen = 37;
- EXPECT_TRUE(lib_.SendToUMA("Test.Metric", 2, 1, 100, 50));
- EXPECT_EQ(kLen, base::ReadFile(kTestUMAEventsFile, buf, 100));
-
- char exp[kLen];
- sprintf(exp, "%c%c%c%chistogram%cTest.Metric 2 1 100 50", kLen, 0, 0, 0, 0);
- EXPECT_EQ(0, memcmp(exp, buf, kLen));
-}
-
-TEST_F(MetricsLibraryTest, SendUserActionToUMA) {
- char buf[100];
- const int kLen = 30;
- EXPECT_TRUE(lib_.SendUserActionToUMA("SomeKeyPressed"));
- EXPECT_EQ(kLen, base::ReadFile(kTestUMAEventsFile, buf, 100));
-
- char exp[kLen];
- sprintf(exp, "%c%c%c%cuseraction%cSomeKeyPressed", kLen, 0, 0, 0, 0);
- EXPECT_EQ(0, memcmp(exp, buf, kLen));
-}
-
-TEST_F(MetricsLibraryTest, SendSparseToUMA) {
- char buf[100];
- const int kLen = 4 + sizeof("sparsehistogram") + sizeof("Test.Sparse 1234");
- EXPECT_TRUE(lib_.SendSparseToUMA("Test.Sparse", 1234));
- EXPECT_EQ(kLen, base::ReadFile(kTestUMAEventsFile, buf, 100));
-
- char exp[kLen];
- sprintf(exp, "%c%c%c%csparsehistogram%cTest.Sparse 1234", kLen, 0, 0, 0, 0);
- EXPECT_EQ(0, memcmp(exp, buf, kLen));
-}
-
-TEST_F(MetricsLibraryTest, SendCrashToUMA) {
- EXPECT_TRUE(lib_.SendCrashToUMA("kernel"));
- char exp[100];
- int len = sprintf(exp, "%c%c%c%ccrash%ckernel",
- 0, 0, 0, 0, 0) + 1;
- exp[0] = len;
- char buf[100];
- EXPECT_EQ(len, base::ReadFile(kTestUMAEventsFile, buf, 100));
- EXPECT_EQ(0, memcmp(exp, buf, len));
-}
-
class CMetricsLibraryTest : public testing::Test {
protected:
virtual void SetUp() {
@@ -290,51 +207,6 @@
EXPECT_TRUE(CMetricsLibraryAreMetricsEnabled(lib_));
}
-TEST_F(CMetricsLibraryTest, SendEnumToUMA) {
- char buf[100];
- const int kLen = 40;
- EXPECT_TRUE(CMetricsLibrarySendEnumToUMA(lib_, "Test.EnumMetric", 1, 3));
- EXPECT_EQ(kLen, base::ReadFile(kTestUMAEventsFile, buf, 100));
-
- char exp[kLen];
- sprintf(exp, "%c%c%c%clinearhistogram%cTest.EnumMetric 1 3",
- kLen, 0, 0, 0, 0);
- EXPECT_EQ(0, memcmp(exp, buf, kLen));
-}
-
-TEST_F(CMetricsLibraryTest, SendToUMA) {
- char buf[100];
- const int kLen = 37;
- EXPECT_TRUE(CMetricsLibrarySendToUMA(lib_, "Test.Metric", 2, 1, 100, 50));
- EXPECT_EQ(kLen, base::ReadFile(kTestUMAEventsFile, buf, 100));
-
- char exp[kLen];
- sprintf(exp, "%c%c%c%chistogram%cTest.Metric 2 1 100 50", kLen, 0, 0, 0, 0);
- EXPECT_EQ(0, memcmp(exp, buf, kLen));
-}
-
-TEST_F(CMetricsLibraryTest, SendUserActionToUMA) {
- char buf[100];
- const int kLen = 30;
- EXPECT_TRUE(CMetricsLibrarySendUserActionToUMA(lib_, "SomeKeyPressed"));
- EXPECT_EQ(kLen, base::ReadFile(kTestUMAEventsFile, buf, 100));
-
- char exp[kLen];
- sprintf(exp, "%c%c%c%cuseraction%cSomeKeyPressed", kLen, 0, 0, 0, 0);
- EXPECT_EQ(0, memcmp(exp, buf, kLen));
-}
-
-TEST_F(CMetricsLibraryTest, SendCrashToUMA) {
- char buf[100];
- char exp[100];
- int len = sprintf(exp, "%c%c%c%ccrash%cuser", 0, 0, 0, 0, 0) + 1;
- exp[0] = len;
- EXPECT_TRUE(CMetricsLibrarySendCrashToUMA(lib_, "user"));
- EXPECT_EQ(len, base::ReadFile(kTestUMAEventsFile, buf, 100));
-
- EXPECT_EQ(0, memcmp(exp, buf, len));
-}
-
int main(int argc, char** argv) {
testing::InitGoogleTest(&argc, argv);
return RUN_ALL_TESTS();