Skip to content

Commit

Permalink
Remove ServerWebExchange dependency in ServerRequestObservationContext
Browse files Browse the repository at this point in the history
Avoiding cycle between http.server and web.server packages.

See gh-30013
  • Loading branch information
jhoeller committed Jun 14, 2023
1 parent 220995b commit f00a8cb
Show file tree
Hide file tree
Showing 7 changed files with 40 additions and 22 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -25,11 +25,12 @@
import org.springframework.http.server.reactive.ServerHttpRequest;
import org.springframework.http.server.reactive.ServerHttpResponse;
import org.springframework.lang.Nullable;
import org.springframework.web.server.ServerWebExchange;

/**
* Context that holds information for metadata collection regarding
* {@link ServerHttpObservationDocumentation#HTTP_REACTIVE_SERVER_REQUESTS reactive HTTP requests} observations.
* {@link ServerHttpObservationDocumentation#HTTP_REACTIVE_SERVER_REQUESTS reactive HTTP requests}
* observations.
*
* <p>This context also extends {@link RequestReplyReceiverContext} for propagating
* tracing information during HTTP request processing.
*
Expand All @@ -39,10 +40,11 @@
public class ServerRequestObservationContext extends RequestReplyReceiverContext<ServerHttpRequest, ServerHttpResponse> {

/**
* Name of the request attribute holding the {@link ServerRequestObservationContext context} for the current observation.
* Name of the request attribute holding the {@link ServerRequestObservationContext context}
* for the current observation.
* @since 6.1
*/
public static final String CURRENT_OBSERVATION_CONTEXT_ATTRIBUTE = ServerRequestObservationContext.class.getName() + ".context";
public static final String CURRENT_OBSERVATION_CONTEXT_ATTRIBUTE = ServerRequestObservationContext.class.getName();


private final Map<String, Object> attributes;
Expand All @@ -53,7 +55,15 @@ public class ServerRequestObservationContext extends RequestReplyReceiverContext
private boolean connectionAborted;


public ServerRequestObservationContext(ServerHttpRequest request, ServerHttpResponse response, Map<String, Object> attributes) {
/**
* Create a new {@code ServerRequestObservationContext} instance.
* @param request the current request
* @param response the current response
* @param attributes the current attributes
*/
public ServerRequestObservationContext(
ServerHttpRequest request, ServerHttpResponse response, Map<String, Object> attributes) {

super((req, key) -> req.getHeaders().getFirst(key));
setCarrier(request);
setResponse(response);
Expand Down Expand Up @@ -89,8 +99,8 @@ public void setPathPattern(@Nullable String pathPattern) {
}

/**
* Whether the current connection was aborted by the client, resulting
* in a {@link reactor.core.publisher.SignalType#CANCEL cancel signal} on the reactive chain,
* Whether the current connection was aborted by the client, resulting in a
* {@link reactor.core.publisher.SignalType#CANCEL cancel signal} on the reactive chain,
* or an {@code AbortedException} when reading the request.
* @return if the connection has been aborted
*/
Expand All @@ -99,8 +109,8 @@ public boolean isConnectionAborted() {
}

/**
* Set whether the current connection was aborted by the client, resulting
* in a {@link reactor.core.publisher.SignalType#CANCEL cancel signal} on the reactive chain,
* Set whether the current connection was aborted by the client, resulting in a
* {@link reactor.core.publisher.SignalType#CANCEL cancel signal} on the reactive chain,
* or an {@code AbortedException} when reading the request.
* @param connectionAborted if the connection has been aborted
*/
Expand All @@ -110,13 +120,15 @@ public void setConnectionAborted(boolean connectionAborted) {


/**
* Get the current {@link ServerRequestObservationContext observation context} from the given exchange, if available.
* @param exchange the current exchange
* Get the current {@link ServerRequestObservationContext observation context}
* from the given attributes, if available.
* @param attributes the current exchange attributes
* @return the current observation context
* @since 6.1
*/
public static Optional<ServerRequestObservationContext> findCurrent(ServerWebExchange exchange) {
return Optional.ofNullable(exchange.getAttribute(CURRENT_OBSERVATION_CONTEXT_ATTRIBUTE));
public static Optional<ServerRequestObservationContext> findCurrent(Map<String, Object> attributes) {
return Optional.ofNullable(
(ServerRequestObservationContext) attributes.get(CURRENT_OBSERVATION_CONTEXT_ATTRIBUTE));
}

}
Original file line number Diff line number Diff line change
Expand Up @@ -296,9 +296,10 @@ public Mono<Void> handle(ServerHttpRequest request, ServerHttpResponse response)
exchange.getLogPrefix() + formatRequest(exchange.getRequest()) +
(traceOn ? ", headers=" + formatHeaders(exchange.getRequest().getHeaders()) : ""));

ServerRequestObservationContext observationContext = new ServerRequestObservationContext(exchange.getRequest(),
exchange.getResponse(), exchange.getAttributes());
exchange.getAttributes().put(ServerRequestObservationContext.CURRENT_OBSERVATION_CONTEXT_ATTRIBUTE, observationContext);
ServerRequestObservationContext observationContext = new ServerRequestObservationContext(
exchange.getRequest(), exchange.getResponse(), exchange.getAttributes());
exchange.getAttributes().put(
ServerRequestObservationContext.CURRENT_OBSERVATION_CONTEXT_ATTRIBUTE, observationContext);

return getDelegate().handle(exchange)
.transformDeferred(call -> transform(exchange, observationContext, call))
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,7 @@ class HttpWebHandlerAdapterObservabilityTests {

private final MockServerHttpResponse response = new MockServerHttpResponse();


@Test
void handlerShouldSetObservationContextOnExchange() {
HttpStatusSuccessStubWebHandler targetHandler = new HttpStatusSuccessStubWebHandler(HttpStatus.OK);
Expand Down Expand Up @@ -97,6 +98,7 @@ private TestObservationRegistryAssert.TestObservationRegistryAssertReturningObse
.hasObservationWithNameEqualTo("http.server.requests").that();
}


private static class HttpStatusSuccessStubWebHandler implements WebHandler {

private final HttpStatus responseStatus;
Expand All @@ -109,12 +111,13 @@ public HttpStatusSuccessStubWebHandler(HttpStatus responseStatus) {

@Override
public Mono<Void> handle(ServerWebExchange exchange) {
this.observationContext = ServerRequestObservationContext.findCurrent(exchange);
this.observationContext = ServerRequestObservationContext.findCurrent(exchange.getAttributes());
exchange.getResponse().setStatusCode(this.responseStatus);
return Mono.empty();
}
}


private static class ReactorContextWebHandler implements WebHandler {

ContextView contextView;
Expand All @@ -129,6 +132,7 @@ public Mono<Void> handle(ServerWebExchange exchange) {
}
}


private static class ThrowingExceptionWebHandler implements WebHandler {

private final Throwable exception;
Expand All @@ -141,11 +145,12 @@ private ThrowingExceptionWebHandler(Throwable exception) {

@Override
public Mono<Void> handle(ServerWebExchange exchange) {
this.observationContext = ServerRequestObservationContext.findCurrent(exchange);
this.observationContext = ServerRequestObservationContext.findCurrent(exchange.getAttributes());
return Mono.error(this.exception);
}
}


private static class BadRequestExceptionHandler implements WebExceptionHandler {

@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -174,7 +174,7 @@ private void setAttributes(
org.springframework.web.filter.reactive.ServerHttpObservationFilter
.findObservationContext(serverRequest.exchange())
.ifPresent(context -> context.setPathPattern(matchingPattern.toString()));
ServerRequestObservationContext.findCurrent(serverRequest.exchange())
ServerRequestObservationContext.findCurrent(serverRequest.exchange().getAttributes())
.ifPresent(context -> context.setPathPattern(matchingPattern.toString()));
}
Map<String, String> uriVariables =
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -170,7 +170,7 @@ protected Object lookupHandler(PathContainer lookupPath, ServerWebExchange excha
org.springframework.web.filter.reactive.ServerHttpObservationFilter
.findObservationContext(exchange)
.ifPresent(context -> context.setPathPattern(pattern.toString()));
ServerRequestObservationContext.findCurrent(exchange)
ServerRequestObservationContext.findCurrent(exchange.getAttributes())
.ifPresent(context -> context.setPathPattern(pattern.toString()));
exchange.getAttributes().put(PATH_WITHIN_HANDLER_MAPPING_ATTRIBUTE, pathWithinMapping);
exchange.getAttributes().put(URI_TEMPLATE_VARIABLES_ATTRIBUTE, matchInfo.getUriVariables());
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -145,7 +145,7 @@ protected void handleMatch(RequestMappingInfo info, HandlerMethod handlerMethod,
org.springframework.web.filter.reactive.ServerHttpObservationFilter
.findObservationContext(exchange)
.ifPresent(context -> context.setPathPattern(bestPattern.toString()));
ServerRequestObservationContext.findCurrent(exchange)
ServerRequestObservationContext.findCurrent(exchange.getAttributes())
.ifPresent(context -> context.setPathPattern(bestPattern.toString()));
exchange.getAttributes().put(URI_TEMPLATE_VARIABLES_ATTRIBUTE, uriVariables);
exchange.getAttributes().put(MATRIX_VARIABLES_ATTRIBUTE, matrixVariables);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -137,7 +137,7 @@ void mappedRequestShouldHoldAttributes() {
assertThat(matchingPattern.getPatternString()).isEqualTo("/match");
assertThat(org.springframework.web.filter.reactive.ServerHttpObservationFilter.findObservationContext(exchange))
.hasValueSatisfying(context -> assertThat(context.getPathPattern()).isEqualTo(matchingPattern.getPatternString()));
assertThat(ServerRequestObservationContext.findCurrent(exchange))
assertThat(ServerRequestObservationContext.findCurrent(exchange.getAttributes()))
.hasValueSatisfying(context -> assertThat(context.getPathPattern()).isEqualTo(matchingPattern.getPatternString()));

ServerRequest serverRequest = exchange.getAttribute(RouterFunctions.REQUEST_ATTRIBUTE);
Expand Down

0 comments on commit f00a8cb

Please sign in to comment.