[GH-ISSUE #108] [FEATURE REQUEST] Allow custom logger by making logger an interface #2055

Closed
opened 2026-03-15 18:55:33 +03:00 by kerem · 7 comments
Owner

Originally created by @cayter on GitHub (Mar 10, 2020).
Original GitHub issue: https://github.com/hibiken/asynq/issues/108

Originally assigned to: @hibiken on GitHub.

Is your feature request related to a problem? Please describe.
I'm currently trying to integrate asynq into my web framework(heavily inspired by Rails which also supports SPA prerender using chromedp when running in docker image and more feature improvements coming from Rails experience) which it already has its own logger.

Describe the solution you'd like
I'd like to plug in a custom logger to the Background instance (I currently call it worker which will be triggered by the command go run . worker in debug build or <BINARY> worker in release build).

Side note
I have reviewed a few other solutions:

I decided to go with asynq as I believe this library will grow to be like Sidekiq as much as possible given what was mentioned in the README.md. Thanks.

Originally created by @cayter on GitHub (Mar 10, 2020). Original GitHub issue: https://github.com/hibiken/asynq/issues/108 Originally assigned to: @hibiken on GitHub. **Is your feature request related to a problem? Please describe.** I'm currently trying to integrate `asynq` into my [web framework](https://github.com/appist/appy)(heavily inspired by Rails which also supports SPA prerender using chromedp when running in docker image and more feature improvements coming from Rails experience) which it already has its own logger. **Describe the solution you'd like** I'd like to plug in a custom logger to the Background instance (I currently call it `worker` which will be triggered by the command `go run . worker` in debug build or `<BINARY> worker` in release build). **Side note** I have reviewed a few other solutions: - https://github.com/gocraft/work (has more features than `asynq` but isn't very reliable which they came up with v2) - https://github.com/taylorchu/work (v2 of gocraft/work) - https://github.com/vmihailenco/taskq I decided to go with `asynq` as I believe this library will grow to be like `Sidekiq` as much as possible given what was mentioned in the `README.md`. Thanks.
kerem 2026-03-15 18:55:33 +03:00
Author
Owner

@hibiken commented on GitHub (Mar 11, 2020):

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.

<!-- gh-comment-id:597430092 --> @hibiken commented on GitHub (Mar 11, 2020): 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.
Author
Owner

@cayter commented on GitHub (Mar 11, 2020):

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.

<!-- gh-comment-id:597436631 --> @cayter commented on GitHub (Mar 11, 2020): > 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: ```go 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.
Author
Owner

@hibiken commented on GitHub (Mar 11, 2020):

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?

<!-- gh-comment-id:597445389 --> @hibiken commented on GitHub (Mar 11, 2020): Got it. Thank you for the explanations 👍 Here's my initial proposal (feel free to give me feedback): ```go 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](https://godoc.org/github.com/sirupsen/logrus#Logger), [glog](https://godoc.org/github.com/golang/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?
Author
Owner

@cayter commented on GitHub (Mar 11, 2020):

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.

<!-- gh-comment-id:597462121 --> @cayter commented on GitHub (Mar 11, 2020): 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. ```go 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: ```go 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.
Author
Owner

@bojanz commented on GitHub (Dec 25, 2020):

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?

<!-- gh-comment-id:751258172 --> @bojanz commented on GitHub (Dec 25, 2020): 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?
Author
Owner

@hibiken commented on GitHub (Dec 25, 2020):

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...))
}
<!-- gh-comment-id:751294948 --> @hibiken commented on GitHub (Dec 25, 2020): > 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? ```go func (a asynqAdapter) Debug(args ...interface{}) { a.log.Debug().Msg(fmt.Sprint(args...)) } ```
Author
Owner

@bojanz commented on GitHub (Dec 26, 2020):

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?

<!-- gh-comment-id:751388117 --> @bojanz commented on GitHub (Dec 26, 2020): 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?
Sign in to join this conversation.
No milestone
No project
No assignees
1 participant
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference
starred/asynq#2055
No description provided.