Bug 1311910 - Add telemetry to measure update error and update timeout rate for V2 and V4. r=francois
MozReview-Commit-ID: JL4aZrUOGH7
--- 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.
*/