Honor CallerSkip when taking stack traces & add the StackSkip field

field.go

@@ -368,7 +368,22 @@ func Stack(key string) Field {
 	// from expanding the zapcore.Field union struct to include a byte slice. Since
 	// taking a stacktrace is already so expensive (~10us), the extra allocation
 	// is okay.
-	return String(key, takeStacktrace())
+	return String(key, takeStacktrace(0))
+}
+
+// StackSkip constructs a field similarly to Stack, but also skips the given
+// number of frames from the top of the stacktrace.
+func StackSkip(key string, skip int) Field {
+	// Skip the call to StackSkip
+	if skip != 0 {
+		skip++
+	}
+
+	// Returning the stacktrace as a string costs an allocation, but saves us
+	// from expanding the zapcore.Field union struct to include a byte slice. Since
+	// taking a stacktrace is already so expensive (~10us), the extra allocation
+	// is okay.
+	return String(key, takeStacktrace(skip))
 }
 
 // Duration constructs a field with the given key and value. The encoder

field_test.go

@@ -259,6 +259,22 @@ func TestStackField(t *testing.T) {
 	f := Stack("stacktrace")
 	assert.Equal(t, "stacktrace", f.Key, "Unexpected field key.")
 	assert.Equal(t, zapcore.StringType, f.Type, "Unexpected field type.")
-	assert.Equal(t, takeStacktrace(), f.String, "Unexpected stack trace")
+	assert.Equal(t, takeStacktrace(0), f.String, "Unexpected stack trace")
+	assertCanBeReused(t, f)
+}
+
+func TestStackSkipField(t *testing.T) {
+	f := StackSkip("stacktrace", 0)
+	assert.Equal(t, "stacktrace", f.Key, "Unexpected field key.")
+	assert.Equal(t, zapcore.StringType, f.Type, "Unexpected field type.")
+	assert.Equal(t, takeStacktrace(0), f.String, "Unexpected stack trace")
+	assertCanBeReused(t, f)
+}
+
+func TestStackSkipFieldWithSkip(t *testing.T) {
+	f := StackSkip("stacktrace", 1)
+	assert.Equal(t, "stacktrace", f.Key, "Unexpected field key.")
+	assert.Equal(t, zapcore.StringType, f.Type, "Unexpected field type.")
+	assert.Equal(t, takeStacktrace(1), f.String, "Unexpected stack trace")
 	assertCanBeReused(t, f)
 }

logger.go

@@ -48,7 +48,8 @@ type Logger struct {
 	addCaller bool
 	addStack  zapcore.LevelEnabler
 
-	callerSkip int
+	callerSkip            int
+	useCallerSkipForStack bool
 }
 
 // New constructs a new Logger from the provided zapcore.Core and Options. If
@@ -304,7 +305,11 @@ func (log *Logger) check(lvl zapcore.Level, msg string) *zapcore.CheckedEntry {
 		}
 	}
 	if log.addStack.Enabled(ce.Entry.Level) {
-		ce.Entry.Stack = Stack("").String
+		if log.useCallerSkipForStack {
+			ce.Entry.Stack = StackSkip("", log.callerSkip+callerSkipOffset).String
+		} else {
+			ce.Entry.Stack = Stack("").String
+		}
 	}
 
 	return ce

options.go

@@ -119,6 +119,14 @@ func AddStacktrace(lvl zapcore.LevelEnabler) Option {
 	})
 }
 
