Use custom logger to downgrade canceled context errors to warnings #2936
Conversation
|
This pull request is now in conflicts. Could you fix it? 🙏 |
|
This pull request does not have a backport label. Could you fix it @orouz? 🙏
|
orouz
force-pushed
the
clog
branch
5 times, most recently
from
f6db853 to
d9a4656
Compare
| if hasErrorType(context.Canceled, args...) { | ||
| l.Warnf(template, args...) | ||
| return | ||
| } |
There was a problem hiding this comment.
this is reason for this PR - downgrading errors to warnings when we see context canceled
| func (l *Logger) Named(name string) *Logger { | ||
| return &Logger{l.Logger.Named(name)} | ||
| } | ||
|
|
||
| func (l *Logger) WithOptions(options ...logp.LogOption) *Logger { | ||
| return &Logger{l.Logger.WithOptions(options...)} | ||
| } | ||
| func (l *Logger) With(args ...any) *Logger { | ||
| return &Logger{l.Logger.With(args...)} | ||
| } |
There was a problem hiding this comment.
we're using these methods from logp and to make types work i had to implement them so their usage returns our custom logger
the implementation just calls the original methods
and the original logger is still available at l.Logger
so i believe it's fine and poses no risk afaik
| // Check if the error is of the same type | ||
| if err, ok := arg.(error); ok && errors.Is(err, errorType) { | ||
| return true | ||
| } | ||
|
|
||
| // Check if the error message contains the error type string | ||
| if str, ok := arg.(string); ok && strings.Contains(str, errorTypeStr) { | ||
| return true | ||
| } |
There was a problem hiding this comment.
need to account for both these cases as we sometimes log err and sometimes err.Error()
orouz
changed the title
| "github.com/gofrs/uuid" | ||
| "k8s.io/api/core/v1" | ||
| v1 "k8s.io/api/core/v1" // revive:disable-line |
There was a problem hiding this comment.
v1 is added by goimports but is considered redundant by revive. didn't find how to get around this other then disable review for the line
Summary of your changes
replace
logpwithclog(custom logger) that wrapsErrorfandErrormethods in order to downgradecontext.Canceledto warningsthe errors described in #1154 (table in google doc) are mostly the same ones shown in #2843, but all originate from the same reason - some ongoing request being canceled.
Screenshots/Data
as an example, here's the log for
GetBucketVersioningto test this, i ran cloudbeat in vscode in debug mode, set a breakpoint here, wait for it to stop and then continue. then sent
pkill -SIGINT __debug_binfrom the terminal to trigger a graceful shutdown while we know a fetcher is currently runningnote that except from the log level, nothing else in the log has changed while using the custom logger.