Add winston logging transport #1830

ofrobots · 2016-11-28T19:55:52Z

Pre-req: Add default resource generator for log entries #1808. (commits from that PR will show up here too until it lands.)
npm run docs parser doesn't like my docs yet (is there any info/docs on what the custom parser requires?)

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

The code is fairly small, but it should be a standalone module because we need to depend upon winston (the transport must be an instance of winston.Transport). Adding this dependency to @google-cloud/logging doesn't make sense (we should not install winston for people who are not asking for winston support).

/cc @cristiancavalli

googlebot · 2016-11-28T19:55:57Z

We found a Contributor License Agreement for you (the sender of this pull request) and all commit authors, but as best as we can tell these commits were authored by someone else. If that's the case, please add them to this pull request and have them confirm that they're okay with these commits being contributed to Google. If we're mistaken and you did author these commits, just reply here to confirm.

googlebot · 2016-12-02T20:04:40Z

CLAs look good, thanks!

packages/logging-winston/src/index.js

+  var metadata = this.metadata_;
+  if (meta) {
+    // Logging proto requires that the label values be strings, so we convert
+    // using util.inspect.


googlebot · 2016-12-02T21:30:14Z

We found a Contributor License Agreement for you (the sender of this pull request) and all commit authors, but as best as we can tell these commits were authored by someone else. If that's the case, please add them to this pull request and have them confirm that they're okay with these commits being contributed to Google. If we're mistaken and you did author these commits, just reply here to confirm.

stephenplusplus · 2016-12-02T21:31:58Z

@ofrobots I've added a commit. The main differences are,

code style (in docs & JSDocs)
examples (using new to instantiate winston & the transport -> winston.add)

Can you take a quick look and make sure it still behaves as intended?

ofrobots

Thanks. Added comments.

packages/logging-winston/README.md

 });
+


packages/logging-winston/README.md

+
+### Elsewhere
+
+If you are not running this client on Google Compute Engine, you need a Google Developers service account. To create a service account:


packages/logging-winston/src/index.js

 */
