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


##########
hugegraph-server/hugegraph-api/src/main/java/org/apache/hugegraph/metrics/SlowQueryLog.java:
##########
@@ -19,25 +19,36 @@
 
 public class SlowQueryLog {
 
-    public Long executeTime;
-
-    public Long startTime;
-
     public String rawQuery;
 
     public String method;
 
-    public Long threshold;
-
     public String path;
 
-    public SlowQueryLog(Long executeTime, Long startTime, String rawQuery, 
String method, Long threshold,
-                        String path) {
-        this.executeTime = executeTime;
-        this.startTime = startTime;
+    public long executeTime;
+
+    public long startTime;
+
+    public long threshold;

Review Comment:
   prefer thresholdTime



##########
hugegraph-server/hugegraph-api/src/main/java/org/apache/hugegraph/api/filter/AccessLogFilter.java:
##########
@@ -77,48 +95,28 @@ public void filter(ContainerRequestContext requestContext, 
ContainerResponseCont
             MetricsUtil.registerCounter(join(metricsName, 
METRICS_PATH_FAILED_COUNTER)).inc();
         }
 
-        // get responseTime
         Object requestTime = requestContext.getProperty(REQUEST_TIME);
-        if(requestTime != null){
+        if (requestTime != null) {
             long now = System.currentTimeMillis();
             long start = (Long) requestTime;
-            long responseTime = now - start;
+            long executeTime = now - start;
 
-            MetricsUtil.registerHistogram(
-                               join(metricsName, 
METRICS_PATH_RESPONSE_TIME_HISTOGRAM))
-                       .update(responseTime);
+            MetricsUtil.registerHistogram(join(metricsName, 
METRICS_PATH_RESPONSE_TIME_HISTOGRAM))
+                       .update(executeTime);
 
             HugeConfig config = configProvider.get();
             long timeThreshold = 
config.get(ServerOptions.SLOW_QUERY_LOG_TIME_THRESHOLD);
-
-            // record slow query log
-            if (timeThreshold > 0 && isSlowQueryLogWhiteAPI(requestContext) && 
responseTime > timeThreshold) {
-                SlowQueryLog log = new SlowQueryLog(responseTime, start, 
(String) requestContext.getProperty(REQUEST_PARAMS_JSON),
-                                                    method, timeThreshold, 
path);
-                LOG.info("Slow query: {}", JsonUtil.toJson(log));
+            // Record slow query if meet needs, watch out the perf
+            if (timeThreshold > 0 && timeThreshold < executeTime &&

Review Comment:
   prefer to just update the small change in this PR so that we don’t have to 
keep track of it.
   `background`: in order to make it easier to read/understand, prefer to 
determine whether the executeTime exceeds a threshold.



##########
hugegraph-server/hugegraph-api/src/main/java/org/apache/hugegraph/metrics/SlowQueryLog.java:
##########
@@ -19,25 +19,36 @@
 
 public class SlowQueryLog {
 
-    public Long executeTime;
-
-    public Long startTime;
-
     public String rawQuery;
 
     public String method;
 
-    public Long threshold;
-
     public String path;
 
-    public SlowQueryLog(Long executeTime, Long startTime, String rawQuery, 
String method, Long threshold,
-                        String path) {
-        this.executeTime = executeTime;
-        this.startTime = startTime;
+    public long executeTime;
+
+    public long startTime;
+
+    public long threshold;
+
+    public SlowQueryLog(String rawQuery, String method, String path,
+                        long executeTime, long startTime, long threshold) {
         this.rawQuery = rawQuery;
         this.method = method;
-        this.threshold = threshold;
         this.path = path;
+        this.executeTime = executeTime;
+        this.startTime = startTime;
+        this.threshold = threshold;
+    }
+
+    @Override
+    public String toString() {
+        return "SlowQueryLog{executeTime=" + executeTime +
+               ", startTime=" + startTime +
+               ", rawQuery='" + rawQuery + '\'' +
+               ", method='" + method + '\'' +
+               ", threshold=" + threshold +

Review Comment:
   ditto



##########
hugegraph-server/hugegraph-api/src/main/java/org/apache/hugegraph/api/filter/AccessLogFilter.java:
##########
@@ -77,48 +95,28 @@ public void filter(ContainerRequestContext requestContext, 
ContainerResponseCont
             MetricsUtil.registerCounter(join(metricsName, 
METRICS_PATH_FAILED_COUNTER)).inc();
         }
 
-        // get responseTime
         Object requestTime = requestContext.getProperty(REQUEST_TIME);
-        if(requestTime != null){
+        if (requestTime != null) {
             long now = System.currentTimeMillis();
             long start = (Long) requestTime;
-            long responseTime = now - start;
+            long executeTime = now - start;
 
-            MetricsUtil.registerHistogram(
-                               join(metricsName, 
METRICS_PATH_RESPONSE_TIME_HISTOGRAM))
-                       .update(responseTime);
+            MetricsUtil.registerHistogram(join(metricsName, 
METRICS_PATH_RESPONSE_TIME_HISTOGRAM))
+                       .update(executeTime);
 
             HugeConfig config = configProvider.get();
             long timeThreshold = 
config.get(ServerOptions.SLOW_QUERY_LOG_TIME_THRESHOLD);
-
-            // record slow query log
-            if (timeThreshold > 0 && isSlowQueryLogWhiteAPI(requestContext) && 
responseTime > timeThreshold) {
-                SlowQueryLog log = new SlowQueryLog(responseTime, start, 
(String) requestContext.getProperty(REQUEST_PARAMS_JSON),
-                                                    method, timeThreshold, 
path);
-                LOG.info("Slow query: {}", JsonUtil.toJson(log));
+            // Record slow query if meet needs, watch out the perf
+            if (timeThreshold > 0 && timeThreshold < executeTime &&

Review Comment:
   prefer to just update the small change in this PR so that we don’t have to 
keep track of it.
   `background`: in order to make it easier to read/understand, prefer to 
determine whether the executeTime exceeds a threshold.



-- 
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