[FEATURE REQUEST] Allow custom logger by making logger an interface

[hibiken]

[FEATURE REQUEST] Allow custom logger by making logger an interface

[cayter]

Would you mind explaining why you need to use a custom logger with Background?

This would allow flexibility to use a more optimised logger like https://github.com/uber-go/zap or https://github.com/rs/zerolog and also standardise the logging message format that users might have.

Also, do you have suggestions on the API for the logger interface?

After looking into the source code, I think we can start with the simplest first:

type Logger interface {
  Debug(args ...interface{})
  Error(args ...interface{})
  Info(args ...interface{})
  Warn(args ...interface{})
}

#104 maybe useful if you are trying to log something before or after the task processing

I'm aware of #104 and it's not directly relevant to what this issue is about though we will still need #104 to be built. The idea of this issue is mainly to ensure asynq internal logging is using a custom logger that the users want.

Hope the above clarifies. Thanks.

[cayter]

Yep, it's almost impossible to fulfill all the 3rd party logger's interface. That's why I'd suggest to go with the simplest/minimal like what you're already doing.

type Logger interface {
    Debugf(format string, args ...interface{})
    Infof(format string, args ...interface{})
    Warnf(format string, args ...interface{})
    Errorf(format string, args ...interface{})
}

Note: I'm using f suffix to ensure we follow golang stdlib's convention for formatting a message printing.

Then if anyone wants to use a different logger, they could just do:

type MyLogger struct {
  *zap.SugaredLogger
}

func NewMyLogger() *MyLogger {
   ...
}

func (l *MyLogger) Debug(format string, args ...interface{}) {
  // If you're using uber-zap sugared logger
  l.Debugf(format, args...)
}

func (l *MyLogger) Info(format string, args ...interface{}) {
  l.Infof(format, args...)
}

...

worker := &asynq.Background{
   ...
   logger: NewMyLogger(),
   ...
}

In addition, this also comes with a benefit which we can easily mock the logger in unit tests.

[bojanz]

Thanks for the quick reply!

Your suggestion is much cleaner. I also see that it matches what the stdlib logger does, so definitely my fault for not researching that, apologies. So, perhaps the solution is not to rethink the interface, but document an example somewhere?

[hibiken]

Thank you for opening this issue!

Would you mind explaining why you need to use a custom logger with Background?

Also, do you have suggestions on the API for the logger interface?

FYI: #104 maybe useful if you are trying to log something before or after the task processing.

[hibiken]

Got it. Thank you for the explanations 👍

Here's my initial proposal (feel free to give me feedback):

type Logger interface {
    Debug(format string, args ...interface{})
    Info(format string, args ...interface{})
    Warn(format string, args ...interface{})
    Error(format string, args ...interface{})
}

I see other popular logging libraries (logrus, glog) have similar interfaces.

These libraries have variant for each log level methods, for example:

• Error(args ...interface{})
• Errorf(format string, args ...interface{})
• Errorln(args ...interface{})

but I feel like having to implement all these to satisfy the interface is a pain.
And I think the first and the last variant can be accomplished by the XXXf(format string, args ..interface{}), so I think only supporting this variant for each log level is sufficient?

What do you think?

[bojanz]

I feel like this feature has regressed since the initial implementation, with the format parameter now removed.

If I have to do something like this:

type asynqAdapter struct {
	log *zerolog.Logger
}

func (a asynqAdapter) Debug(args ...interface{}) {
	a.log.Debug().Msg(args[0].(string))
}

Then why am I even getting an args slice and not a single message? Or am I missing how this is supposed to be integrated with a structured logger?

[hibiken]

I feel like this feature has regressed since the initial implementation, with the format parameter now removed.

We never had a variant with a format string (e.g. Debugf). The discussion in this thread may be confusing because at one point we considered adding the formatted variants, but I decided not to go down that route since we want to make it as easy as possible to satisfy the interface (i.e. less methods to implement)

Then why am I even getting an args slice and not a single message? Or am I missing how this is supposed to be integrated with a structured logger?

That's a valid point. We may need to revisit the API for this custom logger. Please feel free to drop your suggestions here 👍

In the meantime, would something like this work for your use case?

func (a asynqAdapter) Debug(args ...interface{}) {
	a.log.Debug().Msg(fmt.Sprint(args...))
}