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

Format log messages so that Stackdriver error reporting picks up exceptions #363

Merged
merged 1 commit into from
Aug 13, 2018

Conversation

akselsson
Copy link
Contributor

Stackdriver error reporting can automatically pick up errors from log
entries if they are formatted properly:

"When logging error data from App Engine, Kubernetes Engine or Compute
Engine, the only requirement is for the log entry to contain the full
error message and stack trace. It should be logged as a multi-line
textPayload or in the message field of jsonPayload"

"C#: Must be the return value of Exception.ToString()"

https://cloud.google.com/error-reporting/docs/formatting-error-messages

This PR aims to change the formatting of errors to conform with those guidelines.

Note that I considered using tokeniseExceptions from MessageWriter, but that function outputs a stacktrace that is not equal to Exception.ToString() and thus does not conform to Google's recommendations.

Stackdriver error reporting can automatically pick up errors from log
entries if they are formatted properly:

"When logging error data from App Engine, Kubernetes Engine or Compute
Engine, the only requirement is for the log entry to contain the full
error message and stack trace. It should be logged as a multi-line
textPayload or in the message field of jsonPayload"

"C#: Must be the return value of Exception.ToString()"

https://cloud.google.com/error-reporting/docs/formatting-error-messages

This commit changes the formatting of errors to conform with that.
@akselsson
Copy link
Contributor Author

In the future, we may want to add support for supplying serviceContext.service and serviceContext.version to Stackdriver since service is also a required field. I have, however found that it's sufficient to add an application property to jsonPayload instead, which works for us.

@haf haf merged commit 174559c into causiq:master Aug 13, 2018
@haf
Copy link
Member

haf commented Aug 13, 2018

Thank you @akselsson I'll put out a new release with this change in it!

@haf
Copy link
Member

haf commented Aug 13, 2018

Fixes #349

@haf
Copy link
Member

haf commented Aug 13, 2018

Released as v5-beta.22

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants