From 3c049a2bd0b3ff3891bf5bc2d8cd4063daff322a Mon Sep 17 00:00:00 2001
From: Brian Clozel <bclozel@vmware.com>
Date: Wed, 2 Nov 2022 16:21:05 +0100
Subject: [PATCH] Polish

See gh-29388
---
 .../ClientRequestObservationContext.java      | 20 +++++++++++--------
 ...ultClientRequestObservationConvention.java | 14 ++++++-------
 .../function/client/DefaultWebClient.java     |  2 +-
 ...ientRequestObservationConventionTests.java |  8 ++++----
 4 files changed, 24 insertions(+), 20 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 dad692438f7..8e618169959 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
@@ -23,6 +23,9 @@ import org.springframework.lang.Nullable;
 /**
  * Context that holds information for metadata collection
  * during the {@link ClientHttpObservationDocumentation#HTTP_REQUEST HTTP client exchange observations}.
+ * <p>The {@link #getCarrier() tracing context carrier} is a {@link ClientRequest.Builder request builder},
+ * since the actual request is immutable. For {@code KeyValue} extraction, the {@link #getRequest() actual request}
+ * should be used instead.
  *
  * @author Brian Clozel
  * @since 6.0
@@ -35,7 +38,7 @@ public class ClientRequestObservationContext extends RequestReplySenderContext<C
 	private boolean aborted;
 
 	@Nullable
-	private ClientRequest builtRequest;
+	private ClientRequest request;
 
 
 	public ClientRequestObservationContext() {
@@ -44,7 +47,7 @@ public class ClientRequestObservationContext extends RequestReplySenderContext<C
 
 	private static void setRequestHeader(@Nullable ClientRequest.Builder request, String name, String value) {
 		if (request != null) {
-			request.header(name, value);
+			request.headers(headers -> headers.set(name, value));
 		}
 	}
 
@@ -80,16 +83,17 @@ public class ClientRequestObservationContext extends RequestReplySenderContext<C
 	}
 
 	/**
-	 * Return the built request.
+	 * Return the immutable client request.
 	 */
