javeme commented on code in PR #2569:
URL: 
https://github.com/apache/incubator-hugegraph/pull/2569#discussion_r1818078963


##########
hugegraph-server/hugegraph-api/src/main/java/org/apache/hugegraph/api/filter/AccessLogFilter.java:
##########
@@ -77,6 +79,27 @@ private String join(String path1, String path2) {
         return String.join(DELIMITER, path1, path2);
     }
 
+    private String normalizePath(ContainerRequestContext requestContext) {
+        // Replace variable parts of the path with placeholders
+        String requestPath = requestContext.getUriInfo().getPath();
+        // get uri params
+        MultivaluedMap<String, String> pathParameters = 
requestContext.getUriInfo()
+                                                                      
.getPathParameters();
+
+        String newPath = requestPath;
+        for (Map.Entry<String, java.util.List<String>> entry : 
pathParameters.entrySet()) {
+            String key = entry.getKey();
+            String value = entry.getValue().get(0);
+            if ("graph".equals(key)) {
+                newPath = newPath.replace(key, value);
+            }
+            newPath = newPath.replace(value, key);
+        }
+
+        LOG.trace("Original Path: {} New Path: {}", requestPath, newPath);

Review Comment:
   expect `"normalize path, original path: '{}', new path: '{}'"`



##########
hugegraph-server/hugegraph-api/src/main/java/org/apache/hugegraph/api/filter/AccessLogFilter.java:
##########
@@ -88,15 +111,23 @@ public void filter(ContainerRequestContext requestContext,
                        ContainerResponseContext responseContext) throws 
IOException {
         // Grab corresponding request / response info from context;
         URI uri = requestContext.getUriInfo().getRequestUri();
-        String path = uri.getRawPath();
         String method = requestContext.getMethod();
+        String path = normalizePath(requestContext);
         String metricsName = join(path, method);
 
-        MetricsUtil.registerCounter(join(metricsName, 
METRICS_PATH_TOTAL_COUNTER)).inc();
+        int status = responseContext.getStatus();
+        if (status != 500 && status != 415) {
+            MetricsUtil.registerCounter(join(metricsName, 
METRICS_PATH_TOTAL_COUNTER)).inc();
+        }
+
         if (statusOk(responseContext.getStatus())) {
             MetricsUtil.registerCounter(join(metricsName, 
METRICS_PATH_SUCCESS_COUNTER)).inc();
         } else {
-            MetricsUtil.registerCounter(join(metricsName, 
METRICS_PATH_FAILED_COUNTER)).inc();
+            //TODO: The return codes for compatibility need to be further 
detailed.

Review Comment:
   expect a space "// TODO"



##########
hugegraph-server/hugegraph-api/src/main/java/org/apache/hugegraph/api/filter/AccessLogFilter.java:
##########
@@ -105,8 +136,11 @@ public void filter(ContainerRequestContext requestContext,
             long start = (Long) requestTime;
             long executeTime = now - start;
 
-            MetricsUtil.registerHistogram(join(metricsName, 
METRICS_PATH_RESPONSE_TIME_HISTOGRAM))
-                       .update(executeTime);
+            if (status != 500 && status != 415) {

Review Comment:
   prefer to add a method is isStatusXx() for a clear meaning and expansion 



##########
hugegraph-server/hugegraph-api/src/main/java/org/apache/hugegraph/api/filter/AccessLogFilter.java:
##########
@@ -77,6 +79,27 @@ private String join(String path1, String path2) {
         return String.join(DELIMITER, path1, path2);
     }
 
+    private String normalizePath(ContainerRequestContext requestContext) {

Review Comment:
   mark static?



-- 
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: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]


---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to