Remove some now-unnecessary changes

config_test.go

@@ -68,21 +68,19 @@ func TestConfig(t *testing.T) {
 			logger, err := tt.cfg.Build(Hooks(hook))
 			require.NoError(t, err, "Unexpected error constructing logger.")
 
-			withNoStacktraceIgnorePrefixes(func() {
-				logger.Debug("debug")
-				logger.Info("info")
-				logger.Warn("warn")
+			logger.Debug("debug")
+			logger.Info("info")
+			logger.Warn("warn")
 
-				byteContents, err := ioutil.ReadAll(temp)
-				require.NoError(t, err, "Couldn't read log contents from temp file.")
-				logs := string(byteContents)
-				assert.Regexp(t, tt.expectRe, logs, "Unexpected log output.")
+			byteContents, err := ioutil.ReadAll(temp)
+			require.NoError(t, err, "Couldn't read log contents from temp file.")
+			logs := string(byteContents)
+			assert.Regexp(t, tt.expectRe, logs, "Unexpected log output.")
 
-				for i := 0; i < 200; i++ {
-					logger.Info("sampling")
-				}
-				assert.Equal(t, tt.expectN, count.Load(), "Hook called an unexpected number of times.")
-			})
+			for i := 0; i < 200; i++ {
+				logger.Info("sampling")
+			}
+			assert.Equal(t, tt.expectN, count.Load(), "Hook called an unexpected number of times.")
 		})
 	}
 }

field.go