-	public ClientRequest getBuiltRequest() {
-		return this.builtRequest;
+	@Nullable
+	public ClientRequest getRequest() {
+		return this.request;
 	}
 
 	/**
-	 * Set the built request.
+	 * Set the client request.
 	 */
-	public void setBuiltRequest(ClientRequest builtRequest) {
-		this.builtRequest = builtRequest;
+	public void setRequest(ClientRequest request) {
+		this.request = request;
 	}
 }
diff --git a/spring-webflux/src/main/java/org/springframework/web/reactive/function/client/DefaultClientRequestObservationConvention.java b/spring-webflux/src/main/java/org/springframework/web/reactive/function/client/DefaultClientRequestObservationConvention.java
index b024ccc8a77..21a380c801a 100644
--- a/spring-webflux/src/main/java/org/springframework/web/reactive/function/client/DefaultClientRequestObservationConvention.java
+++ b/spring-webflux/src/main/java/org/springframework/web/reactive/function/client/DefaultClientRequestObservationConvention.java
@@ -79,7 +79,7 @@ public class DefaultClientRequestObservationConvention implements ClientRequestO
 
 	@Override
 	public String getContextualName(ClientRequestObservationContext context) {
-		return "http " + context.getBuiltRequest().method().name().toLowerCase();
+		return "http " + context.getRequest().method().name().toLowerCase();
 	}
 
 	@Override
@@ -95,8 +95,8 @@ public class DefaultClientRequestObservationConvention implements ClientRequestO
 	}
 
 	protected KeyValue method(ClientRequestObservationContext context) {
-		if (context.getBuiltRequest() != null) {
-			return KeyValue.of(ClientHttpObservationDocumentation.LowCardinalityKeyNames.METHOD, context.getBuiltRequest().method().name());
+		if (context.getRequest() != null) {
+			return KeyValue.of(ClientHttpObservationDocumentation.LowCardinalityKeyNames.METHOD, context.getRequest().method().name());
 		}
 		else {
 			return METHOD_NONE;
@@ -143,15 +143,15 @@ public class DefaultClientRequestObservationConvention implements ClientRequestO
 	}
 
 	protected KeyValue httpUrl(ClientRequestObservationContext context) {
-		if (context.getBuiltRequest() != null) {
-			return KeyValue.of(ClientHttpObservationDocumentation.HighCardinalityKeyNames.HTTP_URL, context.getBuiltRequest().url().toASCIIString());
+		if (context.getRequest() != null) {
+			return KeyValue.of(ClientHttpObservationDocumentation.HighCardinalityKeyNames.HTTP_URL, context.getRequest().url().toASCIIString());
 		}
 		return HTTP_URL_NONE;
 	}
 
 	protected KeyValue clientName(ClientRequestObservationContext context) {
-		if (context.getBuiltRequest() != null && context.getBuiltRequest().url().getHost() != null) {
-			return KeyValue.of(ClientHttpObservationDocumentation.HighCardinalityKeyNames.CLIENT_NAME, context.getBuiltRequest().url().getHost());
+		if (context.getRequest() != null && context.getRequest().url().getHost() != null) {
+			return KeyValue.of(ClientHttpObservationDocumentation.HighCardinalityKeyNames.CLIENT_NAME, context.getRequest().url().getHost());
 		}
 		return CLIENT_NAME_NONE;
 	}
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 0fedf071d2b..829aa0d365d 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
@@ -467,7 +467,7 @@ class DefaultWebClient implements WebClient {
 						.start();
 				ClientRequest request = requestBuilder.build();
 				observationContext.setUriTemplate((String) request.attribute(URI_TEMPLATE_ATTRIBUTE).orElse(null));
-				observationContext.setBuiltRequest(request);
+				observationContext.setRequest(request);
 				Mono<ClientResponse> responseMono = exchangeFunction.exchange(request)
 						.checkpoint("Request to " + this.httpMethod.name() + " " + this.uri + " [DefaultWebClient]")
 						.switchIfEmpty(NO_HTTP_CLIENT_RESPONSE_ERROR);
diff --git a/spring-webflux/src/test/java/org/springframework/web/reactive/function/client/DefaultClientRequestObservationConventionTests.java b/spring-webflux/src/test/java/org/springframework/web/reactive/function/client/DefaultClientRequestObservationConventionTests.java
index c2bb58789ba..a45c7aeb0f8 100644
--- a/spring-webflux/src/test/java/org/springframework/web/reactive/function/client/DefaultClientRequestObservationConventionTests.java
+++ b/spring-webflux/src/test/java/org/springframework/web/reactive/function/client/DefaultClientRequestObservationConventionTests.java
@@ -45,7 +45,7 @@ class DefaultClientRequestObservationConventionTests {
 	void shouldHaveContextualName() {
 		ClientRequestObservationContext context = new ClientRequestObservationContext();
 		context.setCarrier(ClientRequest.create(HttpMethod.GET, URI.create("/test")));
-		context.setBuiltRequest(context.getCarrier().build());
+		context.setRequest(context.getCarrier().build());
 		assertThat(this.observationConvention.getContextualName(context)).isEqualTo("http get");
 	}
 
@@ -82,7 +82,7 @@ class DefaultClientRequestObservationConventionTests {
 				.attribute(WebClient.class.getName() + ".uriTemplate", "/resource/{id}");
 		ClientRequestObservationContext context = createContext(request);
 		context.setUriTemplate("/resource/{id}");
-		context.setBuiltRequest(context.getCarrier().build());
+		context.setRequest(context.getCarrier().build());
 		assertThat(this.observationConvention.getLowCardinalityKeyValues(context))
 				.contains(KeyValue.of("exception", "none"), KeyValue.of("method", "GET"), KeyValue.of("uri", "/resource/{id}"),
 						KeyValue.of("status", "200"), KeyValue.of("outcome", "SUCCESS"));
@@ -93,7 +93,7 @@ class DefaultClientRequestObservationConventionTests {
 	@Test
 	void shouldAddKeyValuesForRequestWithoutUriTemplate() {
 		ClientRequestObservationContext context = createContext(ClientRequest.create(HttpMethod.GET, URI.create("/resource/42")));
-		context.setBuiltRequest(context.getCarrier().build());
+		context.setRequest(context.getCarrier().build());
 		assertThat(this.observationConvention.getLowCardinalityKeyValues(context))
 				.contains(KeyValue.of("method", "GET"), KeyValue.of("uri", "none"));
 		assertThat(this.observationConvention.getHighCardinalityKeyValues(context)).hasSize(2).contains(KeyValue.of("http.url", "/resource/42"));
@@ -102,7 +102,7 @@ class DefaultClientRequestObservationConventionTests {
 	@Test
 	void shouldAddClientNameKeyValueForRequestWithHost() {
 		ClientRequestObservationContext context = createContext(ClientRequest.create(HttpMethod.GET, URI.create("https://localhost:8080/resource/42")));
-		context.setBuiltRequest(context.getCarrier().build());
+		context.setRequest(context.getCarrier().build());
 		assertThat(this.observationConvention.getHighCardinalityKeyValues(context)).contains(KeyValue.of("client.name", "localhost"));
 	}