am defbd39c: Merge "metricsd: Read build time values from etc/os-release.d."

* commit 'defbd39cebb0aa9a982d91a9900cebfbcd0d59ae':
  metricsd: Read build time values from etc/os-release.d.

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.

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_

metricsd/metrics_daemon.cc

@@ -28,7 +28,7 @@
 #include <base/strings/string_split.h>
 #include <base/strings/string_util.h>
 #include <base/strings/stringprintf.h>
-#include <cutils/properties.h>
+#include <brillo/osrelease_reader.h>
 #include <dbus/dbus.h>
 #include <dbus/message.h>
 
@@ -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,

metricsd/uploader/system_profile_cache.cc

@@ -21,8 +21,9 @@
 #include <base/logging.h>
 #include <base/strings/string_number_conversions.h>
 #include <base/strings/string_util.h>
-#include <cutils/properties.h>
+#include <brillo/osrelease_reader.h>
 #include <string>
+#include <update_engine/client.h>
 #include <vector>
 
 #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") {

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_;

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<metrics::MetricSample> 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;