Extensions: Add metrics to distinguish between the different kinds of declarative API function call…

extensions/browser/api/declarative/declarative_api.cc

Index: extensions/browser/api/declarative/declarative_api.cc
diff --git a/extensions/browser/api/declarative/declarative_api.cc b/extensions/browser/api/declarative/declarative_api.cc
index 8049813f6c18121e5084c8aa23cc54f4a13e53e0..73fe3728897d6a8907c8bfdd3734c25cbdec19f2 100644
--- a/extensions/browser/api/declarative/declarative_api.cc
+++ b/extensions/browser/api/declarative/declarative_api.cc
@@ -9,7 +9,9 @@
 #include "base/base64.h"
 #include "base/bind.h"
 #include "base/bind_helpers.h"
+#include "base/metrics/histogram_macros.h"
 #include "base/single_thread_task_runner.h"
+#include "base/strings/string_util.h"
 #include "base/task_runner_util.h"
 #include "base/values.h"
 #include "content/public/browser/browser_thread.h"
@@ -35,7 +37,56 @@ namespace extensions {
 
 namespace {
 
-const char kDeclarativeEventPrefix[] = "declarative";
+constexpr char kDeclarativeEventPrefix[] = "declarative";
+constexpr char kDeclarativeContentEventPrefix[] = "declarativeContent.";
+constexpr char kDeclarativeWebRequestEventPrefix[] = "declarativeWebRequest.";
+constexpr char kDeclarativeWebRequestWebViewEventPrefix[] =
+    "webViewInternal.declarativeWebRequest.";
+
+// The type of Declarative API. To collect more granular metrics, a distinction
+// is made when the declarative web request API is used from a webview.
+enum class DeclarativeAPIType {
+  kDeclarativeContent,
+  kDeclarativeWebRequest,
+  kDeclarativeWebRequestWebview,
+  kUnknown,
+};
+
+// Describes the possible types of declarative API function calls.
+// These values are written to logs.  New enum values can be added, but existing
+// enum values must never be renumbered or deleted and reused.
+enum DeclarativeAPIFunctionCallType {
+  kDeclarativeContentAddRules = 0,
+  kDeclarativeContentRemoveRules = 1,
+  kDeclarativeContentGetRules = 2,
+  kDeclarativeWebRequestAddRules = 3,
+  kDeclarativeWebRequestRemoveRules = 4,
+  kDeclarativeWebRequestGetRules = 5,
+  kDeclarativeWebRequestWebviewAddRules = 6,
+  kDeclarativeWebRequestWebviewRemoveRules = 7,
+  kDeclarativeWebRequestWebviewGetRules = 8,
+  kDeclarativeApiFunctionCallTypeMax,
+};
+
+DeclarativeAPIType GetDeclarativeAPITypeFromEventName(

[lazyboy, 2017/05/05 23:11:16]

nit: FromEventName is redundant since it should be obvious from the |event_name|
parameter.

[karandeepb, 2017/05/05 23:35:05]

Done.

+    const std::string& event_name) {
+  if (base::StartsWith(event_name, kDeclarativeContentEventPrefix,
+                       base::CompareCase::SENSITIVE))
+    return DeclarativeAPIType::kDeclarativeContent;
+  if (base::StartsWith(event_name, kDeclarativeWebRequestEventPrefix,
+                       base::CompareCase::SENSITIVE))
+    return DeclarativeAPIType::kDeclarativeWebRequest;
+  if (base::StartsWith(event_name, kDeclarativeWebRequestWebViewEventPrefix,
+                       base::CompareCase::SENSITIVE))
+    return DeclarativeAPIType::kDeclarativeWebRequestWebview;
+  return DeclarativeAPIType::kUnknown;
+}
+
+void RecordUMAHelper(DeclarativeAPIFunctionCallType call_type) {
+  DCHECK_LT(call_type, kDeclarativeApiFunctionCallTypeMax);
+  UMA_HISTOGRAM_ENUMERATION("Extensions.DeclarativeAPIFunctionCalls", call_type,
+                            kDeclarativeApiFunctionCallTypeMax);
+}
 
 void ConvertBinaryDictionaryValuesToBase64(base::DictionaryValue* dict);
 
@@ -125,6 +176,8 @@ bool RulesFunction::RunAsync() {
   EXTENSION_FUNCTION_VALIDATE(args_->GetInteger(1, &web_view_instance_id));
   int embedder_process_id = render_frame_host()->GetProcess()->GetID();
 
+  RecordUMA(event_name);

[lazyboy, 2017/05/05 23:11:16]

Since this requires us to search for 3 prefixes in
GetDeclarativeAPITypeFromEventName...
I wonder if we can somehow avoid generating histogram keys when we don't need
them, i.e. when UMA is not enabled. isherman@ might be able to answer that.

[karandeepb, 2017/05/05 23:35:05]

Acknowledged.

[Ilya Sherman, 2017/05/05 23:45:15]

It's possible, but you should not do this.  We do not want to create performance
incentives for users to opt out of UMA -- we want that decision to be made
purely based on users' privacy preferences.  So, please make sure that your
metrics collection code is sufficiently performant that you'd be perfectly happy
to run it for all Chrome users =)

[lazyboy, 2017/05/05 23:59:12]

Good to know that! Thanks.
The code here isn't a big deal, I was just curious.

+
   bool from_web_view = web_view_instance_id != 0;
   // If we are not operating on a particular <webview>, then the key is 0.
   int rules_registry_id = RulesRegistryService::kDefaultRulesRegistryID;
@@ -189,6 +242,26 @@ bool EventsEventAddRulesFunction::RunAsyncOnCorrectThread() {
   return error_.empty();
 }
 
+void EventsEventAddRulesFunction::RecordUMA(
+    const std::string& event_name) const {
+  DeclarativeAPIFunctionCallType call_type = kDeclarativeApiFunctionCallTypeMax;
+  switch (GetDeclarativeAPITypeFromEventName(event_name)) {
+    case DeclarativeAPIType::kDeclarativeContent:
+      call_type = kDeclarativeContentAddRules;
+      break;
+    case DeclarativeAPIType::kDeclarativeWebRequest:
+      call_type = kDeclarativeWebRequestAddRules;
+      break;
+    case DeclarativeAPIType::kDeclarativeWebRequestWebview:
+      call_type = kDeclarativeWebRequestWebviewAddRules;
+      break;
+    case DeclarativeAPIType::kUnknown:
+      NOTREACHED();
+      return;
+  }
+  RecordUMAHelper(call_type);
+}
+
 bool EventsEventRemoveRulesFunction::RunAsyncOnCorrectThread() {
   std::unique_ptr<RemoveRules::Params> params(
       RemoveRules::Params::Create(*args_));
@@ -204,6 +277,26 @@ bool EventsEventRemoveRulesFunction::RunAsyncOnCorrectThread() {
   return error_.empty();
 }
 
+void EventsEventRemoveRulesFunction::RecordUMA(
+    const std::string& event_name) const {
+  DeclarativeAPIFunctionCallType call_type = kDeclarativeApiFunctionCallTypeMax;
+  switch (GetDeclarativeAPITypeFromEventName(event_name)) {
+    case DeclarativeAPIType::kDeclarativeContent:
+      call_type = kDeclarativeContentRemoveRules;
+      break;
+    case DeclarativeAPIType::kDeclarativeWebRequest:
+      call_type = kDeclarativeWebRequestRemoveRules;
+      break;
+    case DeclarativeAPIType::kDeclarativeWebRequestWebview:
+      call_type = kDeclarativeWebRequestWebviewRemoveRules;
+      break;
+    case DeclarativeAPIType::kUnknown:
+      NOTREACHED();
+      return;
+  }
+  RecordUMAHelper(call_type);
+}
+
 bool EventsEventGetRulesFunction::RunAsyncOnCorrectThread() {
   std::unique_ptr<GetRules::Params> params(GetRules::Params::Create(*args_));
   EXTENSION_FUNCTION_VALIDATE(params.get());
@@ -224,4 +317,24 @@ bool EventsEventGetRulesFunction::RunAsyncOnCorrectThread() {
   return true;
 }
 
+void EventsEventGetRulesFunction::RecordUMA(
+    const std::string& event_name) const {
+  DeclarativeAPIFunctionCallType call_type = kDeclarativeApiFunctionCallTypeMax;
+  switch (GetDeclarativeAPITypeFromEventName(event_name)) {
+    case DeclarativeAPIType::kDeclarativeContent:
+      call_type = kDeclarativeContentGetRules;
+      break;
+    case DeclarativeAPIType::kDeclarativeWebRequest:
+      call_type = kDeclarativeWebRequestGetRules;
+      break;
+    case DeclarativeAPIType::kDeclarativeWebRequestWebview:
+      call_type = kDeclarativeWebRequestWebviewGetRules;
+      break;
+    case DeclarativeAPIType::kUnknown:
+      NOTREACHED();
+      return;
+  }
+  RecordUMAHelper(call_type);
+}
+
 }  // namespace extensions