From 5b972ad4de39f5e5fb24d083299ca707b366c174 Mon Sep 17 00:00:00 2001 From: Kat Downes Date: Fri, 21 Sep 2018 10:09:09 +0100 Subject: [PATCH] remove splunk transport so that apps only use the splunkHEC transport (#75) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * remove splunk transport so that apps only use the splunkHEC transport 🐿 v2.10.3 * Update README to remove Splunk forwarder references 🐿 v2.10.3 --- README.md | 22 +++++------ src/lib/agent.js | 19 ---------- src/lib/app-logger.js | 24 +----------- src/lib/transports/splunk.js | 35 ----------------- src/main.js | 4 -- test/lib/agent.test.js | 48 ------------------------ test/lib/app-logger.test.js | 60 ------------------------------ test/lib/transports/splunk.test.js | 39 ------------------- 8 files changed, 11 insertions(+), 240 deletions(-) delete mode 100644 src/lib/agent.js delete mode 100644 src/lib/transports/splunk.js delete mode 100644 test/lib/agent.test.js delete mode 100644 test/lib/transports/splunk.test.js diff --git a/README.md b/README.md index 9d09148..f2651f1 100644 --- a/README.md +++ b/README.md @@ -1,11 +1,12 @@ # Next Logger [![Circle CI](https://circleci.com/gh/Financial-Times/n-logger.svg?style=svg)](https://circleci.com/gh/Financial-Times/n-logger) -Logging utility +N-logger is a Winston wrapper which sends logs to Splunk's Http Event Collector (HEC). ## Installation npm install @financial-times/n-logger + Ensure `SPLUNK_HEC_TOKEN` is in the app's shared folder in Vault. ## Usage @@ -15,6 +16,7 @@ Logging utility logger.info('Saying hello'); logger.warn('Everything’s mostly cool'); logger.error('Uh-oh', { field: 'some value' }); + logger.info({ event: 'UPDATE_NOTIFICATION', data: data }); const err = new Error('Whoops!'); logger.error('Uh-oh', err, { extra_field: 'boo' }); @@ -25,14 +27,14 @@ If using CommonJS modules ### Loggers -By default +By default, the following loggers are added: - * the `console` logger is added - * logger level can be set by `CONSOLE_LOG_LEVEL` env variable; defaults to `silly` - * the `splunk` logger is added if `NODE_ENV === production` - * logger level can be set by `SPLUNK_LOG_LEVEL` env variable; defaults to `warn` - * the `splunkHEC` logger is added if `NODE_ENV === production && SPLUNK_HEC_TOKEN` - * logger level can be set by `SPLUNK_LOG_LEVEL` env variable; defaults to `warn` + * The `console` logger + * logger level can be set by `CONSOLE_LOG_LEVEL` env var (defaults to `silly`). + + + * The `splunkHEC` logger, if `NODE_ENV === production && SPLUNK_HEC_TOKEN` + * logger level can be set by `SPLUNK_LOG_LEVEL` env var (defaults to `warn`). ### API @@ -48,10 +50,6 @@ By default #### removeConsole() -#### addSplunk(splunkUrl, level = 'info', opts = {}) - -#### removeSplunk() - #### addSplunkHEC(level = 'info', opts = {}) #### removeSplunkHEC() diff --git a/src/lib/agent.js b/src/lib/agent.js deleted file mode 100644 index b3718a2..0000000 --- a/src/lib/agent.js +++ /dev/null @@ -1,19 +0,0 @@ -const request = require('request'); -const https = require('https'); - -const httpsAgent = new https.Agent({ keepAlive: true }); - -const url = process.argv[2]; - -const onMessage = message => { - request({ - method : 'POST', - url : url, - followRedirect : true, - strictSSL: false, - pool : httpsAgent, - body : message - }); -}; - -process.on('message', onMessage); diff --git a/src/lib/app-logger.js b/src/lib/app-logger.js index 26a4ebf..4db709b 100644 --- a/src/lib/app-logger.js +++ b/src/lib/app-logger.js @@ -1,14 +1,13 @@ import winston from 'winston'; import Logger from './logger'; -import Splunk from './transports/splunk'; import SplunkHEC from './transports/splunkHEC'; class AppLogger extends Logger { constructor (deps = {}) { super(deps); - Object.assign(this.deps, { winston, Splunk, SplunkHEC }, deps); + Object.assign(this.deps, { winston, SplunkHEC }, deps); this.logger = new (this.deps.winston.Logger)({ transports: [ new (this.deps.winston.transports.Console)({ @@ -40,27 +39,6 @@ class AppLogger extends Logger { this.logger.remove('console'); } - addSplunk (splunkUrl, level = 'info', opts = {}) { - if (this.logger.transports.splunk) { - return; - } - if (!splunkUrl) { - this.warn('No `splunkUrl` supplied'); - return false; - } - this.logger.add( - this.deps.Splunk, - Object.assign({ level, splunkUrl }, opts) - ); - } - - removeSplunk () { - if (!this.logger.transports.splunk) { - return; - } - this.logger.remove('splunk'); - } - addSplunkHEC (level = 'info', opts = {}) { if (this.logger.transports.splunkHEC) { return; diff --git a/src/lib/transports/splunk.js b/src/lib/transports/splunk.js deleted file mode 100644 index 103b18c..0000000 --- a/src/lib/transports/splunk.js +++ /dev/null @@ -1,35 +0,0 @@ -import { fork } from 'child_process'; -import path from 'path'; -import winston from 'winston'; -import formatter from '../formatter'; - -class Splunk extends winston.Transport { - - constructor (opts) { - super(opts); - this.name = 'splunk'; - if (opts && opts.agent) { - this.agent = opts.agent; - this.agent.url = opts.splunkUrl; - } else { - this.agent = fork(path.resolve(__dirname, '..', 'agent.js'), [opts.splunkUrl]); - } - } - - log (level, message, meta, callback) { - const formattedMessage = formatter({ level, message, meta, splunkFriendly: true }); - // HACK: For AWS Lambda - // Compare the breaking API change somewhere ebetween 0.10 and 4.x.x - // https://nodejs.org/docs/v0.10.36/api/child_process.html#child_process_child_send_message_sendhandle - // https://nodejs.org/api/child_process.html#child_process_child_send_message_sendhandle_callback - if (/^v0\.10/.test(process.version)) { - this.agent.send(formattedMessage); - setTimeout(() => callback(undefined, true)); - } else { - this.agent.send(formattedMessage, (err) => callback(err, true)); - } - } - -} - -export default Splunk; diff --git a/src/main.js b/src/main.js index 464d645..ac3d444 100644 --- a/src/main.js +++ b/src/main.js @@ -17,10 +17,6 @@ const getLogger = () => { logger.addSplunkHEC(process.env.SPLUNK_LOG_LEVEL || 'warn'); } - // log to splunk only in production - if (process.env.NODE_ENV === 'production' && process.env.SPLUNK_URL) { - logger.addSplunk(process.env.SPLUNK_URL, process.env.SPLUNK_LOG_LEVEL || 'warn'); - } return logger; } }; diff --git a/test/lib/agent.test.js b/test/lib/agent.test.js deleted file mode 100644 index 94d41e6..0000000 --- a/test/lib/agent.test.js +++ /dev/null @@ -1,48 +0,0 @@ -import { fork } from 'child_process'; -import path from 'path'; -import http from 'http'; -import chai from 'chai'; -chai.should(); - -describe('Agent', () => { - const port = '6666'; - const host = `http://localhost:${port}`; - const appName = 'ft-next-front-page'; - - let promise; - let server; - let requestData = ''; - let agent; - - beforeEach(() => { - promise = new Promise(resolve => { - server = http.createServer(req => { - req.on('data', function (chunk) { - requestData += chunk.toString(); - }); - req.on('end', function (){ - resolve(requestData); - }); - }); - server.listen(port); - }); - agent = fork(path.resolve(__dirname, '..', '..', 'dist', 'lib', 'agent.js'), [`${host}/${appName}`]); - }); - - afterEach(done => { - agent.kill(); - server.close(done); - }); - - it('should exist', () => { - agent.should.exist; - }); - - it('should listen to sent messages', () => { - const msg = 'Hello, world'; - agent.send(msg); - - return promise.then(data => data.should.equal(msg)); - }); - -}); diff --git a/test/lib/app-logger.test.js b/test/lib/app-logger.test.js index 5bdedce..fa47749 100644 --- a/test/lib/app-logger.test.js +++ b/test/lib/app-logger.test.js @@ -183,66 +183,6 @@ describe('Logger', () => { }); - describe('#addSplunk', () => { - - it('should be able to add a splunk logger', () => { - const addSpy = sinon.spy(); - const SplunkSpy = sinon.spy(); - const winston = winstonStub({ add: addSpy }); - const logger = new Logger({ winston, Splunk: SplunkSpy }); - logger.addSplunk('http://splunk.ft.com'); - addSpy.should.always.have.been.calledWithExactly(SplunkSpy, { level: 'info', splunkUrl: 'http://splunk.ft.com' }); - }); - - it('should be able to set the console logger\'s level', () => { - const addSpy = sinon.spy(); - const SplunkSpy = sinon.spy(); - const winston = winstonStub({ add: addSpy }); - const logger = new Logger({ winston, Splunk: SplunkSpy }); - logger.addSplunk('http://splunk.ft.com', 'warn'); - addSpy.should.always.have.been.calledWithExactly(SplunkSpy, { level: 'warn', splunkUrl: 'http://splunk.ft.com' }); - }); - - it('should return false if no `splunkUrl` supplied', () => { - const addSpy = sinon.spy(); - const SplunkSpy = sinon.spy(); - const winston = winstonStub({ add: addSpy }); - const logger = new Logger({ winston, Splunk: SplunkSpy }); - logger.addSplunk().should.be.false; - addSpy.should.not.have.been.called; - }); - - it('should not be able to add if already added', () => { - const addSpy = sinon.spy(function () { this.transports.splunk = true; }); - const winston = winstonStub({ add: addSpy }); - const logger = new Logger({ winston }); - logger.addSplunk('http://splunk.ft.com'); - logger.addSplunk('http://splunk.ft.com'); - addSpy.should.have.been.calledOnce; - }); - - }); - - describe('#removeSplunk', () => { - - it('should be able to remove', () => { - const removeSpy = sinon.spy(); - const winston = winstonStub({ remove: removeSpy, transports: { splunk: true } }); - const logger = new Logger({ winston }); - logger.removeSplunk(); - removeSpy.should.always.have.been.calledWithExactly('splunk'); - }); - - it('should not be able to remove if not added', () => { - const removeSpy = sinon.spy(); - const winston = winstonStub({ remove: removeSpy }); - const logger = new Logger({ winston }); - logger.removeSplunk(); - removeSpy.should.not.have.been.called; - }); - - }); - describe('#addSplunkHEC', () => { it('should be able to add a splunkHEC logger', () => { diff --git a/test/lib/transports/splunk.test.js b/test/lib/transports/splunk.test.js deleted file mode 100644 index 69eca86..0000000 --- a/test/lib/transports/splunk.test.js +++ /dev/null @@ -1,39 +0,0 @@ -import sinon from 'sinon'; -import chai from 'chai'; -chai.should(); - -import Splunk from '../../../dist/lib/transports/splunk'; - -describe('Splunk', () => { - - it('should exist', () => { - Splunk.should.exist; - }); - - it('should be able to instantiate', () => { - const splunkTransport = new Splunk({ splunkUrl: 'http://splunk.ft.com/ft-next-front-page' }); - splunkTransport.should.exist; - splunkTransport.agent.kill(); - }); - - it('should send the correct message to the agent', () => { - const mockAgent = { - send: sinon.spy() - }; - const splunkTransport = new Splunk({ splunkUrl: 'http://splunk.ft.com/ft-next-front-page', agent: mockAgent }); - mockAgent.url.should.equal('http://splunk.ft.com/ft-next-front-page'); - splunkTransport.log('error', 'a message', { field: 'value'}); - mockAgent.send.called.should.be.true; - mockAgent.send.calledWith('a message field=value level=error').should.be.true; - }); - - it('should handle no message', () => { - const mockAgent = { - send: sinon.spy() - }; - const splunkTransport = new Splunk({ splunkUrl: 'http://splunk.ft.com/ft-next-front-page', agent: mockAgent }); - splunkTransport.log('error'); - mockAgent.send.calledWith('level=error').should.be.true; - }); - -});