Add request body extraction for Spring MVC integration

[maciejwalkowiak]

📜 Description

Add request body extraction for Spring MVC integration

💡 Motivation and Context

Fixes #1334

💚 How did you test it?

Unit & integration tests, sample projects updated.

📝 Checklist

• I reviewed the submitted code
• I added tests to verify the changes
• I updated the docs if needed
• No breaking changes

[codecov-commenter]

Codecov Report

Merging #1595 (989a208) into main (5ab8e8d) will decrease coverage by 0.17%.
The diff coverage is 67.00%.

@@             Coverage Diff              @@
##               main    #1595      +/-   ##
============================================
- Coverage     76.07%   75.90%   -0.18%     
- Complexity     1976     2000      +24     
============================================
  Files           198      202       +4     
  Lines          6842     6938      +96     
  Branches        680      689       +9     
============================================
+ Hits           5205     5266      +61     
- Misses         1305     1334      +29     
- Partials        332      338       +6     

Impacted Files	Coverage Δ
.../io/sentry/spring/SentrySpringRequestListener.java	80.95% <ø> (-14.29%)	⬇️
...pring/SentrySpringServletContainerInitializer.java	0.00% <0.00%> (ø)
...java/io/sentry/spring/RequestPayloadExtractor.java	37.50% <37.50%> (ø)
...io/sentry/spring/CachedBodyServletInputStream.java	40.00% <40.00%> (ø)
...main/java/io/sentry/spring/SentryHubRegistrar.java	93.02% <66.66%> (-1.98%)	⬇️
...main/java/io/sentry/spring/SentrySpringFilter.java	69.23% <69.23%> (ø)
...io/sentry/spring/CachedBodyHttpServletRequest.java	80.00% <80.00%> (ø)
...io/sentry/spring/boot/SentryAutoConfiguration.java	98.27% <100.00%> (+0.06%)	⬆️
sentry/src/main/java/io/sentry/SentryOptions.java	85.21% <100.00%> (+0.57%)	⬆️
... and 4 more

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 5ab8e8d...989a208. Read the comment docs.

[bruno-garcia]

Looks good. I had a couple comments we could discuss on a call if best.
It's rather late and I'm tired so I might be missing something

[bruno-garcia]

Might need to confirm if this looks like it's an incoming request. I believe this "format" of crumb might have been designed to represent an outgoing HTTP request.
If it won't get confusing with crumbs representing requests done from this process out to the another for example through rest-template, then we're good.

[maciejwalkowiak]

That's how it's been since 3.0 release. Where can I verify if this is the right format?

[bruno-garcia]

If we have an incoming request into a spring process that makes an outgoing request. Would we be able to differentiate the crumbs? Probably at least through the URL.
Doesn't block this PR

[marandaneto]

deferring the review to @bruno-garcia as there's already the 1st pass.
The sentry module is barely untouched so all good for Android.
It'd be nice to bring this into the OkHttp integration too, another PR of course.

[maciejwalkowiak]

@marandaneto how this would work for OkHttp or other http clients? On the server side, request body is added to events that are triggered within the scope of an HTTP request. I am not sure how would this translate to HTTP clients, as for the clients we just create a breadcrumb and a span.

[bruno-garcia]

NIce! thanks @maciejwalkowiak

[bruno-garcia]

If we have an incoming request into a spring process that makes an outgoing request. Would we be able to differentiate the crumbs? Probably at least through the URL.
Doesn't block this PR

[marandaneto]

@marandaneto how this would work for OkHttp or other http clients? On the server side, request body is added to events that are triggered within the scope of an HTTP request. I am not sure how would this translate to HTTP clients, as for the clients we just create a breadcrumb and a span.

oh yeah indeed, the OkHttp integration still doesn't raise events if the request failed somehow, so yeah it does not make sense right now.
See the Dart integration https://github.com/getsentry/sentry-dart/blob/main/dart/lib/src/http_client/failed_request_client.dart#L187-L207
it captures failed requests if enabled, and also respects the MaxRequestBodySize if enabled too, the idea would be to bring this feature into the Java SDK too, but we're waiting for this feature to mature since we've just released it and gathering feedback about the usage.