diff --git a/spring-web/src/main/java/org/springframework/web/filter/ServerHttpObservationFilter.java b/spring-web/src/main/java/org/springframework/web/filter/ServerHttpObservationFilter.java index 179b4ffcdd6..1c56741a4ba 100644 --- a/spring-web/src/main/java/org/springframework/web/filter/ServerHttpObservationFilter.java +++ b/spring-web/src/main/java/org/springframework/web/filter/ServerHttpObservationFilter.java @@ -21,9 +21,12 @@ import java.util.Optional; import io.micrometer.observation.Observation; import io.micrometer.observation.ObservationRegistry; +import jakarta.servlet.AsyncEvent; +import jakarta.servlet.AsyncListener; import jakarta.servlet.FilterChain; import jakarta.servlet.RequestDispatcher; import jakarta.servlet.ServletException; +import jakarta.servlet.ServletRequest; import jakarta.servlet.http.HttpServletRequest; import jakarta.servlet.http.HttpServletResponse; @@ -94,11 +97,6 @@ public class ServerHttpObservationFilter extends OncePerRequestFilter { return Optional.ofNullable((ServerRequestObservationContext) request.getAttribute(CURRENT_OBSERVATION_CONTEXT_ATTRIBUTE)); } - @Override - protected boolean shouldNotFilterAsyncDispatch() { - return false; - } - @Override @SuppressWarnings("try") protected void doFilterInternal(HttpServletRequest request, HttpServletResponse response, FilterChain filterChain) @@ -115,8 +113,12 @@ public class ServerHttpObservationFilter extends OncePerRequestFilter { throw ex; } finally { - // Only stop Observation if async processing is done or has never been started. - if (!request.isAsyncStarted()) { + // If async is started, register a listener for completion notification. + if (request.isAsyncStarted()) { + request.getAsyncContext().addListener(new ObservationAsyncListener(observation)); + } + // Stop Observation right now if async processing has not been started. + else { Throwable error = fetchException(request); if (error != null) { observation.error(error); @@ -152,13 +154,43 @@ public class ServerHttpObservationFilter extends OncePerRequestFilter { } @Nullable - private Throwable unwrapServletException(Throwable ex) { + static Throwable unwrapServletException(Throwable ex) { return (ex instanceof ServletException) ? ex.getCause() : ex; } @Nullable - private Throwable fetchException(HttpServletRequest request) { + static Throwable fetchException(ServletRequest request) { return (Throwable) request.getAttribute(RequestDispatcher.ERROR_EXCEPTION); } + private static class ObservationAsyncListener implements AsyncListener { + + private final Observation currentObservation; + + public ObservationAsyncListener(Observation currentObservation) { + this.currentObservation = currentObservation; + } + + @Override + public void onStartAsync(AsyncEvent event) { + } + + @Override + public void onTimeout(AsyncEvent event) { + this.currentObservation.stop(); + } + + @Override + public void onComplete(AsyncEvent event) { + this.currentObservation.stop(); + } + + @Override + public void onError(AsyncEvent event) { + this.currentObservation.error(unwrapServletException(event.getThrowable())); + this.currentObservation.stop(); + } + + } + } diff --git a/spring-web/src/test/java/org/springframework/web/filter/ServerHttpObservationFilterTests.java b/spring-web/src/test/java/org/springframework/web/filter/ServerHttpObservationFilterTests.java index 2fcdf919199..7bca8a323fa 100644 --- a/spring-web/src/test/java/org/springframework/web/filter/ServerHttpObservationFilterTests.java +++ b/spring-web/src/test/java/org/springframework/web/filter/ServerHttpObservationFilterTests.java @@ -54,6 +54,11 @@ class ServerHttpObservationFilterTests { private ServerHttpObservationFilter filter = new ServerHttpObservationFilter(this.observationRegistry); + @Test + void filterShouldNotProcessAsyncDispatch() { + assertThat(this.filter.shouldNotFilterAsyncDispatch()).isTrue(); + } + @Test void filterShouldFillObservationContext() throws Exception { this.filter.doFilter(this.request, this.response, this.mockFilterChain); @@ -64,7 +69,7 @@ class ServerHttpObservationFilterTests { assertThat(context.getCarrier()).isEqualTo(this.request); assertThat(context.getResponse()).isEqualTo(this.response); assertThat(context.getPathPattern()).isNull(); - assertThatHttpObservation().hasLowCardinalityKeyValue("outcome", "SUCCESS"); + assertThatHttpObservation().hasLowCardinalityKeyValue("outcome", "SUCCESS").hasBeenStopped(); } @Test @@ -121,6 +126,16 @@ class ServerHttpObservationFilterTests { assertThat(this.response.getHeader("X-Trace-Id")).isEqualTo("badc0ff33"); } + @Test + void shouldCloseObservationAfterAsyncCompletion() throws Exception { + this.request.setAsyncSupported(true); + this.request.startAsync(); + this.filter.doFilter(this.request, this.response, this.mockFilterChain); + this.request.getAsyncContext().complete(); + + assertThatHttpObservation().hasLowCardinalityKeyValue("outcome", "SUCCESS").hasBeenStopped(); + } + private TestObservationRegistryAssert.TestObservationRegistryAssertReturningObservationContextAssert assertThatHttpObservation() { return TestObservationRegistryAssert.assertThat(this.observationRegistry) .hasObservationWithNameEqualTo("http.server.requests").that();