Skip to content

Commit debba65

Browse files
committed
Listen to multiple async operations in ServerHttpObservationFilter
Prior to this commit, the `ServerHttpObservationFilter` was fixed to re-enable instrumentation for async dispatches. This fix involves using an AsyncListener to be notified of exchange completion. This change was incomplete, as this would not work in some cases. If another filter starts the async mode and initiates an ASYNC dispatch, before async handling at the controller level, the async listener is not registered against subsequent async starts. This commit not only ensures that the async listener registers against new async starts, but also ensure that the initial creation and registration only happens during the initial REQUEST dispatch. Fixes gh-33451
1 parent fab8890 commit debba65

File tree

2 files changed

+46
-3
lines changed

2 files changed

+46
-3
lines changed

spring-web/src/main/java/org/springframework/web/filter/ServerHttpObservationFilter.java

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -118,13 +118,13 @@ protected void doFilterInternal(HttpServletRequest request, HttpServletResponse
118118
throw ex;
119119
}
120120
finally {
121-
// If async is started, register a listener for completion notification.
122-
if (request.isAsyncStarted()) {
121+
// If async is started during the first dispatch, register a listener for completion notification.
122+
if (request.isAsyncStarted() && request.getDispatcherType() == DispatcherType.REQUEST) {
123123
request.getAsyncContext().addListener(new ObservationAsyncListener(observation));
124124
}
125125
// scope is opened for ASYNC dispatches, but the observation will be closed
126126
// by the async listener.
127-
else if (request.getDispatcherType() != DispatcherType.ASYNC){
127+
else if (!isAsyncDispatch(request)) {
128128
Throwable error = fetchException(request);
129129
if (error != null) {
130130
observation.error(error);
@@ -168,6 +168,7 @@ public ObservationAsyncListener(Observation currentObservation) {
168168

169169
@Override
170170
public void onStartAsync(AsyncEvent event) {
171+
event.getAsyncContext().addListener(this);
171172
}
172173

173174
@Override

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

Lines changed: 42 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -21,11 +21,16 @@
2121
import io.micrometer.observation.ObservationRegistry;
2222
import io.micrometer.observation.tck.TestObservationRegistry;
2323
import io.micrometer.observation.tck.TestObservationRegistryAssert;
24+
import jakarta.servlet.AsyncContext;
2425
import jakarta.servlet.AsyncEvent;
2526
import jakarta.servlet.AsyncListener;
2627
import jakarta.servlet.DispatcherType;
28+
import jakarta.servlet.Filter;
29+
import jakarta.servlet.FilterChain;
2730
import jakarta.servlet.RequestDispatcher;
2831
import jakarta.servlet.ServletException;
32+
import jakarta.servlet.ServletRequest;
33+
import jakarta.servlet.ServletResponse;
2934
import jakarta.servlet.http.HttpServlet;
3035
import jakarta.servlet.http.HttpServletRequest;
3136
import jakarta.servlet.http.HttpServletResponse;
@@ -139,6 +144,21 @@ void shouldCloseObservationAfterAsyncCompletion() throws Exception {
139144
assertThatHttpObservation().hasLowCardinalityKeyValue("outcome", "SUCCESS").hasBeenStopped();
140145
}
141146

147+
@Test
148+
void shouldRegisterListenerForAsyncStarts() throws Exception {
149+
CustomAsyncFilter customAsyncFilter = new CustomAsyncFilter();
150+
this.mockFilterChain = new MockFilterChain(new NoOpServlet(), customAsyncFilter);
151+
this.request.setAsyncSupported(true);
152+
this.request.setDispatcherType(DispatcherType.REQUEST);
153+
this.filter.doFilter(this.request, this.response, this.mockFilterChain);
154+
customAsyncFilter.asyncContext.dispatch();
155+
this.request.setDispatcherType(DispatcherType.ASYNC);
156+
AsyncContext newAsyncContext = this.request.startAsync();
157+
newAsyncContext.complete();
158+
159+
assertThatHttpObservation().hasLowCardinalityKeyValue("outcome", "SUCCESS").hasBeenStopped();
160+
}
161+
142162
@Test
143163
void shouldCloseObservationAfterAsyncError() throws Exception {
144164
this.request.setAsyncSupported(true);
@@ -187,4 +207,26 @@ protected void doGet(HttpServletRequest req, HttpServletResponse resp) throws Se
187207
}
188208
}
189209

210+
@SuppressWarnings("serial")
211+
static class NoOpServlet extends HttpServlet {
212+
213+
@Override
214+
protected void doGet(HttpServletRequest req, HttpServletResponse resp) throws ServletException, IOException {
215+
216+
}
217+
218+
}
219+
220+
static class CustomAsyncFilter implements Filter {
221+
222+
AsyncContext asyncContext;
223+
224+
@Override
225+
public void doFilter(ServletRequest servletRequest, ServletResponse servletResponse, FilterChain filterChain) throws IOException, ServletException {
226+
this.asyncContext = servletRequest.startAsync();
227+
filterChain.doFilter(servletRequest, servletResponse);
228+
}
229+
230+
}
231+
190232
}

0 commit comments

Comments
 (0)