Turn off logger or noop logger

[craigpastro]

Hi again! In my tests I am using goose to migrate my database to the latest version, and it works great. The problem is that when I run benchmarks goose is logging some statements in-between which affects the output. I have another process which consumes that output, so I had to turn off logging for that one section of my code. Something like

writer := log.Writer()
log.SetOutput(io.Discard)
err = goose.Up(db, migrationDir)
require.NoError(t, err)
log.SetOutput(writer)

I thought it would be nice if there was some way that I could turn off logging in goose, or at least if there was a noop logger I could give to SetLogger, so something like

goose.SetLogger(goose.NewNoopLogger())

Happy to make a PR if you would consider this. Or perhaps there is a better solution?

Thanks!

[mfridman]

Indeed, this is less than ideal. There are more than a few places where the goose library calls the standard library log.Print*. I was wrong, we fixed this and use the interface everywhere except the tests.

You could implement your own noop Logger and set it via goose.SetLogger?

You might also be able to set this globally log.SetOutput(ioutil.Discard), but obviously this isn't great.

We have leaked too many some CLI implementation details into the core goose library. This is one item I'd like to address holistically across the entire library via #379.

In the interim (for the current /v3 version), we could ensure all call sites are using the global logger (which wraps the standard library logger) .. and then you can implement the Logger interface making it noop or whatever.

I double-checked and we indeed use the correct logger throughout goose

type Logger interface {
	Fatal(v ...interface{})
	Fatalf(format string, v ...interface{})
	Print(v ...interface{})
	Println(v ...interface{})
	Printf(format string, v ...interface{})
}

side note, I still genuinely wish there was a standard library interface so goose (and literally every. single. other. package didn't have to define their owner interface).

golang/go#13182
golang/go#28412

[craigpastro]

Yeah, a standard logger interface would be nice for sure.

For the moment I am using log.SetOutput(ioutil.Discard), but you're right, it's not ideal. I also don't want to implement my own logger noop logger just to turn off logs from goose. We are using a logging interface which is different again. haha.

So, yeah, for the moment the best approach is probably log.SetOutput(ioutil.Discard).

Thanks for the thoughts!

[mfridman]

Let's keep this open. Adding a single goose.NewNoopLogger() isn't a bad idea actually.

Quite often packages that expose interfaces will have convenience functions like this, so not totally against it.

[mfridman]

Simple enough to do, and I can see this being useful. DiscardLogger made sense to me, open to suggestions though.

#384

[craigpastro]

Oh, nice! Thank you. I will use this for sure.

I am partial to noop and I think it is pretty common. For example, zap uses NewNop and opentelemetry uses NewNoopTracerProvider. Although, DiscardLogger also sounds fine to me.

[mfridman]

Ye that's a good point, I've seen nop used quite often as well, let's go with that.

[craigpastro]

This is great. Thank you!