cockroachdb / cockroach

CockroachDB — the cloud native, distributed SQL database designed for high availability, effortless scale, and control over data placement.
https://www.cockroachlabs.com
Other
29.85k stars 3.77k forks source link

log: want stack traces to not be redacted #64135

Open andreimatei opened 3 years ago

andreimatei commented 3 years ago

A stack trace produced by debug.Stack() looks like this in the log; one can't even tell that it's a stack trace.

W210423 00:42:43.681711 10427041 kv/kvserver/syncing_write.go:54 ⋮ [n3,s3] 155199 +‹×› 
W210423 00:42:43.681711 10427041 kv/kvserver/syncing_write.go:54 ⋮ [n3,s3] 155199 +‹×› 
W210423 00:42:43.681711 10427041 kv/kvserver/syncing_write.go:54 ⋮ [n3,s3] 155199 +‹×› 
W210423 00:42:43.681711 10427041 kv/kvserver/syncing_write.go:54 ⋮ [n3,s3] 155199 +‹×› 
W210423 00:42:43.681711 10427041 kv/kvserver/syncing_write.go:54 ⋮ [n3,s3] 155199 +‹×› 
...

I'm sure we print these in various places, for example here. One way or another, it'd be good for these to not be redacted. Perhaps create a wrapper for debug.Stack() that returns a SafeValue?

Epic: CRDB-6668

Jira issue: CRDB-6914

knz commented 3 years ago

I don't get it, what's wrong with log.Infof("%+v", errors.New("stack trace"))?

andreimatei commented 3 years ago

Well I wouldn't have thought about that; there's two things that need to click for the writer: that errors have a stack in them and that you need to print the error with %+v in order to get the stack. That's two too many; compare to one method that gives me what I want - the stack trace as a string (no error; why am I creating an error?). It's not what the author of the code above wrote. It's also not what the authors of this or this wrote (I can probably find more).

Of course, having a debug.Stack() wrapper by itself would not mean that people use it. But I think the chances would be better, and we can also have a lint for it I guess.

knz commented 3 years ago

It's also not what the authors of this or this wrote (I can probably find more).

yeah this looks wrong.

Let's make a linter with some suggestions.

jordanlewis commented 1 year ago

I'd like to bump this back up in importance.