From 9910df85cd5f0f326f588928c7e3078cd38c1453 Mon Sep 17 00:00:00 2001 From: Brian Clozel Date: Tue, 12 Mar 2024 09:49:00 +0100 Subject: [PATCH] Remove observation context from ClientRequest Prior to this commit, gh-31609 added the current observation context as a request attribute for `WebClient` calls. While it was not confirmed as the main cause, this feature was linked to several reports of memory leaks. This would indeed attach more memory to the request object graph at runtime - although it shouldn't prevent its collection by the GC. This commit removes this feature and instead developers should get the current observation from the reactor context if they wish to interact with it. Closes gh-32198 --- .../ClientRequestObservationContext.java | 21 ------------------- .../function/client/DefaultWebClient.java | 4 +--- .../client/WebClientObservationTests.java | 17 --------------- 3 files changed, 1 insertion(+), 41 deletions(-) diff --git a/spring-webflux/src/main/java/org/springframework/web/reactive/function/client/ClientRequestObservationContext.java b/spring-webflux/src/main/java/org/springframework/web/reactive/function/client/ClientRequestObservationContext.java index 926684eba85..deba7061c09 100644 --- a/spring-webflux/src/main/java/org/springframework/web/reactive/function/client/ClientRequestObservationContext.java +++ b/spring-webflux/src/main/java/org/springframework/web/reactive/function/client/ClientRequestObservationContext.java @@ -16,8 +16,6 @@ package org.springframework.web.reactive.function.client; -import java.util.Optional; - import io.micrometer.observation.transport.RequestReplySenderContext; import org.springframework.lang.Nullable; @@ -35,14 +33,6 @@ import org.springframework.lang.Nullable; */ public class ClientRequestObservationContext extends RequestReplySenderContext { - /** - * Name of the request attribute holding the {@link ClientRequestObservationContext context} - * for the current observation. - * @since 6.0.15 - */ - public static final String CURRENT_OBSERVATION_CONTEXT_ATTRIBUTE = ClientRequestObservationContext.class.getName(); - - @Nullable private String uriTemplate; @@ -127,15 +117,4 @@ public class ClientRequestObservationContext extends RequestReplySenderContext findCurrent(ClientRequest request) { - return Optional.ofNullable((ClientRequestObservationContext) request.attributes().get(CURRENT_OBSERVATION_CONTEXT_ATTRIBUTE)); - } - } diff --git a/spring-webflux/src/main/java/org/springframework/web/reactive/function/client/DefaultWebClient.java b/spring-webflux/src/main/java/org/springframework/web/reactive/function/client/DefaultWebClient.java index 425e08336d9..296bacb41d9 100644 --- a/spring-webflux/src/main/java/org/springframework/web/reactive/function/client/DefaultWebClient.java +++ b/spring-webflux/src/main/java/org/springframework/web/reactive/function/client/DefaultWebClient.java @@ -450,9 +450,7 @@ final class DefaultWebClient implements WebClient { if (filterFunctions != null) { filterFunction = filterFunctions.andThen(filterFunction); } - ClientRequest request = requestBuilder - .attribute(ClientRequestObservationContext.CURRENT_OBSERVATION_CONTEXT_ATTRIBUTE, observationContext) - .build(); + ClientRequest request = requestBuilder.build(); observationContext.setUriTemplate((String) request.attribute(URI_TEMPLATE_ATTRIBUTE).orElse(null)); observationContext.setRequest(request); Mono responseMono = filterFunction.apply(exchangeFunction) diff --git a/spring-webflux/src/test/java/org/springframework/web/reactive/function/client/WebClientObservationTests.java b/spring-webflux/src/test/java/org/springframework/web/reactive/function/client/WebClientObservationTests.java index 3ef2cdda489..73a35047e14 100644 --- a/spring-webflux/src/test/java/org/springframework/web/reactive/function/client/WebClientObservationTests.java +++ b/spring-webflux/src/test/java/org/springframework/web/reactive/function/client/WebClientObservationTests.java @@ -148,23 +148,6 @@ class WebClientObservationTests { verifyAndGetRequest(); } - @Test - void setsCurrentObservationContextAsRequestAttribute() { - ExchangeFilterFunction assertionFilter = (request, chain) -> { - Optional observationContext = ClientRequestObservationContext.findCurrent(request); - assertThat(observationContext).isPresent(); - return chain.exchange(request).contextWrite(context -> { - Observation currentObservation = context.get(ObservationThreadLocalAccessor.KEY); - assertThat(currentObservation.getContext()).isEqualTo(observationContext.get()); - return context; - }); - }; - this.builder.filter(assertionFilter).build().get().uri("/resource/{id}", 42) - .retrieve().bodyToMono(Void.class) - .block(Duration.ofSeconds(10)); - verifyAndGetRequest(); - } - @Test void recordsObservationWithResponseDetailsWhenFilterFunctionErrors() { ExchangeFilterFunction errorFunction = (req, next) -> next.exchange(req).then(Mono.error(new IllegalStateException()));