Make level configurable for NewStdLog

[heyimalex]

For things like net/http#Server.ErrorLog, it might make sense for all log lines to be at the error level. Since we can add fields and/or name the logger it's not a huge deal to pick these lines out of stream, but it kind of reduces the utility of levels as a catch all this-is-something-that-should-be-seen-by-a-human field.

Something to consider? Understand if it's too minor of a change, honestly having no issues working around it today.

[akshayjshah]

No opposition from me, but I probably won't get to this for a while.

[delicb]

I needed this for exact case that was mentioned (net/http#Server.ErrorLog). However, it turned out that I needed io.Writer that will produce logs as well and zap does not expose writer that it uses, so I wrote it myself and I am willing to create pull request. Would something like this be accepted (ignore hardcoded values for AddCallerSkip for now)?

// logWriter is wrapper around Logger that implements io.Writer interface.
type logWriter struct {
	logFunc func(msg string, fields ...zapcore.Field)
}

// Write implements io.Writer for logWriter.
// It will never return error.
func (l *logWriter) Write(p []byte) (n int, err error) {
	l.logFunc(string(p))
	return len(p), nil
}

// ToWriter creates and returns io.Writer that logs all messages written to in
// by using provided Logger and log level.
// If unsupported log level is provided as parameter there will be not errors,
// but Warn level is used.
func ToWriter(logger *zap.Logger, level zapcore.Level, options ...zap.Option) io.Writer {
	if logger == nil {
		return ioutil.Discard
	}
	var logFunc func(msg string, fields ...zapcore.Field)

	// since we are introducing new level of indirection, if provided
	// implementation is zap.Logger, we know how to indicate to it to skip
	// this level when reporting origin or the log message.
	logger = logger.WithOptions(zap.AddCallerSkip(2)).WithOptions(options...)

	switch level {
	case zapcore.DebugLevel:
		logFunc = logger.Debug
	case zapcore.InfoLevel:
		logFunc = logger.Info
	case zapcore.WarnLevel:
		logFunc = logger.Warn
	case zapcore.ErrorLevel:
		logFunc = logger.Error
	case zapcore.PanicLevel:
		logFunc = logger.Panic
	case zapcore.DPanicLevel:
		logFunc = logger.DPanic
	case zapcore.FatalLevel:
		logFunc = logger.Fatal
	default:
		logFunc = logger.Warn
	}
	return &logWriter{
		logFunc: logFunc,
	}
}

// NewStdLogger creates and returns new logger from standard library that
// writes all messages to provided zap logger. All messages will be logged
// under specified log level.
//
// zap.NewStdLog exist, but it does not allow specifying log level or custom
// options.
func NewStdLogger(logger *zap.Logger, level zapcore.Level, options ...zap.Option) *stdlog.Logger {
	writer := ToWriter(logger.WithOptions(zap.AddCallerSkip(2)), level, options...)
	return stdlog.New(writer, "", 0)
}

[akshayjshah]

I'm happy to take a PR for a NewStdLogAt function that takes a *zap.Logger and a level; it'll need to also return an error, since there's no guarantee that the user-supplied level is valid (e.g., zapcore.Level(42)). I'd prefer not to ignore invalid input.

I'm less convinced by the need to expose an io.Writer. Writers typically accept arbitrary bytes, but loggers only accept valid UTF-8 strings. We could validate the input, but that would be a very unusual writer. Can you tell me a little more about what you're trying to accomplish?

[delicb]

@akshayjshah
It makes sense, all that you wrote. This is code that solves my problem, I did not expect it to be accepted as is. That is why I asked first, before sending PR.

Of course, writer does not have to be public. I had to create it since I am using one library that writes stuff to output (stderr by default) and I can provide custom writer to it. I wanted to capture its output under specific logger name, so I wrote code above. Then I needed net/http#Server.ErrorLog and I built on top of that.

I understand that you do not want Writer to be public and that is fine. I am happy with just keeping that part in my own codebase.

So, would you like me to write NewStdLogAt (without writer part) and create PR? I might be able to do so in next couple of days, so we iterate on concrete proposal.

[akshayjshah]

😻 I'd love a PR that implements NewStdLogAt!

[akshayjshah]

This is (finally) resolved! It'll be included in the upcoming 1.7 release.

[heyimalex]

I appreciate you.