commit 674c318a84e4344fce0fdaee039784ed7900d188
author David Zeuthen <zeuthen@chromium.org> Thu Apr 18 14:05:20 2013 -0700
committer ChromeBot <chrome-bot@google.com> Tue Apr 23 10:39:14 2013 -0700
tree 0dd67f9149ed031304c310b3668648c9b5cafa66
parent be45bef9e283188b00e7def8967f81843669a7f1

Use better histogram parameters for duration metrics

In the current code this is capped at 20 minutes which is not helpful
as we're interested in data for devices that take much longer to
update. Instead, use one year for UpdateDuration and one month for
UpdateDurationUptime. Also move the code that sends the metric into
PayloadState to better conform with newly added code.

Also rename the metrics to they have the suffix Minutes. E.g. the new
metric names are

 Installer.UpdateDurationMinutes
 Installer.UpdateDurationUptimeMinutes

Also add unit test for utils::FormatTimeDelta().

BUG=None
TEST=Unit tests, look at chrome://histogram

Change-Id: I8581ec111f9833bdd72f10888c9626f40779f8b9
Reviewed-on: https://gerrit.chromium.org/gerrit/48558
Commit-Queue: David Zeuthen <zeuthen@chromium.org>
Reviewed-by: David Zeuthen <zeuthen@chromium.org>
Tested-by: David Zeuthen <zeuthen@chromium.org>

payload_state.cc

diff --git a/payload_state.cc b/payload_state.cc
index bae21b9..77652a7 100644
--- a/payload_state.cc
+++ b/payload_state.cc
@@ -143,6 +143,7 @@
   ReportBytesDownloadedMetrics();
   ReportUpdateUrlSwitchesMetric();
   ReportRebootMetrics();
+  ReportDurationMetrics();
 }
 
 void PayloadState::UpdateFailed(ActionExitCode error) {
@@ -668,8 +669,8 @@
     LOG(ERROR) << "The UpdateTimestampStart value ("
                << utils::ToString(stored_time)
                << ") in persisted state is "
-               << duration_according_to_stored_time.InSeconds()
-               << " seconds in the future. Resetting.";
+               << utils::FormatTimeDelta(duration_according_to_stored_time)
+               << " in the future. Resetting.";
     stored_time = now;
   }
 
@@ -717,10 +718,10 @@
   TimeDelta diff = GetUpdateDuration() - stored_delta;
   if (diff < -kDurationSlack) {
     LOG(ERROR) << "The UpdateDurationUptime value ("
-               << stored_delta.InSeconds() << " seconds"
+               << utils::FormatTimeDelta(stored_delta)
                << ") in persisted state is "
-               << diff.InSeconds()
-               << " seconds larger than the wall-clock delta. Resetting.";
+               << utils::FormatTimeDelta(diff)
+               << " larger than the wall-clock delta. Resetting.";
     stored_delta = update_duration_current_;
   }
 
@@ -741,8 +742,7 @@
                    update_duration_uptime_.ToInternalValue());
   if (use_logging) {
     LOG(INFO) << "Update Duration Uptime = "
-              << update_duration_uptime_.InSeconds()
-              << " seconds";
+              << utils::FormatTimeDelta(update_duration_uptime_);
   }
 }
 
@@ -758,6 +758,35 @@
   SetUpdateDurationUptimeExtended(new_uptime, now, false);
 }
 
+void PayloadState::ReportDurationMetrics() {
+  TimeDelta duration = GetUpdateDuration();
+  TimeDelta duration_uptime = GetUpdateDurationUptime();
+  string metric;
+
+  metric = "Installer.UpdateDurationMinutes";
+  system_state_->metrics_lib()->SendToUMA(
+       metric,
+       static_cast<int>(duration.InMinutes()),
+       1,             // min: 1 minute
+       365*24*60,     // max: 1 year (approx)
+       kNumDefaultUmaBuckets);
+  LOG(INFO) << "Uploading " << utils::FormatTimeDelta(duration)
+            << " for metric " <<  metric;
+
+  metric = "Installer.UpdateDurationUptimeMinutes";
+  system_state_->metrics_lib()->SendToUMA(
+       metric,
+       static_cast<int>(duration_uptime.InMinutes()),
+       1,             // min: 1 minute
+       30*24*60,      // max: 1 month (approx)
+       kNumDefaultUmaBuckets);
+  LOG(INFO) << "Uploading " << utils::FormatTimeDelta(duration_uptime)
+            << " for metric " <<  metric;
+
+  prefs_->Delete(kPrefsUpdateTimestampStart);
+  prefs_->Delete(kPrefsUpdateDurationUptime);
+}
+
 string PayloadState::GetPrefsKey(const string& prefix, DownloadSource source) {
   return prefix + "-from-" + utils::ToString(source);
 }