Bug 1467929: Only collect browser JS errors from recent builds. r=Gijs

Telemetry data suggests between 13%-40% of errors being collected are from
builds older than a week. Since Nightly updates twice a day, errors from builds
that old don't reflect the current state of Nightly, so we can ignore them and
save some bandwidth.

A build is considered "recent" if the date encoded at the start of the
appBuildId (YYYYMMDD) is within 7 days of the current date. Since this is mostly
for preventing high load on the collection service, the check does not handle
problems with the local clock being inaccurate in order to simplify the
implementation.

MozReview-Commit-ID: BbCO4kaBprL

--HG--
extra : rebase_source : 0292ae57272f903a6aef176ef4403d56503fc0db
This commit is contained in:
Michael Kelly 2018-06-12 21:49:58 -07:00
Родитель 91c0407bcc
Коммит d8725aa754
2 изменённых файлов: 86 добавлений и 8 удалений

Просмотреть файл

@ -22,6 +22,7 @@ const PREF_PROJECT_ID = "browser.chrome.errorReporter.projectId";
const PREF_PUBLIC_KEY = "browser.chrome.errorReporter.publicKey";
const PREF_SAMPLE_RATE = "browser.chrome.errorReporter.sampleRate";
const PREF_SUBMIT_URL = "browser.chrome.errorReporter.submitUrl";
const RECENT_BUILD_AGE = 1000 * 60 * 60 * 24 * 7; // 7 days
const SDK_NAME = "firefox-error-reporter";
const SDK_VERSION = "1.0.0";
const TELEMETRY_ERROR_COLLECTED = "browser.errors.collected_count";
@ -83,9 +84,22 @@ const MODULE_SAMPLE_RATES = new Map([
* traces; see bug 1426482 for privacy review and server-side mitigation.
*/
class BrowserErrorReporter {
/**
* Generate a Date object corresponding to the date in the appBuildId.
*/
static getAppBuildIdDate() {
const appBuildId = Services.appinfo.appBuildID;
const buildYear = Number.parseInt(appBuildId.slice(0, 4));
// Date constructor uses 0-indexed months
const buildMonth = Number.parseInt(appBuildId.slice(4, 6)) - 1;
const buildDay = Number.parseInt(appBuildId.slice(6, 8));
return new Date(buildYear, buildMonth, buildDay);
}
constructor(options = {}) {
// Test arguments for mocks and changing behavior
this.fetch = options.fetch || defaultFetch;
this.now = options.now || null;
this.chromeOnly = options.chromeOnly !== undefined ? options.chromeOnly : true;
this.registerListener = (
options.registerListener || (() => Services.console.registerListener(this))
@ -94,6 +108,8 @@ class BrowserErrorReporter {
options.unregisterListener || (() => Services.console.unregisterListener(this))
);
XPCOMUtils.defineLazyGetter(this, "appBuildIdDate", BrowserErrorReporter.getAppBuildIdDate);
// Values that don't change between error reports.
this.requestBodyTemplate = {
logger: "javascript",
@ -198,6 +214,13 @@ class BrowserErrorReporter {
return "FILTERED";
}
isRecentBuild() {
// The local clock is not reliable, but this method doesn't need to be
// perfect.
const now = this.now || new Date();
return (now - this.appBuildIdDate) <= RECENT_BUILD_AGE;
}
observe(message) {
if (message instanceof Ci.nsIScriptError) {
ChromeUtils.idleDispatch(() => this.handleMessage(message));
@ -221,6 +244,11 @@ class BrowserErrorReporter {
Services.telemetry.keyedScalarAdd(TELEMETRY_ERROR_COLLECTED_FILENAME, key.slice(0, 69), 1);
}
// Old builds should not send errors to Sentry
if (!this.isRecentBuild()) {
return;
}
// Sample the amount of errors we send out
let sampleRate = Number.parseFloat(this.sampleRatePref);
for (const [regex, rate] of MODULE_SAMPLE_RATES) {

Просмотреть файл

@ -115,6 +115,7 @@ add_task(async function testInitPastMessages() {
fetch: fetchSpy,
registerListener: noop,
unregisterListener: noop,
now: BrowserErrorReporter.getAppBuildIdDate(),
});
await SpecialPowers.pushPrefEnv({set: [
[PREF_ENABLED, true],
@ -143,6 +144,7 @@ add_task(async function testEnabledPrefWatcher() {
unregisterListener() {
listening = false;
},
now: BrowserErrorReporter.getAppBuildIdDate(),
});
await SpecialPowers.pushPrefEnv({set: [
[PREF_ENABLED, false],
@ -160,7 +162,10 @@ add_task(async function testEnabledPrefWatcher() {
add_task(async function testNonErrorLogs() {
const fetchSpy = sinon.spy();
const reporter = new BrowserErrorReporter({fetch: fetchSpy});
const reporter = new BrowserErrorReporter({
fetch: fetchSpy,
now: BrowserErrorReporter.getAppBuildIdDate(),
});
await SpecialPowers.pushPrefEnv({set: [
[PREF_ENABLED, true],
[PREF_SAMPLE_RATE, "1.0"],
@ -199,7 +204,10 @@ add_task(async function testNonErrorLogs() {
add_task(async function testSampling() {
const fetchSpy = sinon.spy();
const reporter = new BrowserErrorReporter({fetch: fetchSpy});
const reporter = new BrowserErrorReporter({
fetch: fetchSpy,
now: BrowserErrorReporter.getAppBuildIdDate(),
});
await SpecialPowers.pushPrefEnv({set: [
[PREF_ENABLED, true],
[PREF_SAMPLE_RATE, "1.0"],
@ -256,7 +264,10 @@ add_task(async function testSampling() {
add_task(async function testNameMessage() {
const fetchSpy = sinon.spy();
const reporter = new BrowserErrorReporter({fetch: fetchSpy});
const reporter = new BrowserErrorReporter({
fetch: fetchSpy,
now: BrowserErrorReporter.getAppBuildIdDate(),
});
await SpecialPowers.pushPrefEnv({set: [
[PREF_ENABLED, true],
[PREF_SAMPLE_RATE, "1.0"],
@ -305,9 +316,34 @@ add_task(async function testNameMessage() {
);
});
add_task(async function testRecentBuild() {
// Create date that is guaranteed to be a month newer than the build date.
const nowDate = BrowserErrorReporter.getAppBuildIdDate();
nowDate.setMonth(nowDate.getMonth() + 1);
const fetchSpy = sinon.spy();
const reporter = new BrowserErrorReporter({
fetch: fetchSpy,
now: nowDate,
});
await SpecialPowers.pushPrefEnv({set: [
[PREF_ENABLED, true],
[PREF_SAMPLE_RATE, "1.0"],
]});
await reporter.handleMessage(createScriptError({message: "Is error"}));
ok(
!fetchPassedError(fetchSpy, "Is error"),
"Reporter does not collect errors from builds older than a week.",
);
});
add_task(async function testFetchArguments() {
const fetchSpy = sinon.spy();
const reporter = new BrowserErrorReporter({fetch: fetchSpy});
const reporter = new BrowserErrorReporter({
fetch: fetchSpy,
now: BrowserErrorReporter.getAppBuildIdDate(),
});
await SpecialPowers.pushPrefEnv({set: [
[PREF_ENABLED, true],
[PREF_SAMPLE_RATE, "1.0"],
@ -405,7 +441,11 @@ add_task(async function testAddonIDMangle() {
const fetchSpy = sinon.spy();
// Passing false here disables category checks on errors, which would
// otherwise block errors directly from extensions.
const reporter = new BrowserErrorReporter({fetch: fetchSpy, chromeOnly: false});
const reporter = new BrowserErrorReporter({
fetch: fetchSpy,
chromeOnly: false,
now: BrowserErrorReporter.getAppBuildIdDate(),
});
await SpecialPowers.pushPrefEnv({set: [
[PREF_ENABLED, true],
[PREF_SAMPLE_RATE, "1.0"],
@ -447,7 +487,11 @@ add_task(async function testExtensionTag() {
const fetchSpy = sinon.spy();
// Passing false here disables category checks on errors, which would
// otherwise block errors directly from extensions.
const reporter = new BrowserErrorReporter({fetch: fetchSpy, chromeOnly: false});
const reporter = new BrowserErrorReporter({
fetch: fetchSpy,
chromeOnly: false,
now: BrowserErrorReporter.getAppBuildIdDate(),
});
await SpecialPowers.pushPrefEnv({set: [
[PREF_ENABLED, true],
[PREF_SAMPLE_RATE, "1.0"],
@ -488,7 +532,10 @@ add_task(async function testExtensionTag() {
add_task(async function testScalars() {
const fetchStub = sinon.stub();
const reporter = new BrowserErrorReporter({fetch: fetchStub});
const reporter = new BrowserErrorReporter({
fetch: fetchStub,
now: BrowserErrorReporter.getAppBuildIdDate(),
});
await SpecialPowers.pushPrefEnv({set: [
[PREF_ENABLED, true],
[PREF_SAMPLE_RATE, "1.0"],
@ -574,7 +621,10 @@ add_task(async function testScalars() {
add_task(async function testCollectedFilenameScalar() {
const fetchStub = sinon.stub();
const reporter = new BrowserErrorReporter(fetchStub);
const reporter = new BrowserErrorReporter({
fetch: fetchStub,
now: BrowserErrorReporter.getAppBuildIdDate(),
});
await SpecialPowers.pushPrefEnv({set: [
[PREF_ENABLED, true],
[PREF_SAMPLE_RATE, "1.0"],