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 Errorf("error: %v", err) and sometimes we log Errorf("error: ", err.Error())
orouz
changed the title
Summary of your changes
replace
logpwithclog(custom logger) that wrapsErrorfin 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.