CallerSkip does not apply to stacktraces captured by zap

[prashantv]

Currently, logging wrappers use CallerSkip to avoid the logging wrapper showing up as the caller. However, this is not used by stacktraces, so we end up with the logging wrappers showing up in stacktraces.

@akshayjshah: I'm thinking we should respect caller skip in stacktraces as well, any thoughts?

[akshayjshah]

On balance, I'd prefer to leave the stacktraces as complete as possible. Managing caller skips is really finicky - we've made mistakes ourselves when wrapping and unwrapping loggers internally. It's not the end of the world when the caller reported in the logs is incorrect, but it'd be a real pain if we mistakenly trimmed off too many stack frames.

Even the trimming functionality we have today is a bit unusual - most other libraries I've seen (including ones in other languages) seem to bias toward more-complete stack traces. Sentry and similar tools highlight stack frames from the application code, so I don't think including extra information is too harmful.

I'll leave this open for a bit longer so we can ruminate.

[liu-xuewen]

I think it's very nice. This is very important. because the invalid log is too much. @akshayjshah It is not very convenient to find bug.

[dan-j]

I know this is an old issue, but I've just been reading through this and reviewed what #525 was trying to achieve.

I understand why the PR was closed, but how about an alternative solution of allowing users to add custom prefixes to omit from stacktraces? This would mean there wouldn't be any breaking changes to existing users as it would be opt-in by providing said prefixes.

For context, I'm using https://github.com/go-logr/zapr and would like to omit the stacks from this wrapper, similar to the OP

[prashantv]

While we can add the ability to omit specific prefixes, I think it may be better to align the caller skip with the stack trace collection, so users don't have to opt-in to prefix stripping, as well as increasing the caller skip.