Wire up Diagnostics Telemetry Log

This change wires up the TelemetryLog to the SystemDataProvider so that
updates are pushed to the log whenever new information is fetched. This
change also adds the SessionLogController to DiagnosticsManager so that
it is instantiated whenever the app is opened.

Bug: 1128204
Change-Id: I7974bbece2e40da9fe5cef1d6db1a32720276033
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2632871Reviewed-by: Zentaro Kavanagh <zentaro@chromium.org>
Commit-Queue: Bailey Berro <baileyberro@chromium.org>
Cr-Commit-Position: refs/heads/master@{#845337}

chromeos/components/diagnostics_ui/backend/diagnostics_manager.cc

@@ -4,14 +4,16 @@
 
 #include "chromeos/components/diagnostics_ui/backend/diagnostics_manager.h"
 
+#include "chromeos/components/diagnostics_ui/backend/session_log_handler.h"
 #include "chromeos/components/diagnostics_ui/backend/system_data_provider.h"
 #include "chromeos/components/diagnostics_ui/backend/system_routine_controller.h"
 
 namespace chromeos {
 namespace diagnostics {
 
-DiagnosticsManager::DiagnosticsManager()
-    : system_data_provider_(std::make_unique<SystemDataProvider>()),
+DiagnosticsManager::DiagnosticsManager(SessionLogHandler* session_log_handler)
+    : system_data_provider_(std::make_unique<SystemDataProvider>(
+          session_log_handler->GetTelemetryLog())),
       system_routine_controller_(std::make_unique<SystemRoutineController>()) {}
 
 DiagnosticsManager::~DiagnosticsManager() = default;

chromeos/components/diagnostics_ui/backend/diagnostics_manager.h

@@ -10,6 +10,7 @@
 namespace chromeos {
 namespace diagnostics {
 
+class SessionLogHandler;
 class SystemDataProvider;
 class SystemRoutineController;
 
@@ -17,7 +18,7 @@ class SystemRoutineController;
 // used by the Diagnostics SWA.
 class DiagnosticsManager {
  public:
-  DiagnosticsManager();
+  explicit DiagnosticsManager(SessionLogHandler* session_log_handler);
   ~DiagnosticsManager();
 
   DiagnosticsManager(const DiagnosticsManager&) = delete;

chromeos/components/diagnostics_ui/backend/system_data_provider.cc

@@ -16,6 +16,7 @@
 #include "base/timer/timer.h"
 #include "chromeos/components/diagnostics_ui/backend/cros_healthd_helpers.h"
 #include "chromeos/components/diagnostics_ui/backend/power_manager_client_conversions.h"
+#include "chromeos/components/diagnostics_ui/backend/telemetry_log.h"
 #include "chromeos/dbus/power_manager/power_supply_properties.pb.h"
 #include "chromeos/services/cros_healthd/public/cpp/service_connection.h"
 #include "chromeos/services/cros_healthd/public/mojom/cros_healthd_probe.mojom.h"
@@ -210,7 +211,11 @@ void PopulateAverageScaledClockSpeed(const healthd::CpuInfo& cpu_info,
 
 }  // namespace
 
-SystemDataProvider::SystemDataProvider() {
+SystemDataProvider::SystemDataProvider()
+    : SystemDataProvider(/*telemetry_log_ptr=*/nullptr) {}
+
+SystemDataProvider::SystemDataProvider(TelemetryLog* telemetry_log_ptr)
+    : telemetry_log_ptr_(telemetry_log_ptr) {
   battery_charge_status_timer_ = std::make_unique<base::RepeatingTimer>();
   battery_health_timer_ = std::make_unique<base::RepeatingTimer>();
   cpu_usage_timer_ = std::make_unique<base::RepeatingTimer>();
@@ -379,6 +384,10 @@ void SystemDataProvider::OnSystemInfoProbeResponse(
 
   PopulateDeviceCapabilities(*info_ptr, *system_info.get());
 
+  if (IsLoggingEnabled()) {
+    telemetry_log_ptr_->UpdateSystemInfo(system_info.Clone());
+  }
+
   std::move(callback).Run(std::move(system_info));
 }
 
@@ -584,6 +593,10 @@ void SystemDataProvider::NotifyBatteryChargeStatusObservers(
   for (auto& observer : battery_charge_status_observers_) {
     observer->OnBatteryChargeStatusUpdated(battery_charge_status.Clone());
   }
+  if (IsLoggingEnabled()) {
+    telemetry_log_ptr_->UpdateBatteryChargeStatus(
+        battery_charge_status.Clone());
+  }
 }
 
 void SystemDataProvider::NotifyBatteryHealthObservers(
@@ -591,6 +604,9 @@ void SystemDataProvider::NotifyBatteryHealthObservers(
   for (auto& observer : battery_health_observers_) {
     observer->OnBatteryHealthUpdated(battery_health.Clone());
   }
+  if (IsLoggingEnabled()) {
+    telemetry_log_ptr_->UpdateBatteryHealth(battery_health.Clone());
+  }
 }
 
 void SystemDataProvider::NotifyMemoryUsageObservers(
@@ -598,6 +614,9 @@ void SystemDataProvider::NotifyMemoryUsageObservers(
   for (auto& observer : memory_usage_observers_) {
     observer->OnMemoryUsageUpdated(memory_usage.Clone());
   }
+  if (IsLoggingEnabled()) {
+    telemetry_log_ptr_->UpdateMemoryUsage(memory_usage.Clone());
+  }
 }
 
 void SystemDataProvider::NotifyCpuUsageObservers(
@@ -605,6 +624,9 @@ void SystemDataProvider::NotifyCpuUsageObservers(
   for (auto& observer : cpu_usage_observers_) {
     observer->OnCpuUsageUpdated(cpu_usage.Clone());
   }
+  if (IsLoggingEnabled()) {
+    telemetry_log_ptr_->UpdateCpuUsage(cpu_usage.Clone());
+  }
 }
 
 void SystemDataProvider::BindCrosHealthdProbeServiceIfNeccessary() {
@@ -620,5 +642,9 @@ void SystemDataProvider::OnProbeServiceDisconnect() {
   probe_service_.reset();
 }
 
+bool SystemDataProvider::IsLoggingEnabled() const {
+  return telemetry_log_ptr_ != nullptr;
+}
+
 }  // namespace diagnostics
 }  // namespace chromeos

chromeos/components/diagnostics_ui/backend/system_data_provider.h

@@ -24,10 +24,13 @@ class RepeatingTimer;
 namespace chromeos {
 namespace diagnostics {
 
+class TelemetryLog;
+
 class SystemDataProvider : public mojom::SystemDataProvider,
                            public PowerManagerClient::Observer {
  public:
   SystemDataProvider();
+  SystemDataProvider(TelemetryLog* telemetry_log_ptr);
 
   ~SystemDataProvider() override;
 
@@ -109,6 +112,10 @@ class SystemDataProvider : public mojom::SystemDataProvider,
   void ComputeAndPopulateCpuUsage(const cros_healthd::mojom::CpuInfo& cpu_info,
                                   mojom::CpuUsage& out_cpu_usage);
 
+  bool IsLoggingEnabled() const;
+
+  TelemetryLog* telemetry_log_ptr_ = nullptr;  // Not owned.
+
   CpuUsageData previous_cpu_usage_data_;
 
   mojo::Remote<cros_healthd::mojom::CrosHealthdProbeService> probe_service_;

chromeos/components/diagnostics_ui/backend/system_data_provider_unittest.cc

@@ -10,6 +10,8 @@
 #include "base/bind.h"
 #include "base/run_loop.h"
 #include "base/strings/string16.h"
+#include "base/strings/string_number_conversions.h"
+#include "base/strings/string_split.h"
 #include "base/system/sys_info.h"
 #include "base/test/bind.h"
 #include "base/test/task_environment.h"
@@ -17,6 +19,7 @@
 #include "base/timer/mock_timer.h"
 #include "chromeos/components/diagnostics_ui/backend/cpu_usage_data.h"
 #include "chromeos/components/diagnostics_ui/backend/power_manager_client_conversions.h"
+#include "chromeos/components/diagnostics_ui/backend/telemetry_log.h"
 #include "chromeos/dbus/cros_healthd/cros_healthd_client.h"
 #include "chromeos/dbus/cros_healthd/fake_cros_healthd_client.h"
 #include "chromeos/dbus/power/fake_power_manager_client.h"
@@ -938,5 +941,66 @@ TEST_F(SystemDataProviderTest, CpuUsageScaledClock) {
                          /*expected_scaled_speed=*/2000);
 }
 
+TEST_F(SystemDataProviderTest, GetSystemInfoLogs) {
+  TelemetryLog log;
+  system_data_provider_ = std::make_unique<SystemDataProvider>(&log);
+
+  const std::string expected_board_name = "board_name";
+  const std::string expected_marketing_name = "marketing_name";
+  const std::string expected_cpu_model = "cpu_model";
+  const uint32_t expected_total_memory_kib = 1234;
+  const uint16_t expected_cpu_threads_count = 5678;
+  const uint32_t expected_cpu_max_clock_speed_khz = 91011;
+  const bool expected_has_battery = true;
+  const std::string expected_milestone_version = "M99";
+
+  SetCrosHealthdSystemInfoResponse(
+      expected_board_name, expected_marketing_name, expected_cpu_model,
+      expected_total_memory_kib, expected_cpu_threads_count,
+      expected_cpu_max_clock_speed_khz, expected_has_battery,
+      expected_milestone_version);
+
+  base::RunLoop run_loop;
+  system_data_provider_->GetSystemInfo(
+      base::BindLambdaForTesting([&](mojom::SystemInfoPtr ptr) {
+        ASSERT_TRUE(ptr);
+        EXPECT_EQ(expected_board_name, ptr->board_name);
+        EXPECT_EQ(expected_marketing_name, ptr->marketing_name);
+        EXPECT_EQ(expected_cpu_model, ptr->cpu_model_name);
+        EXPECT_EQ(expected_total_memory_kib, ptr->total_memory_kib);
+        EXPECT_EQ(expected_cpu_threads_count, ptr->cpu_threads_count);
+        EXPECT_EQ(expected_cpu_max_clock_speed_khz,
+                  ptr->cpu_max_clock_speed_khz);
+        EXPECT_EQ(expected_milestone_version,
+                  ptr->version_info->milestone_version);
+        EXPECT_EQ(expected_has_battery, ptr->device_capabilities->has_battery);
+
+        run_loop.Quit();
+      }));
+  run_loop.Run();
+
+  // Check the contents of the telemetry log
+  const std::vector<std::string> log_contents = base::SplitString(
+      log.GetContents(), "\n", base::WhitespaceHandling::TRIM_WHITESPACE,
+      base::SplitResult::SPLIT_WANT_NONEMPTY);
+
+  EXPECT_EQ("Board Name: " + expected_board_name, log_contents[1]);
+  EXPECT_EQ("Marketing Name: " + expected_marketing_name, log_contents[2]);
+  EXPECT_EQ("CpuModel Name: " + expected_cpu_model, log_contents[3]);
+  EXPECT_EQ(
+      "Total Memory (kib): " + base::NumberToString(expected_total_memory_kib),
+      log_contents[4]);
+  EXPECT_EQ(
+      "Thread Count:  " + base::NumberToString(expected_cpu_threads_count),
+      log_contents[5]);
+  EXPECT_EQ("Cpu Max Clock Speed (kHz):  " +
+                base::NumberToString(expected_cpu_max_clock_speed_khz),
+            log_contents[6]);
+  EXPECT_EQ("Milestone Version: " + expected_milestone_version,
+            log_contents[7]);
+  EXPECT_EQ("Has Battery: " + base::NumberToString(expected_has_battery),
+            log_contents[8]);
+}
+
 }  // namespace diagnostics
 }  // namespace chromeos

chromeos/components/diagnostics_ui/diagnostics_ui.cc

@@ -9,6 +9,7 @@
 #include "base/containers/span.h"
 #include "base/memory/ptr_util.h"
 #include "chromeos/components/diagnostics_ui/backend/diagnostics_manager.h"
+#include "chromeos/components/diagnostics_ui/backend/session_log_handler.h"
 #include "chromeos/components/diagnostics_ui/backend/system_data_provider.h"
 #include "chromeos/components/diagnostics_ui/backend/system_routine_controller.h"
 #include "chromeos/components/diagnostics_ui/mojom/system_data_provider.mojom.h"
@@ -126,8 +127,10 @@ void SetUpWebUIDataSource(content::WebUIDataSource* source,
 
 DiagnosticsUI::DiagnosticsUI(content::WebUI* web_ui)
     : ui::MojoWebUIController(web_ui),
-      diagnostics_manager_(
-          std::make_unique<diagnostics::DiagnosticsManager>()) {
+      session_log_handler_(std::make_unique<diagnostics::SessionLogHandler>()) {
+  diagnostics_manager_ = std::make_unique<diagnostics::DiagnosticsManager>(
+      session_log_handler_.get());
+
   auto html_source = base::WrapUnique(
       content::WebUIDataSource::Create(kChromeUIDiagnosticsAppHost));
   html_source->OverrideContentSecurityPolicy(

chromeos/components/diagnostics_ui/diagnostics_ui.h

@@ -19,6 +19,7 @@ namespace chromeos {
 namespace diagnostics {
 
 class DiagnosticsManager;
+class SessionLogHandler;
 
 }  // namespace diagnostics
 
@@ -41,6 +42,7 @@ class DiagnosticsUI : public ui::MojoWebUIController {
  private:
   WEB_UI_CONTROLLER_TYPE_DECL();
 
+  std::unique_ptr<diagnostics::SessionLogHandler> session_log_handler_;
   std::unique_ptr<diagnostics::DiagnosticsManager> diagnostics_manager_;
 };