Restrict "uri" KeyValue for client observations

Prior to this commit, the "uri" KeyValue for low cardinality metadata
would contain the entire uri template given to the HTTP client when
creating the request. This was a breaking change for existing metrics
dashboards, as previous support was removing the protocol, host and port
parts of the URI.
Indeed, this information is available in the "client.name" and
"http.uri" KayValue.

This commit parses and removes the protocol+host+port information from
the uri template for the "uri" KeyValue.

Fixes gh-29885

Author: bclozel

Diff for: framework-docs/src/docs/asciidoc/integration/observability.adoc

@@ -132,7 +132,7 @@ Instrumentation is using the `org.springframework.http.client.observation.Client
 |===
 |Name | Description
 |`method` _(required)_|Name of HTTP request method or `"none"` if the request could not be created.
-|`uri` _(required)_|URI template used for HTTP request, or `"none"` if none was provided.
+|`uri` _(required)_|URI template used for HTTP request, or `"none"` if none was provided. Only the path part of the URI is considered.
 |`client.name` _(required)_|Client name derived from the request URI host.
 |`status` _(required)_|HTTP response raw status code, or `"IO_ERROR"` in case of `IOException`, or `"CLIENT_ERROR"` if no response was received.
 |`outcome` _(required)_|Outcome of the HTTP client exchange.
@@ -158,7 +158,7 @@ Instrumentation is using the `org.springframework.web.reactive.function.client.C
 |===
 |Name | Description
 |`method` _(required)_|Name of HTTP request method or `"none"` if the request could not be created.
-|`uri` _(required)_|URI template used for HTTP request, or `"none"` if none was provided.
+|`uri` _(required)_|URI template used for HTTP request, or `"none"` if none was provided. Only the path part of the URI is considered.
 |`client.name` _(required)_|Client name derived from the request URI host.
 |`status` _(required)_|HTTP response raw status code, or `"IO_ERROR"` in case of `IOException`, or `"CLIENT_ERROR"` if no response was received.
 |`outcome` _(required)_|Outcome of the HTTP client exchange.

Diff for: spring-web/src/main/java/org/springframework/http/client/observation/ClientHttpObservationDocumentation.java

