Added UMA for usage of BudgetAPI calls.

chrome/browser/budget_service/budget_manager.cc

 // Copyright 2016 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/budget_service/budget_manager.h"
 
 #include <stdint.h>
 
 #include "base/callback.h"
 #include "base/memory/ptr_util.h"
@@ skipping 56 matching lines @@
   return SiteEngagementScore::kMaxPoints + 1.0;
 }
 
 void BudgetManager::GetBudget(const url::Origin& origin,
                               const GetBudgetCallback& callback) {
   if (origin.unique() || !content::IsOriginSecure(origin.GetURL())) {
     callback.Run(blink::mojom::BudgetServiceErrorType::NOT_SUPPORTED,
                  mojo::Array<blink::mojom::BudgetStatePtr>());
     return;
   }
-  db_.GetBudgetDetails(origin, callback);
+  db_.GetBudgetDetails(origin,
+                       base::Bind(&BudgetManager::DidGetBudget,
+                                  weak_ptr_factory_.GetWeakPtr(), callback));
 }
 
 void BudgetManager::Reserve(const url::Origin& origin,
                             blink::mojom::BudgetOperationType type,
                             const ReserveCallback& callback) {
   if (origin.unique() || !content::IsOriginSecure(origin.GetURL())) {
     callback.Run(blink::mojom::BudgetServiceErrorType::NOT_SUPPORTED,
                  false /* success */);
     return;
   }
@@ skipping 27 matching lines @@
     return;
   }
 
   // If there wasn't a reservation already, try to directly consume budget.
   // The callback will return directly to the caller.
   db_.SpendBudget(origin, GetCost(type),
                   base::Bind(&BudgetManager::DidConsume,
                              weak_ptr_factory_.GetWeakPtr(), callback));
 }
 
+void BudgetManager::DidGetBudget(
+    const GetBudgetCallback& callback,
+    const blink::mojom::BudgetServiceErrorType error,
+    mojo::Array<blink::mojom::BudgetStatePtr> budget) {
+  // If there was an error, just record a budget of 0, so the API query is still
+  // counted.
+  if (error != blink::mojom::BudgetServiceErrorType::NONE)
+    UMA_HISTOGRAM_COUNTS_100("PushMessaging.BudgetAPIQuery", 0);
+  else
+    UMA_HISTOGRAM_COUNTS_100("PushMessaging.BudgetAPIQuery",
+                             budget[0]->budget_at);

[Peter Beverloo, 2016/11/28 10:59:00]

What is the added value of recording the immediate available budget here? I.e.
how do you plan to use it?

I would personally just make this a UMA_HISTOGRAM_BOOLEAN() histogram as well,
where the boolean is the condition on line 133.

[harkness, 2016/11/28 13:58:04]

I'd like to have an idea of whether the people who are using the API are using
up their budget, so they're constantly checking their budget and finding that
they're running out, vs people checking budget and having plenty.

[Peter Beverloo, 2016/11/28 14:14:06]

But since UMA gives us aggregated data, we have no way to check the chronology
of the calls, nor cross-reference them with the origins for which they're
called. That effectively gives us a boolean [0, >0], right? What insight does
the granularity of having 50 buckets give us?

[harkness, 2016/11/28 16:09:09]

My gut feel for the API is that it will be used by two types of consumers. One
is the consumer who is very aggressive about using up their budget and will be
constantly bouncing off 0 budget. The other is the user who only very rarely
either checks budget or uses budget.  

So what I expect to see is a distribution of budget in this histogram that
corresponds closely to the SES score of service workers who query budget. Then,
I expect to see a big spike of usage in the 0-10 range, which I would guess will
come from 1% of origins that uses the feature a lot.

Unfortunately, you're right that if the histogram doesn't match what I expect,
I'm not sure what conclusions we'll be able to draw from it. If you feel that's
enough reason to restrict it to just the count, I can do that, but it didn't
seem like there was a significant harm to having the extra info.

[Peter Beverloo, 2016/11/28 18:22:04]

We have three variables for values in this histogram:

  1) The amount of engagement a user has with a website. The highest engaged
site for a particular user definitely doesn't have to be a 100.
  2) Whether that particular website checks their budget.
  3) Whether that particular website *uses* their budget.

This histogram only knows (2). We cannot associate any of this with a particular
user, their engagement and/or current budget, or origin. As such, I can't really
come up with any valid conclusion we can make based on the data.

Now, you're right that the cost of having a histogram is reasonably low. It is a
situation of death by a thousand needles though, so we should have reasonable
confidence that a histogram is going to get us good data.

If I've made a logic error somewhere and you feel we can get good data, let's do
it, but please clarify what it is that we'll get in histograms.xml.

[harkness, 2016/11/29 15:27:21]

Discussed in person, summarizing here. The budget in this histogram is of
limited use by itself, but might be useful in combination with other histograms.
It might not, but I would rather have the data and not use it than need it and
not have it.

Peter was also concerned that the data might be misinterpreted, so I've added
more to the comment in histograms.xml to clarify the fuzziness of the metric.

To be clear, we agree completely on having the histogram, because the count of
queries is very useful, it's just whether the count is a raw count or a
histogram of budget that we disagreed on.

+
+  callback.Run(error, std::move(budget));
+}
+
 void BudgetManager::DidConsume(const ConsumeCallback& callback,
                                blink::mojom::BudgetServiceErrorType error,
                                bool success) {
   // The caller of Consume only cares whether it succeeded or failed and not
   // why. So, only return a combined bool.
   if (error != blink::mojom::BudgetServiceErrorType::NONE) {
     callback.Run(false /* success */);
     return;
   }
   callback.Run(success);
 }
 
 void BudgetManager::DidReserve(const url::Origin& origin,
                                const ReserveCallback& callback,
                                blink::mojom::BudgetServiceErrorType error,
                                bool success) {
   // If the call succeeded, write the new reservation into the map.
   if (success && error == blink::mojom::BudgetServiceErrorType::NONE)
     reservation_map_[origin]++;
 
+  UMA_HISTOGRAM_BOOLEAN("PushMessaging.BudgetAPIReserve", success);
+
   callback.Run(error, success);
 }