-
Notifications
You must be signed in to change notification settings - Fork 14.8k
MINOR: Include exception objects when logging group coordinator errors #20806
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: trunk
Are you sure you want to change the base?
MINOR: Include exception objects when logging group coordinator errors #20806
Conversation
...so that we capture stack traces for these errors.
|
I've been trying to investigate some errors in group coordinator loading/unloading and the lack of stack traces is getting in the way. eg. EDIT: The first error is due to https://issues.apache.org/jira/projects/KAFKA/issues/KAFKA-19857. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@squah-confluent thanks for this patch
| } catch (Throwable ex) { | ||
| log.error("Failed to load metadata from {} with epoch {} due to {}.", | ||
| tp, epoch, ex.toString()); | ||
| tp, epoch, ex.toString(), ex); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Because the whole exception ex is logged, changing ex.toString() to ex.getMessage() would eliminate a lot of redundant information in the log
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for reviewing! I've updated the PR to use getMessage instead.
| // already make an effort to catch exceptions in the unload method. | ||
| log.error("Failed to unload metadata for {} with epoch {} due to {}.", | ||
| tp, partitionEpoch, ex.toString()); | ||
| tp, partitionEpoch, ex.toString(), ex); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ditto
...so that we capture stack traces for these errors.