Send metrics with embedded system profiles after system startup.

components/metrics/metrics_service.cc

 // Copyright 2014 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.
 
 //------------------------------------------------------------------------------
 // Description of the life cycle of a instance of MetricsService.
 //
 //  OVERVIEW
 //
 // A MetricsService instance is typically created at application startup.  It is
@@ skipping 765 matching lines @@
 bool MetricsService::PrepareInitialStabilityLog(
     const std::string& prefs_previous_version) {
   DCHECK_EQ(INITIALIZED, state_);
 
   std::unique_ptr<MetricsLog> initial_stability_log(
       CreateLog(MetricsLog::INITIAL_STABILITY_LOG));
 
   // Do not call NotifyOnDidCreateMetricsLog here because the stability
   // log describes stats from the _previous_ session.
   std::string system_profile_app_version;
-  if (!initial_stability_log->LoadSavedEnvironmentFromPrefs(
-          &system_profile_app_version)) {
-    return false;
+  for (auto& provider : metrics_providers_) {
+    if (initial_stability_log->LoadSavedEnvironmentFromProvider(
+            provider.get(), &system_profile_app_version)) {

[Alexei Svitkine (slow), 2017/05/31 20:31:24]

Instead of replacing the profile of the stability log, can we generate a
separate log from each persistent file that has a system profile?

Otherwise if we have a scenario of:
  1. Session A crash
  2. Session B crash
  3. Session C OK

With your current CL, we will group stability stats + metrics from A and B with
one of the profiles of A or B - but that will not be correct for some of the
metrics in that report.

Instead, we should have:
  persistent histogram from A reported separately with its embedded profile
  persistent histogram from B reported separately with its embedded profile
  stability log just containing the non-persistent stuff with the system profile
from local state

[bcwhite, 2017/05/31 20:48:29]

Currently there is only ever one persistent histogram file and it's always for
the previous run so this scenario doesn't occur.  There are also other metrics
files (like from Startup and Crashpad) that don't have an embedded profile so
they can't be reported independently.

But generally I agree with you.  I'd like to get the system profile into every
persistent file and then move the reporting completely out to a separate task
that can be run sometime after startup or even by an outside process such as
Omaha.

Is this acceptable as a step along that path?

[Alexei Svitkine (slow), 2017/06/01 15:00:29]

Per offline discussion, I prefer to not have this intermediate step - as this
may report the stability metrics with the persistent histogram's system proto -
which can also be wrong. Let's just do the final solution.

+      break;
+    }
+  }
+  if (system_profile_app_version.empty()) {
+    if (!initial_stability_log->LoadSavedEnvironmentFromPrefs(
+            &system_profile_app_version)) {
+      return false;
+    }
   }
   if (system_profile_app_version != prefs_previous_version)
     StabilityMetricsProvider(local_state_).LogStabilityVersionMismatch();
 
   log_manager_.PauseCurrentLog();
   log_manager_.BeginLoggingWithLog(std::move(initial_stability_log));
 
   // Note: Some stability providers may record stability stats via histograms,
   //       so this call has to be after BeginLoggingWithLog().
   log_manager_.current_log()->RecordStabilityMetrics(
@@ skipping 194 matching lines @@
   // Redundant setting to assure that we always reset this value at shutdown
   // (and that we don't use some alternate path, and not call LogCleanShutdown).
   clean_shutdown_status_ = CLEANLY_SHUTDOWN;
   client_->OnLogCleanShutdown();
   clean_exit_beacon_.WriteBeaconValue(true);
   SetExecutionPhase(ExecutionPhase::SHUTDOWN_COMPLETE, local_state_);
   StabilityMetricsProvider(local_state_).MarkSessionEndCompleted(end_completed);
 }
 
 }  // namespace metrics