@@ -191,8 +191,8 @@ func NamedError(key string, err error) zapcore.Field {
 
 // Stack constructs a field that stores a stacktrace of the current goroutine
 // under provided key. Keep in mind that taking a stacktrace is eager and
-// extremely expensive (relatively speaking); this function both makes an
-// allocation and takes ~10 microseconds.
+// expensive (relatively speaking); this function both makes an allocation and
+// takes about two microseconds.
 func Stack(key string) zapcore.Field {
 	// 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

field_test.go

@@ -164,11 +164,9 @@ func TestFieldConstructors(t *testing.T) {
 }
 
 func TestStackField(t *testing.T) {
-	withNoStacktraceIgnorePrefixes(func() {
-		f := Stack("stacktrace")
-		assert.Equal(t, "stacktrace", f.Key, "Unexpected field key.")
-		assert.Equal(t, zapcore.StringType, f.Type, "Unexpected field type.")
-		assert.Contains(t, f.String, "zap.TestStackField", "Expected stacktrace to contain caller.")
-		assertCanBeReused(t, f)
-	})
+	f := Stack("stacktrace")
+	assert.Equal(t, "stacktrace", f.Key, "Unexpected field key.")
+	assert.Equal(t, zapcore.StringType, f.Type, "Unexpected field type.")
+	assert.Contains(t, f.String, "zap.TestStackField", "Expected stacktrace to contain caller.")
+	assertCanBeReused(t, f)
 }

logger_test.go

@@ -360,24 +360,22 @@ func TestLoggerAddCallerFail(t *testing.T) {
 }
 
 func TestLoggerAddStacktrace(t *testing.T) {
-	withNoStacktraceIgnorePrefixes(func() {
-		assertHasStack := func(t testing.TB, obs observer.LoggedEntry) {
-			assert.Contains(t, obs.Entry.Stack, "zap.TestLoggerAddStacktrace", "Expected to find test function in stacktrace.")
-		}
-		withLogger(t, DebugLevel, opts(AddStacktrace(InfoLevel)), func(logger *Logger, logs *observer.ObservedLogs) {
-			logger.Debug("")
-			assert.Empty(
-				t,
-				logs.AllUntimed()[0].Entry.Stack,
-				"Unexpected stacktrack at DebugLevel.",
-			)
+	assertHasStack := func(t testing.TB, obs observer.LoggedEntry) {
+		assert.Contains(t, obs.Entry.Stack, "zap.TestLoggerAddStacktrace", "Expected to find test function in stacktrace.")
+	}
+	withLogger(t, DebugLevel, opts(AddStacktrace(InfoLevel)), func(logger *Logger, logs *observer.ObservedLogs) {
+		logger.Debug("")
+		assert.Empty(
+			t,
+			logs.AllUntimed()[0].Entry.Stack,
+			"Unexpected stacktrack at DebugLevel.",
+		)
 
-			logger.Info("")
-			assertHasStack(t, logs.AllUntimed()[1])
+		logger.Info("")
+		assertHasStack(t, logs.AllUntimed()[1])
 
-			logger.Warn("")
-			assertHasStack(t, logs.AllUntimed()[2])
-		})
+		logger.Warn("")
+		assertHasStack(t, logs.AllUntimed()[2])
 	})
 }
 

options.go

@@ -92,8 +92,7 @@ func AddCallerSkip(skip int) Option {
 }
 
 // AddStacktrace configures the Logger to record a stack trace for all messages at
-// or above a given level. Keep in mind that taking a stacktrace takes several
-// microseconds; relative to the cost of logging, this is quite slow.
+// or above a given level.
 func AddStacktrace(lvl zapcore.LevelEnabler) Option {
 	return optionFunc(func(log *Logger) {
 		log.addStack = lvl

stacktrace.go

@@ -40,26 +40,22 @@ var (
 	}
 )
 
-// takeStacktrace attempts to use the provided byte slice to take a stacktrace.
-// If the provided slice isn't large enough, takeStacktrace will allocate
-// successively larger slices until it can capture the whole stack.
 func takeStacktrace() string {
 	buffer := bufferpool.Get()
 	defer bufferpool.Put(buffer)
 	programCounters := _stacktracePool.Get().(*programCounters)
 	defer _stacktracePool.Put(programCounters)
 
 	for {
-		// skipping 2 skips the call to runtime.Counters and takeStacktrace
-		// so that the program counters start at the caller of takeStacktrace
+		// Skip the call to runtime.Counters and takeStacktrace so that the
+		// program counters start at the caller of takeStacktrace.
 		n := runtime.Callers(2, programCounters.pcs)
-		if n < len(programCounters.pcs) {
+		if n < cap(programCounters.pcs) {
 			programCounters.pcs = programCounters.pcs[:n]
 			break
 		}
-		// Do not put programCounters back in pool, will put larger buffer in
-		// This is in case our size is always wrong, we optimize to pul
-		// correctly-sized buffers back in the pool
+		// Don't put the too-short counter slice back into the pool; this lets
+		// the pool adjust if we consistently take deep stacktraces.
 		programCounters = newProgramCounters(len(programCounters.pcs) * 2)
 	}
 

stacktrace_test.go

@@ -22,50 +22,20 @@ package zap
 
 import (
 	"strings"
-	"sync"
 	"testing"
 
 	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
 )
 
-var (
-	_stacktraceTestLock sync.Mutex
-)
-
-func TestEmptyStacktrace(t *testing.T) {
-	withStacktraceIgnorePrefixes(
-		[]string{
-			"go.uber.org/zap",
-			"runtime.",
-			"testing.",
-		},
-		func() {
-			assert.Empty(t, takeStacktrace(), "stacktrace should be empty if we ignore runtime, testing, and functions in this package")
-		},
-	)
-}
-
-func TestAllStacktrace(t *testing.T) {
-	withNoStacktraceIgnorePrefixes(
-		func() {
-			stacktrace := takeStacktrace()
-			assert.True(t, strings.HasPrefix(stacktrace, "go.uber.org/zap.TestAllStacktrace"),
-				"stacktrace should start with TestAllStacktrace if no prefixes set, but is %s", stacktrace)
-		},
+func TestTakeStacktrace(t *testing.T) {
+	trace := takeStacktrace()
+	lines := strings.Split(trace, "\n")
+	require.True(t, len(lines) > 0, "Expected stacktrace to have at least one frame.")
+	assert.Contains(
+		t,
+		lines[0],
+		"TestTakeStacktrace",
+		"Expected stacktrace to start with this test function, but top frame is %s.", lines[0],
 	)
 }
-
-func withNoStacktraceIgnorePrefixes(f func()) {
-	withStacktraceIgnorePrefixes([]string{}, f)
-}
-
-func withStacktraceIgnorePrefixes(prefixes []string, f func()) {
-	_stacktraceTestLock.Lock()
-	defer _stacktraceTestLock.Unlock()
-	existing := _stacktraceIgnorePrefixes
-	_stacktraceIgnorePrefixes = prefixes
-	defer func() {
-		_stacktraceIgnorePrefixes = existing
-	}()
-	f()
-}