mirror of
https://github.com/hibiken/asynq.git
synced 2026-04-26 07:25:56 +03:00
[GH-ISSUE #108] [FEATURE REQUEST] Allow custom logger by making logger an interface #2055
Labels
No labels
CLI
bug
designing
documentation
duplicate
enhancement
good first issue
good first issue
help wanted
idea
invalid
investigate
needs-more-info
performance
pr-welcome
pull-request
question
wontfix
work in progress
work in progress
work-around-available
No milestone
No project
No assignees
1 participant
Notifications
Due date
No due date set.
Dependencies
No dependencies set.
Reference
starred/asynq#2055
Loading…
Add table
Add a link
Reference in a new issue
No description provided.
Delete branch "%!s()"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
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
asynqinto 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
workerwhich will be triggered by the commandgo run . workerin debug build or<BINARY> workerin release build).Side note
I have reviewed a few other solutions:
asynqbut isn't very reliable which they came up with v2)I decided to go with
asynqas I believe this library will grow to be likeSidekiqas much as possible given what was mentioned in theREADME.md. Thanks.@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.
@cayter commented on GitHub (Mar 11, 2020):
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.
After looking into the source code, I think we can start with the simplest first:
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
asynqinternal logging is using a custom logger that the users want.Hope the above clarifies. Thanks.
@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):
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?
@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.
Note: I'm using
fsuffix 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:
In addition, this also comes with a benefit which we can easily mock the logger in unit tests.
@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:
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 commented on GitHub (Dec 25, 2020):
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)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?
@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?