Bug 1236383 (part 1) - remove and rework some Sync/FxA telemetry probes. r=gfritzsche/kitcambridge

This commit is contained in:
Mark Hammond 2016-01-12 12:21:27 +11:00
parent c6c5eceb06
commit 9e8ad68bca
11 changed files with 49 additions and 198 deletions

View File

@ -395,11 +395,6 @@ TokenServerClient.prototype = {
error.cause = "unknown-service";
}
if (response.status == 401 || response.status == 403) {
Services.telemetry.getKeyedHistogramById(
"TOKENSERVER_AUTH_ERRORS").add(error.cause || "unknown");
}
// A Retry-After header should theoretically only appear on a 503, but
// we'll look for it on any error response.
this._maybeNotifyBackoff(response, "retry-after");

View File

@ -18,17 +18,6 @@ Cu.import("resource://gre/modules/Credentials.jsm");
const HOST = Services.prefs.getCharPref("identity.fxaccounts.auth.uri");
const STATUS_CODE_TO_OTHER_ERRORS_LABEL = {
400: 0,
401: 1,
403: 2,
410: 3,
411: 4,
413: 5,
429: 6,
500: 7,
};
const SIGNIN = "/account/login";
const SIGNUP = "/account/create";
@ -420,17 +409,9 @@ this.FxAccountsClient.prototype = {
"fxaBackoffTimer"
);
}
if (error.errno == ERRNO_UNVERIFIED_ACCOUNT) {
Services.telemetry.getKeyedHistogramById(
"FXA_UNVERIFIED_ACCOUNT_ERRORS").add(path);
} else if (isInvalidTokenError(error)) {
if (isInvalidTokenError(error)) {
Services.telemetry.getKeyedHistogramById(
"FXA_HAWK_ERRORS").add(path);
} else if (error.code >= 500 || error.code in STATUS_CODE_TO_OTHER_ERRORS_LABEL) {
let label = STATUS_CODE_TO_OTHER_ERRORS_LABEL[
error.code >= 500 ? 500 : error.code];
Services.telemetry.getKeyedHistogramById(
"FXA_SERVER_ERRORS").add(path, label);
}
deferred.reject(error);
}

View File

