Merge "metricsd: Don't crash when some metadata is missing."

metricsd/uploader/metrics_log.cc

@@ -48,6 +48,6 @@ void MetricsLog::IncrementUncleanShutdownCount() {
   stability->set_unclean_system_shutdown_count(current + 1);
 }
 
-void MetricsLog::PopulateSystemProfile(SystemProfileSetter* profile_setter) {
-  profile_setter->Populate(uma_proto());
+bool MetricsLog::PopulateSystemProfile(SystemProfileSetter* profile_setter) {
+  return profile_setter->Populate(uma_proto());
 }

metricsd/uploader/metrics_log.h

@@ -39,7 +39,7 @@ class MetricsLog : public metrics::MetricsLogBase {
   void IncrementUncleanShutdownCount();
 
   // Populate the system profile with system information using setter.
-  void PopulateSystemProfile(SystemProfileSetter* setter);
+  bool PopulateSystemProfile(SystemProfileSetter* setter);
 
  private:
   FRIEND_TEST(UploadServiceTest, LogContainsAggregatedValues);

metricsd/uploader/system_profile_cache.cc

@@ -75,7 +75,7 @@ bool SystemProfileCache::Initialize() {
 
   if (!base::SysInfo::GetLsbReleaseValue("BRILLO_BUILD_TARGET_ID",
                                          &profile_.build_target_id)) {
-    LOG(ERROR) << "Could not initialize system profile.";
+    LOG(ERROR) << "BRILLO_BUILD_TARGET_ID is not set in /etc/lsb-release.";
     return false;
   }
 
@@ -109,11 +109,12 @@ bool SystemProfileCache::InitializeOrCheck() {
   return initialized_ || Initialize();
 }
 
-void SystemProfileCache::Populate(
+bool SystemProfileCache::Populate(
     metrics::ChromeUserMetricsExtension* metrics_proto) {
   CHECK(metrics_proto);
-  CHECK(InitializeOrCheck())
-      << "failed to initialize system information.";
+  if (not InitializeOrCheck()) {
+    return false;
+  }
 
   // The client id is hashed before being sent.
   metrics_proto->set_client_id(
@@ -132,6 +133,8 @@ void SystemProfileCache::Populate(
   metrics::SystemProfileProto_BrilloDeviceData* device_data =
       profile_proto->mutable_brillo();
   device_data->set_build_target_id(profile_.build_target_id);
+
+  return true;
 }
 
 std::string SystemProfileCache::GetPersistentGUID(

metricsd/uploader/system_profile_cache.h

@@ -52,7 +52,7 @@ class SystemProfileCache : public SystemProfileSetter {
   SystemProfileCache(bool testing, const std::string& config_root);
 
   // Populates the ProfileSystem protobuf with system information.
-  void Populate(metrics::ChromeUserMetricsExtension* metrics_proto) override;
+  bool Populate(metrics::ChromeUserMetricsExtension* metrics_proto) override;
 
   // Converts a string representation of the channel to a
   // SystemProfileProto_Channel

metricsd/uploader/system_profile_setter.h

@@ -27,7 +27,7 @@ class SystemProfileSetter {
  public:
   virtual ~SystemProfileSetter() {}
   // Populates the protobuf with system informations.
-  virtual void Populate(metrics::ChromeUserMetricsExtension* profile_proto) = 0;
+  virtual bool Populate(metrics::ChromeUserMetricsExtension* profile_proto) = 0;
 };
 
 #endif  // METRICS_UPLOADER_SYSTEM_PROFILE_SETTER_H_

metricsd/uploader/upload_service.cc

@@ -73,7 +73,6 @@ void UploadService::StartNewLog() {
   CHECK(!staged_log_) << "the staged log should be discarded before starting "
                          "a new metrics log";
   MetricsLog* log = new MetricsLog();
-  log->PopulateSystemProfile(system_profile_setter_.get());
   current_log_.reset(log);
 }
 
@@ -97,13 +96,12 @@ void UploadService::UploadEvent() {
   // Previous upload successful, reading metrics sample from the file.
   ReadMetrics();
   GatherHistograms();
-
-  // No samples found. Exit to avoid sending an empty log.
-  if (!current_log_)
-    return;
-
   StageCurrentLog();
-  SendStagedLog();
+
+  // If a log is available for upload, upload it.
+  if (staged_log_) {
+    SendStagedLog();
+  }
 }
 
 void UploadService::SendStagedLog() {
@@ -225,6 +223,11 @@ void UploadService::StageCurrentLog() {
 
   staged_log_.swap(current_log_);
   staged_log_->CloseLog();
+  if (!staged_log_->PopulateSystemProfile(system_profile_setter_.get())) {
+    LOG(WARNING) << "Error while adding metadata to the log. Discarding the "
+                 << "log.";
+    staged_log_.reset();
+  }
   failed_upload_count_ = 0;
 }