HttpInterface API for providing the request body

[Ramblurr]

This is related to #223 and the log4j library.

We've implemented our own caching of the request body and want to include it in Sentry events, however the HttpInterface doesn't even have a "data" field. This means we need to copy/paste and re-implemnent the entire stack from HttpInterface, HttpInterfaceBinding, and HttpEventBuilderHelper, DefaultRavenFactory#createMarshaller(Dsn dsn)

That is an onerous amount of work just to get the request payload to show up in the "BODY" view of the sentry UI.

An easier, but not as nice solution is to add it as the logged message it self, but that requires special handling on all log.error() calls.

1. Can the 'body' field be added to the HttpInterface even if the default implementation does nothing?
2. Can some API be added to inject the payload body?

Regarding 2, there are several possibilities. One is a static method on RavenServletRequestListener that lets the caller inject the payload. Another is that the "requestBody" (or some such key) on the MDC is special cased.

[tkaemming]

We should definitely add it to the HttpInterface and binding. I'm guessing this was an oversight on our part since we don't actually include it automatically anywhere.

(I haven't put much thought into the second question yet.)

[bretthoerner]

I think for (2) we should do whatever we end up doing for User and other Context related stuff, like: #303

Which ends up being close to the MDC answer except it'll be type safe and hopefully allow this to even be used in async frameworks like Netty (if we do it right).

[bretthoerner]

A String body field was added in 7.8.5. This doesn't automate anything, but it should reduce the amount of customization required for now: 10ac4a5

[ben-manes]

This was a bit ugly to implement on the client-side. Here's the steps that I took, so that hopefully customizing the client can be made to feel more natural.

1. Added a Servlet filter to cache the request body in a thread local, if an acceptable content-type and content-length.
   1. Servlet 3.0 makes is fairly complex if done right (never is) due to additional APIs to be proxied in ServletInputStream. It felt error prone to keep it simple and incorrect, or unnecessarily complex. I ended up using a JAX-RS ContainerRequestFilter & ContainerResponseFilter instead to change the entity's InputStream. This is cleaner code, but less generic if non-JAXRS routes are added.
2. Added a customized DefaultSentryClientFactory to override createSentryClient(dsn). This calls the super method and then replaces the existing HttpEventBuilderHelper with my own.
3. Forked HttpEventBuilderHelper to add the body, since addHttpInterface is private. Switched to ForwardedAddressResolver while in the code.

Suggestions:

• It would be helpful if guidance on the body's length was provided in case the event is rejected as too large.
  • In this respect, some fields could be dropped based on priority. Users may prefer receiving the alert with less information than be entirely ignorant. It would be useful to have a hook that to allows for slimming the event down and retrying on a limit error (or a more generalized handler).
• It would be nice if the APIs were designed to facilitate customizations outside of inheritance and forking. The latter being troublesome as the code will evolve independently, though inheritance isn't wonderful either. Customization feels like an after thought that could be given more attention.
• Provide a customizable Servlet filter for caching the body, taking care to implement it correctly to the spec. Then users can choose to add it with all the caveats understood.

[bretthoerner]

@ben-manes thanks the writeup. I agree with all the suggestions, will try to get time for this.

[marandaneto]

Sentry-Java SDK v3has the Request field for that