Skip to content

Commit

Permalink
Remove observation context from ClientRequest
Browse files Browse the repository at this point in the history
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
  • Loading branch information
bclozel committed Mar 12, 2024
1 parent 88b3844 commit 9910df8
Show file tree
Hide file tree
Showing 3 changed files with 1 addition and 41 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -35,14 +33,6 @@
*/
public class ClientRequestObservationContext extends RequestReplySenderContext<ClientRequest.Builder, ClientResponse> {

/**
* 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;

Expand Down Expand Up @@ -127,15 +117,4 @@ public ClientRequest getRequest() {
}


/**
* Get the current {@link ClientRequestObservationContext observation context}
* from the given request, if available.
* @param request the current client request
* @return the current observation context
* @since 6.0.15
*/
public static Optional<ClientRequestObservationContext> findCurrent(ClientRequest request) {
return Optional.ofNullable((ClientRequestObservationContext) request.attributes().get(CURRENT_OBSERVATION_CONTEXT_ATTRIBUTE));
}

}
Original file line number Diff line number Diff line change
Expand Up @@ -450,9 +450,7 @@ public Mono<ClientResponse> exchange() {
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<ClientResponse> responseMono = filterFunction.apply(exchangeFunction)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -148,23 +148,6 @@ void setsCurrentObservationInReactorContext() {
verifyAndGetRequest();
}

@Test
void setsCurrentObservationContextAsRequestAttribute() {
ExchangeFilterFunction assertionFilter = (request, chain) -> {
Optional<ClientRequestObservationContext> 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()));
Expand Down

0 comments on commit 9910df8

Please sign in to comment.