+function LoggingWinston(options) {


packages/logging-winston/src/index.js

- *     either succeeds or gives up writing the log entry to the remote server.
+ * @param {object=} metadata - Winston-provided metadata that should be attached
+ *     to the log entry. Each property will be converted to a string using
+ *     `JSON.stringify`.


packages/logging-winston/src/index.js

-  if (_.isFunction(meta) && !callback) {
-    callback = meta;
-    meta = null;
+LoggingWinston.prototype.log = function(levelName, msg, metadata, callback) {


packages/logging-winston/src/index.js


-  var metadata = this.metadata_;
-  if (meta) {
+  if (metadata) {


coveralls · 2016-12-06T08:39:09Z

Coverage decreased (-0.2%) to 99.792% when pulling 568fec4 on ofrobots:logging-winston into ac87e18 on GoogleCloudPlatform:master.

packages/logging-winston/README.md


-If you are running your application in a different environment, for example, locally, on-premise, or on another cloud provider, you may need to provide some configuration.
+If you are running this client on Google Compute Engine, we handle authentication for you with no configuration. You just need to make sure that when you [set up the GCE instance][gce-how-to], you add the correct scopes for the APIs you want to access.


stephenplusplus · 2016-12-12T20:03:19Z

I'll finish up the unit tests for this. Are we including this in the google-cloud bundle, e.g.:

var gcloud = require('google-cloud')();
var transport = gcloud.loggingWinston;

Also, is there a better name than logging-winston? Generally, var names coincide with the module name, so this isn't ideal:

var stackdriverTransport = require('@google-cloud/logging-winston')

ofrobots · 2016-12-15T01:41:10Z

/cc @JustinBeckwith @omaray for more opinions on the name.

This library isn't a client library but rather an integration library. I think the general rule of var names coinciding doesn't have to apply here. Module names are primarily for discoverability of purpose, so I still prefer @google-cloud/logging-winston. The fact that this returns a winston transport would make complete sense to winston users.

Perhaps we should change the examples to the following to make things more elegant:

var transport = require('@google-cloud/logging-winston');

I don't have very strong opinions on bundling this with google-cloud as I am not too familiar with the precedents in this library. My personal preference would be to not include integration libraries in the default bundle. The users who need to use it can add it explicitly.

ofrobots · 2016-12-16T18:16:42Z

I've improved the examples.

ofrobots · 2017-01-04T15:54:15Z

@stephenplusplus I've add some more unit tests. PTAL.

stephenplusplus · 2017-01-09T15:42:43Z

I made some alterations from your last commit with regards to the input validation and testing strategy, keeping it more in line with the conventions here-- letting validation happen "last-minute", by an upstream module or API request, meaning we try to only throw if the next line of code in our script wouldn't work if they gave us something invalid. And with the tests, we stub everything to make sure the tests are as isolated as possible from logic that exists anywhere else.

PTAL!

ofrobots · 2017-02-03T07:00:40Z

Sorry for the delay. LGTM.

ofrobots · 2017-02-07T23:59:23Z

When should we expect this to land?

ofrobots · 2017-02-09T16:46:07Z

Thanks for landing. Can we publish a package for this too?

stephenplusplus · 2017-02-09T17:05:30Z

Published as 0.1.0.

googlebot added the cla: no This human has *not* signed the Contributor License Agreement. label Nov 28, 2016

stephenplusplus force-pushed the logging-winston branch from 46b858d to b51faa7 Compare December 2, 2016 20:04

googlebot added cla: yes This human has signed the Contributor License Agreement. and removed cla: no This human has *not* signed the Contributor License Agreement. labels Dec 2, 2016

stephenplusplus reviewed Dec 2, 2016

View reviewed changes

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 Dec 2, 2016

ofrobots commented Dec 5, 2016

View reviewed changes

stephenplusplus added the api: logging Issues related to the Cloud Logging API. label Dec 5, 2016

stephenplusplus force-pushed the logging-winston branch from 6faa1ab to 568fec4 Compare December 5, 2016 21:23

ofrobots commented Dec 6, 2016

View reviewed changes

ofrobots force-pushed the logging-winston branch 3 times, most recently from a8bdbb4 to a3ee52a Compare January 3, 2017 22:19

zbjornson mentioned this pull request Feb 3, 2017

Support HttpRequest property of LogEntry #1959

Closed

ofrobots and others added 3 commits February 8, 2017 12:59

logging-winston: initial import

158e3e4

logging-winston: add system tests

74592a4

code style

4285ab7

stephenplusplus and others added 11 commits February 8, 2017 12:59

resort options

4527e2b

review changes

d28e305

make sure metadata is object before looping over it

f23e117

add transport to Winston

eb1e955

add constructor test

5f15e0d

improve examples

4ecd2f3

validate options.level{s} and add some tests

7fbbb69

test refactor

f588e5a

include missing packages & fix system tests

21b8017

add loggingwinston to docs config

055368b

update deps

039e1b1

stephenplusplus force-pushed the logging-winston branch from 57c5c5e to 039e1b1 Compare February 8, 2017 18:16

change package name in docs config

9dc6ff2

stephenplusplus merged commit 4a9bd94 into googleapis:master Feb 8, 2017

ofrobots deleted the logging-winston branch February 9, 2017 14:52

ofrobots mentioned this pull request Feb 9, 2017

Initial import of logging-bunyan #1977

Merged

takeratta mentioned this pull request Jul 6, 2023

[Snyk] Fix for 1 vulnerabilities takeratta/google-cloud-node#2

Open


		### Elsewhere

		If you are not running this client on Google Compute Engine, you need a Google Developers service account. To create a service account:


		If you are running your application in a different environment, for example, locally, on-premise, or on another cloud provider, you may need to provide some configuration.
		If you are running this client on Google Compute Engine, we handle authentication for you with no configuration. You just need to make sure that when you [set up the GCE instance][gce-how-to], you add the correct scopes for the APIs you want to access.

		*/
		function LoggingWinston(options) {

Add winston logging transport #1830

Add winston logging transport #1830

Conversation

ofrobots commented Nov 28, 2016 • edited by stephenplusplus Loading

googlebot commented Nov 28, 2016

googlebot commented Dec 2, 2016

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

googlebot commented Dec 2, 2016

stephenplusplus commented Dec 2, 2016

ofrobots left a comment

Choose a reason for hiding this comment

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.

This comment was marked as spam.

This comment was marked as spam.

coveralls commented Dec 6, 2016

This comment was marked as spam.

This comment was marked as spam.

stephenplusplus commented Dec 12, 2016 • edited Loading

ofrobots commented Dec 15, 2016

ofrobots commented Dec 16, 2016

ofrobots commented Jan 4, 2017

stephenplusplus commented Jan 9, 2017

ofrobots commented Feb 3, 2017

ofrobots commented Feb 7, 2017

ofrobots commented Feb 9, 2017

stephenplusplus commented Feb 9, 2017

ofrobots commented Nov 28, 2016 •

edited by stephenplusplus

Loading

stephenplusplus commented Dec 12, 2016 •

edited

Loading