getsentry / sentry-java

A Sentry SDK for Java, Android and other JVM languages.
https://docs.sentry.io/
MIT License
1.14k stars 432 forks source link

SentryServletRequestListener and Async requests with Tomcat #3568

Open jasonculverhouse opened 1 month ago

jasonculverhouse commented 1 month ago

Integration

sentry-servlet

Java Version

17

Version

7.11.0

Steps to Reproduce

When using "async" servlet requests that use the SentryServletRequestListener. I see the "http" bread crumbs from other requests.

The same thread is not used for the push and pop events for an async request.

It would take me some time but I could Probably write an example.

Note that this pattern works fine if I don't call as everything stays in the same thread ServletRequest.startAsync()

I don't see any example using the javax.servlet.AsyncContext

public class SentryServletRequestListener implements ServletRequestListener {

  @Override
  public void requestDestroyed(@NotNull ServletRequestEvent servletRequestEvent) {
    hub.popScope();
  }

  @Override
  public void requestInitialized(@NotNull ServletRequestEvent servletRequestEvent) {
    hub.pushScope();
   ...
      hub.addBreadcrumb(
          Breadcrumb.http(httpRequest.getRequestURI(), httpRequest.getMethod()), hint);

Expected Result

Bread Crumbs don't leak and build up over time.

Actual Result

For an app that does not use async context

Sentry objects are ~ number of threads in the application.

 141:           799         134232  io.sentry.Scope
 178:           799          83096  io.sentry.protocol.Contexts
 201:           797          57384  io.sentry.Hub
 234:           799          44744  io.sentry.PropagationContext
 286:           799          31960  io.sentry.CircularFifoQueue
 287:           799          31960  io.sentry.Stack$StackItem
 289:           798          31920  io.sentry.Baggage
 322:           799          25568  io.sentry.SynchronizedQueue
 325:           797          25504  io.sentry.Stack
 326:           797          25504  io.sentry.TracesSampler

If I use an async method Breadcrumb start to accumulate mostly with the

  65:         21468        1545696  io.sentry.Breadcrumb
 109:          2714         455952  io.sentry.Scope
 135:          2714         282256  io.sentry.protocol.Contexts
 171:          2713         151928  io.sentry.PropagationContext
 194:          2714         108560  io.sentry.CircularFifoQueue
 195:          2714         108560  io.sentry.Stack$StackItem
 197:          2711         108440  io.sentry.Baggage
 232:          2714          86848  io.sentry.SynchronizedQueue
 266:           893          64296  io.sentry.Hub
 349:          1495          35880  io.sentry.servlet.SentryRequestHttpServletRequestProcessor

The http messages/bread crumbs that accumulate are always the methods that are async.

image

lbloder commented 1 month ago

Hi @jasonculverhouse, Thanks for reaching out!

Unfortunately you are correct, Async servlets are currently not supported in Sentry. We will discuss internally and let you know!

In the meantime, I'm looking at a workaround to provide to you, but it needs some testing. Again, as soon as I have something, I will let you know.

lbloder commented 2 weeks ago

Hi @jasonculverhouse, Sorry for the delay, but I haven't found a viable workaround yet. Just wanted to let you know, that we are still looking into this and haven't forgotten. We'll bump the prio on this and I'll hopefully be able to alot enough time the week after next to get this resolved.