javeme commented on code in PR #2466:
URL:
https://github.com/apache/incubator-hugegraph/pull/2466#discussion_r1712628600
##########
hugegraph-server/hugegraph-api/src/main/java/org/apache/hugegraph/api/filter/PathFilter.java:
##########
@@ -32,27 +39,37 @@ public class PathFilter implements ContainerRequestFilter {
public static final String REQUEST_TIME = "request_time";
public static final String REQUEST_PARAMS_JSON = "request_params_json";
+ public static final int MAX_SLOW_LOG_BODY_LENGTH = 512;
@Override
public void filter(ContainerRequestContext context) throws IOException {
- context.setProperty(REQUEST_TIME, System.currentTimeMillis());
+ long startTime = System.currentTimeMillis();
+
+ context.setProperty(REQUEST_TIME, startTime);
+
+ collectRequestParams(context);
+ }
- // TODO: temporarily comment it to fix loader bug, handle it later
- /*// record the request json
+ private void collectRequestParams(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);
+ if (method.equals(HttpMethod.POST) || method.equals(HttpMethod.PUT) ||
+ method.equals(HttpMethod.DELETE)) {
+ BufferedInputStream bufferedStream = new
BufferedInputStream(context.getEntityStream());
+
+ bufferedStream.mark(Integer.MAX_VALUE);
+ String body = IOUtils.toString(bufferedStream,
+ Charsets.toCharset(CHARSET));
+ body = body.length() > MAX_SLOW_LOG_BODY_LENGTH ?
+ body.substring(0, MAX_SLOW_LOG_BODY_LENGTH) : body;
+
+ context.setProperty(REQUEST_PARAMS_JSON, body);
+
+ 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 -- seems still missing
##########
hugegraph-server/hugegraph-api/src/main/java/org/apache/hugegraph/api/filter/AccessLogFilter.java:
##########
@@ -128,6 +134,16 @@ public void filter(ContainerRequestContext requestContext,
}
}
+ private String getClientIP(ContainerRequestContext requestContext) {
+ try {
+ UriInfo uriInfo = requestContext.getUriInfo();
+ String host = uriInfo.getRequestUri().getHost();
Review Comment:
return here if host is an ip or empty?
##########
hugegraph-server/hugegraph-api/src/main/java/org/apache/hugegraph/api/filter/AccessLogFilter.java:
##########
@@ -114,9 +118,11 @@ 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,
Review Comment:
prefer this style: defining a local var for better readability:
for both: getClientIP and REQUEST_PARAMS_JSON.
##########
hugegraph-server/hugegraph-api/src/main/java/org/apache/hugegraph/api/filter/AccessLogFilter.java:
##########
@@ -128,6 +134,16 @@ public void filter(ContainerRequestContext requestContext,
}
}
+ private String getClientIP(ContainerRequestContext requestContext) {
+ try {
+ UriInfo uriInfo = requestContext.getUriInfo();
+ String host = uriInfo.getRequestUri().getHost();
+ return InetAddress.getByName(host).getHostAddress();
+ } catch (UnknownHostException e) {
+ return "unknown";
Review Comment:
prefer "<unknown_ip>"
--
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-unsubscr...@hugegraph.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscr...@hugegraph.apache.org
For additional commands, e-mail: issues-h...@hugegraph.apache.org
