ronantakizawa/codereview-bench huggingface dataset:
Repository: uber-go/zap
File: zapcore/sampler.go (Go)

Code before:
		// We raced with another goroutine trying to reset, and it also reset
		// the counter to 1, so we need to reincrement the counter.
		return c.counter.Inc()
	}

	return 1
}

// SamplingDecision represents a decision made by sampler.
type SamplingDecision uint8

const (
	// LogDropped means that a log was dropped.
	LogDropped SamplingDecision = iota
	// LogSampled means that a log was successfully sampled.
	LogSampled
)

// optionFunc wraps a func so it satisfies the SamplerOption interface.
type optionFunc func(*sampler)

func (f optionFunc) apply(s *sampler) {
	f(s)
}

// SamplerOption configures a Sampler option.
type SamplerOption interface {
	apply(*sampler)
}

// NopSamplingHook is the default hook used by sampler.
func NopSamplingHook(_ Entry, _ SamplingDecision) {}

// SamplerHook registers a function  which will be called when Sampler makes a
// decision. Currently a hook is called when a log is dropped and
// zapcore.LogDropped decision is emitted.
//
// This hook is useful for side effects, for example emitting number of dropped
// logs. Note, there is no access to Fields in this hook. In the future, this
// hook can be expanded to emit whether this is first entry that was dropped,
// first after a period, etc.
func SamplerHook(hook func(entry Entry, dec SamplingDecision)) SamplerOption {
	return optionFunc(func(s *sampler) {
		s.hook = hook
	})
}

// NewSamplerWithOptions creates a Core that samples incoming entries, which
// caps the CPU and I/O load of logging while attempting to preserve a
// representative subset of your logs.
//

Review comment (suggestion):
```suggestion
// SamplerOption configures a Sampler.
```

Code after:
		// We raced with another goroutine trying to reset, and it also reset
		// the counter to 1, so we need to reincrement the counter.
		return c.counter.Inc()
	}

	return 1
}

// SamplingDecision is a decision represented as a bit field made by sampler.
// More decisions may be added in the future.
type SamplingDecision uint32

const (
	// LogDropped indicates that the Sampler dropped a log entry.
	LogDropped SamplingDecision = 1 << iota
	// LogSampled indicates that the Sampler sampled a log entry.
	LogSampled
)

// optionFunc wraps a func so it satisfies the SamplerOption interface.
type optionFunc func(*sampler)

func (f optionFunc) apply(s *sampler) {
	f(s)
}

// SamplerOption configures a Sampler.
type SamplerOption interface {
	apply(*sampler)
}

// nopSamplingHook is the default hook used by sampler.
func nopSamplingHook(Entry, SamplingDecision) {}

// SamplerHook registers a function  which will be called when Sampler makes a
// decision.
//
// This hook may be used to get visibility into the performance of the sampler.
// For example, use it to track metrics of dropped versus sampled logs.
//
//  var dropped atomic.Int64
//  zapcore.SamplerHook(func(ent zapcore.Entry, dec zapcore.SamplingDecision) {
//    if dec&zapcore.LogDropped > 0 {
//      dropped.Inc()
//    }
//  })
func SamplerHook(hook func(entry Entry, dec SamplingDecision)) SamplerOption {
	return optionFunc(func(s *sampler) {
		s.hook = hook
	})
}
Repository: uber-go/zap
File: config_test.go (Go)

Code before:
		{
			desc: "missing encoder time in encoder config",
			cfg: Config{
				Level:    NewAtomicLevelAt(zapcore.InfoLevel),
				Encoding: "json",
				EncoderConfig: zapcore.EncoderConfig{
					MessageKey: "msg",
					TimeKey:    "ts",
				},
			},
			expectErr: "missing EncodeTime in EncoderConfig",
		},
	}

	for _, tt := range tests {
		t.Run(tt.desc, func(t *testing.T) {
			cfg := tt.cfg
			_, err := cfg.Build()
			require.Error(t, err)
			assert.Equal(t, tt.expectErr, err.Error())
		})
	}
}

func makeSamplerCountingHook() (func(zapcore.Entry, zapcore.SamplingDecision),
	*atomic.Int64, *atomic.Int64) {
	droppedCount := new(atomic.Int64)
	sampledCount := new(atomic.Int64)
	h := func(_ zapcore.Entry, dec zapcore.SamplingDecision) {
		if dec == zapcore.LogDropped {
			droppedCount.Inc()
		} else if dec == zapcore.LogSampled {
			sampledCount.Inc()
		}
	}
	return h, droppedCount, sampledCount
}

