bitflicker64 opened a new issue, #3063:
URL: https://github.com/apache/hugegraph/issues/3063
## Source
- **File:**
`hugegraph-server/hugegraph-api/src/main/java/org/apache/hugegraph/api/filter/AccessLogFilter.java`
- **References:** L45-L46, L149-L150
---
I have enough information to answer the question. Let me analyze the
TODO/FIXME comment and explain the two issues it describes.
The TODO is at line 150 in `AccessLogFilter.java`:
```java
// TODO: set RequestBody null, handle it later & should record "client IP"
LOG.info("[Slow Query] execTime={}ms, body={}, method={}, path={}, query={}",
executeTime, null, method, path, uri.getQuery());
```
There are two distinct issues here:
1. **`body={}` is hardcoded as `null`**: The log statement passes `null` as
the `body` argument. The TODO says "set RequestBody null, handle it later" —
meaning the actual request body content is not being read and logged. For slow
query diagnostics, you'd want to know what query/body was sent. The challenge
is that `AccessLogFilter` is a `ContainerResponseFilter`, meaning it runs after
the request body has already been consumed by the resource method. Reading the
`InputStream` from `requestContext.getEntityStream()` at this point would
return an empty stream (already consumed). To fix this, a
`ContainerRequestFilter` would need to cache the body (e.g., store it as a
request property), and then `AccessLogFilter` could read it from the property.
2. **Client IP is not logged**: The log format string doesn't include a
client IP field at all. The TODO says "should record 'client IP'". Looking at
`AuthenticationFilter`, it uses `requestProvider.get().getRemoteAddr()` (a
Grizzly `Request`) to get the client IP. `AccessLogFilter` doesn't inject the
Grizzly `Request` provider, so it has no way to get the client IP currently.
**Files affected:**
- `AccessLogFilter.java` — the primary file needing changes (log format,
body reading, IP extraction)
- Potentially `PathFilter.java` — if the body needs to be cached in the
request filter phase (PathFilter is a `ContainerRequestFilter` that already
sets `REQUEST_TIME` as a property), it could also cache the body. Or a new
request filter could be added.
- Possibly a new filter class if body caching is done separately.
So the minimum files affected would be:
1. `AccessLogFilter.java` — add client IP to log, inject `Request` provider
2. `PathFilter.java` — optionally cache the request body as a property (or
this could be done in `AccessLogFilter` itself if it also implements
`ContainerRequestFilter`)
Actually, looking more carefully: `AccessLogFilter` is only a
`ContainerResponseFilter`. To read the body, it would need to either:
- Also implement `ContainerRequestFilter` (and cache the body in a request
property)
- Or rely on another filter (like `PathFilter`) to cache the body
The most likely minimal change is:
1. `AccessLogFilter.java` — add `ContainerRequestFilter` implementation to
cache body, add IP to log
2. That's potentially just 1 file if `AccessLogFilter` is extended to also
be a request filter
But realistically, since body caching requires wrapping the InputStream (to
allow re-reading), it might need a helper class too.
Let me give a clear, concise answer.
There are two distinct problems captured in that single TODO comment at line
150:
---
## The TODO in context
```java
// TODO: set RequestBody null, handle it later & should record "client IP"
LOG.info("[Slow Query] execTime={}ms, body={}, method={}, path={}, query={}",
executeTime, null, method, path, uri.getQuery());
```
---
## Problem 1: Request body is always `null`
The second argument to `LOG.info(...)` is hardcoded as `null`. The intent is
to log the actual request body (e.g., the Gremlin query string) for slow query
diagnostics, but it is deferred with "handle it later."
The root cause is architectural: `AccessLogFilter` implements only
`ContainerResponseFilter`, meaning it runs **after** the request body
`InputStream` has already been fully consumed by the resource method. Calling
`requestContext.getEntityStream()` at this point returns an empty stream.
To fix this, the body must be **cached during the request phase** — either
by:
- Making `AccessLogFilter` also implement `ContainerRequestFilter` and
wrapping/caching the `InputStream` into a request property, or
- Having `PathFilter` (which already runs as a `ContainerRequestFilter` and
sets `REQUEST_TIME`) also cache the body as a property.
`PathFilter` already sets a property constant `REQUEST_PARAMS_JSON` (line
53), which suggests this caching was planned but not implemented:
---
## Problem 2: Client IP is not logged at all
The log format string has no `ip` field. The TODO says it "should record
client IP," but `AccessLogFilter` has no mechanism to retrieve it — it doesn't
inject the Grizzly `Request` object.
Compare with `AuthenticationFilter`, which **does** inject the Grizzly
`Request` and reads the remote address:
```java
@Context
private jakarta.inject.Provider<Request> requestProvider;
...
peer = request.getRemoteAddr() + ":" + request.getRemotePort();
```
`AccessLogFilter` would need the same `@Context` injection of
`jakarta.inject.Provider<Request>` to extract `remoteAddr`.
---
## Files affected
The fix is self-contained to the filter package. Minimum files:
| File | Change needed |
|---|---|
| `AccessLogFilter.java` | Inject Grizzly `Request`, add IP to log format,
read cached body from request property |
| `PathFilter.java` | Cache the request body into a request property (using
`REQUEST_PARAMS_JSON`) during the request phase |
That's **2 files**. If body-caching logic is complex enough to warrant a
dedicated wrapper class (e.g., a `CachedBodyInputStream`), a 3rd new file could
be added, but it is not strictly required.
--
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]
