tequilaOS/platform_system_core
2
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity

metrics library: convert to proper C++/libbase

Browse source 
This mostly converts fprintfs to proper logs, char* to
std::string wherever appropriate.

BUG=chromium:355796
TEST=unit tests

Change-Id: Ieb1cb110be5e281b7e0c764a0dfce895f33d4a3c
Reviewed-on: https://chromium-review.googlesource.com/199610
Reviewed-by: Luigi Semenzato <semenzato@chromium.org>
Commit-Queue: Luigi Semenzato <semenzato@chromium.org>
Tested-by: Luigi Semenzato <semenzato@chromium.org>
This commit is contained in:
Luigi Semenzato 2014-05-13 15:16:24 -07:00 committed by chrome-internal-fetch
parent 3e8a851625
commit 41c5450523
3 changed files with 76 additions and 142 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

156
metrics/metrics_library.cc
View file

@ -4,6 +4,8 @@
#include "metrics_library.h"
#include <base/logging.h>
#include <base/strings/stringprintf.h>
#include <errno.h>
#include <sys/file.h>
#include <sys/stat.h>
@ -51,23 +53,6 @@ static const char *kCrosEventNames[] = {
time_t MetricsLibrary::cached_enabled_time_ = 0;
bool MetricsLibrary::cached_enabled_ = false;
using std::string;
// TODO(sosa@chromium.org) - use Chromium logger instead of stderr
static void PrintError(const char* message, const char* file,
int code) {
static const char kProgramName[] = "libmetrics";
if (code == 0) {
fprintf(stderr, "%s: %s\n", kProgramName, message);
} else if (file == NULL) {
fprintf(stderr, "%s: ", kProgramName);
perror(message);
} else {
fprintf(stderr, "%s: %s: ", kProgramName, file);
perror(message);
}
}
// 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.
@ -84,10 +69,7 @@ static int WriteFileDescriptor(const int fd, const char* data, int size) {
return bytes_written_total;
}
MetricsLibrary::MetricsLibrary()
: uma_events_file_(NULL),
consent_file_(kConsentFile) {}
MetricsLibrary::MetricsLibrary() : consent_file_(kConsentFile) {}
MetricsLibrary::~MetricsLibrary() {}
// We take buffer and buffer_size as parameters in order to simplify testing
@ -178,7 +160,7 @@ bool MetricsLibrary::AreMetricsEnabled() {
// still respect the consent file if it is present for migration purposes.
// TODO(pastarmovj)
if (!has_policy) {
enabled = stat(consent_file_, &stat_buffer) >= 0;
enabled = stat(consent_file_.c_str(), &stat_buffer) >= 0;
}
if (enabled && !IsGuestMode())
@ -189,14 +171,20 @@ bool MetricsLibrary::AreMetricsEnabled() {
return cached_enabled_;
}
bool MetricsLibrary::SendMessageToChrome(int32_t length, const char* message) {
int chrome_fd = HANDLE_EINTR(open(uma_events_file_,
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.
int chrome_fd = HANDLE_EINTR(open(uma_events_file_.c_str(),
O_WRONLY | O_APPEND | O_CREAT,
READ_WRITE_ALL_FILE_FLAGS));
// If we failed to open it, return.
if (chrome_fd < 0) {
PrintError("open", uma_events_file_, errno);
PLOG(ERROR) << uma_events_file_ << ": open";
return false;
}
@ -206,16 +194,16 @@ bool MetricsLibrary::SendMessageToChrome(int32_t length, const char* message) {
fchmod(chrome_fd, READ_WRITE_ALL_FILE_FLAGS);
// Grab an exclusive lock to protect Chrome from truncating
// underneath us. Keep the file locked as briefly as possible.
// underneath us.
if (HANDLE_EINTR(flock(chrome_fd, LOCK_EX)) < 0) {
PrintError("flock", uma_events_file_, errno);
PLOG(ERROR) << uma_events_file_ << ": flock";
IGNORE_EINTR(close(chrome_fd));
return false;
}
bool success = true;
if (WriteFileDescriptor(chrome_fd, message, length) != length) {
PrintError("write", uma_events_file_, errno);
if (WriteFileDescriptor(chrome_fd, message.c_str(), size) != size) {
PLOG(ERROR) << uma_events_file_ << ": write";
success = false;
}
@ -224,44 +212,28 @@ bool MetricsLibrary::SendMessageToChrome(int32_t length, const char* message) {
return success;
}
int32_t MetricsLibrary::FormatChromeMessage(int32_t buffer_size, char* buffer,
const char* format, ...) {
int32_t message_length;
size_t len_size = sizeof(message_length);
// Format the non-LENGTH contents in the buffer by leaving space for
// LENGTH at the start of the buffer.
va_list args;
va_start(args, format);
message_length = vsnprintf(&buffer[len_size], buffer_size - len_size,
format, args);
va_end(args);
if (message_length < 0) {
PrintError("chrome message format error", NULL, 0);
return -1;
}
// +1 to account for the trailing \0.
message_length += len_size + 1;
if (message_length > buffer_size) {
PrintError("chrome message too long", NULL, 0);
return -1;
}
// Prepend LENGTH to the message.
memcpy(buffer, &message_length, len_size);
return message_length;
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;
}
bool MetricsLibrary::SendToAutotest(const string& name, int value) {
bool MetricsLibrary::SendToAutotest(const std::string& name, int value) {
FILE* autotest_file = fopen(kAutotestPath, "a+");
if (autotest_file == NULL) {
PrintError("fopen", kAutotestPath, errno);
PLOG(ERROR) << kAutotestPath << ": fopen";
return false;
}
@ -270,74 +242,48 @@ bool MetricsLibrary::SendToAutotest(const string& name, int value) {
return true;
}
bool MetricsLibrary::SendToUMA(const string& name, int sample,
int min, int max, int nbuckets) {
bool MetricsLibrary::SendToUMA(const std::string& name,
int sample,
int min,
int max,
int nbuckets) {
// Format the message.
char message[kBufferSize];
int32_t message_length =
FormatChromeMessage(kBufferSize, message,
"histogram%c%s %d %d %d %d", '\0',
name.c_str(), sample, min, max, nbuckets);
if (message_length < 0)
return false;
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_length, message);
return SendMessageToChrome(message);
}
bool MetricsLibrary::SendEnumToUMA(const std::string& name, int sample,
int max) {
// Format the message.
char message[kBufferSize];
int32_t message_length =
FormatChromeMessage(kBufferSize, message,
"linearhistogram%c%s %d %d", '\0',
name.c_str(), sample, max);
if (message_length < 0)
return false;
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_length, message);
return SendMessageToChrome(message);
}
bool MetricsLibrary::SendSparseToUMA(const std::string& name, int sample) {
// Format the message.
char message[kBufferSize];
int32_t message_length =
FormatChromeMessage(kBufferSize, message, "sparsehistogram%c%s %d",
'\0', name.c_str(), sample);
if (message_length < 0)
return false;
std::string value = base::StringPrintf("%s %d", name.c_str(), sample);
std::string message = FormatChromeMessage("sparsehistogram", value);
// Send the message.
return SendMessageToChrome(message_length, message);
return SendMessageToChrome(message);
}
bool MetricsLibrary::SendUserActionToUMA(const std::string& action) {
// Format the message.
char message[kBufferSize];
int32_t message_length =
FormatChromeMessage(kBufferSize, message,
"useraction%c%s", '\0', action.c_str());
if (message_length < 0)
return false;
std::string message = FormatChromeMessage("useraction", action);
// Send the message.
return SendMessageToChrome(message_length, message);
return SendMessageToChrome(message);
}
bool MetricsLibrary::SendCrashToUMA(const char *crash_kind) {
// Format the message.
char message[kBufferSize];
int32_t message_length =
FormatChromeMessage(kBufferSize, message,
"crash%c%s", '\0', crash_kind);
if (message_length < 0)
return false;
std::string message = FormatChromeMessage("crash", crash_kind);
// Send the message.
return SendMessageToChrome(message_length, message);
return SendMessageToChrome(message);
}
void MetricsLibrary::SetPolicyProvider(policy::PolicyProvider* provider) {

24
metrics/metrics_library.h
View file

@ -133,22 +133,16 @@ class MetricsLibrary : public MetricsLibraryInterface {
// Sends message of size |length| to Chrome for transport to UMA and
// returns true on success.
bool SendMessageToChrome(int32_t length, const char* message);
bool SendMessageToChrome(const std::string& message);
// Formats a name/value message for Chrome in |buffer| and returns the
// length of the message or a negative value on error.
// Serializes a name/value pair into a message buffer.
//
// Message format is: | LENGTH(binary) | NAME | \0 | VALUE | \0 |
// The serialized format is: | LENGTH | NAME | \0 | VALUE | \0 |
//
// The arbitrary |format| argument covers the non-LENGTH portion of the
// message. The caller is responsible to store the \0 character
// between NAME and VALUE (e.g. "%s%c%d", name, '\0', value).
//
// Ideally we'd use "3, 4" here instead of "4, 5", but it seems clang/gcc
// have an implicit first arg ("this"). http://crbug.com/329356
__attribute__((__format__(__printf__, 4, 5)))
int32_t FormatChromeMessage(int32_t buffer_size, char* buffer,
const char* format, ...);
// 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);
@ -159,8 +153,8 @@ class MetricsLibrary : public MetricsLibraryInterface {
// Cached state of whether or not metrics were enabled.
static bool cached_enabled_;
const char* uma_events_file_;
const char* consent_file_;
std::string uma_events_file_;
std::string consent_file_;
scoped_ptr<policy::PolicyProvider> policy_provider_;

38
metrics/metrics_library_test.cc
View file

@ -29,10 +29,10 @@ ACTION_P(SetMetricsPolicy, enabled) {
class MetricsLibraryTest : public testing::Test {
protected:
virtual void SetUp() {
EXPECT_EQ(NULL, lib_.uma_events_file_);
EXPECT_TRUE(lib_.uma_events_file_.empty());
lib_.Init();
EXPECT_TRUE(NULL != lib_.uma_events_file_);
lib_.uma_events_file_ = kTestUMAEventsFile.value().c_str();
EXPECT_FALSE(lib_.uma_events_file_.empty());
lib_.uma_events_file_ = kTestUMAEventsFile.value();
device_policy_ = new policy::MockDevicePolicy();
EXPECT_CALL(*device_policy_, LoadPolicy())
.Times(AnyNumber())
@ -165,18 +165,12 @@ TEST_F(MetricsLibraryTest, AreMetricsEnabledCaching) {
}
TEST_F(MetricsLibraryTest, FormatChromeMessage) {
char buf[7];
const int kLen = 6;
EXPECT_EQ(kLen, lib_.FormatChromeMessage(7, buf, "%d", 1));
char exp[kLen];
sprintf(exp, "%c%c%c%c1", kLen, 0, 0, 0);
EXPECT_EQ(0, memcmp(exp, buf, kLen));
}
TEST_F(MetricsLibraryTest, FormatChromeMessageTooLong) {
char buf[7];
EXPECT_EQ(-1, lib_.FormatChromeMessage(7, buf, "test"));
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) {
@ -192,8 +186,8 @@ TEST_F(MetricsLibraryTest, SendEnumToUMA) {
}
TEST_F(MetricsLibraryTest, SendMessageToChrome) {
EXPECT_TRUE(lib_.SendMessageToChrome(4, "test"));
EXPECT_TRUE(lib_.SendMessageToChrome(7, "content"));
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);
@ -204,8 +198,8 @@ TEST_F(MetricsLibraryTest, SendMessageToChromeUMAEventsBadFileLocation) {
// created.
static const char kDoesNotExistFile[] = "/does/not/exist";
lib_.uma_events_file_ = kDoesNotExistFile;
static const char kDummyMessage[] = "Dummy Message";
EXPECT_FALSE(lib_.SendMessageToChrome(strlen(kDummyMessage), kDummyMessage));
const std::string kDummyMessage = "Dummy Message";
EXPECT_FALSE(lib_.SendMessageToChrome(kDummyMessage));
base::DeleteFile(FilePath(kDoesNotExistFile), false);
}
@ -258,10 +252,10 @@ class CMetricsLibraryTest : public testing::Test {
virtual void SetUp() {
lib_ = CMetricsLibraryNew();
MetricsLibrary& ml = *reinterpret_cast<MetricsLibrary*>(lib_);
EXPECT_EQ(NULL, ml.uma_events_file_);
EXPECT_TRUE(ml.uma_events_file_.empty());
CMetricsLibraryInit(lib_);
EXPECT_TRUE(NULL != ml.uma_events_file_);
ml.uma_events_file_ = kTestUMAEventsFile.value().c_str();
EXPECT_FALSE(ml.uma_events_file_.empty());
ml.uma_events_file_ = kTestUMAEventsFile.value();
device_policy_ = new policy::MockDevicePolicy();
EXPECT_CALL(*device_policy_, LoadPolicy())
.Times(AnyNumber())