-
Notifications
You must be signed in to change notification settings - Fork 3
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
Adding stack trace to error messages #779
Adding stack trace to error messages #779
Conversation
@@ -197,6 +204,16 @@ func SetError(ctx context.Context, err error) error { | |||
|
|||
ctxLogger.SetError(err) | |||
|
|||
// add stack trace if available | |||
if serr, ok := err.(stackTracer); ok { | |||
st := serr.StackTrace() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should we limit the stack trace at all, or would we prefer to dump the whole thing?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In what way would you like to limit it?
I didn't consider limiting it. But I need to test it on staging, in order to see how it looks in Datadog (for example, if their UI have a nice way to fold long messages into "show more")
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think I have seen some people only take the top 5 lines for example, but I don't really know how well this works in practice. Lets give it a go in staging and see how it is.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I am waiting for definitive resolition of "will it cost us any money" question from @mariusvasile (slack thread) before doing any testing.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As you say, you are avoiding turning on error tracking, and are instead just adding more bytes to the logs, but this may add a lot of bytes of data.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As you say, you are avoiding turning on error tracking, and are instead just adding more bytes to the lots
I assume that the size of logs is a significant concern for Cuvva, as we're as a company aiming to achieve efficiency in every aspect.
However, I also feel that this level of focus on size/cost efficiency comes at the expense of other factors like system observability and engineering efficiency.
Moreover, the rigorous scrutiny seems to be applied primarily to new initiatives.
While working on my stack traces proposal, I noticed that our lambdas print stack traces 3 times in each error message. I wouldn’t dwell on this if we weren’t discussing the acceptability of increasing log sizes by printing stack traces once, while existing components do it three times.
No description provided.