Bug 1311910 - Add telemetry to measure update error and update timeout rate for V2 and V4. r=francois draft
authorDimiL <dlee@mozilla.com>
Mon, 19 Dec 2016 09:43:02 +0800
changeset 450818 efbf72a1e1d8cf00671512d15c7d6b5c52a0fdb3
parent 450805 2824deb82146bba070995a762cfc98ddb2d1decb
child 539835 923e443c7b837d89934100dc43ee954ca92d6e42
push id38955
push userdlee@mozilla.com
push dateMon, 19 Dec 2016 01:44:31 +0000
reviewersfrancois
bugs1311910
milestone53.0a1
Bug 1311910 - Add telemetry to measure update error and update timeout rate for V2 and V4. r=francois MozReview-Commit-ID: JL4aZrUOGH7
toolkit/components/telemetry/Histograms.json
toolkit/components/url-classifier/LookupCacheV4.cpp
toolkit/components/url-classifier/LookupCacheV4.h
toolkit/components/url-classifier/nsUrlClassifierDBService.cpp
toolkit/components/url-classifier/nsUrlClassifierStreamUpdater.cpp
xpcom/base/ErrorList.h
xpcom/base/nsError.h
--- a/toolkit/components/telemetry/Histograms.json
+++ b/toolkit/components/telemetry/Histograms.json
@@ -4140,23 +4140,24 @@
   "URLCLASSIFIER_LC_COMPLETIONS": {
     "alert_emails": ["safebrowsing-telemetry@mozilla.org"],
     "expires_in_version": "never",
     "kind": "exponential",
     "high": 200,
     "n_buckets": 10,
     "description": "Size of the completion cache in entries"
   },
-  "URLCLASSIFIER_UPDATE_REMOTE_STATUS": {
+  "URLCLASSIFIER_UPDATE_REMOTE_STATUS2": {
     "alert_emails": ["safebrowsing-telemetry@mozilla.org"],
     "expires_in_version": "never",
     "kind": "enumerated",
+    "keyed": true,
     "n_values": 16,
-    "bug_numbers": [1150921],
-    "description": "Server HTTP status code from SafeBrowsing database updates. (0=1xx, 1=200, 2=2xx, 3=204, 4=3xx, 5=400, 6=4xx, 7=403, 8=404, 9=408, 10=413, 11=5xx, 12=502|504|511, 13=503, 14=505, 15=Other)"
+    "bug_numbers": [1311910],
+    "description": "Server HTTP status code from SafeBrowsing database updates. (0=1xx, 1=200, 2=2xx, 3=204, 4=3xx, 5=400, 6=4xx, 7=403, 8=404, 9=408, 10=413, 11=5xx, 12=502|504|511, 13=503, 14=505, 15=Other). Keyed by provider"
   },
   "URLCLASSIFIER_COMPLETE_REMOTE_STATUS": {
     "alert_emails": ["safebrowsing-telemetry@mozilla.org"],
     "expires_in_version": "never",
     "kind": "enumerated",
     "n_values": 16,
     "bug_numbers": [1150921],
     "description": "Server HTTP status code from remote SafeBrowsing gethash lookups. (0=1xx, 1=200, 2=2xx, 3=204, 4=3xx, 5=400, 6=4xx, 7=403, 8=404, 9=408, 10=413, 11=5xx, 12=502|504|511, 13=503, 14=505, 15=Other)"
@@ -4171,23 +4172,24 @@
   },
   "URLCLASSIFIER_COMPLETE_TIMEOUT": {
     "alert_emails": ["safebrowsing-telemetry@mozilla.org"],
     "expires_in_version": "56",
     "kind": "boolean",
     "bug_numbers": [1172688],
     "description": "This metric is recorded every time a gethash lookup is performed, `true` is recorded if the lookup times out."
   },
-  "URLCLASSIFIER_UPDATE_ERROR_TYPE": {
+  "URLCLASSIFIER_UPDATE_ERROR": {
     "alert_emails": ["safebrowsing-telemetry@mozilla.org"],
-    "expires_in_version": "58",
-    "kind": "enumerated",
-    "n_values": 10,
-    "bug_numbers": [1305801],
-    "description": "An error was encountered while parsing a partial update returned by a Safe Browsing V4 server (0 = addition of an already existing prefix, 1 = parser got into an infinite loop, 2 = removal index out of bounds, 3 = checksum mismatch, 4 = missing checksum)"
+    "expires_in_version": "59",
+    "kind": "enumerated",
+    "keyed": true,
+    "n_values": 16,
+    "bug_numbers": [1311910],
+    "description": "Whether or not an error was encountered while processing a Safe Browsing update (0 = success, 1 = unspecified error, 2 = addition of an already existing prefix, 3 = parser got into an infinite loop, 4 = removal index out of bounds, 5 = checksum mismatch, 6 = missing checksum). Keyed by provider"
   },
   "URLCLASSIFIER_PREFIX_MATCH": {
     "alert_emails": ["safebrowsing-telemetry@mozilla.org"],
     "expires_in_version": "58",
     "kind": "enumerated",
     "n_values": 4,
     "bug_numbers": [1298257],
     "description": "Classifier prefix matching result (0 = no match, 1 = match only V2, 2 = match only V4, 3 = match both V2 and V4)"
--- a/toolkit/components/url-classifier/LookupCacheV4.cpp
+++ b/toolkit/components/url-classifier/LookupCacheV4.cpp
@@ -226,19 +226,17 @@ LookupCacheV4::ApplyUpdate(TableUpdateV4
     }
 
     bool pickOld;
 
     // If both prefix sets are not empty, then compare to find the smaller one.
     if (!isOldMapEmpty && !isAddMapEmpty) {
       if (smallestOldPrefix == smallestAddPrefix) {
         LOG(("Add prefix should not exist in the original prefix set."));
-        Telemetry::Accumulate(Telemetry::URLCLASSIFIER_UPDATE_ERROR_TYPE,
-                              DUPLICATE_PREFIX);
-        return NS_ERROR_FAILURE;
+        return NS_ERROR_UC_UPDATE_DUPLICATE_PREFIX;
       }
 
       // Compare the smallest string in old prefix set and add prefix set,
       // merge the smaller one into new map to ensure merged string still
       // follows lexigraphic order.
       pickOld = smallestOldPrefix < smallestAddPrefix;
     } else if (!isOldMapEmpty && isAddMapEmpty) {
       pickOld = true;
@@ -269,45 +267,38 @@ LookupCacheV4::ApplyUpdate(TableUpdateV4
       smallestAddPrefix.SetLength(0);
     }
   }
 
   // We expect index will be greater to 0 because max number of runs will be
   // the number of original prefix plus add prefix.
   if (index <= 0) {
     LOG(("There are still prefixes remaining after reaching maximum runs."));
-    Telemetry::Accumulate(Telemetry::URLCLASSIFIER_UPDATE_ERROR_TYPE,
-                          INFINITE_LOOP);
-    return NS_ERROR_FAILURE;
+    return NS_ERROR_UC_UPDATE_INFINITE_LOOP;
   }
 
   if (removalIndex < removalArray.Length()) {
     LOG(("There are still prefixes to remove after exhausting the old PrefixSet."));
-    Telemetry::Accumulate(Telemetry::URLCLASSIFIER_UPDATE_ERROR_TYPE,
-                          WRONG_REMOVAL_INDICES);
-    return NS_ERROR_FAILURE;
+    return NS_ERROR_UC_UPDATE_WRONG_REMOVAL_INDICES;
   }
 
   nsAutoCString checksum;
   crypto->Finish(false, checksum);
   if (aTableUpdate->Checksum().IsEmpty()) {
     LOG(("Update checksum missing."));
-    Telemetry::Accumulate(Telemetry::URLCLASSIFIER_UPDATE_ERROR_TYPE,
-                          MISSING_CHECKSUM);
+    Telemetry::Accumulate(Telemetry::URLCLASSIFIER_UPDATE_ERROR, mProvider,
+        NS_ERROR_GET_CODE(NS_ERROR_UC_UPDATE_MISSING_CHECKSUM));
 
     // Generate our own checksum to tableUpdate to ensure there is always
     // checksum in .metadata
     std::string stdChecksum(checksum.BeginReading(), checksum.Length());
     aTableUpdate->NewChecksum(stdChecksum);
-
   } else if (aTableUpdate->Checksum() != checksum){
     LOG(("Checksum mismatch after applying partial update"));
-    Telemetry::Accumulate(Telemetry::URLCLASSIFIER_UPDATE_ERROR_TYPE,
-                          CHECKSUM_MISMATCH);
-    return NS_ERROR_FAILURE;
+    return NS_ERROR_UC_UPDATE_CHECKSUM_MISMATCH;
   }
 
   return NS_OK;
 }
 
 nsresult
 LookupCacheV4::InitCrypto(nsCOMPtr<nsICryptoHash>& aCrypto)
 {
--- a/toolkit/components/url-classifier/LookupCacheV4.h
+++ b/toolkit/components/url-classifier/LookupCacheV4.h
@@ -48,23 +48,15 @@ protected:
   virtual size_t SizeOfPrefixSet() override;
 
 private:
   virtual int Ver() const override { return VER; }
 
   nsresult InitCrypto(nsCOMPtr<nsICryptoHash>& aCrypto);
   nsresult VerifyChecksum(const nsACString& aChecksum);
 
-  enum UPDATE_ERROR_TYPES {
-    DUPLICATE_PREFIX = 0,
-    INFINITE_LOOP = 1,
-    WRONG_REMOVAL_INDICES = 2,
-    CHECKSUM_MISMATCH = 3,
-    MISSING_CHECKSUM = 4,
-  };
-
   RefPtr<VariableLengthPrefixSet> mVLPrefixSet;
 };
 
 } // namespace safebrowsing
 } // namespace mozilla
 
 #endif
