Send UMA stability stats in a separate UMA log on startup.

chrome/browser/metrics/metrics_service.h

 // Copyright (c) 2012 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.
 
 // This file defines a service that collects information about the user
 // experience in order to help improve future versions of the app.
 
 #ifndef CHROME_BROWSER_METRICS_METRICS_SERVICE_H_
 #define CHROME_BROWSER_METRICS_METRICS_SERVICE_H_
 
@@ skipping 96 matching lines @@
     STARTUP_TIMEBOMB_ARM = 300,
     THREAD_WATCHER_START = 400,
     MAIN_MESSAGE_LOOP_RUN = 500,
     SHUTDOWN_TIMEBOMB_ARM = 600,
     SHUTDOWN_COMPLETE = 700,
   };
 
   MetricsService();
   virtual ~MetricsService();
 
+  // Initializes metrics recording state. Updates various bookkeeping values in
+  // prefs and sets up the scheduler. This is a separate function rather than
+  // being done by the constructor so that field trials could be created before
+  // this is run. Takes |reporting_will_be_enabled| parameter which specifies
+  // whether UMA is enabled.
+  void InitializeMetricsRecordingState(bool reporting_will_be_enabled);

[Ilya Sherman, 2013/12/06 06:21:26]

nit: Please define an enum to use in place of the boolean, since the meaning of
"true" or "false" is currently not obvious at call sites.

[Ilya Sherman, 2013/12/06 06:21:26]

Why is this method public?  It doesn't look like you added any clients of this
code as part of this CL.

[Alexei Svitkine (slow), 2013/12/06 17:59:22]

Yes, that was my mistake - I forgot to include the corresponding changes to
chrome_browser_main.cc as part of the original CL - fixed now. The reason this
is public is so that it can be called after field trials are created (so that
its behavior can be based on field trial state).

[Alexei Svitkine (slow), 2013/12/06 17:59:22]

Done.

+
   // Starts the metrics system, turning on recording and uploading of metrics.
   // Should be called when starting up with metrics enabled, or when metrics
   // are turned on.
   void Start();
 
   // Starts the metrics system in a special test-only mode. Metrics won't ever
   // be uploaded or persisted in this mode, but metrics will be recorded in
   // memory.
   void StartRecordingForTests();
 
@@ skipping 119 matching lines @@
   // registered at a time for a given trial_name. Only the last group name that
   // is registered for a given trial name will be recorded. The values passed
   // in must not correspond to any real field trial in the code.
   // To use this method, SyntheticTrialGroup should friend your class.
   void RegisterSyntheticFieldTrial(const SyntheticTrialGroup& trial_group);
 
  private:
   // The MetricsService has a lifecycle that is stored as a state.
   // See metrics_service.cc for description of this lifecycle.
   enum State {
-    INITIALIZED,            // Constructor was called.
-    INIT_TASK_SCHEDULED,    // Waiting for deferred init tasks to complete.
-    INIT_TASK_DONE,         // Waiting for timer to send initial log.
-    INITIAL_LOG_READY,      // Initial log generated, and waiting for reply.
-    SENDING_OLD_LOGS,       // Sending unsent logs from previous session.
-    SENDING_CURRENT_LOGS,   // Sending standard current logs as they acrue.
+    INITIALIZED,                    // Constructor was called.
+    INIT_TASK_SCHEDULED,            // Waiting for deferred init task to finish.

[Ilya Sherman, 2013/12/06 06:21:26]

nit: "tasks" seems more accurate, since this can be waiting on any of a number
of steps.

[Alexei Svitkine (slow), 2013/12/06 17:59:22]

Unfortunately, it goes above the 80 col limit (if we want to keep the //'s
aligned). We could change the comments to be before each enum, WDYT?

[Ilya Sherman, 2013/12/07 06:51:53]

You can wrap the line like so:

    INIT_TASK_SCHEDULED,            // Waiting for deferred init tasks to
                                    // complete.

[Alexei Svitkine (slow), 2013/12/09 21:17:02]

Done.

+    INIT_TASK_DONE,                 // Waiting for timer to send initial log.
+    SENDING_INITIAL_STABILITY_LOG,  // Initial stability log being sent.
+    SENDING_INITIAL_METRICS_LOG,    // Initial metrics log being sent.

[Ilya Sherman, 2013/12/06 06:21:26]

Jim made a good point in a previous discussion that there might not be much
reason anymore to distinguish between the initial log and ongoing logs, other
than stability logs.  Is the plan to get rid of or otherwise rename this state
once the field trial is at 100%?

[Alexei Svitkine (slow), 2013/12/06 17:59:22]

The code already treats it as an ongoing log (semantically) when the field trial
is on. However, it is special still - for example it has profiler data and
doesn't have counts, so having a special name "initial metrics log" still makes
sense as the current code stands. However, long term I agree we should try to
make it similar to regular ongoing logs - but we'd probably want to figure out
how we want to log profiler data in the future before that happens, for example.
So I'd leave it as is.

[Ilya Sherman, 2013/12/07 06:51:53]

Ok.

+    SENDING_OLD_LOGS,               // Sending unsent logs from last session.
+    SENDING_CURRENT_LOGS,           // Sending ongoing logs as they accrue.
   };
 
   enum ShutdownCleanliness {
     CLEANLY_SHUTDOWN = 0xdeadbeef,
     NEED_TO_SHUTDOWN = ~CLEANLY_SHUTDOWN
   };
 
   // Designates which entropy source was returned from this MetricsService.
   // This is used for testing to validate that we return the correct source
   // depending on the state of the service.