func TestConfigWithSamplingHook(t *testing.T) {
	shook, dcount, scount := makeSamplerCountingHook()
	cfg := Config{
		Level:       NewAtomicLevelAt(InfoLevel),
		Development: false,
		Sampling: &SamplingConfig{
			Initial:    100,
			Thereafter: 100,
			Hook:       shook,
		},
		Encoding:         "json",
		EncoderConfig:    NewProductionEncoderConfig(),
		OutputPaths:      []string{"stderr"},

Review comment (bug):
can you name the returns? we also can do, `dropped, sampled *atomic.Int64` to avoid duplicating the type.

And a super nit: The way this declaration is broken up over multiple lines looks a little odd. We don't tend to force line length < 80, can we put the whole declaration on one line?

Code after:
		{
			desc: "missing encoder time in encoder config",
			cfg: Config{
				Level:    NewAtomicLevelAt(zapcore.InfoLevel),
				Encoding: "json",
				EncoderConfig: zapcore.EncoderConfig{
					MessageKey: "msg",
					TimeKey:    "ts",
				},
			},
			expectErr: "missing EncodeTime in EncoderConfig",
		},
	}

	for _, tt := range tests {
		t.Run(tt.desc, func(t *testing.T) {
			cfg := tt.cfg
			_, err := cfg.Build()
			require.Error(t, err)
			assert.Equal(t, tt.expectErr, err.Error())
		})
	}
}

func makeSamplerCountingHook() (h func(zapcore.Entry, zapcore.SamplingDecision),
	dropped, sampled *atomic.Int64) {
	dropped = new(atomic.Int64)
	sampled = new(atomic.Int64)
	h = func(_ zapcore.Entry, dec zapcore.SamplingDecision) {
		if dec&zapcore.LogDropped > 0 {
			dropped.Inc()
		} else if dec&zapcore.LogSampled > 0 {
			sampled.Inc()
		}
	}
	return h, dropped, sampled
}

func TestConfigWithSamplingHook(t *testing.T) {
	shook, dcount, scount := makeSamplerCountingHook()
	cfg := Config{
		Level:       NewAtomicLevelAt(InfoLevel),
		Development: false,
		Sampling: &SamplingConfig{
			Initial:    100,
			Thereafter: 100,
			Hook:       shook,
		},
		Encoding:         "json",
		EncoderConfig:    NewProductionEncoderConfig(),
		OutputPaths:      []string{"stderr"},
Repository: uber-go/zap
File: zapcore/sampler.go (Go)

Code before:
		hash ^= uint32(s[i])
		hash *= prime32
	}
	return hash
}

func (c *counter) IncCheckReset(t time.Time, tick time.Duration) uint64 {
	tn := t.UnixNano()
	resetAfter := c.resetAt.Load()
	if resetAfter > tn {
		return c.counter.Inc()
	}

	c.counter.Store(1)

	newResetAfter := tn + tick.Nanoseconds()
	if !c.resetAt.CAS(resetAfter, newResetAfter) {
		// We raced with another goroutine trying to reset, and it also reset
		// the counter to 1, so we need to reincrement the counter.
		return c.counter.Inc()
	}

	return 1
}

// SamplingDecision is a decision made by sampler.
type SamplingDecision uint8

const (
	// LogDropped indicates that the Sampler dropped a log entry.
	LogDropped SamplingDecision = 1 << iota
	// LogSampled indicates that the Sampler sampled a log entry.
	LogSampled
)

// optionFunc wraps a func so it satisfies the SamplerOption interface.
type optionFunc func(*sampler)

func (f optionFunc) apply(s *sampler) {
	f(s)
}

// SamplerOption configures a Sampler.
type SamplerOption interface {
	apply(*sampler)
}

// NopSamplingHook is the default hook used by sampler.
func NopSamplingHook(Entry, SamplingDecision) {}

// SamplerHook registers a function  which will be called when Sampler makes a

Review comment (suggestion):
we should comment that more decision values may be added in future, so it's not safe to assume these are the only values.

Code after:
		hash ^= uint32(s[i])
		hash *= prime32
	}
	return hash
}

func (c *counter) IncCheckReset(t time.Time, tick time.Duration) uint64 {
	tn := t.UnixNano()
	resetAfter := c.resetAt.Load()
	if resetAfter > tn {
		return c.counter.Inc()
	}

	c.counter.Store(1)

	newResetAfter := tn + tick.Nanoseconds()
	if !c.resetAt.CAS(resetAfter, newResetAfter) {
		// We raced with another goroutine trying to reset, and it also reset
		// the counter to 1, so we need to reincrement the counter.
		return c.counter.Inc()
	}

	return 1
}