--- a/toolkit/components/url-classifier/nsUrlClassifierDBService.cpp
+++ b/toolkit/components/url-classifier/nsUrlClassifierDBService.cpp
@@ -601,16 +601,32 @@ nsUrlClassifierDBServiceWorker::FinishUp
 
   if (NS_SUCCEEDED(mUpdateStatus)) {
     mUpdateStatus = ApplyUpdate();
   } else {
     LOG(("nsUrlClassifierDBServiceWorker::FinishUpdate() Not running "
          "ApplyUpdate() since the update has already failed."));
   }
 
+  nsCOMPtr<nsIUrlClassifierUtils> urlUtil =
+    do_GetService(NS_URLCLASSIFIERUTILS_CONTRACTID);
+
+  nsCString provider;
+  // Assume that all the tables in update should have the same provider.
+  urlUtil->GetTelemetryProvider(mUpdateTables.SafeElementAt(0, EmptyCString()), provider);
+
+  nsresult updateStatus = mUpdateStatus;
+  if (NS_FAILED(mUpdateStatus)) {
+   updateStatus = NS_ERROR_GET_MODULE(mUpdateStatus) == NS_ERROR_MODULE_URL_CLASSIFIER ?
+     mUpdateStatus : NS_ERROR_UC_UPDATE_UNKNOWN;
+  }
+
+  Telemetry::Accumulate(Telemetry::URLCLASSIFIER_UPDATE_ERROR, provider,
+                        NS_ERROR_GET_CODE(updateStatus));
+
   mMissCache.Clear();
 
   if (NS_SUCCEEDED(mUpdateStatus)) {
     LOG(("Notifying success: %d", mUpdateWaitSec));
     mUpdateObserver->UpdateSuccess(mUpdateWaitSec);
   } else if (NS_ERROR_NOT_IMPLEMENTED == mUpdateStatus) {
     LOG(("Treating NS_ERROR_NOT_IMPLEMENTED a successful update "
          "but still mark it spoiled."));
--- a/toolkit/components/url-classifier/nsUrlClassifierStreamUpdater.cpp
+++ b/toolkit/components/url-classifier/nsUrlClassifierStreamUpdater.cpp
@@ -5,16 +5,17 @@
 
 #include "nsCRT.h"
 #include "nsIHttpChannel.h"
 #include "nsIObserverService.h"
 #include "nsIStringStream.h"
 #include "nsIUploadChannel.h"
 #include "nsIURI.h"
 #include "nsIUrlClassifierDBService.h"
+#include "nsUrlClassifierUtils.h"
 #include "nsNetUtil.h"
 #include "nsStreamUtils.h"
 #include "nsStringStream.h"
 #include "nsToolkitCompsCID.h"
 #include "nsUrlClassifierStreamUpdater.h"
 #include "mozilla/BasePrincipal.h"
 #include "mozilla/ErrorNames.h"
 #include "mozilla/Logging.h"
@@ -638,32 +639,38 @@ nsUrlClassifierStreamUpdater::OnStartReq
       if (NS_SUCCEEDED(rv) && uri) {
         uri->GetAsciiSpec(spec);
       }
       LOG(("nsUrlClassifierStreamUpdater::OnStartRequest "
            "(status=%s, uri=%s, this=%p)", errorName.get(),
            spec.get(), this));
     }
 
