Bug 1315893 - Add telemetry to measure update time for V2 and V4. r=francois draft
authorThomas Nguyen <tnguyen@mozilla.com>
Thu, 24 Nov 2016 11:13:54 +0800
changeset 443238 6aa4d0fd01afd24e9e45f55a5d45da255962ca92
parent 443220 34fce7c12173bdd6dda54c2ebf6d344252f1ac48
child 537997 03c2c1c5e8e1f0ab3195ecdf06b70293602ba544
push id36932
push usertnguyen@mozilla.com
push dateThu, 24 Nov 2016 03:14:19 +0000
reviewersfrancois
bugs1315893
milestone53.0a1
Bug 1315893 - Add telemetry to measure update time for V2 and V4. r=francois MozReview-Commit-ID: KtrVJXHXMo
toolkit/components/telemetry/Histograms.json
toolkit/components/telemetry/histogram-whitelists.json
toolkit/components/url-classifier/Classifier.cpp
toolkit/components/url-classifier/nsIUrlClassifierUtils.idl
toolkit/components/url-classifier/nsUrlClassifierUtils.cpp
--- a/toolkit/components/telemetry/Histograms.json
+++ b/toolkit/components/telemetry/Histograms.json
@@ -4054,24 +4054,26 @@
   "URLCLASSIFIER_CL_CHECK_TIME": {
     "alert_emails": ["safebrowsing-telemetry@mozilla.org"],
     "expires_in_version": "never",
     "kind": "exponential",
     "high": 500,
     "n_buckets": 10,
     "description": "Time spent per classifier lookup (ms)"
   },
