Spring/spring-framework
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

Stop observations for async requests in Servlet filter

Browse Source 
Prior to this commit, the `ServerHttpObservationFilter` would support
async dispatches and would do the following:

1. start the observation
2. call the filter chain
3. if async has started, do nothing
4. if not in async mode, stop the observation

This behavior would effectively rely on Async implementations to
complete and dispatch the request back to the container for an async
dispatch. This is what Spring web frameworks do and guarantee.

Some implementations complete the async request but do not dispatch
back; as a result, observations could leak as they are never stopped.

This commit changes the support of async requests. The filter now
opts-out of async dispatches - the filter will not be called for those
anymore. Instead, if the application started async mode during the
initial container dispatch, the filter will register an AsyncListener to
be notified of the outcome of the async handling.

Fixes gh-32730
This commit is contained in:
Brian Clozel
2024-06-07 18:46:59 +02:00
parent 172987c874
commit 6681394886
2 changed files with 58 additions and 11 deletions
Download Patch File Download Diff File Expand all files Collapse all files

17
spring-web/src/test/java/org/springframework/web/filter/ServerHttpObservationFilterTests.java
View File

@@ -50,6 +50,11 @@ class ServerHttpObservationFilterTests {
private final MockHttpServletResponse response = new MockHttpServletResponse();
@Test
void filterShouldNotProcessAsyncDispatch() {
assertThat(this.filter.shouldNotFilterAsyncDispatch()).isTrue();
}
@Test
void filterShouldFillObservationContext() throws Exception {
this.filter.doFilter(this.request, this.response, this.mockFilterChain);
@@ -60,7 +65,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
@@ -109,6 +114,16 @@ class ServerHttpObservationFilterTests {
.hasLowCardinalityKeyValue("status", "500");
}
@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();