// SamplingDecision is a decision represented as a bit field made by sampler.
// More decisions may be added in the future.
type SamplingDecision uint32

const (
	// LogDropped indicates that the Sampler dropped a log entry.
	LogDropped SamplingDecision = 1 << iota
	// LogSampled indicates that the Sampler sampled a log entry.
	LogSampled
)

// optionFunc wraps a func so it satisfies the SamplerOption interface.
type optionFunc func(*sampler)

func (f optionFunc) apply(s *sampler) {
	f(s)
}

// SamplerOption configures a Sampler.
type SamplerOption interface {
	apply(*sampler)
}

// nopSamplingHook is the default hook used by sampler.
func nopSamplingHook(Entry, SamplingDecision) {}
Repository: uber-go/zap
File: stacktrace_test.go (Go)

Code before:
package zap

import (
	"strings"
	"testing"

	"github.com/stretchr/testify/assert"
	"github.com/stretchr/testify/require"
)

func TestTakeStacktrace(t *testing.T) {
	trace := takeStacktrace(0)
	lines := strings.Split(trace, "\n")
	require.True(t, len(lines) > 0, "Expected stacktrace to have at least one frame.")
	assert.Contains(
		t,
		lines[0],
		"go.uber.org/zap.TestTakeStacktrace",
		"Expected stacktrace to start with the test %s.", lines[0],
	)
}

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++ {

Review comment (suggestion):
minor: Can use NotEmpty.
```suggestion
	require.NotEmpty(t, lines, "Expected stacktrace to have at least one frame.")
```

Code after:
package zap

import (
	"strings"
	"testing"

	"github.com/stretchr/testify/assert"
	"github.com/stretchr/testify/require"
)

func TestTakeStacktrace(t *testing.T) {
	trace := takeStacktrace(0)
	lines := strings.Split(trace, "\n")
	require.NotEmpty(t, lines, "Expected stacktrace to have at least one frame.")
	assert.Contains(
		t,
		lines[0],
		"go.uber.org/zap.TestTakeStacktrace",
		"Expected stacktrace to start with the test.",
	)
}

func TestTakeStacktraceWithSkip(t *testing.T) {
	trace := takeStacktrace(1)
	lines := strings.Split(trace, "\n")
	require.NotEmpty(t, lines, "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).",
	)
}

func TestTakeStacktraceWithSkipInnerFunc(t *testing.T) {
	var trace string
	func() {
		trace = takeStacktrace(2)
	}()
	lines := strings.Split(trace, "\n")
	require.NotEmpty(t, lines, "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).",
	)
}

func BenchmarkTakeStacktrace(b *testing.B) {
	for i := 0; i < b.N; i++ {
Repository: uber-go/zap
File: stacktrace_test.go (Go)

Code before:

	"github.com/stretchr/testify/assert"
	"github.com/stretchr/testify/require"
)

func TestTakeStacktrace(t *testing.T) {
	trace := takeStacktrace(0)
	lines := strings.Split(trace, "\n")
	require.True(t, len(lines) > 0, "Expected stacktrace to have at least one frame.")
	assert.Contains(
		t,
		lines[0],
		"go.uber.org/zap.TestTakeStacktrace",
		"Expected stacktrace to start with the test %s.", lines[0],
	)
}

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(0)
	}
}


Review comment (suggestion):
minor: Testify will include the `lines[0]` in the output so we don't need to.
```suggestion
		"Expected stacktrace to start with the test runner (skipping our own frame).",
```

Code after:

	"github.com/stretchr/testify/assert"
	"github.com/stretchr/testify/require"
)

func TestTakeStacktrace(t *testing.T) {
	trace := takeStacktrace(0)
	lines := strings.Split(trace, "\n")
	require.NotEmpty(t, lines, "Expected stacktrace to have at least one frame.")
	assert.Contains(
		t,
		lines[0],
		"go.uber.org/zap.TestTakeStacktrace",
		"Expected stacktrace to start with the test.",
	)
}

func TestTakeStacktraceWithSkip(t *testing.T) {
	trace := takeStacktrace(1)
	lines := strings.Split(trace, "\n")
	require.NotEmpty(t, lines, "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).",
	)
}

func TestTakeStacktraceWithSkipInnerFunc(t *testing.T) {
	var trace string
	func() {
		trace = takeStacktrace(2)
	}()
	lines := strings.Split(trace, "\n")
	require.NotEmpty(t, lines, "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).",
	)
}

