PingCentre should be responsible for telemetry logging and should all…

…ow toggling stage/prod.

system-addon/lib/ActivityStream.jsm

@@ -86,11 +86,6 @@ const PREFS_CONFIG = new Map([
     value: true,
     value_local_dev: false
   }],
-  ["telemetry.log", {
-    title: "Log telemetry events in the console",
-    value: false,
-    value_local_dev: true
-  }],
   ["telemetry.ping.endpoint", {
     title: "Telemetry server endpoint",
     value: "https://tiles.services.mozilla.com/v4/links/activity-stream"

system-addon/lib/PingCentre.jsm

@@ -7,19 +7,19 @@ Cu.import("resource://gre/modules/Preferences.jsm");
 Cu.import("resource://gre/modules/XPCOMUtils.jsm");
 Cu.importGlobalProperties(["fetch"]);
 
+XPCOMUtils.defineLazyModuleGetter(this, "AppConstants",
+  "resource://gre/modules/AppConstants.jsm");
 XPCOMUtils.defineLazyModuleGetter(this, "ClientID",
   "resource://gre/modules/ClientID.jsm");
 XPCOMUtils.defineLazyModuleGetter(this, "console",
   "resource://gre/modules/Console.jsm");
 
-// This is intentionally a different pref-branch than the SDK-based add-on
-// used, to avoid extra weirdness for people who happen to have the SDK-based
-// installed.  Though maybe we should just forcibly disable the old add-on?
-const PREF_BRANCH = "browser.newtabpage.activity-stream.";
+const PREF_BRANCH = "browser.ping-centre.";
 
-const ENDPOINT_PREF = `${PREF_BRANCH}telemetry.ping.endpoint`;
 const TELEMETRY_PREF = `${PREF_BRANCH}telemetry`;
-const LOGGING_PREF = `${PREF_BRANCH}telemetry.log`;
+const LOGGING_PREF = `${PREF_BRANCH}log`;
+const STAGING_ENDPOINT_PREF = `${PREF_BRANCH}staging.endpoint`;
+const PRODUCTION_ENDPOINT_PREF = `${PREF_BRANCH}production.endpoint`;
 
 const FHR_UPLOAD_ENABLED_PREF = "datareporting.healthreport.uploadEnabled";
 
@@ -28,15 +28,15 @@ const FHR_UPLOAD_ENABLED_PREF = "datareporting.healthreport.uploadEnabled";
  *
  * @param {string} topic - a unique ID for users of PingCentre to distinguish
  *                  their data on the server side.
- * @param {string} pingEndpoint - the URL where the POST with data is sent.
+ * @param {string} pingEndpoint - optional pref for URL where the POST is sent.
  * @param {Object} args - optional arguments
  * @param {Function} args.prefInitHook - if present, will be called back
  *                   inside the Prefs constructor. Typically used from tests
  *                   to save off a pointer to a fake Prefs instance so that
  *                   stubs and spies can be inspected by the test code.
  */
 class PingCentre {
-  constructor(topic, pingEndpoint, args) {
+  constructor(topic, overrideEndpointPref, args) {
     let prefArgs = {};
     if (args) {
       if ("prefInitHook" in args) {
@@ -50,7 +50,8 @@ class PingCentre {
 
     this._topic = topic;
     this._prefs = new Preferences(prefArgs);
-    this._pingEndpoint = pingEndpoint || this._prefs.get(ENDPOINT_PREF);
+
+    this._setPingEndpoint(topic, overrideEndpointPref);
 
     this._enabled = this._prefs.get(TELEMETRY_PREF);
     this._onTelemetryPrefChange = this._onTelemetryPrefChange.bind(this);
@@ -77,6 +78,15 @@ class PingCentre {
     return this._enabled && this._fhrEnabled;
   }
 
+  _setPingEndpoint(topic, overrideEndpointPref) {
+    this._pingEndpoint = this._prefs.get(overrideEndpointPref) ||
+      this._prefs.get(STAGING_ENDPOINT_PREF);
+
+    if (AppConstants.MOZ_UPDATE_CHANNEL === "release") {
+      this._pingEndpoint = this._prefs.get(PRODUCTION_ENDPOINT_PREF);
+    }
+  }
+
   _onLoggingPrefChange(prefVal) {
     this.logging = prefVal;
   }
@@ -129,7 +139,7 @@ class PingCentre {
 
 this.PingCentre = PingCentre;
 this.PingCentreConstants = {
-  ENDPOINT_PREF,
+  PRODUCTION_ENDPOINT_PREF,
   FHR_UPLOAD_ENABLED_PREF,
   TELEMETRY_PREF,
   LOGGING_PREF

system-addon/lib/TelemetryFeed.jsm

@@ -21,6 +21,8 @@ XPCOMUtils.defineLazyServiceGetter(this, "gUUIDGenerator",
   "@mozilla.org/uuid-generator;1",
   "nsIUUIDGenerator");
 
+const ACTIVITY_STREAM_ENDPOINT_PREF = "browser.newtabpage.activity-stream.telemetry.ping.endpoint";
+
 // This is a mapping table between the user preferences and its encoding code
 const USER_PREFS_ENCODING = {
   "showSearch": 1 << 0,
@@ -81,7 +83,8 @@ this.TelemetryFeed = class TelemetryFeed {
    * Lazily initialize PingCentre to send pings
    */
   get pingCentre() {
-    Object.defineProperty(this, "pingCentre", {value: new PingCentre("activity-stream")});
+    Object.defineProperty(this, "pingCentre",
+      {value: new PingCentre("activity-stream", ACTIVITY_STREAM_ENDPOINT_PREF)});
     return this.pingCentre;
   }
 
@@ -228,7 +231,9 @@ this.TelemetryFeed = class TelemetryFeed {
   }
 
   async sendEvent(event_object) {
-    this.pingCentre.sendPing(event_object);
+    if (this._prefs.get("telemetry")) {
+      this.pingCentre.sendPing(event_object);
+    }
   }
 
   onAction(action) {
@@ -302,4 +307,4 @@ this.TelemetryFeed = class TelemetryFeed {
   }
 };
 
-this.EXPORTED_SYMBOLS = ["TelemetryFeed", "USER_PREFS_ENCODING"];
+this.EXPORTED_SYMBOLS = ["TelemetryFeed", "USER_PREFS_ENCODING", "ACTIVITY_STREAM_ENDPOINT_PREF"];

system-addon/test/unit/lib/PingCentre.test.js

@@ -3,8 +3,10 @@
 
 const {GlobalOverrider, FakePrefs} = require("test/unit/utils");
 const {PingCentre, PingCentreConstants} = require("lib/PingCentre.jsm");
-const {ENDPOINT_PREF, FHR_UPLOAD_ENABLED_PREF, TELEMETRY_PREF, LOGGING_PREF} =
-  PingCentreConstants;
+const {
+  PRODUCTION_ENDPOINT_PREF, FHR_UPLOAD_ENABLED_PREF, TELEMETRY_PREF,
+  LOGGING_PREF
+} = PingCentreConstants;
 
 /**
  * A reference to the fake preferences object created by the PingCentre
@@ -17,14 +19,15 @@ const prefInitHook = function() {
 
 const tsArgs = {prefInitHook};
 const FAKE_TELEMETRY_ID = "foo123";
+const FAKE_AS_ENDPOINT_PREF = "some.as.endpoint.pref";
 
 describe("PingCentre", () => {
   let globals;
   let tSender;
   let sandbox;
   let fetchStub;
   const fakeEndpointUrl = "http://127.0.0.1/stuff";
-  const fakePingJSON = JSON.stringify({action: "fake_action", monkey: 1});
+  const fakePingJSON = {action: "fake_action", monkey: 1};
   const fakeFetchHttpErrorResponse = {ok: false, status: 400};
   const fakeFetchSuccessResponse = {ok: true, status: 200};
 
@@ -36,6 +39,7 @@ describe("PingCentre", () => {
     globals.set("Preferences", FakePrefs);
     globals.set("fetch", fetchStub);
     globals.set("ClientID", {getClientID: sandbox.spy(async () => FAKE_TELEMETRY_ID)});
+    globals.set("AppConstants", {MOZ_UPDATE_CHANNEL: "beta"});
     sandbox.spy(global.Components.utils, "reportError");
   });
 
@@ -45,7 +49,7 @@ describe("PingCentre", () => {
   });
 
   it("should add .telemetryClientId from the ClientID module", async () => {
-    tSender = new PingCentre("activity-stream", fakeEndpointUrl, tsArgs);
+    tSender = new PingCentre("activity-stream", "", tsArgs);
     assert.equal(await tSender.telemetryClientId, FAKE_TELEMETRY_ID);
   });
 
@@ -65,7 +69,7 @@ describe("PingCentre", () => {
 
   describe("telemetry.enabled pref changes from true to false", () => {
     beforeEach(() => {
-      FakePrefs.prototype.prefs[IS_PRODUCTION_PREF] = true;
+      globals.set("AppConstants", {MOZ_UPDATE_CHANNEL: "release"});
       FakePrefs.prototype.prefs[PRODUCTION_ENDPOINT_PREF] = fakeEndpointUrl;
     });
 
@@ -170,8 +174,8 @@ describe("PingCentre", () => {
       FakePrefs.prototype.prefs = {};
       FakePrefs.prototype.prefs[FHR_UPLOAD_ENABLED_PREF] = true;
       FakePrefs.prototype.prefs[TELEMETRY_PREF] = true;
-      FakePrefs.prototype.prefs[ENDPOINT_PREF] = fakeEndpointUrl;
-      tSender = new PingCentre("activity-stream", fakeEndpointUrl, tsArgs);
+      FakePrefs.prototype.prefs[FAKE_AS_ENDPOINT_PREF] = fakeEndpointUrl;
+      tSender = new PingCentre("activity-stream", FAKE_AS_ENDPOINT_PREF, tsArgs);
     });
 
     it("should not send if the PingCentre is disabled", async () => {

system-addon/test/unit/lib/TelemetryFeed.test.js

@@ -323,6 +323,7 @@ describe("TelemetryFeed", () => {
   describe("#sendEvent", () => {
     it("should call PingCentre", async () => {
       sandbox.stub(instance.pingCentre, "sendPing");
+      FakePrefs.prototype.prefs.telemetry = true;
       const event = {};
       await instance.sendEvent(event);
       assert.calledWith(instance.pingCentre.sendPing, event);