fix(server): fix server slow log, support loader import & client IP #2466

SunnyBoy-WYH · 2024-03-01T13:24:06Z

we support slow log before ,but it cause bug when loader batch import data, the feat PR see #2327

and later we downgrade it ,see pr : #2347

the bug due to:

we need get post body from req, and we need set it back to request, so it changed.
the loader request use the "GZIP" header, after get post body, server cant read it

so we ready to resolve it , use BufferedInputStream to cache the stream:

` 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);

`
case：
[Slow Query] ip=127.0.0.1 execTime=22ms, body={"gremlin":"hugegraph.backendStoreFeatures() .supportsSharedStorage();","bindings":{},"language":"gremlin-groovy","aliases":{"g":"__g_hugegraph"}}, method=POST, path=/gremlin, query=null

and then i test it

codecov · 2024-03-01T13:32:18Z

Codecov Report

Attention: Patch coverage is 92.30769% with 2 lines in your changes missing coverage. Please review.

Project coverage is 49.91%. Comparing base (506850c) to head (071943c).
Report is 1 commits behind head on master.

Files	Patch %	Lines
...g/apache/hugegraph/api/filter/AccessLogFilter.java	77.77%	2 Missing ⚠️

Additional details and impacted files

@@             Coverage Diff              @@
##             master    #2466      +/-   ##
============================================
- Coverage     56.94%   49.91%   -7.03%     
+ Complexity      827      741      -86     
============================================
  Files           612      612              
  Lines         49672    49697      +25     
  Branches       6681     6685       +4     
============================================
- Hits          28284    24806    -3478     
- Misses        18572    22317    +3745     
+ Partials       2816     2574     -242

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

javeme · 2024-03-03T04:24:29Z

hugegraph-server/hugegraph-api/src/main/java/org/apache/hugegraph/api/filter/PathFilter.java


-        // TODO: temporarily comment it to fix loader bug, handle it later
-        /*// record the request json
+    private void recordRequestJson(ContainerRequestContext context) throws IOException {


prefer collectRequestParams(), seems 'record' means output to log

javeme · 2024-03-03T04:31:29Z

hugegraph-server/hugegraph-api/src/main/java/org/apache/hugegraph/api/filter/PathFilter.java

+            bufferedStream.mark(Integer.MAX_VALUE);
+
+            context.setProperty(REQUEST_PARAMS_JSON,
+                                IOUtils.toString(bufferedStream, Charsets.toCharset(CHARSET)));


maybe it's very large for batch-write, can we cut part of the content?

maybe we can cut the special length? like 512?

or we will make it can be config? like 512/1024/2048 and default will be 512? @imbajin @liuxiaocs7 @VGalaxies

javeme · 2024-03-03T04:31:55Z

hugegraph-server/hugegraph-api/src/main/java/org/apache/hugegraph/api/filter/PathFilter.java

-                                                                   .getPathParameters();
-            requestParamsJson = pathParameters.toString();
+            context.setProperty(REQUEST_PARAMS_JSON,
+                                context.getUriInfo().getPathParameters().toString());


can we also add all other branchs like PUT/DELETE

javeme · 2024-03-03T04:35:22Z

...raph-server/hugegraph-api/src/main/java/org/apache/hugegraph/api/filter/AccessLogFilter.java

+                LOG.info("[Slow Query] ip={} execTime={}ms, body={}, method={}, path={}, query={}",
+                         getClientIP(requestContext), executeTime,
+                         requestContext.getProperty(REQUEST_PARAMS_JSON), method, path,
+                         uri.getQuery());


are they repeated when GET: uri.getQuery() and REQUEST_PARAMS_JSON

for GET ， REQUEST_PARAMS_JSON will extract the id info from /example/{id} ; uri.getQuery() will extract id=1 from http://example.com/resource?id=1

javeme · 2024-08-10T11:33:38Z

...raph-server/hugegraph-api/src/main/java/org/apache/hugegraph/api/filter/AccessLogFilter.java

-                         executeTime, null, method, path, uri.getQuery());
+
+                LOG.info("[Slow Query] ip={} execTime={}ms, body={}, method={}, path={}, query={}",
+                         getClientIP(requestContext), executeTime,


prefer this style: defining a local var for better readability:
for both: getClientIP and REQUEST_PARAMS_JSON.

javeme · 2024-08-10T11:35:56Z

...raph-server/hugegraph-api/src/main/java/org/apache/hugegraph/api/filter/AccessLogFilter.java

+    private String getClientIP(ContainerRequestContext requestContext) {
+        try {
+            UriInfo uriInfo = requestContext.getUriInfo();
+            String host = uriInfo.getRequestUri().getHost();


return here if host is an ip or empty?

javeme · 2024-08-10T11:35:58Z

...raph-server/hugegraph-api/src/main/java/org/apache/hugegraph/api/filter/AccessLogFilter.java

+            String host = uriInfo.getRequestUri().getHost();
+            return InetAddress.getByName(host).getHostAddress();
+        } catch (UnknownHostException e) {
+            return "unknown";


prefer "<unknown_ip>"

javeme · 2024-08-10T11:36:59Z

hugegraph-server/hugegraph-api/src/main/java/org/apache/hugegraph/api/filter/PathFilter.java

-                                                                   .getPathParameters();
-            requestParamsJson = pathParameters.toString();
+            context.setProperty(REQUEST_PARAMS_JSON,
+                                context.getUriInfo().getPathParameters().toString());


can we also add all other branchs like PUT/DELETE -- seems still missing

fix(server): fix server slow log, support loader import & client IP

ee09bd1

dosubot bot added size:M This PR changes 30-99 lines, ignoring generated files. api Changes of API bug Something isn't working labels Mar 1, 2024

javeme reviewed Mar 3, 2024

View reviewed changes

SunnyBoy-WYH added 2 commits June 15, 2024 11:24

Merge remote-tracking branch 'origin/master' into slow-log-fix

868adcf

fix(server): support put/delete & when 512 length then log cut

071943c

imbajin requested review from javeme and imbajin June 24, 2024 16:04

javeme reviewed Aug 10, 2024

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix(server): fix server slow log, support loader import & client IP #2466

fix(server): fix server slow log, support loader import & client IP #2466

SunnyBoy-WYH commented Mar 1, 2024 •

edited

Loading

codecov bot commented Mar 1, 2024 •

edited

Loading

javeme Mar 3, 2024

javeme Mar 3, 2024

SunnyBoy-WYH Mar 3, 2024

SunnyBoy-WYH Jun 15, 2024

javeme Mar 3, 2024

javeme Mar 3, 2024

SunnyBoy-WYH Jun 15, 2024

javeme Aug 10, 2024

javeme Aug 10, 2024

javeme Aug 10, 2024

javeme Aug 10, 2024

fix(server): fix server slow log, support loader import & client IP #2466

Are you sure you want to change the base?

fix(server): fix server slow log, support loader import & client IP #2466

Conversation

SunnyBoy-WYH commented Mar 1, 2024 • edited Loading

codecov bot commented Mar 1, 2024 • edited Loading

Codecov Report

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

SunnyBoy-WYH commented Mar 1, 2024 •

edited

Loading

codecov bot commented Mar 1, 2024 •

edited

Loading