Support HttpServletRequest Parameters for Tracing

[cemo]

Please support trace HttpServletRequest parameters as well. org.springframework.boot.actuate.trace.WebRequestTraceFilter#getTrace is currently limited to method, parameters and headers.

[philwebb]

@dsyer is this by design? Something for 1.3 perhaps?

[dsyer]

It's by design really because parameters might be hard to render (a multipart request body shows up as parameters for instance).

[cemo]

This is a part of apache commons file upload:

          public static final boolean isMultipartContent(
                  HttpServletRequest request) {
              if (!"post".equals(request.getMethod().toLowerCase())) {
                  return false;
              }
              String contentType = request.getContentType();
              if (contentType == null) {
                  return false;
              }
              if (contentType.toLowerCase().startsWith(MULTIPART)) {
                  return true;
              }
              return false;
          }

Skipping multipart request body parameters is totally fine for me. I think that adding parameters would make tracing much more useful.

[cemo]

@philwebb @dsyer Would you accept a PR for this issue?

[izeye]

+1 for this feature.

[aames]

@philwebb @dsyer it's been a while since this enhancement request (and no changes have been made in the getTrace method).

Would you accept a PR from @cemo? Or do you prefer to retain the current functionality as-is?

[dsyer]

I'm just cautious because reading the parameters can make the body inaccessible (and obviously the body might be needed). Definitely open to pull requests, but it needs to be a thorough job.

[aames]

What about using a wrapper class to prevent that? Something like this: http://stackoverflow.com/questions/681263/modify-httpservletrequest-body/834883#834883

[cemo]

@bclozel @dsyer @aames

Spring has a nice class org.springframework.web.util.ContentCachingRequestWrapper. I have filed an issue to Spring for improve ContentCachingRequestWrapper to use for this issue as well. However its fixed version has changed last week. It was targeted to 4.2-RC1. @bclozel any chance to review and fix this issue as well?

[dsyer]

Brian explained quite nicely in that Spring issue why this is complex, and probably not advisable unless the user understands the risks and opts in (ie not really appropriate for the trace filter out of the box). If someone wants to propose a change we can still consider it, but like I said before, the bar has to be set quite high.

[cemo]

@wilkinsona @philwebb I am closing this issue. #3948 duplicates this and already resolved.