-  "URLCLASSIFIER_CL_UPDATE_TIME": {
+  "URLCLASSIFIER_CL_KEYED_UPDATE_TIME": {
     "alert_emails": ["safebrowsing-telemetry@mozilla.org"],
     "expires_in_version": "never",
+    "keyed": true,
     "kind": "exponential",
     "low": 20,
-    "high": 15000,
-    "n_buckets": 15,
-    "description": "Time spent per classifier update (ms)"
+    "high": 120000,
+    "n_buckets": 30,
+    "bug_numbers": [1315893],
+    "description": "Time spent per classifier update (ms), keyed by the name of the provider."
   },
   "URLCLASSIFIER_PS_FILELOAD_TIME": {
     "alert_emails": ["safebrowsing-telemetry@mozilla.org"],
     "expires_in_version": "never",
     "kind": "exponential",
     "high": 1000,
     "n_buckets": 10,
     "description": "Time spent loading PrefixSet from file (ms)"
--- a/toolkit/components/telemetry/histogram-whitelists.json
+++ b/toolkit/components/telemetry/histogram-whitelists.json
@@ -1665,17 +1665,16 @@
     "UPDATE_STATUS_ERROR_CODE_PARTIAL_STAGE",
     "UPDATE_STATUS_ERROR_CODE_PARTIAL_STARTUP",
     "UPDATE_STATUS_ERROR_CODE_UNKNOWN_STAGE",
     "UPDATE_STATUS_ERROR_CODE_UNKNOWN_STARTUP",
     "UPDATE_UNABLE_TO_APPLY_EXTERNAL",
     "UPDATE_UNABLE_TO_APPLY_NOTIFY",
     "UPDATE_WIZ_LAST_PAGE_CODE",
     "URLCLASSIFIER_CL_CHECK_TIME",
-    "URLCLASSIFIER_CL_UPDATE_TIME",
     "URLCLASSIFIER_LC_COMPLETIONS",
     "URLCLASSIFIER_LC_PREFIXES",
     "URLCLASSIFIER_LOOKUP_TIME",
     "URLCLASSIFIER_PS_CONSTRUCT_TIME",
     "URLCLASSIFIER_PS_FALLOCATE_TIME",
     "URLCLASSIFIER_PS_FILELOAD_TIME",
     "VIDEO_CANPLAYTYPE_H264_CONSTRAINT_SET_FLAG",
     "VIDEO_CANPLAYTYPE_H264_LEVEL",
--- a/toolkit/components/url-classifier/Classifier.cpp
+++ b/toolkit/components/url-classifier/Classifier.cpp
@@ -498,17 +498,29 @@ Classifier::Check(const nsACString& aSpe
   }
 
   return NS_OK;
 }
 
 nsresult
 Classifier::ApplyUpdates(nsTArray<TableUpdate*>* aUpdates)
 {
-  Telemetry::AutoTimer<Telemetry::URLCLASSIFIER_CL_UPDATE_TIME> timer;
+  if (!aUpdates || aUpdates->Length() == 0) {
+    return NS_OK;
+  }
+
+  nsCOMPtr<nsIUrlClassifierUtils> urlUtil =
+    do_GetService(NS_URLCLASSIFIERUTILS_CONTRACTID);
+
+  nsCString provider;
+  // Assume all TableUpdate objects should have the same provider.
+  urlUtil->GetTelemetryProvider((*aUpdates)[0]->TableName(), provider);
+
+  Telemetry::AutoTimer<Telemetry::URLCLASSIFIER_CL_KEYED_UPDATE_TIME>
+    keyedTimer(provider);
 
   PRIntervalTime clockStart = 0;
   if (LOG_ENABLED()) {
     clockStart = PR_IntervalNow();
   }
 
   nsresult rv;
 
--- a/toolkit/components/url-classifier/nsIUrlClassifierUtils.idl
+++ b/toolkit/components/url-classifier/nsIUrlClassifierUtils.idl
@@ -27,16 +27,28 @@ interface nsIUrlClassifierUtils : nsISup
    *
    * @param tableName The table name that we want to lookup
    *
    * @returns the provider name that the given table belongs.
    */
   ACString getProvider(in ACString tableName);
 
   /**
+   * Get the provider used for Telemetry.
+   * Because recording Telemetry will leak user-controlled strings,
+   * only built-in providers should be recorded.
+   *
+   * @param tableName The table name that we want to lookup
+   *
+   * @returns the filtered provider for telemetry.
+   *
+   */
+  ACString getTelemetryProvider(in ACString tableName);
+
+  /**
    * Get the protocol version for the given provider.
    *
    * @param provider String the provider name. e.g. "google"
    *
    * @returns String to indicate the protocol version. e.g. "2.2"
    */
   ACString getProtocolVersion(in ACString provider);
 
--- a/toolkit/components/url-classifier/nsUrlClassifierUtils.cpp
+++ b/toolkit/components/url-classifier/nsUrlClassifierUtils.cpp
@@ -283,16 +283,32 @@ nsUrlClassifierUtils::GetProvider(const 
     aProvider = provider ? *provider : EmptyCString();
   } else {
     aProvider = EmptyCString();
   }
   return NS_OK;
 }
 
 NS_IMETHODIMP
+nsUrlClassifierUtils::GetTelemetryProvider(const nsACString& aTableName,
+                                  nsACString& aProvider)
+{
+  GetProvider(aTableName, aProvider);
+  // Filter out build-in providers: mozilla, google, google4
+  // Empty provider is filtered as "other"
+  if (!NS_LITERAL_CSTRING("mozilla").Equals(aProvider) &&
+      !NS_LITERAL_CSTRING("google").Equals(aProvider) &&
+      !NS_LITERAL_CSTRING("google4").Equals(aProvider)) {
+    aProvider.Assign(NS_LITERAL_CSTRING("other"));
+  }
+
+  return NS_OK;
+}
+
+NS_IMETHODIMP
 nsUrlClassifierUtils::GetProtocolVersion(const nsACString& aProvider,
                                          nsACString& aVersion)
 {
   nsCOMPtr<nsIPrefBranch> prefBranch = do_GetService(NS_PREFSERVICE_CONTRACTID);
   if (prefBranch) {
       nsPrintfCString prefName("browser.safebrowsing.provider.%s.pver",
                                nsCString(aProvider).get());
       nsXPIDLCString version;