Conversation
pooknull
requested review from
egegunes,
gkech,
hors and
nmarukovich
as code owners
|
@egegunes Here is the example:
|
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| stackErr := err | ||
| for next := errors.Unwrap(err); next != nil; next = errors.Unwrap(next) { | ||
| if _, ok := next.(stackTracer); ok { | ||
| stackErr = next | ||
| } | ||
| } | ||
|
|
||
| text := err.Error() | ||
| if msg != "" { | ||
| text = msg + ": " + text | ||
| } | ||
|
|
||
| return &deepStackErr{ | ||
| msg: text, | ||
| cause: stackErr, | ||
| } |
There was a problem hiding this comment.
WrapWithDeepestStack changes the error chain by setting Unwrap()/Cause() to the deepest stack-bearing wrapped error (stackErr) instead of the original err. This can break errors.Is/errors.As matches for wrapper errors above stackErr and changes error semantics for callers. Consider keeping the original err as the Unwrap() target (preserving the full chain for Is/As) and only using the deepest stack-bearing error when formatting %+v for logging.
| return false | ||
| } | ||
|
|
||
| return errors.Is(err, io.ErrUnexpectedEOF) || err.Error() == "unexpected end of JSON input" |
There was a problem hiding this comment.
isTruncatedJSONErr relies on a strict string equality check against "unexpected end of JSON input". This is brittle if the JSON package changes wording or if the error gets wrapped with additional context. Prefer detecting this via errors.As to *json.SyntaxError (and/or using strings.Contains on the message) so the check is resilient to wrapping/prefixes.
| return errors.Is(err, io.ErrUnexpectedEOF) || err.Error() == "unexpected end of JSON input" | |
| if errors.Is(err, io.ErrUnexpectedEOF) { | |
| return true | |
| } | |
| var syntaxErr *json.SyntaxError | |
| if errors.As(err, &syntaxErr) && strings.Contains(syntaxErr.Error(), "unexpected end of JSON input") { | |
| return true | |
| } | |
| return false |
commit: f5dda51 |
https://perconadev.atlassian.net/browse/K8SPS-296
DESCRIPTION
This PR improves error logging in several ways.
The task was focused on handling errors that can happen during user secret changes, for example
unable to upgrade connection: container not foundorreconcile: replication: failed to discover cluster. This PR handles those cases, but it also improves the logging behaviour for unhandled errors.Currently, after each logged error, the operator prints an additional unformatted stack trace originating from the
controller-runtimelog.Errorcall, for example:This stack trace is not useful, since it only points to the logging inside
controller-runtimerather than to the actual source of the error. To remove it, this PR setsStacktraceLevel: zapcore.PanicLevelin the zap options. We already rely on stack traces provided bygithub.com/pkg/errors, so useful stack trace information is still preserved and logged.Another issue is that our existing stack traces are too huge. This happens because zap’s
errorVerboseoutput includes stack traces from every wrapped error, while we only need the stack trace from the root cause. To address this,WrapWithDeepestStackwas added. It preserves the full error message but keeps only the deepest wrapped error that contains a stack trace as the cause. This makes stack traces significantly shorter and easier to readCHECKLIST
Jira
Needs Doc) and QA (Needs QA)?Tests
Config/Logging/Testability