Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit e86591e5 authored by Steve Fung's avatar Steve Fung Committed by chrome-internal-fetch
Browse files

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: default avatarAlex Vakulenko <avakulenko@chromium.org>
Commit-Queue: Steve Fung <stevefung@chromium.org>
Tested-by: default avatarSteve Fung <stevefung@chromium.org>
parent 73b40b46

metrics/metrics_daemon.cc

+95 −75
Original line number Diff line number Diff line
@@ -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 @@ namespace { 


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 kDaysPerWeek = 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 @@ static const int kMemuseIntervals[] = { 
};



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 @@ MetricsDaemon::MetricsDaemon() 
      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 @@ double MetricsDaemon::GetActiveTime() { 
  }

}



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 @@ void MetricsDaemon::Run(bool run_as_daemon) { 
    version_cumulative_cpu_use_->Set(0);

  }



  Loop();

  return chromeos::DBusDaemon::Run();

}



void MetricsDaemon::RunUploaderTest() {
@@ -223,6 +221,7 @@ void MetricsDaemon::Init(bool testing, 
                         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 @@ void MetricsDaemon::Init(bool testing, 
  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 @@ void MetricsDaemon::Init(bool testing, 
  memuse_final_time_ = GetActiveTime() + kMemuseIntervals[0];

  ScheduleMemuseCallback(kMemuseIntervals[0]);



  // Don't setup D-Bus and GLib in test mode.

  if (testing)

    return;



  g_type_init();

  dbus_threads_init_default();

  if (testing_)

    return EX_OK;



  DBusError error;

  dbus_error_init(&error);

  bus_->AssertOnDBusThread();

  CHECK(bus_->SetUpAsyncOperations());



  DBusConnection* connection = dbus_bus_get(DBUS_BUS_SYSTEM, &error);

  LOG_IF(FATAL, dbus_error_is_set(&error)) <<

      "No D-Bus connection: " << SAFE_MESSAGE(error);

  if (bus_->is_connected()) {

    const std::string match_rule =

        base::StringPrintf(kCrashReporterMatchRule,

                           kCrashReporterInterface,

                           kCrashReporterUserCrashSignal);



  dbus_connection_setup_with_g_main(connection, nullptr);

    bus_->AddFilterFunction(&MetricsDaemon::MessageFilter, this);



  vector<string> matches;

  matches.push_back(

      base::StringPrintf("type='signal',interface='%s',path='/',member='%s'",

                         kCrashReporterInterface,

                         kCrashReporterUserCrashSignal));

    DBusError error;

    dbus_error_init(&error);

    bus_->AddMatch(match_rule, &error);



  // 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));



  update_stats_timeout_id_ =

      g_timeout_add(kUpdateStatsIntervalMs, &HandleUpdateStatsTimeout, this);

  base::MessageLoop::current()->PostDelayedTask(FROM_HERE,

      base::Bind(&MetricsDaemon::HandleUpdateStatsTimeout,

                 base::Unretained(this)),

      base::TimeDelta::FromMilliseconds(kUpdateStatsIntervalMs));



  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 @@ void MetricsDaemon::ScheduleStatsCallback(int wait) { 
  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 @@ void MetricsDaemon::SendCpuThrottleMetrics() { 
  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 @@ void MetricsDaemon::ScheduleMeminfoCallback(int wait) { 
  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 @@ void MetricsDaemon::ScheduleMemuseCallback(double interval) { 
  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 @@ void MetricsDaemon::UpdateStats(TimeTicks now_ticks, 
  }

}



// 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));

}

metrics/metrics_daemon.h

+17 −34
Original line number Diff line number Diff line
@@ -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 @@ class MetricsDaemon { 
            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 @@ class MetricsDaemon { 
  // 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 @@ class MetricsDaemon { 
  // 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 @@ class MetricsDaemon { 
  // 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 @@ class MetricsDaemon { 
  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 @@ class MetricsDaemon { 
  // 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 @@ class MetricsDaemon { 
  // 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_;

metrics/metrics_daemon_main.cc

+6 −3
Original line number Diff line number Diff line
@@ -73,6 +73,11 @@ int main(int argc, char** argv) { 
  // 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 @@ int main(int argc, char** argv) { 
              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();

}

metrics/metrics_daemon_test.cc

+2 −4
Original line number Diff line number Diff line
@@ -29,6 +29,7 @@ using base::TimeTicks; 
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, ReportDailyUse) { 


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, SendSample) { 


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 @@ TEST_F(MetricsDaemonTest, SendZramMetrics) { 


int main(int argc, char** argv) {

  testing::InitGoogleTest(&argc, argv);

  // Some libchrome calls need this.

  base::AtExitManager at_exit_manager;



  return RUN_ALL_TESTS();

}