logging-bunyan: make it a proper Writeable stream

ofrobots · ofrobots · commit 300a4d739784 · 2017-03-20T22:47:19.000-07:00
Fixes: #1988
diff --git a/packages/logging-bunyan/src/index.js b/packages/logging-bunyan/src/index.js
@@ -22,6 +22,8 @@
 
 var extend = require('extend');
 var logging = require('@google-cloud/logging');
+var util = require('util');
+var Writable = require('stream').Writable;
 
 /**
  * Map of Stackdriver logging levels.
@@ -95,7 +97,12 @@ function LoggingBunyan(options) {
   this.log_ = logging(options).log(this.logName_, {
     removeCircular: true
   });
+
+  Writable.call(this, {
+    objectMode: true
+  });
 }
+util.inherits(LoggingBunyan, Writable);
 
 /**
  * Convenience method that Builds a bunyan stream object that you can put in
@@ -121,13 +128,13 @@ LoggingBunyan.prototype.stream = function(level) {
 };
 
 /**
- * Relay a log entry to the logging agent. This is normally called by bunyan.
+ * Format a bunyan record into a Stackdriver log entry.
  *
- * @param {object} record - Bunyan log record.
+ * @param {object} - Bunyan log record.
  *
  * @private
  */
-LoggingBunyan.prototype.write = function(record) {
+LoggingBunyan.prototype._formatEntry = function(record) {
   if (typeof record === 'string') {
     throw new Error(
       '@google-cloud/logging-bunyan only works as a raw bunyan stream type.'
@@ -158,19 +165,29 @@ LoggingBunyan.prototype.write = function(record) {
     }
   }
 
-  var level = BUNYAN_TO_STACKDRIVER[record.level];
-
   var entryMetadata = {
     resource: this.resource_,
     timestamp: record.time
   };
 
-  var entry = this.log_.entry(entryMetadata, record);
+  return this.log_.entry(entryMetadata, record);
+};
 
-  this.log_[level](entry, function() {
-    // no-op to avoid a promise being returned.
-  });
+/**
+ * Relay a log entry to the logging agent. This is called by bunyan through
+ * Writable#write.
+ *
+ * @param {object} record - Bunyan log record.
+ *
+ * @private
+ */
+LoggingBunyan.prototype._write = function(record, encoding, callback) {
+  var entry = this._formatEntry(record);
+  var level = BUNYAN_TO_STACKDRIVER[record.level];
+  this.log_[level](entry, callback);
 };
 
+// TODO(ofrobots): implement _writev as well.
+
 module.exports = LoggingBunyan;
 module.exports.BUNYAN_TO_STACKDRIVER = BUNYAN_TO_STACKDRIVER;
diff --git a/packages/logging-bunyan/system-test/logging-bunyan.js b/packages/logging-bunyan/system-test/logging-bunyan.js
@@ -24,7 +24,7 @@ var env = require('../../../system-test/env.js');
 var logging = require('@google-cloud/logging')(env);
 var loggingBunyan = require('../')(env);
 
-describe.skip('LoggingBunyan', function() {
+describe('LoggingBunyan', function() {
   var WRITE_CONSISTENCY_DELAY_MS = 20000;
 
   var logger = bunyan.createLogger({
@@ -57,9 +57,8 @@ describe.skip('LoggingBunyan', function() {
         ],
         level: 'error',
         verify: function(entry) {
-          assert.strictEqual(entry.data.msg, 'second');
+          assert(entry.data.message.startsWith('Error: second'));
           assert.strictEqual(entry.data.pid, process.pid);
-          assert.ok(entry.data.message.startsWith('Error: second'));
         }
       },
 
@@ -70,8 +69,9 @@ describe.skip('LoggingBunyan', function() {
           },
           'third'
         ],
+        level: 'info',
         verify: function(entry) {
-          assert.strictEqual(entry.data.msg, 'third');
+          assert.strictEqual(entry.data.message, 'third');
           assert.strictEqual(entry.data.pid, process.pid);
           assert.deepStrictEqual(entry.data.test, {
             circular: '[Circular]'
diff --git a/packages/logging-bunyan/test/index.js b/packages/logging-bunyan/test/index.js
@@ -25,6 +25,7 @@ describe('logging-bunyan', function() {
   var fakeLoggingOptions_;
   var fakeLogName_;
   var fakeLogOptions_;
+  var fakeWritableOptions_;
 
   function fakeLogging(options) {
     fakeLoggingOptions_ = options;
@@ -37,6 +38,18 @@ describe('logging-bunyan', function() {
     };
   }
 
+  function fakeWritable(options) {
+    fakeWritableOptions_ = options;
+  }
+
+  fakeWritable.prototype.write = function(chunk, encoding, callback) {
+    setImmediate(callback);
+  };
+
+  var fakeStream = {
+    Writable: fakeWritable
+  };
+
   var LoggingBunyanCached;
   var LoggingBunyan;
   var loggingBunyan;
@@ -46,9 +59,15 @@ describe('logging-bunyan', function() {
     resource: {}
   };
 
+  var RECORD = {
+    level: 30,
+    time: '2012-06-19T21:34:19.906Z'
+  };
+
   before(function() {
     LoggingBunyan = proxyquire('../src/index.js', {
-      '@google-cloud/logging': fakeLogging
+      '@google-cloud/logging': fakeLogging,
+      stream: fakeStream
     });
 
     LoggingBunyanCached = extend(true, {}, LoggingBunyan);
@@ -70,6 +89,11 @@ describe('logging-bunyan', function() {
       assert(loggingBunyan instanceof LoggingBunyan);
     });
 
+    it('should be an object mode Writable', function() {
+      assert(loggingBunyan instanceof fakeWritable);
+      assert.deepStrictEqual(fakeWritableOptions_, { objectMode: true });
+    });
+
     it('should localize the provided resource', function() {
       assert.strictEqual(loggingBunyan.resource_, OPTIONS.resource);
     });
@@ -102,22 +126,11 @@ describe('logging-bunyan', function() {
     });
   });
 
-  describe('write', function() {
-    var STACKDRIVER_LEVEL = 'info';
-
-    var RECORD = {
-      level: 30,
-      time: '2012-06-19T21:34:19.906Z'
-    };
-
-    beforeEach(function() {
-      fakeLogInstance.entry = function() {};
-      loggingBunyan.log_[STACKDRIVER_LEVEL] = function() {};
-    });
+  describe('_formatEntry', function() {
 
     it('should throw an error if record is a string', function() {
       assert.throws(function() {
-        loggingBunyan.write('string record');
+        loggingBunyan._formatEntry('string record');
       }, new RegExp(
         '@google-cloud/logging-bunyan only works as a raw bunyan stream type.'
       ));
@@ -133,25 +146,7 @@ describe('logging-bunyan', function() {
         done();
       };
 
-      loggingBunyan.write(RECORD);
-    });
-
-    it('should write to the correct log', function(done) {
-      var customLevel = 'custom-level';
-      var entry = {};
-
-      loggingBunyan.log_.entry = function() {
-        return entry;
-      };
-
-      LoggingBunyan.BUNYAN_TO_STACKDRIVER[RECORD.level] = customLevel;
-
-      loggingBunyan.log_[customLevel] = function(entry_) {
-        assert.strictEqual(entry_, entry);
-        done();
-      };
-
-      loggingBunyan.write(RECORD);
+      loggingBunyan._formatEntry(RECORD);
     });
 
     it('should rename the msg property to message', function(done) {
@@ -163,7 +158,7 @@ describe('logging-bunyan', function() {
         done();
       };
 
-      loggingBunyan.write(recordWithMsg);
+      loggingBunyan._formatEntry(recordWithMsg);
     });
 
     it('should inject the error stack as the message', function(done) {
@@ -186,7 +181,7 @@ describe('logging-bunyan', function() {
         done();
       };
 
-      loggingBunyan.write(record);
+      loggingBunyan._formatEntry(record);
     });
 
     it('should leave message property intact when present', function(done) {
@@ -203,10 +198,47 @@ describe('logging-bunyan', function() {
         done();
       };
 
-      loggingBunyan.write(record);
+      loggingBunyan._formatEntry(record);
     });
   });
 
+  describe('_write', function() {
+    var STACKDRIVER_LEVEL = 'info';
+
+    beforeEach(function() {
+      fakeLogInstance.entry = function() {};
+      loggingBunyan.log_[STACKDRIVER_LEVEL] = function() {};
+    });
+
+    it('should format the record', function(done) {
+      loggingBunyan._formatEntry = function(record) {
+        assert.strictEqual(record, RECORD);
+        done();
+      };
+
+      loggingBunyan._write(RECORD, '', assert.ifError);
+    });
+
+    it('should write to the correct log', function(done) {
+      var customLevel = 'custom-level';
+      var entry = {};
+
+      loggingBunyan.log_.entry = function() {
+        return entry;
+      };
+
+      LoggingBunyan.BUNYAN_TO_STACKDRIVER[RECORD.level] = customLevel;
+
+      loggingBunyan.log_[customLevel] = function(entry_) {
+        assert.strictEqual(entry_, entry);
+        done();
+      };
+
+      loggingBunyan._write(RECORD, '', assert.ifError);
+    });
+  });
+
+
   describe('BUNYAN_TO_STACKDRIVER', function() {
     it('should correctly map to Stackdriver Logging levels', function() {
       assert.deepEqual(LoggingBunyan.BUNYAN_TO_STACKDRIVER, {
