Merge pull request #7088 from farhan787/issue-#6956 r=@shane-tomlinson

fix(stderr events): allow independent on/off configurations for amplitude, client_metrics and flow events

server/lib/amplitude.js

@@ -17,8 +17,10 @@
 const { GROUPS, initialize } = require('fxa-shared/metrics/amplitude');
 const logger = require('./logging/log')();
 const ua = require('./user-agent');
+const config = require('./configuration');
 
-const SERVICES = require('./configuration').get('oauth_client_id_map');
+const SERVICES = config.get('oauth_client_id_map');
+const amplitude = config.get('amplitude');
 
 // Maps view name to email type
 const EMAIL_TYPES = {
@@ -133,7 +135,7 @@ const transform = initialize(SERVICES, EVENTS, FUZZY_EVENTS);
 module.exports = receiveEvent;
 
 function receiveEvent (event, request, data) {
-  if (! event || ! request || ! data) {
+  if (amplitude.disabled || ! event || ! request || ! data) {
     return;
   }
 

server/lib/configuration.js

@@ -30,6 +30,14 @@ const conf = module.exports = convict({
     env: 'ALLOWED_PARENT_ORIGINS',
     format: Array
   },
+  amplitude: {
+    disabled: {
+      default: false,
+      doc: 'Disable amplitude events',
+      env: 'AMPLITUDE_DISABLED',
+      format: Boolean
+    }
+  },
   are_dist_resources: {
     default: false,
     doc: 'Check if the resources are under the /dist directory',
@@ -217,6 +225,12 @@ const conf = module.exports = convict({
     env: 'FLOW_ID_KEY',
     format: String
   },
+  flow_metrics_disabled: {
+    default: false,
+    doc: 'Disable flow metrics output to stderr',
+    env: 'FLOW_METRICS_DISABLED',
+    format: Boolean
+  },
   fxa_client_configuration: {
     max_age: {
       default: '1 day',

server/lib/flow-event.js

@@ -29,6 +29,7 @@ const VERSION = 1;
 const FLOW_BEGIN_EVENT = 'flow.begin';
 const FLOW_ID_KEY = config.get('flow_id_key');
 const FLOW_ID_EXPIRY = config.get('flow_id_expiry');
+const FLOW_METRICS_DISABLED = config.get('flow_metrics_disabled');
 
 const ENTRYPOINT_PATTERN = /^[\w.-]+$/;
 const SERVICE_PATTERN = /^(sync|content-server|none|[0-9a-f]{16})$/;
@@ -44,8 +45,6 @@ const VALID_FLOW_EVENT_PROPERTIES = [
 
 const UTM_PATTERN = /^[\w.%-]+$/;
 
-const IS_DISABLED = config.get('client_metrics').stderr_collector_disabled;
-
 const PERFORMANCE_TIMINGS = [
   // These timings are only an approximation, to be used as extra signals
   // when looking for correlations in the flow data. They're not perfect
@@ -84,7 +83,7 @@ const PERFORMANCE_TIMINGS = [
 const AUTH_VIEWS = new Set([ 'enter-email', 'force-auth', 'signin', 'signup' ]);
 
 const metricsRequest = (req, metrics, requestReceivedTime) => {
-  if (IS_DISABLED || ! isValidFlowData(metrics, requestReceivedTime)) {
+  if (FLOW_METRICS_DISABLED || ! isValidFlowData(metrics, requestReceivedTime)) {
     return;
   }
 

tests/server/amplitude.js

@@ -11,13 +11,21 @@ const pkg = require('../../package.json');
 const logger = {
   info: sinon.spy()
 };
+const amplitudeConfig = {
+  disabled: false
+};
+
 const amplitude = proxyquire(path.resolve('server/lib/amplitude'), {
   './configuration': {
-    get () {
-      return {
-        '0': 'amo',
-        '1': 'pocket'
-      };
+    get (name) {
+      if (name === 'oauth_client_id_map') {
+        return {
+          '0': 'amo',
+          '1': 'pocket'
+        };
+      } else if (name === 'amplitude') {
+        return amplitudeConfig;
+      }
     }
   },
   './logging/log': () => logger
@@ -26,6 +34,7 @@ const APP_VERSION = /^[0-9]+\.([0-9]+)\./.exec(pkg.version)[1];
 
 registerSuite('amplitude', {
   beforeEach: function() {
+    amplitudeConfig.disabled = false;
     sinon.stub(process.stderr, 'write').callsFake(() => {});
   },
 
@@ -46,6 +55,27 @@ registerSuite('amplitude', {
       assert.lengthOf(amplitude, 3);
     },
 
+    'disable writing amplitude events': {
+      'logger.info was not called': () => {
+        amplitudeConfig.disabled = true;
+        amplitude({
+          time: 'a',
+          type: 'flow.signin_from.bar'
+        }, {
+          connection: {},
+          headers: {
+            'x-forwarded-for': '63.245.221.32'
+          }
+        }, {
+          flowBeginTime: 'b',
+          flowId: 'c',
+          uid: 'd'
+        });
+
+        assert.equal(logger.info.callCount, 0);
+      }
+    },
+
     'does not throw if arguments are missing': () => {
       assert.doesNotThrow(amplitude);
       assert.doesNotThrow(() => amplitude('foo'));

tests/server/flow-event.js

@@ -13,11 +13,9 @@ registerSuite('flow-event', {
   beforeEach: function() {
     config = {
       /*eslint-disable camelcase*/
-      client_metrics: {
-        stderr_collector_disabled: false
-      },
       flow_id_expiry: 7200000,
-      flow_id_key: 'foo'
+      flow_id_key: 'foo',
+      flow_metrics_disabled: false
       /*eslint-enable camelcase*/
     };
     sandbox = sinon.sandbox.create();
@@ -60,6 +58,34 @@ registerSuite('flow-event', {
       assert.lengthOf(flowEvent, 3);
     },
 
+    'call flowEvent with flow_metrics_disabled true': {
+      beforeEach() {
+        /*eslint-disable camelcase*/
+        config.flow_metrics_disabled = true;
+        /*eslint-enable camelcase*/
+
+        const timeSinceFlowBegin = 1000;
+        flowMetricsValidateResult = true;
+        setup({
+          events: [
+            {offset: 5, type: 'wibble'},
+            {offset: 5, type: 'flow.begin'},
+            {offset: 5.9, type: 'screen.signup'},
+            {offset: timeSinceFlowBegin, type: 'flow.signup.good-offset-now'},
+            {offset: timeSinceFlowBegin + 1, type: 'flow.signup.bad-offset-future'},
+            {offset: timeSinceFlowBegin - config.flow_id_expiry - 1, type: 'flow.signup.bad-offset-expired'},
+            {offset: timeSinceFlowBegin - config.flow_id_expiry, type: 'flow.signup.good-offset-oldest'},
+            {offset: 500, type: 'flow.timing.foo.1'},
+            {offset: 500, type: 'flow.timing.bar.baz.2'}
+          ],
+        }, timeSinceFlowBegin);
+      },
+
+      'process.stderr.write was not called': () => {
+        assert.equal(process.stderr.write.callCount, 0);
+      }
+    },
+
     'call flowEvent with valid flow data': {
       beforeEach() {
         const timeSinceFlowBegin = 1000;