Logger / Interceptor

[ludoo0d0a]

Another required feature would be to trace/log/intercept requests. Such metricRegistry behaviour but a method retrieving headers and content (or just RequestEntity).

I think I can extend RequestDataExtractor, but not sure it is the better way. What do you think ?

[ludoo0d0a]

As I thought extractor is not the best place. In extractor, we can log separate things (body, headers, url) but not all in the same time.

I suggest to add an interceptor just after new RequestEntity in ReverseProxyFilter.doFilterInternal

RequestEntity<byte[]> requestEntity = new RequestEntity<>(body, headers, method, destination.getUri()); this.interceptor.trace(requestEntity , destination); //new trace

this.interceptor could be an optional bean like the others beans.

[mkopylec]

Logging request headers and body can lead to security issues, because headers and body can contain data such as passwords, keys, etc. You can't log such data :)

[ludoo0d0a]

You're right ! But I don't want to use it as front proxy, but like a "router" between business services only. (I tried zuul but live reloading mappings fails, and strongly relies on eureka and ribbon). The goal is to monitor these business services. Credentials are not a problem, because of b2b. And sensitive data, if there are, come from our internal services to another internal service.

So I hope you can consider anyway to add a such interceptor (with a void implementation by default).

I already succeed by extending ReverseProxyFilter and introduce logBefore, logSend, logReceive callbacks, but a proper implementation in the root project would be better ;) !

[mkopylec]

Ok, I will create such a feature, disabled by default so no one will accidently log any sensitive data. You will have to explicitly turn it on in application.yml file. Thanks for advice!

[ludoo0d0a]

Here is my version.

Feel free to be inspired :)

Notice addForwardHeaders is added after interceptor to avoid "fixed" headers.

`

protected void doFilterInternal(HttpServletRequest request, HttpServletResponse response, FilterChain filterChain) throws ServletException, IOException {

    String uri = extractor.extractUri(request);

    log.trace("Incoming: {} {}", request.getMethod(), uri);

    byte[] body = extractor.extractBody(request);
    HttpHeaders headers = extractor.extractHttpHeaders(request);
    HttpMethod method = extractor.extractHttpMethod(request);
    requestInterceptor.logBefore(uri, method, headers, body);

    addForwardHeaders(request, headers);

    ResponseEntity<byte[]> responseEntity = retryOperations.execute(context -> {
        ForwardDestination destination = resolveForwardDestination(uri);
        if (destination == null) {
            log.trace("Forwarding: {} {} -> no mapping found", request.getMethod(), uri);
            return null;
        }
        context.setAttribute(MAPPING_NAME_RETRY_ATTRIBUTE, destination.getMappingName());
        RequestEntity<byte[]> requestEntity = new RequestEntity<>(body, headers, method, destination.getUri());
        requestInterceptor.logSend(requestEntity, destination);
        ResponseEntity<byte[]> result = sendRequest(requestEntity, destination.getMappingMetricsName());

        requestInterceptor.logReceive(requestEntity, destination, result);

        log.debug("Forwarding: {} {} -> {} {}", request.getMethod(), uri, destination.getUri(), result.getStatusCode().value());

        return result;
    });
    if (responseEntity == null) {
        filterChain.doFilter(request, response);
        if (response.getStatus() == SC_NOT_FOUND) {
            updateMappingsIfAllowed();
        }
    } else {
        processResponse(response, responseEntity);
    }
}`

[mkopylec]

Great, I will consider your changes :)

[mkopylec]

I have created a prototype version of the logging feature. Here are some sample log outputs:

2016-07-05 22:21:50.674 TRACE 1280 --- [tp423629474-656] c.g.m.c.c.logging.ProxyingProcessLogger  : 
  Incoming HTTP request details:
    - method: GET
    - uri: /uri/1/path/1
    - body: 
    - headers: {User-Agent=[Java/1.8.0_77], Connection=[keep-alive], Host=[localhost:54614], Accept=[text/plain, application/json, application/*+json, */*], Content-Type=[text/plain;charset=ISO-8859-1]}

2016-07-05 22:21:50.712 DEBUG 1280 --- [tp423629474-656] c.g.m.c.c.logging.ProxyingProcessLogger  : 
  Forwarding process details:
    - mapping name: proxy 1
    - request method: GET
    - origin uri: /uri/1/path/1
    - forward destination: http://localhost:8081/path/1
    - forward request body: 
    - forward headers: {User-Agent=[Java/1.8.0_77], Connection=[keep-alive], Host=[localhost:54614], Accept=[text/plain, application/json, application/*+json, */*], Content-Type=[text/plain;charset=ISO-8859-1], X-Forwarded-For=[127.0.0.1], X-Forwarded-Proto=[http], X-Forwarded-Host=[localhost], X-Forwarded-Port=[54614]}
    - response status: 200
    - response body: Sample body
    - response headers: {Date=[Tue, 05 Jul 2016 20:21:50 GMT], Vary=[Accept-Encoding, User-Agent], Connection=[close], Server=[Jetty(9.2.13.v20150730)], Content-Length=[11]}

2016-07-05 22:21:51.188 TRACE 1280 --- [tp423629474-660] c.g.m.c.c.logging.ProxyingProcessLogger  : 
  Forwarding process details:
    - mapping name: <no mapping found for incoming HTTP request>
    - request method: GET
    - origin uri: /not/mapped/uri
    - request body: 
    - request headers: {User-Agent=[Java/1.8.0_77], Connection=[keep-alive], Host=[localhost:54614], Accept=[text/plain, application/json, application/*+json, */*], Content-Type=[text/plain;charset=ISO-8859-1], X-Forwarded-For=[127.0.0.1], X-Forwarded-Proto=[http], X-Forwarded-Host=[localhost], X-Forwarded-Port=[54614]}

Is that ok for you?

[ludoo0d0a]

Hi,great. you're so fast :) It seems very good. I just hope serializer can be changed (like you did with optional bean) so that serialization can be changed (save in DB, or other text format)

[mkopylec]

Done :) See https://github.com/mkopylec/charon-spring-boot-starter#tracing

[ludoo0d0a]

great, great, great :) !!!!!

Can I make 2 little remarks :

• performance around settings (on each request you check settings, don't you think to cache value in constructor would be better fro perfromance ?)
• error management around sendRequest would be great (and I'm currently dig into spring retry to notify/trace retries...) so something like that :

   try {
        result = sendRequest(requestEntity, destination.getMappingMetricsName());
        log.debug("Forwarding: {} {} -> {} {}", method, originUri, destination.getUri(), result.getStatusCode().value());
        traceInterceptor.onForwardComplete(response.getStatusCode(), response.getBody(), response.getHeaders()));
    } catch (Exception e) {
        log.error("Internal Network error on a singe retry", e);
        traceInterceptor.onForwardError(requestEntity, destination, e);
    }

[mkopylec]

Performance is not an issue here, the JIT will optimize the code well. I am not caching any value from @ConfigurationProperties because I want to make them changeable after application startup.

The second tip is a good tip :) I'll open another ticket for this.

[mkopylec]

https://github.com/mkopylec/charon-spring-boot-starter/issues/16