Merge lp:~benji/juju-gui/websocket-logging into lp:juju-gui/experimental

[Benji York]

Adds a websocket recording and download feature.

When the websocket_capture feature flag is enabled (e.g.,
/:flags:/websocket_capture/) the websocket traffic will be recorded. When the
user presses Control-Shift-d, the log will be downloaded to their local
machine.

https://codereview.appspot.com/9953045/

[Brad Crittenden (bac)]

LGTM with minors.

https://codereview.appspot.com/9953045/diff/1/Makefile
File Makefile (right):

https://codereview.appspot.com/9953045/diff/1/Makefile#newcode320
Makefile:320: build-debug/juju-ui/websocketLogging.js \
camelCaseFileNames? I-thought-we-liked-hyphens.

https://codereview.appspot.com/9953045/diff/1/app/app.js
File app/app.js (right):

https://codereview.appspot.com/9953045/diff/1/app/app.js#newcode240
app/app.js:240: 'C-S-d': {
I assume that combination is a vimism.

https://codereview.appspot.com/9953045/diff/1/app/assets/javascripts/reconnecting-websocket.js
File app/assets/javascripts/reconnecting-websocket.js (right):

https://codereview.appspot.com/9953045/diff/1/app/assets/javascripts/reconnecting-websocket.js#newcode49
app/assets/javascripts/reconnecting-websocket.js:49:
bless you.

https://codereview.appspot.com/9953045/diff/1/app/websocketLogging.js
File app/websocketLogging.js (right):

https://codereview.appspot.com/9953045/diff/1/app/websocketLogging.js#newcode4
app/websocketLogging.js:4: Copyright (C) 2012-2013 Canonical Ltd.
i think new files should just list the year they were created. yeah, i
know, that's picky...

https://codereview.appspot.com/9953045/diff/1/app/websocketLogging.js#newcode91
app/websocketLogging.js:91: @param {Object} cfg general init config
object.
I think this was c-n-p as it should be 'log' and description.

Why do you even pass log since you bind 'this'? Just for clarity I
predict you'll say.

https://codereview.appspot.com/9953045/diff/1/app/websocketLogging.js#newcode110
app/websocketLogging.js:110: /* global saveAs: false */
I don't understand this comment. Is it a lint directive?

It might be nice to state where saveAs normally is found.

https://codereview.appspot.com/9953045/diff/1/test/test_websocket_logging.js
File test/test_websocket_logging.js (right):

https://codereview.appspot.com/9953045/diff/1/test/test_websocket_logging.js#newcode4
test/test_websocket_logging.js:4: Copyright (C) 2012-2013 Canonical Ltd.
s/2012-//

https://codereview.appspot.com/9953045/

[Nicola Larosa (teknico)]

LGTM, nice, thanks.

https://codereview.appspot.com/9953045/

[lp:~benji/juju-gui/websocket-logging]

706. By Benji York on 2013-06-03

changes stemming from review

707. By Benji York on 2013-06-03

add some yuidoc directives

=== modified file 'Makefile'
--- Makefile	2013-05-29 19:06:28 +0000
+++ Makefile	2013-06-03 17:41:26 +0000
@@ -317,6 +317,7 @@
 
 LINK_DEBUG_FILES=$(call shared-link-files-list,debug) \
 	build-debug/juju-ui/app.js \
+	build-debug/juju-ui/websocket-logging.js \
 	build-debug/juju-ui/models \
 	build-debug/juju-ui/store \
 	build-debug/juju-ui/subapps \
@@ -363,6 +364,7 @@
 $(LINK_DEBUG_FILES):
 	$(call link-files,debug)
 	ln -sf "$(PWD)/app/app.js" build-debug/juju-ui/
+	ln -sf "$(PWD)/app/websocket-logging.js" build-debug/juju-ui/
 	ln -sf "$(PWD)/app/models" build-debug/juju-ui/
 	ln -sf "$(PWD)/app/store" build-debug/juju-ui/
 	ln -sf "$(PWD)/app/subapps" build-debug/juju-ui/

=== modified file 'app/app.js'
--- app/app.js	2013-05-31 18:07:02 +0000
+++ app/app.js	2013-06-03 17:41:26 +0000
@@ -226,7 +226,6 @@
 
       'S-d': {
         callback: function(evt) {
-          /* global saveAs: false */
           this.env.exportEnvironment(function(r) {
             var exportData = JSON.stringify(r.result, undefined, 2);
             var exportBlob = new Blob([exportData],
@@ -235,8 +234,16 @@
           });
         },
         help: 'Export the environment'
+      },
+
+      'C-S-d': {
+        callback: function(evt) {
+          Y.fire('saveWebsocketLog');
+        },
+        help: 'Save the websocket log to a file'
       }
 
+
     },
 
     /**
@@ -354,6 +361,10 @@
         });
       }
 
+      if (window.flags.websocket_capture) {
+        this.websocketLogging = new Y.juju.WebsocketLogging();
+      }
+
       this.renderEnvironment = true;
       // If this property has a value other than '/' then
       // navigate to it after logging in.
@@ -1301,6 +1312,7 @@
     'juju-views',
     'juju-view-login',
     'juju-landscape',
+    'juju-websocket-logging',
     'io',
     'json-parse',
     'app-base',

=== modified file 'app/assets/javascripts/reconnecting-websocket.js'
--- app/assets/javascripts/reconnecting-websocket.js	2012-07-24 18:18:43 +0000
+++ app/assets/javascripts/reconnecting-websocket.js	2013-06-03 17:41:26 +0000
@@ -38,7 +38,7 @@
  *  onopen  // sometime later...
  *  onmessage
  *  onmessage
- *  etc... 
+ *  etc...
  *
  * It is API compatible with the standard WebSocket API.
  *
@@ -46,7 +46,7 @@
  */
 
 YUI.add("reconnecting-websocket", function(Y) {
-            
+
 function ReconnectingWebSocket(url, protocols) {
 
     // These can be altered by calling code.
@@ -58,7 +58,7 @@
     var ws;
     var forcedClose = false;
     var timedOut = false;
-    
+
     this.url = url;
     this.protocols = protocols;
     this.readyState = WebSocket.CONNECTING;
@@ -81,7 +81,7 @@
         if (self.debug || ReconnectingWebSocket.debugAll) {
             console.debug('ReconnectingWebSocket', 'attempt-connect', url);
         }
-        
+
         var localWs = ws;
         var timeout = setTimeout(function() {
             if (self.debug || ReconnectingWebSocket.debugAll) {
@@ -91,7 +91,7 @@
             localWs.close();
             timedOut = false;
         }, self.timeoutInterval);
-        
+
         ws.onopen = function(event) {
             clearTimeout(timeout);
             if (self.debug || ReconnectingWebSocket.debugAll) {
@@ -101,7 +101,7 @@
             reconnectAttempt = false;
             self.onopen(event);
         };
-        
+
         ws.onclose = function(event) {
             clearTimeout(timeout);
             ws = null;
@@ -125,7 +125,8 @@
             if (self.debug || ReconnectingWebSocket.debugAll) {
                 console.debug('ReconnectingWebSocket', 'onmessage', url, event.data);
             }
-        	self.onmessage(event);
+            Y.fire('websocketReceive', event.data);
+            self.onmessage(event);
         };
         ws.onerror = function(event) {
             if (self.debug || ReconnectingWebSocket.debugAll) {
@@ -141,6 +142,7 @@
             if (self.debug || ReconnectingWebSocket.debugAll) {
                 console.debug('ReconnectingWebSocket', 'send', url, data);
             }
+            Y.fire('websocketSend', data);
             return ws.send(data);
         } else {
             throw 'INVALID_STATE_ERR : Pausing to reconnect websocket';
@@ -174,4 +176,4 @@
 
 }, "0.1.0", {
         requires: ["base"]
-});
\ No newline at end of file
+});

=== modified file 'app/modules-debug.js'
--- app/modules-debug.js	2013-05-24 14:10:58 +0000
+++ app/modules-debug.js	2013-06-03 17:41:26 +0000
@@ -323,6 +323,10 @@
           fullpath: '/juju-ui/store/charm.js'
         },
 
+        'juju-websocket-logging': {
+          fullpath: '/juju-ui/websocket-logging.js'
+        },
+
         'juju-controllers': {
           use: [
             'juju-env',

=== added directory 'app/subapps/websocketLogging'
=== added file 'app/websocket-logging.js'
--- app/websocket-logging.js	1970-01-01 00:00:00 +0000
+++ app/websocket-logging.js	2013-06-03 17:41:26 +0000
@@ -0,0 +1,123 @@
+/*
+This file is part of the Juju GUI, which lets users view and manage Juju
+environments within a graphical interface (https://launchpad.net/juju-gui).
+Copyright (C) 2013 Canonical Ltd.
+
+This program is free software: you can redistribute it and/or modify it under
+the terms of the GNU Affero General Public License version 3, as published by
+the Free Software Foundation.
+
+This program is distributed in the hope that it will be useful, but WITHOUT
+ANY WARRANTY; without even the implied warranties of MERCHANTABILITY,
+SATISFACTORY QUALITY, or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU Affero
+General Public License for more details.
+
+You should have received a copy of the GNU Affero General Public License along
+with this program.  If not, see <http://www.gnu.org/licenses/>.
+*/
+
+'use strict';
+
+YUI.add('juju-websocket-logging', function(Y) {
+  var juju = Y.namespace('juju');
+
+  /**
+    A logger of websocket traffic for debugging purposes.
+
+    @class Browser
+    @extends {juju.SubApp}
+  */
+  juju.WebsocketLogging = Y.Base.create('WebsocketLogging', Y.Base, [], {
+
+    // The stored messages, their direction, and time.
+    log: undefined,
+
+    /**
+      Websocket logger initialization.
+
+      @method initializer
+      @param {Object} cfg general init config object.
+      @return {undefined} Side-effects only.
+    */
+    initializer: function(cfg) {
+      this.log = [];
+      this.setUpEventListeners();
+    },
+
+    /**
+      Bind the event listeners.
+
+      @method setUpEventListeners
+      @return {undefined} Side-effects only.
+    */
+    setUpEventListeners: function() {
+      var self = this;
+      Y.on('websocketSend', function(data) {
+        self.logMessage('to server', data);
+      });
+      Y.on('websocketReceive', function(data) {
+        self.logMessage('to client', data);
+      });
+      Y.on('saveWebsocketLog', function() {self.saveLog(self.log);});
+    },
+
+    /**
+      Generate a date/time string representing the current time.
+
+      @method timestamp
+      @static
+      @return {String} The current time in ISO 8601 format.
+    */
+    timestamp: function() {
+      return new Date().toISOString();
+    },
+
+    /**
+      Record a websocket message, its direction, and the current time.
+
+      @method logMessage
+      @param {String} direction The direction the message was sent.  Either 'to
+        server' or 'to client'.
+      @param {String} message The message that was sent over the websocket.
+      @return {undefined} Side-effects only.
+    */
+    logMessage: function(direction, message) {
+      this.log.push([this.timestamp(), direction, message].join('\n'));
+    },
+
+    /**
+      Save a log of websocket traffic
+
+      @method saveLog
+      @static
+      @param {Array} log An array of log lines.
+      @return {undefined} Side-effects only.
+    */
+    saveLog: function(log) {
+      var data = log.join('\n');
+      var mimeType = 'text/plain;charset=utf-8';
+      var blob;
+      try {
+        blob = new Blob([data],
+            {type: mimeType});
+      } catch (e) {
+        // BlobBuilder has been deprecated in favour of Blob, but some browsers
+        // (including phantomjs) have not added the replacement Blob
+        // constructor API, so we have this fall-back code.
+        var BlobBuilder = window.WebKitBlobBuilder || window.MozBlobBuilder;
+        var builder = new BlobBuilder();
+        builder.append(data);
+        blob = builder.getBlob(mimeType);
+      }
+      // The saveAs() function is a global implemented in
+      // app/assets/javascripts/FileSaver.js.
+      saveAs(blob, 'websocket-log.txt');
+    }
+
+  });
+
+}, '0.1.0', {
+  requires: [
+    'sub-app'
+  ]
+});

=== modified file 'lib/websocketreplay.py'
--- lib/websocketreplay.py	2013-05-29 16:45:11 +0000
+++ lib/websocketreplay.py	2013-06-03 17:41:26 +0000
@@ -85,16 +85,22 @@
         direction the message was sent
     """
     seen_request_ids = set()
+    direction = None
     for line in source:
         line = line.strip()
         # The data format is a bit icky, but since we only send json-encoded
         # messages, pulling them out of the log is straight-forward.
-        if line.startswith('{'):
+        if line.startswith('to '):
+            direction = line
+        elif line.startswith('{'):
             message = json.loads(line)
-            direction = infer_direction(message, seen_request_ids)
+            # If the log does not contain direction info we have to guess.
+            if direction is None:
+                direction = infer_direction(message, seen_request_ids)
             if 'request_id' in message:
                 seen_request_ids.add(message['request_id'])
             yield Frame(message, direction)
+            message = direction = None
 
 
 def print_with_color(color, *args, **kws):

=== modified file 'test/index.html'
--- test/index.html	2013-05-24 14:23:27 +0000
+++ test/index.html	2013-06-03 17:41:26 +0000
@@ -106,6 +106,7 @@
   <script src="test_unit_view.js"></script>
   <script src="test_utils.js"></script>
   <script src="test_viewport_module.js"></script>
+  <script src="test_websocket_logging.js"></script>
   <script>
   YUI_config = {
       async: false,

=== added file 'test/test_websocket_logging.js'
--- test/test_websocket_logging.js	1970-01-01 00:00:00 +0000
+++ test/test_websocket_logging.js	2013-06-03 17:41:26 +0000
@@ -0,0 +1,109 @@
+/*
+This file is part of the Juju GUI, which lets users view and manage Juju
+environments within a graphical interface (https://launchpad.net/juju-gui).
+Copyright (C) 2013 Canonical Ltd.
+
+This program is free software: you can redistribute it and/or modify it under
+the terms of the GNU Affero General Public License version 3, as published by
+the Free Software Foundation.
+
+This program is distributed in the hope that it will be useful, but WITHOUT
+ANY WARRANTY; without even the implied warranties of MERCHANTABILITY,
+SATISFACTORY QUALITY, or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU Affero
+General Public License for more details.
+
+You should have received a copy of the GNU Affero General Public License along
+with this program.  If not, see <http://www.gnu.org/licenses/>.
+*/
+
+'use strict';
+
+var saveAs;
+
+(function() {
+  describe('websocket recording', function() {
+    var Y, WebsocketLogging, websocketLogging;
+
+    before(function(done) {
+      Y = YUI(GlobalConfig).use(
+          'juju-websocket-logging', function(Y) {
+            WebsocketLogging = Y.namespace('juju').WebsocketLogging;
+            done();
+          });
+    });
+
+    beforeEach(function() {
+      websocketLogging = new WebsocketLogging();
+      // Provide a noop saveAs global which is normally provided by the app.
+      saveAs = function() {};
+    });
+
+    afterEach(function() {
+      // We need to clean up all the event handlers.
+      Y.detach('websocketSend');
+      Y.detach('websocketReceive');
+      Y.detach('saveWebsocketLog');
+    });
+
+    it('can save a log', function() {
+      // When asked to save a log, the global saveAs function is called with a
+      // blob containing the serialized log entries.
+      saveAs = function(blob, fileName) {
+        assert.isTrue(blob !== undefined);
+        assert.equal(blob.type, 'text/plain;charset=utf-8');
+        assert.equal(blob.size, 6);
+        assert.equal(fileName, 'websocket-log.txt');
+      };
+      websocketLogging.saveLog(['my', 'log']);
+    });
+
+    it('responds to the saveWebsocketLog event', function(done) {
+      // When the saveWebsocketLog event is fired, the saveLog method is called
+      // with the current log of websocket events.
+      var logEntry = 'this is a log entry';
+      websocketLogging.saveLog = function(log) {
+        assert.deepEqual(log, [logEntry]);
+        done();
+      };
+      websocketLogging.log.push(logEntry);
+      // Since we changed one of the event handlers, we have to re-bind the
+      // event listeners.
+      Y.detach('saveWebsocketLog');
+      websocketLogging.setUpEventListeners();
+      Y.fire('saveWebsocketLog');
+    });
+
+    it('logs messages', function() {
+      // We want the timestamp to be fixed in time so we know what to expect
+      // for the assertion below.
+      websocketLogging.timestamp = function() {
+        return '2013-05-31T16:15:05.923Z';
+      };
+      websocketLogging.logMessage('to server', 'ping');
+      websocketLogging.logMessage('to client', 'pong');
+      assert.deepEqual(websocketLogging.log,
+          ['2013-05-31T16:15:05.923Z\nto server\nping',
+           '2013-05-31T16:15:05.923Z\nto client\npong']);
+    });
+
+    it('logs server messages on the websocketReceive event', function(done) {
+      websocketLogging.logMessage = function(direction, message) {
+        assert.equal(direction, 'to client');
+        assert.equal(message, 'message to the client');
+        done();
+      };
+      Y.fire('websocketReceive', 'message to the client');
+    });
+
+    it('logs client messages on the websocketSend event', function(done) {
+      websocketLogging.logMessage = function(direction, message) {
+        assert.equal(direction, 'to server');
+        assert.equal(message, 'message to the server');
+        done();
+      };
+      Y.fire('websocketSend', 'message to the server');
+    });
+
+  });
+
+})();

=== modified file 'test/test_websocketreplay.py'
--- test/test_websocketreplay.py	2013-05-29 16:45:11 +0000
+++ test/test_websocketreplay.py	2013-06-03 17:41:26 +0000
@@ -86,6 +86,45 @@
                 direction='to client')],
             list(read_frames(log)))
 
+    def test_included_direction_is_used(self):
+        # If the log contains direction info (whether or not the message was
+        # sent from the client and to the server or vice versa) that info will
+        # be used.
+        log = """\
+            junk
+            to mars
+            {"foo": 1}
+            junk
+            to venus
+            {"foo": 2}
+            junk""".split('\n')
+        self.assertEqual(
+            [Frame(
+                message={u'foo': 1},
+                direction='to mars'),
+             Frame(
+                message={u'foo': 2},
+                direction='to venus')],
+            list(read_frames(log)))
+
+    def test_missing_direction_is_infered(self):
+        # If the log does not contain direction info, the directionality of
+        # each message will deduced from the message's contents.
+        log = """\
+            junk
+            {"foo": 1, "request_id": 1}
+            junk
+            {"foo": 2}
+            junk""".split('\n')
+        self.assertEqual(
+            [Frame(
+                message={u'foo': 1, u'request_id': 1},
+                direction='to server'),
+             Frame(
+                message={u'foo': 2},
+                direction='to client')],
+            list(read_frames(log)))
+
     def test_result_is_iterator(self):
         # The object returned from read_frames() is an iterator.  This is so
         # we can consume it incrementally without having to keep up with how