@ -517,6 +517,7 @@ LoginManagerStorage.prototype = {
// next startup.
if (!this._isLoggedIn) {
log.info("not saving credentials to login manager - not logged in");
Services.telemetry.getHistogramById("FXA_SECURE_CREDENTIALS_SAVE_WITH_MP_LOCKED").add(1);
throw new this.STORAGE_LOCKED();
}
// write the data to the login manager.

View File

@ -626,10 +626,7 @@ add_task(function test_accountExists() {
});
add_task(function* test_client_metrics() {
["FXA_UNVERIFIED_ACCOUNT_ERRORS", "FXA_HAWK_ERRORS", "FXA_SERVER_ERRORS"].forEach(name => {
let histogram = Services.telemetry.getKeyedHistogramById(name);
histogram.clear();
});
Services.telemetry.getKeyedHistogramById("FXA_HAWK_ERRORS").clear();
function writeResp(response, msg) {
if (typeof msg === "object") {
@ -637,24 +634,9 @@ add_task(function* test_client_metrics() {
}
response.bodyOutputStream.write(msg, msg.length);
}
function assertSnapshot(name, key, message) {
let histogram = Services.telemetry.getKeyedHistogramById(name);
let snapshot = histogram.snapshot(key);
do_check_eq(snapshot.sum, 1, message);
histogram.clear();
}
let server = httpd_setup(
{
"/account/keys": function(request, response) {
response.setHeader("Content-Type", "application/json; charset=utf-8");
response.setStatusLine(request.httpVersion, 400, "Bad Request");
writeResp(response, {
error: "unverified account",
code: 400,
errno: 104,
});
},
"/session/destroy": function(request, response) {
response.setHeader("Content-Type", "application/json; charset=utf-8");
response.setStatusLine(request.httpVersion, 401, "Unauthorized");
@ -664,48 +646,19 @@ add_task(function* test_client_metrics() {
errno: 111,
});
},
"/recovery_email/status": function(request, response) {
response.setHeader("Content-Type", "application/json; charset=utf-8");
response.setStatusLine(request.httpVersion, 401, "Unauthorized");
writeResp(response, {
error: " invalid request signature",
code: 401,
errno: 109,
});
},
"/recovery_email/resend_code": function(request, response) {
response.setHeader("Content-Type", "text/html");
response.setStatusLine(request.httpVersion, 504, "Sad Server");
writeResp(response, "<!doctype html><title>Simulated proxy error</title>");
},
}
);
let client = new FxAccountsClient(server.baseURI);
yield rejects(client.accountKeys(ACCOUNT_KEYS.keyFetch), function(err) {
return err.errno == 104;
});
assertSnapshot("FXA_UNVERIFIED_ACCOUNT_ERRORS", "/account/keys",
"Should report unverified account errors");
yield rejects(client.signOut(FAKE_SESSION_TOKEN), function(err) {
return err.errno == 111;
});
assertSnapshot("FXA_HAWK_ERRORS", "/session/destroy",
"Should report Hawk authentication errors");
yield rejects(client.recoveryEmailStatus(FAKE_SESSION_TOKEN), function(err) {
return err.errno == 109;
});
assertSnapshot("FXA_SERVER_ERRORS", "/recovery_email/status",
"Should report 400-class errors");
yield rejects(client.resendVerificationEmail(FAKE_SESSION_TOKEN), function(err) {
return err.code == 504;
});
assertSnapshot("FXA_SERVER_ERRORS", "/recovery_email/resend_code",
"Should report 500-class errors");
let histogram = Services.telemetry.getKeyedHistogramById("FXA_HAWK_ERRORS");
let snapshot = histogram.snapshot("/session/destroy");
do_check_eq(snapshot.sum, 1, "Should report Hawk authentication errors");
histogram.clear();
yield deferredStop(server);
});

View File

@ -475,7 +475,6 @@ this.BrowserIDManager.prototype = {
unlockAndVerifyAuthState: function() {
if (this._canFetchKeys()) {
log.debug("unlockAndVerifyAuthState already has (or can fetch) sync keys");
Services.telemetry.getHistogramById("WEAVE_CAN_FETCH_KEYS").add(1);
return Promise.resolve(STATUS_OK);
}
// so no keys - ensure MP unlocked.
@ -494,13 +493,10 @@ this.BrowserIDManager.prototype = {
// lost them - the user will need to reauth before continuing.
let result;
if (this._canFetchKeys()) {
// A ternary would be more compact, but adding 0 to a flag histogram
// crashes the process with a C++ assertion error. See
// FlagHistogram::Accumulate in ipc/chromium/src/base/histogram.cc.
Services.telemetry.getHistogramById("WEAVE_CAN_FETCH_KEYS").add(1);
result = STATUS_OK;
} else {
result = LOGIN_FAILED_LOGIN_REJECTED;
Services.telemetry.getHistogramById("WEAVE_HAS_NO_KEYS_WHEN_UNLOCKED").add(1);
}
log.debug("unlockAndVerifyAuthState re-fetched credentials and is returning", result);
return result;
@ -632,7 +628,6 @@ this.BrowserIDManager.prototype = {
} else if (err.code && err.code === 401) {
err = new AuthenticationError(err);
}
Services.telemetry.getHistogramById("WEAVE_FXA_KEY_FETCH_ERRORS").add();
// TODO: write tests to make sure that different auth error cases are handled here
// properly: auth error getting assertion, auth error getting token (invalid generation
@ -641,6 +636,7 @@ this.BrowserIDManager.prototype = {
this._log.error("Authentication error in _fetchTokenForUser", err);
// set it to the "fatal" LOGIN_FAILED_LOGIN_REJECTED reason.
this._authFailureReason = LOGIN_FAILED_LOGIN_REJECTED;
Services.telemetry.getHistogramById("WEAVE_FXA_KEY_FETCH_AUTH_ERRORS").add(1);
} else {
this._log.error("Non-authentication error in _fetchTokenForUser", err);
// for now assume it is just a transient network related problem

View File

@ -861,9 +861,6 @@ ErrorHandler.prototype = {
break;
case 401:
Services.telemetry.getKeyedHistogramById(
"WEAVE_STORAGE_AUTH_ERRORS").add(cause);
this.service.logout();
this._log.info("Got 401 response; resetting clusterURL.");
Svc.Prefs.reset("clusterURL");

View File

@ -50,20 +50,6 @@ const STORAGE_INFO_TYPES = [INFO_COLLECTIONS,
INFO_COLLECTION_COUNTS,
INFO_QUOTA];
// A structure mapping a (boolean) telemetry probe name to a preference name.
// The probe will record true if the pref is modified, false otherwise.
const TELEMETRY_CUSTOM_SERVER_PREFS = {
WEAVE_CUSTOM_LEGACY_SERVER_CONFIGURATION: "services.sync.serverURL",
WEAVE_CUSTOM_FXA_SERVER_CONFIGURATION: "identity.fxaccounts.auth.uri",
WEAVE_CUSTOM_TOKEN_SERVER_CONFIGURATION: [
// The new prefname we use for the tokenserver URI.
"identity.sync.tokenserver.uri",
// The old deprecated prefname we previously used for the tokenserver URI.
"services.sync.tokenServerURI",
],
};
function Sync11Service() {
this._notify = Utils.notify("weave:service:");
}
@ -385,13 +371,6 @@ Sync11Service.prototype = {
Svc.Obs.notify("weave:engine:start-tracking");
}
// Telemetry probes to indicate if the user is using custom servers.
for (let [probeName, prefName] of Iterator(TELEMETRY_CUSTOM_SERVER_PREFS)) {
let prefNames = Array.isArray(prefName) ? prefName : [prefName];
let isCustomized = prefNames.some(pref => Services.prefs.prefHasUserValue(pref));
Services.telemetry.getHistogramById(probeName).add(isCustomized);
}
// Send an event now that Weave service is ready. We don't do this
// synchronously so that observers can import this module before
// registering an observer.
@ -648,8 +627,6 @@ Sync11Service.prototype = {
// One kind of exception: HMAC failure.
if (Utils.isHMACMismatch(ex)) {
Services.telemetry.getHistogramById(
"WEAVE_HMAC_ERRORS").add();
this.status.login = LOGIN_FAILED_INVALID_PASSPHRASE;
this.status.sync = CREDENTIALS_CHANGED;
}
@ -756,8 +733,6 @@ Sync11Service.prototype = {
case 401:
this._log.warn("401: login failed.");
Services.telemetry.getKeyedHistogramById(
"WEAVE_STORAGE_AUTH_ERRORS").add("info/collections");
// Fall through to the 404 case.
case 404:

View File

@ -410,7 +410,7 @@ add_task(function test_getTokenErrors() {
"should reject due to 401");
Assert.equal(Status.login, LOGIN_FAILED_LOGIN_REJECTED, "login was rejected");
let keyFetchErrorCount = sumHistogram("WEAVE_FXA_KEY_FETCH_ERRORS");
let keyFetchErrorCount = sumHistogram("WEAVE_FXA_KEY_FETCH_AUTH_ERRORS");
Assert.equal(keyFetchErrorCount, 1, "Should record key fetch error for rejected logins");
// XXX - other interesting responses to return?
@ -429,8 +429,8 @@ add_task(function test_getTokenErrors() {
"should reject due to non-JSON response");
Assert.equal(Status.login, LOGIN_FAILED_NETWORK_ERROR, "login state is LOGIN_FAILED_NETWORK_ERROR");
keyFetchErrorCount = sumHistogram("WEAVE_FXA_KEY_FETCH_ERRORS");
Assert.equal(keyFetchErrorCount, 1, "Should record key fetch errors for invalid responses");
keyFetchErrorCount = sumHistogram("WEAVE_FXA_KEY_FETCH_AUTH_ERRORS");
Assert.equal(keyFetchErrorCount, 0, "Should not record key fetch errors for invalid responses");
});
add_task(function test_getTokenErrorWithRetry() {
@ -457,8 +457,8 @@ add_task(function test_getTokenErrorWithRetry() {
// Sync will have the value in ms with some slop - so check it is at least that.
Assert.ok(Status.backoffInterval >= 100000);
let keyFetchErrorCount = sumHistogram("WEAVE_FXA_KEY_FETCH_ERRORS");
Assert.equal(keyFetchErrorCount, 1, "Should record key fetch errors for 503 from FxA");
let keyFetchErrorCount = sumHistogram("WEAVE_FXA_KEY_FETCH_AUTH_ERRORS");
Assert.equal(keyFetchErrorCount, 0, "Should not record key fetch errors for 503 from FxA");
_("Arrange for a 200 with an X-Backoff header.");
Status.backoffInterval = 0;
@ -477,8 +477,8 @@ add_task(function test_getTokenErrorWithRetry() {
// The observer should have fired - check it got the value in the response.
Assert.ok(Status.backoffInterval >= 200000);
keyFetchErrorCount = sumHistogram("WEAVE_FXA_KEY_FETCH_ERRORS");
Assert.equal(keyFetchErrorCount, 1, "Should record key fetch errors for backoff response from FxA");
keyFetchErrorCount = sumHistogram("WEAVE_FXA_KEY_FETCH_AUTH_ERRORS");
Assert.equal(keyFetchErrorCount, 0, "Should not record key fetch errors for 503/backoff response from FxA");
});
add_task(function test_getKeysErrorWithBackoff() {
@ -514,8 +514,8 @@ add_task(function test_getKeysErrorWithBackoff() {
// Sync will have the value in ms with some slop - so check it is at least that.
Assert.ok(Status.backoffInterval >= 100000);
let keyFetchErrorCount = sumHistogram("WEAVE_FXA_KEY_FETCH_ERRORS");
Assert.equal(keyFetchErrorCount, 1, "Should record key fetch errors for 503 from FxA");
let keyFetchErrorCount = sumHistogram("WEAVE_FXA_KEY_FETCH_AUTH_ERRORS");
Assert.equal(keyFetchErrorCount, 0, "Should not record key fetch errors for 503 from FxA");
});
add_task(function test_getKeysErrorWithRetry() {
@ -551,8 +551,8 @@ add_task(function test_getKeysErrorWithRetry() {
// Sync will have the value in ms with some slop - so check it is at least that.
Assert.ok(Status.backoffInterval >= 100000);
let keyFetchErrorCount = sumHistogram("WEAVE_FXA_KEY_FETCH_ERRORS");
Assert.equal(keyFetchErrorCount, 1, "Should record key fetch errors for 503 from FxA");
let keyFetchErrorCount = sumHistogram("WEAVE_FXA_KEY_FETCH_AUTH_ERRORS");
Assert.equal(keyFetchErrorCount, 0, "Should not record key fetch errors for 503 from FxA");
});
add_task(function test_getHAWKErrors() {
@ -571,7 +571,7 @@ add_task(function test_getHAWKErrors() {
});
Assert.equal(Status.login, LOGIN_FAILED_LOGIN_REJECTED, "login was rejected");
let keyFetchErrorCount = sumHistogram("WEAVE_FXA_KEY_FETCH_ERRORS");
let keyFetchErrorCount = sumHistogram("WEAVE_FXA_KEY_FETCH_AUTH_ERRORS");
Assert.equal(keyFetchErrorCount, 1, "Should record key fetch errors for 401 from FxA");
// XXX - other interesting responses to return?
@ -590,8 +590,8 @@ add_task(function test_getHAWKErrors() {
});
Assert.equal(Status.login, LOGIN_FAILED_NETWORK_ERROR, "login state is LOGIN_FAILED_NETWORK_ERROR");
keyFetchErrorCount = sumHistogram("WEAVE_FXA_KEY_FETCH_ERRORS");
Assert.equal(keyFetchErrorCount, 1, "Should record key fetch errors for invalid response from FxA");
keyFetchErrorCount = sumHistogram("WEAVE_FXA_KEY_FETCH_AUTH_ERRORS");
Assert.equal(keyFetchErrorCount, 0, "Should not record key fetch errors for invalid response from FxA");
});
add_task(function test_getGetKeysFailing401() {
@ -614,7 +614,7 @@ add_task(function test_getGetKeysFailing401() {
});
Assert.equal(Status.login, LOGIN_FAILED_LOGIN_REJECTED, "login was rejected");
let keyFetchErrorCount = sumHistogram("WEAVE_FXA_KEY_FETCH_ERRORS");
let keyFetchErrorCount = sumHistogram("WEAVE_FXA_KEY_FETCH_AUTH_ERRORS");
Assert.equal(keyFetchErrorCount, 1, "Should record key fetch errors for 401 from FxA");
});
@ -638,8 +638,8 @@ add_task(function test_getGetKeysFailing503() {
});
Assert.equal(Status.login, LOGIN_FAILED_NETWORK_ERROR, "state reflects network error");
let keyFetchErrorCount = sumHistogram("WEAVE_FXA_KEY_FETCH_ERRORS");
Assert.equal(keyFetchErrorCount, 1, "Should record key fetch errors for 503 from FxA");
let keyFetchErrorCount = sumHistogram("WEAVE_FXA_KEY_FETCH_AUTH_ERRORS");
Assert.equal(keyFetchErrorCount, 0, "Should not record key fetch errors for 503 from FxA");
});
add_task(function test_getKeysMissing() {
@ -730,8 +730,7 @@ add_task(function* test_signedInUserMissing() {
let status = yield browseridManager.unlockAndVerifyAuthState();
Assert.equal(status, LOGIN_FAILED_LOGIN_REJECTED);
let canFetchKeysCount = sumHistogram("WEAVE_CAN_FETCH_KEYS");
Assert.equal(canFetchKeysCount, 0);
Assert.equal(sumHistogram("WEAVE_HAS_NO_KEYS_WHEN_UNLOCKED"), 1);
});
// End of tests

View File

@ -181,9 +181,6 @@ add_identity_test(this, function test_401_logout() {
_("Got weave:service:login:error in second sync.");
Svc.Obs.remove("weave:service:login:error", onLoginError);
let errorCount = sumHistogram("WEAVE_STORAGE_AUTH_ERRORS", { key: "info/collections" });
do_check_eq(errorCount, 2);
let expected = isConfiguredWithLegacyIdentity() ?
LOGIN_FAILED_LOGIN_REJECTED : LOGIN_FAILED_NETWORK_ERROR;

View File

@ -160,10 +160,6 @@ function run_test() {
do_check_false(Service.verifyAndFetchSymmetricKeys());
do_check_eq(Service.status.login, LOGIN_FAILED_INVALID_PASSPHRASE);
let hmacErrors = sumHistogram("WEAVE_HMAC_ERRORS");
do_check_eq(hmacErrors, 1);
} finally {
Svc.Prefs.resetBranch("");
server.stop(do_test_finished);

View File

@ -8060,21 +8060,6 @@
"n_buckets": 10,
"description": "The number of times a sync successfully completed in this session"
},
"WEAVE_CUSTOM_LEGACY_SERVER_CONFIGURATION": {
"expires_in_version": "default",
"kind": "boolean",
"description": "Whether legacy Sync is configured to use a custom server"
},
"WEAVE_CUSTOM_FXA_SERVER_CONFIGURATION": {
"expires_in_version": "default",
"kind": "boolean",
"description": "Whether FxA Sync is configured to use a custom authentication server"
},
"WEAVE_CUSTOM_TOKEN_SERVER_CONFIGURATION": {
"expires_in_version": "50",
"kind": "boolean",
"description": "Whether FxA Sync is configured to use a custom token server"
},
"WEBCRYPTO_EXTRACTABLE_IMPORT": {
"expires_in_version": "never",
"kind": "boolean",
@ -9640,43 +9625,29 @@
},
"FXA_CONFIGURED": {
"alert_emails": ["fx-team@mozilla.com"],
"expires_in_version": "47",
"bug_numbers": [1236383],
"expires_in_version": "never",
"kind": "flag",
"releaseChannelCollection": "opt-out",
"description": "If the user is signed in to a Firefox Account on this device. Recorded once per session just after startup as Sync is initialized."
},
"FXA_UNVERIFIED_ACCOUNT_ERRORS": {
"FXA_SECURE_CREDENTIALS_SAVE_WITH_MP_LOCKED": {
"alert_emails": ["fx-team@mozilla.com"],
"expires_in_version": "47",
"bug_numbers": [1236383],
"expires_in_version": "49",
"kind": "count",
"keyed": true,
"releaseChannelCollection": "opt-out",
"description": "FxA key fetch and certificate signing errors caused by an unverified account. Keyed on the FxA auth server endpoint."
"description": "The number of times FxA had credentials to save in the login manager but the master-password was locked, implying they may lose those credentials after a restart."
},
"FXA_HAWK_ERRORS": {
"alert_emails": ["fx-team@mozilla.com"],
"expires_in_version": "47",
"bug_numbers": [1236383],
"expires_in_version": "49",
"kind": "count",
"keyed": true,
"releaseChannelCollection": "opt-out",
"description": "FxA error responses caused by invalid Hawk credentials. Keyed on the FxA auth server endpoint."
},
"FXA_SERVER_ERRORS": {
"alert_emails": ["fx-team@mozilla.com"],
"expires_in_version": "47",
"kind": "count",
"keyed": true,
"releaseChannelCollection": "opt-out",
"description": "400 and 500-class server errors returned by the FxA auth server. Keyed on the endpoint."
},
"TOKENSERVER_AUTH_ERRORS": {
"alert_emails": ["fx-team@mozilla.com"],
"expires_in_version": "47",
"kind": "count",
"keyed": true,
"releaseChannelCollection": "opt-out",
"description": "Token server errors caused by invalid BrowserID assertions. Keyed on the token server error cause."
},
"WEAVE_DEVICE_COUNT_DESKTOP": {
"alert_emails": ["fx-team@mozilla.com"],
"bug_numbers": [1232050],
@ -9697,7 +9668,8 @@
},
"WEAVE_ENGINE_APPLY_NEW_FAILURES": {
"alert_emails": ["fx-team@mozilla.com"],
"expires_in_version": "47",
"bug_numbers": [1236383],
"expires_in_version": "49",
"kind": "count",
"keyed": true,
"releaseChannelCollection": "opt-out",
@ -9705,7 +9677,8 @@
},
"WEAVE_ENGINE_APPLY_FAILURES": {
"alert_emails": ["fx-team@mozilla.com"],
"expires_in_version": "47",
"bug_numbers": [1236383],
"expires_in_version": "49",
"kind": "count",
"keyed": true,
"releaseChannelCollection": "opt-out",
@ -9713,40 +9686,28 @@
},
"WEAVE_ENGINE_SYNC_ERRORS": {
"alert_emails": ["fx-team@mozilla.com"],
"expires_in_version": "47",
"bug_numbers": [1236383],
"expires_in_version": "never",
"kind": "count",
"keyed": true,
"releaseChannelCollection": "opt-out",
"description": "Exceptions thrown by a Sync engine. Keyed on the engine name."
},
"WEAVE_CAN_FETCH_KEYS": {
"WEAVE_HAS_NO_KEYS_WHEN_UNLOCKED": {
"alert_emails": ["fx-team@mozilla.com"],
"expires_in_version": "47",
"kind": "flag",
"releaseChannelCollection": "opt-out",
"description": "Whether Sync keys are present in account storage."
},
"WEAVE_FXA_KEY_FETCH_ERRORS": {
"alert_emails": ["fx-team@mozilla.com"],
"expires_in_version": "47",
"bug_numbers": [1236383],
"expires_in_version": "49",
"kind": "count",
"releaseChannelCollection": "opt-out",
"description": "Errors encountered fetching Sync keys, including network errors."
"description": "Records how often Sync doesn't have the necessary keys when the master-password is unlocked."
},
"WEAVE_STORAGE_AUTH_ERRORS": {
"WEAVE_FXA_KEY_FETCH_AUTH_ERRORS": {
"alert_emails": ["fx-team@mozilla.com"],
"expires_in_version": "47",
"kind": "count",
"keyed": true,
"releaseChannelCollection": "opt-out",
"description": "Sync storage server authentication errors. Keyed on the Sync record name."
},
"WEAVE_HMAC_ERRORS": {
"alert_emails": ["fx-team@mozilla.com"],
"expires_in_version": "47",
"bug_numbers": [1236383],
"expires_in_version": "49",
"kind": "count",
"releaseChannelCollection": "opt-out",
"description": "Sync cryptoKeys collection HMAC mismatches."
"description": "Authentication errors encountered fetching Sync keys."
},
"CONTENT_DOCUMENTS_DESTROYED": {
"expires_in_version": "never",