You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@hugegraph.apache.org by "SunnyBoy-WYH (via GitHub)" <gi...@apache.org> on 2024/03/01 13:24:11 UTC
[PR] fix(server): fix server slow log, support loader import & client IP [incubator-hugegraph]
SunnyBoy-WYH opened a new pull request, #2466:
URL: https://github.com/apache/incubator-hugegraph/pull/2466
before we support slow log ,but it cause bug when loader batch import data, PR https://github.com/apache/incubator-hugegraph/pull/2327
and later we downgrade it ,see pr : https://github.com/apache/incubator-hugegraph/pull/2347
the bug due to:
1. we need get post body from req, and we need set it back to request, so it changed.
2. the loader request use the "GZIP" header, after get post body, server cant read it
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@hugegraph.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@hugegraph.apache.org
For additional commands, e-mail: issues-help@hugegraph.apache.org
Re: [PR] fix(server): fix server slow log, support loader import & client IP [incubator-hugegraph]
Posted by "javeme (via GitHub)" <gi...@apache.org>.
javeme commented on code in PR #2466:
URL: https://github.com/apache/incubator-hugegraph/pull/2466#discussion_r1510163410
##########
hugegraph-server/hugegraph-api/src/main/java/org/apache/hugegraph/api/filter/PathFilter.java:
##########
@@ -35,24 +42,29 @@ public class PathFilter implements ContainerRequestFilter {
@Override
public void filter(ContainerRequestContext context) throws IOException {
- context.setProperty(REQUEST_TIME, System.currentTimeMillis());
+ long startTime = System.currentTimeMillis();
+
+ context.setProperty(REQUEST_TIME, startTime);
+
+ recordRequestJson(context);
+ }
- // TODO: temporarily comment it to fix loader bug, handle it later
- /*// record the request json
+ private void recordRequestJson(ContainerRequestContext context) throws IOException {
Review Comment:
prefer `collectRequestParams()`, seems 'record' means output to log
##########
hugegraph-server/hugegraph-api/src/main/java/org/apache/hugegraph/api/filter/PathFilter.java:
##########
@@ -35,24 +42,29 @@ public class PathFilter implements ContainerRequestFilter {
@Override
public void filter(ContainerRequestContext context) throws IOException {
- context.setProperty(REQUEST_TIME, System.currentTimeMillis());
+ long startTime = System.currentTimeMillis();
+
+ context.setProperty(REQUEST_TIME, startTime);
+
+ recordRequestJson(context);
+ }
- // TODO: temporarily comment it to fix loader bug, handle it later
- /*// record the request json
+ private void recordRequestJson(ContainerRequestContext context) throws IOException {
String method = context.getMethod();
- String requestParamsJson = "";
if (method.equals(HttpMethod.POST)) {
- requestParamsJson = IOUtils.toString(context.getEntityStream(),
- Charsets.toCharset(CHARSET));
- // replace input stream because we have already read it
- InputStream in = IOUtils.toInputStream(requestParamsJson, Charsets.toCharset(CHARSET));
- context.setEntityStream(in);
+ BufferedInputStream bufferedStream = new BufferedInputStream(context.getEntityStream());
+
+ bufferedStream.mark(Integer.MAX_VALUE);
+
+ context.setProperty(REQUEST_PARAMS_JSON,
+ IOUtils.toString(bufferedStream, Charsets.toCharset(CHARSET)));
+
+ bufferedStream.reset();
+
+ context.setEntityStream(bufferedStream);
} else if (method.equals(HttpMethod.GET)) {
- MultivaluedMap<String, String> pathParameters = context.getUriInfo()
- .getPathParameters();
- requestParamsJson = pathParameters.toString();
+ context.setProperty(REQUEST_PARAMS_JSON,
+ context.getUriInfo().getPathParameters().toString());
Review Comment:
can we also add all other branchs like PUT/DELETE
##########
hugegraph-server/hugegraph-api/src/main/java/org/apache/hugegraph/api/filter/PathFilter.java:
##########
@@ -35,24 +42,29 @@ public class PathFilter implements ContainerRequestFilter {
@Override
public void filter(ContainerRequestContext context) throws IOException {
- context.setProperty(REQUEST_TIME, System.currentTimeMillis());
+ long startTime = System.currentTimeMillis();
+
+ context.setProperty(REQUEST_TIME, startTime);
+
+ recordRequestJson(context);
+ }
- // TODO: temporarily comment it to fix loader bug, handle it later
- /*// record the request json
+ private void recordRequestJson(ContainerRequestContext context) throws IOException {
String method = context.getMethod();
- String requestParamsJson = "";
if (method.equals(HttpMethod.POST)) {
- requestParamsJson = IOUtils.toString(context.getEntityStream(),
- Charsets.toCharset(CHARSET));
- // replace input stream because we have already read it
- InputStream in = IOUtils.toInputStream(requestParamsJson, Charsets.toCharset(CHARSET));
- context.setEntityStream(in);
+ BufferedInputStream bufferedStream = new BufferedInputStream(context.getEntityStream());
+
+ bufferedStream.mark(Integer.MAX_VALUE);
+
+ context.setProperty(REQUEST_PARAMS_JSON,
+ IOUtils.toString(bufferedStream, Charsets.toCharset(CHARSET)));
Review Comment:
maybe it's very large for batch-write, can we cut part of the content?
##########
hugegraph-server/hugegraph-api/src/main/java/org/apache/hugegraph/api/filter/AccessLogFilter.java:
##########
@@ -109,13 +113,25 @@ public void filter(ContainerRequestContext requestContext,
// Record slow query if meet needs, watch out the perf
if (timeThreshold > 0 && executeTime > timeThreshold &&
needRecordLog(requestContext)) {
- // TODO: set RequestBody null, handle it later & should record "client IP"
- LOG.info("[Slow Query] execTime={}ms, body={}, method={}, path={}, query={}",
- executeTime, null, method, path, uri.getQuery());
+
+ LOG.info("[Slow Query] ip={} execTime={}ms, body={}, method={}, path={}, query={}",
+ getClientIP(requestContext), executeTime,
+ requestContext.getProperty(REQUEST_PARAMS_JSON), method, path,
+ uri.getQuery());
Review Comment:
are they repeated when GET: uri.getQuery() and REQUEST_PARAMS_JSON
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@hugegraph.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@hugegraph.apache.org
For additional commands, e-mail: issues-help@hugegraph.apache.org
Re: [PR] fix(server): fix server slow log, support loader import & client IP [incubator-hugegraph]
Posted by "codecov[bot] (via GitHub)" <gi...@apache.org>.
codecov[bot] commented on PR #2466:
URL: https://github.com/apache/incubator-hugegraph/pull/2466#issuecomment-1973212722
## [Codecov](https://app.codecov.io/gh/apache/incubator-hugegraph/pull/2466?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) Report
Attention: Patch coverage is `90.47619%` with `2 lines` in your changes are missing coverage. Please review.
> Project coverage is 48.28%. Comparing base [(`47a68f0`)](https://app.codecov.io/gh/apache/incubator-hugegraph/commit/47a68f098ae43cdc43369cab858e93c8a8b5f92c?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) to head [(`ee09bd1`)](https://app.codecov.io/gh/apache/incubator-hugegraph/pull/2466?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache).
> Report is 1 commits behind head on master.
| [Files](https://app.codecov.io/gh/apache/incubator-hugegraph/pull/2466?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | Patch % | Lines |
|---|---|---|
| [...g/apache/hugegraph/api/filter/AccessLogFilter.java](https://app.codecov.io/gh/apache/incubator-hugegraph/pull/2466?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-aHVnZWdyYXBoLXNlcnZlci9odWdlZ3JhcGgtYXBpL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9odWdlZ3JhcGgvYXBpL2ZpbHRlci9BY2Nlc3NMb2dGaWx0ZXIuamF2YQ==) | 77.77% | [2 Missing :warning: ](https://app.codecov.io/gh/apache/incubator-hugegraph/pull/2466?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
<details><summary>Additional details and impacted files</summary>
```diff
@@ Coverage Diff @@
## master #2466 +/- ##
=============================================
- Coverage 63.80% 48.28% -15.52%
+ Complexity 829 272 -557
=============================================
Files 511 511
Lines 42622 42583 -39
Branches 5947 5927 -20
=============================================
- Hits 27193 20562 -6631
- Misses 12679 19699 +7020
+ Partials 2750 2322 -428
```
</details>
[:umbrella: View full report in Codecov by Sentry](https://app.codecov.io/gh/apache/incubator-hugegraph/pull/2466?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache).
:loudspeaker: Have feedback on the report? [Share it here](https://about.codecov.io/codecov-pr-comment-feedback/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@hugegraph.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@hugegraph.apache.org
For additional commands, e-mail: issues-help@hugegraph.apache.org
Re: [PR] fix(server): fix server slow log, support loader import & client IP [incubator-hugegraph]
Posted by "SunnyBoy-WYH (via GitHub)" <gi...@apache.org>.
SunnyBoy-WYH commented on code in PR #2466:
URL: https://github.com/apache/incubator-hugegraph/pull/2466#discussion_r1510233703
##########
hugegraph-server/hugegraph-api/src/main/java/org/apache/hugegraph/api/filter/PathFilter.java:
##########
@@ -35,24 +42,29 @@ public class PathFilter implements ContainerRequestFilter {
@Override
public void filter(ContainerRequestContext context) throws IOException {
- context.setProperty(REQUEST_TIME, System.currentTimeMillis());
+ long startTime = System.currentTimeMillis();
+
+ context.setProperty(REQUEST_TIME, startTime);
+
+ recordRequestJson(context);
+ }
- // TODO: temporarily comment it to fix loader bug, handle it later
- /*// record the request json
+ private void recordRequestJson(ContainerRequestContext context) throws IOException {
String method = context.getMethod();
- String requestParamsJson = "";
if (method.equals(HttpMethod.POST)) {
- requestParamsJson = IOUtils.toString(context.getEntityStream(),
- Charsets.toCharset(CHARSET));
- // replace input stream because we have already read it
- InputStream in = IOUtils.toInputStream(requestParamsJson, Charsets.toCharset(CHARSET));
- context.setEntityStream(in);
+ BufferedInputStream bufferedStream = new BufferedInputStream(context.getEntityStream());
+
+ bufferedStream.mark(Integer.MAX_VALUE);
+
+ context.setProperty(REQUEST_PARAMS_JSON,
+ IOUtils.toString(bufferedStream, Charsets.toCharset(CHARSET)));
Review Comment:
maybe we can cut the special length? like 512?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@hugegraph.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@hugegraph.apache.org
For additional commands, e-mail: issues-help@hugegraph.apache.org