From 99cbc20778975f6f73be2dc9568128ce2a0da80a Mon Sep 17 00:00:00 2001 From: epriestley Date: Tue, 14 Apr 2020 13:17:41 -0700 Subject: [PATCH] Reduce the verbosity of the "Aphlict" log Summary: See PHI1692. Currently, the Aphlict log is ridiculously verbose. As an initial pass at improving this: - When starting in "debug" mode, pass "--debug=1" to Node. - In Node, separate logging into "log" (lower-volume, more-important messages) and "trace" (higher-volume, less-important messages). - Only print "trace" messages in "debug" mode. Test Plan: Ran Aphlict in debug and non-debug modes. Behavior unchanged in debug mode, but log has more sensible verbosity in non-debug mode. Differential Revision: https://secure.phabricator.com/D21115 --- .../PhabricatorAphlictManagementWorkflow.php | 9 ++++- support/aphlict/server/aphlict_server.js | 9 ++++- .../aphlict/server/lib/AphlictAdminServer.js | 18 +++++++-- .../aphlict/server/lib/AphlictClientServer.js | 37 ++++++++++++++----- support/aphlict/server/lib/AphlictLog.js | 14 +++++++ 5 files changed, 70 insertions(+), 17 deletions(-) diff --git a/src/applications/aphlict/management/PhabricatorAphlictManagementWorkflow.php b/src/applications/aphlict/management/PhabricatorAphlictManagementWorkflow.php index 5bcb66c135..3d1a26c91d 100644 --- a/src/applications/aphlict/management/PhabricatorAphlictManagementWorkflow.php +++ b/src/applications/aphlict/management/PhabricatorAphlictManagementWorkflow.php @@ -550,11 +550,18 @@ abstract class PhabricatorAphlictManagementWorkflow } private function getStartCommand(array $server_argv) { + $launch_argv = array(); + + if ($this->debug) { + $launch_argv[] = '--debug=1'; + } + return csprintf( - '%R %Ls -- %s %Ls', + '%R %Ls -- %s %Ls %Ls', $this->getNodeBinary(), $this->getNodeArgv(), $this->getAphlictScriptPath(), + $launch_argv, $server_argv); } diff --git a/support/aphlict/server/aphlict_server.js b/support/aphlict/server/aphlict_server.js index 34bddf4fa9..3ccfefb6f5 100644 --- a/support/aphlict/server/aphlict_server.js +++ b/support/aphlict/server/aphlict_server.js @@ -9,6 +9,7 @@ var fs = require('fs'); function parse_command_line_arguments(argv) { var args = { test: false, + debug: false, config: null }; @@ -34,12 +35,16 @@ function parse_config(args) { require('./lib/AphlictLog'); -var debug = new JX.AphlictLog() - .addConsole(console); +var debug = new JX.AphlictLog(); var args = parse_command_line_arguments(process.argv); var config = parse_config(args); +if (args.test || args.debug) { + debug.addConsole(console); + debug.setTrace(true); +} + function set_exit_code(code) { process.on('exit', function() { process.exit(code); diff --git a/support/aphlict/server/lib/AphlictAdminServer.js b/support/aphlict/server/lib/AphlictAdminServer.js index dd428063c2..e2192e8afd 100644 --- a/support/aphlict/server/lib/AphlictAdminServer.js +++ b/support/aphlict/server/lib/AphlictAdminServer.js @@ -4,7 +4,6 @@ var JX = require('./javelin').JX; require('./AphlictListenerList'); -var http = require('http'); var url = require('url'); JX.install('AphlictAdminServer', { @@ -54,6 +53,17 @@ JX.install('AphlictAdminServer', { return this; }, + trace: function() { + var logger = this.getLogger(); + if (!logger) { + return; + } + + logger.trace.apply(logger, arguments); + + return this; + }, + listen: function() { return this._server.listen.apply(this._server, arguments); }, @@ -76,7 +86,7 @@ JX.install('AphlictAdminServer', { try { var msg = JSON.parse(body); - self.log( + self.trace( 'Received notification (' + instance + '): ' + JSON.stringify(msg)); ++self._messagesIn; @@ -201,13 +211,13 @@ JX.install('AphlictAdminServer', { listener.writeMessage(message); ++this._messagesOut; - this.log( + this.trace( '<%s> Wrote Message', listener.getDescription()); } catch (error) { list.removeListener(listener); - this.log( + this.trace( '<%s> Write Error: %s', listener.getDescription(), error); diff --git a/support/aphlict/server/lib/AphlictClientServer.js b/support/aphlict/server/lib/AphlictClientServer.js index 989b3f1db1..4157906bb6 100644 --- a/support/aphlict/server/lib/AphlictClientServer.js +++ b/support/aphlict/server/lib/AphlictClientServer.js @@ -3,7 +3,6 @@ var JX = require('./javelin').JX; require('./AphlictListenerList'); -require('./AphlictLog'); var url = require('url'); var util = require('util'); @@ -60,6 +59,17 @@ JX.install('AphlictClientServer', { return this; }, + trace: function() { + var logger = this.getLogger(); + if (!logger) { + return; + } + + logger.trace.apply(logger, arguments); + + return this; + }, + _onrequest: function(request, response) { // The websocket code upgrades connections before they get here, so // this only handles normal HTTP connections. We just fail them with @@ -104,17 +114,24 @@ JX.install('AphlictClientServer', { var listener = self.getListenerList(instance).addListener(ws); - function log() { - self.log( - util.format('<%s>', listener.getDescription()) + + function msg(argv) { + return util.format('<%s>', listener.getDescription()) + ' ' + - util.format.apply(null, arguments)); + util.format.apply(null, argv); } - log('Connected from %s.', ws._socket.remoteAddress); + function log() { + self.log(msg(arguments)); + } + + function trace() { + self.trace(msg(arguments)); + } + + trace('Connected from %s.', ws._socket.remoteAddress); ws.on('message', function(data) { - log('Received message: %s', data); + trace('Received message: %s', data); var message; try { @@ -126,14 +143,14 @@ JX.install('AphlictClientServer', { switch (message.command) { case 'subscribe': - log( + trace( 'Subscribed to: %s', JSON.stringify(message.data)); listener.subscribe(message.data); break; case 'unsubscribe': - log( + trace( 'Unsubscribed from: %s', JSON.stringify(message.data)); listener.unsubscribe(message.data); @@ -180,7 +197,7 @@ JX.install('AphlictClientServer', { ws.on('close', function() { self.getListenerList(instance).removeListener(listener); - log('Disconnected.'); + trace('Disconnected.'); }); }); diff --git a/support/aphlict/server/lib/AphlictLog.js b/support/aphlict/server/lib/AphlictLog.js index 77d40793cb..62513b0f01 100644 --- a/support/aphlict/server/lib/AphlictLog.js +++ b/support/aphlict/server/lib/AphlictLog.js @@ -14,6 +14,12 @@ JX.install('AphlictLog', { members: { _consoles: null, _logs: null, + _trace: null, + + setTrace: function(trace) { + this._trace = trace; + return this; + }, addConsole: function(console) { this._consoles.push(console); @@ -29,6 +35,14 @@ JX.install('AphlictLog', { return this; }, + trace: function() { + if (!this._trace) { + return; + } + + return this.log.apply(this, arguments); + }, + log: function() { var str = util.format.apply(null, arguments); var date = new Date().toLocaleString();