javeme commented on code in PR #2327:
URL:
https://github.com/apache/incubator-hugegraph/pull/2327#discussion_r1365800116
##########
hugegraph-api/src/main/java/org/apache/hugegraph/api/filter/AccessLogFilter.java:
##########
@@ -62,13 +79,25 @@ public void filter(ContainerRequestContext requestContext,
ContainerResponseCont
// get responseTime
Object requestTime = requestContext.getProperty(REQUEST_TIME);
- if(requestTime!=null){
+ if(requestTime != null){
Review Comment:
add a var `long start = (Long) requestTime`?
##########
hugegraph-api/src/main/java/org/apache/hugegraph/config/ServerOptions.java:
##########
@@ -272,4 +272,20 @@ public static synchronized ServerOptions instance() {
disallowEmpty(),
"disable"
);
+
+ public static final ConfigOption<Long> SLOW_QUERY_LOG_TIME_THRESHOLD =
+ new ConfigOption<>(
+ "log.slow_query_threshold",
+ "The slow query log time threshold(ms) of rest server.",
+ disallowEmpty(),
Review Comment:
prefer nonNeg
##########
hugegraph-api/src/main/java/org/apache/hugegraph/api/filter/AccessLogFilter.java:
##########
@@ -62,13 +79,25 @@ public void filter(ContainerRequestContext requestContext,
ContainerResponseCont
// get responseTime
Object requestTime = requestContext.getProperty(REQUEST_TIME);
- if(requestTime!=null){
+ if(requestTime != null){
long now = System.currentTimeMillis();
long responseTime = (now - (long)requestTime);
MetricsUtil.registerHistogram(
join(metricsName,
METRICS_PATH_RESPONSE_TIME_HISTOGRAM))
.update(responseTime);
+
+ HugeConfig config = configProvider.get();
+ Boolean enableSlowQueryLog =
config.get(ServerOptions.ENABLE_SLOW_QUERY_LOG);
+ Long timeThreshold =
config.get(ServerOptions.SLOW_QUERY_LOG_TIME_THRESHOLD);
Review Comment:
can we let `timeThreshold > 0` mean enableSlowQueryLog?
##########
hugegraph-api/src/main/java/org/apache/hugegraph/config/ServerOptions.java:
##########
@@ -272,4 +272,20 @@ public static synchronized ServerOptions instance() {
disallowEmpty(),
"disable"
);
+
+ public static final ConfigOption<Long> SLOW_QUERY_LOG_TIME_THRESHOLD =
+ new ConfigOption<>(
+ "log.slow_query_threshold",
+ "The slow query log time threshold(ms) of rest server.",
Review Comment:
prefer `The threshold time(ms) of logging slow query.`
##########
hugegraph-api/src/main/java/org/apache/hugegraph/api/filter/AccessLogFilter.java:
##########
@@ -62,13 +79,25 @@ public void filter(ContainerRequestContext requestContext,
ContainerResponseCont
// get responseTime
Object requestTime = requestContext.getProperty(REQUEST_TIME);
- if(requestTime!=null){
+ if(requestTime != null){
long now = System.currentTimeMillis();
long responseTime = (now - (long)requestTime);
MetricsUtil.registerHistogram(
join(metricsName,
METRICS_PATH_RESPONSE_TIME_HISTOGRAM))
.update(responseTime);
+
+ HugeConfig config = configProvider.get();
+ Boolean enableSlowQueryLog =
config.get(ServerOptions.ENABLE_SLOW_QUERY_LOG);
+ Long timeThreshold =
config.get(ServerOptions.SLOW_QUERY_LOG_TIME_THRESHOLD);
+
+ // record slow query log
+ if (enableSlowQueryLog && isSlowQueryLogWhiteAPI(requestContext) &&
+ timeThreshold < responseTime) {
Review Comment:
prefer `responseTime > timeThreshold`
##########
hugegraph-api/src/main/java/org/apache/hugegraph/api/filter/AccessLogFilter.java:
##########
@@ -62,13 +79,25 @@ public void filter(ContainerRequestContext requestContext,
ContainerResponseCont
// get responseTime
Object requestTime = requestContext.getProperty(REQUEST_TIME);
- if(requestTime!=null){
+ if(requestTime != null){
long now = System.currentTimeMillis();
long responseTime = (now - (long)requestTime);
MetricsUtil.registerHistogram(
join(metricsName,
METRICS_PATH_RESPONSE_TIME_HISTOGRAM))
.update(responseTime);
+
+ HugeConfig config = configProvider.get();
+ Boolean enableSlowQueryLog =
config.get(ServerOptions.ENABLE_SLOW_QUERY_LOG);
+ Long timeThreshold =
config.get(ServerOptions.SLOW_QUERY_LOG_TIME_THRESHOLD);
+
+ // record slow query log
+ if (enableSlowQueryLog && isSlowQueryLogWhiteAPI(requestContext) &&
+ timeThreshold < responseTime) {
+ SlowQueryLog log = new SlowQueryLog(responseTime, (Long)
requestTime,
+ (String)
requestContext.getProperty(REQUEST_PARAMS_JSON), method, timeThreshold, path);
Review Comment:
wrap line before "method,"
##########
hugegraph-api/src/main/java/org/apache/hugegraph/api/filter/AccessLogFilter.java:
##########
@@ -62,13 +79,25 @@ public void filter(ContainerRequestContext requestContext,
ContainerResponseCont
// get responseTime
Object requestTime = requestContext.getProperty(REQUEST_TIME);
- if(requestTime!=null){
+ if(requestTime != null){
long now = System.currentTimeMillis();
long responseTime = (now - (long)requestTime);
MetricsUtil.registerHistogram(
join(metricsName,
METRICS_PATH_RESPONSE_TIME_HISTOGRAM))
.update(responseTime);
+
+ HugeConfig config = configProvider.get();
+ Boolean enableSlowQueryLog =
config.get(ServerOptions.ENABLE_SLOW_QUERY_LOG);
+ Long timeThreshold =
config.get(ServerOptions.SLOW_QUERY_LOG_TIME_THRESHOLD);
+
+ // record slow query log
+ if (enableSlowQueryLog && isSlowQueryLogWhiteAPI(requestContext) &&
+ timeThreshold < responseTime) {
+ SlowQueryLog log = new SlowQueryLog(responseTime, (Long)
requestTime,
+ (String)
requestContext.getProperty(REQUEST_PARAMS_JSON), method, timeThreshold, path);
+ LOG.info("slow query log: {}", JsonUtil.toJson(log));
Review Comment:
"Slow query: {}"
--
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]
