Skip to content

Commit

Permalink
Guard against no-op observation
Browse files Browse the repository at this point in the history
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
  • Loading branch information
philwebb committed Oct 19, 2022
1 parent 0889e47 commit f93fda2
Show file tree
Hide file tree
Showing 2 changed files with 15 additions and 1 deletion.
Original file line number Diff line number Diff line change
Expand Up @@ -130,7 +130,9 @@ private Observation createOrFetchObservation(HttpServletRequest request, HttpSer
observation = ServerHttpObservationDocumentation.HTTP_REQUESTS.observation(this.observationConvention,
DEFAULT_OBSERVATION_CONVENTION, () -> context, this.observationRegistry).start();
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;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@

package org.springframework.web.filter;

import io.micrometer.observation.ObservationRegistry;
import io.micrometer.observation.tck.TestObservationRegistry;
import io.micrometer.observation.tck.TestObservationRegistryAssert;
import jakarta.servlet.RequestDispatcher;
Expand All @@ -33,6 +34,7 @@

/**
* Tests for {@link ServerHttpObservationFilter}.
*
* @author Brian Clozel
*/
class ServerHttpObservationFilterTests {
Expand Down Expand Up @@ -61,6 +63,16 @@ void filterShouldFillObservationContext() throws Exception {
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
void filterShouldUseThrownException() throws Exception {
IllegalArgumentException customError = new IllegalArgumentException("custom error");
Expand Down

0 comments on commit f93fda2

Please sign in to comment.