+// UseCallerSkipForStacktrace configures the logger to use the number of frames
+// that should be skipped for caller (CallerSkip) annotation when taking stacktraces.
+func UseCallerSkipForStacktrace(enabled bool) Option {
+	return optionFunc(func(log *Logger) {
+		log.useCallerSkipForStack = enabled
+	})
+}
+
 // IncreaseLevel increase the level of the logger. It has no effect if
 // the passed in level tries to decrease the level of the logger.
 func IncreaseLevel(lvl zapcore.LevelEnabler) Option {

stacktrace.go

@@ -43,15 +43,15 @@ var (
 	_zapStacktraceVendorContains = addPrefix("/vendor/", _zapStacktracePrefixes...)
 )
 
-func takeStacktrace() string {
+func takeStacktrace(skip int) string {
 	buffer := bufferpool.Get()
 	defer buffer.Free()
 	programCounters := _stacktracePool.Get().(*programCounters)
 	defer _stacktracePool.Put(programCounters)
 
 	var numFrames int
 	for {
-		// Skip the call to runtime.Counters and takeStacktrace so that the
+		// Skip the call to runtime.Callers and takeStacktrace so that the
 		// program counters start at the caller of takeStacktrace.
 		numFrames = runtime.Callers(2, programCounters.pcs)
 		if numFrames < len(programCounters.pcs) {
@@ -62,8 +62,9 @@ func takeStacktrace() string {
 		programCounters = newProgramCounters(len(programCounters.pcs) * 2)
 	}
 
+	// Skip all consecutive zap frames at the beginning if no skip is set.
+	skipZapFrames := skip == 0
 	i := 0
-	skipZapFrames := true // skip all consecutive zap frames at the beginning.
 	frames := runtime.CallersFrames(programCounters.pcs[:numFrames])
 
 	// Note: On the last iteration, frames.Next() returns false, with a valid
@@ -76,6 +77,11 @@ func takeStacktrace() string {
 			skipZapFrames = false
 		}
 
+		if skip > 0 {
+			skip--
+			continue
+		}
+
 		if i != 0 {
 			buffer.AppendByte('\n')
 		}

stacktrace_ext_test.go

@@ -120,6 +120,32 @@ func TestStacktraceFiltersVendorZap(t *testing.T) {
 	})
 }
 
+func TestStacktraceUseCallerSkipForStacktraceWithoutCallerSkip(t *testing.T) {
+	withLogger(t, func(logger *zap.Logger, out *bytes.Buffer) {
+		logger = logger.WithOptions(zap.UseCallerSkipForStacktrace(true))
+		func() {
+			logger.Error("test log")
+		}()
+
+		require.Contains(t, out.String(), "TestStacktraceUseCallerSkipForStacktraceWithoutCallerSkip.", "Should not skip too much")
+		require.Contains(t, out.String(), "TestStacktraceUseCallerSkipForStacktraceWithoutCallerSkip", "Should not skip too much")
+		verifyNoZap(t, out.String())
+	})
+}
+
+func TestStacktraceUseCallerSkipForStacktraceWithCallerSkip(t *testing.T) {
+	withLogger(t, func(logger *zap.Logger, out *bytes.Buffer) {
+		logger = logger.WithOptions(zap.UseCallerSkipForStacktrace(true), zap.AddCallerSkip(2))
+		func() {
+			logger.Error("test log")
+		}()
+
+		require.NotContains(t, out.String(), "TestStacktraceUseCallerSkipForStacktraceWithCallerSkip.", "Should skip as requested by caller skip")
+		require.Contains(t, out.String(), "TestStacktraceUseCallerSkipForStacktraceWithCallerSkip", "Should not skip too much")
+		verifyNoZap(t, out.String())
+	})
+}
+
 // withLogger sets up a logger with a real encoder set up, so that any marshal functions are called.
 // The inbuilt observer does not call Marshal for objects/arrays, which we need for some tests.
 func withLogger(t *testing.T, fn func(logger *zap.Logger, out *bytes.Buffer)) {

stacktrace_test.go

@@ -29,7 +29,7 @@ import (
 )
 
 func TestTakeStacktrace(t *testing.T) {
-	trace := takeStacktrace()
+	trace := takeStacktrace(0)
 	lines := strings.Split(trace, "\n")
 	require.True(t, len(lines) > 0, "Expected stacktrace to have at least one frame.")
 	assert.Contains(
@@ -68,8 +68,35 @@ func TestIsZapFrame(t *testing.T) {
 	})
 }
 
+func TestTakeStacktraceWithSkip(t *testing.T) {
+	trace := takeStacktrace(1)
+	lines := strings.Split(trace, "\n")
+	require.True(t, len(lines) > 0, "Expected stacktrace to have at least one frame.")
+	assert.Contains(
+		t,
+		lines[0],
+		"testing.",
+		"Expected stacktrace to start with the test runner (skipping our own frame) %s.", lines[0],
+	)
+}
+
+func TestTakeStacktraceWithSkipInnerFunc(t *testing.T) {
+	var trace string
+	func() {
+		trace = takeStacktrace(2)
+	}()
+	lines := strings.Split(trace, "\n")
+	require.True(t, len(lines) > 0, "Expected stacktrace to have at least one frame.")
+	assert.Contains(
+		t,
+		lines[0],
+		"testing.",
+		"Expected stacktrace to start with the test function (skipping the test function) %s.", lines[0],
+	)
+}
+
 func BenchmarkTakeStacktrace(b *testing.B) {
 	for i := 0; i < b.N; i++ {
-		takeStacktrace()
+		takeStacktrace(0)
 	}
 }