Guard against no-op observation
Update `ServerHttpObservationFilter` to check if the `Observation` is a no-op before adding the `ServerRequestObservationContext`. Prior to this commit, if the `Observation` is a no-op then the context type added with the `CURRENT_OBSERVATION_CONTEXT_ATTRIBUTE` would not be a `ServerRequestObservationContext`. This would mean that `findObservationContext` would throw a `ClassCastException`. Fixes gh-29356
This commit is contained in:
		
							parent
							
								
									0889e47608
								
							
						
					
					
						commit
						f93fda2a95
					
				| 
						 | 
					@ -130,7 +130,9 @@ public class ServerHttpObservationFilter extends OncePerRequestFilter {
 | 
				
			||||||
			observation = ServerHttpObservationDocumentation.HTTP_REQUESTS.observation(this.observationConvention,
 | 
								observation = ServerHttpObservationDocumentation.HTTP_REQUESTS.observation(this.observationConvention,
 | 
				
			||||||
					DEFAULT_OBSERVATION_CONVENTION, () -> context, this.observationRegistry).start();
 | 
										DEFAULT_OBSERVATION_CONVENTION, () -> context, this.observationRegistry).start();
 | 
				
			||||||
			request.setAttribute(CURRENT_OBSERVATION_ATTRIBUTE, observation);
 | 
								request.setAttribute(CURRENT_OBSERVATION_ATTRIBUTE, observation);
 | 
				
			||||||
			request.setAttribute(CURRENT_OBSERVATION_CONTEXT_ATTRIBUTE, observation.getContext());
 | 
								if (!observation.isNoop()) {
 | 
				
			||||||
 | 
									request.setAttribute(CURRENT_OBSERVATION_CONTEXT_ATTRIBUTE, observation.getContext());
 | 
				
			||||||
 | 
								}
 | 
				
			||||||
		}
 | 
							}
 | 
				
			||||||
		return observation;
 | 
							return observation;
 | 
				
			||||||
	}
 | 
						}
 | 
				
			||||||
| 
						 | 
					
 | 
				
			||||||
| 
						 | 
					@ -16,6 +16,7 @@
 | 
				
			||||||
 | 
					
 | 
				
			||||||
package org.springframework.web.filter;
 | 
					package org.springframework.web.filter;
 | 
				
			||||||
 | 
					
 | 
				
			||||||
 | 
					import io.micrometer.observation.ObservationRegistry;
 | 
				
			||||||
import io.micrometer.observation.tck.TestObservationRegistry;
 | 
					import io.micrometer.observation.tck.TestObservationRegistry;
 | 
				
			||||||
import io.micrometer.observation.tck.TestObservationRegistryAssert;
 | 
					import io.micrometer.observation.tck.TestObservationRegistryAssert;
 | 
				
			||||||
import jakarta.servlet.RequestDispatcher;
 | 
					import jakarta.servlet.RequestDispatcher;
 | 
				
			||||||
| 
						 | 
					@ -33,6 +34,7 @@ import static org.assertj.core.api.Assertions.assertThatThrownBy;
 | 
				
			||||||
 | 
					
 | 
				
			||||||
/**
 | 
					/**
 | 
				
			||||||
 * Tests for {@link ServerHttpObservationFilter}.
 | 
					 * Tests for {@link ServerHttpObservationFilter}.
 | 
				
			||||||
 | 
					 *
 | 
				
			||||||
 * @author Brian Clozel
 | 
					 * @author Brian Clozel
 | 
				
			||||||
 */
 | 
					 */
 | 
				
			||||||
class ServerHttpObservationFilterTests {
 | 
					class ServerHttpObservationFilterTests {
 | 
				
			||||||
| 
						 | 
					@ -61,6 +63,16 @@ class ServerHttpObservationFilterTests {
 | 
				
			||||||
		assertThatHttpObservation().hasLowCardinalityKeyValue("outcome", "SUCCESS");
 | 
							assertThatHttpObservation().hasLowCardinalityKeyValue("outcome", "SUCCESS");
 | 
				
			||||||
	}
 | 
						}
 | 
				
			||||||
 | 
					
 | 
				
			||||||
 | 
						@Test
 | 
				
			||||||
 | 
						void filterShouldAcceptNoOpObservationContext() throws Exception {
 | 
				
			||||||
 | 
							ServerHttpObservationFilter filter = new ServerHttpObservationFilter(ObservationRegistry.NOOP);
 | 
				
			||||||
 | 
							filter.doFilter(this.request, this.response, this.mockFilterChain);
 | 
				
			||||||
 | 
					
 | 
				
			||||||
 | 
							ServerRequestObservationContext context = (ServerRequestObservationContext) this.request
 | 
				
			||||||
 | 
									.getAttribute(ServerHttpObservationFilter.CURRENT_OBSERVATION_CONTEXT_ATTRIBUTE);
 | 
				
			||||||
 | 
							assertThat(context).isNull();
 | 
				
			||||||
 | 
						}
 | 
				
			||||||
 | 
					
 | 
				
			||||||
	@Test
 | 
						@Test
 | 
				
			||||||
	void filterShouldUseThrownException() throws Exception {
 | 
						void filterShouldUseThrownException() throws Exception {
 | 
				
			||||||
		IllegalArgumentException customError = new IllegalArgumentException("custom error");
 | 
							IllegalArgumentException customError = new IllegalArgumentException("custom error");
 | 
				
			||||||
| 
						 | 
					
 | 
				
			||||||
		Loading…
	
		Reference in New Issue