Google Now Message Center Stats

chrome/browser/notifications/google_now_notification_stats_collector.cc

Index: chrome/browser/notifications/google_now_notification_stats_collector.cc
diff --git a/chrome/browser/notifications/google_now_notification_stats_collector.cc b/chrome/browser/notifications/google_now_notification_stats_collector.cc
new file mode 100644
index 0000000000000000000000000000000000000000..8f6dfebda53aa88505cf6415823f53d90f79160b
--- /dev/null
+++ b/chrome/browser/notifications/google_now_notification_stats_collector.cc
@@ -0,0 +1,72 @@
+// 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.
+
+#include "chrome/browser/notifications/google_now_notification_stats_collector.h"
+
+#include <string>
+
+#include "base/metrics/sparse_histogram.h"
+#include "chrome/browser/browser_process.h"
+#include "chrome/browser/notifications/notification.h"
+#include "chrome/browser/notifications/notification_ui_manager.h"
+#include "content/public/browser/user_metrics.h"
+#include "ui/message_center/notification.h"
+
+namespace {
+const char kChromeNowExtensionID[] = "pafkbggdmjlpgkdkcbjmhmfcdpncadgh";
+}
+
+GoogleNowNotificationStatsCollector::GoogleNowNotificationStatsCollector(
+    message_center::MessageCenter* message_center)
+    : message_center_(message_center) {
+  message_center_->AddObserver(this);
+}
+
+GoogleNowNotificationStatsCollector::~GoogleNowNotificationStatsCollector() {
+  message_center_->RemoveObserver(this);
+}
+
+void GoogleNowNotificationStatsCollector::OnNotificationPoppedUp(
+    const std::string& notification_id) {
+  if (IsNotificationIdForGoogleNow(notification_id)) {
+    content::RecordAction(
+        base::UserMetricsAction(
+            "GoogleNow.MessageCenter.NotificationPoppedUp"));
+  }
+}
+
+void GoogleNowNotificationStatsCollector::OnCenterVisibilityChanged(
+    message_center::Visibility visibility) {
+  if (visibility == message_center::VISIBILITY_MESSAGE_CENTER) {
+    UMA_HISTOGRAM_SPARSE_SLOWLY(

[Ilya Sherman, 2014/05/05 21:37:59]

I don't think you need a sparse histogram for this.  One of the COUNTS
histograms should do nicely.

[robliao, 2014/05/05 22:39:28]

We want to avoid bucketing if at all possible. How should we set up
HISTOGRAM_CUSTOM_COUNTS to do this?
On 2014/05/05 21:37:59, Ilya Sherman wrote:

[Ilya Sherman, 2014/05/05 23:27:36]

Why do you want to avoid bucketing?  What sorts of values are you trying to
measure?  If you're pretty confident that a user would never have more than,
say, 10 notifications, and you only want the corresponding 10 buckets, that's
fine; but in that case you should impose a maximum limit and emit to an overflow
bucket if that limit is exceeded.

[robliao, 2014/05/05 23:33:55]

We're interested in measuring the following specific numbers
0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10+

The 10+ may partially expand if there are a lot of 10+ numbers.

On 2014/05/05 23:27:36, Ilya Sherman wrote:

[Ilya Sherman, 2014/05/05 23:39:07]

That sounds fine.  Please implement that cap.  If you think there's a reasonable
chance that 10+ would someday need to expand, feel free to choose a higher cap. 
If the cap starts getting quite large -- say, past 25 or so -- you'll probably
want to start thinking about exponentially sized buckets anyway.  (Do you really
care to distinguish between 23 and 24 notifications?)

On 2014/05/05 23:33:55, robliao wrote:

[robliao, 2014/05/05 23:45:56]

When crunching stats for review, yes.

To the original question, what's the right way to set this up?
HISTOGRAM_CUSTOM_COUNTS(
    "GoogleNow.MessageCenter.Displayed.NotificationsVisible",
    CountVisibleGoogleNowNotifications(),
    0,
    10,
    12)?

It's not clear based off the API what we're getting when we specify the bucket
count.
Do we get these buckets?
0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10+

On 2014/05/05 23:39:07, Ilya Sherman wrote:

[Ilya Sherman, 2014/05/05 23:53:32]

If you want linear buckets, you might as well keep the sparse histogram (you
could also use a linear histogram, if you prefer; there's not much practical
difference in this case).  Just add code to impose the cap, i.e. to clamp the
logged value.

[robliao, 2014/05/06 00:06:15]

We discovered that our sparse histograms on the UMA site displayed as 23-24. Is
that [23, 24)?

Since we're going sparse, is there harm is not clamping the value?
On 2014/05/05 23:53:32, Ilya Sherman wrote:

[Ilya Sherman, 2014/05/06 00:10:52]

Yep, 'tis.
Yes: The backend does not do well with enormous histograms.  If your logging
code has a bug and somehow ends up logging well above what you *think* is the
maximum, this can slow down the whole pipeline.

[robliao, 2014/05/06 00:17:33]

Okay, so how does this sound then?
UMA_HISTOGRAM_SPARSE_SLOWLY(
  "GoogleNow.MessageCenter.Displayed.NotificationsVisible",
  min(CountVisibleGoogleNowNotifications(), 10);

On 2014/05/06 00:10:52, Ilya Sherman wrote:

[Ilya Sherman, 2014/05/06 00:18:15]

Sounds fine.

[robliao, 2014/05/06 00:25:29]

It's a done deal.
On 2014/05/06 00:18:15, Ilya Sherman wrote:

+        "GoogleNow.MessageCenter.Displayed.NotificationsVisible",
+        CountVisibleGoogleNowNotifications());
+  }
+}
+
+bool GoogleNowNotificationStatsCollector::IsNotificationIdForGoogleNow(
+  const std::string& notification_id) {
+  bool isGoogleNowNotification = false;
+  const Notification* const notification =
+      g_browser_process->notification_ui_manager()->FindById(notification_id);
+  if (notification) {
+    isGoogleNowNotification =
+        (notification->notifier_id().id == kChromeNowExtensionID);
+  }
+  return isGoogleNowNotification;
+}
+
+int GoogleNowNotificationStatsCollector::CountVisibleGoogleNowNotifications() {
+  typedef message_center::NotificationList::Notifications Notifications;
+  const Notifications visible_notifications =
+      message_center_->GetVisibleNotifications();
+  int google_now_notification_count = 0;
+  for (Notifications::iterator iter = visible_notifications.begin();
+      iter != visible_notifications.end();
+      iter++) {

[Ilya Sherman, 2014/05/05 21:37:59]

++iter

[robliao, 2014/05/05 22:39:28]

Done.

+    if ((*iter)->notifier_id().id == kChromeNowExtensionID)
+      google_now_notification_count++;
+  }
+  return google_now_notification_count;
+}