diff --git a/metricsd/Android.mk b/metricsd/Android.mk index 3deb7d391..8dbaad9fb 100644 --- a/metricsd/Android.mk +++ b/metricsd/Android.mk @@ -69,11 +69,11 @@ metrics_daemon_shared_libraries := $(libmetrics_shared_libraries) \ libchrome-dbus \ libchromeos-http \ libchromeos-dbus \ - libcutils \ libdbus \ libmetrics \ libprotobuf-cpp-lite \ librootdev \ + libupdate_engine_client \ libweaved \ # Shared library for metrics. diff --git a/metricsd/constants.h b/metricsd/constants.h index 8a00fc47d..7e1e116e5 100644 --- a/metricsd/constants.h +++ b/metricsd/constants.h @@ -27,10 +27,9 @@ static const char kStagedLogName[] = "staged_log"; static const char kFailedUploadCountName[] = "failed_upload_count"; static const char kDefaultVersion[] = "0.0.0.0"; -// System properties used. -static const char kProductIdProperty[] = "ro.product.product_id"; -static const char kChannelProperty[] = "ro.product.channel"; -static const char kProductVersionProperty[] = "ro.product.version"; +// Build time properties name. +static const char kProductId[] = "product_id"; +static const char kProductVersion[] = "product_version"; } // namespace metrics #endif // METRICS_CONSTANTS_H_ diff --git a/metricsd/metrics_daemon.cc b/metricsd/metrics_daemon.cc index 2b7e98c13..ed786e1fb 100644 --- a/metricsd/metrics_daemon.cc +++ b/metricsd/metrics_daemon.cc @@ -28,7 +28,7 @@ #include #include #include -#include +#include #include #include @@ -153,23 +153,19 @@ void MetricsDaemon::RunUploaderTest() { } uint32_t MetricsDaemon::GetOsVersionHash() { - static uint32_t cached_version_hash = 0; - static bool version_hash_is_cached = false; - if (version_hash_is_cached) - return cached_version_hash; - version_hash_is_cached = true; - - char version[PROPERTY_VALUE_MAX]; - // The version might not be set for development devices. In this case, use the - // zero version. - property_get(metrics::kProductVersionProperty, version, - metrics::kDefaultVersion); - - cached_version_hash = base::Hash(version); - if (testing_) { - cached_version_hash = 42; // return any plausible value for the hash + brillo::OsReleaseReader reader; + reader.Load(); + string version; + if (!reader.GetString(metrics::kProductVersion, &version)) { + LOG(ERROR) << "failed to read the product version."; + version = metrics::kDefaultVersion; } - return cached_version_hash; + + uint32_t version_hash = base::Hash(version); + if (testing_) { + version_hash = 42; // return any plausible value for the hash + } + return version_hash; } void MetricsDaemon::Init(bool testing, diff --git a/metricsd/uploader/system_profile_cache.cc b/metricsd/uploader/system_profile_cache.cc index 1d87be50a..1995510f8 100644 --- a/metricsd/uploader/system_profile_cache.cc +++ b/metricsd/uploader/system_profile_cache.cc @@ -21,8 +21,9 @@ #include #include #include -#include +#include #include +#include #include #include "constants.h" @@ -73,16 +74,27 @@ bool SystemProfileCache::Initialize() { CHECK(!initialized_) << "this should be called only once in the metrics_daemon lifetime."; - profile_.product_id = GetProperty(metrics::kProductIdProperty); + brillo::OsReleaseReader reader; + std::string channel; + if (testing_) { + reader.LoadTestingOnly(metrics_directory_); + channel = "unknown"; + } else { + reader.Load(); + auto client = update_engine::UpdateEngineClient::CreateInstance(); + if (!client->GetChannel(&channel)) { + LOG(ERROR) << "failed to read the current channel from update engine."; + } + } - if (profile_.product_id.empty()) { - LOG(ERROR) << "System property " << metrics::kProductIdProperty - << " is not set."; + if (!reader.GetString(metrics::kProductId, &profile_.product_id)) { + LOG(ERROR) << "product_id is not set."; return false; } - std::string channel = GetProperty(metrics::kChannelProperty); - profile_.version = GetProperty(metrics::kProductVersionProperty); + if (!reader.GetString(metrics::kProductVersion, &profile_.version)) { + LOG(ERROR) << "failed to read the product version"; + } if (channel.empty() || profile_.version.empty()) { // If the channel or version is missing, the image is not official. @@ -154,18 +166,6 @@ std::string SystemProfileCache::GetPersistentGUID( return guid; } -std::string SystemProfileCache::GetProperty(const std::string& name) { - if (testing_) { - std::string content; - base::ReadFileToString(metrics_directory_.Append(name), &content); - return content; - } else { - char value[PROPERTY_VALUE_MAX]; - property_get(name.data(), value, ""); - return std::string(value); - } -} - metrics::SystemProfileProto_Channel SystemProfileCache::ProtoChannelFromString( const std::string& channel) { if (channel == "stable") { diff --git a/metricsd/uploader/system_profile_cache.h b/metricsd/uploader/system_profile_cache.h index 341015731..97fb33a6f 100644 --- a/metricsd/uploader/system_profile_cache.h +++ b/metricsd/uploader/system_profile_cache.h @@ -76,11 +76,6 @@ class SystemProfileCache : public SystemProfileSetter { // Initializes |profile_| only if it has not been yet initialized. bool InitializeOrCheck(); - // Gets a system property as a string. - // When |testing_| is true, reads the value from |metrics_directory_|/|name| - // instead. - std::string GetProperty(const std::string& name); - bool initialized_; bool testing_; base::FilePath metrics_directory_; diff --git a/metricsd/uploader/upload_service_test.cc b/metricsd/uploader/upload_service_test.cc index 305fd0cb4..236376a50 100644 --- a/metricsd/uploader/upload_service_test.cc +++ b/metricsd/uploader/upload_service_test.cc @@ -58,9 +58,11 @@ class UploadServiceTest : public testing::Test { } void SetTestingProperty(const std::string& name, const std::string& value) { + base::FilePath filepath = dir_.path().Append("etc/os-release.d").Append(name); + ASSERT_TRUE(base::CreateDirectory(filepath.DirName())); ASSERT_EQ( value.size(), - base::WriteFile(dir_.path().Append(name), value.data(), value.size())); + base::WriteFile(filepath, value.data(), value.size())); } base::ScopedTempDir dir_; @@ -225,9 +227,8 @@ TEST_F(UploadServiceTest, ValuesInConfigFileAreSent) { SenderMock* sender = new SenderMock(); upload_service_->sender_.reset(sender); - SetTestingProperty(metrics::kChannelProperty, "beta"); - SetTestingProperty(metrics::kProductIdProperty, "hello"); - SetTestingProperty(metrics::kProductVersionProperty, "1.2.3.4"); + SetTestingProperty(metrics::kProductId, "hello"); + SetTestingProperty(metrics::kProductVersion, "1.2.3.4"); scoped_ptr histogram = metrics::MetricSample::SparseHistogramSample("myhistogram", 1); @@ -242,8 +243,6 @@ TEST_F(UploadServiceTest, ValuesInConfigFileAreSent) { EXPECT_TRUE(sender->is_good_proto()); EXPECT_EQ(1, sender->last_message_proto().histogram_event().size()); - EXPECT_EQ(metrics::SystemProfileProto::CHANNEL_BETA, - sender->last_message_proto().system_profile().channel()); EXPECT_NE(0, sender->last_message_proto().client_id()); EXPECT_NE(0, sender->last_message_proto().system_profile().build_timestamp()); EXPECT_NE(0, sender->last_message_proto().session_id()); @@ -269,7 +268,7 @@ TEST_F(UploadServiceTest, PersistentGUID) { } TEST_F(UploadServiceTest, SessionIdIncrementedAtInitialization) { - SetTestingProperty(metrics::kProductIdProperty, "hello"); + SetTestingProperty(metrics::kProductId, "hello"); SystemProfileCache cache(true, dir_.path()); cache.Initialize(); int session_id = cache.profile_.session_id;