Backed out changeset 68e32f5b50e7 (bug 1444554) for failing its own test. r=backout
authorKris Maglione <maglione.k@gmail.com>
Tue, 20 Mar 2018 20:31:57 -0700
changeset 462703 9f016656fb7439ff9eca989e1c119c8ebd12af2f
parent 462702 c2d1f86caeaf0a7dd9356bae8d5d6fe059d29c30
child 462704 a95c9add03af42a289d00dda46bb021f51930254
push id9165
push userasasaki@mozilla.com
push dateThu, 26 Apr 2018 21:04:54 +0000
treeherdermozilla-beta@064c3804de2e [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersbackout
bugs1444554
milestone61.0a1
backs out68e32f5b50e77717939fbf4f7a96bda0eaa2561b
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Backed out changeset 68e32f5b50e7 (bug 1444554) for failing its own test. r=backout MozReview-Commit-ID: C6TooaguKcm
browser/modules/BrowserErrorReporter.jsm
browser/modules/test/browser/browser_BrowserErrorReporter.js
toolkit/components/telemetry/Scalars.yaml
--- a/browser/modules/BrowserErrorReporter.jsm
+++ b/browser/modules/BrowserErrorReporter.jsm
@@ -19,23 +19,16 @@ const ERROR_PREFIX_RE = /^[^\W]+:/m;
 const PREF_ENABLED = "browser.chrome.errorReporter.enabled";
 const PREF_LOG_LEVEL = "browser.chrome.errorReporter.logLevel";
 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 SDK_NAME = "firefox-error-reporter";
 const SDK_VERSION = "1.0.0";
-const TELEMETRY_ERROR_COLLECTED = "browser.errors.collected_count";
-const TELEMETRY_ERROR_COLLECTED_FILENAME = "browser.errors.collected_count_by_filename";
-const TELEMETRY_ERROR_COLLECTED_STACK = "browser.errors.collected_with_stack_count";
-const TELEMETRY_ERROR_REPORTED = "browser.errors.reported_success_count";
-const TELEMETRY_ERROR_REPORTED_FAIL = "browser.errors.reported_failure_count";
-const TELEMETRY_ERROR_SAMPLE_RATE = "browser.errors.sample_rate";
-
 
 // https://developer.mozilla.org/en-US/docs/Mozilla/Tech/XPCOM/Reference/Interface/nsIScriptError#Categories
 const REPORTED_CATEGORIES = new Set([
   "XPConnect JavaScript",
   "component javascript",
   "chrome javascript",
   "chrome registration",
   "XBL",
@@ -47,23 +40,16 @@ const REPORTED_CATEGORIES = new Set([
 
 const PLATFORM_NAMES = {
   linux: "Linux",
   win: "Windows",
   macosx: "macOS",
   android: "Android",
 };
 
-// Filename URI regexes that we are okay with reporting to Telemetry. URIs not
-// matching these patterns may contain local file paths.
-const TELEMETRY_REPORTED_PATTERNS = new Set([
-  /^resource:\/\/(?:\/|gre)/,
-  /^chrome:\/\/(?:global|browser|devtools)/,
-]);
-
 /**
  * Collects nsIScriptError messages logged to the browser console and reports
  * them to a remotely-hosted error collection service.
  *
  * This is a PROTOTYPE; it will be removed in the future and potentially
  * replaced with a more robust implementation. It is meant to only collect
  * errors from Nightly (and local builds if enabled for development purposes)
  * and has not been reviewed for use outside of Nightly.
@@ -119,23 +105,16 @@ class BrowserErrorReporter {
 
     XPCOMUtils.defineLazyPreferenceGetter(
       this,
       "collectionEnabled",
       PREF_ENABLED,
       false,
       this.handleEnabledPrefChanged.bind(this),
     );
-    XPCOMUtils.defineLazyPreferenceGetter(
-      this,
-      "sampleRatePref",
-      PREF_SAMPLE_RATE,
-      "0.0",
-      this.handleSampleRatePrefChanged.bind(this),
-    );
   }
 
   /**
    * Lazily-created logger
    */
   get logger() {
     const logger = Log.repository.getLogger("BrowserErrorReporter");
     logger.addAppender(new Log.ConsoleAppender(new Log.BasicFormatter()));
@@ -168,57 +147,31 @@ class BrowserErrorReporter {
       this.registerListener();
     } else {
       try {
         this.unregisterListener();
       } catch (err) {} // It probably wasn't registered.
     }
   }
 
-  handleSampleRatePrefChanged(prefName, previousValue, newValue) {
-    Services.telemetry.scalarSet(TELEMETRY_ERROR_SAMPLE_RATE, newValue);
-  }
-
-  shouldReportFilename(filename) {
-    for (const pattern of TELEMETRY_REPORTED_PATTERNS) {
-      if (filename.match(pattern)) {
-        return true;
-      }
-    }
-    return false;
-  }
-
   async observe(message) {
     try {
       message.QueryInterface(Ci.nsIScriptError);
     } catch (err) {
       return; // Not an error
     }
 
     const isWarning = message.flags & message.warningFlag;
     const isFromChrome = REPORTED_CATEGORIES.has(message.category);
     if ((this.chromeOnly && !isFromChrome) || isWarning) {
       return;
     }
 
-    // Record that we collected an error prior to applying the sample rate
-    Services.telemetry.scalarAdd(TELEMETRY_ERROR_COLLECTED, 1);
-    if (message.stack) {
-      Services.telemetry.scalarAdd(TELEMETRY_ERROR_COLLECTED_STACK, 1);
-    }
-    if (message.sourceName) {
-      let filename = "FILTERED";
-      if (this.shouldReportFilename(message.sourceName)) {
-        filename = message.sourceName;
-      }
-      Services.telemetry.keyedScalarAdd(TELEMETRY_ERROR_COLLECTED_FILENAME, filename.slice(0, 69), 1);
-    }
-
     // Sample the amount of errors we send out
-    const sampleRate = Number.parseFloat(this.sampleRatePref);
+    const sampleRate = Number.parseFloat(Services.prefs.getCharPref(PREF_SAMPLE_RATE));
     if (!Number.isFinite(sampleRate) || (Math.random() >= sampleRate)) {
       return;
     }
 
     const exceptionValue = {};
     const requestBody = {
       ...this.requestBodyTemplate,
       timestamp: new Date().toISOString().slice(0, -1), // Remove trailing "Z"
@@ -251,20 +204,18 @@ class BrowserErrorReporter {
         headers: {
           "Content-Type": "application/json",
           "Accept": "application/json",
         },
         // Sentry throws an auth error without a referrer specified.
         referrer: "https://fake.mozilla.org",
         body: JSON.stringify(requestBody)
       });
-      Services.telemetry.scalarAdd(TELEMETRY_ERROR_REPORTED, 1);
       this.logger.debug("Sent error successfully.");
     } catch (error) {
-      Services.telemetry.scalarAdd(TELEMETRY_ERROR_REPORTED_FAIL, 1);
       this.logger.warn(`Failed to send error: ${error}`);
     }
   }
 }
 
 function defaultFetch(...args) {
   // Do not make network requests while running in automation
   if (Cu.isInAutomation) {
--- a/browser/modules/test/browser/browser_BrowserErrorReporter.js
+++ b/browser/modules/test/browser/browser_BrowserErrorReporter.js
@@ -61,22 +61,16 @@ function fetchCallForMessage(fetchSpy, m
 }
 
 // Helper to test if a fetch spy was called with the given error message.
 // Used in tests where unrelated JS errors from other code are logged.
 function fetchPassedError(fetchSpy, message) {
   return fetchCallForMessage(fetchSpy, message) !== null;
 }
 
-add_task(async function testSetup() {
-  const canRecordExtended = Services.telemetry.canRecordExtended;
-  Services.telemetry.canRecordExtended = true;
-  registerCleanupFunction(() => Services.telemetry.canRecordExtended = canRecordExtended);
-});
-
 add_task(async function testInitPrefDisabled() {
   let listening = false;
   const reporter = new BrowserErrorReporter({
     registerListener() {
       listening = true;
     },
   });
   await SpecialPowers.pushPrefEnv({set: [
@@ -450,143 +444,8 @@ add_task(async function testExtensionTag
   await extension.unload();
   reporter.uninit();
 
   await reporter.observe(createScriptError({message: "testExtensionTag not from extension"}));
   call = fetchCallForMessage(fetchSpy, "testExtensionTag not from extension");
   body = JSON.parse(call.args[1].body);
   is(body.tags.isExtensionError, undefined, "Normal errors do not have an isExtensionError tag.");
 });
-
-add_task(async function testScalars() {
-  const fetchStub = sinon.stub();
-  const reporter = new BrowserErrorReporter(fetchStub);
-  await SpecialPowers.pushPrefEnv({set: [
-    [PREF_ENABLED, true],
-    [PREF_SAMPLE_RATE, "1.0"],
-  ]});
-
-  Services.telemetry.clearScalars();
-
-  const messages = [
-    createScriptError({message: "No name"}),
-    createScriptError({message: "Also no name", sourceName: "resource://gre/modules/Foo.jsm"}),
-    createScriptError({message: "More no name", sourceName: "resource://gre/modules/Bar.jsm"}),
-    createScriptError({message: "Yeah sures", sourceName: "unsafe://gre/modules/Bar.jsm"}),
-    createScriptError({
-      message: "long",
-      sourceName: "resource://gre/modules/long/long/long/long/long/long/long/long/long/long/",
-    }),
-    {message: "Not a scripterror instance."},
-
-    // No easy way to create an nsIScriptError with a stack, so let's pretend.
-    Object.create(
-      createScriptError({message: "Whatever"}),
-      {stack: {value: new Error().stack}},
-    ),
-  ];
-
-  // Use observe to avoid errors from other code messing up our counts.
-  for (const message of messages) {
-    await reporter.observe(message);
-  }
-
-  await SpecialPowers.pushPrefEnv({set: [[PREF_SAMPLE_RATE, "0.0"]]});
-  await reporter.observe(createScriptError({message: "Additionally no name"}));
-
-  await SpecialPowers.pushPrefEnv({set: [[PREF_SAMPLE_RATE, "1.0"]]});
-  fetchStub.throws(new Error("Could not report"));
-  await reporter.observe(createScriptError({message: "Maybe name?"}));
-
-  const optin = Ci.nsITelemetry.DATASET_RELEASE_CHANNEL_OPTIN;
-  const scalars = Services.telemetry.snapshotScalars(optin, false).parent;
-  is(
-    scalars[TELEMETRY_ERROR_COLLECTED],
-    8,
-    `${TELEMETRY_ERROR_COLLECTED} is incremented when an error is collected.`,
-  );
-  is(
-    scalars[TELEMETRY_ERROR_SAMPLE_RATE],
-    "1.0",
-    `${TELEMETRY_ERROR_SAMPLE_RATE} contains the last sample rate used.`,
-  );
-  is(
-    scalars[TELEMETRY_ERROR_REPORTED],
-    6,
-    `${TELEMETRY_ERROR_REPORTED} is incremented when an error is reported.`,
-  );
-  is(
-    scalars[TELEMETRY_ERROR_REPORTED_FAIL],
-    1,
-    `${TELEMETRY_ERROR_REPORTED_FAIL} is incremented when an error fails to be reported.`,
-  );
-  is(
-    scalars[TELEMETRY_ERROR_COLLECTED_STACK],
-    1,
-    `${TELEMETRY_ERROR_REPORTED_FAIL} is incremented when an error with a stack trace is collected.`,
-  );
-
-  const keyedScalars = Services.telemetry.snapshotKeyedScalars(optin, false).parent;
-  Assert.deepEqual(
-    keyedScalars[TELEMETRY_ERROR_COLLECTED_FILENAME],
-    {
-      "FILTERED": 1,
-      "resource://gre/modules/Foo.jsm": 1,
-      "resource://gre/modules/Bar.jsm": 1,
-      // Cut off at 70-character limit
-      "resource://gre/modules/long/long/long/long/long/long/long/long/long/l": 1,
-    },
-    `${TELEMETRY_ERROR_COLLECTED_FILENAME} is incremented when an error is collected.`,
-  );
-
-  resetConsole();
-});
-
-add_task(async function testCollectedFilenameScalar() {
-  const fetchStub = sinon.stub();
-  const reporter = new BrowserErrorReporter(fetchStub);
-  await SpecialPowers.pushPrefEnv({set: [
-    [PREF_ENABLED, true],
-    [PREF_SAMPLE_RATE, "1.0"],
-  ]});
-
-  const testCases = [
-    ["chrome://unknown/module.jsm", false],
-    ["resource://unknown/module.jsm", false],
-    ["unknown://unknown/module.jsm", false],
-
-    ["resource://gre/modules/Foo.jsm", true],
-    ["resource:///modules/Foo.jsm", true],
-    ["chrome://global/Foo.jsm", true],
-    ["chrome://browser/Foo.jsm", true],
-    ["chrome://devtools/Foo.jsm", true],
-  ];
-
-  for (const [filename, shouldMatch] of testCases) {
-    Services.telemetry.clearScalars();
-
-    // Use observe to avoid errors from other code messing up our counts.
-    await reporter.observe(createScriptError({
-      message: "Fine",
-      sourceName: filename,
-    }));
-
-    const keyedScalars = (
-      Services.telemetry.snapshotKeyedScalars(Ci.nsITelemetry.DATASET_RELEASE_CHANNEL_OPTIN, false).parent
-    );
-
-    let matched = null;
-    if (shouldMatch) {
-      matched = keyedScalars[TELEMETRY_ERROR_COLLECTED_FILENAME][filename] === 1;
-    } else {
-      matched = keyedScalars[TELEMETRY_ERROR_COLLECTED_FILENAME].FILTERED === 1;
-    }
-
-    ok(
-      matched,
-      shouldMatch
-        ? `${TELEMETRY_ERROR_COLLECTED_FILENAME} logs a key for ${filename}.`
-        : `${TELEMETRY_ERROR_COLLECTED_FILENAME} logs a FILTERED key for ${filename}.`,
-    );
-  }
-
-  resetConsole();
-});
--- a/toolkit/components/telemetry/Scalars.yaml
+++ b/toolkit/components/telemetry/Scalars.yaml
@@ -1486,104 +1486,16 @@ sw:
     notification_emails:
       - sw-telemetry@mozilla.com
       - echuang@mozilla.com
     release_channel_collection: opt-out
     record_in_processes:
       - 'main'
       - 'content'
 
-# The following section contains the BrowserErrorReporter scalars.
-browser.errors:
-  collected_count:
-    bug_numbers:
-      - 1444554
-    description: >
-      The count of all browser chrome JS errors that were collected locally.
-    expires: "64"
-    kind: uint
-    notification_emails:
-      - nightly-js-errors@mozilla.com
-      - mkelly@mozilla.com
-    record_in_processes:
-      - 'main'
-
-  collected_with_stack_count:
-    bug_numbers:
-      - 1444554
-    description: >
-      The count of browser chrome JS errors that were collected locally and had
-      a usable stack trace.
-    expires: "64"
-    kind: uint
-    notification_emails:
-      - nightly-js-errors@mozilla.com
-      - mkelly@mozilla.com
-    record_in_processes:
-      - 'main'
-
-  reported_success_count:
-    bug_numbers:
-      - 1444554
-    description: >
-      The count of all browser chrome JS errors that were reported to the
-      remote collection service.
-    expires: "64"
-    kind: uint
-    notification_emails:
-      - nightly-js-errors@mozilla.com
-      - mkelly@mozilla.com
-    record_in_processes:
-      - 'main'
-
-  reported_failure_count:
-    bug_numbers:
-      - 1444554
-    description: >
-      The count of all browser chrome JS errors that we attempted to report to
-      the remote collection service, but failed to.
-    expires: "64"
-    kind: uint
-    notification_emails:
-      - nightly-js-errors@mozilla.com
-      - mkelly@mozilla.com
-    record_in_processes:
-      - 'main'
-
-  sample_rate:
-    bug_numbers:
-      - 1444554
-    description: >
-      The sample rate at which collected errors were reported.
-    expires: "64"
-    kind: string
-    notification_emails:
-      - nightly-js-errors@mozilla.com
-      - mkelly@mozilla.com
-    record_in_processes:
-      - 'main'
-
-  collected_count_by_filename:
-    bug_numbers:
-      - 1444554
-    description: >
-      The count of all browser chrome JS errors that were collected locally,
-      keyed by the filename of the file in which the error occurred. Collected
-      filenames are limited to specific paths under the resource:// and
-      chrome:// protocols; non-matching filenames are reported as "FILTERED".
-      Long filenames are truncated to the first 70 characters.
-    keyed: true
-    expires: "64"
-    kind: uint
-    notification_emails:
-      - nightly-js-errors@mozilla.com
-      - mkelly@mozilla.com
-    record_in_processes:
-      - 'main'
-
 # The following section is for probes testing the Telemetry system. They will not be
 # submitted in pings and are only used for testing.
 telemetry.test:
   unsigned_int_kind:
     bug_numbers:
       - 1276190
     description: >
       This is a test uint type with a really long description, maybe spanning even multiple