logging-{winston,bunyan}: format logs to work better with Log Viewer and Error Reporting #1997

ofrobots · 2017-02-14T23:00:40Z

Stackdriver Logs Viewer picks up the summary line from the 'message' field
of the jsonPayload or from the textPayload. See:
https://cloud.google.com/logging/docs/view/logs_viewer_v2#expanding

Furthermore, for error messages at severity 'error' and higher, Stackdriver
Error Reporting will pick up error messages if the full stack trace is
included in the textPayload or in the message property of the
jsonPaylod. See:
https://cloud.google.com/error-reporting/docs/formatting-error-messages

The commit also fixes some non-determinism in the winston test caused by the sending logs through async.

ofrobots · 2017-02-14T23:01:28Z

@cristiancavalli @kjin PTAL.

packages/logging-bunyan/src/index.js

+  //
+  if (!record.message) {
+    if (record.err && record.err.stack) {
+      record.message = record.err.stack;


packages/logging-winston/src/index.js

+  //
+  if (is.error(metadata)) {
+    if (msg.length === 0) {
+      msg = metadata.stack;


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

    ];

    it('should properly write log entries', function(done) {
-      async.each(testData, function(test, callback) {


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

+        args: [ 'fifth message', new Error('fifth') ],
+        level: 'error',
+        verify: function(entry) {
+          assert.ok(entry.data.startsWith('fifth message: Error: fifth'));


ofrobots · 2017-02-22T17:08:55Z

~~This is currently blocked on #2014~~.

Reduced scope to formatting only. PTAL.

packages/logging-bunyan/src/index.js

+  //
+  if (!record.message) {
+    if (record.err && record.err.stack) {
+      record.message = record.err.stack;


cristiancavalli · 2017-03-14T16:56:01Z

LGTM once CI's pass

kjin

LGTM.

ofrobots · 2017-03-15T05:40:29Z

@stephenplusplus this is good to go from our side. PTAL.

packages/logging-winston/src/index.js

+        (metadata && metadata.stack) ?
+            (msg.length === 0 ? metadata.stack : msg + ': ' + metadata.stack) :
+            msg
+  };


packages/logging-bunyan/src/index.js

+      record.message = record.msg;
+    }
+  }
+


packages/logging-bunyan/src/index.js

+  // TODO(ofrobots): when resource.type is 'global' we need to additionally
+  // provide serviceContext.service as part of the entry for Error Reporting to
+  // automatically pick up the error.
+  if (!record.message) {


packages/logging-winston/src/index.js

+  // TODO(ofrobots): when resource.type is 'global' we need to additionally
+  // provide serviceContext.service as part of the entry for Error Reporting to
+  // automatically pick up the error.
+  var data = {


packages/logging-bunyan/src/index.js

+    // to automatically pick up the error.
+    if (record.err && record.err.stack) {
+      record.message = record.err.stack;
+    } else if (record.msg) {


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

+        args: ['fifth message', new Error('fifth')],
+        level: 'error',
+        verify: function(entry) {
+          assert.ok(


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

+        args: [new Error('forth')],
+        level: 'error',
+        verify: function(entry) {
+          assert.ok(entry.data.message.startsWith('Error: forth'));


packages/logging-winston/src/index.js

+  // provide serviceContext.service as part of the entry for Error Reporting to
+  // automatically pick up the error.
+  if (metadata && metadata.stack) {
+    msg += ' ' + metadata.stack;


packages/logging-winston/test/index.js

+      loggingWinston.log(LEVEL, MESSAGE, error, assert.ifError);
+    });
+
+    it('should append the stack when metadata is an error, empty message',


googlebot · 2017-03-17T16:03:29Z

We found a Contributor License Agreement for you (the sender of this pull request), but were unable to find agreements for the commit author(s). If you authored these, maybe you used a different email address in the git commits than was used to sign the CLA (login here to double check)? If these were authored by someone else, then they will need to sign a CLA as well, and confirm that they're okay with these being contributed to Google.

stephenplusplus · 2017-03-17T16:16:24Z

npm test:

  1) logging-bunyan write should rename the msg property to message:
     TypeError: this.log_[level] is not a function
      at LoggingBunyan.write (C:\Users\sawch\dev\google-cloud-node\packages\logging-bunyan\src\index.js:170:19)
      at Context.<anonymous> (C:\Users\sawch\dev\google-cloud-node\packages\logging-bunyan\test\index.js:166:21)
      at callFnAsync (C:\Users\sawch\dev\google-cloud-node\node_modules\mocha\lib\runnable.js:349:8)
      at Test.Runnable.run (C:\Users\sawch\dev\google-cloud-node\node_modules\mocha\lib\runnable.js:301:7)
      at Runner.runTest (C:\Users\sawch\dev\google-cloud-node\node_modules\mocha\lib\runner.js:422:10)
      at C:\Users\sawch\dev\google-cloud-node\node_modules\mocha\lib\runner.js:528:12
      at next (C:\Users\sawch\dev\google-cloud-node\node_modules\mocha\lib\runner.js:342:14)
      at C:\Users\sawch\dev\google-cloud-node\node_modules\mocha\lib\runner.js:352:7
      at next (C:\Users\sawch\dev\google-cloud-node\node_modules\mocha\lib\runner.js:284:14)
      at C:\Users\sawch\dev\google-cloud-node\node_modules\mocha\lib\runner.js:315:7
      at done (C:\Users\sawch\dev\google-cloud-node\node_modules\mocha\lib\runnable.js:287:5)
      at callFn (C:\Users\sawch\dev\google-cloud-node\node_modules\mocha\lib\runnable.js:344:7)
      at Hook.Runnable.run (C:\Users\sawch\dev\google-cloud-node\node_modules\mocha\lib\runnable.js:319:7)
      at next (C:\Users\sawch\dev\google-cloud-node\node_modules\mocha\lib\runner.js:298:10)
      at Immediate.<anonymous> (C:\Users\sawch\dev\google-cloud-node\node_modules\mocha\lib\runner.js:320:5)

…and Error Reporting Stackdriver Logs Viewer picks up the summary line from the 'message' field of the jsonPayload or from the textPayload. See: https://cloud.google.com/logging/docs/view/logs_viewer_v2#expanding Furthermore, for error messages at severity 'error' and higher, Stackdriver Error Reporting will pick up error messages if the full stack trace is included in the textPayload or in the `message` property of the jsonPaylod. See: https://cloud.google.com/error-reporting/docs/formatting-error-messages Fixes: googleapis#1989

ofrobots · 2017-03-17T22:12:37Z

@stephenplusplus I am unable to reproduce that locally. Is that windows only failure? Are the appveyor results to be trusted?

stephenplusplus · 2017-03-17T22:14:54Z

That was actually on my computer. I'll have to check that I didn't have the repo in some weird state and report back.

ofrobots · 2017-03-17T22:15:32Z

Actually, I see it to be failing on CircleCI as well.
EDIT: rebuilding Circle build with SSH enabled as I cannot reproduce this.

ofrobots · 2017-03-17T23:51:25Z

I cannot reproduce on an ssh session on CircleCI either. Also, it seems that the first run on the test on Circle (with Node.js v4) passes. The second test (Node.js v6) fails. Running the test individually with either v4 or v6 passes.

Somehow this is dependent on how the test is run from the script and/or dependent on something else non-deterministic.

ofrobots · 2017-03-18T00:09:36Z

Let's see if ce8caf7 fixes things.

coveralls · 2017-03-18T00:21:15Z

Coverage increased (+0.0001%) to 99.924% when pulling ce8caf7 on ofrobots:better-log-viewer into d153530 on GoogleCloudPlatform:master.

ofrobots · 2017-03-20T16:27:27Z

@stephenplusplus Fixed the problem. Does this LGTY?

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

kjin previously requested changes Feb 16, 2017

View reviewed changes

ofrobots force-pushed the better-log-viewer branch from 22af7ca to 76fac44 Compare February 22, 2017 17:01

ofrobots mentioned this pull request Feb 28, 2017

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

Closed

ofrobots changed the title ~~logging-{winston,bunyan}: format logs to work better with Log Viewer and Error Reporting.~~ logging-{winston,bunyan}: format logs to work better with Log Viewer Mar 14, 2017

cristiancavalli approved these changes Mar 14, 2017

View reviewed changes

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

//

if (!record.message) {

if (record.err && record.err.stack) {

record.message = record.err.stack;

This comment was marked as spam.

Sign in to view

ofrobots force-pushed the better-log-viewer branch from da155b7 to f610645 Compare March 14, 2017 17:24

ofrobots changed the title ~~logging-{winston,bunyan}: format logs to work better with Log Viewer~~ logging-{winston,bunyan}: format logs to work better with Log Viewer and Error Reporting Mar 14, 2017

kjin approved these changes Mar 14, 2017

View reviewed changes

stephenplusplus suggested changes Mar 16, 2017

View reviewed changes

ofrobots force-pushed the better-log-viewer branch from f610645 to ce3b162 Compare March 16, 2017 21:05

stephenplusplus reviewed Mar 16, 2017

View reviewed changes

stephenplusplus reviewed Mar 17, 2017

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 Mar 17, 2017

stephenplusplus approved these changes Mar 17, 2017

View reviewed changes

stephenplusplus added api: logging-bunyan api: logging-winston labels Mar 17, 2017

stephenplusplus force-pushed the better-log-viewer branch from 3d88d32 to f6e1535 Compare March 17, 2017 16:15

ofrobots added 4 commits March 17, 2017 12:27

error reporting prefers jsonPayload

3144efa

drop devDep on async

8c90482

add TODO about serviceContext.service and global resource types

9e434dc

ofrobots and others added 5 commits March 17, 2017 12:27

bunyan: address comments

b153405

winston: address comments

ef8bac7

winston: remove : when appending the stack

8ecb9b1

address comments

4ed40ca

one liner

a392b84

stephenplusplus force-pushed the better-log-viewer branch from f6e1535 to a392b84 Compare March 17, 2017 16:27

ensure test records have a level

ce8caf7

stephenplusplus merged commit d04cf34 into googleapis:master Mar 20, 2017

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

logging-{winston,bunyan}: format logs to work better with Log Viewer and Error Reporting #1997

logging-{winston,bunyan}: format logs to work better with Log Viewer and Error Reporting #1997

Conversation

ofrobots commented Feb 14, 2017

ofrobots commented Feb 14, 2017

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.

ofrobots commented Feb 22, 2017 • edited Loading

This comment was marked as spam.

cristiancavalli commented Mar 14, 2017

kjin left a comment

Choose a reason for hiding this comment

ofrobots commented Mar 15, 2017

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.

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.

googlebot commented Mar 17, 2017

stephenplusplus commented Mar 17, 2017

ofrobots commented Mar 17, 2017

stephenplusplus commented Mar 17, 2017

ofrobots commented Mar 17, 2017 • edited Loading

ofrobots commented Mar 17, 2017

ofrobots commented Mar 18, 2017

coveralls commented Mar 18, 2017

ofrobots commented Mar 20, 2017

ofrobots commented Feb 22, 2017 •

edited

Loading

ofrobots commented Mar 17, 2017 •

edited

Loading