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
fix(server): fix server slow log, support loader import & client IP #2466
base: master
Are you sure you want to change the base?
Conversation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #2466 +/- ##
============================================
+ Coverage 63.80% 66.25% +2.44%
Complexity 829 829
============================================
Files 511 511
Lines 42622 42643 +21
Branches 5947 5949 +2
============================================
+ Hits 27193 28251 +1058
+ Misses 12679 11579 -1100
- Partials 2750 2813 +63 ☔ View full report in Codecov by Sentry. |
|
||
// TODO: temporarily comment it to fix loader bug, handle it later | ||
/*// record the request json | ||
private void recordRequestJson(ContainerRequestContext context) throws IOException { |
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.
prefer collectRequestParams()
, seems 'record' means output to log
bufferedStream.mark(Integer.MAX_VALUE); | ||
|
||
context.setProperty(REQUEST_PARAMS_JSON, | ||
IOUtils.toString(bufferedStream, Charsets.toCharset(CHARSET))); |
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.
maybe it's very large for batch-write, can we cut part of the content?
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.
maybe we can cut the special length? like 512?
.getPathParameters(); | ||
requestParamsJson = pathParameters.toString(); | ||
context.setProperty(REQUEST_PARAMS_JSON, | ||
context.getUriInfo().getPathParameters().toString()); |
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.
can we also add all other branchs like PUT/DELETE
LOG.info("[Slow Query] ip={} execTime={}ms, body={}, method={}, path={}, query={}", | ||
getClientIP(requestContext), executeTime, | ||
requestContext.getProperty(REQUEST_PARAMS_JSON), method, path, | ||
uri.getQuery()); |
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.
are they repeated when GET: uri.getQuery() and REQUEST_PARAMS_JSON
fix #2468
we support slow log before ,but it cause bug when loader batch import data, the feat PR see #2327
and later we downgrade it ,see pr : #2347
the bug due to:
so we ready to resolve it , use BufferedInputStream to cache the stream:
` BufferedInputStream bufferedStream = new BufferedInputStream(context.getEntityStream());
`