refactor(server): remove separate notifier process (#1800) r=vbudhram

README.md

@@ -44,7 +44,6 @@ This runs a script `scripts/start-local.sh` as defined in `package.json`. This w
 * `bin/key_server.js` on port 9000
 * `test/mail_helper.js` on port 9001
 * `./node_modules/fxa-customs-server/bin/customs_server.js` on port 7000
-* `bin/notifier.js` (no port)
 
 When you `Ctrl-c` your server, all 4 processes will be stopped.
 

bin/key_server.js

@@ -29,7 +29,10 @@ function run(config) {
       v && v.constructor === RegExp ? v.toString() : v
     )
 
-  log.stdout.write('{"event":"config","data":' + stringifiedConfig + '}\n')
+  log.notifier.send({
+    event: 'config',
+    data: config
+  })
 
   if (config.env !== 'prod') {
     log.info(stringifiedConfig, 'starting config')

docs/self-host.docker

@@ -10,7 +10,7 @@ USER fxa
 RUN npm install --production \
   && npm cache clear
 
-CMD node ./bin/key_server.js | node ./bin/notifier.js
+CMD node ./bin/key_server.js
 
 # Expose ports
 EXPOSE 9000

lib/log.js

@@ -12,6 +12,7 @@ const logConfig = config.get('log')
 const StatsDCollector = require('./metrics/statsd')
 const unbuffer = require('./crypto/butil').unbuffer
 
+
 function Lug(options) {
   EventEmitter.call(this)
   this.name = options.name || 'fxa-auth-server'
@@ -27,6 +28,8 @@ function Lug(options) {
 
   this.statsd = new StatsDCollector(this.logger)
   this.statsd.init()
+
+  this.notifier = require('./notifier')(this)
 }
 util.inherits(Lug, EventEmitter)
 
@@ -138,9 +141,7 @@ Lug.prototype.summary = function (request, response) {
 
 
 // Broadcast an event to attached services, such as sync.
-// In production, these events are read from stdout
-// and broadcast to relying services over SNS/SQS.
-
+// In production, these events are broadcast to relying services over SNS/SQS.
 Lug.prototype.notifyAttachedServices = function (name, request, data) {
   return request.gatherMetricsContext({})
     .then(
@@ -150,7 +151,7 @@ Lug.prototype.notifyAttachedServices = function (name, request, data) {
           data: unbuffer(data)
         }
         e.data.metricsContext = metricsContextData
-        this.stdout.write(JSON.stringify(e) + '\n')
+        this.notifier.send(e)
       }
     )
 }

lib/notifier.js

@@ -0,0 +1,55 @@
+/* This Source Code Form is subject to the terms of the Mozilla Public
+ * License, v. 2.0. If a copy of the MPL was not distributed with this
+ * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
+
+'use strict'
+
+/**
+ * This notifier is called by the logger via `notifyAttachedServices`
+ * to send notifications to Amazon SNS/SQS.
+ */
+const AWS = require('aws-sdk')
+const config = require('../config')
+
+const notifierSnsTopicArn = config.get('snsTopicArn')
+let sns = { publish: function (msg, cb) {
+  cb(null, {disabled: true})
+}}
+
+if (notifierSnsTopicArn !== 'disabled') {
+  // Pull the region info out of the topic arn.
+  // For some reason we need to pass this in explicitly.
+  // Format is "arn:aws:sns:<region>:<other junk>"
+  const region = notifierSnsTopicArn.split(':')[3]
+  // This will pull in default credentials, region data etc
+  // from the metadata service available to the instance.
+  // It's magic, and it's awesome.
+  sns = new AWS.SNS({region: region})
+}
+
+module.exports = function notifierLog(log) {
+  return {
+    send: (event, callback) => {
+      const msg = event.data || {}
+      msg.event = event.event
+
+      sns.publish({
+        TopicArn: notifierSnsTopicArn,
+        Message: JSON.stringify(msg)
+      }, (err, data) => {
+        if (err) {
+          log.error({op: 'Notifier.publish', err: err})
+        } else {
+          log.trace({op: 'Notifier.publish', success: true, data: data})
+        }
+
+        if (callback) {
+          callback(err, data)
+        }
+      })
+
+    },
+    // exported for testing purposes
+    __sns: sns
+  }
+}

scripts/start-local-mysql.sh

@@ -11,7 +11,7 @@ node ./node_modules/fxa-auth-db-mysql/bin/db_patcher.js
 node ./node_modules/fxa-auth-db-mysql/bin/server.js &
 DB=$!
 
-node ./bin/key_server.js | node ./bin/notifier.js >/dev/null
+node ./bin/key_server.js
 
 kill $MH
 kill $DB

scripts/start-local.sh

@@ -9,7 +9,7 @@ MH=$!
 node ./node_modules/fxa-auth-db-mysql/bin/mem.js &
 DB=$!
 
-node ./bin/key_server.js | node ./bin/notifier.js >/dev/null
+node ./bin/key_server.js
 
 kill $MH
 kill $DB

scripts/start-server.sh

@@ -1,3 +1,3 @@
 #!/usr/bin/env bash
-node ./bin/key_server.js | node ./bin/notifier.js >/dev/null
+node ./bin/key_server.js
 exit $PIPESTATUS[0]

test/local/ip_profiling.js

@@ -348,6 +348,7 @@ describe('IP Profiling', () => {
       })
 
       mockRequest.app = {
+        clientAddress: '63.245.221.32',
         isSuspiciousRequest: true
       }
 

test/local/notifier.js

@@ -0,0 +1,86 @@
+/* This Source Code Form is subject to the terms of the Mozilla Public
+ * License, v. 2.0. If a copy of the MPL was not distributed with this
+ * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
+
+'use strict'
+
+const ROOT_DIR = '../..'
+
+const proxyquire = require('proxyquire')
+const assert = require('insist')
+const sinon = require('sinon')
+
+describe('notifier', () => {
+  const log = {
+    error: sinon.spy(),
+    trace: sinon.spy()
+  }
+
+  beforeEach(() => {
+    log.error.reset()
+    log.trace.reset()
+  })
+
+  it('works with sns configuration', () => {
+    const config = {
+      get: (key) => {
+        if (key === 'snsTopicArn') {
+          return 'arn:aws:sns:us-west-2:927034868275:foo'
+        }
+      }
+    }
+
+    const notifier = proxyquire(`${ROOT_DIR}/lib/notifier`, {
+      '../config': config
+    })(log)
+
+    notifier.__sns.publish = sinon.spy((event, cb) => {
+      cb(null, event)
+    })
+
+    notifier.send({
+      event: {
+        stuff: true
+      }
+    })
+
+    assert.deepEqual(log.trace.args[0][0], {
+      op: 'Notifier.publish',
+      data: {
+        TopicArn: 'arn:aws:sns:us-west-2:927034868275:foo',
+        Message: '{\"event\":{\"stuff\":true}}'
+      },
+      success: true
+    })
+    assert.equal(log.error.called, false)
+  })
+
+  it('works with disabled configuration', () => {
+    const config = {
+      get: (key) => {
+        if (key === 'snsTopicArn') {
+          return 'disabled'
+        }
+      }
+    }
+    const notifier = proxyquire(`${ROOT_DIR}/lib/notifier`, {
+      '../config': config
+    })(log)
+
+    notifier.send({
+      stuff: true
+    }, () => {
+      assert.deepEqual(log.trace.args[0][0], {
+        op: 'Notifier.publish',
+        data: {
+          disabled: true
+        },
+        success: true
+      })
+      assert.equal(log.trace.args[0][0].data.disabled, true)
+      assert.equal(log.error.called, false)
+    })
+
+  })
+
+})

test/local/routes/account.js

@@ -178,24 +178,16 @@ describe('/account/reset', function () {
 
 describe('/account/create', () => {
   it('should create an account', () => {
-    // We want to test what's actually written to stdout by the logger.
-    const mockLog = log('ERROR', 'test', {
-      stdout: {
-        on: sinon.spy(),
-        write: sinon.spy()
-      },
-      stderr: {
-        on: sinon.spy(),
-        write: sinon.spy()
-      }
-    })
+    const mockLog = log('ERROR', 'test')
     mockLog.activityEvent = sinon.spy(() => {
       return P.resolve()
     })
     mockLog.flowEvent = sinon.spy(() => {
       return P.resolve()
     })
     mockLog.error = sinon.spy()
+    mockLog.notifier.send = sinon.spy()
+
     const mockMetricsContext = mocks.mockMetricsContext()
     const mockRequest = mocks.mockRequest({
       locale: 'en-GB',
@@ -266,8 +258,8 @@ describe('/account/create', () => {
     return runTest(route, mockRequest, () => {
       assert.equal(mockDB.createAccount.callCount, 1, 'createAccount was called')
 
-      assert.equal(mockLog.stdout.write.callCount, 1, 'an sqs event was logged')
-      var eventData = JSON.parse(mockLog.stdout.write.getCall(0).args[0])
+      assert.equal(mockLog.notifier.send.callCount, 1, 'an sqs event was logged')
+      var eventData = mockLog.notifier.send.getCall(0).args[0]
       assert.equal(eventData.event, 'login', 'it was a login event')
       assert.equal(eventData.data.service, 'sync', 'it was for sync')
       assert.equal(eventData.data.email, TEST_EMAIL, 'it was for the correct email')
@@ -363,23 +355,14 @@ describe('/account/login', function () {
       codeLifetime: 1000
     }
   }
-  // We want to test what's actually written to stdout by the logger.
-  const mockLog = log('ERROR', 'test', {
-    stdout: {
-      on: sinon.spy(),
-      write: sinon.spy()
-    },
-    stderr: {
-      on: sinon.spy(),
-      write: sinon.spy()
-    }
-  })
+  const mockLog = log('ERROR', 'test')
   mockLog.activityEvent = sinon.spy(() => {
     return P.resolve()
   })
   mockLog.flowEvent = sinon.spy(() => {
     return P.resolve()
   })
+  mockLog.notifier.send = sinon.spy()
   const mockMetricsContext = mocks.mockMetricsContext()
 
   const mockRequest = mocks.mockRequest({
@@ -470,7 +453,6 @@ describe('/account/login', function () {
   afterEach(() => {
     mockLog.activityEvent.reset()
     mockLog.flowEvent.reset()
-    mockLog.stdout.write.reset()
     mockMailer.sendNewDeviceLoginNotification = sinon.spy(() => P.resolve([]))
     mockMailer.sendVerifyLoginEmail.reset()
     mockMailer.sendVerifyCode.reset()
@@ -492,8 +474,8 @@ describe('/account/login', function () {
       assert.equal(mockDB.emailRecord.callCount, 1, 'db.emailRecord was called')
       assert.equal(mockDB.createSessionToken.callCount, 1, 'db.createSessionToken was called')
 
-      assert.equal(mockLog.stdout.write.callCount, 1, 'an sqs event was logged')
-      var eventData = JSON.parse(mockLog.stdout.write.getCall(0).args[0])
+      assert.equal(mockLog.notifier.send.callCount, 1, 'an sqs event was logged')
+      var eventData = mockLog.notifier.send.getCall(0).args[0]
       assert.equal(eventData.event, 'login', 'it was a login event')
       assert.equal(eventData.data.service, 'sync', 'it was for sync')
       assert.equal(eventData.data.email, TEST_EMAIL, 'it was for the correct email')