Backed out changeset a760e111f2f1 (bug 1444554) for merge conflicts on browser_BrowserErrorReporter.js and failures after merging to autoland. a=backout
authorCosmin Sabou <csabou@mozilla.com>
Wed, 21 Mar 2018 02:51:03 +0200
changeset 409073 3d21d31141dc5e2d2aff89203458125a3cce6c64
parent 409072 6d0f84afc194e287b6f144d17cd88371a5357d04
child 409074 9fa586e87f34bef3bb15431d149ec7aeae0f7d24
child 409116 279aeaacb2867de4ffa49f254347ac14bf6a195c
child 409159 5bf16c2dca2275996db533045e1e8b4c3324ba7e
push id33673
push usercsabou@mozilla.com
push dateWed, 21 Mar 2018 00:51:12 +0000
treeherdermozilla-central@3d21d31141dc [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersbackout
bugs1444554
milestone61.0a1
backs outa760e111f2f157fbb18e66117f026ca298d4343e
first release with
nightly linux32
3d21d31141dc / 61.0a1 / 20180321040527 / files
nightly linux64
3d21d31141dc / 61.0a1 / 20180321040527 / files
nightly mac
3d21d31141dc / 61.0a1 / 20180321040527 / files
nightly win32
3d21d31141dc / 61.0a1 / 20180321040527 / files
nightly win64
3d21d31141dc / 61.0a1 / 20180321040527 / files
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
releases
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Backed out changeset a760e111f2f1 (bug 1444554) for merge conflicts on browser_BrowserErrorReporter.js and failures after merging to autoland. a=backout
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.
@@ -113,23 +99,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()));
@@ -162,73 +141,43 @@ class BrowserErrorReporter {
       Services.console.registerListener(this);
     } else {
       try {
         Services.console.unregisterListener(this);
       } 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 extensions = new Map();
     for (let extension of WebExtensionPolicy.getActiveExtensions()) {
       extensions.set(extension.mozExtensionHostname, extension);
     }
 
     // Replaces any instances of moz-extension:// URLs with internal UUIDs to use
     // the add-on ID instead.
     function mangleExtURL(string, anchored = true) {
-      if (!string) {
-        return string;
-      }
-
       let re = new RegExp(`${anchored ? "^" : ""}moz-extension://([^/]+)/`, "g");
 
       return string.replace(re, (m0, m1) => {
         let id = extensions.has(m1) ? extensions.get(m1).id : m1;
         return `moz-extension://${id}/`;
       });
     }
 
@@ -282,20 +231,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}`);
     }
   }
 
   async normalizeStackFrame(frame) {
     const normalizedFrame = {
       function: frame.functionDisplayName,
       module: frame.source,
--- a/browser/modules/test/browser/browser_BrowserErrorReporter.js
+++ b/browser/modules/test/browser/browser_BrowserErrorReporter.js
@@ -11,22 +11,16 @@ registerCleanupFunction(function() {
   delete window.sinon;
 });
 
 const PREF_ENABLED = "browser.chrome.errorReporter.enabled";
 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 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";
 
 function createScriptError(options = {}) {
   const scriptError = Cc["@mozilla.org/scripterror;1"].createInstance(Ci.nsIScriptError);
   scriptError.init(
     options.message || "",
     options.sourceName || null,
     options.sourceLine || null,
     options.lineNumber || null,
@@ -88,22 +82,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() {
   const fetchSpy = sinon.spy();
   const reporter = new BrowserErrorReporter(fetchSpy);
   await SpecialPowers.pushPrefEnv({set: [
     [PREF_ENABLED, false],
     [PREF_SAMPLE_RATE, "1.0"],
   ]});
 
@@ -456,143 +444,8 @@ add_task(async function testAddonIDMangl
     stackFrame.module.startsWith(`moz-extension://${id}/`),
     "Stack frame filenames use the proper add-on ID instead of internal UUIDs.",
   );
 
   await extension.unload();
   reporter.uninit();
   resetConsole();
 });
-
-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