+    nsCOMPtr<nsIUrlClassifierUtils> urlUtil =
+      do_GetService(NS_URLCLASSIFIERUTILS_CONTRACTID);
+
+    nsCString provider;
+    urlUtil->GetTelemetryProvider(mStreamTable, provider);
+
     if (NS_FAILED(status)) {
       // Assume we're overloading the server and trigger backoff.
       downloadError = true;
-      mozilla::Telemetry::Accumulate(mozilla::Telemetry::URLCLASSIFIER_UPDATE_REMOTE_STATUS,
-                                     15 /* unknown response code */);
+      mozilla::Telemetry::Accumulate(mozilla::Telemetry::URLCLASSIFIER_UPDATE_REMOTE_STATUS2,
+                                     provider, 15 /* unknown response code */);
 
     } else {
       bool succeeded = false;
       rv = httpChannel->GetRequestSucceeded(&succeeded);
       NS_ENSURE_SUCCESS(rv, rv);
 
       uint32_t requestStatus;
       rv = httpChannel->GetResponseStatus(&requestStatus);
       NS_ENSURE_SUCCESS(rv, rv);
-      mozilla::Telemetry::Accumulate(mozilla::Telemetry::URLCLASSIFIER_UPDATE_REMOTE_STATUS,
-                                     HTTPStatusToBucket(requestStatus));
+      mozilla::Telemetry::Accumulate(mozilla::Telemetry::URLCLASSIFIER_UPDATE_REMOTE_STATUS2,
+                                     provider, HTTPStatusToBucket(requestStatus));
       LOG(("nsUrlClassifierStreamUpdater::OnStartRequest %s (%d)", succeeded ?
            "succeeded" : "failed", requestStatus));
       if (!succeeded) {
         // 404 or other error, pass error status back
         strStatus.AppendInt(requestStatus);
         downloadError = true;
       }
     }
--- a/xpcom/base/ErrorList.h
+++ b/xpcom/base/ErrorList.h
@@ -984,16 +984,28 @@
   ERROR(NS_ERROR_DOM_MEDIA_CDM_ERR,             FAILURE(13)),
   ERROR(NS_ERROR_DOM_MEDIA_NEED_NEW_DECODER,    FAILURE(14)),
 
   /* Internal platform-related errors */
   ERROR(NS_ERROR_DOM_MEDIA_CUBEB_INITIALIZATION_ERR,  FAILURE(101)),
 #undef MODULE
 
   /* ======================================================================= */
+  /* 42: NS_ERROR_MODULE_URL_CLASSIFIER */
+  /* ======================================================================= */
+#define MODULE NS_ERROR_MODULE_URL_CLASSIFIER
+  ERROR(NS_ERROR_UC_UPDATE_UNKNOWN,                 FAILURE(1)),
+  ERROR(NS_ERROR_UC_UPDATE_DUPLICATE_PREFIX,        FAILURE(2)),
+  ERROR(NS_ERROR_UC_UPDATE_INFINITE_LOOP,           FAILURE(3)),
+  ERROR(NS_ERROR_UC_UPDATE_WRONG_REMOVAL_INDICES,   FAILURE(4)),
+  ERROR(NS_ERROR_UC_UPDATE_CHECKSUM_MISMATCH,       FAILURE(5)),
+  ERROR(NS_ERROR_UC_UPDATE_MISSING_CHECKSUM,        FAILURE(6)),
+#undef MODULE
+
+  /* ======================================================================= */
   /* 51: NS_ERROR_MODULE_GENERAL */
   /* ======================================================================= */
 #define MODULE NS_ERROR_MODULE_GENERAL
   /* Error code used internally by the incremental downloader to cancel the
    * network channel when the download is already complete. */
   ERROR(NS_ERROR_DOWNLOAD_COMPLETE,      FAILURE(1)),
   /* Error code used internally by the incremental downloader to cancel the
    * network channel when the response to a range request is 200 instead of
--- a/xpcom/base/nsError.h
+++ b/xpcom/base/nsError.h
@@ -75,16 +75,17 @@
 #define NS_ERROR_MODULE_DOM_FILEHANDLE 34
 #define NS_ERROR_MODULE_SIGNED_JAR 35
 #define NS_ERROR_MODULE_DOM_FILESYSTEM 36
 #define NS_ERROR_MODULE_DOM_BLUETOOTH 37
 #define NS_ERROR_MODULE_SIGNED_APP 38
 #define NS_ERROR_MODULE_DOM_ANIM 39
 #define NS_ERROR_MODULE_DOM_PUSH 40
 #define NS_ERROR_MODULE_DOM_MEDIA 41
+#define NS_ERROR_MODULE_URL_CLASSIFIER 42
 
 /* NS_ERROR_MODULE_GENERAL should be used by modules that do not
  * care if return code values overlap. Callers of methods that
  * return such codes should be aware that they are not
  * globally unique. Implementors should be careful about blindly
  * returning codes from other modules that might also use
  * the generic base.
  */