Commit cf7c7d76 authored by Paul Moy's avatar Paul Moy Committed by Commit Bot

DeviceStatusCollector: start migration to cros_healthd

Switch the collection of cached_vpd and storage information
from runtime_probe to cros_healthd.

to a sona device with some code to log what StatusUploader is
uploading. I then checked that data was consistent with the data
reported by the command-line invocation of cros_healthd.

Bug: b:128683357
Test: manual and unit tests. Manually, I built and deployed Chrome
Change-Id: Id680449eed59cba7ed92bc4b4934e466d457ff79
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1809612Reviewed-by: default avatarPavol Marko <pmarko@chromium.org>
Reviewed-by: default avatarSteven Bennetts <stevenjb@chromium.org>
Reviewed-by: default avatarMaksim Ivanov <emaxx@chromium.org>
Commit-Queue: Paul Moy <pmoy@chromium.org>
Cr-Commit-Position: refs/heads/master@{#700324}
parent fd8fe93c
......@@ -433,6 +433,7 @@ void DeviceCloudPolicyManagerChromeOS::CreateStatusUploader() {
DeviceStatusCollector::TpmStatusFetcher(),
DeviceStatusCollector::EMMCLifetimeFetcher(),
DeviceStatusCollector::StatefulPartitionInfoFetcher(),
DeviceStatusCollector::CrosHealthdDataFetcher(),
true /* is_enterprise_device */),
task_runner_, kDeviceStatusUploadFrequency));
}
......
......@@ -70,6 +70,8 @@
#include "chromeos/network/network_handler.h"
#include "chromeos/network/network_state.h"
#include "chromeos/network/network_state_handler.h"
#include "chromeos/services/cros_healthd/public/cpp/service_connection.h"
#include "chromeos/services/cros_healthd/public/mojom/cros_healthd_probe.mojom.h"
#include "chromeos/settings/cros_settings_names.h"
#include "chromeos/settings/timezone_settings.h"
#include "chromeos/system/statistics_provider.h"
......@@ -563,6 +565,13 @@ class DeviceStatusCollectorState : public StatusCollectorState {
base::BindOnce(&DeviceStatusCollectorState::OnTpmStatusReceived, this));
}
void FetchCrosHealthdData(
const policy::DeviceStatusCollector::CrosHealthdDataFetcher&
cros_healthd_data_fetcher) {
cros_healthd_data_fetcher.Run(base::BindOnce(
&DeviceStatusCollectorState::OnCrosHealthdDataReceived, this));
}
void FetchProbeData(const policy::DeviceStatusCollector::ProbeDataFetcher&
probe_data_fetcher) {
probe_data_fetcher.Run(
......@@ -652,6 +661,37 @@ class DeviceStatusCollectorState : public StatusCollectorState {
tpm_status_struct.boot_lockbox_finalized);
}
// Stores the contents of |probe_result| to |response_params_|.
void OnCrosHealthdDataReceived(
chromeos::cros_healthd::mojom::TelemetryInfoPtr probe_result) {
// Make sure we edit the state on the right thread.
DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
if (probe_result.is_null())
return;
const auto& block_device_info = probe_result->block_device_info;
if (block_device_info) {
em::StorageStatus* const storage_status_out =
response_params_.device_status->mutable_storage_status();
for (const auto& storage : block_device_info.value()) {
em::DiskInfo* const disk_info_out = storage_status_out->add_disks();
disk_info_out->set_serial(base::NumberToString(storage->serial));
disk_info_out->set_manufacturer(
base::NumberToString(storage->manufacturer_id));
disk_info_out->set_model(storage->name);
disk_info_out->set_type(storage->type);
disk_info_out->set_size(storage->size);
}
}
const auto& vpd_info = probe_result->vpd_info;
if (!vpd_info.is_null()) {
em::SystemStatus* const system_status_out =
response_params_.device_status->mutable_system_status();
system_status_out->set_vpd_sku_number(vpd_info->sku_number);
}
}
// Note that we use proto3 syntax for ProbeResult, so missing fields will
// have default values.
void OnProbeDataReceived(
......@@ -717,35 +757,6 @@ class DeviceStatusCollectorState : public StatusCollectorState {
}
}
}
if (probe_result.value().storage_size() > 0) {
em::StorageStatus* const storage_status =
response_params_.device_status->mutable_storage_status();
for (const auto& storage : probe_result.value().storage()) {
if (storage.name() != kGenericDeviceName)
continue;
em::DiskInfo* const disk_info = storage_status->add_disks();
disk_info->set_serial(base::NumberToString(storage.values().serial()));
disk_info->set_manufacturer(
base::NumberToString(storage.values().manfid()));
disk_info->set_model(storage.values().name());
disk_info->set_type(storage.values().type());
disk_info->set_size(storage.values().size());
}
}
if (probe_result.value().vpd_cached_size() > 0) {
em::SystemStatus* const system_status =
response_params_.device_status->mutable_system_status();
// vpd_cached values are a repeated field in ProbeResult protobuf,
// while logically it should be optional. Using iteration + value checks
// just for future-proofing code.
for (const auto& vpd_values : probe_result.value().vpd_cached()) {
if (vpd_values.name() != kGenericDeviceName)
continue;
const std::string& sku_number = vpd_values.values().vpd_sku_number();
if (!sku_number.empty())
system_status->set_vpd_sku_number(sku_number);
}
}
}
void OnEMMCLifetimeReceived(const em::DiskLifetimeEstimation& est) {
......@@ -807,6 +818,7 @@ DeviceStatusCollector::DeviceStatusCollector(
const TpmStatusFetcher& tpm_status_fetcher,
const EMMCLifetimeFetcher& emmc_lifetime_fetcher,
const StatefulPartitionInfoFetcher& stateful_partition_info_fetcher,
const CrosHealthdDataFetcher& cros_healthd_data_fetcher,
bool is_enterprise_reporting)
: StatusCollector(provider,
chromeos::CrosSettings::Get(),
......@@ -820,6 +832,7 @@ DeviceStatusCollector::DeviceStatusCollector(
tpm_status_fetcher_(tpm_status_fetcher),
emmc_lifetime_fetcher_(emmc_lifetime_fetcher),
stateful_partition_info_fetcher_(stateful_partition_info_fetcher),
cros_healthd_data_fetcher_(cros_healthd_data_fetcher),
runtime_probe_(
chromeos::DBusThreadManager::Get()->GetRuntimeProbeClient()),
is_enterprise_reporting_(is_enterprise_reporting) {
......@@ -857,6 +870,12 @@ DeviceStatusCollector::DeviceStatusCollector(
if (stateful_partition_info_fetcher_.is_null())
stateful_partition_info_fetcher_ = base::Bind(&ReadStatefulPartitionInfo);
if (cros_healthd_data_fetcher_.is_null()) {
cros_healthd_data_fetcher_ =
base::BindRepeating(&DeviceStatusCollector::FetchCrosHealthdData,
weak_factory_.GetWeakPtr());
}
idle_poll_timer_.Start(FROM_HERE,
TimeDelta::FromSeconds(kIdlePollIntervalSeconds), this,
&DeviceStatusCollector::CheckIdleState);
......@@ -1383,15 +1402,31 @@ void DeviceStatusCollector::AddDataSample(std::unique_ptr<SampledData> sample,
std::move(callback).Run();
}
void DeviceStatusCollector::FetchCrosHealthdData(
CrosHealthdDataReceiver callback) {
using chromeos::cros_healthd::mojom::ProbeCategoryEnum;
DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
std::vector<ProbeCategoryEnum> categories_to_probe = {
ProbeCategoryEnum::kCachedVpdData};
if (report_storage_status_)
categories_to_probe.push_back(ProbeCategoryEnum::kNonRemovableBlockDevices);
chromeos::cros_healthd::ServiceConnection::GetInstance()->ProbeTelemetryInfo(
categories_to_probe, std::move(callback));
}
void DeviceStatusCollector::FetchProbeData(ProbeDataReceiver callback) {
DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
runtime_probe::ProbeRequest request;
if (report_power_status_)
request.add_categories(runtime_probe::ProbeRequest::battery);
if (report_storage_status_)
request.add_categories(runtime_probe::ProbeRequest::storage);
request.add_categories(runtime_probe::ProbeRequest::vpd_cached);
// Note that we could send a probe request without any categories. The reason
// for that is that the OnProbeDataFetched callback also samples CPU
// temperature independently of querying runtime_probe. Since cros_healthd is
// replacing runtime_probe in DeviceStatusCollector, it doesn't make sense to
// refactor the runtime_probe code to fix this oddity at the moment.
auto sample = std::make_unique<SampledData>();
sample->timestamp = base::Time::Now();
auto completion_callback =
......@@ -1742,6 +1777,7 @@ bool DeviceStatusCollector::GetHardwareStatus(
if (report_power_status_ || report_storage_status_) {
state->FetchEMMCLifeTime(emmc_lifetime_fetcher_);
state->FetchProbeData(probe_data_fetcher_);
state->FetchCrosHealthdData(cros_healthd_data_fetcher_);
} else {
// Sample CPU temperature in a background thread.
state->SampleCPUTempInfo(cpu_temp_fetcher_);
......
......@@ -30,6 +30,7 @@
#include "chromeos/dbus/power/power_manager_client.h"
#include "chromeos/dbus/runtime_probe/runtime_probe.pb.h"
#include "chromeos/dbus/runtime_probe_client.h"
#include "chromeos/services/cros_healthd/public/mojom/cros_healthd.mojom.h"
#include "components/policy/proto/device_management_backend.pb.h"
#include "components/prefs/pref_member.h"
#include "components/session_manager/core/session_manager.h"
......@@ -158,6 +159,14 @@ class DeviceStatusCollector : public StatusCollector,
const base::circular_deque<std::unique_ptr<SampledData>>&)>;
// Gets the ProbeResult/sampled data and passes it to ProbeDataReceiver.
using ProbeDataFetcher = base::RepeatingCallback<void(ProbeDataReceiver)>;
// Format of the function that asynchronously receives data from cros_healthd.
using CrosHealthdDataReceiver =
base::OnceCallback<void(chromeos::cros_healthd::mojom::TelemetryInfoPtr)>;
// Gets the data from cros_healthd and passes it to CrosHealthdDataReceiver.
using CrosHealthdDataFetcher =
base::RepeatingCallback<void(CrosHealthdDataReceiver)>;
// Reads EMMC usage lifetime from /var/log/storage_info.txt
using EMMCLifetimeFetcher =
base::RepeatingCallback<enterprise_management::DiskLifetimeEstimation(
......@@ -180,6 +189,7 @@ class DeviceStatusCollector : public StatusCollector,
const TpmStatusFetcher& tpm_status_fetcher,
const EMMCLifetimeFetcher& emmc_lifetime_fetcher,
const StatefulPartitionInfoFetcher& stateful_partition_info_fetcher,
const CrosHealthdDataFetcher& cros_healthd_data_fetcher,
bool is_enterprise_reporting);
~DeviceStatusCollector() override;
......@@ -324,6 +334,10 @@ class DeviceStatusCollector : public StatusCollector,
void AddDataSample(std::unique_ptr<SampledData> sample,
SamplingCallback callback);
// CrosHealthdDataReceiver interface implementation, fetches data from
// cros_healthd and passes it to |callback|.
void FetchCrosHealthdData(CrosHealthdDataReceiver callback);
// ProbeDataReceiver interface implementation, fetches data from
// RuntimeProbe passes it to |callback| via OnProbeDataFetched().
void FetchProbeData(ProbeDataReceiver callback);
......@@ -412,6 +426,8 @@ class DeviceStatusCollector : public StatusCollector,
StatefulPartitionInfoFetcher stateful_partition_info_fetcher_;
CrosHealthdDataFetcher cros_healthd_data_fetcher_;
PowerStatusCallback power_status_callback_;
// Runtime probe client. Used to fetch hardware data.
......
......@@ -65,6 +65,7 @@ class MockDeviceStatusCollector : public policy::DeviceStatusCollector {
policy::DeviceStatusCollector::TpmStatusFetcher(),
policy::DeviceStatusCollector::EMMCLifetimeFetcher(),
policy::DeviceStatusCollector::StatefulPartitionInfoFetcher(),
policy::DeviceStatusCollector::CrosHealthdDataFetcher(),
true /* is_enterprise_device */) {}
MOCK_METHOD1(GetStatusAsync, void(const policy::StatusCollectorCallback&));
......
......@@ -21,5 +21,7 @@ component("cros_healthd") {
"cros_healthd_client.h",
"fake_cros_healthd_client.cc",
"fake_cros_healthd_client.h",
"fake_cros_healthd_service.cc",
"fake_cros_healthd_service.h",
]
}
......@@ -111,12 +111,6 @@ void CrosHealthdClient::InitializeFake() {
new FakeCrosHealthdClient();
}
// static
void CrosHealthdClient::InitializeFakeWithMockService(
mojo::PendingRemote<cros_healthd::mojom::CrosHealthdService> mock_service) {
new FakeCrosHealthdClient(std::move(mock_service));
}
// static
void CrosHealthdClient::Shutdown() {
DCHECK(g_instance);
......
......@@ -12,6 +12,7 @@
#include "base/files/scoped_file.h"
#include "base/macros.h"
#include "chromeos/services/cros_healthd/public/mojom/cros_healthd.mojom.h"
#include "chromeos/services/cros_healthd/public/mojom/cros_healthd_probe.mojom.h"
#include "mojo/public/cpp/bindings/pending_remote.h"
#include "mojo/public/cpp/bindings/remote.h"
......@@ -31,10 +32,6 @@ class COMPONENT_EXPORT(CROS_HEALTHD) CrosHealthdClient {
// Creates and initializes a fake global instance if not already created.
static void InitializeFake();
static void InitializeFakeWithMockService(
mojo::PendingRemote<cros_healthd::mojom::CrosHealthdService>
mock_service);
// Destroys the global instance.
static void Shutdown();
......
......@@ -5,25 +5,45 @@
#include "chromeos/dbus/cros_healthd/fake_cros_healthd_client.h"
#include "base/callback.h"
#include "chromeos/services/cros_healthd/public/mojom/cros_healthd.mojom.h"
namespace chromeos {
FakeCrosHealthdClient::FakeCrosHealthdClient() = default;
namespace {
FakeCrosHealthdClient::FakeCrosHealthdClient(
mojo::PendingRemote<cros_healthd::mojom::CrosHealthdService> mock_service)
: mock_service_(std::move(mock_service)) {}
// Used to track the fake instance, mirrors the instance in the base class.
FakeCrosHealthdClient* g_instance = nullptr;
FakeCrosHealthdClient::~FakeCrosHealthdClient() = default;
} // namespace
FakeCrosHealthdClient::FakeCrosHealthdClient() {
DCHECK(!g_instance);
g_instance = this;
}
FakeCrosHealthdClient::~FakeCrosHealthdClient() {
DCHECK_EQ(this, g_instance);
g_instance = nullptr;
}
// static
FakeCrosHealthdClient* FakeCrosHealthdClient::Get() {
return g_instance;
}
mojo::Remote<cros_healthd::mojom::CrosHealthdService>
FakeCrosHealthdClient::BootstrapMojoConnection(
base::OnceCallback<void(bool success)> result_callback) {
mojo::Remote<cros_healthd::mojom::CrosHealthdService> remote(
std::move(mock_service_));
receiver_.BindNewPipeAndPassRemote());
std::move(result_callback).Run(/*success=*/true);
return remote;
}
void FakeCrosHealthdClient::SetProbeTelemetryInfoResponseForTesting(
TelemetryInfoPtr& info) {
fake_service_.SetProbeTelemetryInfoResponseForTesting(info);
}
} // namespace chromeos
......@@ -9,25 +9,42 @@
#include "base/files/scoped_file.h"
#include "base/macros.h"
#include "chromeos/dbus/cros_healthd/cros_healthd_client.h"
#include "chromeos/dbus/cros_healthd/fake_cros_healthd_service.h"
#include "chromeos/services/cros_healthd/public/mojom/cros_healthd.mojom.h"
#include "chromeos/services/cros_healthd/public/mojom/cros_healthd_probe.mojom.h"
#include "mojo/public/cpp/bindings/receiver.h"
namespace chromeos {
// Fake implementation of CrosHealthdClient.
class FakeCrosHealthdClient : public CrosHealthdClient {
class COMPONENT_EXPORT(CROS_HEALTHD) FakeCrosHealthdClient
: public CrosHealthdClient {
public:
using TelemetryInfoPtr = cros_healthd::mojom::TelemetryInfoPtr;
// FakeCrosHealthdClient can be embedded in unit tests, but the
// InitializeFake/Shutdown pattern should be preferred. Constructing the
// instance will set the global instance for the fake and for the base class,
// so the static Get() accessor can be used with that pattern.
FakeCrosHealthdClient();
explicit FakeCrosHealthdClient(
mojo::PendingRemote<cros_healthd::mojom::CrosHealthdService>
mock_service);
~FakeCrosHealthdClient() override;
// Checks that a FakeCrosHealthdClient instance was initialized and returns
// it.
static FakeCrosHealthdClient* Get();
// CrosHealthdClient overrides:
mojo::Remote<cros_healthd::mojom::CrosHealthdService> BootstrapMojoConnection(
base::OnceCallback<void(bool success)> result_callback) override;
// Set the TelemetryInfoPtr that will be used in the response to any
// ProbeTelemetryInfo IPCs received.
void SetProbeTelemetryInfoResponseForTesting(TelemetryInfoPtr& info);
private:
mojo::PendingRemote<cros_healthd::mojom::CrosHealthdService> mock_service_;
cros_healthd::FakeCrosHealthdService fake_service_;
mojo::Receiver<cros_healthd::mojom::CrosHealthdService> receiver_{
&fake_service_};
DISALLOW_COPY_AND_ASSIGN(FakeCrosHealthdClient);
};
......
// Copyright 2019 The Chromium Authors. All rights reserved.
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.
#include "chromeos/dbus/cros_healthd/fake_cros_healthd_service.h"
#include <utility>
namespace chromeos {
namespace cros_healthd {
FakeCrosHealthdService::FakeCrosHealthdService() = default;
FakeCrosHealthdService::~FakeCrosHealthdService() = default;
void FakeCrosHealthdService::ProbeTelemetryInfo(
const std::vector<mojom::ProbeCategoryEnum>& categories,
ProbeTelemetryInfoCallback callback) {
std::move(callback).Run(response_info_.Clone());
}
void FakeCrosHealthdService::SetProbeTelemetryInfoResponseForTesting(
mojom::TelemetryInfoPtr& response_info) {
response_info_.Swap(&response_info);
}
} // namespace cros_healthd
} // namespace chromeos
// Copyright 2019 The Chromium Authors. All rights reserved.
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.
#ifndef CHROMEOS_DBUS_CROS_HEALTHD_FAKE_CROS_HEALTHD_SERVICE_H_
#define CHROMEOS_DBUS_CROS_HEALTHD_FAKE_CROS_HEALTHD_SERVICE_H_
#include "base/macros.h"
#include "chromeos/services/cros_healthd/public/mojom/cros_healthd.mojom.h"
#include "chromeos/services/cros_healthd/public/mojom/cros_healthd_probe.mojom.h"
namespace chromeos {
namespace cros_healthd {
class FakeCrosHealthdService final : public mojom::CrosHealthdService {
public:
FakeCrosHealthdService();
~FakeCrosHealthdService() override;
// CrosHealthdService overrides:
void ProbeTelemetryInfo(
const std::vector<mojom::ProbeCategoryEnum>& categories,
ProbeTelemetryInfoCallback callback) override;
// Set the TelemetryInfoPtr that will be used in the response to any
// ProbeTelemetryInfo IPCs received.
void SetProbeTelemetryInfoResponseForTesting(
mojom::TelemetryInfoPtr& response_info);
private:
// Used as the response to any ProbeTelemetryInfo IPCs received.
mojom::TelemetryInfoPtr response_info_{mojom::TelemetryInfo::New()};
DISALLOW_COPY_AND_ASSIGN(FakeCrosHealthdService);
};
} // namespace cros_healthd
} // namespace chromeos
#endif // CHROMEOS_DBUS_CROS_HEALTHD_FAKE_CROS_HEALTHD_SERVICE_H_
......@@ -12,6 +12,7 @@
#include "base/run_loop.h"
#include "base/test/task_environment.h"
#include "chromeos/dbus/cros_healthd/cros_healthd_client.h"
#include "chromeos/dbus/cros_healthd/fake_cros_healthd_client.h"
#include "chromeos/services/cros_healthd/public/mojom/cros_healthd.mojom.h"
#include "chromeos/services/cros_healthd/public/mojom/cros_healthd_probe.mojom.h"
#include "testing/gmock/include/gmock/gmock.h"
......@@ -59,52 +60,46 @@ mojom::TelemetryInfoPtr MakeTelemetryInfo() {
);
}
class MockCrosHealthdService : public mojom::CrosHealthdService {
public:
mojo::PendingRemote<mojom::CrosHealthdService> GetPendingRemote() {
return receiver_.BindNewPipeAndPassRemote();
}
MOCK_METHOD2(
ProbeTelemetryInfo,
void(const std::vector<mojom::ProbeCategoryEnum>& categories_to_test,
ProbeTelemetryInfoCallback callback));
private:
mojo::Receiver<mojom::CrosHealthdService> receiver_{this};
};
class CrosHealthdServiceConnectionTest : public testing::Test {
public:
CrosHealthdServiceConnectionTest() = default;
void SetUp() override {
CrosHealthdClient::InitializeFakeWithMockService(
mock_service_.GetPendingRemote());
}
void SetUp() override { CrosHealthdClient::InitializeFake(); }
void TearDown() override { CrosHealthdClient::Shutdown(); }
MockCrosHealthdService* mock_service() { return &mock_service_; }
private:
base::test::TaskEnvironment task_environment_;
StrictMock<MockCrosHealthdService> mock_service_;
DISALLOW_COPY_AND_ASSIGN(CrosHealthdServiceConnectionTest);
};
TEST_F(CrosHealthdServiceConnectionTest, ProbeTelemetryInfo) {
EXPECT_CALL(*mock_service(), ProbeTelemetryInfo(_, _))
.WillOnce(WithArgs<1>(Invoke(
[](mojom::CrosHealthdService::ProbeTelemetryInfoCallback callback) {
std::move(callback).Run(MakeTelemetryInfo());
})));
// Test that we can send a request without categories.
auto empty_info = mojom::TelemetryInfo::New();
FakeCrosHealthdClient::Get()->SetProbeTelemetryInfoResponseForTesting(
empty_info);
const std::vector<mojom::ProbeCategoryEnum> no_categories = {};
bool callback_done = false;
ServiceConnection::GetInstance()->ProbeTelemetryInfo(
no_categories, base::BindOnce(
[](bool* callback_done, mojom::TelemetryInfoPtr info) {
EXPECT_EQ(info, mojom::TelemetryInfo::New());
*callback_done = true;
},
&callback_done));
base::RunLoop().RunUntilIdle();
EXPECT_TRUE(callback_done);
// Test that we can request all categories.
auto response_info = MakeTelemetryInfo();
FakeCrosHealthdClient::Get()->SetProbeTelemetryInfoResponseForTesting(
response_info);
const std::vector<mojom::ProbeCategoryEnum> categories_to_test = {
mojom::ProbeCategoryEnum::kBattery,
mojom::ProbeCategoryEnum::kNonRemovableBlockDevices,
mojom::ProbeCategoryEnum::kCachedVpdData};
bool callback_done = false;
callback_done = false;
ServiceConnection::GetInstance()->ProbeTelemetryInfo(
categories_to_test,
base::BindOnce(
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment