Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

SunnyBoy-WYH
Copy link
Contributor

@SunnyBoy-WYH SunnyBoy-WYH commented Mar 1, 2024

fix #2468

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:

  1. we need get post body from req, and we need set it back to request, so it changed.
  2. 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);

`

@dosubot 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
Copy link

codecov bot commented Mar 1, 2024

Codecov Report

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

Project coverage is 66.25%. Comparing base (47a68f0) to head (ee09bd1).
Report is 2 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     63.80%   66.25%   +2.44%     
  Complexity      829      829              
============================================
  Files           511      511              
  Lines         42622    42643      +21     
  Branches       5947     5949       +2     
============================================
+ Hits          27193    28251    +1058     
+ Misses        12679    11579    -1100     
- Partials       2750     2813      +63     

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


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

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

bufferedStream.mark(Integer.MAX_VALUE);

context.setProperty(REQUEST_PARAMS_JSON,
IOUtils.toString(bufferedStream, Charsets.toCharset(CHARSET)));
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

maybe we can cut the special length? like 512?

.getPathParameters();
requestParamsJson = pathParameters.toString();
context.setProperty(REQUEST_PARAMS_JSON,
context.getUriInfo().getPathParameters().toString());
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can we also add all other branchs like PUT/DELETE

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

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
api Changes of API bug Something isn't working size:M This PR changes 30-99 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Bug] server slow log, support loader import & client IP
2 participants