Initial import of logging-bunyan #1977

ofrobots · 2017-02-09T18:20:01Z

This PR adds a new module @google-cloud/logging-bunyan that provides simple, minimal config way of logging to Stackdriver Logging from applications using bunyan.

packages/logging-bunyan/test/index.js

+
+'use strict';
+
+    var assert = require('assert');


zbjornson · 2017-02-09T19:18:44Z

Hello! There's an unofficial, moderately popular Bunyan transport as well: https://github.com/mlazarov/bunyan-stackdriver

If it helps, the key differences I see are:

Batching of API calls, which is critical to avoid API errors. (I think the limits in the readme are incorrect, but they're low enough to matter.).
Handling of log entries with circular references. (Discussed in logging: Serialization of circular structs #1354.)
Handling of log entries with types that are not strict JSON types (Date, Error, RegExp, undefined). (Cloud Logging Error serialization #1877, logging: Serialization of undefined values #1353, logging: Serialization of Native Types #1352.) For these types and circular references, the resolution was that it's the consumer's responsibility to deal with it, which I think means it should be handled in this module?
This implementation doesn't provide a true stream.Writable, which means Bunyan's reemitErrorEvents won't work.
When https://github.com/mlazarov/bunyan-stackdriver/pull/7/files lands, the helpful formatting of error messages for better display in the UI.

ofrobots · 2017-02-09T23:12:18Z

Hello @zbjornson! Thanks for chiming in. We wanted to start officially support a module for working with bunyan (and winston: #1830). It would be great to have your collaboration.

This is an initial version of the library here, but I do have some thoughts on the points you highlighted:

Batching: It would good to have batching indeed, and we need it for logging-winston too, but ideally batching should be built into the base logging library. This is something we have to hash out still. /cc @stephenplusplus
We do need to deal with circular log entries and non JSON values in this module (and in logging-winston).
I was not familiar with Bunyan's reemitErrorEvents. Thanks for pointing it out.

We can start addressing these in follow-ons. Again, collaboration would be most welcome!

zbjornson · 2017-02-10T00:28:35Z

Sounds great! I'm a bit short on time for the next few days to actually code, but happy to review and discuss anything. Let me know how I can help.

ofrobots · 2017-02-10T17:12:06Z

@GoogleCloudPlatform/node-team, @stephenplusplus This is ready for review. PTAL.

matthewloring

Do we want to check in the yarn.lock file?

ofrobots · 2017-02-10T21:27:18Z

Yes. No reason not to.

…

On Fri, Feb 10, 2017 at 12:50 PM Matthew Loring ***@***.***> wrote: ***@***.**** commented on this pull request. Do we want to check in the yarn.lock file? — You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub <#1977 (review)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AAE0qV1G2agVxUxelhCotlqk9407fQ9vks5rbM2jgaJpZM4L8fEU> .

packages/logging-bunyan/README.md

+  streams: [loggingBunyan.stream('info')]
+});
+
+logger.error('warp nacelles offline');


packages/logging-bunyan/package.json

@@ -0,0 +1,46 @@
+{
+  "name": "@google-cloud/logging-bunyan",
+  "version": "0.0.0",


packages/logging-bunyan/src/index.js

+  this.log_[level](entry, function() {});
+};
+
+module.exports = LoggingBunyan;


packages/logging-bunyan/system-test/logging-bunyan.js

+    });
+
+  });
+});


packages/logging-bunyan/src/index.js

+        '@google-cloud/logging-bunyan only works as a raw bunyan stream type.');
+  }
+  var level = BUNYAN_TO_STACKDRIVER[rec.level];
+  var entryMetadata = {resource: this.resource_, timestamp: rec.time};


packages/logging-bunyan/src/index.js

+
+  options = options || {};
+
+  this.logName_ = options.logName || 'bunyan_log';


stephenplusplus · 2017-02-13T17:40:29Z

Once functionality is approved by the reviewers, I'll do some simple linting/style stuff.

ofrobots · 2017-02-13T18:06:36Z

@zbjornson for the issues/feature requests you raised:

Batching: will track through automatic batching interface for cloud logging client #1296.
Circular log entires: logging-winston and logging-bunyan should deal with circular objects #1987
Non JSON log entries: @google-cloud/logging can now 'deal with' non JSON types. That is to say, they are converted to strings before being sent to the API. I don't think anything further is needed in the higher level layers. Please correct me if I am wrong here.
Proper Writable: logging-bunyan should be a proper Writable stream #1988
Better readability in Logging UI, interop with Error Reporting: logging-{winston,bunyan} should integrate well with Logging UI and with Error Reporting #1989

zbjornson · 2017-02-13T18:42:17Z

bueno, thanks! Looks like everything is addressed here or tracked in other issues.

googlebot · 2017-02-13T19:50:22Z

So there's good news and bad news.

👍 The good news is that everyone that needs to sign a CLA (the pull request submitter and all commit authors) have done so. Everything is all good there.

😕 The bad news is that it appears that one or more commits were authored by someone other than the pull request submitter. We need to confirm that they're okay with their commits being contributed to this project. Please have them confirm that here in the pull request.

Note to project maintainer: This is a terminal state, meaning the cla/google commit status will not change from this state. It's up to you to confirm consent of the commit author(s) and merge this pull request when appropriate.

stephenplusplus · 2017-02-13T19:51:52Z

@ofrobots I went over it for lint/nit/style changes. The biggest change was probably that I removed tests that were using the Bunyan module inside of the unit test file for index.js. I saw the system tests were using the real module, so if more testing with Bunyan is needed, please pop them in there.

ofrobots · 2017-02-13T23:44:54Z

LGTM. The removed tests are not that significant test of the interop to be really worth keeping.

packages/logging-bunyan/README.md

@@ -35,19 +29,12 @@ logger.info('shields at 99%');

 It's incredibly easy to get authenticated and start using Google's APIs. You can set your credentials on a global basis as well as on a per-API basis. See each individual API section below to see how you can auth on a per-API-basis. This is useful if you want to use different accounts for different Google Cloud services.

-### On Google Cloud Platform
+### On Google Compute Engine


ofrobots · 2017-02-14T21:51:39Z

Let us know if there is anything more needed from our side before this can land. Once this lands, we can start work on some follow-on PRs.

stephenplusplus · 2017-02-14T21:52:44Z

Just waiting on me?

stephenplusplus · 2017-02-14T21:55:46Z

@google-cloud/[email protected] published.

googlebot added the cla: yes This human has signed the Contributor License Agreement. label Feb 9, 2017

DominicKramer reviewed Feb 9, 2017

View reviewed changes

packages/logging-bunyan/test/index.js Outdated

'use strict';

var assert = require('assert');

This comment was marked as spam.

Sign in to view

This comment was marked as spam.

Sign in to view

Initial import of logging-bunyan

0e63ff0

ofrobots force-pushed the logging-bunyan branch from 6d5ec6f to 0e63ff0 Compare February 10, 2017 17:06

ofrobots mentioned this pull request Feb 10, 2017

Add configurable buffering to logging #1981

Closed

matthewloring reviewed Feb 10, 2017

View reviewed changes

stephenplusplus added 2 commits February 13, 2017 11:10

add logging-winston to manifest

1b5602e

Update manifest for logging-winston-0.1.0 [ci skip]

1a31783

matthewloring reviewed Feb 13, 2017

View reviewed changes

add newlines

220b879

matthewloring approved these changes Feb 13, 2017

View reviewed changes

This was referenced Feb 13, 2017

logging-winston and logging-bunyan should deal with circular objects #1987

Closed

logging-bunyan should be a proper Writable stream #1988

Closed

logging-{winston,bunyan} should integrate well with Logging UI and with Error Reporting #1989

Closed

ofrobots added 2 commits February 13, 2017 13:10

logging-winston: avoid printing to console during test (googleapis#1985)

b27d9b3

Merge 220b879 into b27d9b3

c8da6f6

lint

b9d63a7

googlebot added cla: no This human has *not* signed the Contributor License Agreement. and removed cla: yes This human has signed the Contributor License Agreement. labels Feb 13, 2017

ofrobots commented Feb 14, 2017

View reviewed changes

docs: replace Compute Engine with Cloud Platform

d07aea3

stephenplusplus approved these changes Feb 14, 2017

View reviewed changes

stephenplusplus merged commit 49a7beb into googleapis:master Feb 14, 2017

ofrobots deleted the logging-bunyan branch February 14, 2017 21:53

gaofanmichael mentioned this pull request Oct 26, 2017

logging-{winston,bunyan} should integrate well with Logging UI and with Error Reporting googleapis/nodejs-logging-bunyan#5

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Initial import of logging-bunyan #1977

Initial import of logging-bunyan #1977

ofrobots commented Feb 9, 2017 •

edited

Loading

This comment was marked as spam.

This comment was marked as spam.

zbjornson commented Feb 9, 2017 •

edited

Loading

ofrobots commented Feb 9, 2017

zbjornson commented Feb 10, 2017

ofrobots commented Feb 10, 2017

matthewloring left a comment

ofrobots commented Feb 10, 2017 via email

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

stephenplusplus commented Feb 13, 2017

ofrobots commented Feb 13, 2017

zbjornson commented Feb 13, 2017

googlebot commented Feb 13, 2017

stephenplusplus commented Feb 13, 2017

ofrobots commented Feb 13, 2017

This comment was marked as spam.

This comment was marked as spam.

ofrobots commented Feb 14, 2017

stephenplusplus commented Feb 14, 2017

stephenplusplus commented Feb 14, 2017

+                  });
+                });
+              });


		options = options \|\| {};

		this.logName_ = options.logName \|\| 'bunyan_log';

Initial import of logging-bunyan #1977

Initial import of logging-bunyan #1977

Conversation

ofrobots commented Feb 9, 2017 • edited Loading

This comment was marked as spam.

This comment was marked as spam.

zbjornson commented Feb 9, 2017 • edited Loading

ofrobots commented Feb 9, 2017

zbjornson commented Feb 10, 2017

ofrobots commented Feb 10, 2017

matthewloring left a comment

Choose a reason for hiding this comment

ofrobots commented Feb 10, 2017 via email

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

stephenplusplus commented Feb 13, 2017

ofrobots commented Feb 13, 2017

zbjornson commented Feb 13, 2017

googlebot commented Feb 13, 2017

stephenplusplus commented Feb 13, 2017

ofrobots commented Feb 13, 2017

This comment was marked as spam.

This comment was marked as spam.

ofrobots commented Feb 14, 2017

stephenplusplus commented Feb 14, 2017

stephenplusplus commented Feb 14, 2017

ofrobots commented Feb 9, 2017 •

edited

Loading

zbjornson commented Feb 9, 2017 •

edited

Loading