Add UMA histograms for policy loading.

chrome/browser/policy/policy_load_status.cc

Index: chrome/browser/policy/policy_load_status.cc
diff --git a/chrome/browser/policy/policy_load_status.cc b/chrome/browser/policy/policy_load_status.cc
new file mode 100644
index 0000000000000000000000000000000000000000..db183e318c7d7a3b872786ae91f04322c53dd81c
--- /dev/null
+++ b/chrome/browser/policy/policy_load_status.cc
@@ -0,0 +1,71 @@
+// Copyright (c) 2013 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.
+
+#include "chrome/browser/policy/policy_load_status.h"
+
+#include "base/metrics/histogram.h"
+#include "base/stringprintf.h"
+#include "chrome/browser/policy/policy_types.h"
+
+namespace policy {
+
+namespace {
+
+// Histogram Name constants.
+const char kHistogramName[] = "Enterprise.PolicyLoadStatus";
+const char kHistogramNameNone[] = "none";
+const char kHistogramNameScopeMachine[] = "machine";
+const char kHistogramNameScopeUser[] = "user";
+const char kHistogramNameLevelMandatory[] = "mandatory";
+const char kHistogramNameLevelRecommended[] = "recommended";
+
+// Gets the name of the histogram to send the sample to.
+std::string GetHistogramName(int scope, int level) {
+  const char* scope_name = kHistogramNameNone;
+  switch (scope) {
+    case POLICY_SCOPE_MACHINE:
+      scope_name = kHistogramNameScopeMachine;
+      break;
+    case POLICY_SCOPE_USER:
+      scope_name = kHistogramNameScopeUser;
+      break;
+  }
+
+  const char* level_name = kHistogramNameNone;
+  switch (level) {
+    case POLICY_LEVEL_MANDATORY:
+      level_name = kHistogramNameLevelMandatory;
+      break;
+    case POLICY_LEVEL_RECOMMENDED:
+      level_name = kHistogramNameLevelRecommended;
+      break;
+  }
+
+  return std::string(kHistogramName) + "_" + scope_name + "_" + level_name;
+}
+
+}  // namespace
+
+PolicyLoadStatusSample::PolicyLoadStatusSample(int scope, int level)

[jar (doing other things), 2013/04/16 19:32:11]

Just to be sure I'm following.... you'll potentially make 9 distinct histograms,
each with about 9 buckets, eh?

I don't like to see too many distinct histograms... but I'm not sure I see a
nice (easy to write) way to do this. You *could* create one histogram with 81
buckets... but you might actually like having it broken down as you currently
set it up.

[Mattias Nissler (ping if slow), 2013/04/17 10:16:35]

Correct, the intent was to distinguish load operations in different contexts.
Note that on a given platform, we only fill 4 distinct histograms at max - the
platform either does loads for a specific scope or level, or the loader loads
all data at once for all scopes and/or levels.
Thinking more about it, there's probably not much reason to have distinct
histograms, at least not for getting basic health stats from the entire
population. I've now cut this down to use only a single histogram; I guess we
can add more complicated sampling if we have specific debugging needs in the
future. Joao, are you cool with that?

+  : histogram_(base::LinearHistogram::FactoryGet(
+        GetHistogramName(scope, level), 0, POLICY_LOAD_STATUS_SIZE,
+        POLICY_LOAD_STATUS_SIZE + 1,
+        base::Histogram::kUmaTargetedHistogramFlag)) {
+  Add(POLICY_LOAD_STATUS_STARTED);
+}
+
+PolicyLoadStatusSample::~PolicyLoadStatusSample() {
+  for (int i = 0; i < POLICY_LOAD_STATUS_SIZE; ++i) {
+    if (status_bits_[i]) {
+      LOG(ERROR) << "sampling " << i;

[jar (doing other things), 2013/04/16 19:32:11]

nit: use DLOG, as this probably won't be helpful to users, and will just clog
most logs.

[Joao da Silva, 2013/04/16 19:32:57]

Please remove this, or make it a VLOG.

[Mattias Nissler (ping if slow), 2013/04/17 10:16:35]

Doh, debugging artifact. Fixed.

+      histogram_->Add(i);

[jar (doing other things), 2013/04/16 19:32:11]

I was curious about why you put this into a bit vector, and then into the
histogram.
Are there cases where the same bit is set multiple times during the life of the
PolicyLoadStatusSample?

[Mattias Nissler (ping if slow), 2013/04/17 10:16:35]

Yes, that's exactly the reason. We may loop over multiple files and I want to
record only one sample even if we encounter an error in more than one file. That
way, we get statistics that indicate the frequency of specific error types
w.r.t. the total number of policy load attempts. For the load attempts, I record
a POLICY_LOAD_STATUS_STARTED sample unconditionally.

+    }
+  }
+}
+
+void PolicyLoadStatusSample::Add(PolicyLoadStatus status) {
+  status_bits_[status] = true;
+}
+
+}  // namespace policy