From a0ddcd07c885c56e0360434221ae1d4666abc0cb Mon Sep 17 00:00:00 2001 From: Brian Clozel Date: Mon, 12 Sep 2022 11:36:32 +0200 Subject: [PATCH] Instrument RestTemplate for Observability This commit introduces Micrometer as an API dependency to the spring-web module. Micrometer is used here to instrument `RestTemplate` and record `Observation` for HTTP client exchanges. This will replace Spring Boot's `MetricsClientHttpRequestInterceptor` which uses the request interceptor contract for instrumentation. This approach is limited as measurements and tags aren't always precise and overhead is more important than a direct instrumentation. See gh-28341 --- spring-web/spring-web.gradle | 2 + .../observation/ClientHttpObservation.java | 137 +++++++++++++ .../ClientHttpObservationContext.java | 64 ++++++ .../ClientHttpObservationConvention.java | 34 ++++ ...efaultClientHttpObservationConvention.java | 152 ++++++++++++++ .../http/client/observation/package-info.java | 10 + .../web/client/RestTemplate.java | 108 ++++++++-- ...tClientHttpObservationConventionTests.java | 118 +++++++++++ .../client/RestTemplateObservationTests.java | 191 ++++++++++++++++++ 9 files changed, 804 insertions(+), 12 deletions(-) create mode 100644 spring-web/src/main/java/org/springframework/http/client/observation/ClientHttpObservation.java create mode 100644 spring-web/src/main/java/org/springframework/http/client/observation/ClientHttpObservationContext.java create mode 100644 spring-web/src/main/java/org/springframework/http/client/observation/ClientHttpObservationConvention.java create mode 100644 spring-web/src/main/java/org/springframework/http/client/observation/DefaultClientHttpObservationConvention.java create mode 100644 spring-web/src/main/java/org/springframework/http/client/observation/package-info.java create mode 100644 spring-web/src/test/java/org/springframework/http/client/observation/DefaultClientHttpObservationConventionTests.java create mode 100644 spring-web/src/test/java/org/springframework/web/client/RestTemplateObservationTests.java diff --git a/spring-web/spring-web.gradle b/spring-web/spring-web.gradle index 1cd38acf8a..b52b6431d8 100644 --- a/spring-web/spring-web.gradle +++ b/spring-web/spring-web.gradle @@ -6,6 +6,7 @@ apply plugin: "kotlinx-serialization" dependencies { api(project(":spring-beans")) api(project(":spring-core")) + api("io.micrometer:micrometer-observation") compileOnly("io.projectreactor.tools:blockhound") optional(project(":spring-aop")) optional(project(":spring-context")) @@ -74,6 +75,7 @@ dependencies { testImplementation("org.xmlunit:xmlunit-assertj") testImplementation("org.xmlunit:xmlunit-matchers") testImplementation("io.projectreactor.tools:blockhound") + testImplementation("io.micrometer:micrometer-observation-test") testRuntimeOnly("com.sun.mail:jakarta.mail") testRuntimeOnly("com.sun.xml.bind:jaxb-core") testRuntimeOnly("com.sun.xml.bind:jaxb-impl") diff --git a/spring-web/src/main/java/org/springframework/http/client/observation/ClientHttpObservation.java b/spring-web/src/main/java/org/springframework/http/client/observation/ClientHttpObservation.java new file mode 100644 index 0000000000..3315072374 --- /dev/null +++ b/spring-web/src/main/java/org/springframework/http/client/observation/ClientHttpObservation.java @@ -0,0 +1,137 @@ +/* + * Copyright 2002-2022 the original author or authors. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * https://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.springframework.http.client.observation; + +import io.micrometer.common.docs.KeyName; +import io.micrometer.observation.Observation; +import io.micrometer.observation.ObservationConvention; +import io.micrometer.observation.docs.DocumentedObservation; + +import org.springframework.http.client.ClientHttpRequestFactory; + + +/** + * Documented {@link io.micrometer.common.KeyValue KeyValues} for {@link ClientHttpRequestFactory HTTP client observations}. + *

This class is used by automated tools to document KeyValues attached to the HTTP client observations. + * @author Brian Clozel + * @since 6.0 + */ +public enum ClientHttpObservation implements DocumentedObservation { + + /** + * Observation created for a client HTTP exchange. + */ + HTTP_REQUEST { + @Override + public Class> getDefaultConvention() { + return DefaultClientHttpObservationConvention.class; + } + + @Override + public KeyName[] getLowCardinalityKeyNames() { + return LowCardinalityKeyNames.values(); + } + + @Override + public KeyName[] getHighCardinalityKeyNames() { + return HighCardinalityKeyNames.values(); + } + + }; + + public enum LowCardinalityKeyNames implements KeyName { + + /** + * Name of HTTP request method or {@code "None"} if the request could not be created. + */ + METHOD { + @Override + public String asString() { + return "method"; + } + + }, + + /** + * URI template used for HTTP request, or {@code ""} if none was provided. + */ + URI { + @Override + public String asString() { + return "uri"; + } + }, + + /** + * HTTP response raw status code, or {@code "IO_ERROR"} in case of {@code IOException}, + * or {@code "CLIENT_ERROR"} if no response was received. + */ + STATUS { + @Override + public String asString() { + return "status"; + } + }, + + /** + * Name of the exception thrown during the exchange, or {@code "None"} if no exception happened. + */ + EXCEPTION { + @Override + public String asString() { + return "exception"; + } + }, + + /** + * Outcome of the HTTP client exchange. + * @see org.springframework.http.HttpStatus.Series + */ + OUTCOME { + @Override + public String asString() { + return "outcome"; + } + } + + } + + public enum HighCardinalityKeyNames implements KeyName { + + /** + * HTTP request URI. + */ + URI_EXPANDED { + @Override + public String asString() { + return "uri.expanded"; + } + }, + + /** + * Client name derived from the request URI host. + */ + CLIENT_NAME { + @Override + public String asString() { + return "client.name"; + } + } + + } + +} diff --git a/spring-web/src/main/java/org/springframework/http/client/observation/ClientHttpObservationContext.java b/spring-web/src/main/java/org/springframework/http/client/observation/ClientHttpObservationContext.java new file mode 100644 index 0000000000..ff22ee4609 --- /dev/null +++ b/spring-web/src/main/java/org/springframework/http/client/observation/ClientHttpObservationContext.java @@ -0,0 +1,64 @@ +/* + * Copyright 2002-2022 the original author or authors. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * https://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.springframework.http.client.observation; + +import io.micrometer.observation.transport.RequestReplySenderContext; + +import org.springframework.http.client.ClientHttpRequest; +import org.springframework.http.client.ClientHttpRequestFactory; +import org.springframework.http.client.ClientHttpResponse; +import org.springframework.lang.Nullable; + +/** + * Context that holds information for metadata collection + * during the {@link ClientHttpRequestFactory client HTTP} observations. + *

This context also extends {@link RequestReplySenderContext} for propagating tracing + * information with the HTTP client exchange. + * @author Brian Clozel + * @since 6.0 + */ +public class ClientHttpObservationContext extends RequestReplySenderContext { + + @Nullable + private String uriTemplate; + + public ClientHttpObservationContext() { + super(ClientHttpObservationContext::setRequestHeader); + } + + private static void setRequestHeader(@Nullable ClientHttpRequest request, String name, String value) { + if (request != null) { + request.getHeaders().set(name, value); + } + } + + /** + * Return the URI template used for the current client exchange, {@code null} if none was used. + */ + @Nullable + public String getUriTemplate() { + return this.uriTemplate; + } + + /** + * Set the URI template used for the current client exchange. + */ + public void setUriTemplate(@Nullable String uriTemplate) { + this.uriTemplate = uriTemplate; + } + +} diff --git a/spring-web/src/main/java/org/springframework/http/client/observation/ClientHttpObservationConvention.java b/spring-web/src/main/java/org/springframework/http/client/observation/ClientHttpObservationConvention.java new file mode 100644 index 0000000000..171bdc635d --- /dev/null +++ b/spring-web/src/main/java/org/springframework/http/client/observation/ClientHttpObservationConvention.java @@ -0,0 +1,34 @@ +/* + * Copyright 2002-2022 the original author or authors. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * https://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.springframework.http.client.observation; + +import io.micrometer.observation.Observation; +import io.micrometer.observation.ObservationConvention; + +/** + * Interface for an {@link ObservationConvention} related to RestTemplate HTTP exchanges. + * @author Brian Clozel + * @since 6.0 + */ +public interface ClientHttpObservationConvention extends ObservationConvention { + + @Override + default boolean supportsContext(Observation.Context context) { + return context instanceof ClientHttpObservationContext; + } + +} diff --git a/spring-web/src/main/java/org/springframework/http/client/observation/DefaultClientHttpObservationConvention.java b/spring-web/src/main/java/org/springframework/http/client/observation/DefaultClientHttpObservationConvention.java new file mode 100644 index 0000000000..026ed5550d --- /dev/null +++ b/spring-web/src/main/java/org/springframework/http/client/observation/DefaultClientHttpObservationConvention.java @@ -0,0 +1,152 @@ +/* + * Copyright 2002-2022 the original author or authors. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * https://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.springframework.http.client.observation; + +import java.io.IOException; + +import io.micrometer.common.KeyValue; +import io.micrometer.common.KeyValues; + +import org.springframework.http.HttpStatus; +import org.springframework.http.client.ClientHttpResponse; +import org.springframework.lang.Nullable; +import org.springframework.util.StringUtils; + +/** + * Default implementation for a {@link ClientHttpObservationConvention}, + * extracting information from the {@link ClientHttpObservationContext}. + * + * @author Brian Clozel + * @since 6.0 + */ +public class DefaultClientHttpObservationConvention implements ClientHttpObservationConvention { + + private static final String DEFAULT_NAME = "http.client.requests"; + + private static final KeyValue URI_NONE = KeyValue.of(ClientHttpObservation.LowCardinalityKeyNames.URI, "none"); + + private static final KeyValue METHOD_NONE = KeyValue.of(ClientHttpObservation.LowCardinalityKeyNames.METHOD, "none"); + + private static final KeyValue EXCEPTION_NONE = KeyValue.of(ClientHttpObservation.LowCardinalityKeyNames.EXCEPTION, "none"); + + private static final KeyValue OUTCOME_UNKNOWN = KeyValue.of(ClientHttpObservation.LowCardinalityKeyNames.OUTCOME, "UNKNOWN"); + + private static final KeyValue URI_EXPANDED_NONE = KeyValue.of(ClientHttpObservation.HighCardinalityKeyNames.URI_EXPANDED, "none"); + + private final String name; + + /** + * Create a convention with the default name {@code "http.client.requests"}. + */ + public DefaultClientHttpObservationConvention() { + this(DEFAULT_NAME); + } + + /** + * Create a convention with a custom name. + * @param name the observation name + */ + public DefaultClientHttpObservationConvention(String name) { + this.name = name; + } + + @Override + public String getName() { + return this.name; + } + + @Override + public KeyValues getLowCardinalityKeyValues(ClientHttpObservationContext context) { + return KeyValues.of(uri(context), method(context), status(context), exception(context), outcome(context)); + } + + protected KeyValue uri(ClientHttpObservationContext context) { + if (context.getUriTemplate() != null) { + return KeyValue.of(ClientHttpObservation.LowCardinalityKeyNames.URI, context.getUriTemplate()); + } + return URI_NONE; + } + + protected KeyValue method(ClientHttpObservationContext context) { + if (context.getCarrier() != null) { + return KeyValue.of(ClientHttpObservation.LowCardinalityKeyNames.METHOD, context.getCarrier().getMethod().name()); + } + else { + return METHOD_NONE; + } + } + + protected KeyValue status(ClientHttpObservationContext context) { + return KeyValue.of(ClientHttpObservation.LowCardinalityKeyNames.STATUS, getStatusMessage(context.getResponse())); + } + + private String getStatusMessage(@Nullable ClientHttpResponse response) { + try { + if (response == null) { + return "CLIENT_ERROR"; + } + return String.valueOf(response.getStatusCode().value()); + } + catch (IOException ex) { + return "IO_ERROR"; + } + } + + protected KeyValue exception(ClientHttpObservationContext context) { + return context.getError().map(exception -> { + String simpleName = exception.getClass().getSimpleName(); + return KeyValue.of(ClientHttpObservation.LowCardinalityKeyNames.EXCEPTION, + StringUtils.hasText(simpleName) ? simpleName : exception.getClass().getName()); + }).orElse(EXCEPTION_NONE); + } + + protected static KeyValue outcome(ClientHttpObservationContext context) { + try { + if (context.getResponse() != null) { + HttpStatus status = HttpStatus.resolve(context.getResponse().getStatusCode().value()); + if (status != null) { + return KeyValue.of(ClientHttpObservation.LowCardinalityKeyNames.OUTCOME, status.series().name()); + } + } + } + catch (IOException ex) { + // Continue + } + return OUTCOME_UNKNOWN; + } + + @Override + public KeyValues getHighCardinalityKeyValues(ClientHttpObservationContext context) { + return KeyValues.of(requestUri(context), clientName(context)); + } + + protected KeyValue requestUri(ClientHttpObservationContext context) { + if (context.getCarrier() != null) { + return KeyValue.of(ClientHttpObservation.HighCardinalityKeyNames.URI_EXPANDED, context.getCarrier().getURI().toASCIIString()); + } + return URI_EXPANDED_NONE; + } + + protected KeyValue clientName(ClientHttpObservationContext context) { + String host = "none"; + if (context.getCarrier() != null && context.getCarrier().getURI().getHost() != null) { + host = context.getCarrier().getURI().getHost(); + } + return KeyValue.of(ClientHttpObservation.HighCardinalityKeyNames.CLIENT_NAME, host); + } + +} diff --git a/spring-web/src/main/java/org/springframework/http/client/observation/package-info.java b/spring-web/src/main/java/org/springframework/http/client/observation/package-info.java new file mode 100644 index 0000000000..6f3bbf9b96 --- /dev/null +++ b/spring-web/src/main/java/org/springframework/http/client/observation/package-info.java @@ -0,0 +1,10 @@ +/** + * This package provides support for client HTTP + * {@link io.micrometer.observation.Observation}. + */ +@NonNullApi +@NonNullFields +package org.springframework.http.client.observation; + +import org.springframework.lang.NonNullApi; +import org.springframework.lang.NonNullFields; diff --git a/spring-web/src/main/java/org/springframework/web/client/RestTemplate.java b/spring-web/src/main/java/org/springframework/web/client/RestTemplate.java index 24bb8ce96e..6660b33fec 100644 --- a/spring-web/src/main/java/org/springframework/web/client/RestTemplate.java +++ b/spring-web/src/main/java/org/springframework/web/client/RestTemplate.java @@ -28,6 +28,9 @@ import java.util.Set; import java.util.stream.Collectors; import java.util.stream.Stream; +import io.micrometer.observation.Observation; +import io.micrometer.observation.ObservationRegistry; + import org.springframework.core.ParameterizedTypeReference; import org.springframework.core.SpringProperties; import org.springframework.http.HttpEntity; @@ -40,6 +43,10 @@ import org.springframework.http.ResponseEntity; import org.springframework.http.client.ClientHttpRequest; import org.springframework.http.client.ClientHttpRequestFactory; import org.springframework.http.client.ClientHttpResponse; +import org.springframework.http.client.observation.ClientHttpObservation; +import org.springframework.http.client.observation.ClientHttpObservationContext; +import org.springframework.http.client.observation.ClientHttpObservationConvention; +import org.springframework.http.client.observation.DefaultClientHttpObservationConvention; import org.springframework.http.client.support.InterceptingHttpAccessor; import org.springframework.http.converter.ByteArrayHttpMessageConverter; import org.springframework.http.converter.GenericHttpMessageConverter; @@ -119,6 +126,8 @@ public class RestTemplate extends InterceptingHttpAccessor implements RestOperat private static final boolean kotlinSerializationJsonPresent; + private static final ClientHttpObservationConvention DEFAULT_OBSERVATION_CONVENTION = new DefaultClientHttpObservationConvention(); + static { ClassLoader classLoader = RestTemplate.class.getClassLoader(); romePresent = ClassUtils.isPresent("com.rometools.rome.feed.WireFeed", classLoader); @@ -142,6 +151,11 @@ public class RestTemplate extends InterceptingHttpAccessor implements RestOperat private final ResponseExtractor headersExtractor = new HeadersExtractor(); + private ObservationRegistry observationRegistry = ObservationRegistry.NOOP; + + @Nullable + private ClientHttpObservationConvention observationConvention; + /** * Create a new instance of the {@link RestTemplate} using default settings. @@ -323,6 +337,30 @@ public class RestTemplate extends InterceptingHttpAccessor implements RestOperat return this.uriTemplateHandler; } + /** + * Configure an {@link ObservationRegistry} for collecting spans and metrics + * for request execution. By default, {@link Observation} are No-Ops. + * @param observationRegistry the observation registry to use + * @since 6.0 + */ + public void setObservationRegistry(ObservationRegistry observationRegistry) { + Assert.notNull(observationRegistry, "observationRegistry must not be null"); + this.observationRegistry = observationRegistry; + } + + /** + * Configure an {@link Observation.ObservationConvention} that sets the name of the + * {@link Observation observation} as well as its {@link io.micrometer.common.KeyValues} + * extracted from the {@link ClientHttpObservationContext}. + * If none set, the {@link DefaultClientHttpObservationConvention default convention} will be used. + * @param observationConvention the observation convention to use + * @since 6.0 + * @see #setObservationRegistry(ObservationRegistry) + */ + public void setObservationConvention(ClientHttpObservationConvention observationConvention) { + Assert.notNull(observationConvention, "observationConvention must not be null"); + this.observationConvention = observationConvention; + } // GET @@ -658,7 +696,7 @@ public class RestTemplate extends InterceptingHttpAccessor implements RestOperat RequestCallback requestCallback = httpEntityCallback(entity, responseType); ResponseExtractor> responseExtractor = responseEntityExtractor(responseType); - return nonNull(doExecute(resolveUrl(entity), entity.getMethod(), requestCallback, responseExtractor)); + return nonNull(doExecute(resolveUrl(entity), resolveUriTemplate(entity), entity.getMethod(), requestCallback, responseExtractor)); } @Override @@ -668,7 +706,7 @@ public class RestTemplate extends InterceptingHttpAccessor implements RestOperat Type type = responseType.getType(); RequestCallback requestCallback = httpEntityCallback(entity, type); ResponseExtractor> responseExtractor = responseEntityExtractor(type); - return nonNull(doExecute(resolveUrl(entity), entity.getMethod(), requestCallback, responseExtractor)); + return nonNull(doExecute(resolveUrl(entity), resolveUriTemplate(entity), entity.getMethod(), requestCallback, responseExtractor)); } private URI resolveUrl(RequestEntity entity) { @@ -689,6 +727,16 @@ public class RestTemplate extends InterceptingHttpAccessor implements RestOperat } } + @Nullable + private String resolveUriTemplate(RequestEntity entity) { + if (entity instanceof RequestEntity.UriTemplateRequestEntity templated) { + return templated.getUriTemplate(); + } + else { + return null; + } + } + // General execution @@ -705,11 +753,11 @@ public class RestTemplate extends InterceptingHttpAccessor implements RestOperat */ @Override @Nullable - public T execute(String url, HttpMethod method, @Nullable RequestCallback requestCallback, + public T execute(String uriTemplate, HttpMethod method, @Nullable RequestCallback requestCallback, @Nullable ResponseExtractor responseExtractor, Object... uriVariables) throws RestClientException { - URI expanded = getUriTemplateHandler().expand(url, uriVariables); - return doExecute(expanded, method, requestCallback, responseExtractor); + URI url = getUriTemplateHandler().expand(uriTemplate, uriVariables); + return doExecute(url, uriTemplate, method, requestCallback, responseExtractor); } /** @@ -725,12 +773,12 @@ public class RestTemplate extends InterceptingHttpAccessor implements RestOperat */ @Override @Nullable - public T execute(String url, HttpMethod method, @Nullable RequestCallback requestCallback, + public T execute(String uriTemplate, HttpMethod method, @Nullable RequestCallback requestCallback, @Nullable ResponseExtractor responseExtractor, Map uriVariables) throws RestClientException { - URI expanded = getUriTemplateHandler().expand(url, uriVariables); - return doExecute(expanded, method, requestCallback, responseExtractor); + URI url = getUriTemplateHandler().expand(uriTemplate, uriVariables); + return doExecute(url, uriTemplate, method, requestCallback, responseExtractor); } /** @@ -749,7 +797,7 @@ public class RestTemplate extends InterceptingHttpAccessor implements RestOperat public T execute(URI url, HttpMethod method, @Nullable RequestCallback requestCallback, @Nullable ResponseExtractor responseExtractor) throws RestClientException { - return doExecute(url, method, requestCallback, responseExtractor); + return doExecute(url, null, method, requestCallback, responseExtractor); } /** @@ -761,20 +809,49 @@ public class RestTemplate extends InterceptingHttpAccessor implements RestOperat * @param requestCallback object that prepares the request (can be {@code null}) * @param responseExtractor object that extracts the return value from the response (can be {@code null}) * @return an arbitrary object, as returned by the {@link ResponseExtractor} + * @deprecated in favor of {@link #doExecute(URI, String, HttpMethod, RequestCallback, ResponseExtractor)} */ @Nullable + @Deprecated protected T doExecute(URI url, @Nullable HttpMethod method, @Nullable RequestCallback requestCallback, @Nullable ResponseExtractor responseExtractor) throws RestClientException { - Assert.notNull(url, "URI is required"); + return doExecute(url, null, method, requestCallback, responseExtractor); + } + + /** + * Execute the given method on the provided URI. + *

The {@link ClientHttpRequest} is processed using the {@link RequestCallback}; + * the response with the {@link ResponseExtractor}. + * @param url the fully-expanded URL to connect to + * @param uriTemplate the URI template that was used for creating the expanded URL + * @param method the HTTP method to execute (GET, POST, etc.) + * @param requestCallback object that prepares the request (can be {@code null}) + * @param responseExtractor object that extracts the return value from the response (can be {@code null}) + * @return an arbitrary object, as returned by the {@link ResponseExtractor} + */ + @Nullable + @SuppressWarnings("try") + protected T doExecute(URI url, @Nullable String uriTemplate, @Nullable HttpMethod method, @Nullable RequestCallback requestCallback, + @Nullable ResponseExtractor responseExtractor) throws RestClientException { + + Assert.notNull(url, "url is required"); Assert.notNull(method, "HttpMethod is required"); + ClientHttpObservationContext observationContext = new ClientHttpObservationContext(); + Observation observation = ClientHttpObservation.HTTP_REQUEST.observation(this.observationConvention, + DEFAULT_OBSERVATION_CONVENTION, observationContext, this.observationRegistry).start(); + observationContext.setUriTemplate(uriTemplate); ClientHttpResponse response = null; try { ClientHttpRequest request = createRequest(url, method); + observationContext.setCarrier(request); if (requestCallback != null) { requestCallback.doWithRequest(request); } - response = request.execute(); + try (Observation.Scope scope = observation.openScope()) { + response = request.execute(); + } + observationContext.setResponse(response); handleResponse(url, method, response); return (responseExtractor != null ? responseExtractor.extractData(response) : null); } @@ -782,13 +859,20 @@ public class RestTemplate extends InterceptingHttpAccessor implements RestOperat String resource = url.toString(); String query = url.getRawQuery(); resource = (query != null ? resource.substring(0, resource.indexOf('?')) : resource); - throw new ResourceAccessException("I/O error on " + method.name() + + ResourceAccessException exception = new ResourceAccessException("I/O error on " + method.name() + " request for \"" + resource + "\": " + ex.getMessage(), ex); + observation.error(exception); + throw exception; + } + catch (RestClientException exc) { + observation.error(exc); + throw exc; } finally { if (response != null) { response.close(); } + observation.stop(); } } diff --git a/spring-web/src/test/java/org/springframework/http/client/observation/DefaultClientHttpObservationConventionTests.java b/spring-web/src/test/java/org/springframework/http/client/observation/DefaultClientHttpObservationConventionTests.java new file mode 100644 index 0000000000..b31dc375ab --- /dev/null +++ b/spring-web/src/test/java/org/springframework/http/client/observation/DefaultClientHttpObservationConventionTests.java @@ -0,0 +1,118 @@ +/* + * Copyright 2002-2022 the original author or authors. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * https://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.springframework.http.client.observation; + + +import java.io.IOException; + +import io.micrometer.common.KeyValue; +import io.micrometer.observation.Observation; +import org.junit.jupiter.api.Test; + +import org.springframework.http.HttpMethod; +import org.springframework.http.client.ClientHttpRequest; +import org.springframework.http.client.ClientHttpResponse; +import org.springframework.web.testfixture.http.client.MockClientHttpRequest; +import org.springframework.web.testfixture.http.client.MockClientHttpResponse; + +import static org.assertj.core.api.Assertions.assertThat; +import static org.mockito.BDDMockito.given; +import static org.mockito.Mockito.mock; + +/** + * Tests for {@link DefaultClientHttpObservationConvention}. + * + * @author Brian Clozel + */ +class DefaultClientHttpObservationConventionTests { + + private final DefaultClientHttpObservationConvention observationConvention = new DefaultClientHttpObservationConvention(); + + @Test + void supportsOnlyClientHttpObservationContext() { + assertThat(this.observationConvention.supportsContext(new ClientHttpObservationContext())).isTrue(); + assertThat(this.observationConvention.supportsContext(new Observation.Context())).isFalse(); + } + + @Test + void addsKeyValuesForNullExchange() { + ClientHttpObservationContext context = new ClientHttpObservationContext(); + assertThat(this.observationConvention.getLowCardinalityKeyValues(context)).hasSize(5) + .contains(KeyValue.of("method", "none"), KeyValue.of("uri", "none"), KeyValue.of("status", "CLIENT_ERROR"), + KeyValue.of("exception", "none"), KeyValue.of("outcome", "UNKNOWN")); + assertThat(this.observationConvention.getHighCardinalityKeyValues(context)).hasSize(2) + .contains(KeyValue.of("client.name", "none"), KeyValue.of("uri.expanded", "none")); + } + + @Test + void addsKeyValuesForExchangeWithException() { + ClientHttpObservationContext context = new ClientHttpObservationContext(); + context.setError(new IllegalStateException("Could not create client request")); + assertThat(this.observationConvention.getLowCardinalityKeyValues(context)).hasSize(5) + .contains(KeyValue.of("method", "none"), KeyValue.of("uri", "none"), KeyValue.of("status", "CLIENT_ERROR"), + KeyValue.of("exception", "IllegalStateException"), KeyValue.of("outcome", "UNKNOWN")); + assertThat(this.observationConvention.getHighCardinalityKeyValues(context)).hasSize(2) + .contains(KeyValue.of("client.name", "none"), KeyValue.of("uri.expanded", "none")); + } + + @Test + void addsKeyValuesForRequestWithUriTemplate() { + ClientHttpObservationContext context = createContext( + new MockClientHttpRequest(HttpMethod.GET, "/resource/{id}", 42), new MockClientHttpResponse()); + context.setUriTemplate("/resource/{id}"); + assertThat(this.observationConvention.getLowCardinalityKeyValues(context)) + .contains(KeyValue.of("exception", "none"), KeyValue.of("method", "GET"), KeyValue.of("uri", "/resource/{id}"), + KeyValue.of("status", "200"), KeyValue.of("outcome", "SUCCESSFUL")); + assertThat(this.observationConvention.getHighCardinalityKeyValues(context)).hasSize(2) + .contains(KeyValue.of("client.name", "none"), KeyValue.of("uri.expanded", "/resource/42")); + } + + @Test + void addsKeyValuesForRequestWithoutUriTemplate() { + ClientHttpObservationContext context = createContext( + new MockClientHttpRequest(HttpMethod.GET, "/resource/42"), new MockClientHttpResponse()); + assertThat(this.observationConvention.getLowCardinalityKeyValues(context)) + .contains(KeyValue.of("method", "GET"), KeyValue.of("uri", "none")); + assertThat(this.observationConvention.getHighCardinalityKeyValues(context)).hasSize(2) + .contains(KeyValue.of("uri.expanded", "/resource/42")); + } + + @Test + void addsClientNameForRequestWithHost() { + ClientHttpObservationContext context = createContext( + new MockClientHttpRequest(HttpMethod.GET, "https://localhost:8080/resource/42"), + new MockClientHttpResponse()); + assertThat(this.observationConvention.getHighCardinalityKeyValues(context)).contains(KeyValue.of("client.name", "localhost")); + } + + @Test + void addsKeyValueForNonResolvableStatus() throws Exception { + ClientHttpObservationContext context = new ClientHttpObservationContext(); + ClientHttpResponse response = mock(ClientHttpResponse.class); + context.setResponse(response); + given(response.getStatusCode()).willThrow(new IOException("test error")); + assertThat(this.observationConvention.getLowCardinalityKeyValues(context)).contains(KeyValue.of("status", "IO_ERROR")); + } + + private ClientHttpObservationContext createContext(ClientHttpRequest request, ClientHttpResponse response) { + ClientHttpObservationContext context = new ClientHttpObservationContext(); + context.setCarrier(request); + context.setResponse(response); + return context; + } + +} diff --git a/spring-web/src/test/java/org/springframework/web/client/RestTemplateObservationTests.java b/spring-web/src/test/java/org/springframework/web/client/RestTemplateObservationTests.java new file mode 100644 index 0000000000..10b5eab75f --- /dev/null +++ b/spring-web/src/test/java/org/springframework/web/client/RestTemplateObservationTests.java @@ -0,0 +1,191 @@ +/* + * Copyright 2002-2022 the original author or authors. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * https://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.springframework.web.client; + +import java.io.ByteArrayInputStream; +import java.io.IOException; +import java.net.URI; +import java.util.Collections; +import java.util.List; +import java.util.Map; + +import io.micrometer.observation.tck.TestObservationRegistry; +import io.micrometer.observation.tck.TestObservationRegistryAssert; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; +import org.mockito.BDDMockito; + +import org.springframework.http.HttpHeaders; +import org.springframework.http.HttpInputMessage; +import org.springframework.http.HttpMethod; +import org.springframework.http.HttpStatus; +import org.springframework.http.MediaType; +import org.springframework.http.client.ClientHttpRequest; +import org.springframework.http.client.ClientHttpRequestFactory; +import org.springframework.http.client.ClientHttpResponse; +import org.springframework.http.converter.HttpMessageConverter; + +import static org.assertj.core.api.Assertions.assertThatExceptionOfType; +import static org.mockito.ArgumentMatchers.any; +import static org.mockito.ArgumentMatchers.eq; +import static org.mockito.BDDMockito.given; +import static org.mockito.Mockito.mock; +import static org.springframework.http.HttpMethod.GET; + +/** + * Tests for the client HTTP observations with {@link RestTemplate}. + * @author Brian Clozel + */ +public class RestTemplateObservationTests { + + + private final TestObservationRegistry observationRegistry = TestObservationRegistry.create(); + + private final ClientHttpRequestFactory requestFactory = mock(ClientHttpRequestFactory.class); + + private final ClientHttpRequest request = mock(ClientHttpRequest.class); + + private final ClientHttpResponse response = mock(ClientHttpResponse.class); + + private final ResponseErrorHandler errorHandler = mock(ResponseErrorHandler.class); + + @SuppressWarnings("unchecked") + private final HttpMessageConverter converter = mock(HttpMessageConverter.class); + + private final RestTemplate template = new RestTemplate(List.of(converter)); + + + @BeforeEach + void setupEach() { + this.template.setRequestFactory(this.requestFactory); + this.template.setErrorHandler(this.errorHandler); + this.template.setObservationRegistry(this.observationRegistry); + } + + @Test + void executeVarArgsAddsUriTemplateAsKeyValue() throws Exception { + mockSentRequest(GET, "https://example.com/hotels/42/bookings/21"); + mockResponseStatus(HttpStatus.OK); + + template.execute("https://example.com/hotels/{hotel}/bookings/{booking}", GET, + null, null, "42", "21"); + + assertThatHttpObservation().hasLowCardinalityKeyValue("uri", "https://example.com/hotels/{hotel}/bookings/{booking}"); + } + + @Test + void executeArgsMapAddsUriTemplateAsKeyValue() throws Exception { + mockSentRequest(GET, "https://example.com/hotels/42/bookings/21"); + mockResponseStatus(HttpStatus.OK); + + Map vars = Map.of("hotel", "42", "booking", "21"); + + template.execute("https://example.com/hotels/{hotel}/bookings/{booking}", GET, + null, null, vars); + + assertThatHttpObservation().hasLowCardinalityKeyValue("uri", "https://example.com/hotels/{hotel}/bookings/{booking}"); + } + + + @Test + void executeAddsSucessAsOutcome() throws Exception { + mockSentRequest(GET, "https://example.org"); + mockResponseStatus(HttpStatus.OK); + mockResponseBody("Hello World", MediaType.TEXT_PLAIN); + + template.execute("https://example.org", GET, null, null); + + assertThatHttpObservation().hasLowCardinalityKeyValue("outcome", "SUCCESSFUL"); + } + + @Test + void executeAddsServerErrorAsOutcome() throws Exception { + String url = "https://example.org"; + mockSentRequest(GET, url); + mockResponseStatus(HttpStatus.INTERNAL_SERVER_ERROR); + BDDMockito.willThrow(new HttpServerErrorException(HttpStatus.INTERNAL_SERVER_ERROR)) + .given(errorHandler).handleError(new URI(url), GET, response); + + assertThatExceptionOfType(HttpServerErrorException.class).isThrownBy(() -> + template.execute(url, GET, null, null)); + + assertThatHttpObservation().hasLowCardinalityKeyValue("outcome", "SERVER_ERROR"); + } + + @Test + void executeAddsExceptionAsKeyValue() throws Exception { + mockSentRequest(GET, "https://example.org/resource"); + mockResponseStatus(HttpStatus.OK); + + given(converter.canRead(String.class, null)).willReturn(true); + MediaType supportedMediaType = new MediaType("test", "supported"); + given(converter.getSupportedMediaTypes()).willReturn(Collections.singletonList(supportedMediaType)); + MediaType other = new MediaType("test", "other"); + mockResponseBody("Test Body", other); + given(converter.canRead(String.class, other)).willReturn(false); + + assertThatExceptionOfType(RestClientException.class).isThrownBy(() -> + template.getForObject("https://example.org/{p}", String.class, "resource")); + assertThatHttpObservation().hasLowCardinalityKeyValue("exception", "UnknownContentTypeException"); + } + + @Test + void executeWithIoExceptionAddsUnknownOutcome() throws Exception { + String url = "https://example.org/resource"; + mockSentRequest(GET, url); + given(request.execute()).willThrow(new IOException("Socket failure")); + + assertThatExceptionOfType(ResourceAccessException.class).isThrownBy(() -> + template.getForObject(url, String.class)); + assertThatHttpObservation().hasLowCardinalityKeyValue("outcome", "UNKNOWN"); + } + + + private void mockSentRequest(HttpMethod method, String uri) throws Exception { + mockSentRequest(method, uri, new HttpHeaders()); + } + + private void mockSentRequest(HttpMethod method, String uri, HttpHeaders requestHeaders) throws Exception { + given(requestFactory.createRequest(new URI(uri), method)).willReturn(request); + given(request.getHeaders()).willReturn(requestHeaders); + given(request.getMethod()).willReturn(method); + given(request.getURI()).willReturn(URI.create(uri)); + } + + private void mockResponseStatus(HttpStatus responseStatus) throws Exception { + given(request.execute()).willReturn(response); + given(errorHandler.hasError(response)).willReturn(responseStatus.isError()); + given(response.getStatusCode()).willReturn(responseStatus); + given(response.getStatusText()).willReturn(responseStatus.getReasonPhrase()); + } + + private void mockResponseBody(String expectedBody, MediaType mediaType) throws Exception { + HttpHeaders responseHeaders = new HttpHeaders(); + responseHeaders.setContentType(mediaType); + responseHeaders.setContentLength(expectedBody.length()); + given(response.getHeaders()).willReturn(responseHeaders); + given(response.getBody()).willReturn(new ByteArrayInputStream(expectedBody.getBytes())); + given(converter.read(eq(String.class), any(HttpInputMessage.class))).willReturn(expectedBody); + } + + + private TestObservationRegistryAssert.TestObservationRegistryAssertReturningObservationContextAssert assertThatHttpObservation() { + return TestObservationRegistryAssert.assertThat(this.observationRegistry) + .hasObservationWithNameEqualTo("http.client.requests").that(); + } + +}