metrics: Convert Metrics to DBusDaemon
In order to remove glib, convert Metrics from the glib message loop to
DBusDaemon, which is based on base::MessageLoop. Also use the DBusDaemon's
dbus::Bus object directly to set up the CrashReporter signal handling, as
the ObjectProxy used by chromeos-dbus-bindings requires all signals be sent
from registered name owners.
BUG=chromium:431838
TEST=Unittests and hwtests pass.
TEST=`test_that -b panther <IP> platform_MetricsUploader` passes
CQ-DEPEND=CL:236652
Change-Id: I6bc1f66999a43065b0d48325b031cd36bb782b76
Reviewed-on: https://chromium-review.googlesource.com/234359
Reviewed-by: Alex Vakulenko <avakulenko@chromium.org>
Commit-Queue: Steve Fung <stevefung@chromium.org>
Tested-by: Steve Fung <stevefung@chromium.org>
diff --git a/metrics/metrics_daemon.cc b/metrics/metrics_daemon.cc
index 7226594..acd96c0 100644
--- a/metrics/metrics_daemon.cc
+++ b/metrics/metrics_daemon.cc
@@ -12,6 +12,7 @@
#include <inttypes.h>
#include <math.h>
#include <string.h>
+#include <sysexits.h>
#include <time.h>
#include <base/files/file_path.h>
@@ -24,7 +25,8 @@
#include <base/strings/stringprintf.h>
#include <base/sys_info.h>
#include <chromeos/dbus/service_constants.h>
-#include <dbus/dbus-glib-lowlevel.h>
+#include <dbus/dbus.h>
+#include <dbus/message.h>
#include "uploader/upload_service.h"
using base::FilePath;
@@ -43,6 +45,8 @@
const char kCrashReporterInterface[] = "org.chromium.CrashReporter";
const char kCrashReporterUserCrashSignal[] = "UserCrash";
+const char kCrashReporterMatchRule[] =
+ "type='signal',interface='%s',path='/',member='%s'";
const int kSecondsPerMinute = 60;
const int kMinutesPerHour = 60;
@@ -53,7 +57,7 @@
const int kSecondsPerWeek = kSecondsPerDay * kDaysPerWeek;
// Interval between calls to UpdateStats().
-const guint kUpdateStatsIntervalMs = 300000;
+const uint32_t kUpdateStatsIntervalMs = 300000;
const char kKernelCrashDetectedFile[] = "/var/run/kernel-crash-detected";
const char kUncleanShutdownDetectedFile[] =
@@ -135,8 +139,7 @@
};
MetricsDaemon::MetricsDaemon()
- : update_stats_timeout_id_(-1),
- memuse_final_time_(0),
+ : memuse_final_time_(0),
memuse_interval_index_(0),
read_sectors_(0),
write_sectors_(0),
@@ -147,8 +150,6 @@
latest_cpu_use_ticks_(0) {}
MetricsDaemon::~MetricsDaemon() {
- if (update_stats_timeout_id_ > -1)
- g_source_remove(update_stats_timeout_id_);
}
double MetricsDaemon::GetActiveTime() {
@@ -162,10 +163,7 @@
}
}
-void MetricsDaemon::Run(bool run_as_daemon) {
- if (run_as_daemon && daemon(0, 0) != 0)
- return;
-
+int MetricsDaemon::Run() {
if (CheckSystemCrash(kKernelCrashDetectedFile)) {
ProcessKernelCrash();
}
@@ -183,7 +181,7 @@
version_cumulative_cpu_use_->Set(0);
}
- Loop();
+ return chromeos::DBusDaemon::Run();
}
void MetricsDaemon::RunUploaderTest() {
@@ -223,6 +221,7 @@
const string& metrics_file,
const string& config_root) {
testing_ = testing;
+ uploader_active_ = uploader_active;
config_root_ = config_root;
DCHECK(metrics_lib != nullptr);
metrics_lib_ = metrics_lib;
@@ -276,6 +275,17 @@
vmstats_path_ = vmstats_path;
scaling_max_freq_path_ = scaling_max_freq_path;
cpuinfo_max_freq_path_ = cpuinfo_max_freq_path;
+
+ // If testing, initialize Stats Reporter without connecting DBus
+ if (testing_)
+ StatsReporterInit();
+}
+
+int MetricsDaemon::OnInit() {
+ int return_code = chromeos::DBusDaemon::OnInit();
+ if (return_code != EX_OK)
+ return return_code;
+
StatsReporterInit();
// Start collecting meminfo stats.
@@ -283,56 +293,67 @@
memuse_final_time_ = GetActiveTime() + kMemuseIntervals[0];
ScheduleMemuseCallback(kMemuseIntervals[0]);
- // Don't setup D-Bus and GLib in test mode.
- if (testing)
- return;
+ if (testing_)
+ return EX_OK;
- g_type_init();
- dbus_threads_init_default();
+ bus_->AssertOnDBusThread();
+ CHECK(bus_->SetUpAsyncOperations());
- DBusError error;
- dbus_error_init(&error);
+ if (bus_->is_connected()) {
+ const std::string match_rule =
+ base::StringPrintf(kCrashReporterMatchRule,
+ kCrashReporterInterface,
+ kCrashReporterUserCrashSignal);
- DBusConnection* connection = dbus_bus_get(DBUS_BUS_SYSTEM, &error);
- LOG_IF(FATAL, dbus_error_is_set(&error)) <<
- "No D-Bus connection: " << SAFE_MESSAGE(error);
+ bus_->AddFilterFunction(&MetricsDaemon::MessageFilter, this);
- dbus_connection_setup_with_g_main(connection, nullptr);
+ DBusError error;
+ dbus_error_init(&error);
+ bus_->AddMatch(match_rule, &error);
- vector<string> matches;
- matches.push_back(
- base::StringPrintf("type='signal',interface='%s',path='/',member='%s'",
- kCrashReporterInterface,
- kCrashReporterUserCrashSignal));
-
- // Registers D-Bus matches for the signals we would like to catch.
- for (vector<string>::const_iterator it = matches.begin();
- it != matches.end(); ++it) {
- const char* match = it->c_str();
- DLOG(INFO) << "adding dbus match: " << match;
- dbus_bus_add_match(connection, match, &error);
- LOG_IF(FATAL, dbus_error_is_set(&error)) <<
- "unable to add a match: " << SAFE_MESSAGE(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;
}
- // Adds the D-Bus filter routine to be called back whenever one of
- // the registered D-Bus matches is successful. The daemon is not
- // activated for D-Bus messages that don't match.
- CHECK(dbus_connection_add_filter(connection, MessageFilter, this, nullptr));
+ base::MessageLoop::current()->PostDelayedTask(FROM_HERE,
+ base::Bind(&MetricsDaemon::HandleUpdateStatsTimeout,
+ base::Unretained(this)),
+ base::TimeDelta::FromMilliseconds(kUpdateStatsIntervalMs));
- update_stats_timeout_id_ =
- g_timeout_add(kUpdateStatsIntervalMs, &HandleUpdateStatsTimeout, this);
-
- if (uploader_active) {
+ if (uploader_active_) {
LOG(INFO) << "uploader enabled";
upload_service_.reset(new UploadService(new SystemProfileCache(), server_));
upload_service_->Init(upload_interval_, metrics_file_);
}
+
+ return EX_OK;
}
-void MetricsDaemon::Loop() {
- GMainLoop* loop = g_main_loop_new(nullptr, false);
- g_main_loop_run(loop);
+void MetricsDaemon::OnShutdown(int* return_code) {
+ if (!testing_ && bus_->is_connected()) {
+ const std::string match_rule =
+ base::StringPrintf(kCrashReporterMatchRule,
+ kCrashReporterInterface,
+ kCrashReporterUserCrashSignal);
+
+ bus_->RemoveFilterFunction(&MetricsDaemon::MessageFilter, this);
+
+ DBusError error;
+ dbus_error_init(&error);
+ bus_->RemoveMatch(match_rule, &error);
+
+ if (dbus_error_is_set(&error)) {
+ LOG(ERROR) << "Failed to remove match rule \"" << match_rule << "\". Got "
+ << error.name << ": " << error.message;
+ }
+ }
+ chromeos::DBusDaemon::OnShutdown(return_code);
}
// static
@@ -481,7 +502,9 @@
if (testing_) {
return;
}
- g_timeout_add_seconds(wait, StatsCallbackStatic, this);
+ base::MessageLoop::current()->PostDelayedTask(FROM_HERE,
+ base::Bind(&MetricsDaemon::StatsCallback, base::Unretained(this)),
+ base::TimeDelta::FromSeconds(wait));
}
bool MetricsDaemon::DiskStatsReadStats(uint64_t* read_sectors,
@@ -637,12 +660,6 @@
SendLinearSample(kMetricScaledCpuFrequencyName, percent, 101, 102);
}
-// static
-gboolean MetricsDaemon::StatsCallbackStatic(void* handle) {
- (static_cast<MetricsDaemon*>(handle))->StatsCallback();
- return false; // one-time callback
-}
-
// Collects disk and vm stats alternating over a short and a long interval.
void MetricsDaemon::StatsCallback() {
@@ -756,25 +773,31 @@
if (testing_) {
return;
}
- g_timeout_add_seconds(wait, MeminfoCallbackStatic, this);
+ base::TimeDelta waitDelta = base::TimeDelta::FromSeconds(wait);
+ base::MessageLoop::current()->PostDelayedTask(FROM_HERE,
+ base::Bind(&MetricsDaemon::MeminfoCallback, base::Unretained(this),
+ base::TimeDelta::FromMilliseconds(kMetricMeminfoInterval)),
+ waitDelta);
}
-// static
-gboolean MetricsDaemon::MeminfoCallbackStatic(void* handle) {
- return (static_cast<MetricsDaemon*>(handle))->MeminfoCallback();
-}
-
-bool MetricsDaemon::MeminfoCallback() {
+void MetricsDaemon::MeminfoCallback(base::TimeDelta wait) {
string meminfo_raw;
const FilePath meminfo_path("/proc/meminfo");
if (!base::ReadFileToString(meminfo_path, &meminfo_raw)) {
LOG(WARNING) << "cannot read " << meminfo_path.value().c_str();
- return false;
+ return;
}
// Make both calls even if the first one fails.
bool success = ProcessMeminfo(meminfo_raw);
- return ReportZram(base::FilePath(FILE_PATH_LITERAL("/sys/block/zram0"))) &&
+ bool reschedule =
+ ReportZram(base::FilePath(FILE_PATH_LITERAL("/sys/block/zram0"))) &&
success;
+ if (reschedule) {
+ base::MessageLoop::current()->PostDelayedTask(FROM_HERE,
+ base::Bind(&MetricsDaemon::MeminfoCallback, base::Unretained(this),
+ base::TimeDelta::FromMilliseconds(kMetricMeminfoInterval)),
+ wait);
+ }
}
// static
@@ -941,14 +964,9 @@
if (testing_) {
return;
}
- g_timeout_add_seconds(interval, MemuseCallbackStatic, this);
-}
-
-// static
-gboolean MetricsDaemon::MemuseCallbackStatic(void* handle) {
- MetricsDaemon* daemon = static_cast<MetricsDaemon*>(handle);
- daemon->MemuseCallback();
- return false;
+ base::MessageLoop::current()->PostDelayedTask(FROM_HERE,
+ base::Bind(&MetricsDaemon::MemuseCallback, base::Unretained(this)),
+ base::TimeDelta::FromSeconds(interval));
}
void MetricsDaemon::MemuseCallback() {
@@ -1139,8 +1157,10 @@
}
}
-// static
-gboolean MetricsDaemon::HandleUpdateStatsTimeout(gpointer data) {
- static_cast<MetricsDaemon*>(data)->UpdateStats(TimeTicks::Now(), Time::Now());
- return TRUE;
+void MetricsDaemon::HandleUpdateStatsTimeout() {
+ UpdateStats(TimeTicks::Now(), Time::Now());
+ base::MessageLoop::current()->PostDelayedTask(FROM_HERE,
+ base::Bind(&MetricsDaemon::HandleUpdateStatsTimeout,
+ base::Unretained(this)),
+ base::TimeDelta::FromMilliseconds(kUpdateStatsIntervalMs));
}
diff --git a/metrics/metrics_daemon.h b/metrics/metrics_daemon.h
index f4f1430..397fd21 100644
--- a/metrics/metrics_daemon.h
+++ b/metrics/metrics_daemon.h
@@ -7,8 +7,6 @@
#include <stdint.h>
-#include <dbus/dbus.h>
-#include <glib.h>
#include <map>
#include <string>
#include <vector>
@@ -16,6 +14,7 @@
#include <base/files/file_path.h>
#include <base/memory/scoped_ptr.h>
#include <base/time/time.h>
+#include <chromeos/daemons/dbus_daemon.h>
#include <gtest/gtest_prod.h> // for FRIEND_TEST
#include "metrics/metrics_library.h"
@@ -24,12 +23,12 @@
using chromeos_metrics::PersistentInteger;
-class MetricsDaemon {
+class MetricsDaemon : public chromeos::DBusDaemon {
public:
MetricsDaemon();
~MetricsDaemon();
- // Initializes.
+ // Initializes metrics class variables.
void Init(bool testing,
bool uploader_active,
MetricsLibraryInterface* metrics_lib,
@@ -42,9 +41,14 @@
const std::string& metrics_file,
const std::string& config_root);
- // Does all the work. If |run_as_daemon| is true, daemonizes by
- // forking.
- void Run(bool run_as_daemon);
+ // Initializes DBus and MessageLoop variables before running the MessageLoop.
+ int OnInit() override;
+
+ // Clean up data set up in OnInit before shutting down message loop.
+ void OnShutdown(int* return_code) override;
+
+ // Does all the work.
+ int Run() override;
// Triggers an upload event and exit. (Used to test UploadService)
void RunUploaderTest();
@@ -147,9 +151,6 @@
// Returns the active time since boot (uptime minus sleep time) in seconds.
double GetActiveTime();
- // Creates the event loop and enters it.
- void Loop();
-
// D-Bus filter callback.
static DBusHandlerResult MessageFilter(DBusConnection* connection,
DBusMessage* message,
@@ -227,22 +228,14 @@
// Parse cumulative vm statistics from a C string. Returns true for success.
bool VmStatsParseStats(const char* stats, struct VmstatRecord* record);
- // Reports disk and vm statistics (static version for glib). Arguments are a
- // glib artifact.
- static gboolean StatsCallbackStatic(void* handle);
-
// Reports disk and vm statistics.
void StatsCallback();
// Schedules meminfo collection callback.
void ScheduleMeminfoCallback(int wait);
- // Reports memory statistics (static version for glib). Argument is a glib
- // artifact.
- static gboolean MeminfoCallbackStatic(void* handle);
-
- // Reports memory statistics. Returns false on failure.
- bool MeminfoCallback();
+ // Reports memory statistics. Reschedules callback on success.
+ void MeminfoCallback(base::TimeDelta wait);
// Parses content of /proc/meminfo and sends fields of interest to UMA.
// Returns false on errors. |meminfo_raw| contains the content of
@@ -259,9 +252,6 @@
// Schedule a memory use callback in |interval| seconds.
void ScheduleMemuseCallback(double interval);
- // Static wrapper for MemuseCallback. Always returns false.
- static gboolean MemuseCallbackStatic(void* handle);
-
// Calls MemuseCallbackWork, and possibly schedules next callback, if enough
// active time has passed. Otherwise reschedules itself to simulate active
// time callbacks (i.e. wall clock time minus sleep time).
@@ -288,7 +278,7 @@
void UpdateStats(base::TimeTicks now_ticks, base::Time now_wall_time);
// Invoked periodically by |update_stats_timeout_id_| to call UpdateStats().
- static gboolean HandleUpdateStatsTimeout(gpointer data);
+ void HandleUpdateStatsTimeout();
// Reports zram statistics.
bool ReportZram(const base::FilePath& zram_dir);
@@ -301,6 +291,9 @@
// Test mode.
bool testing_;
+ // Whether the uploader is enabled or disabled.
+ bool uploader_active_;
+
// Root of the configuration files to use.
std::string config_root_;
@@ -315,16 +308,6 @@
// The last time that UpdateStats() was called.
base::TimeTicks last_update_stats_time_;
- // ID of a GLib timeout that repeatedly runs UpdateStats().
- gint update_stats_timeout_id_;
-
- // Sleep period until the next daily usage aggregation performed by
- // the daily use monitor (see ScheduleUseMonitor).
- int usemon_interval_;
-
- // Scheduled daily use monitor source (see ScheduleUseMonitor).
- GSource* usemon_source_;
-
// End time of current memuse stat collection interval.
double memuse_final_time_;
diff --git a/metrics/metrics_daemon_main.cc b/metrics/metrics_daemon_main.cc
index 9322771..cc0812d 100644
--- a/metrics/metrics_daemon_main.cc
+++ b/metrics/metrics_daemon_main.cc
@@ -73,6 +73,11 @@
// Also log to stderr when not running as daemon.
chromeos::InitLog(chromeos::kLogToSyslog | chromeos::kLogHeader |
(FLAGS_daemon ? 0 : chromeos::kLogToStderr));
+
+ if (FLAGS_daemon && daemon(0, 0) != 0) {
+ return errno;
+ }
+
MetricsLibrary metrics_lib;
metrics_lib.Init();
MetricsDaemon daemon;
@@ -88,12 +93,10 @@
FLAGS_metrics_file,
FLAGS_config_root);
-
- base::AtExitManager at_exit_manager;
if (FLAGS_uploader_test) {
daemon.RunUploaderTest();
return 0;
}
- daemon.Run(FLAGS_daemon);
+ daemon.Run();
}
diff --git a/metrics/metrics_daemon_test.cc b/metrics/metrics_daemon_test.cc
index ed6856a..abf8e41 100644
--- a/metrics/metrics_daemon_test.cc
+++ b/metrics/metrics_daemon_test.cc
@@ -29,6 +29,7 @@
using std::string;
using std::vector;
using ::testing::_;
+using ::testing::AnyNumber;
using ::testing::AtLeast;
using ::testing::Return;
using ::testing::StrictMock;
@@ -227,7 +228,7 @@
TEST_F(MetricsDaemonTest, MessageFilter) {
// Ignore calls to SendToUMA.
- EXPECT_CALL(metrics_lib_, SendToUMA(_, _, _, _, _)).Times(AtLeast(0));
+ EXPECT_CALL(metrics_lib_, SendToUMA(_, _, _, _, _)).Times(AnyNumber());
DBusMessage* msg = dbus_message_new(DBUS_MESSAGE_TYPE_METHOD_CALL);
DBusHandlerResult res =
@@ -265,7 +266,6 @@
TEST_F(MetricsDaemonTest, ReportDiskStats) {
uint64_t read_sectors_now, write_sectors_now;
-
CreateFakeDiskStatsFile(kFakeDiskStats1.c_str());
daemon_.DiskStatsReadStats(&read_sectors_now, &write_sectors_now);
EXPECT_EQ(read_sectors_now, kFakeReadSectors[1]);
@@ -399,8 +399,6 @@
int main(int argc, char** argv) {
testing::InitGoogleTest(&argc, argv);
- // Some libchrome calls need this.
- base::AtExitManager at_exit_manager;
return RUN_ALL_TESTS();
}