From b267547fb48fb467ee1e85448b443d1d8c1ed40e Mon Sep 17 00:00:00 2001 From: Brian Clozel Date: Mon, 30 Jan 2023 10:09:41 +0100 Subject: [PATCH] Contribute "uri" KeyValue for "/" client requests Prior to this commit, client HTTP requests performed by `WebClient` could miss the "uri" KeyValue for simple "/" requests. This can happen when the baseUri is configured for the client with a host and a root base path like "https://example.org/"; given the nature of the `WebClient` API, in these cases, one can perform requests like this: ``` WebClient client = WebClient.builder() .observationRegistry(registry) .baseUrl("https://example.org/") .build(); String response = client.get().retrieve().bodyToMono(String.class).block(); ``` Such a call would contribute a `"none"` value for the `"uri"` KeyValue. While only templates should be allowed for this keyvalue, we can assume that requests to `"/"` should be recorded anyway and won't cause cardinality explosion. Fixes gh-29879 --- .../client/DefaultClientRequestObservationConvention.java | 6 ++++++ .../DefaultClientRequestObservationConventionTests.java | 7 +++++++ 2 files changed, 13 insertions(+) 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 bdf77e535b1..760bd135918 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 @@ -38,6 +38,8 @@ public class DefaultClientRequestObservationConvention implements ClientRequestO private static final String DEFAULT_NAME = "http.client.requests"; + private static final String ROOT_PATH = "/"; + 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,6 +96,10 @@ public class DefaultClientRequestObservationConvention implements ClientRequestO if (context.getUriTemplate() != null) { return KeyValue.of(LowCardinalityKeyNames.URI, context.getUriTemplate()); } + ClientRequest request = context.getRequest(); + if (request != null && ROOT_PATH.equals(request.url().getPath())) { + return KeyValue.of(LowCardinalityKeyNames.URI, ROOT_PATH); + } return URI_NONE; } 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 e195a3506ba..28d31c7ce68 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 @@ -107,6 +107,13 @@ class DefaultClientRequestObservationConventionTests { assertThat(this.observationConvention.getLowCardinalityKeyValues(context)).contains(KeyValue.of("client.name", "localhost")); } + @Test + void shouldAddRootUriEvenIfTemplateMissing() { + ClientRequestObservationContext context = createContext(ClientRequest.create(HttpMethod.GET, URI.create("https://example.org/"))); + context.setRequest(context.getCarrier().build()); + assertThat(this.observationConvention.getLowCardinalityKeyValues(context)).contains(KeyValue.of("uri", "/")); + } + private ClientRequestObservationContext createContext(ClientRequest.Builder request) { ClientRequestObservationContext context = new ClientRequestObservationContext(); context.setCarrier(request);