Bug 1311910 - Add telemetry to measure update error and update timeout rate for V2 and V4
MozReview-Commit-ID: 5xFbV33l8Q7
* * *
[mq]: 1311910_review
MozReview-Commit-ID: DnMdKs28Ybd
* * *
[mq]: 1311910_review
MozReview-Commit-ID: xl7zG0Zx2L
--- a/toolkit/components/telemetry/Histograms.json
+++ b/toolkit/components/telemetry/Histograms.json
@@ -4130,46 +4130,48 @@
"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",
+ "keyed": true,
"kind": "enumerated",
"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": "Whether or not an error was encountered while processing a Safe Browsing update. (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)"
},
"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": "An error was encountered while 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,37 @@ 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
@@ -131,17 +131,18 @@ nsIThread* nsUrlClassifierDBService::gDb
// Once we've committed to shutting down, don't do work in the background
// thread.
static bool gShuttingDownThread = false;
static mozilla::Atomic<int32_t> gFreshnessGuarantee(CONFIRM_AGE_DEFAULT_SEC);
NS_IMPL_ISUPPORTS(nsUrlClassifierDBServiceWorker,
- nsIUrlClassifierDBService)
+ nsIUrlClassifierDBService,
+ nsIURIClassifierCallback)
nsUrlClassifierDBServiceWorker::nsUrlClassifierDBServiceWorker()
: mInStream(false)
, mGethashNoise(0)
, mPendingLookupLock("nsUrlClassifierDBServerWorker.mPendingLookupLock")
{
}
@@ -346,17 +347,22 @@ nsUrlClassifierDBServiceWorker::AddNoise
result->hash.prefix = noiseEntries[i];
result->mNoise = true;
result->mTableName.Assign(tableName);
}
return NS_OK;
}
-
+NS_IMETHODIMP
+nsUrlClassifierDBServiceWorker::OnClassifyComplete(nsresult aErrorCode)
+{
+ return NS_OK;
+
+}
// Lookup a key in the db.
NS_IMETHODIMP
nsUrlClassifierDBServiceWorker::Lookup(nsIPrincipal* aPrincipal,
const nsACString& aTables,
nsIUrlClassifierCallback* c)
{
if (gShuttingDownThread) {
return NS_ERROR_ABORT;
@@ -601,16 +607,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."));
@@ -1827,16 +1849,29 @@ nsUrlClassifierDBService::Observe(nsISup
// Just read everything again.
ReadTablesFromPrefs();
} else if (NS_LITERAL_STRING(CONFIRM_AGE_PREF).Equals(aData)) {
gFreshnessGuarantee = Preferences::GetInt(CONFIRM_AGE_PREF,
CONFIRM_AGE_DEFAULT_SEC);
}
} else if (!strcmp(aTopic, "quit-application")) {
Shutdown();
+ nsresult rv;
+ nsCOMPtr<nsIScriptSecurityManager> secman =
+ do_GetService(NS_SCRIPTSECURITYMANAGER_CONTRACTID, &rv);
+
+ nsCOMPtr<nsIURI> selfURI;
+ NS_NewURI(getter_AddRefs(selfURI), "http://www.selfuri.com");
+
+ nsCOMPtr<nsIPrincipal> selfURIPrincipal;
+ secman->GetCodebasePrincipal(selfURI, getter_AddRefs(selfURIPrincipal));
+ bool expectCallback;
+ Classify(selfURIPrincipal, false, mWorker,
+ &expectCallback);
+
} else if (!strcmp(aTopic, "profile-before-change")) {
// Unit test does not receive "quit-application",
// need call shutdown in this case
Shutdown();
LOG(("joining background thread"));
mWorkerProxy = nullptr;
nsIThread *backgroundThread = gDbBackgroundThread;
gDbBackgroundThread = nullptr;
--- a/toolkit/components/url-classifier/nsUrlClassifierDBService.h
+++ b/toolkit/components/url-classifier/nsUrlClassifierDBService.h
@@ -135,23 +135,26 @@ private:
// The list of tables that should never be hash completed.
nsTArray<nsCString> mDisallowCompletionsTables;
// Thread that we do the updates on.
static nsIThread* gDbBackgroundThread;
};
-class nsUrlClassifierDBServiceWorker final : public nsIUrlClassifierDBService
+class nsUrlClassifierDBServiceWorker final : public nsIUrlClassifierDBService,
+ public nsIURIClassifierCallback
{
public:
nsUrlClassifierDBServiceWorker();
NS_DECL_THREADSAFE_ISUPPORTS
NS_DECL_NSIURLCLASSIFIERDBSERVICE
+NS_DECL_NSIURICLASSIFIERCALLBACK
+
nsresult Init(uint32_t aGethashNoise, nsCOMPtr<nsIFile> aCacheDir);
// Queue a lookup for the worker to perform, called in the main thread.
// tables is a comma-separated list of tables to query
nsresult QueueLookup(const nsACString& lookupKey,
const nsACString& tables,
nsIUrlClassifierLookupCallback* callback);
--- 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.
*/