-
Notifications
You must be signed in to change notification settings - Fork 26
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
Response code logging tweaks - different log levels for success, redi… #427
Response code logging tweaks - different log levels for success, redi… #427
Conversation
Thank you @scypio for this fix. |
@@ -144,12 +145,21 @@ private String encode(String value) { | |||
} | |||
|
|||
private ClientResponse toResponse(Buffer buffer, final HttpClientResponse httpResponse) { | |||
if (httpResponse.statusCode() >= 300 && httpResponse.statusCode() < 400) { //redirect responses | |||
LOGGER.info("Repository 3xx response: {}, Headers[{}]", httpResponse.statusCode(), | |||
if (HttpStatusClass.SUCCESS.contains(httpResponse.statusCode())) { |
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.
For a better readability of each condition, get the .statusCode()
into variable and do the check against that variable
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.
Done
HTTP status code logging tweaks - PR changes
Http status logging tweaks - typo fixed
Would it be possible to add information about requested resource while logging response? |
What do you think about adding request path (especially in warn and error level) in the message? |
@Skejven @mateuszgrab-cognifide I added request URL logging for 4xx, 5xx response codes |
…rect and error responses
Currently every response from repository that return HTTP status code different than 200 is logged using ERROR logger level. The strategy has been adjusted.
Description
2xx - success - log at debug
3xx - redirect - log at info
4xx - client error - log at warn
5xx - server error - log at error
other - log at warn
#423
Motivation and Context
Too many error messages may trigger unwanted warnings for devops. Error log level should indicate that something is wrong.
Screenshots (if appropriate):
Types of changes
Checklist:
I hereby agree to the terms of the Knot.x Contributor License Agreement.