@@ skipping 70 matching lines @@
   void EnableRecording();
   void DisableRecording();
 
   // If in_idle is true, sets idle_since_last_transmission to true.
   // If in_idle is false and idle_since_last_transmission_ is true, sets
   // idle_since_last_transmission to false and starts the timer (provided
   // starting the timer is permitted).
   void HandleIdleSinceLastTransmission(bool in_idle);
 
   // Set up client ID, session ID, etc.
-  void InitializeMetricsState();
+  void InitializeMetricsState(bool reporting_will_be_enabled);
 
   // Generates a new client ID to use to identify self to metrics server.
   static std::string GenerateClientID();
 
   // Schedule the next save of LocalState information.  This is called
   // automatically by the task that performs each save to schedule the next one.
   void ScheduleNextStateSave();
 
   // Save the LocalState information immediately. This should not be called by
   // anybody other than the scheduler to avoid doing too many writes. When you
@@ skipping 23 matching lines @@
   // Callbacks for various stages of final log info collection. Do not call
   // these directly.
   void OnMemoryDetailCollectionDone();
   void OnHistogramSynchronizationDone();
   void OnFinalLogInfoCollectionDone();
 
   // Either closes the current log or creates and closes the initial log
   // (depending on |state_|), and stages it for upload.
   void StageNewLog();
 
-  // Record stats, client ID, Session ID, etc. in a special "first" log.
-  void PrepareInitialLog();
+  // Prepares the initial stability log, based on the saved system profile from
+  // a previous session and metrics stability stats and stages it for upload.

[Ilya Sherman, 2013/12/06 06:21:26]

nit: "and metrics stability stats" -> rephrase somehow?  Maybe try to clarify
that this *only* uploads data coming from a previous session.

[Alexei Svitkine (slow), 2013/12/06 17:59:22]

Done.

+  void PrepareInitialStabilityLog();
+
+  // Prepares the initial metrics log, which includes startup histograms and
+  // profiler data, as well as incremental stability-related metrics.
+  void PrepareInitialMetricsLog(MetricsLog::LogType log_type);
 
   // Uploads the currently staged log (which must be non-null).
   void SendStagedLog();
 
   // Prepared the staged log to be passed to the server. Upon return,
   // current_fetch_ should be reset with its upload data set to a compressed
   // copy of the staged log.
   void PrepareFetchWithStagedLog();
 
   // Implementation of net::URLFetcherDelegate. Called after transmission
@@ skipping 72 matching lines @@
   // WiFi adapter, etc.  For non Chrome OS hosts, this will be an
   // empty string.
   std::string hardware_class_;
 
   // The list of plugins which was retrieved on the file thread.
   std::vector<content::WebPluginInfo> plugins_;
 
   // Google Update statistics, which were retrieved on a blocking pool thread.
   GoogleUpdateMetrics google_update_metrics_;
 
-  // The initial log, used to record startup metrics.
-  scoped_ptr<MetricsLog> initial_log_;
+  // The initial metrics log, used to record startup metrics (histograms and
+  // profiler data). Note that if a crash occurred in the previous session, an
+  // initial stability log may be sent before this.
+  scoped_ptr<MetricsLog> initial_metrics_log_;
 
   // The outstanding transmission appears as a URL Fetch operation.
   scoped_ptr<net::URLFetcher> current_fetch_;
 
   // The TCP/UDP echo server to collect network connectivity stats.
   std::string network_stats_server_;
 
   // The HTTP pipelining test server.
   std::string http_pipelining_test_server_;
 
@@ skipping 82 matching lines @@
   friend class extensions::ExtensionDownloader;
   friend class extensions::ManifestFetchData;
 
   // Returns true if prefs::kMetricsReportingEnabled is set.
   static bool IsMetricsReportingEnabled();
 
   DISALLOW_IMPLICIT_CONSTRUCTORS(MetricsServiceHelper);
 };
 
 #endif  // CHROME_BROWSER_METRICS_METRICS_SERVICE_H_