From 50be8cbf9162384af6968edfe5274459464982ca Mon Sep 17 00:00:00 2001 From: Brian Clozel Date: Wed, 7 Dec 2022 13:16:57 +0100 Subject: [PATCH] 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 --- .../ClientObservationConventionAdapter.java | 15 +++++---------- .../ClientObservationConventionAdapterTests.java | 13 +++++++++++-- 2 files changed, 16 insertions(+), 12 deletions(-) diff --git a/spring-boot-project/spring-boot-actuator-autoconfigure/src/main/java/org/springframework/boot/actuate/autoconfigure/observation/web/client/ClientObservationConventionAdapter.java b/spring-boot-project/spring-boot-actuator-autoconfigure/src/main/java/org/springframework/boot/actuate/autoconfigure/observation/web/client/ClientObservationConventionAdapter.java index 6466202b8a..0f3230a5bf 100644 --- a/spring-boot-project/spring-boot-actuator-autoconfigure/src/main/java/org/springframework/boot/actuate/autoconfigure/observation/web/client/ClientObservationConventionAdapter.java +++ b/spring-boot-project/spring-boot-actuator-autoconfigure/src/main/java/org/springframework/boot/actuate/autoconfigure/observation/web/client/ClientObservationConventionAdapter.java @@ -55,19 +55,14 @@ class ClientObservationConventionAdapter implements ClientRequestObservationConv @Override public KeyValues getLowCardinalityKeyValues(ClientRequestObservationContext context) { - mutateClientRequest(context); - Iterable 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 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(); diff --git a/spring-boot-project/spring-boot-actuator-autoconfigure/src/test/java/org/springframework/boot/actuate/autoconfigure/observation/web/client/ClientObservationConventionAdapterTests.java b/spring-boot-project/spring-boot-actuator-autoconfigure/src/test/java/org/springframework/boot/actuate/autoconfigure/observation/web/client/ClientObservationConventionAdapterTests.java index 6740fd4154..6df42c0093 100644 --- a/spring-boot-project/spring-boot-actuator-autoconfigure/src/test/java/org/springframework/boot/actuate/autoconfigure/observation/web/client/ClientObservationConventionAdapterTests.java +++ b/spring-boot-project/spring-boot-actuator-autoconfigure/src/test/java/org/springframework/boot/actuate/autoconfigure/observation/web/client/ClientObservationConventionAdapterTests.java @@ -29,6 +29,7 @@ import org.springframework.http.HttpStatus; 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; @@ -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(); @@ -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}"); } @@ -73,6 +74,14 @@ class ClientObservationConventionAdapterTests { @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"));