Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Cloud Logging Error serialization #1877

Closed
dsimmons opened this issue Dec 12, 2016 · 4 comments
Closed

Cloud Logging Error serialization #1877

dsimmons opened this issue Dec 12, 2016 · 4 comments
Assignees
Labels
api: logging Issues related to the Cloud Logging API. priority: p2 Moderately-important priority. Fix may not be included in next release.

Comments

@dsimmons
Copy link

Environment details

  • OS: OSX 10.12.1
  • Node.js version: v7.0.0
  • npm version: 3.10.8
  • google-cloud-node version: 0.44.2

Steps to reproduce

  1. require google-cloud
  2. Use something like log.error.
const entry = log.entry('gce_instance', new Error('oh no!'));
log.error(entry);

NOTE: For what it's worth, I created the above snippet on-the-fly, so it may contain mistakes or not run as-is -- my existing code is intertwined with project-specific configuration and business logic, so it's not feasible to copy/paste directly. The above snippet sufficiently illustrates my perspective though! 😄

Description

Per a bit of trial-and-error, it appears as if there's not a default serializer of some kind for Javascript Error objects when using Cloud Logging through log#write or any of the helper functions (eg. error, alert, critical).

While a new logging entry is indeed created within the GCP Logging administrative view, it doesn't include anything beyond the default metadata for each entry written. In other words, I know that an error occurred, but the extent to which I'm able to troubleshoot it from there is pretty close to nil. In a sense, it effectively skips over the "payload" of the log entry (the Error itself) and creates said entry with the body omitted.

Specifically, the scenario in which this is pertinent is with catching and logging errors. Typically, my callback function or promise error handler would invoke logger.error(err) -- in fact, that's what led me to discover this existing handling in the first place! For many of my production logging entries, I was seeing that errors were being thrown, but the log entry itself wasn't very useful apart from that 😄

TL;DR / Proposal

I think slightly more intuitive behavior would be to have some sort of reasonable default in place as a serialization mechanism for the Error type/class, like using Error.message / toString().

Currently, I've switched my code to doing something like the following in the interim: (err && err.stack).

@stephenplusplus
Copy link
Contributor

stephenplusplus commented Dec 12, 2016

@dsimmons that's a pretty sane idea. Right now, I believe you'll have the expected behavior if you just nest the Error object inside of an object, i.e.:

const entry = log.entry('gce_instance', {
  error: new Error('oh no!')
});
Entry {
    metadata:
     { timestamp: 2016-12-12T16:23:44.685Z,
       payload: 'jsonPayload',
       logName: 'projects/nth-circlet-705/logs/sys_log',
       resource: [Object],
       protoPayload: null,
       textPayload: '',
       jsonPayload: [Object],
       severity: 'ERROR',
       insertId: 'yz0oqlf9i4i8v',
       httpRequest: null,
       labels: {},
       operation: null },
    data: { err: 'Error: Uh oh!' } }

We could convert an error object using the two standard properties, name and message:

var error = new Error('Uh oh!')

var entry = log.entry(error)
// same as
var entry = log.entry({
  name: error.name,
  message: error.message
})

What do you think?

@stephenplusplus stephenplusplus added enhancement api: logging Issues related to the Cloud Logging API. labels Dec 12, 2016
@dsimmons
Copy link
Author

Doh, I hadn't thought to try that!

I think that's a very sane proposal and right around what I had in mind, especially as a "lowest common denominator" / "out of the box" serialization mechanism.

Like you said, the above two properties are standard, and for that reason, they make a lot of sense. It may be worth considering/investigating the possibility of doing some sort of "enhanced" behavior that captures the full stack trace -- if I'm not mistaken, error.message just captures the first line / the string that was used to create the Error object. However, I'm not sure there's a fully standard way of capturing/accessing the stack trace across the board, so for that reason, I'm less confident as to whether it makes sense to be handled by the library versus left up to the consumer.

But, to wrap back around, the above would just be an added nicety. I think in any situation, what you proposed would be very helpful and a fantastic place to start -- even just knowing what the error.message is without the stack trace is helpful in diagnosing issues (especially versus the alternative of knowing that an Error was thrown somewhere and nothing else).

@bjwatson bjwatson added priority: p2 Moderately-important priority. Fix may not be included in next release. Status: Acknowledged labels Mar 2, 2017
@stephenplusplus stephenplusplus assigned swcloud and unassigned jmuk Jun 2, 2017
@evaogbe
Copy link
Contributor

evaogbe commented Aug 11, 2017

This issue may no longer be relevant due to its age. Feel free to re-open.

@evaogbe
Copy link
Contributor

evaogbe commented Feb 2, 2021

I am no longer on the team

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
api: logging Issues related to the Cloud Logging API. priority: p2 Moderately-important priority. Fix may not be included in next release.
Projects
None yet
Development

No branches or pull requests

7 participants