Merge "libutils: fix cache removal when callback invalidates the key"
diff --git a/adb/Android.mk b/adb/Android.mk
index 5ddc937..c38cf93 100644
--- a/adb/Android.mk
+++ b/adb/Android.mk
@@ -261,9 +261,12 @@
libadb \
libbase \
libcrypto_static \
- libcutils \
liblog \
+# Don't use libcutils on Windows.
+LOCAL_STATIC_LIBRARIES_darwin := libcutils
+LOCAL_STATIC_LIBRARIES_linux := libcutils
+
LOCAL_CXX_STL := libc++_static
# Don't add anything here, we don't want additional shared dependencies
diff --git a/adb/commandline.cpp b/adb/commandline.cpp
index c13872a..73c8912 100644
--- a/adb/commandline.cpp
+++ b/adb/commandline.cpp
@@ -591,14 +591,15 @@
std::vector<std::string> args;
if (use_shell_protocol) {
args.push_back(kShellServiceArgShellProtocol);
+
+ const char* terminal_type = getenv("TERM");
+ if (terminal_type != nullptr) {
+ args.push_back(std::string("TERM=") + terminal_type);
+ }
}
if (!type_arg.empty()) {
args.push_back(type_arg);
}
- const char* terminal_type = getenv("TERM");
- if (terminal_type != nullptr) {
- args.push_back(std::string("TERM=") + terminal_type);
- }
// Shell service string can look like: shell[,arg1,arg2,...]:[command].
return android::base::StringPrintf("shell%s%s:%s",
diff --git a/adb/file_sync_client.cpp b/adb/file_sync_client.cpp
index a2d2a66..ad38369 100644
--- a/adb/file_sync_client.cpp
+++ b/adb/file_sync_client.cpp
@@ -403,9 +403,9 @@
if (!sc.SendRequest(ID_RECV, rpath)) return false;
adb_unlink(lpath);
- if (!mkdirs(adb_dirname(lpath))) {
- sc.Error("failed to create parent directory '%s': %s",
- adb_dirname(lpath).c_str(), strerror(errno));
+ const std::string dirpath = adb_dirname(lpath);
+ if (!mkdirs(dirpath.c_str())) {
+ sc.Error("failed to create parent directory '%s': %s", dirpath.c_str(), strerror(errno));
return false;
}
diff --git a/base/include/base/unique_fd.h b/base/include/base/unique_fd.h
new file mode 100644
index 0000000..4117775
--- /dev/null
+++ b/base/include/base/unique_fd.h
@@ -0,0 +1,79 @@
+/*
+ * Copyright (C) 2015 The Android Open Source Project
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+#ifndef ANDROID_BASE_UNIQUE_FD_H
+#define ANDROID_BASE_UNIQUE_FD_H
+
+#include <unistd.h>
+
+#include <base/macros.h>
+
+/* Container for a file descriptor that automatically closes the descriptor as
+ * it goes out of scope.
+ *
+ * unique_fd ufd(open("/some/path", "r"));
+ *
+ * if (ufd.get() < 0) // invalid descriptor
+ * return error;
+ *
+ * // Do something useful
+ *
+ * return 0; // descriptor is closed here
+ */
+namespace android {
+namespace base {
+
+class unique_fd final {
+ public:
+ unique_fd() : value_(-1) {}
+
+ explicit unique_fd(int value) : value_(value) {}
+ ~unique_fd() { clear(); }
+
+ unique_fd(unique_fd&& other) : value_(other.release()) {}
+ unique_fd& operator = (unique_fd&& s) {
+ reset(s.release());
+ return *this;
+ }
+
+ void reset(int new_value) {
+ if (value_ >= 0)
+ close(value_);
+ value_ = new_value;
+ }
+
+ void clear() {
+ reset(-1);
+ }
+
+ int get() const { return value_; }
+
+ int release() {
+ int ret = value_;
+ value_ = -1;
+ return ret;
+ }
+
+ private:
+ int value_;
+
+ DISALLOW_COPY_AND_ASSIGN(unique_fd);
+};
+
+} // namespace base
+} // namespace android
+
+#endif // ANDROID_BASE_UNIQUE_FD_H
diff --git a/debuggerd/.clang-format b/debuggerd/.clang-format
new file mode 100644
index 0000000..9b7478c
--- /dev/null
+++ b/debuggerd/.clang-format
@@ -0,0 +1,15 @@
+BasedOnStyle: Google
+AllowShortBlocksOnASingleLine: false
+AllowShortFunctionsOnASingleLine: false
+
+ColumnLimit: 100
+CommentPragmas: NOLINT:.*
+DerivePointerAlignment: false
+IndentWidth: 2
+ContinuationIndentWidth: 2
+PointerAlignment: Left
+TabWidth: 2
+UseTab: Never
+PenaltyExcessCharacter: 32
+
+Cpp11BracedListStyle: false
diff --git a/debuggerd/debuggerd.cpp b/debuggerd/debuggerd.cpp
index 1287fb9..884d4d5 100644
--- a/debuggerd/debuggerd.cpp
+++ b/debuggerd/debuggerd.cpp
@@ -34,9 +34,10 @@
#include <log/logger.h>
-#include <cutils/sockets.h>
-#include <cutils/properties.h>
#include <cutils/debugger.h>
+#include <cutils/properties.h>
+#include <cutils/sockets.h>
+#include <nativehelper/ScopedFd.h>
#include <linux/input.h>
@@ -336,160 +337,146 @@
static void handle_request(int fd) {
ALOGV("handle_request(%d)\n", fd);
+ ScopedFd closer(fd);
debugger_request_t request;
memset(&request, 0, sizeof(request));
int status = read_request(fd, &request);
- if (!status) {
- ALOGV("BOOM: pid=%d uid=%d gid=%d tid=%d\n",
- request.pid, request.uid, request.gid, request.tid);
+ if (status != 0) {
+ return;
+ }
+
+ ALOGV("BOOM: pid=%d uid=%d gid=%d tid=%d\n", request.pid, request.uid, request.gid, request.tid);
#if defined(__LP64__)
- // On 64 bit systems, requests to dump 32 bit and 64 bit tids come
- // to the 64 bit debuggerd. If the process is a 32 bit executable,
- // redirect the request to the 32 bit debuggerd.
- if (is32bit(request.tid)) {
- // Only dump backtrace and dump tombstone requests can be redirected.
- if (request.action == DEBUGGER_ACTION_DUMP_BACKTRACE
- || request.action == DEBUGGER_ACTION_DUMP_TOMBSTONE) {
- redirect_to_32(fd, &request);
- } else {
- ALOGE("debuggerd: Not allowed to redirect action %d to 32 bit debuggerd\n",
- request.action);
- }
- close(fd);
- return;
- }
-#endif
-
- // At this point, the thread that made the request is blocked in
- // a read() call. If the thread has crashed, then this gives us
- // time to PTRACE_ATTACH to it before it has a chance to really fault.
- //
- // The PTRACE_ATTACH sends a SIGSTOP to the target process, but it
- // won't necessarily have stopped by the time ptrace() returns. (We
- // currently assume it does.) We write to the file descriptor to
- // ensure that it can run as soon as we call PTRACE_CONT below.
- // See details in bionic/libc/linker/debugger.c, in function
- // debugger_signal_handler().
- if (ptrace(PTRACE_ATTACH, request.tid, 0, 0)) {
- ALOGE("ptrace attach failed: %s\n", strerror(errno));
+ // On 64 bit systems, requests to dump 32 bit and 64 bit tids come
+ // to the 64 bit debuggerd. If the process is a 32 bit executable,
+ // redirect the request to the 32 bit debuggerd.
+ if (is32bit(request.tid)) {
+ // Only dump backtrace and dump tombstone requests can be redirected.
+ if (request.action == DEBUGGER_ACTION_DUMP_BACKTRACE ||
+ request.action == DEBUGGER_ACTION_DUMP_TOMBSTONE) {
+ redirect_to_32(fd, &request);
} else {
- bool detach_failed = false;
- bool tid_unresponsive = false;
- bool attach_gdb = should_attach_gdb(&request);
- if (TEMP_FAILURE_RETRY(write(fd, "\0", 1)) != 1) {
- ALOGE("failed responding to client: %s\n", strerror(errno));
- } else {
- char* tombstone_path = NULL;
-
- if (request.action == DEBUGGER_ACTION_CRASH) {
- close(fd);
- fd = -1;
- }
-
- int total_sleep_time_usec = 0;
- for (;;) {
- int signal = wait_for_sigstop(request.tid, &total_sleep_time_usec, &detach_failed);
- if (signal == -1) {
- tid_unresponsive = true;
- break;
- }
-
- switch (signal) {
- case SIGSTOP:
- if (request.action == DEBUGGER_ACTION_DUMP_TOMBSTONE) {
- ALOGV("stopped -- dumping to tombstone\n");
- tombstone_path = engrave_tombstone(request.pid, request.tid,
- signal, request.original_si_code,
- request.abort_msg_address, true,
- &detach_failed, &total_sleep_time_usec);
- } else if (request.action == DEBUGGER_ACTION_DUMP_BACKTRACE) {
- ALOGV("stopped -- dumping to fd\n");
- dump_backtrace(fd, -1, request.pid, request.tid, &detach_failed,
- &total_sleep_time_usec);
- } else {
- ALOGV("stopped -- continuing\n");
- status = ptrace(PTRACE_CONT, request.tid, 0, 0);
- if (status) {
- ALOGE("ptrace continue failed: %s\n", strerror(errno));
- }
- continue; // loop again
- }
- break;
-
- case SIGABRT:
- case SIGBUS:
- case SIGFPE:
- case SIGILL:
- case SIGSEGV:
-#ifdef SIGSTKFLT
- case SIGSTKFLT:
+ ALOGE("debuggerd: Not allowed to redirect action %d to 32 bit debuggerd\n", request.action);
+ }
+ return;
+ }
#endif
- case SIGTRAP:
- ALOGV("stopped -- fatal signal\n");
- // Send a SIGSTOP to the process to make all of
- // the non-signaled threads stop moving. Without
- // this we get a lot of "ptrace detach failed:
- // No such process".
- kill(request.pid, SIGSTOP);
- // don't dump sibling threads when attaching to GDB because it
- // makes the process less reliable, apparently...
- tombstone_path = engrave_tombstone(request.pid, request.tid,
- signal, request.original_si_code,
- request.abort_msg_address, !attach_gdb,
- &detach_failed, &total_sleep_time_usec);
- break;
- default:
- ALOGE("process stopped due to unexpected signal %d\n", signal);
- break;
- }
- break;
- }
+ // At this point, the thread that made the request is blocked in
+ // a read() call. If the thread has crashed, then this gives us
+ // time to PTRACE_ATTACH to it before it has a chance to really fault.
+ //
+ // The PTRACE_ATTACH sends a SIGSTOP to the target process, but it
+ // won't necessarily have stopped by the time ptrace() returns. (We
+ // currently assume it does.) We write to the file descriptor to
+ // ensure that it can run as soon as we call PTRACE_CONT below.
+ // See details in bionic/libc/linker/debugger.c, in function
+ // debugger_signal_handler().
+ if (ptrace(PTRACE_ATTACH, request.tid, 0, 0)) {
+ ALOGE("ptrace attach failed: %s\n", strerror(errno));
+ return;
+ }
- if (request.action == DEBUGGER_ACTION_DUMP_TOMBSTONE) {
- if (tombstone_path) {
- write(fd, tombstone_path, strlen(tombstone_path));
- }
- close(fd);
- fd = -1;
- }
- free(tombstone_path);
- }
+ bool detach_failed = false;
+ bool tid_unresponsive = false;
+ bool attach_gdb = should_attach_gdb(&request);
+ if (TEMP_FAILURE_RETRY(write(fd, "\0", 1)) != 1) {
+ ALOGE("failed responding to client: %s\n", strerror(errno));
+ return;
+ }
- if (!tid_unresponsive) {
- ALOGV("detaching");
- if (attach_gdb) {
- // stop the process so we can debug
- kill(request.pid, SIGSTOP);
- }
- if (ptrace(PTRACE_DETACH, request.tid, 0, 0)) {
- ALOGE("ptrace detach from %d failed: %s", request.tid, strerror(errno));
- detach_failed = true;
- } else if (attach_gdb) {
- // if debug.db.uid is set, its value indicates if we should wait
- // for user action for the crashing process.
- // in this case, we log a message and turn the debug LED on
- // waiting for a gdb connection (for instance)
- wait_for_user_action(request);
- }
- }
-
- // resume stopped process (so it can crash in peace).
- kill(request.pid, SIGCONT);
-
- // If we didn't successfully detach, we're still the parent, and the
- // actual parent won't receive a death notification via wait(2). At this point
- // there's not much we can do about that.
- if (detach_failed) {
- ALOGE("debuggerd committing suicide to free the zombie!\n");
- kill(getpid(), SIGKILL);
- }
+ std::unique_ptr<char> tombstone_path;
+ int total_sleep_time_usec = 0;
+ while (true) {
+ int signal = wait_for_sigstop(request.tid, &total_sleep_time_usec, &detach_failed);
+ if (signal == -1) {
+ tid_unresponsive = true;
+ break;
}
+ switch (signal) {
+ case SIGSTOP:
+ if (request.action == DEBUGGER_ACTION_DUMP_TOMBSTONE) {
+ ALOGV("stopped -- dumping to tombstone\n");
+ tombstone_path.reset(engrave_tombstone(
+ request.pid, request.tid, signal, request.original_si_code, request.abort_msg_address,
+ true, &detach_failed, &total_sleep_time_usec));
+ } else if (request.action == DEBUGGER_ACTION_DUMP_BACKTRACE) {
+ ALOGV("stopped -- dumping to fd\n");
+ dump_backtrace(fd, -1, request.pid, request.tid, &detach_failed, &total_sleep_time_usec);
+ } else {
+ ALOGV("stopped -- continuing\n");
+ status = ptrace(PTRACE_CONT, request.tid, 0, 0);
+ if (status) {
+ ALOGE("ptrace continue failed: %s\n", strerror(errno));
+ }
+ continue; // loop again
+ }
+ break;
+
+ case SIGABRT:
+ case SIGBUS:
+ case SIGFPE:
+ case SIGILL:
+ case SIGSEGV:
+#ifdef SIGSTKFLT
+ case SIGSTKFLT:
+#endif
+ case SIGTRAP:
+ ALOGV("stopped -- fatal signal\n");
+ // Send a SIGSTOP to the process to make all of
+ // the non-signaled threads stop moving. Without
+ // this we get a lot of "ptrace detach failed:
+ // No such process".
+ kill(request.pid, SIGSTOP);
+ // don't dump sibling threads when attaching to GDB because it
+ // makes the process less reliable, apparently...
+ tombstone_path.reset(engrave_tombstone(
+ request.pid, request.tid, signal, request.original_si_code, request.abort_msg_address,
+ !attach_gdb, &detach_failed, &total_sleep_time_usec));
+ break;
+
+ default:
+ ALOGE("process stopped due to unexpected signal %d\n", signal);
+ break;
+ }
+ break;
}
- if (fd >= 0) {
- close(fd);
+
+ if (request.action == DEBUGGER_ACTION_DUMP_TOMBSTONE) {
+ if (tombstone_path) {
+ write(fd, tombstone_path.get(), strlen(tombstone_path.get()));
+ }
+ }
+
+ if (!tid_unresponsive) {
+ ALOGV("detaching");
+ if (attach_gdb) {
+ // stop the process so we can debug
+ kill(request.pid, SIGSTOP);
+ }
+ if (ptrace(PTRACE_DETACH, request.tid, 0, 0)) {
+ ALOGE("ptrace detach from %d failed: %s", request.tid, strerror(errno));
+ detach_failed = true;
+ } else if (attach_gdb) {
+ // if debug.db.uid is set, its value indicates if we should wait
+ // for user action for the crashing process.
+ // in this case, we log a message and turn the debug LED on
+ // waiting for a gdb connection (for instance)
+ wait_for_user_action(request);
+ }
+ }
+
+ // resume stopped process (so it can crash in peace).
+ kill(request.pid, SIGCONT);
+
+ // If we didn't successfully detach, we're still the parent, and the
+ // actual parent won't receive a death notification via wait(2). At this point
+ // there's not much we can do about that.
+ if (detach_failed) {
+ ALOGE("debuggerd committing suicide to free the zombie!\n");
+ kill(getpid(), SIGKILL);
}
}
diff --git a/include/log/log.h b/include/log/log.h
index 1cdf7bc..086d742 100644
--- a/include/log/log.h
+++ b/include/log/log.h
@@ -614,9 +614,11 @@
/*
* Use the per-tag properties "log.tag.<tagname>" to generate a runtime
- * result of non-zero to expose a log.
+ * result of non-zero to expose a log. prio is ANDROID_LOG_VERBOSE to
+ * ANDROID_LOG_FATAL. default_prio if no property. Undefined behavior if
+ * any other value.
*/
-int __android_log_is_loggable(int prio, const char *tag, int def);
+int __android_log_is_loggable(int prio, const char *tag, int default_prio);
int __android_log_error_write(int tag, const char *subTag, int32_t uid, const char *data,
uint32_t dataLen);
diff --git a/liblog/fake_log_device.c b/liblog/fake_log_device.c
index 8a8ece2..cb80ee6 100644
--- a/liblog/fake_log_device.c
+++ b/liblog/fake_log_device.c
@@ -99,6 +99,10 @@
static void lock()
{
+ /*
+ * If we trigger a signal handler in the middle of locked activity and the
+ * signal handler logs a message, we could get into a deadlock state.
+ */
pthread_mutex_lock(&fakeLogDeviceLock);
}
@@ -106,9 +110,12 @@
{
pthread_mutex_unlock(&fakeLogDeviceLock);
}
+
#else // !defined(_WIN32)
+
#define lock() ((void)0)
#define unlock() ((void)0)
+
#endif // !defined(_WIN32)
diff --git a/liblog/log_is_loggable.c b/liblog/log_is_loggable.c
index 814d96d..e128edb 100644
--- a/liblog/log_is_loggable.c
+++ b/liblog/log_is_loggable.c
@@ -23,6 +23,22 @@
#include <android/log.h>
+static pthread_mutex_t lock_loggable = PTHREAD_MUTEX_INITIALIZER;
+
+static void lock()
+{
+ /*
+ * If we trigger a signal handler in the middle of locked activity and the
+ * signal handler logs a message, we could get into a deadlock state.
+ */
+ pthread_mutex_lock(&lock_loggable);
+}
+
+static void unlock()
+{
+ pthread_mutex_unlock(&lock_loggable);
+}
+
struct cache {
const prop_info *pinfo;
uint32_t serial;
@@ -49,9 +65,7 @@
cache->c = buf[0];
}
-static pthread_mutex_t lock = PTHREAD_MUTEX_INITIALIZER;
-
-static int __android_log_level(const char *tag, int def)
+static int __android_log_level(const char *tag, int default_prio)
{
/* sizeof() is used on this array below */
static const char log_namespace[] = "persist.log.tag.";
@@ -86,7 +100,7 @@
strcpy(key, log_namespace);
- pthread_mutex_lock(&lock);
+ lock();
current_global_serial = __system_property_area_serial();
@@ -156,7 +170,7 @@
global_serial = current_global_serial;
- pthread_mutex_unlock(&lock);
+ unlock();
switch (toupper(c)) {
case 'V': return ANDROID_LOG_VERBOSE;
@@ -168,36 +182,47 @@
case 'A': return ANDROID_LOG_FATAL;
case 'S': return -1; /* ANDROID_LOG_SUPPRESS */
}
- return def;
+ return default_prio;
}
-int __android_log_is_loggable(int prio, const char *tag, int def)
+int __android_log_is_loggable(int prio, const char *tag, int default_prio)
{
- int logLevel = __android_log_level(tag, def);
+ int logLevel = __android_log_level(tag, default_prio);
return logLevel >= 0 && prio >= logLevel;
}
+/*
+ * Timestamp state generally remains constant, since a change is
+ * rare, we can accept a trylock failure gracefully. Use a separate
+ * lock from is_loggable to keep contention down b/25563384.
+ */
+static pthread_mutex_t lock_timestamp = PTHREAD_MUTEX_INITIALIZER;
+
char android_log_timestamp()
{
static struct cache r_time_cache = { NULL, -1, 0 };
static struct cache p_time_cache = { NULL, -1, 0 };
- static uint32_t serial;
- uint32_t current_serial;
char retval;
- pthread_mutex_lock(&lock);
+ if (pthread_mutex_trylock(&lock_timestamp)) {
+ /* We are willing to accept some race in this context */
+ if (!(retval = p_time_cache.c)) {
+ retval = r_time_cache.c;
+ }
+ } else {
+ static uint32_t serial;
+ uint32_t current_serial = __system_property_area_serial();
+ if (current_serial != serial) {
+ refresh_cache(&r_time_cache, "ro.logd.timestamp");
+ refresh_cache(&p_time_cache, "persist.logd.timestamp");
+ serial = current_serial;
+ }
+ if (!(retval = p_time_cache.c)) {
+ retval = r_time_cache.c;
+ }
- current_serial = __system_property_area_serial();
- if (current_serial != serial) {
- refresh_cache(&r_time_cache, "ro.logd.timestamp");
- refresh_cache(&p_time_cache, "persist.logd.timestamp");
- serial = current_serial;
+ pthread_mutex_unlock(&lock_timestamp);
}
- if (!(retval = p_time_cache.c)) {
- retval = r_time_cache.c;
- }
-
- pthread_mutex_unlock(&lock);
return tolower(retval ?: 'r');
}
diff --git a/liblog/logd_write.c b/liblog/logd_write.c
index a4310ae..83c6dc2 100644
--- a/liblog/logd_write.c
+++ b/liblog/logd_write.c
@@ -54,14 +54,35 @@
static int __write_to_log_init(log_id_t, struct iovec *vec, size_t nr);
static int (*write_to_log)(log_id_t, struct iovec *vec, size_t nr) = __write_to_log_init;
-#if !defined(_WIN32)
-static pthread_mutex_t log_init_lock = PTHREAD_MUTEX_INITIALIZER;
-#endif
#ifndef __unused
#define __unused __attribute__((__unused__))
#endif
+#if !defined(_WIN32)
+static pthread_mutex_t log_init_lock = PTHREAD_MUTEX_INITIALIZER;
+
+static void lock()
+{
+ /*
+ * If we trigger a signal handler in the middle of locked activity and the
+ * signal handler logs a message, we could get into a deadlock state.
+ */
+ pthread_mutex_lock(&log_init_lock);
+}
+
+static void unlock()
+{
+ pthread_mutex_unlock(&log_init_lock);
+}
+
+#else /* !defined(_WIN32) */
+
+#define lock() ((void)0)
+#define unlock() ((void)0)
+
+#endif /* !defined(_WIN32) */
+
#if FAKE_LOG_DEVICE
static int log_fds[(int)LOG_ID_MAX] = { -1, -1, -1, -1, -1 };
#else
@@ -277,15 +298,11 @@
if (ret < 0) {
ret = -errno;
if (ret == -ENOTCONN) {
-#if !defined(_WIN32)
- pthread_mutex_lock(&log_init_lock);
-#endif
+ lock();
close(logd_fd);
logd_fd = -1;
ret = __write_to_log_initialize();
-#if !defined(_WIN32)
- pthread_mutex_unlock(&log_init_lock);
-#endif
+ unlock();
if (ret < 0) {
return ret;
@@ -329,18 +346,14 @@
static int __write_to_log_init(log_id_t log_id, struct iovec *vec, size_t nr)
{
-#if !defined(_WIN32)
- pthread_mutex_lock(&log_init_lock);
-#endif
+ lock();
if (write_to_log == __write_to_log_init) {
int ret;
ret = __write_to_log_initialize();
if (ret < 0) {
-#if !defined(_WIN32)
- pthread_mutex_unlock(&log_init_lock);
-#endif
+ unlock();
#if (FAKE_LOG_DEVICE == 0)
if (pstore_fd >= 0) {
__write_to_log_daemon(log_id, vec, nr);
@@ -352,9 +365,7 @@
write_to_log = __write_to_log_daemon;
}
-#if !defined(_WIN32)
- pthread_mutex_unlock(&log_init_lock);
-#endif
+ unlock();
return write_to_log(log_id, vec, nr);
}
diff --git a/liblog/logprint.c b/liblog/logprint.c
index 9f12c96..ebf9786 100644
--- a/liblog/logprint.c
+++ b/liblog/logprint.c
@@ -908,7 +908,7 @@
} else if (*message == '\b') {
strcpy(buf, "\\b");
} else if (*message == '\t') {
- strcpy(buf, "\\t");
+ strcpy(buf, "\t"); // Do not escape tabs
} else if (*message == '\v') {
strcpy(buf, "\\v");
} else if (*message == '\f') {
diff --git a/metricsd/Android.mk b/metricsd/Android.mk
index 291c983..97c0d28 100644
--- a/metricsd/Android.mk
+++ b/metricsd/Android.mk
@@ -25,11 +25,14 @@
metrics_client_sources := \
metrics_client.cc
-metrics_daemon_common := \
+metrics_collector_common := \
collectors/averaged_statistics_collector.cc \
collectors/cpu_usage_collector.cc \
collectors/disk_usage_collector.cc \
- metrics_daemon.cc \
+ metrics_collector.cc \
+ persistent_integer.cc \
+
+metricsd_common := \
persistent_integer.cc \
serialization/metric_sample.cc \
serialization/serialization_utils.cc \
@@ -40,14 +43,16 @@
uploader/system_profile_cache.cc \
uploader/upload_service.cc \
-metrics_tests_sources := \
+metrics_collector_tests_sources := \
collectors/averaged_statistics_collector_test.cc \
collectors/cpu_usage_collector_test.cc \
- metrics_daemon_test.cc \
+ metrics_collector_test.cc \
metrics_library_test.cc \
persistent_integer_test.cc \
serialization/serialization_utils_unittest.cc \
timer_test.cc \
+
+metricsd_tests_sources := \
uploader/metrics_hashes_unittest.cc \
uploader/metrics_log_base_unittest.cc \
uploader/mock/sender_mock.cc \
@@ -56,7 +61,6 @@
metrics_CFLAGS := -Wall \
-Wno-char-subscripts \
-Wno-missing-field-initializers \
- -Wno-unused-function \
-Wno-unused-parameter \
-Werror \
-fvisibility=default
@@ -67,17 +71,22 @@
metrics_includes := external/gtest/include \
$(LOCAL_PATH)/include
libmetrics_shared_libraries := libchrome libbrillo
-metrics_daemon_shared_libraries := $(libmetrics_shared_libraries) \
- libbrillo-http \
+metrics_collector_shared_libraries := $(libmetrics_shared_libraries) \
libbrillo-dbus \
+ libbrillo-http \
libchrome-dbus \
libdbus \
libmetrics \
- libprotobuf-cpp-lite \
librootdev \
- libupdate_engine_client \
libweaved \
+metricsd_shared_libraries := \
+ libbrillo \
+ libbrillo-http \
+ libchrome \
+ libprotobuf-cpp-lite \
+ libupdate_engine_client \
+
# Shared library for metrics.
# ========================================================
include $(CLEAR_VARS)
@@ -107,10 +116,10 @@
LOCAL_SRC_FILES := $(metrics_client_sources)
include $(BUILD_EXECUTABLE)
-# Protobuf library for metrics_daemon.
+# Protobuf library for metricsd.
# ========================================================
include $(CLEAR_VARS)
-LOCAL_MODULE := metrics_daemon_protos
+LOCAL_MODULE := metricsd_protos
LOCAL_MODULE_CLASS := STATIC_LIBRARIES
generated_sources_dir := $(call local-generated-sources-dir)
LOCAL_EXPORT_C_INCLUDE_DIRS += \
@@ -118,40 +127,71 @@
LOCAL_SRC_FILES := $(call all-proto-files-under,uploader/proto)
include $(BUILD_STATIC_LIBRARY)
-# metrics daemon.
+# metrics_collector daemon.
# ========================================================
include $(CLEAR_VARS)
-LOCAL_MODULE := metrics_daemon
+LOCAL_MODULE := metrics_collector
LOCAL_C_INCLUDES := $(metrics_includes)
LOCAL_CFLAGS := $(metrics_CFLAGS)
+LOCAL_CLANG := true
LOCAL_CPP_EXTENSION := $(metrics_cpp_extension)
LOCAL_CPPFLAGS := $(metrics_CPPFLAGS)
-LOCAL_INIT_RC := metrics_daemon.rc
+LOCAL_INIT_RC := metrics_collector.rc
LOCAL_REQUIRED_MODULES := \
metrics.json \
- metrics.schema.json \
-
+ metrics.schema.json
LOCAL_RTTI_FLAG := -frtti
-LOCAL_SHARED_LIBRARIES := $(metrics_daemon_shared_libraries)
-LOCAL_CLANG := true
-LOCAL_SRC_FILES := $(metrics_daemon_common) \
- metrics_daemon_main.cc
-LOCAL_STATIC_LIBRARIES := metrics_daemon_protos
+LOCAL_SHARED_LIBRARIES := $(metrics_collector_shared_libraries)
+LOCAL_SRC_FILES := $(metrics_collector_common) \
+ metrics_collector_main.cc
include $(BUILD_EXECUTABLE)
-# Unit tests for metrics.
+# metricsd daemon.
# ========================================================
include $(CLEAR_VARS)
-LOCAL_MODULE := metrics_tests
-LOCAL_CLANG := true
+LOCAL_MODULE := metricsd
+LOCAL_C_INCLUDES := $(metrics_includes)
LOCAL_CFLAGS := $(metrics_CFLAGS)
+LOCAL_CLANG := true
+LOCAL_CPP_EXTENSION := $(metrics_cpp_extension)
+LOCAL_CPPFLAGS := $(metrics_CPPFLAGS)
+LOCAL_INIT_RC := metricsd.rc
+LOCAL_REQUIRED_MODULES := \
+ metrics_collector
+LOCAL_RTTI_FLAG := -frtti
+LOCAL_SHARED_LIBRARIES := $(metricsd_shared_libraries)
+LOCAL_STATIC_LIBRARIES := metricsd_protos
+LOCAL_SRC_FILES := $(metricsd_common) \
+ metricsd_main.cc
+include $(BUILD_EXECUTABLE)
+
+# Unit tests for metricsd.
+# ========================================================
+include $(CLEAR_VARS)
+LOCAL_MODULE := metricsd_tests
+LOCAL_CFLAGS := $(metrics_CFLAGS)
+LOCAL_CLANG := true
LOCAL_CPP_EXTENSION := $(metrics_cpp_extension)
LOCAL_CPPFLAGS := $(metrics_CPPFLAGS) -Wno-sign-compare
LOCAL_RTTI_FLAG := -frtti
-LOCAL_SHARED_LIBRARIES := $(metrics_daemon_shared_libraries)
-LOCAL_SRC_FILES := $(metrics_tests_sources) $(metrics_daemon_common)
-LOCAL_STATIC_LIBRARIES := libBionicGtestMain libgmock metrics_daemon_protos
+LOCAL_SHARED_LIBRARIES := $(metricsd_shared_libraries) libmetrics
+LOCAL_SRC_FILES := $(metricsd_tests_sources) $(metricsd_common)
+LOCAL_STATIC_LIBRARIES := libBionicGtestMain libgmock metricsd_protos
+include $(BUILD_NATIVE_TEST)
+# Unit tests for metrics_collector.
+# ========================================================
+include $(CLEAR_VARS)
+LOCAL_MODULE := metrics_collector_tests
+LOCAL_CFLAGS := $(metrics_CFLAGS)
+LOCAL_CLANG := true
+LOCAL_CPP_EXTENSION := $(metrics_cpp_extension)
+LOCAL_CPPFLAGS := $(metrics_CPPFLAGS) -Wno-sign-compare
+LOCAL_RTTI_FLAG := -frtti
+LOCAL_SHARED_LIBRARIES := $(metrics_collector_shared_libraries)
+LOCAL_SRC_FILES := $(metrics_collector_tests_sources) \
+ $(metrics_collector_common)
+LOCAL_STATIC_LIBRARIES := libBionicGtestMain libgmock
include $(BUILD_NATIVE_TEST)
# Weave schema files
diff --git a/metricsd/collectors/averaged_statistics_collector.cc b/metricsd/collectors/averaged_statistics_collector.cc
index 0931e7b..bac2870 100644
--- a/metricsd/collectors/averaged_statistics_collector.cc
+++ b/metricsd/collectors/averaged_statistics_collector.cc
@@ -21,7 +21,7 @@
#include <base/strings/string_number_conversions.h>
#include <base/strings/string_split.h>
-#include "metrics_daemon.h"
+#include "metrics_collector.h"
namespace {
@@ -90,7 +90,7 @@
}
void AveragedStatisticsCollector::ReadInitialValues() {
- stats_start_time_ = MetricsDaemon::GetActiveTime();
+ stats_start_time_ = MetricsCollector::GetActiveTime();
DiskStatsReadStats(&read_sectors_, &write_sectors_);
VmStatsReadStats(&vmstats_);
}
@@ -168,7 +168,7 @@
void AveragedStatisticsCollector::Collect() {
uint64_t read_sectors_now, write_sectors_now;
struct VmstatRecord vmstats_now;
- double time_now = MetricsDaemon::GetActiveTime();
+ double time_now = MetricsCollector::GetActiveTime();
double delta_time = time_now - stats_start_time_;
bool diskstats_success = DiskStatsReadStats(&read_sectors_now,
&write_sectors_now);
diff --git a/metricsd/metrics_daemon.cc b/metricsd/metrics_collector.cc
similarity index 82%
rename from metricsd/metrics_daemon.cc
rename to metricsd/metrics_collector.cc
index e711660..28194a1 100644
--- a/metricsd/metrics_daemon.cc
+++ b/metricsd/metrics_collector.cc
@@ -14,7 +14,7 @@
* limitations under the License.
*/
-#include "metrics_daemon.h"
+#include "metrics_collector.h"
#include <sysexits.h>
#include <time.h>
@@ -33,7 +33,6 @@
#include <dbus/message.h>
#include "constants.h"
-#include "uploader/upload_service.h"
using base::FilePath;
using base::StringPrintf;
@@ -78,9 +77,9 @@
// Zram sysfs entries.
-const char MetricsDaemon::kComprDataSizeName[] = "compr_data_size";
-const char MetricsDaemon::kOrigDataSizeName[] = "orig_data_size";
-const char MetricsDaemon::kZeroPagesName[] = "zero_pages";
+const char MetricsCollector::kComprDataSizeName[] = "compr_data_size";
+const char MetricsCollector::kOrigDataSizeName[] = "orig_data_size";
+const char MetricsCollector::kZeroPagesName[] = "zero_pages";
// Memory use stats collection intervals. We collect some memory use interval
// at these intervals after boot, and we stop collecting after the last one,
@@ -94,15 +93,15 @@
600 * kSecondsPerMinute, // 12.5 hour mark
};
-MetricsDaemon::MetricsDaemon()
+MetricsCollector::MetricsCollector()
: memuse_final_time_(0),
memuse_interval_index_(0) {}
-MetricsDaemon::~MetricsDaemon() {
+MetricsCollector::~MetricsCollector() {
}
// static
-double MetricsDaemon::GetActiveTime() {
+double MetricsCollector::GetActiveTime() {
struct timespec ts;
int r = clock_gettime(CLOCK_MONOTONIC, &ts);
if (r < 0) {
@@ -113,7 +112,7 @@
}
}
-int MetricsDaemon::Run() {
+int MetricsCollector::Run() {
if (CheckSystemCrash(kKernelCrashDetectedFile)) {
ProcessKernelCrash();
}
@@ -134,16 +133,7 @@
return brillo::DBusDaemon::Run();
}
-void MetricsDaemon::RunUploaderTest() {
- upload_service_.reset(new UploadService(
- new SystemProfileCache(true, metrics_directory_),
- metrics_lib_,
- server_));
- upload_service_->Init(upload_interval_, metrics_directory_);
- upload_service_->UploadEvent();
-}
-
-uint32_t MetricsDaemon::GetOsVersionHash() {
+uint32_t MetricsCollector::GetOsVersionHash() {
brillo::OsReleaseReader reader;
reader.Load();
string version;
@@ -159,24 +149,15 @@
return version_hash;
}
-void MetricsDaemon::Init(bool testing,
- bool uploader_active,
- bool dbus_enabled,
+void MetricsCollector::Init(bool testing,
MetricsLibraryInterface* metrics_lib,
const string& diskstats_path,
- const base::TimeDelta& upload_interval,
- const string& server,
const base::FilePath& metrics_directory) {
CHECK(metrics_lib);
testing_ = testing;
- uploader_active_ = uploader_active;
- dbus_enabled_ = dbus_enabled;
metrics_directory_ = metrics_directory;
metrics_lib_ = metrics_lib;
- upload_interval_ = upload_interval;
- server_ = server;
-
daily_active_use_.reset(
new PersistentInteger("Platform.UseTime.PerDay"));
version_cumulative_active_use_.reset(
@@ -221,9 +202,8 @@
cpu_usage_collector_.reset(new CpuUsageCollector(metrics_lib_));
}
-int MetricsDaemon::OnInit() {
- int return_code = dbus_enabled_ ? brillo::DBusDaemon::OnInit() :
- brillo::Daemon::OnInit();
+int MetricsCollector::OnInit() {
+ int return_code = brillo::DBusDaemon::OnInit();
if (return_code != EX_OK)
return return_code;
@@ -237,66 +217,58 @@
if (testing_)
return EX_OK;
- if (dbus_enabled_) {
- bus_->AssertOnDBusThread();
- CHECK(bus_->SetUpAsyncOperations());
+ bus_->AssertOnDBusThread();
+ CHECK(bus_->SetUpAsyncOperations());
- if (bus_->is_connected()) {
- const std::string match_rule =
- base::StringPrintf(kCrashReporterMatchRule,
- kCrashReporterInterface,
- kCrashReporterUserCrashSignal);
-
- bus_->AddFilterFunction(&MetricsDaemon::MessageFilter, this);
-
- DBusError error;
- dbus_error_init(&error);
- bus_->AddMatch(match_rule, &error);
-
- if (dbus_error_is_set(&error)) {
- LOG(ERROR) << "Failed to add match rule \"" << match_rule << "\". Got "
- << error.name << ": " << error.message;
- return EX_SOFTWARE;
- }
- } else {
- LOG(ERROR) << "DBus isn't connected.";
- return EX_UNAVAILABLE;
- }
-
- device_ = weaved::Device::CreateInstance(
- bus_,
- base::Bind(&MetricsDaemon::UpdateWeaveState, base::Unretained(this)));
- device_->AddCommandHandler(
- "_metrics._enableAnalyticsReporting",
- base::Bind(&MetricsDaemon::OnEnableMetrics, base::Unretained(this)));
- device_->AddCommandHandler(
- "_metrics._disableAnalyticsReporting",
- base::Bind(&MetricsDaemon::OnDisableMetrics, base::Unretained(this)));
- }
-
- latest_cpu_use_microseconds_ = cpu_usage_collector_->GetCumulativeCpuUse();
- base::MessageLoop::current()->PostDelayedTask(FROM_HERE,
- base::Bind(&MetricsDaemon::HandleUpdateStatsTimeout,
- base::Unretained(this)),
- base::TimeDelta::FromMilliseconds(kUpdateStatsIntervalMs));
-
- if (uploader_active_) {
- upload_service_.reset(
- new UploadService(new SystemProfileCache(), metrics_lib_, server_));
- upload_service_->Init(upload_interval_, metrics_directory_);
- }
-
- return EX_OK;
-}
-
-void MetricsDaemon::OnShutdown(int* return_code) {
- if (!testing_ && dbus_enabled_ && bus_->is_connected()) {
+ if (bus_->is_connected()) {
const std::string match_rule =
base::StringPrintf(kCrashReporterMatchRule,
kCrashReporterInterface,
kCrashReporterUserCrashSignal);
- bus_->RemoveFilterFunction(&MetricsDaemon::MessageFilter, this);
+ bus_->AddFilterFunction(&MetricsCollector::MessageFilter, this);
+
+ DBusError error;
+ dbus_error_init(&error);
+ bus_->AddMatch(match_rule, &error);
+
+ if (dbus_error_is_set(&error)) {
+ LOG(ERROR) << "Failed to add match rule \"" << match_rule << "\". Got "
+ << error.name << ": " << error.message;
+ return EX_SOFTWARE;
+ }
+ } else {
+ LOG(ERROR) << "DBus isn't connected.";
+ return EX_UNAVAILABLE;
+ }
+
+ device_ = weaved::Device::CreateInstance(
+ bus_,
+ base::Bind(&MetricsCollector::UpdateWeaveState, base::Unretained(this)));
+ device_->AddCommandHandler(
+ "_metrics._enableAnalyticsReporting",
+ base::Bind(&MetricsCollector::OnEnableMetrics, base::Unretained(this)));
+ device_->AddCommandHandler(
+ "_metrics._disableAnalyticsReporting",
+ base::Bind(&MetricsCollector::OnDisableMetrics, base::Unretained(this)));
+
+ latest_cpu_use_microseconds_ = cpu_usage_collector_->GetCumulativeCpuUse();
+ base::MessageLoop::current()->PostDelayedTask(FROM_HERE,
+ base::Bind(&MetricsCollector::HandleUpdateStatsTimeout,
+ base::Unretained(this)),
+ base::TimeDelta::FromMilliseconds(kUpdateStatsIntervalMs));
+
+ return EX_OK;
+}
+
+void MetricsCollector::OnShutdown(int* return_code) {
+ if (!testing_ && bus_->is_connected()) {
+ const std::string match_rule =
+ base::StringPrintf(kCrashReporterMatchRule,
+ kCrashReporterInterface,
+ kCrashReporterUserCrashSignal);
+
+ bus_->RemoveFilterFunction(&MetricsCollector::MessageFilter, this);
DBusError error;
dbus_error_init(&error);
@@ -310,7 +282,8 @@
brillo::DBusDaemon::OnShutdown(return_code);
}
-void MetricsDaemon::OnEnableMetrics(const std::weak_ptr<weaved::Command>& cmd) {
+void MetricsCollector::OnEnableMetrics(
+ const std::weak_ptr<weaved::Command>& cmd) {
auto command = cmd.lock();
if (!command)
return;
@@ -327,7 +300,7 @@
command->Complete({}, nullptr);
}
-void MetricsDaemon::OnDisableMetrics(
+void MetricsCollector::OnDisableMetrics(
const std::weak_ptr<weaved::Command>& cmd) {
auto command = cmd.lock();
if (!command)
@@ -345,7 +318,7 @@
command->Complete({}, nullptr);
}
-void MetricsDaemon::UpdateWeaveState() {
+void MetricsCollector::UpdateWeaveState() {
if (!device_)
return;
@@ -360,9 +333,9 @@
}
// static
-DBusHandlerResult MetricsDaemon::MessageFilter(DBusConnection* connection,
- DBusMessage* message,
- void* user_data) {
+DBusHandlerResult MetricsCollector::MessageFilter(DBusConnection* connection,
+ DBusMessage* message,
+ void* user_data) {
int message_type = dbus_message_get_type(message);
if (message_type != DBUS_MESSAGE_TYPE_SIGNAL) {
DLOG(WARNING) << "unexpected message type " << message_type;
@@ -374,7 +347,7 @@
const std::string member(dbus_message_get_member(message));
DLOG(INFO) << "Got " << interface << "." << member << " D-Bus signal";
- MetricsDaemon* daemon = static_cast<MetricsDaemon*>(user_data);
+ MetricsCollector* daemon = static_cast<MetricsCollector*>(user_data);
DBusMessageIter iter;
dbus_message_iter_init(message, &iter);
@@ -389,7 +362,7 @@
return DBUS_HANDLER_RESULT_HANDLED;
}
-void MetricsDaemon::ProcessUserCrash() {
+void MetricsCollector::ProcessUserCrash() {
// Counts the active time up to now.
UpdateStats(TimeTicks::Now(), Time::Now());
@@ -402,7 +375,7 @@
user_crashes_weekly_count_->Add(1);
}
-void MetricsDaemon::ProcessKernelCrash() {
+void MetricsCollector::ProcessKernelCrash() {
// Counts the active time up to now.
UpdateStats(TimeTicks::Now(), Time::Now());
@@ -417,7 +390,7 @@
kernel_crashes_version_count_->Add(1);
}
-void MetricsDaemon::ProcessUncleanShutdown() {
+void MetricsCollector::ProcessUncleanShutdown() {
// Counts the active time up to now.
UpdateStats(TimeTicks::Now(), Time::Now());
@@ -430,7 +403,7 @@
any_crashes_weekly_count_->Add(1);
}
-bool MetricsDaemon::CheckSystemCrash(const string& crash_file) {
+bool MetricsCollector::CheckSystemCrash(const string& crash_file) {
FilePath crash_detected(crash_file);
if (!base::PathExists(crash_detected))
return false;
@@ -441,7 +414,7 @@
return true;
}
-void MetricsDaemon::StatsReporterInit() {
+void MetricsCollector::StatsReporterInit() {
disk_usage_collector_->Schedule();
cpu_usage_collector_->Init();
@@ -452,18 +425,18 @@
averaged_stats_collector_->ScheduleWait();
}
-void MetricsDaemon::ScheduleMeminfoCallback(int wait) {
+void MetricsCollector::ScheduleMeminfoCallback(int wait) {
if (testing_) {
return;
}
base::TimeDelta waitDelta = base::TimeDelta::FromSeconds(wait);
base::MessageLoop::current()->PostDelayedTask(FROM_HERE,
- base::Bind(&MetricsDaemon::MeminfoCallback, base::Unretained(this),
+ base::Bind(&MetricsCollector::MeminfoCallback, base::Unretained(this),
waitDelta),
waitDelta);
}
-void MetricsDaemon::MeminfoCallback(base::TimeDelta wait) {
+void MetricsCollector::MeminfoCallback(base::TimeDelta wait) {
string meminfo_raw;
const FilePath meminfo_path(kMeminfoFileName);
if (!base::ReadFileToString(meminfo_path, &meminfo_raw)) {
@@ -473,15 +446,15 @@
// Make both calls even if the first one fails.
if (ProcessMeminfo(meminfo_raw)) {
base::MessageLoop::current()->PostDelayedTask(FROM_HERE,
- base::Bind(&MetricsDaemon::MeminfoCallback, base::Unretained(this),
+ base::Bind(&MetricsCollector::MeminfoCallback, base::Unretained(this),
wait),
wait);
}
}
// static
-bool MetricsDaemon::ReadFileToUint64(const base::FilePath& path,
- uint64_t* value) {
+bool MetricsCollector::ReadFileToUint64(const base::FilePath& path,
+ uint64_t* value) {
std::string content;
if (!base::ReadFileToString(path, &content)) {
PLOG(WARNING) << "cannot read " << path.MaybeAsASCII();
@@ -496,7 +469,7 @@
return true;
}
-bool MetricsDaemon::ReportZram(const base::FilePath& zram_dir) {
+bool MetricsCollector::ReportZram(const base::FilePath& zram_dir) {
// Data sizes are in bytes. |zero_pages| is in number of pages.
uint64_t compr_data_size, orig_data_size, zero_pages;
const size_t page_size = 4096;
@@ -533,7 +506,7 @@
return true;
}
-bool MetricsDaemon::ProcessMeminfo(const string& meminfo_raw) {
+bool MetricsCollector::ProcessMeminfo(const string& meminfo_raw) {
static const MeminfoRecord fields_array[] = {
{ "MemTotal", "MemTotal" }, // SPECIAL CASE: total system memory
{ "MemFree", "MemFree" },
@@ -604,8 +577,8 @@
return true;
}
-bool MetricsDaemon::FillMeminfo(const string& meminfo_raw,
- vector<MeminfoRecord>* fields) {
+bool MetricsCollector::FillMeminfo(const string& meminfo_raw,
+ vector<MeminfoRecord>* fields) {
vector<string> lines;
unsigned int nlines = Tokenize(meminfo_raw, "\n", &lines);
@@ -636,16 +609,16 @@
return true;
}
-void MetricsDaemon::ScheduleMemuseCallback(double interval) {
+void MetricsCollector::ScheduleMemuseCallback(double interval) {
if (testing_) {
return;
}
base::MessageLoop::current()->PostDelayedTask(FROM_HERE,
- base::Bind(&MetricsDaemon::MemuseCallback, base::Unretained(this)),
+ base::Bind(&MetricsCollector::MemuseCallback, base::Unretained(this)),
base::TimeDelta::FromSeconds(interval));
}
-void MetricsDaemon::MemuseCallback() {
+void MetricsCollector::MemuseCallback() {
// Since we only care about active time (i.e. uptime minus sleep time) but
// the callbacks are driven by real time (uptime), we check if we should
// reschedule this callback due to intervening sleep periods.
@@ -666,7 +639,7 @@
}
}
-bool MetricsDaemon::MemuseCallbackWork() {
+bool MetricsCollector::MemuseCallbackWork() {
string meminfo_raw;
const FilePath meminfo_path(kMeminfoFileName);
if (!base::ReadFileToString(meminfo_path, &meminfo_raw)) {
@@ -676,7 +649,7 @@
return ProcessMemuse(meminfo_raw);
}
-bool MetricsDaemon::ProcessMemuse(const string& meminfo_raw) {
+bool MetricsCollector::ProcessMemuse(const string& meminfo_raw) {
static const MeminfoRecord fields_array[] = {
{ "MemTotal", "MemTotal" }, // SPECIAL CASE: total system memory
{ "ActiveAnon", "Active(anon)" },
@@ -702,12 +675,12 @@
return true;
}
-void MetricsDaemon::SendSample(const string& name, int sample,
- int min, int max, int nbuckets) {
+void MetricsCollector::SendSample(const string& name, int sample,
+ int min, int max, int nbuckets) {
metrics_lib_->SendToUMA(name, sample, min, max, nbuckets);
}
-void MetricsDaemon::SendKernelCrashesCumulativeCountStats() {
+void MetricsCollector::SendKernelCrashesCumulativeCountStats() {
// Report the number of crashes for this OS version, but don't clear the
// counter. It is cleared elsewhere on version change.
int64_t crashes_count = kernel_crashes_version_count_->Get();
@@ -752,7 +725,7 @@
}
}
-void MetricsDaemon::SendAndResetDailyUseSample(
+void MetricsCollector::SendAndResetDailyUseSample(
const scoped_ptr<PersistentInteger>& use) {
SendSample(use->Name(),
use->GetAndClear(),
@@ -761,7 +734,7 @@
50); // number of buckets
}
-void MetricsDaemon::SendAndResetCrashIntervalSample(
+void MetricsCollector::SendAndResetCrashIntervalSample(
const scoped_ptr<PersistentInteger>& interval) {
SendSample(interval->Name(),
interval->GetAndClear(),
@@ -770,7 +743,7 @@
50); // number of buckets
}
-void MetricsDaemon::SendAndResetCrashFrequencySample(
+void MetricsCollector::SendAndResetCrashFrequencySample(
const scoped_ptr<PersistentInteger>& frequency) {
SendSample(frequency->Name(),
frequency->GetAndClear(),
@@ -779,16 +752,16 @@
50); // number of buckets
}
-void MetricsDaemon::SendLinearSample(const string& name, int sample,
- int max, int nbuckets) {
+void MetricsCollector::SendLinearSample(const string& name, int sample,
+ int max, int nbuckets) {
// TODO(semenzato): add a proper linear histogram to the Chrome external
// metrics API.
LOG_IF(FATAL, nbuckets != max + 1) << "unsupported histogram scale";
metrics_lib_->SendEnumToUMA(name, sample, max);
}
-void MetricsDaemon::UpdateStats(TimeTicks now_ticks,
- Time now_wall_time) {
+void MetricsCollector::UpdateStats(TimeTicks now_ticks,
+ Time now_wall_time) {
const int elapsed_seconds = (now_ticks - last_update_stats_time_).InSeconds();
daily_active_use_->Add(elapsed_seconds);
version_cumulative_active_use_->Add(elapsed_seconds);
@@ -823,10 +796,10 @@
}
}
-void MetricsDaemon::HandleUpdateStatsTimeout() {
+void MetricsCollector::HandleUpdateStatsTimeout() {
UpdateStats(TimeTicks::Now(), Time::Now());
base::MessageLoop::current()->PostDelayedTask(FROM_HERE,
- base::Bind(&MetricsDaemon::HandleUpdateStatsTimeout,
+ base::Bind(&MetricsCollector::HandleUpdateStatsTimeout,
base::Unretained(this)),
base::TimeDelta::FromMilliseconds(kUpdateStatsIntervalMs));
}
diff --git a/metricsd/metrics_daemon.h b/metricsd/metrics_collector.h
similarity index 84%
rename from metricsd/metrics_daemon.h
rename to metricsd/metrics_collector.h
index 54ae188..e080ac0 100644
--- a/metricsd/metrics_daemon.h
+++ b/metricsd/metrics_collector.h
@@ -14,8 +14,8 @@
* limitations under the License.
*/
-#ifndef METRICS_METRICS_DAEMON_H_
-#define METRICS_METRICS_DAEMON_H_
+#ifndef METRICS_METRICS_COLLECTOR_H_
+#define METRICS_METRICS_COLLECTOR_H_
#include <stdint.h>
@@ -36,23 +36,18 @@
#include "collectors/disk_usage_collector.h"
#include "metrics/metrics_library.h"
#include "persistent_integer.h"
-#include "uploader/upload_service.h"
using chromeos_metrics::PersistentInteger;
-class MetricsDaemon : public brillo::DBusDaemon {
+class MetricsCollector : public brillo::DBusDaemon {
public:
- MetricsDaemon();
- ~MetricsDaemon();
+ MetricsCollector();
+ ~MetricsCollector();
// Initializes metrics class variables.
void Init(bool testing,
- bool uploader_active,
- bool dbus_enabled,
MetricsLibraryInterface* metrics_lib,
const std::string& diskstats_path,
- const base::TimeDelta& upload_interval,
- const std::string& server,
const base::FilePath& metrics_directory);
// Initializes DBus and MessageLoop variables before running the MessageLoop.
@@ -64,9 +59,6 @@
// Does all the work.
int Run() override;
- // Triggers an upload event and exit. (Used to test UploadService)
- void RunUploaderTest();
-
// Returns the active time since boot (uptime minus sleep time) in seconds.
static double GetActiveTime();
@@ -77,24 +69,24 @@
static const char kZeroPagesName[];
private:
- friend class MetricsDaemonTest;
- FRIEND_TEST(MetricsDaemonTest, CheckSystemCrash);
- FRIEND_TEST(MetricsDaemonTest, ComputeEpochNoCurrent);
- FRIEND_TEST(MetricsDaemonTest, ComputeEpochNoLast);
- FRIEND_TEST(MetricsDaemonTest, GetHistogramPath);
- FRIEND_TEST(MetricsDaemonTest, IsNewEpoch);
- FRIEND_TEST(MetricsDaemonTest, MessageFilter);
- FRIEND_TEST(MetricsDaemonTest, ProcessKernelCrash);
- FRIEND_TEST(MetricsDaemonTest, ProcessMeminfo);
- FRIEND_TEST(MetricsDaemonTest, ProcessMeminfo2);
- FRIEND_TEST(MetricsDaemonTest, ProcessUncleanShutdown);
- FRIEND_TEST(MetricsDaemonTest, ProcessUserCrash);
- FRIEND_TEST(MetricsDaemonTest, ReportCrashesDailyFrequency);
- FRIEND_TEST(MetricsDaemonTest, ReportKernelCrashInterval);
- FRIEND_TEST(MetricsDaemonTest, ReportUncleanShutdownInterval);
- FRIEND_TEST(MetricsDaemonTest, ReportUserCrashInterval);
- FRIEND_TEST(MetricsDaemonTest, SendSample);
- FRIEND_TEST(MetricsDaemonTest, SendZramMetrics);
+ friend class MetricsCollectorTest;
+ FRIEND_TEST(MetricsCollectorTest, CheckSystemCrash);
+ FRIEND_TEST(MetricsCollectorTest, ComputeEpochNoCurrent);
+ FRIEND_TEST(MetricsCollectorTest, ComputeEpochNoLast);
+ FRIEND_TEST(MetricsCollectorTest, GetHistogramPath);
+ FRIEND_TEST(MetricsCollectorTest, IsNewEpoch);
+ FRIEND_TEST(MetricsCollectorTest, MessageFilter);
+ FRIEND_TEST(MetricsCollectorTest, ProcessKernelCrash);
+ FRIEND_TEST(MetricsCollectorTest, ProcessMeminfo);
+ FRIEND_TEST(MetricsCollectorTest, ProcessMeminfo2);
+ FRIEND_TEST(MetricsCollectorTest, ProcessUncleanShutdown);
+ FRIEND_TEST(MetricsCollectorTest, ProcessUserCrash);
+ FRIEND_TEST(MetricsCollectorTest, ReportCrashesDailyFrequency);
+ FRIEND_TEST(MetricsCollectorTest, ReportKernelCrashInterval);
+ FRIEND_TEST(MetricsCollectorTest, ReportUncleanShutdownInterval);
+ FRIEND_TEST(MetricsCollectorTest, ReportUserCrashInterval);
+ FRIEND_TEST(MetricsCollectorTest, SendSample);
+ FRIEND_TEST(MetricsCollectorTest, SendZramMetrics);
// Type of scale to use for meminfo histograms. For most of them we use
// percent of total RAM, but for some we use absolute numbers, usually in
@@ -233,13 +225,6 @@
// Test mode.
bool testing_;
- // Whether the uploader is enabled or disabled.
- bool uploader_active_;
-
- // Whether or not dbus should be used.
- // If disabled, we will not collect the frequency of crashes.
- bool dbus_enabled_;
-
// Root of the configuration files to use.
base::FilePath metrics_directory_;
@@ -291,11 +276,7 @@
scoped_ptr<DiskUsageCollector> disk_usage_collector_;
scoped_ptr<AveragedStatisticsCollector> averaged_stats_collector_;
- base::TimeDelta upload_interval_;
- std::string server_;
-
- scoped_ptr<UploadService> upload_service_;
std::unique_ptr<weaved::Device> device_;
};
-#endif // METRICS_METRICS_DAEMON_H_
+#endif // METRICS_METRICS_COLLECTOR_H_
diff --git a/metricsd/metrics_collector.rc b/metricsd/metrics_collector.rc
new file mode 100644
index 0000000..2e7e0ae
--- /dev/null
+++ b/metricsd/metrics_collector.rc
@@ -0,0 +1,4 @@
+service metricscollector /system/bin/metrics_collector --foreground --logtosyslog
+ class late_start
+ user system
+ group system dbus
diff --git a/metricsd/metrics_daemon_main.cc b/metricsd/metrics_collector_main.cc
similarity index 72%
rename from metricsd/metrics_daemon_main.cc
rename to metricsd/metrics_collector_main.cc
index 9bb67e8..117426e 100644
--- a/metricsd/metrics_daemon_main.cc
+++ b/metricsd/metrics_collector_main.cc
@@ -23,7 +23,7 @@
#include <rootdev.h>
#include "constants.h"
-#include "metrics_daemon.h"
+#include "metrics_collector.h"
// Returns the path to the disk stats in the sysfs. Returns the null string if
@@ -51,26 +51,6 @@
int main(int argc, char** argv) {
DEFINE_bool(foreground, false, "Don't daemonize");
- // The uploader is disabled by default on ChromeOS as Chrome is responsible
- // for sending the metrics.
- DEFINE_bool(uploader, false, "activate the uploader");
-
- // Upload the metrics once and exit. (used for testing)
- DEFINE_bool(uploader_test,
- false,
- "run the uploader once and exit");
-
- // Enable dbus.
- DEFINE_bool(withdbus, true, "Enable dbus");
-
- // Upload Service flags.
- DEFINE_int32(upload_interval_secs,
- 1800,
- "Interval at which metrics_daemon sends the metrics. (needs "
- "-uploader)");
- DEFINE_string(server,
- metrics::kMetricsServer,
- "Server to upload the metrics to. (needs -uploader)");
DEFINE_string(metrics_directory,
metrics::kMetricsDirectory,
"Root of the configuration files (testing only)");
@@ -102,20 +82,11 @@
MetricsLibrary metrics_lib;
metrics_lib.InitWithNoCaching();
- MetricsDaemon daemon;
- daemon.Init(FLAGS_uploader_test,
- FLAGS_uploader | FLAGS_uploader_test,
- FLAGS_withdbus,
+ MetricsCollector daemon;
+ daemon.Init(false,
&metrics_lib,
MetricsMainDiskStatsPath(),
- base::TimeDelta::FromSeconds(FLAGS_upload_interval_secs),
- FLAGS_server,
base::FilePath(FLAGS_metrics_directory));
- if (FLAGS_uploader_test) {
- daemon.RunUploaderTest();
- return 0;
- }
-
daemon.Run();
}
diff --git a/metricsd/metrics_daemon_test.cc b/metricsd/metrics_collector_test.cc
similarity index 88%
rename from metricsd/metrics_daemon_test.cc
rename to metricsd/metrics_collector_test.cc
index 2e8a85b..a0e7087 100644
--- a/metricsd/metrics_daemon_test.cc
+++ b/metricsd/metrics_collector_test.cc
@@ -24,7 +24,7 @@
#include <gtest/gtest.h>
#include "constants.h"
-#include "metrics_daemon.h"
+#include "metrics_collector.h"
#include "metrics/metrics_library_mock.h"
#include "persistent_integer_mock.h"
@@ -40,7 +40,7 @@
using chromeos_metrics::PersistentIntegerMock;
-class MetricsDaemonTest : public testing::Test {
+class MetricsCollectorTest : public testing::Test {
protected:
virtual void SetUp() {
brillo::FlagHelper::Init(0, nullptr, "");
@@ -48,14 +48,7 @@
chromeos_metrics::PersistentInteger::SetMetricsDirectory(
temp_dir_.path().value());
- daemon_.Init(true,
- false,
- true,
- &metrics_lib_,
- "",
- base::TimeDelta::FromMinutes(30),
- metrics::kMetricsServer,
- temp_dir_.path());
+ daemon_.Init(true, &metrics_lib_, "", temp_dir_.path());
}
// Adds a metrics library mock expectation that the specified metric
@@ -107,8 +100,8 @@
value_string.length()));
}
- // The MetricsDaemon under test.
- MetricsDaemon daemon_;
+ // The MetricsCollector under test.
+ MetricsCollector daemon_;
// Temporary directory used for tests.
base::ScopedTempDir temp_dir_;
@@ -118,13 +111,13 @@
StrictMock<MetricsLibraryMock> metrics_lib_;
};
-TEST_F(MetricsDaemonTest, MessageFilter) {
+TEST_F(MetricsCollectorTest, MessageFilter) {
// Ignore calls to SendToUMA.
EXPECT_CALL(metrics_lib_, SendToUMA(_, _, _, _, _)).Times(AnyNumber());
DBusMessage* msg = dbus_message_new(DBUS_MESSAGE_TYPE_METHOD_CALL);
DBusHandlerResult res =
- MetricsDaemon::MessageFilter(/* connection */ nullptr, msg, &daemon_);
+ MetricsCollector::MessageFilter(/* connection */ nullptr, msg, &daemon_);
EXPECT_EQ(DBUS_HANDLER_RESULT_NOT_YET_HANDLED, res);
DeleteDBusMessage(msg);
@@ -133,7 +126,7 @@
"org.chromium.CrashReporter",
"UserCrash",
signal_args);
- res = MetricsDaemon::MessageFilter(/* connection */ nullptr, msg, &daemon_);
+ res = MetricsCollector::MessageFilter(/* connection */ nullptr, msg, &daemon_);
EXPECT_EQ(DBUS_HANDLER_RESULT_HANDLED, res);
DeleteDBusMessage(msg);
@@ -144,18 +137,18 @@
"org.chromium.UnknownService.Manager",
"StateChanged",
signal_args);
- res = MetricsDaemon::MessageFilter(/* connection */ nullptr, msg, &daemon_);
+ res = MetricsCollector::MessageFilter(/* connection */ nullptr, msg, &daemon_);
EXPECT_EQ(DBUS_HANDLER_RESULT_NOT_YET_HANDLED, res);
DeleteDBusMessage(msg);
}
-TEST_F(MetricsDaemonTest, SendSample) {
+TEST_F(MetricsCollectorTest, SendSample) {
ExpectSample("Dummy.Metric", 3);
daemon_.SendSample("Dummy.Metric", /* sample */ 3,
/* min */ 1, /* max */ 100, /* buckets */ 50);
}
-TEST_F(MetricsDaemonTest, ProcessMeminfo) {
+TEST_F(MetricsCollectorTest, ProcessMeminfo) {
string meminfo =
"MemTotal: 2000000 kB\nMemFree: 500000 kB\n"
"Buffers: 1000000 kB\nCached: 213652 kB\n"
@@ -192,13 +185,13 @@
EXPECT_TRUE(daemon_.ProcessMeminfo(meminfo));
}
-TEST_F(MetricsDaemonTest, ProcessMeminfo2) {
+TEST_F(MetricsCollectorTest, ProcessMeminfo2) {
string meminfo = "MemTotal: 2000000 kB\nMemFree: 1000000 kB\n";
// Not enough fields.
EXPECT_FALSE(daemon_.ProcessMeminfo(meminfo));
}
-TEST_F(MetricsDaemonTest, SendZramMetrics) {
+TEST_F(MetricsCollectorTest, SendZramMetrics) {
EXPECT_TRUE(daemon_.testing_);
// |compr_data_size| is the size in bytes of compressed data.
@@ -210,13 +203,13 @@
const uint64_t zero_pages = 10 * 1000 * 1000 / page_size;
CreateUint64ValueFile(
- temp_dir_.path().Append(MetricsDaemon::kComprDataSizeName),
+ temp_dir_.path().Append(MetricsCollector::kComprDataSizeName),
compr_data_size);
CreateUint64ValueFile(
- temp_dir_.path().Append(MetricsDaemon::kOrigDataSizeName),
+ temp_dir_.path().Append(MetricsCollector::kOrigDataSizeName),
orig_data_size);
CreateUint64ValueFile(
- temp_dir_.path().Append(MetricsDaemon::kZeroPagesName), zero_pages);
+ temp_dir_.path().Append(MetricsCollector::kZeroPagesName), zero_pages);
const uint64_t real_orig_size = orig_data_size + zero_pages * page_size;
const uint64_t zero_ratio_percent =
diff --git a/metricsd/metrics_daemon.rc b/metricsd/metrics_daemon.rc
deleted file mode 100644
index 8b24749..0000000
--- a/metricsd/metrics_daemon.rc
+++ /dev/null
@@ -1,7 +0,0 @@
-on post-fs-data
- mkdir /data/misc/metrics 0770 system system
-
-service metrics_daemon /system/bin/metrics_daemon --uploader --foreground --logtosyslog
- class late_start
- user system
- group system dbus inet
diff --git a/metricsd/metricsd.rc b/metricsd/metricsd.rc
new file mode 100644
index 0000000..b5e7b82
--- /dev/null
+++ b/metricsd/metricsd.rc
@@ -0,0 +1,7 @@
+on post-fs-data
+ mkdir /data/misc/metrics 0770 system system
+
+service metricsd /system/bin/metricsd --foreground --logtosyslog
+ class late_start
+ user system
+ group system dbus inet
diff --git a/metricsd/metricsd_main.cc b/metricsd/metricsd_main.cc
new file mode 100644
index 0000000..ab71e6b
--- /dev/null
+++ b/metricsd/metricsd_main.cc
@@ -0,0 +1,80 @@
+/*
+ * Copyright (C) 2015 The Android Open Source Project
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+#include <base/at_exit.h>
+#include <base/command_line.h>
+#include <base/files/file_path.h>
+#include <base/logging.h>
+#include <base/strings/string_util.h>
+#include <base/time/time.h>
+#include <brillo/flag_helper.h>
+#include <brillo/syslog_logging.h>
+
+#include "constants.h"
+#include "uploader/upload_service.h"
+
+
+int main(int argc, char** argv) {
+ DEFINE_bool(foreground, false, "Don't daemonize");
+
+ // Upload the metrics once and exit. (used for testing)
+ DEFINE_bool(uploader_test,
+ false,
+ "run the uploader once and exit");
+
+ // Upload Service flags.
+ DEFINE_int32(upload_interval_secs,
+ 1800,
+ "Interval at which metrics_daemon sends the metrics. (needs "
+ "-uploader)");
+ DEFINE_string(server,
+ metrics::kMetricsServer,
+ "Server to upload the metrics to. (needs -uploader)");
+ DEFINE_string(metrics_directory,
+ metrics::kMetricsDirectory,
+ "Root of the configuration files (testing only)");
+
+ DEFINE_bool(logtostderr, false, "Log to standard error");
+ DEFINE_bool(logtosyslog, false, "Log to syslog");
+
+ brillo::FlagHelper::Init(argc, argv, "Brillo metrics daemon.");
+
+ int logging_location = (FLAGS_foreground ? brillo::kLogToStderr
+ : brillo::kLogToSyslog);
+ if (FLAGS_logtosyslog)
+ logging_location = brillo::kLogToSyslog;
+
+ if (FLAGS_logtostderr)
+ logging_location = brillo::kLogToStderr;
+
+ // Also log to stderr when not running as daemon.
+ brillo::InitLog(logging_location | brillo::kLogHeader);
+
+ if (FLAGS_logtostderr && FLAGS_logtosyslog) {
+ LOG(ERROR) << "only one of --logtosyslog and --logtostderr can be set";
+ return 1;
+ }
+
+ if (!FLAGS_foreground && daemon(0, 0) != 0) {
+ return errno;
+ }
+
+ UploadService service(FLAGS_server,
+ base::TimeDelta::FromSeconds(FLAGS_upload_interval_secs),
+ base::FilePath(FLAGS_metrics_directory));
+
+ service.Run();
+}
diff --git a/metricsd/persistent_integer.cc b/metricsd/persistent_integer.cc
index e849f00..ddc4b50 100644
--- a/metricsd/persistent_integer.cc
+++ b/metricsd/persistent_integer.cc
@@ -22,7 +22,6 @@
#include <base/posix/eintr_wrapper.h>
#include "constants.h"
-#include "metrics/metrics_library.h"
namespace chromeos_metrics {
diff --git a/metricsd/uploader/system_profile_cache.cc b/metricsd/uploader/system_profile_cache.cc
index 637cf9e..8928a0d 100644
--- a/metricsd/uploader/system_profile_cache.cc
+++ b/metricsd/uploader/system_profile_cache.cc
@@ -84,10 +84,12 @@
auto client = update_engine::UpdateEngineClient::CreateInstance();
if (!client->GetChannel(&channel)) {
LOG(ERROR) << "failed to read the current channel from update engine.";
+ return false;
}
}
- if (!reader.GetString(metrics::kProductId, &profile_.product_id)) {
+ if (!reader.GetString(metrics::kProductId, &profile_.product_id)
+ || profile_.product_id.empty()) {
LOG(ERROR) << "product_id is not set.";
return false;
}
diff --git a/metricsd/uploader/system_profile_cache.h b/metricsd/uploader/system_profile_cache.h
index ae54a2a..0a21ad4 100644
--- a/metricsd/uploader/system_profile_cache.h
+++ b/metricsd/uploader/system_profile_cache.h
@@ -69,6 +69,7 @@
FRIEND_TEST(UploadServiceTest, ReadKeyValueFromFile);
FRIEND_TEST(UploadServiceTest, SessionIdIncrementedAtInitialization);
FRIEND_TEST(UploadServiceTest, ValuesInConfigFileAreSent);
+ FRIEND_TEST(UploadServiceTest, ProductIdMandatory);
// Fetches all informations and populates |profile_|
bool Initialize();
diff --git a/metricsd/uploader/upload_service.cc b/metricsd/uploader/upload_service.cc
index b630cec..ca5024e 100644
--- a/metricsd/uploader/upload_service.cc
+++ b/metricsd/uploader/upload_service.cc
@@ -16,6 +16,8 @@
#include "uploader/upload_service.h"
+#include <sysexits.h>
+
#include <string>
#include <base/bind.h>
@@ -39,38 +41,34 @@
const int UploadService::kMaxFailedUpload = 10;
-UploadService::UploadService(SystemProfileSetter* setter,
- MetricsLibraryInterface* metrics_lib,
- const std::string& server)
- : system_profile_setter_(setter),
- metrics_lib_(metrics_lib),
- histogram_snapshot_manager_(this),
+UploadService::UploadService(const std::string& server,
+ const base::TimeDelta& upload_interval,
+ const base::FilePath& metrics_directory)
+ : histogram_snapshot_manager_(this),
sender_(new HttpSender(server)),
failed_upload_count_(metrics::kFailedUploadCountName),
- testing_(false) {
-}
-
-UploadService::UploadService(SystemProfileSetter* setter,
- MetricsLibraryInterface* metrics_lib,
- const std::string& server,
- bool testing)
- : UploadService(setter, metrics_lib, server) {
- testing_ = testing;
-}
-
-void UploadService::Init(const base::TimeDelta& upload_interval,
- const base::FilePath& metrics_directory) {
- base::StatisticsRecorder::Initialize();
+ upload_interval_(upload_interval) {
metrics_file_ = metrics_directory.Append(metrics::kMetricsEventsFileName);
staged_log_path_ = metrics_directory.Append(metrics::kStagedLogName);
+ consent_file_ = metrics_directory.Append(metrics::kConsentFileName);
+}
- if (!testing_) {
- base::MessageLoop::current()->PostDelayedTask(FROM_HERE,
- base::Bind(&UploadService::UploadEventCallback,
- base::Unretained(this),
- upload_interval),
- upload_interval);
- }
+int UploadService::OnInit() {
+ base::StatisticsRecorder::Initialize();
+
+ system_profile_setter_.reset(new SystemProfileCache());
+
+ base::MessageLoop::current()->PostDelayedTask(FROM_HERE,
+ base::Bind(&UploadService::UploadEventCallback,
+ base::Unretained(this),
+ upload_interval_),
+ upload_interval_);
+ return EX_OK;
+}
+
+void UploadService::InitForTest(SystemProfileSetter* setter) {
+ base::StatisticsRecorder::Initialize();
+ system_profile_setter_.reset(setter);
}
void UploadService::StartNewLog() {
@@ -114,7 +112,7 @@
void UploadService::SendStagedLog() {
// If metrics are not enabled, discard the log and exit.
- if (!metrics_lib_->AreMetricsEnabled()) {
+ if (!AreMetricsEnabled()) {
LOG(INFO) << "Metrics disabled. Don't upload metrics samples.";
base::DeleteFile(staged_log_path_, false);
return;
@@ -263,3 +261,8 @@
failed_upload_count_.Set(0);
}
}
+
+bool UploadService::AreMetricsEnabled() {
+ return base::PathExists(consent_file_);
+}
+
diff --git a/metricsd/uploader/upload_service.h b/metricsd/uploader/upload_service.h
index 77df74b..7faf357 100644
--- a/metricsd/uploader/upload_service.h
+++ b/metricsd/uploader/upload_service.h
@@ -19,11 +19,11 @@
#include <string>
-#include "base/metrics/histogram_base.h"
-#include "base/metrics/histogram_flattener.h"
-#include "base/metrics/histogram_snapshot_manager.h"
+#include <base/metrics/histogram_base.h>
+#include <base/metrics/histogram_flattener.h>
+#include <base/metrics/histogram_snapshot_manager.h>
+#include <brillo/daemons/daemon.h>
-#include "metrics/metrics_library.h"
#include "persistent_integer.h"
#include "uploader/metrics_log.h"
#include "uploader/sender.h"
@@ -67,14 +67,14 @@
// - if the upload fails, we keep the staged log in memory to retry
// uploading later.
//
-class UploadService : public base::HistogramFlattener {
+class UploadService : public base::HistogramFlattener, public brillo::Daemon {
public:
- explicit UploadService(SystemProfileSetter* setter,
- MetricsLibraryInterface* metrics_lib,
- const std::string& server);
+ UploadService(const std::string& server,
+ const base::TimeDelta& upload_interval,
+ const base::FilePath& metrics_directory);
- void Init(const base::TimeDelta& upload_interval,
- const base::FilePath& metrics_directory);
+ // Initializes the upload service.
+ int OnInit();
// Starts a new log. The log needs to be regenerated after each successful
// launch as it is destroyed when staging the log.
@@ -114,11 +114,8 @@
FRIEND_TEST(UploadServiceTest, UnknownCrashIgnored);
FRIEND_TEST(UploadServiceTest, ValuesInConfigFileAreSent);
- // Private constructor for use in unit testing.
- UploadService(SystemProfileSetter* setter,
- MetricsLibraryInterface* metrics_lib,
- const std::string& server,
- bool testing);
+ // Initializes the upload service for testing.
+ void InitForTest(SystemProfileSetter* setter);
// If a staged log fails to upload more than kMaxFailedUpload times, it
// will be discarded.
@@ -136,6 +133,9 @@
// Adds a crash to the current log.
void AddCrash(const std::string& crash_name);
+ // Returns true iff metrics reporting is enabled.
+ bool AreMetricsEnabled();
+
// Aggregates all histogram available in memory and store them in the current
// log.
void GatherHistograms();
@@ -158,12 +158,14 @@
MetricsLog* GetOrCreateCurrentLog();
scoped_ptr<SystemProfileSetter> system_profile_setter_;
- MetricsLibraryInterface* metrics_lib_;
base::HistogramSnapshotManager histogram_snapshot_manager_;
scoped_ptr<Sender> sender_;
chromeos_metrics::PersistentInteger failed_upload_count_;
scoped_ptr<MetricsLog> current_log_;
+
+ base::TimeDelta upload_interval_;
+ base::FilePath consent_file_;
base::FilePath metrics_file_;
base::FilePath staged_log_path_;
diff --git a/metricsd/uploader/upload_service_test.cc b/metricsd/uploader/upload_service_test.cc
index 47e7b91..24e3127 100644
--- a/metricsd/uploader/upload_service_test.cc
+++ b/metricsd/uploader/upload_service_test.cc
@@ -44,11 +44,11 @@
metrics_lib_.InitForTest(dir_.path());
ASSERT_EQ(0, base::WriteFile(
dir_.path().Append(metrics::kConsentFileName), "", 0));
- upload_service_.reset(new UploadService(new MockSystemProfileSetter(),
- &metrics_lib_, "", true));
+ upload_service_.reset(new UploadService("", base::TimeDelta(),
+ dir_.path()));
upload_service_->sender_.reset(new SenderMock);
- upload_service_->Init(base::TimeDelta::FromMinutes(30), dir_.path());
+ upload_service_->InitForTest(new MockSystemProfileSetter);
upload_service_->GatherHistograms();
upload_service_->Reset();
}
@@ -58,7 +58,8 @@
}
void SetTestingProperty(const std::string& name, const std::string& value) {
- base::FilePath filepath = dir_.path().Append("etc/os-release.d").Append(name);
+ base::FilePath filepath =
+ dir_.path().Append("etc/os-release.d").Append(name);
ASSERT_TRUE(base::CreateDirectory(filepath.DirName()));
ASSERT_EQ(
value.size(),
@@ -159,8 +160,7 @@
}
TEST_F(UploadServiceTest, LogEmptyByDefault) {
- UploadService upload_service(new MockSystemProfileSetter(), &metrics_lib_,
- "");
+ UploadService upload_service("", base::TimeDelta(), dir_.path());
// current_log_ should be initialized later as it needs AtExitManager to exit
// in order to gather system information from SysInfo.
@@ -291,3 +291,14 @@
EXPECT_EQ(1, sender->send_call_count());
}
+
+// The product id must be set for metrics to be uploaded.
+// If it is not set, the system profile cache should fail to initialize.
+TEST_F(UploadServiceTest, ProductIdMandatory) {
+ SystemProfileCache cache(true, dir_.path());
+ ASSERT_FALSE(cache.Initialize());
+ SetTestingProperty(metrics::kProductId, "");
+ ASSERT_FALSE(cache.Initialize());
+ SetTestingProperty(metrics::kProductId, "hello");
+ ASSERT_TRUE(cache.Initialize());
+}