func BenchmarkTakeStacktrace(b *testing.B) {
	for i := 0; i < b.N; i++ {
		takeStacktrace(0)
	}
}
Repository: uber-go/zap
File: stacktrace_test.go (Go)

Code before:
		t,
		lines[0],
		"go.uber.org/zap.TestTakeStacktrace",
		"Expected stacktrace to start with the test %s.", lines[0],
	)
}

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(0)
	}
}


Review comment (suggestion):
```suggestion
	require.NotEmpty(t, lines, "Expected stacktrace to have at least one frame.")
```

Code after:
		t,
		lines[0],
		"go.uber.org/zap.TestTakeStacktrace",
		"Expected stacktrace to start with the test.",
	)
}

func TestTakeStacktraceWithSkip(t *testing.T) {
	trace := takeStacktrace(1)
	lines := strings.Split(trace, "\n")
	require.NotEmpty(t, lines, "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).",
	)
}

func TestTakeStacktraceWithSkipInnerFunc(t *testing.T) {
	var trace string
	func() {
		trace = takeStacktrace(2)
	}()
	lines := strings.Split(trace, "\n")
	require.NotEmpty(t, lines, "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).",
	)
}

func BenchmarkTakeStacktrace(b *testing.B) {
	for i := 0; i < b.N; i++ {
		takeStacktrace(0)
	}
}
Repository: uber-go/zap
File: stacktrace_test.go (Go)

Code before:
}

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(0)
	}
}


Review comment (suggestion):
```suggestion
		"Expected stacktrace to start with the test function (skipping the test function.",
```

Code after:
}

func TestTakeStacktraceWithSkip(t *testing.T) {
	trace := takeStacktrace(1)
	lines := strings.Split(trace, "\n")
	require.NotEmpty(t, lines, "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).",
	)
}

func TestTakeStacktraceWithSkipInnerFunc(t *testing.T) {
	var trace string
	func() {
		trace = takeStacktrace(2)
	}()
	lines := strings.Split(trace, "\n")
	require.NotEmpty(t, lines, "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).",
	)
}

func BenchmarkTakeStacktrace(b *testing.B) {
	for i := 0; i < b.N; i++ {
		takeStacktrace(0)
	}
}
Repository: uber-go/zap
File: logger.go (Go)

Code before:
	willWrite := ce != nil

	// Set up any required terminal behavior.
	switch ent.Level {
	case zapcore.PanicLevel:
		ce = ce.Should(ent, zapcore.WriteThenPanic)
	case zapcore.FatalLevel:
		ce = ce.Should(ent, zapcore.WriteThenFatal)
	case zapcore.DPanicLevel:
		if log.development {
			ce = ce.Should(ent, zapcore.WriteThenPanic)
		}
	}

	// Only do further annotation if we're going to write this message; checked
	// entries that exist only for terminal behavior don't benefit from
	// annotation.
	if !willWrite {
		return ce
	}

	// Thread the error output through to the CheckedEntry.
	ce.ErrorOutput = log.errorOutput
	if log.addCaller || log.addFunction {
		pc := make([]uintptr, 1)
		numFrames := runtime.Callers(log.callerSkip+callerSkipOffset+1, pc)
		frame, _ := runtime.CallersFrames(pc[:numFrames]).Next()
		defined := frame.PC != 0
		if !defined {
			fmt.Fprintf(log.errorOutput, "%v Logger.check error: failed to get caller\n", time.Now().UTC())
			log.errorOutput.Sync()
		}

		if log.addCaller {
			ce.Entry.Caller = zapcore.NewEntryCaller(frame.PC, frame.File, frame.Line, defined)
		}
		if log.addFunction {
			ce.Entry.Function = frame.Function
		}
	}
	if log.addStack.Enabled(ce.Entry.Level) {
		ce.Entry.Stack = Stack("").String
	}

	return ce
}


Review comment (performance):
I think there is no additional performance impact, since `runtime.Caller` does the same thing under the hood, but can you check benchmarks to verify that there's no impact?

