Merge pull request #4774 from mozilla/feat/issue-4656

feat(metrics): add metrics to amplitude events

packages/fxa-auth-server/bin/key_server.js

@@ -6,7 +6,8 @@
 
 const error = require('../lib/error');
 const jwtool = require('fxa-jwtool');
-const StatsD = require('hot-shots');
+const { StatsD } = require('hot-shots');
+const { Container } = require('typedi');
 
 async function run(config) {
   const statsd = config.statsd.enabled
@@ -22,6 +23,7 @@ async function run(config) {
         timing: () => {},
         close: () => {},
       };
+  Container.set(StatsD, statsd);
 
   const log = require('../lib/log')({ ...config.log, statsd });
   require('../lib/oauth/logging')(log);

packages/fxa-auth-server/lib/metrics/amplitude.js

@@ -12,6 +12,9 @@
 
 'use strict';
 
+const { Container } = require('typedi');
+const { StatsD } = require('hot-shots');
+
 const { GROUPS, initialize } = require('../../../fxa-shared/metrics/amplitude');
 const { version: VERSION } = require('../../package.json');
 
@@ -150,6 +153,7 @@ module.exports = (log, config) => {
     data = {},
     metricsContext = {}
   ) {
+    const statsd = Container.get(StatsD);
     if (!eventType || !request) {
       log.error('amplitude.badArgument', {
         err: 'Bad argument',
@@ -262,8 +266,11 @@ module.exports = (log, config) => {
         },
       };
       log.info('rawAmplitudeData', rawEvent);
+      statsd.increment('amplitude.event.raw');
     }
 
+    statsd.increment('amplitude.event');
+
     const amplitudeEvent = transformEvent(event, {
       ...data,
       devices,
@@ -297,6 +304,8 @@ module.exports = (log, config) => {
           time: amplitudeEvent.time + 1,
         });
       }
+    } else {
+      statsd.increment('amplitude.event.dropped');
     }
   }
 };

packages/fxa-auth-server/package-lock.json

@@ -7152,6 +7152,11 @@
       "integrity": "sha1-hnrHTjhkGHsdPUfZlqeOxciDB3c=",
       "dev": true
     },
