Skip to content

Commit

Permalink
Fix IllegalArgumentException in WebClient convention adapter
Browse files Browse the repository at this point in the history
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
  • Loading branch information
bclozel committed Dec 7, 2022
1 parent 87fd27c commit 50be8cb
Show file tree
Hide file tree
Showing 2 changed files with 16 additions and 12 deletions.
Expand Up @@ -55,19 +55,14 @@ public boolean supportsContext(Observation.Context context) {

@Override
public KeyValues getLowCardinalityKeyValues(ClientRequestObservationContext context) {
mutateClientRequest(context);
Iterable<Tag> tags = this.tagsProvider.tags(context.getRequest(), context.getResponse(), context.getError());
ClientRequest request = context.getRequest();
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);
}

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
public KeyValues getHighCardinalityKeyValues(ClientRequestObservationContext context) {
return KeyValues.empty();
Expand Down
Expand Up @@ -29,6 +29,7 @@
import org.springframework.web.reactive.function.client.ClientRequest;
import org.springframework.web.reactive.function.client.ClientRequestObservationContext;
import org.springframework.web.reactive.function.client.ClientResponse;
import org.springframework.web.reactive.function.client.WebClient;

import static org.assertj.core.api.Assertions.assertThat;

Expand All @@ -45,7 +46,8 @@ class ClientObservationConventionAdapterTests {
private ClientObservationConventionAdapter convention = new ClientObservationConventionAdapter(TEST_METRIC_NAME,
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();

Expand All @@ -55,7 +57,6 @@ class ClientObservationConventionAdapterTests {
void setup() {
this.context = new ClientRequestObservationContext();
this.context.setCarrier(this.requestBuilder);
this.context.setRequest(this.requestBuilder.build());
this.context.setResponse(this.response);
this.context.setUriTemplate("/resource/{name}");
}
Expand All @@ -73,6 +74,14 @@ void shouldOnlySupportClientObservationContext() {

@Test
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"),
KeyValue.of("outcome", "SUCCESS"), KeyValue.of("uri", "/resource/{name}"),
KeyValue.of("method", "GET"));
Expand Down

0 comments on commit 50be8cb

Please sign in to comment.