-
Notifications
You must be signed in to change notification settings - Fork 129
Convert byte array to string for logging in ProxyResponse #578
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
Conversation
gateway-ha/src/main/java/io/trino/gateway/proxyserver/ProxyResponseHandler.java
Outdated
Show resolved
Hide resolved
willmostly
left a comment
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.
Looks good % @oneonestar 's comment
8a7b688 to
601ef5b
Compare
gateway-ha/src/main/java/io/trino/gateway/proxyserver/ProxyResponseHandler.java
Outdated
Show resolved
Hide resolved
willmostly
left a comment
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.
Almost there
gateway-ha/src/main/java/io/trino/gateway/ha/config/ProxyResponseConfiguration.java
Show resolved
Hide resolved
f864013 to
a99357f
Compare
| private final List<String> statementPaths; | ||
| private final boolean includeClusterInfoInResponse; | ||
| private final TrinoRequestUser.TrinoRequestUserProvider trinoRequestUserProvider; | ||
| private final ProxyResponseConfiguration proxyResponseConfiguration; |
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.
Please avoid putting mutable objects in fields.
| { | ||
| try { | ||
| return new ProxyResponse(response.getStatusCode(), response.getHeaders(), response.getInputStream().readAllBytes()); | ||
| return new ProxyResponse(response.getStatusCode(), response.getHeaders(), new String(response.getInputStream().readNBytes((int) responseSize.toBytes()), StandardCharsets.UTF_8)); |
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.
Static import UTF_8.
Description
byte[]cannot get logged. Changing it to actualStringtype for loggingAdditional context and related issues
Release notes
(X) This is not user-visible or is docs only, and no release notes are required.
( ) Release notes are required. Please propose a release note for me.
( ) Release notes are required, with the following suggested text: