diff --git a/metrics/metrics_client.cc b/metrics/metrics_client.cc index cdea0124c..17f933cbb 100644 --- a/metrics/metrics_client.cc +++ b/metrics/metrics_client.cc @@ -2,22 +2,16 @@ // Use of this source code is governed by a BSD-style license that can be // found in the LICENSE file. -#include -#include -#include -#include - +#include #include -#include #include "metrics_library.h" -using namespace std; - // Usage: metrics_client [-ab] metric_name metric_value int main(int argc, char** argv) { bool send_to_autotest = false; bool send_to_chrome = true; + bool secs_to_msecs = false; int metric_name_index = 1; int metric_value_index = 2; bool print_usage = false; @@ -25,7 +19,7 @@ int main(int argc, char** argv) { if (argc >= 3) { // Parse arguments int flag; - while ((flag = getopt(argc, argv, "ab")) != -1) { + while ((flag = getopt(argc, argv, "abt")) != -1) { switch (flag) { case 'a': send_to_autotest = true; @@ -35,6 +29,9 @@ int main(int argc, char** argv) { send_to_chrome = true; send_to_autotest = true; break; + case 't': + secs_to_msecs = true; + break; default: print_usage = true; break; @@ -52,22 +49,31 @@ int main(int argc, char** argv) { } if (print_usage) { - cerr << "Usage: metrics_client [-ab] name value" << endl; - cerr << endl; - cerr << " default: send metric to chrome only" << endl; - cerr << " -a: send metric to autotest only" << endl; - cerr << " -b: send metric to both chrome and autotest" << endl; + fprintf(stderr, + "Usage: metrics_client [-abt] name value\n" + "\n" + " default: send metric with integer value to chrome only\n" + " -a: send metric to autotest only\n" + " -b: send metric to both chrome and autotest\n" + " -t: convert value from float seconds to int milliseconds\n"); return 1; } + const char* name = argv[metric_name_index]; + int value; + if (secs_to_msecs) { + float secs = strtof(argv[metric_value_index], NULL); + value = static_cast(secs * 1000.0f); + } else { + value = atoi(argv[metric_value_index]); + } + // Send metrics if (send_to_autotest) { - MetricsLibrary::SendToAutotest(argv[metric_name_index], - argv[metric_value_index]); + MetricsLibrary::SendToAutotest(name, value); } if (send_to_chrome) { - MetricsLibrary::SendToChrome(argv[metric_name_index], - argv[metric_value_index]); + MetricsLibrary::SendToChrome(name, value); } return 0; } diff --git a/metrics/metrics_daemon.cc b/metrics/metrics_daemon.cc index a924b8ab4..9bb9c205f 100644 --- a/metrics/metrics_daemon.cc +++ b/metrics/metrics_daemon.cc @@ -113,12 +113,10 @@ void MetricsDaemon::LogNetworkStateChange(const char* newstate) { if (diff.tv_sec >= INT_MAX / 1000) { diff_ms = INT_MAX; } - char buffer[100]; - snprintf(buffer, sizeof(buffer), "%d", diff_ms); if (testing_) { - TestPublishMetric(network_states_[old_id].stat_name, buffer); + TestPublishMetric(network_states_[old_id].stat_name, diff_ms); } else { - ChromePublishMetric(network_states_[old_id].stat_name, buffer); + ChromePublishMetric(network_states_[old_id].stat_name, diff_ms); } } network_state_id_ = new_id; @@ -135,10 +133,10 @@ MetricsDaemon::GetNetworkStateId(const char* state_name) { return static_cast(-1); } -void MetricsDaemon::ChromePublishMetric(const char* name, const char* value) { +void MetricsDaemon::ChromePublishMetric(const char* name, int value) { MetricsLibrary::SendToChrome(name, value); } -void MetricsDaemon::TestPublishMetric(const char* name, const char* value) { +void MetricsDaemon::TestPublishMetric(const char* name, int value) { LOG(INFO) << "received metric: " << name << " " << value; } diff --git a/metrics/metrics_daemon.h b/metrics/metrics_daemon.h index 2ac1ea0f8..dc097b5e1 100644 --- a/metrics/metrics_daemon.h +++ b/metrics/metrics_daemon.h @@ -64,10 +64,10 @@ class MetricsDaemon { NetworkStateId GetNetworkStateId(const char* state_name); // Sends a stat to Chrome for transport to UMA. - void ChromePublishMetric(const char* name, const char* value); + void ChromePublishMetric(const char* name, int value); // Prints a stat for testing. - void TestPublishMetric(const char* name, const char* value); + void TestPublishMetric(const char* name, int value); #if 0 // Fetches a name-value hash table from DBus. diff --git a/metrics/metrics_library.cc b/metrics/metrics_library.cc index 99ad61640..f482145a3 100644 --- a/metrics/metrics_library.cc +++ b/metrics/metrics_library.cc @@ -13,21 +13,23 @@ #include #include -#include -#include + +#include +#include +#include #define READ_WRITE_ALL_FILE_FLAGS \ (S_IRUSR | S_IWUSR | S_IRGRP | S_IWGRP | S_IROTH | S_IWOTH) static const char kAutotestPath[] = "/tmp/.chromeos-metrics-autotest"; static const char kChromePath[] = "/tmp/.chromeos-metrics"; -static const int kBufferSize = 4096; +static const int32_t kBufferSize = 1024; using namespace std; // TODO(sosa@chromium.org) - use Chromium logger instead of stderr -void MetricsLibrary::PrintError(const char *message, const char *file, - int code) { +static void PrintError(const char *message, const char *file, + int code) { const char *kProgramName = "metrics_library"; if (code == 0) { fprintf(stderr, "%s: %s\n", kProgramName, message); @@ -40,61 +42,105 @@ void MetricsLibrary::PrintError(const char *message, const char *file, } } -void MetricsLibrary::SendToAutotest(string name, string value) { - FILE *autotest_file = fopen(kAutotestPath, "a+"); - if (autotest_file == NULL) { - PrintError("fopen", kAutotestPath, errno); - return; - } - - fprintf(autotest_file, "%s=%s\n", name.c_str(), value.c_str()); - fclose(autotest_file); -} - -void MetricsLibrary::SendToChrome(string name, string value) { +// Sends message of size length to Chrome and returns true on success. +static bool SendMessageToChrome(int32_t length, const char *message) { int chrome_fd = open(kChromePath, O_WRONLY | O_APPEND | O_CREAT, READ_WRITE_ALL_FILE_FLAGS); - // If we failed to open it, return + // If we failed to open it, return. if (chrome_fd < 0) { PrintError("open", kChromePath, errno); - return; + return false; } - // Need to chmod because open flags are anded with umask. - if (fchmod(chrome_fd, READ_WRITE_ALL_FILE_FLAGS) < 0) { - PrintError("fchmod", kChromePath, errno); - close(chrome_fd); - return; - } + // Need to chmod because open flags are anded with umask. Ignore the + // exit code -- a chronos process may fail chmoding because the file + // has been created by a root process but that should be OK. + fchmod(chrome_fd, READ_WRITE_ALL_FILE_FLAGS); - // Grab an exclusive lock to protect Chrome from truncating underneath us + // Grab an exclusive lock to protect Chrome from truncating + // underneath us. Keep the file locked as briefly as possible. if (flock(chrome_fd, LOCK_EX) < 0) { PrintError("flock", kChromePath, errno); close(chrome_fd); - return; + return false; } - // Message format is: LENGTH (binary), NAME, VALUE - char message[kBufferSize]; - char *curr_ptr = message; - int32_t message_length = - name.length() + value.length() + 2 + sizeof(message_length); - if (message_length > static_cast(sizeof(message))) - PrintError("name/value too long", NULL, 0); - - // Make sure buffer is blanked - memset(message, 0, sizeof(message)); - memcpy(curr_ptr, &message_length, sizeof(message_length)); - curr_ptr += sizeof(message_length); - strncpy(curr_ptr, name.c_str(), name.length()); - curr_ptr += name.length() + 1; - strncpy(curr_ptr, value.c_str(), value.length()); - if (write(chrome_fd, message, message_length) != message_length) + bool success = true; + if (write(chrome_fd, message, length) != length) { PrintError("write", kChromePath, errno); + success = false; + } - // Release the file lock and close file - if (flock(chrome_fd, LOCK_UN) < 0) + // Release the file lock and close file. + if (flock(chrome_fd, LOCK_UN) < 0) { PrintError("unlock", kChromePath, errno); + success = false; + } close(chrome_fd); + return success; +} + +// Formats a name/value message for Chrome in buffer and returns the +// length of the message or a negative value on error. +// +// Message format is: | LENGTH(binary) | 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). +static int32_t 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; +} + +bool MetricsLibrary::SendToAutotest(string name, int value) { + FILE *autotest_file = fopen(kAutotestPath, "a+"); + if (autotest_file == NULL) { + PrintError("fopen", kAutotestPath, errno); + return false; + } + + fprintf(autotest_file, "%s=%d\n", name.c_str(), value); + fclose(autotest_file); + return true; +} + +bool MetricsLibrary::SendToChrome(string name, int value) { + // Format the message. + char message[kBufferSize]; + int32_t message_length = + FormatChromeMessage(kBufferSize, message, + "%s%c%d", name.c_str(), '\0', value); + + if (message_length < 0) + return false; + + // Send the message. + return SendMessageToChrome(message_length, message); } diff --git a/metrics/metrics_library.h b/metrics/metrics_library.h index f1268c971..ebc972cae 100644 --- a/metrics/metrics_library.h +++ b/metrics/metrics_library.h @@ -12,7 +12,6 @@ #ifndef METRICS_LIBRARY_H_ #define METRICS_LIBRARY_H_ -#include #include // TODO(sosa@chromium.org): Add testing for send methods @@ -20,14 +19,10 @@ // Library used to send metrics both Autotest and Chrome class MetricsLibrary { public: - // Sends histogram data to Chrome. - static void SendToChrome(std::string name, std::string value); - // Sends to Autotest. - static void SendToAutotest(std::string name, std::string value); - - private: - // Prints message to stderr - static void PrintError(const char *message, const char *file, int code); + // Sends histogram data to Chrome and returns true on success. + static bool SendToChrome(std::string name, int value); + // Sends to Autotest and returns true on success. + static bool SendToAutotest(std::string name, int value); }; #endif /* METRICS_LIBRARY_H_ */