einride/cloudrunner-go

How to use cloudrunner.Logger properly?

Closed this issue · 5 comments

I got this

[go-integration-test] panic: cloudrunner.Logger must be called with a context from cloudrunner.Run [recovered]

error during an integration test while I added some code like this in a go file that talks to some db:

func (r *Repo) getStuff(ctx context.Context) ([]*stuff, error){
    logger := cloudrunner.Logger(ctx)
    logger.Info("...")
    ...
}

Is it because I used the logger wrongly?

Thank you.

Is the method called through an HTTP or GRPC server setup by cloudrunner?

Calling the method directly will panic as cloudrunner.Logger does not setup a default logger.

If you need to run the method directly, one way would be to do the same as cloudrunner.Logger method does but return a Nop logger instead

logger, ok := cloudzap.GetLogger(ctx)
if !ok {
		logger = zap.NewNop()	
}

This way when you call it directly it will just return a dummy logger that does nothing. Of course with this, if you somehow were to call the method directly in production you not get any logs out of it indicating that there is a configuration issue

@liufuyang - found this issue because I just encountered similar issue in a test, i.e., moved from injecting *zap.Logger into a struct to getting the logger from context instead.

Solved this in my tests by providing a new context with a *zap.Logger, like this to continue on your example:

func Test_getStuff(t *testing.T) {
    ctx := cloudzap.WithLogger(context.Background(), zap.NewExample())
    _, err := repo.getStuff(ctx)
    assert.NilError(t, err)
}

As mentioned in @alethenorio example, zap.NewNop() may be used as well to omit logs during the tests.

For testing, use zaptest.NewLogger(t).
https://pkg.go.dev/go.uber.org/zap/zaptest#NewLogger

// NewLogger builds a new Logger that logs all messages to the given
// testing.TB.
//
// logger := zaptest.NewLogger(t)
//
// Use this with a *testing.T or *testing.B to get logs which get printed only
// if a test fails or if you ran go test -v.

So basically we can and should do this in test?

ctx = cloudzap.WithLogger(ctx, zaptest.NewLogger(t))

Closing it now as we seem to have a solution :)