Increase UMA upload interval, and upload usage stats more often

r=evanm
Review URL: http://codereview.chromium.org/14186

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@7165 0039d316-1c4b-4281-b951-d872f2087c98

3 files changed, 81 insertions, 27 deletions

diff --git a/chrome/browser/metrics_log.cc b/chrome/browser/metrics_log.cc
index 16a24b7..5efcb47 100644
--- a/chrome/browser/metrics_log.cc
+++ b/chrome/browser/metrics_log.cc
@@ -305,6 +305,17 @@ std::string MetricsLog::GetInstallDate() const {
   }
 }
 
+void MetricsLog::RecordIncrementalStabilityElements() {
+  DCHECK(!locked_);
+
+  PrefService* pref = g_browser_process->local_state();
+  DCHECK(pref);
+
+  OPEN_ELEMENT_FOR_SCOPE("stability");
+  WriteRequiredStabilityElements(pref);
+  WriteRealtimeStabilityElements(pref);
+}
+
 void MetricsLog::WriteStabilityElement() {
   DCHECK(!locked_);
 
@@ -316,26 +327,17 @@ void MetricsLog::WriteStabilityElement() {
   //       sent, but that's true for all the metrics.
 
   OPEN_ELEMENT_FOR_SCOPE("stability");
+  WriteRequiredStabilityElements(pref);
+  WriteRealtimeStabilityElements(pref);
 
-  WriteIntAttribute("launchcount",
-                    pref->GetInteger(prefs::kStabilityLaunchCount));
-  pref->SetInteger(prefs::kStabilityLaunchCount, 0);
-  WriteIntAttribute("crashcount",
-                    pref->GetInteger(prefs::kStabilityCrashCount));
-  pref->SetInteger(prefs::kStabilityCrashCount, 0);
+  // TODO(jar): The following are all optional, so we *could* optimize them for
+  // values of zero (and not include them).
   WriteIntAttribute("incompleteshutdowncount",
                     pref->GetInteger(
                         prefs::kStabilityIncompleteSessionEndCount));
   pref->SetInteger(prefs::kStabilityIncompleteSessionEndCount, 0);
-  WriteIntAttribute("pageloadcount",
-                    pref->GetInteger(prefs::kStabilityPageLoadCount));
-  pref->SetInteger(prefs::kStabilityPageLoadCount, 0);
-  WriteIntAttribute("renderercrashcount",
-                    pref->GetInteger(prefs::kStabilityRendererCrashCount));
-  pref->SetInteger(prefs::kStabilityRendererCrashCount, 0);
-  WriteIntAttribute("rendererhangcount",
-                    pref->GetInteger(prefs::kStabilityRendererHangCount));
-  pref->SetInteger(prefs::kStabilityRendererHangCount, 0);
+
+
   WriteIntAttribute("breakpadregistrationok",
       pref->GetInteger(prefs::kStabilityBreakpadRegistrationSuccess));
   pref->SetInteger(prefs::kStabilityBreakpadRegistrationSuccess, 0);
@@ -395,6 +397,41 @@ void MetricsLog::WriteStabilityElement() {
   }
 }
 
+void MetricsLog::WriteRequiredStabilityElements(PrefService* pref) {
+  // The server refuses data that doesn't have certain values.  crashcount and
+  // launchcount are currently "required" in the "stability" group.
+  WriteIntAttribute("launchcount",
+                    pref->GetInteger(prefs::kStabilityLaunchCount));
+  pref->SetInteger(prefs::kStabilityLaunchCount, 0);
+  WriteIntAttribute("crashcount",
+                    pref->GetInteger(prefs::kStabilityCrashCount));
+  pref->SetInteger(prefs::kStabilityCrashCount, 0);
+}
+
+void MetricsLog::WriteRealtimeStabilityElements(PrefService* pref) {
+  // Update the stats which are critical for real-time stability monitoring.
+  // Since these are "optional," only list ones that are non-zero, as the counts
+  // are aggergated (summed) server side.
+
+  int count = pref->GetInteger(prefs::kStabilityPageLoadCount);
+  if (count) {
+    WriteIntAttribute("pageloadcount", count);
+    pref->SetInteger(prefs::kStabilityPageLoadCount, 0);
+  }
+
+  count = pref->GetInteger(prefs::kStabilityRendererCrashCount);
+  if (count) {
+    WriteIntAttribute("renderercrashcount", count);
+    pref->SetInteger(prefs::kStabilityRendererCrashCount, 0);
+  }
+
+  count = pref->GetInteger(prefs::kStabilityRendererHangCount);
+  if (count) {
+    WriteIntAttribute("rendererhangcount", count);
+    pref->SetInteger(prefs::kStabilityRendererHangCount, 0);
+  }
+}
+
 void MetricsLog::WritePluginList(
          const std::vector<WebPluginInfo>& plugin_list) {
   DCHECK(!locked_);
diff --git a/chrome/browser/metrics_log.h b/chrome/browser/metrics_log.h
index 225e2a3..84faabe 100644
--- a/chrome/browser/metrics_log.h
+++ b/chrome/browser/metrics_log.h
@@ -70,9 +70,16 @@ class MetricsLog {
   // user uses the Omnibox to open a URL.
   void RecordOmniboxOpenedURL(const AutocompleteLog& log);
 
+  // Record any changes in a given histogram for transmission.
   void RecordHistogramDelta(const Histogram& histogram,
                             const Histogram::SampleSet& snapshot);
 
+  // Record recent delta for critical stability metrics.  We can't wait for a
+  // restart to gather these, as that delay biases our observation away from
+  // users that run happily for a looooong time.  We send increments with each
+  // uma log upload, just as we send histogram data.
+  void RecordIncrementalStabilityElements();
+
   // Stop writing to this record and generate the encoded representation.
   // None of the Record* methods can be called after this is called.
   void CloseLog();
@@ -150,6 +157,14 @@ class MetricsLog {
   // NOTE: Has the side-effect of clearing those counts.
   void WriteStabilityElement();
 
+  // Within the stability group, write required elements.
+  void WriteRequiredStabilityElements(PrefService* pref);
+  // Within the stability group, write elements that need to be updated asap
+  // and can't be delayed until the user decides to restart chromium.
+  // Delaying these stats would bias metrics away from happy long lived
+  // chromium processes (ones that don't crash, and keep on running).
+  void WriteRealtimeStabilityElements(PrefService* pref);
+
   // Writes the list of installed plugins.
   void WritePluginList(const std::vector<WebPluginInfo>& plugin_list);
 
diff --git a/chrome/browser/metrics_service.cc b/chrome/browser/metrics_service.cc
index 94ea1ef..7a022fa 100644
--- a/chrome/browser/metrics_service.cc
+++ b/chrome/browser/metrics_service.cc
@@ -38,7 +38,7 @@
 // the MS was first constructed.  Note that even though the initial log is
 // commonly sent a full minute after startup, the initial log does not include
 // much in the way of user stats.   The most common interlog period (delay)
-// is 5 minutes. That time period starts when the first user action causes a
+// is 20 minutes. That time period starts when the first user action causes a
 // logging event.  This means that if there is no user action, there may be long
 // periods without any (ongoing) log transmissions.  Ongoing log typically
 // contain very detailed records of user activities (ex: opened tab, closed
@@ -122,7 +122,7 @@
 // logs from the current session (see next state).
 //
 //    SENDING_CURRENT_LOGS,   // Sending standard current logs as they accrue.
-// Current logs are being accumulated.  Typically every 5 minutes a log is
+// Current logs are being accumulated.  Typically every 20 minutes a log is
 // closed and finalized for transmission, at the same time as a new log is
 // started.
 //
@@ -198,7 +198,7 @@ static const char kMetricsType[] = "application/vnd.mozilla.metrics.bz2";
 static const int kInitialInterlogDuration = 60;  // one minute
 
 // The default maximum number of events in a log uploaded to the UMA server.
-static const int kInitialEventLimit = 600;
+static const int kInitialEventLimit = 2400;
 
 // If an upload fails, and the transmission was over this byte count, then we
 // will discard the log, and not try to retransmit it.  We also don't persist
@@ -216,7 +216,7 @@ static const int kUnsentLogDelay = 15;  // 15 seconds
 // sending the next log.  If the channel is busy, such as when there is a
 // failure during an attempt to transmit a previous log, then a log may wait
 // (and continue to accrue now log entries) for a much greater period of time.
-static const int kMinSecondsPerLog = 5 * 60;  // five minutes
+static const int kMinSecondsPerLog = 20 * 60;  // twenty minutes
 
 // When we don't succeed at transmitting a log to a server, we progressively
 // wait longer and longer before sending the next log.  This backoff process
@@ -678,10 +678,13 @@ void MetricsService::StopRecording(MetricsLog** log) {
     StartRecording();  // Start trivial log to hold our histograms.
   }
 
-  // Put incremental histogram data at the end of every log transmission.
+  // Put incremental data (histogram deltas, and realtime stats deltas) at the
+  // end of every log transmission.
   // Don't bother if we're going to discard current_log_.
-  if (log)
+  if (log) {
+    current_log_->RecordIncrementalStabilityElements();
     RecordCurrentHistograms();
+  }
 
   current_log_->CloseLog();
   if (log)
@@ -1369,7 +1372,7 @@ bool MetricsService::NodeProbabilityTest(xmlNodePtr node,
   // If a probability is specified in the node, we use it instead.
   xmlChar* probability_value = xmlGetProp(node, BAD_CAST "probability");
   if (probability_value)
-      probability = atoi(reinterpret_cast<char*>(probability_value));
+    probability = atoi(reinterpret_cast<char*>(probability_value));
 
   return ProbabilityTest(probability, props.salt, props.denominator);
 }
@@ -1480,8 +1483,8 @@ void MetricsService::IncrementPrefValue(const wchar_t* path) {
 
 void MetricsService::LogLoadStarted() {
   IncrementPrefValue(prefs::kStabilityPageLoadCount);
-  // We need to save the prefs, as page load count is a critical stat, and
-  // it might be lost due to a crash :-(.
+  // We need to save the prefs, as page load count is a critical stat, and it
+  // might be lost due to a crash :-(.
 }
 
 void MetricsService::LogRendererInSandbox(bool on_sandbox_desktop) {
@@ -1677,10 +1680,9 @@ void MetricsService::RecordCurrentHistograms() {
        histograms.end() != it;
        ++it) {
     if ((*it)->flags() & kUmaTargetedHistogramFlag)
-      // TODO(petersont):
+      // TODO(petersont): Only record historgrams if they are not precluded by
+      // the UMA response data.
       // Bug http://code.google.com/p/chromium/issues/detail?id=2739.
-      // Only record historgrams if they are not
-      // precluded by the UMA response data.
       RecordHistogram(**it);
   }
 }