Merge pull request #33 from Financial-Times/remove-new-lines

Remove new lines from logged values

package.json

@@ -25,7 +25,8 @@
     "eslint": "^3.0.1",
     "lintspaces-cli": "^0.3.0",
     "mocha": "^2.2.1",
-    "sinon": "^1.17.0"
+    "sinon": "^1.17.0",
+    "sinon-chai": "^2.8.0"
   },
   "engines": {
     "node": ">=4.4.7"

src/lib/format.js

@@ -1,39 +1,25 @@
-const extractErrorDetails = err => {
-	const deets = {
-		error_message: err.message,
-		error_name: err.name
-	};
-	if (err.stack) {
-		// logs need to all be on one line, so remove newlines from the stacktrace
-		deets.error_stack = err.stack.replace(/\n/g, '; ');
-	}
-
-	return deets;
-};
+const sanitise = value => (typeof value === 'string') ? value.replace(/"/g, '\'').replace(/\n/g, '; ') : value;
 
-const deQuote = value => (typeof value === 'string') ? value.replace(/"/g, '\'') : value;
+const formatMessage = message => sanitise(message);
 
-const formatMessage = message => deQuote(message);
-
-const formatError = obj => (obj instanceof Error) ? extractErrorDetails(obj) : obj;
+const formatValue = value => {
+	if (Array.isArray(value)) {
+		return `"${value.map(sanitise).join(',')}"`;
+	} else {
+		// wrap in quotes, if it contains a space
+		const sanitisedValue = sanitise(value);
+		return (/\s/.test(sanitisedValue)) ? `"${sanitisedValue}"` : sanitisedValue;
+	}
+};
 
 const formatFields = fields => {
 	const formattedFields = Object.keys(fields)
-		.map(key => {
-			let value = formatValue(fields[key]);
-			return `${key}=${value}`;
+		.map(fieldName => {
+			const fieldValue = formatValue(fields[fieldName]);
+			return `${fieldName}=${fieldValue}`;
 		});
 
 	return formattedFields.join(' ');
 };
 
-const formatValue = value => {
-	if (Array.isArray(value)) {
-		return `"${value.join(',')}"`;
-	} else {
-		// wrap in quotes, if it contains a space
-		return (/\s/.test(value)) ? `"${deQuote(value)}"` : deQuote(value);
-	}
-};
-
-export { formatError, formatMessage, formatFields, formatValue }
+export { formatMessage as message, formatFields as fields }

src/lib/formatter.js

@@ -1,10 +1,11 @@
-import { formatMessage, formatFields } from './format';
-
-const nonEmpty = item => item;
+import * as format from './format';
+import * as utils from './utils';
 
 export default ({ level, message = '', meta = {} }) => {
-	const fields = level ? Object.assign({ level }, meta) : meta;
-	return [formatMessage(message), formatFields(fields)]
-		.filter(nonEmpty)
+	if (level) {
+		meta.level = level
+	}
+	return [format.message(message), format.fields(meta)]
+		.filter(utils.identity)
 		.join(' ');
 }

src/lib/logger.js

@@ -0,0 +1,103 @@
+import winston from 'winston';
+
+import Splunk from './transports/splunk';
+import formatter from './formatter';
+import * as utils from './utils';
+
+const extractErrorDetails = obj => {
+	if (obj instanceof Error) {
+		const deets = {
+			error_message: obj.message,
+			error_name: obj.name
+		};
+		if ('stack' in obj) {
+			deets.error_stack = obj.stack;
+		}
+
+		return deets;
+	} else {
+		return obj;
+	}
+};
+
+const loggerArgs = (level, message, ...metas) => {
+	const args = [level];
+	// if not a string, assume it's a meta object
+	if (typeof message === 'string') {
+		args.push(message);
+	} else {
+		metas.unshift(message)
+	}
+	if (metas.length) {
+		args.push(
+			metas.reduceRight((currentFormattedMetas, meta) =>
+				Object.assign({}, currentFormattedMetas, extractErrorDetails(meta)),
+			{})
+		);
+	}
+
+	return args;
+};
+
+class Logger {
+
+	constructor (deps = {}) {
+		this.deps = Object.assign({ winston, formatter, Splunk }, deps);
+		this.logger = new (this.deps.winston.Logger)();
+		// create logging methods
+		Object.keys(this.logger.levels).forEach(level =>
+			this[level] = (...args) => this.log(level, ...args)
+		);
+	}
+
+	log (level, message, ...metas) {
+		const args = loggerArgs(level, message, ...metas).filter(utils.identity);
+		this.logger.log.apply(this.logger, args);
+	}
+
+	addConsole (level = 'info', opts = {}) {
+		if (this.logger.transports.console) {
+			return;
+		}
+		this.logger.add(
+			this.deps.winston.transports.Console,
+			Object.assign({ level, formatter: this.deps.formatter }, opts)
+		);
+	}
+
+	removeConsole () {
+		if (!this.logger.transports.console) {
+			return;
+		}
+		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');
+	}
+
+	clearLoggers () {
+		Object.keys(this.logger.transports)
+			.forEach(logger => this.logger.remove(logger));
+	}
+
+}
+
+export default Logger;

src/lib/utils.js

@@ -0,0 +1,3 @@
+const identity = item => item;
+
+export { identity }

src/main.js

@@ -1,88 +1,4 @@
-import winston from 'winston';
-import Splunk from './lib/transports/splunk';
-import formatter from './lib/formatter';
-import { formatError } from './lib/format';
-
-const loggerArgs = (level, message, ...metas) => {
-	const args = [level];
-	// if not a string, assume it's a meta object
-	if (typeof message === 'string') {
-		args.push(message);
-	} else {
-		metas.unshift(message)
-	}
-	if (metas.length) {
-		args.push(
-			metas.reduceRight((currentFormattedMetas, meta) =>
-				Object.assign({}, currentFormattedMetas, formatError(meta)),
-			{})
-		);
-	}
-
-	return args;
-};
-
-const empty = item => item;
-
-class Logger {
-
-	constructor () {
-		this.logger = new (winston.Logger)();
-		// create logging methods
-		Object.keys(this.logger.levels).forEach(level =>
-			this[level] = (...args) => this.log(level, ...args)
-		);
-	}
-
-	log (level, message, ...metas) {
-		const args = loggerArgs(level, message, ...metas).filter(empty);
-		this.logger.log.apply(this.logger, args);
-	}
-
-	addConsole (level = 'info', opts = {}) {
-		if (this.logger.transports.console) {
-			return;
-		}
-		this.logger.add(
-			winston.transports.Console,
-			Object.assign({ level, formatter }, opts)
-		);
-	}
-
-	removeConsole () {
-		if (!this.logger.transports.console) {
-			return;
-		}
-		this.logger.remove('console');
-	}
-
-	addSplunk (splunkUrl, level = 'info', opts = {}) {
-		if (this.logger.transports.splunk) {
-			return;
-		}
-		if (!splunkUrl) {
-			this.logger.warn('No `splunkUrl` supplied');
-			return false;
-		}
-		this.logger.add(
-			Splunk,
-			Object.assign({ level, splunkUrl }, opts)
-		);
-	}
-
-	removeSplunk () {
-		if (!this.logger.transports.splunk) {
-			return;
-		}
-		this.logger.remove('splunk');
-	}
-
-	clearLoggers () {
-		Object.keys(this.logger.transports)
-			.forEach(logger => this.logger.remove(logger));
-	}
-
-}
+import Logger from './lib/logger';
 
 const logger = new Logger();
 

test/lib/agent.test.js

@@ -26,7 +26,7 @@ describe('Agent', () => {
 			});
 			server.listen(port);
 		});
-		agent = fork(path.resolve(__dirname, '..', '..', 'build/lib/agent.js'), [`${host}/${appName}`]);
+		agent = fork(path.resolve(__dirname, '..', '..', 'build', 'lib', 'agent.js'), [`${host}/${appName}`]);
 	});
 
 	afterEach(done => {