Fix IllegalArgumentException in WebClient convention adapter

Prior to this commit, the `ClientObservationConventionAdapter` would
fail with an `IllegalArgumentException` when the observation is first
started: at this point, the carrier (the request builder here) is
present, but the full request not yet fully built.

This commit ensures that the convention adapter uses the request and, if
not available, the request builder to adapt to the
`WebClientExchangeTagsProvider`.

Fixes gh-33483
This commit is contained in:
Brian Clozel 2022-12-07 13:16:57 +01:00
parent 87fd27c329
commit 50be8cbf91
2 changed files with 16 additions and 12 deletions

View File

@ -55,19 +55,14 @@ class ClientObservationConventionAdapter implements ClientRequestObservationConv
@Override @Override
public KeyValues getLowCardinalityKeyValues(ClientRequestObservationContext context) { public KeyValues getLowCardinalityKeyValues(ClientRequestObservationContext context) {
mutateClientRequest(context); ClientRequest request = context.getRequest();
Iterable<Tag> tags = this.tagsProvider.tags(context.getRequest(), context.getResponse(), context.getError()); if (request == null) {
request = context.getCarrier().attribute(URI_TEMPLATE_ATTRIBUTE, context.getUriTemplate()).build();
}
Iterable<Tag> tags = this.tagsProvider.tags(request, context.getResponse(), context.getError());
return KeyValues.of(tags, Tag::getKey, Tag::getValue); return KeyValues.of(tags, Tag::getKey, Tag::getValue);
} }
private void mutateClientRequest(ClientRequestObservationContext context) {
// WebClientExchangeTagsProvider relies on a request attribute to get the URI
// template, we need to adapt to that.
ClientRequest clientRequest = ClientRequest.from(context.getRequest())
.attribute(URI_TEMPLATE_ATTRIBUTE, context.getUriTemplate()).build();
context.setRequest(clientRequest);
}
@Override @Override
public KeyValues getHighCardinalityKeyValues(ClientRequestObservationContext context) { public KeyValues getHighCardinalityKeyValues(ClientRequestObservationContext context) {
return KeyValues.empty(); return KeyValues.empty();

View File

@ -29,6 +29,7 @@ import org.springframework.http.HttpStatus;
import org.springframework.web.reactive.function.client.ClientRequest; import org.springframework.web.reactive.function.client.ClientRequest;
import org.springframework.web.reactive.function.client.ClientRequestObservationContext; import org.springframework.web.reactive.function.client.ClientRequestObservationContext;
import org.springframework.web.reactive.function.client.ClientResponse; import org.springframework.web.reactive.function.client.ClientResponse;
import org.springframework.web.reactive.function.client.WebClient;
import static org.assertj.core.api.Assertions.assertThat; import static org.assertj.core.api.Assertions.assertThat;
@ -45,7 +46,8 @@ class ClientObservationConventionAdapterTests {
private ClientObservationConventionAdapter convention = new ClientObservationConventionAdapter(TEST_METRIC_NAME, private ClientObservationConventionAdapter convention = new ClientObservationConventionAdapter(TEST_METRIC_NAME,
new DefaultWebClientExchangeTagsProvider()); new DefaultWebClientExchangeTagsProvider());
private ClientRequest.Builder requestBuilder = ClientRequest.create(HttpMethod.GET, URI.create("/resource/test")); private ClientRequest.Builder requestBuilder = ClientRequest.create(HttpMethod.GET, URI.create("/resource/test"))
.attribute(WebClient.class.getName() + ".uriTemplate", "/resource/{name}");
private ClientResponse response = ClientResponse.create(HttpStatus.OK).body("foo").build(); private ClientResponse response = ClientResponse.create(HttpStatus.OK).body("foo").build();
@ -55,7 +57,6 @@ class ClientObservationConventionAdapterTests {
void setup() { void setup() {
this.context = new ClientRequestObservationContext(); this.context = new ClientRequestObservationContext();
this.context.setCarrier(this.requestBuilder); this.context.setCarrier(this.requestBuilder);
this.context.setRequest(this.requestBuilder.build());
this.context.setResponse(this.response); this.context.setResponse(this.response);
this.context.setUriTemplate("/resource/{name}"); this.context.setUriTemplate("/resource/{name}");
} }
@ -73,6 +74,14 @@ class ClientObservationConventionAdapterTests {
@Test @Test
void shouldPushTagsAsLowCardinalityKeyValues() { void shouldPushTagsAsLowCardinalityKeyValues() {
this.context.setRequest(this.requestBuilder.build());
assertThat(this.convention.getLowCardinalityKeyValues(this.context)).contains(KeyValue.of("status", "200"),
KeyValue.of("outcome", "SUCCESS"), KeyValue.of("uri", "/resource/{name}"),
KeyValue.of("method", "GET"));
}
@Test
void doesNotFailWithEmptyRequest() {
assertThat(this.convention.getLowCardinalityKeyValues(this.context)).contains(KeyValue.of("status", "200"), assertThat(this.convention.getLowCardinalityKeyValues(this.context)).contains(KeyValue.of("status", "200"),
KeyValue.of("outcome", "SUCCESS"), KeyValue.of("uri", "/resource/{name}"), KeyValue.of("outcome", "SUCCESS"), KeyValue.of("uri", "/resource/{name}"),
KeyValue.of("method", "GET")); KeyValue.of("method", "GET"));