@@ -70,6 +70,7 @@ public String asString() {
 
 		/**
 		 * URI template used for HTTP request, or {@value KeyValue#NONE_VALUE} if none was provided.
+		 * Only the path part of the URI is considered.
 		 */
 		URI {
 			@Override

Diff for: spring-web/src/main/java/org/springframework/http/client/observation/DefaultClientRequestObservationConvention.java

@@ -17,6 +17,7 @@
 package org.springframework.http.client.observation;
 
 import java.io.IOException;
+import java.util.regex.Pattern;
 
 import io.micrometer.common.KeyValue;
 import io.micrometer.common.KeyValues;
@@ -39,6 +40,8 @@ public class DefaultClientRequestObservationConvention implements ClientRequestO
 
 	private static final String DEFAULT_NAME = "http.client.requests";
 
+	private static final Pattern PATTERN_BEFORE_PATH = Pattern.compile("^https?://[^/]+/");
+
 	private static final KeyValue URI_NONE = KeyValue.of(LowCardinalityKeyNames.URI, KeyValue.NONE_VALUE);
 
 	private static final KeyValue METHOD_NONE = KeyValue.of(LowCardinalityKeyNames.METHOD, KeyValue.NONE_VALUE);
@@ -92,11 +95,16 @@ public KeyValues getLowCardinalityKeyValues(ClientRequestObservationContext cont
 
 	protected KeyValue uri(ClientRequestObservationContext context) {
 		if (context.getUriTemplate() != null) {
-			return KeyValue.of(LowCardinalityKeyNames.URI, context.getUriTemplate());
+			return KeyValue.of(LowCardinalityKeyNames.URI, extractPath(context.getUriTemplate()));
 		}
 		return URI_NONE;
 	}
 
+	private static String extractPath(String uriTemplate) {
+		String path = PATTERN_BEFORE_PATH.matcher(uriTemplate).replaceFirst("");
+		return (path.startsWith("/") ? path : "/" + path);
+	}
+
 	protected KeyValue method(ClientRequestObservationContext context) {
 		if (context.getCarrier() != null) {
 			return KeyValue.of(LowCardinalityKeyNames.METHOD, context.getCarrier().getMethod().name());

Diff for: spring-web/src/test/java/org/springframework/http/client/observation/DefaultClientRequestObservationConventionTests.java

@@ -73,6 +73,18 @@ void addsKeyValuesForRequestWithUriTemplate() {
 		assertThat(this.observationConvention.getHighCardinalityKeyValues(context)).contains(KeyValue.of("http.url", "/resource/42"));
 	}
 
+	@Test
+	void addsKeyValuesForRequestWithUriTemplateWithHost() {
+		ClientRequestObservationContext context = createContext(
+				new MockClientHttpRequest(HttpMethod.GET, "https://example.org/resource/{id}", 42), new MockClientHttpResponse());
+		context.setUriTemplate("https://example.org/resource/{id}");
+		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("client.name", "example.org"), KeyValue.of("outcome", "SUCCESS"));
+		assertThat(this.observationConvention.getHighCardinalityKeyValues(context)).contains(KeyValue.of("http.url", "https://example.org/resource/42"));
+	}
+
+
 	@Test
 	void addsKeyValuesForRequestWithoutUriTemplate() {
 		ClientRequestObservationContext context = createContext(

Diff for: spring-web/src/test/java/org/springframework/web/client/RestTemplateObservationTests.java

@@ -88,7 +88,7 @@ void executeVarArgsAddsUriTemplateAsKeyValue() throws Exception {
 		template.execute("https://example.com/hotels/{hotel}/bookings/{booking}", GET,
 				null, null, "42", "21");
 
-		assertThatHttpObservation().hasLowCardinalityKeyValue("uri", "https://example.com/hotels/{hotel}/bookings/{booking}");
+		assertThatHttpObservation().hasLowCardinalityKeyValue("uri", "/hotels/{hotel}/bookings/{booking}");
 	}
 
 	@Test
@@ -101,7 +101,7 @@ void executeArgsMapAddsUriTemplateAsKeyValue() throws Exception {
 		template.execute("https://example.com/hotels/{hotel}/bookings/{booking}", GET,
 				null, null, vars);
 
-		assertThatHttpObservation().hasLowCardinalityKeyValue("uri", "https://example.com/hotels/{hotel}/bookings/{booking}");
+		assertThatHttpObservation().hasLowCardinalityKeyValue("uri", "/hotels/{hotel}/bookings/{booking}");
 	}
 
 

Diff for: spring-webflux/src/main/java/org/springframework/web/reactive/function/client/ClientHttpObservationDocumentation.java

@@ -67,6 +67,7 @@ public String asString() {
 
 		/**
 		 * URI template used for HTTP request, or {@value KeyValue#NONE_VALUE} if none was provided.
+		 * Only the path part of the URI is considered.
 		 */
 		URI {
 			@Override
@@ -123,7 +124,7 @@ public String asString() {
 	public enum HighCardinalityKeyNames implements KeyName {
 
 		/**
-		 * HTTP request URI.
+		 * The full HTTP request URI.
 		 */
 		HTTP_URL {
 			@Override

Diff for: spring-webflux/src/main/java/org/springframework/web/reactive/function/client/DefaultClientRequestObservationConvention.java

@@ -17,6 +17,7 @@
 package org.springframework.web.reactive.function.client;
 
 import java.io.IOException;
+import java.util.regex.Pattern;
 
 import io.micrometer.common.KeyValue;
 import io.micrometer.common.KeyValues;
@@ -40,6 +41,8 @@ public class DefaultClientRequestObservationConvention implements ClientRequestO
 
 	private static final String ROOT_PATH = "/";
 
+	private static final Pattern PATTERN_BEFORE_PATH = Pattern.compile("^https?://[^/]+/");
+
 	private static final KeyValue URI_NONE = KeyValue.of(LowCardinalityKeyNames.URI, KeyValue.NONE_VALUE);
 
 	private static final KeyValue METHOD_NONE = KeyValue.of(LowCardinalityKeyNames.METHOD, KeyValue.NONE_VALUE);
@@ -94,7 +97,7 @@ public KeyValues getLowCardinalityKeyValues(ClientRequestObservationContext cont
 
 	protected KeyValue uri(ClientRequestObservationContext context) {
 		if (context.getUriTemplate() != null) {
-			return KeyValue.of(LowCardinalityKeyNames.URI, context.getUriTemplate());
+			return KeyValue.of(LowCardinalityKeyNames.URI, extractPath(context.getUriTemplate()));
 		}
 		ClientRequest request = context.getRequest();
 		if (request != null && ROOT_PATH.equals(request.url().getPath())) {
@@ -103,6 +106,11 @@ protected KeyValue uri(ClientRequestObservationContext context) {
 		return URI_NONE;
 	}
 
+	private static String extractPath(String uriTemplate) {
+		String path = PATTERN_BEFORE_PATH.matcher(uriTemplate).replaceFirst("");
+		return (path.startsWith("/") ? path : "/" + path);
+	}
+
 	protected KeyValue method(ClientRequestObservationContext context) {
 		if (context.getRequest() != null) {
 			return KeyValue.of(LowCardinalityKeyNames.METHOD, context.getRequest().method().name());

Diff for: spring-webflux/src/test/java/org/springframework/web/reactive/function/client/DefaultClientRequestObservationConventionTests.java

@@ -114,6 +114,14 @@ void shouldAddRootUriEvenIfTemplateMissing() {
 		assertThat(this.observationConvention.getLowCardinalityKeyValues(context)).contains(KeyValue.of("uri", "/"));
 	}
 
+	@Test
+	void shouldOnlyConsiderPathForUriKeyValue() {
+		ClientRequestObservationContext context = createContext(ClientRequest.create(HttpMethod.GET, URI.create("https://example.org/resource/42")));
+		context.setUriTemplate("https://example.org/resource/{id}");
+		context.setRequest(context.getCarrier().build());
+		assertThat(this.observationConvention.getLowCardinalityKeyValues(context)).contains(KeyValue.of("uri", "/resource/{id}"));
+	}
+
 	private ClientRequestObservationContext createContext(ClientRequest.Builder request) {
 		ClientRequestObservationContext context = new ClientRequestObservationContext();
 		context.setCarrier(request);