Code after:

	// Set up any required terminal behavior.
	switch ent.Level {
	case zapcore.PanicLevel:
		ce = ce.Should(ent, zapcore.WriteThenPanic)
	case zapcore.FatalLevel:
		ce = ce.Should(ent, zapcore.WriteThenFatal)
	case zapcore.DPanicLevel:
		if log.development {
			ce = ce.Should(ent, zapcore.WriteThenPanic)
		}
	}

	// Only do further annotation if we're going to write this message; checked
	// entries that exist only for terminal behavior don't benefit from
	// annotation.
	if !willWrite {
		return ce
	}

	// Thread the error output through to the CheckedEntry.
	ce.ErrorOutput = log.errorOutput
	if log.addCaller {
		frame, defined := getCallerFrame(log.callerSkip + callerSkipOffset)
		if !defined {
			fmt.Fprintf(log.errorOutput, "%v Logger.check error: failed to get caller\n", time.Now().UTC())
			log.errorOutput.Sync()
		}

		ce.Entry.Caller = zapcore.EntryCaller{
			Defined:  defined,
			PC:       frame.PC,
			File:     frame.File,
			Line:     frame.Line,
			Function: frame.Function,
		}
	}
	if log.addStack.Enabled(ce.Entry.Level) {
		ce.Entry.Stack = Stack("").String
	}

	return ce
}

// getCallerFrame gets caller frame. The argument skip is the number of stack
// frames to ascend, with 0 identifying the caller of getCallerFrame. The
// boolean ok is false if it was not possible to recover the information.
//
// Note: This implementation is similar to runtime.Caller, but it returns the whole frame.
func getCallerFrame(skip int) (frame runtime.Frame, ok bool) {
	const skipOffset = 2 // skip getCallerFrame and Callers
Repository: uber-go/zap
File: logger.go (Go)

Code before:
		if log.development {
			ce = ce.Should(ent, zapcore.WriteThenPanic)
		}
	}

	// Only do further annotation if we're going to write this message; checked
	// entries that exist only for terminal behavior don't benefit from
	// annotation.
	if !willWrite {
		return ce
	}

	// Thread the error output through to the CheckedEntry.
	ce.ErrorOutput = log.errorOutput
	if log.addCaller {
		pc := make([]uintptr, 1)
		numFrames := runtime.Callers(log.callerSkip+callerSkipOffset+1, pc)
		frame, _ := runtime.CallersFrames(pc[:numFrames]).Next()
		defined := frame.PC != 0
		if !defined {
			fmt.Fprintf(log.errorOutput, "%v Logger.check error: failed to get caller\n", time.Now().UTC())
			log.errorOutput.Sync()
		}

		ce.Entry.Caller = zapcore.NewEntryCaller(frame.PC, frame.File, frame.Line, defined)
		ce.Entry.Caller.Function = frame.Function
	}
	if log.addStack.Enabled(ce.Entry.Level) {
		ce.Entry.Stack = Stack("").String
	}

	return ce
}


Review comment (suggestion):
since we're setting fields anyway, we could just do `zapcore.EntryCaller{PC: frame.PC, File: frame.File, ...}` instead of using the constructor but then modifying (prefer to create once and use that instead of mutating the `Entry.Caller`)

Code after:
		if log.development {
			ce = ce.Should(ent, zapcore.WriteThenPanic)
		}
	}

	// Only do further annotation if we're going to write this message; checked
	// entries that exist only for terminal behavior don't benefit from
	// annotation.
	if !willWrite {
		return ce
	}

	// Thread the error output through to the CheckedEntry.
	ce.ErrorOutput = log.errorOutput
	if log.addCaller {
		frame, defined := getCallerFrame(log.callerSkip + callerSkipOffset)
		if !defined {
			fmt.Fprintf(log.errorOutput, "%v Logger.check error: failed to get caller\n", time.Now().UTC())
			log.errorOutput.Sync()
		}

		ce.Entry.Caller = zapcore.EntryCaller{
			Defined:  defined,
			PC:       frame.PC,
			File:     frame.File,
			Line:     frame.Line,
			Function: frame.Function,
		}
	}
	if log.addStack.Enabled(ce.Entry.Level) {
		ce.Entry.Stack = Stack("").String
	}

	return ce
}

// getCallerFrame gets caller frame. The argument skip is the number of stack
// frames to ascend, with 0 identifying the caller of getCallerFrame. The
// boolean ok is false if it was not possible to recover the information.
//
// Note: This implementation is similar to runtime.Caller, but it returns the whole frame.
func getCallerFrame(skip int) (frame runtime.Frame, ok bool) {
	const skipOffset = 2 // skip getCallerFrame and Callers

	pc := make([]uintptr, 1)
	numFrames := runtime.Callers(skip+skipOffset, pc[:])
	if numFrames < 1 {
		return
	}

	frame, _ = runtime.CallersFrames(pc).Next()