+    "typedi": {
+      "version": "0.8.0",
+      "resolved": "https://registry.npmjs.org/typedi/-/typedi-0.8.0.tgz",
+      "integrity": "sha512-/c7Bxnm6eh5kXx2I+mTuO+2OvoWni5+rXA3PhXwVWCtJRYmz3hMok5s1AKLzoDvNAZqj/Q/acGstN0ri5aQoOA=="
+    },
     "typescript": {
       "version": "3.7.2",
       "resolved": "https://registry.npmjs.org/typescript/-/typescript-3.7.2.tgz",

packages/fxa-auth-server/package.json

@@ -86,6 +86,7 @@
     "safe-url-assembler": "1.3.5",
     "sandbox": "0.8.6",
     "stripe": "^8.1.0",
+    "typedi": "^0.8.0",
     "urijs": "1.19.1",
     "uuid": "1.4.1",
     "verror": "^1.10.0",

packages/fxa-auth-server/test/local/metrics/amplitude.js

@@ -6,6 +6,10 @@
 
 const { assert } = require('chai');
 const { version } = require('../../../package.json');
+const { StatsD } = require('hot-shots');
+const { Container } = require('typedi');
+const sinon = require('sinon');
+
 const amplitudeModule = require('../../../lib/metrics/amplitude');
 const mocks = require('../../mocks');
 const mockAmplitudeConfig = {
@@ -117,6 +121,8 @@ describe('metrics/amplitude', () => {
 
     describe('raw events enabled', () => {
       it('logged a raw event', async () => {
+        const statsd = { increment: sinon.spy() };
+        Container.set(StatsD, statsd);
         mockAmplitudeConfig.rawEvents = true;
         const now = Date.now();
         await amplitude(
@@ -225,12 +231,19 @@ describe('metrics/amplitude', () => {
           event: expectedEvent,
           context: expectedContext,
         });
+        sinon.assert.calledTwice(statsd.increment);
+        sinon.assert.calledWith(
+          statsd.increment.firstCall,
+          'amplitude.event.raw'
+        );
+        sinon.assert.calledWith(statsd.increment.secondCall, 'amplitude.event');
       });
     });
 
     describe('account.confirmed', () => {
       beforeEach(() => {
         const now = Date.now();
+        Container.set(StatsD, { increment: sinon.spy() });
         return amplitude(
           'account.confirmed',
           mocks.mockRequest({
@@ -306,6 +319,8 @@ describe('metrics/amplitude', () => {
         });
         assert.ok(args[0].time > Date.now() - 1000);
         assert.ok(/^([0-9]+)\.([0-9])$/.test(args[0].app_version));
+        const statsd = Container.get(StatsD);
+        sinon.assert.calledWith(statsd.increment.firstCall, 'amplitude.event');
       });
     });
 
@@ -775,6 +790,7 @@ describe('metrics/amplitude', () => {
 
     describe('email.wibble.bounced', () => {
       beforeEach(() => {
+        Container.set(StatsD, { increment: sinon.spy() });
         return amplitude('email.wibble.bounced', mocks.mockRequest({}));
       });
 
@@ -785,6 +801,16 @@ describe('metrics/amplitude', () => {
       it('did not call log.amplitudeEvent', () => {
         assert.equal(log.amplitudeEvent.callCount, 0);
       });
+
+      it('incremented amplitude dropped', () => {
+        const statsd = Container.get(StatsD);
+        sinon.assert.calledTwice(statsd.increment);
+        sinon.assert.calledWith(statsd.increment.firstCall, 'amplitude.event');
+        sinon.assert.calledWith(
+          statsd.increment.secondCall,
+          'amplitude.event.dropped'
+        );
+      });
     });
 
     describe('email.wibble.sent', () => {

packages/fxa-content-server/server/lib/amplitude.js

@@ -32,6 +32,7 @@ const { version: VERSION } = require('../../package.json');
 const SERVICES = config.get('oauth_client_id_map');
 const amplitude = config.get('amplitude');
 const Sentry = require('@sentry/node');
+const statsd = require('./statsd');
 
 // Maps view name to email type
 const EMAIL_TYPES = {
@@ -443,6 +444,7 @@ function receiveEvent(event, request, data) {
       },
     };
     logger.info('rawAmplitudeData', rawEvent);
+    statsd.increment('amplitude.event.raw');
   }
 
   const userAgent = ua.parse(request.headers['user-agent']);
@@ -458,6 +460,7 @@ function receiveEvent(event, request, data) {
       mapLocation(data.location)
     )
   );
+  statsd.increment('amplitude.event');
 
   if (amplitudeEvent) {
     if (amplitude.schemaValidation) {
@@ -486,6 +489,8 @@ function receiveEvent(event, request, data) {
     }
 
     logger.info('amplitudeEvent', amplitudeEvent);
+  } else {
+    statsd.increment('amplitude.event.dropped');
   }
 }
 

packages/fxa-content-server/tests/server/amplitude.js

@@ -11,6 +11,9 @@ const pkg = require('../../package.json');
 const logger = {
   info: sinon.spy(),
 };
+const statsd = {
+  increment: sinon.spy(),
+};
 const amplitudeConfig = {
   disabled: false,
   rawEvents: false,
@@ -32,6 +35,7 @@ const amplitude = proxyquire(path.resolve('server/lib/amplitude'), {
     },
   },
   './logging/log': () => logger,
+  './statsd': statsd,
 });
 const APP_VERSION = /([0-9]+)\.([0-9])$/.exec(pkg.version)[0];
 
@@ -45,6 +49,7 @@ registerSuite('amplitude', {
   afterEach: function() {
     process.stderr.write.restore();
     logger.info.resetHistory();
+    statsd.increment.resetHistory();
   },
 
   tests: {
@@ -184,6 +189,16 @@ registerSuite('amplitude', {
       assert.isTrue(
         logger.info.calledOnceWith('rawAmplitudeData', { event, context })
       );
+      sinon.assert.calledThrice(statsd.increment);
+      sinon.assert.calledWith(
+        statsd.increment.firstCall,
+        'amplitude.event.raw'
+      );
+      sinon.assert.calledWith(statsd.increment.secondCall, 'amplitude.event');
+      sinon.assert.calledWith(
+        statsd.increment.thirdCall,
+        'amplitude.event.dropped'
+      );
     },
 
     'flow.reset-password.submit': () => {
@@ -234,6 +249,8 @@ registerSuite('amplitude', {
       const args = logger.info.args[0];
       assert.lengthOf(args, 2);
       assert.equal(args[0], 'amplitudeEvent');
+      sinon.assert.calledOnce(statsd.increment);
+      sinon.assert.calledWith(statsd.increment.firstCall, 'amplitude.event');
       assert.deepEqual(args[1], {
         app_version: APP_VERSION,
         country: 'United States',

packages/fxa-payments-server/package-lock.json

@@ -12116,9 +12116,9 @@
       "integrity": "sha512-f/wzC2QaWBs7t9IYqB4T3sR1xviIViXJRJTWBlx2Gf3g0Xi5vI7Yy4koXQ1c9OYDGHN9sBy1DQ2AB8fqZBWhUg=="
     },
     "hot-shots": {
-      "version": "7.1.0",
-      "resolved": "https://registry.npmjs.org/hot-shots/-/hot-shots-7.1.0.tgz",
-      "integrity": "sha512-O0ubzu4UmoRna8DweA+KSnFwqF9Xkg+Mu0/Q8DWK2nu97n8ZFWcEGao7aJ8tqIPD0Mz09ApypwJN7SOxI66Ujw==",
+      "version": "7.4.0",
+      "resolved": "https://registry.npmjs.org/hot-shots/-/hot-shots-7.4.0.tgz",
+      "integrity": "sha512-5A9WP38HdgRcatk2LHyFF5d5guGsm76V/6u+i6dgg0eZzHYGF1bndtmCZIIzIE+LTGZwci6+AP+CgV6vHW8JAQ==",
       "requires": {
         "unix-dgram": "2.0.x"
       }
@@ -22239,6 +22239,11 @@
       "resolved": "https://registry.npmjs.org/typedarray/-/typedarray-0.0.6.tgz",
       "integrity": "sha1-hnrHTjhkGHsdPUfZlqeOxciDB3c="
     },
+    "typedi": {
+      "version": "0.8.0",
+      "resolved": "https://registry.npmjs.org/typedi/-/typedi-0.8.0.tgz",
+      "integrity": "sha512-/c7Bxnm6eh5kXx2I+mTuO+2OvoWni5+rXA3PhXwVWCtJRYmz3hMok5s1AKLzoDvNAZqj/Q/acGstN0ri5aQoOA=="
+    },
     "typescript": {
       "version": "3.8.3",
       "resolved": "https://registry.npmjs.org/typescript/-/typescript-3.8.3.tgz",
@@ -22338,9 +22343,9 @@
       "integrity": "sha512-rBJeI5CXAlmy1pV+617WB9J63U6XcazHHF2f2dbJix4XzpUF0RS3Zbj0FGIOCAva5P/d/GBOYaACQ1w+0azUkg=="
     },
     "unix-dgram": {
-      "version": "2.0.3",
-      "resolved": "https://registry.npmjs.org/unix-dgram/-/unix-dgram-2.0.3.tgz",
-      "integrity": "sha512-Bay5CkSLcdypcBCsxvHEvaG3mftzT5FlUnRToPWEAVxwYI8NI/8zSJ/Gknlp86MPhV6hBA8I8TBsETj2tssoHQ==",
+      "version": "2.0.4",
+      "resolved": "https://registry.npmjs.org/unix-dgram/-/unix-dgram-2.0.4.tgz",
+      "integrity": "sha512-7tpK6x7ls7J7pDrrAU63h93R0dVhRbPwiRRCawR10cl+2e1VOvF3bHlVJc6WI1dl/8qk5He673QU+Ogv7bPNaw==",
       "optional": true,
       "requires": {
         "bindings": "^1.3.0",

packages/fxa-payments-server/package.json

@@ -131,6 +131,7 @@
     "redux-thunk": "^2.3.0",
     "serve-static": "^1.13.2",
     "type-to-reducer": "^1.2.0",
+    "typedi": "^0.8.0",
     "uuid": "^7.0.2"
   },
   "engines": {

packages/fxa-payments-server/server/lib/amplitude.js

@@ -20,6 +20,8 @@ const log = require('./logging/log')();
 const ua = require('../../../fxa-shared/metrics/user-agent');
 const { version: VERSION } = require('../../package.json');
 const Sentry = require('@sentry/node');
+const { Container } = require('typedi');
+const { StatsD } = require('hot-shots');
 
 const FUZZY_EVENTS = new Map([
   [
@@ -35,6 +37,7 @@ const FUZZY_EVENTS = new Map([
 const transform = initialize({}, {}, FUZZY_EVENTS);
 
 module.exports = (event, request, data) => {
+  const statsd = Container.get(StatsD);
   if (!amplitude.enabled || !event || !request || !data) {
     return;
   }
@@ -85,10 +88,13 @@ module.exports = (event, request, data) => {
       },
     };
     log.info('rawAmplitudeData', rawEvent);
+    statsd.increment('amplitude.event.raw');
   }
 
   const userAgent = ua.parse(request.headers['user-agent']);
 
+  statsd.increment('amplitude.event');
+
   const amplitudeEvent = transform(event, {
     version: VERSION,
     ...mapBrowser(userAgent),
@@ -127,5 +133,7 @@ module.exports = (event, request, data) => {
     // Amplitude events are logged to stdout, where they are picked up by the
     // stackdriver logging agent.
     log.info('amplitudeEvent', amplitudeEvent);
+  } else {
+    statsd.increment('amplitude.event.dropped');
   }
 };

packages/fxa-payments-server/server/lib/amplitude.test.js

@@ -26,6 +26,8 @@ jest.mock('@sentry/node', () => ({
   ...mockSentry,
 }));
 const Sentry = require('@sentry/node');
+const { Container } = require('typedi');
+const { StatsD } = require('hot-shots');
 const amplitude = require('./amplitude');
 const log = require('./logging/log')();
 jest.spyOn(log, 'info').mockImplementation(() => {});
@@ -89,14 +91,18 @@ describe('lib/amplitude', () => {
     mockSchemaValidatorFn.mockReset();
     mockAmplitudeConfig.schemaValidation = true;
     mockAmplitudeConfig.rawEvents = false;
+    Container.set(StatsD, { increment: jest.fn() });
   });
   it('logs a correctly formatted message', () => {
+    const statsd = Container.get(StatsD);
     amplitude(mocks.event, mocks.request, mocks.data);
     expect(log.info).toHaveBeenCalledTimes(1);
     expect(log.info.mock.calls[0][0]).toMatch('amplitudeEvent');
     expect(log.info.mock.calls[0][1]).toMatchObject(expectedOutput);
+    expect(statsd.increment).toHaveBeenCalledTimes(1);
   });
   it('logs raw events', () => {
+    const statsd = Container.get(StatsD);
     const expectedContext = {
       eventSource: 'payments',
       version: pkg.version,
@@ -120,6 +126,9 @@ describe('lib/amplitude', () => {
       event: mocks.event,
       context: expectedContext,
     });
+    expect(statsd.increment).toHaveBeenCalledTimes(2);
+    expect(statsd.increment.mock.calls[0][0]).toBe('amplitude.event.raw');
+    expect(statsd.increment.mock.calls[1][0]).toBe('amplitude.event');
   });
   describe('validates inputs', () => {
     it('returns if `event` is missing', () => {
@@ -135,8 +144,12 @@ describe('lib/amplitude', () => {
       expect(log.info).not.toHaveBeenCalled();
     });
     it('returns if the message format does not match `amplitude.str.str`', () => {
+      const statsd = Container.get(StatsD);
       amplitude(mocks.invalidEventType, mocks.request, mocks.data);
       expect(log.info).not.toHaveBeenCalled();
+      expect(statsd.increment).toHaveBeenCalledTimes(2);
+      expect(statsd.increment.mock.calls[0][0]).toBe('amplitude.event');
+      expect(statsd.increment.mock.calls[1][0]).toBe('amplitude.event.dropped');
     });
     it('calls validate to perform schema validation', () => {
       amplitude(mocks.event, mocks.request, mocks.data);

packages/fxa-payments-server/server/lib/server.js

@@ -7,6 +7,7 @@
 module.exports = () => {
   const path = require('path');
   const fs = require('fs');
+  const { Container } = require('typedi');
 
   // setup version first for the rest of the modules
   const log = require('./logging/log');
@@ -42,7 +43,9 @@ module.exports = () => {
       })
     : {
         timing: NOOP,
+        increment: NOOP,
       };
+  Container.set(StatsD, statsd);
 
   const routes = require('./routes')(statsd);