root
/
spring-framework
mirror of https://github.com/spring-projects/spring-framework.git
1
0
Fork 0
Code Issues Actions 8 Packages Projects Releases Wiki Activity

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
This commit is contained in:
Brian Clozel 2023-01-30 10:09:41 +01:00
parent e0f60dc09d
commit b267547fb4
2 changed files with 13 additions and 0 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

6
spring-webflux/src/main/java/org/springframework/web/reactive/function/client/DefaultClientRequestObservationConvention.java
View File

@ -38,6 +38,8 @@ public class DefaultClientRequestObservationConvention implements ClientRequestO
private static final String DEFAULT_NAME = "http.client.requests"; 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 URI_NONE = KeyValue.of(LowCardinalityKeyNames.URI, KeyValue.NONE_VALUE);
private static final KeyValue METHOD_NONE = KeyValue.of(LowCardinalityKeyNames.METHOD, 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) { if (context.getUriTemplate() != null) {
return KeyValue.of(LowCardinalityKeyNames.URI, context.getUriTemplate()); 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; return URI_NONE;
} }

7
spring-webflux/src/test/java/org/springframework/web/reactive/function/client/DefaultClientRequestObservationConventionTests.java
View File

@ -107,6 +107,13 @@ class DefaultClientRequestObservationConventionTests {
assertThat(this.observationConvention.getLowCardinalityKeyValues(context)).contains(KeyValue.of("client.name", "localhost")); 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) { private ClientRequestObservationContext createContext(ClientRequest.Builder request) {
ClientRequestObservationContext context = new ClientRequestObservationContext(); ClientRequestObservationContext context = new ClientRequestObservationContext();
context.setCarrier(request); context.setCarrier(request);