Always include purposefully-written messages for severe error cases#187
Conversation
florimondmanca
added
the
changelog/Added
| StringWriter sw = new StringWriter(); | ||
| e.printStackTrace(new PrintWriter(sw)); | ||
| logger.finer(message + ": " + sw.toString()); | ||
| logger.warning(message + ": " + sw.toString()); |
There was a problem hiding this comment.
Eventually I will remove this from the PR since it's unrelated, but:
A customer running on 2.8.1 mentioned that they were only seeing "An unexpected error occurred", but no other information — in particular, no stack trace. I wonder if it's because FINER level logs may not show up in all situations?
Should we change the log level here so that stack traces always show up?
Interestingly there's no logger.error(), so logger.warning() is the closest log level, but I believe it could still be disabled in practice. WDYT?
There was a problem hiding this comment.
Can we try with warning instead of finer and verify that the stack trace does show up?
There was a problem hiding this comment.
Yes. This is independent of the detailed error messages though so I've dropped it from this PR and will submit something separately.
|
@FlorianVeaux Thanks, think I still have to get CI in check and remove that |
|
Are tests failing locally? Seems like the jenkins CI is unable to checkout the repo 🤷 |
mgarabed
left a comment
mgarabed
left a comment
There was a problem hiding this comment.
This is great - having the unique messages will definitely help troubleshooting errors! 💯
2307aff
|
Remaining checkout issue in the |
3 participants
What does this PR do?
Update usages of
DatadogUtilities.severe(...)so that in each case we provide a purposefully written error message. This avoids users getting an unhelpful "An unexpected error occurred", and should help move forward some support cases.Description of the Change
Update all usages of
DatadogUtilities.severe(...). None should be passingmessage: nullanymore — please check.(I haven't updated the implementation of
severe()to disallow passingnullyet, I think we can treat that as a follow-up if necessary.)Alternate Designs
Possible Drawbacks
Verification Process
Additional Notes
Release Notes
Review checklist (to be filled by reviewers)
changelog/label attached. If applicable it should have thebackward-incompatiblelabel attached.do-not-merge/label attached.kind/andseverity/labels attached at least.