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 index 33150723743..ae4fa33ea24 100644 --- 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 @@ -56,7 +56,7 @@ public enum ClientHttpObservation implements DocumentedObservation { public enum LowCardinalityKeyNames implements KeyName { /** - * Name of HTTP request method or {@code "None"} if the request could not be created. + * Name of HTTP request method or {@code "none"} if the request could not be created. */ METHOD { @Override @@ -67,7 +67,7 @@ public enum ClientHttpObservation implements DocumentedObservation { }, /** - * URI template used for HTTP request, or {@code ""} if none was provided. + * URI template used for HTTP request, or {@code "none"} if none was provided. */ URI { @Override @@ -88,7 +88,7 @@ public enum ClientHttpObservation implements DocumentedObservation { }, /** - * Name of the exception thrown during the exchange, or {@code "None"} if no exception happened. + * Name of the exception thrown during the exchange, or {@code "none"} if no exception happened. */ EXCEPTION { @Override 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 index c1c40d7e32b..478b84bcbf7 100644 --- 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 @@ -23,7 +23,6 @@ import io.micrometer.common.KeyValues; import org.springframework.http.client.ClientHttpResponse; import org.springframework.http.observation.HttpOutcome; -import org.springframework.lang.Nullable; import org.springframework.util.StringUtils; /** @@ -41,10 +40,16 @@ public class DefaultClientHttpObservationConvention implements ClientHttpObserva private static final KeyValue METHOD_NONE = KeyValue.of(ClientHttpObservation.LowCardinalityKeyNames.METHOD, "none"); + private static final KeyValue STATUS_IO_ERROR = KeyValue.of(ClientHttpObservation.LowCardinalityKeyNames.STATUS, "IO_ERROR"); + + private static final KeyValue STATUS_CLIENT_ERROR = KeyValue.of(ClientHttpObservation.LowCardinalityKeyNames.STATUS, "CLIENT_ERROR"); + private static final KeyValue EXCEPTION_NONE = KeyValue.of(ClientHttpObservation.LowCardinalityKeyNames.EXCEPTION, "none"); private static final KeyValue URI_EXPANDED_NONE = KeyValue.of(ClientHttpObservation.HighCardinalityKeyNames.URI_EXPANDED, "none"); + private static final KeyValue CLIENT_NAME_NONE = KeyValue.of(ClientHttpObservation.HighCardinalityKeyNames.CLIENT_NAME, "none"); + private final String name; /** @@ -94,18 +99,15 @@ public class DefaultClientHttpObservationConvention implements ClientHttpObserva } protected KeyValue status(ClientHttpObservationContext context) { - return KeyValue.of(ClientHttpObservation.LowCardinalityKeyNames.STATUS, getStatusMessage(context.getResponse())); - } - - private String getStatusMessage(@Nullable ClientHttpResponse response) { + ClientHttpResponse response = context.getResponse(); + if (response == null) { + return STATUS_CLIENT_ERROR; + } try { - if (response == null) { - return "CLIENT_ERROR"; - } - return String.valueOf(response.getStatusCode().value()); + return KeyValue.of(ClientHttpObservation.LowCardinalityKeyNames.STATUS, String.valueOf(response.getStatusCode().value())); } catch (IOException ex) { - return "IO_ERROR"; + return STATUS_IO_ERROR; } } @@ -118,14 +120,14 @@ public class DefaultClientHttpObservationConvention implements ClientHttpObserva } protected static KeyValue outcome(ClientHttpObservationContext context) { - try { - if (context.getResponse() != null) { + if (context.getResponse() != null) { + try { HttpOutcome httpOutcome = HttpOutcome.forStatus(context.getResponse().getStatusCode()); return httpOutcome.asKeyValue(); } - } - catch (IOException ex) { - // Continue + catch (IOException ex) { + // Continue + } } return HttpOutcome.UNKNOWN.asKeyValue(); } @@ -143,11 +145,10 @@ public class DefaultClientHttpObservationConvention implements ClientHttpObserva } 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, context.getCarrier().getURI().getHost()); } - return KeyValue.of(ClientHttpObservation.HighCardinalityKeyNames.CLIENT_NAME, host); + return CLIENT_NAME_NONE; } } diff --git a/spring-web/src/main/java/org/springframework/http/observation/HttpOutcome.java b/spring-web/src/main/java/org/springframework/http/observation/HttpOutcome.java index fd66bf94a2a..c9440669a8f 100644 --- a/spring-web/src/main/java/org/springframework/http/observation/HttpOutcome.java +++ b/spring-web/src/main/java/org/springframework/http/observation/HttpOutcome.java @@ -22,12 +22,12 @@ import org.springframework.http.HttpStatusCode; /** * The outcome of an HTTP request. - *

Used as the {@code "outcome} {@link io.micrometer.common.KeyValue} + *

Used as the {@code "outcome"} {@link io.micrometer.common.KeyValue} * for HTTP {@link io.micrometer.observation.Observation observations}. * * @author Brian Clozel * @author Andy Wilkinson - * @since 6.0.0 + * @since 6.0 */ public enum HttpOutcome { diff --git a/spring-web/src/main/java/org/springframework/web/client/RestOperations.java b/spring-web/src/main/java/org/springframework/web/client/RestOperations.java index 0217c4f9bbd..214b19b2a0d 100644 --- a/spring-web/src/main/java/org/springframework/web/client/RestOperations.java +++ b/spring-web/src/main/java/org/springframework/web/client/RestOperations.java @@ -653,7 +653,7 @@ public interface RestOperations { * Execute the HTTP method to the given URI template, preparing the request with the * {@link RequestCallback}, and reading the response with a {@link ResponseExtractor}. *

URI Template variables are expanded using the given URI variables, if any. - * @param url the URL + * @param uriTemplate the URI template * @param method the HTTP method (GET, POST, etc) * @param requestCallback object that prepares the request * @param responseExtractor object that extracts the return value from the response @@ -661,7 +661,7 @@ public interface RestOperations { * @return an arbitrary object, as returned by the {@link ResponseExtractor} */ @Nullable - T execute(String url, HttpMethod method, @Nullable RequestCallback requestCallback, + T execute(String uriTemplate, HttpMethod method, @Nullable RequestCallback requestCallback, @Nullable ResponseExtractor responseExtractor, Object... uriVariables) throws RestClientException; @@ -669,7 +669,7 @@ public interface RestOperations { * Execute the HTTP method to the given URI template, preparing the request with the * {@link RequestCallback}, and reading the response with a {@link ResponseExtractor}. *

URI Template variables are expanded using the given URI variables map. - * @param url the URL + * @param uriTemplate the URI template * @param method the HTTP method (GET, POST, etc) * @param requestCallback object that prepares the request * @param responseExtractor object that extracts the return value from the response @@ -677,7 +677,7 @@ public interface RestOperations { * @return an arbitrary object, as returned by the {@link ResponseExtractor} */ @Nullable - T execute(String url, HttpMethod method, @Nullable RequestCallback requestCallback, + T execute(String uriTemplate, HttpMethod method, @Nullable RequestCallback requestCallback, @Nullable ResponseExtractor responseExtractor, Map uriVariables) throws RestClientException; 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 a5d4e93ac6a..e5d5686ed8a 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 @@ -340,7 +340,7 @@ public class RestTemplate extends InterceptingHttpAccessor implements RestOperat /** * Configure an {@link ObservationRegistry} for collecting spans and metrics - * for request execution. By default, {@link Observation} are No-Ops. + * for request execution. By default, {@link Observation observations} are no-ops. * @param observationRegistry the observation registry to use * @since 6.0 */ @@ -830,6 +830,7 @@ 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} + * @since 6.0 */ @Nullable @SuppressWarnings("try") diff --git a/spring-web/src/main/java/org/springframework/web/observation/DefaultHttpRequestsObservationConvention.java b/spring-web/src/main/java/org/springframework/web/observation/DefaultHttpRequestsObservationConvention.java index 43360b84289..b4364ca4f01 100644 --- a/spring-web/src/main/java/org/springframework/web/observation/DefaultHttpRequestsObservationConvention.java +++ b/spring-web/src/main/java/org/springframework/web/observation/DefaultHttpRequestsObservationConvention.java @@ -22,6 +22,7 @@ import io.micrometer.common.KeyValues; import org.springframework.http.HttpStatus; import org.springframework.http.HttpStatusCode; import org.springframework.http.observation.HttpOutcome; +import org.springframework.util.StringUtils; /** * Default {@link HttpRequestsObservationConvention}. @@ -118,9 +119,12 @@ public class DefaultHttpRequestsObservationConvention implements HttpRequestsObs } protected KeyValue exception(HttpRequestsObservationContext context) { - return context.getError().map(throwable -> - KeyValue.of(HttpRequestsObservation.LowCardinalityKeyNames.EXCEPTION, throwable.getClass().getSimpleName())) - .orElse(EXCEPTION_NONE); + return context.getError().map(throwable -> { + String simpleName = throwable.getClass().getSimpleName(); + return KeyValue.of(HttpRequestsObservation.LowCardinalityKeyNames.EXCEPTION, + StringUtils.hasText(simpleName) ? simpleName : throwable.getClass().getName()); + }) + .orElse(EXCEPTION_NONE); } protected KeyValue outcome(HttpRequestsObservationContext context) { diff --git a/spring-web/src/main/java/org/springframework/web/observation/HttpRequestsObservation.java b/spring-web/src/main/java/org/springframework/web/observation/HttpRequestsObservation.java index 2e6a8c976a0..39107c6ea56 100644 --- a/spring-web/src/main/java/org/springframework/web/observation/HttpRequestsObservation.java +++ b/spring-web/src/main/java/org/springframework/web/observation/HttpRequestsObservation.java @@ -54,7 +54,7 @@ public enum HttpRequestsObservation implements DocumentedObservation { public enum LowCardinalityKeyNames implements KeyName { /** - * Name of HTTP request method or {@code "None"} if the request was not received properly. + * Name of HTTP request method or {@code "none"} if the request was not received properly. */ METHOD { @Override @@ -65,7 +65,7 @@ public enum HttpRequestsObservation implements DocumentedObservation { }, /** - * HTTP response raw status code, or {@code "STATUS_UNKNOWN"} if no response was created. + * HTTP response raw status code, or {@code "UNKNOWN"} if no response was created. */ STATUS { @Override @@ -87,7 +87,7 @@ public enum HttpRequestsObservation implements DocumentedObservation { }, /** - * Name of the exception thrown during the exchange, or {@code "None"} if no exception happened. + * Name of the exception thrown during the exchange, or {@code "none"} if no exception happened. */ EXCEPTION { @Override diff --git a/spring-web/src/main/java/org/springframework/web/observation/HttpRequestsObservationContext.java b/spring-web/src/main/java/org/springframework/web/observation/HttpRequestsObservationContext.java index b62280696a3..6007e88e18d 100644 --- a/spring-web/src/main/java/org/springframework/web/observation/HttpRequestsObservationContext.java +++ b/spring-web/src/main/java/org/springframework/web/observation/HttpRequestsObservationContext.java @@ -36,8 +36,8 @@ public class HttpRequestsObservationContext extends RequestReplyReceiverContext< public HttpRequestsObservationContext(HttpServletRequest request, HttpServletResponse response) { super(HttpServletRequest::getHeader); - this.setCarrier(request); - this.setResponse(response); + setCarrier(request); + setResponse(response); } @Nullable diff --git a/spring-web/src/main/java/org/springframework/web/observation/HttpRequestsObservationFilter.java b/spring-web/src/main/java/org/springframework/web/observation/HttpRequestsObservationFilter.java index 8d42cd6a4e1..9dc78c5be78 100644 --- a/spring-web/src/main/java/org/springframework/web/observation/HttpRequestsObservationFilter.java +++ b/spring-web/src/main/java/org/springframework/web/observation/HttpRequestsObservationFilter.java @@ -37,7 +37,7 @@ import org.springframework.web.filter.OncePerRequestFilter; * for HTTP exchanges. This collects information about the execution time and * information gathered from the {@link HttpRequestsObservationContext}. *

Web Frameworks can fetch the current {@link HttpRequestsObservationContext context} - * as a {@link #CURRENT_OBSERVATION_ATTRIBUTE request attribute} and contribute + * as a {@link #CURRENT_OBSERVATION_CONTEXT_ATTRIBUTE request attribute} and contribute * additional information to it. * The configured {@link HttpRequestsObservationConvention} will use this context to collect * {@link io.micrometer.common.KeyValue metadata} and attach it to the observation. @@ -61,17 +61,17 @@ public class HttpRequestsObservationFilter extends OncePerRequestFilter { private final HttpRequestsObservationConvention observationConvention; /** - * Create a {@code HttpRequestsObservationFilter} that records observations + * Create an {@code HttpRequestsObservationFilter} that records observations * against the given {@link ObservationRegistry}. The default * {@link DefaultHttpRequestsObservationConvention convention} will be used. * @param observationRegistry the registry to use for recording observations */ public HttpRequestsObservationFilter(ObservationRegistry observationRegistry) { - this(observationRegistry, new DefaultHttpRequestsObservationConvention()); + this(observationRegistry, DEFAULT_OBSERVATION_CONVENTION); } /** - * Create a {@code HttpRequestsObservationFilter} that records observations + * Create an {@code HttpRequestsObservationFilter} that records observations * against the given {@link ObservationRegistry} with a custom convention. * @param observationRegistry the registry to use for recording observations * @param observationConvention the convention to use for all recorded observations diff --git a/spring-web/src/main/java/org/springframework/web/observation/reactive/DefaultHttpRequestsObservationConvention.java b/spring-web/src/main/java/org/springframework/web/observation/reactive/DefaultHttpRequestsObservationConvention.java index ca44f78be3a..257cfa181f2 100644 --- a/spring-web/src/main/java/org/springframework/web/observation/reactive/DefaultHttpRequestsObservationConvention.java +++ b/spring-web/src/main/java/org/springframework/web/observation/reactive/DefaultHttpRequestsObservationConvention.java @@ -21,6 +21,7 @@ import io.micrometer.common.KeyValues; import org.springframework.http.HttpStatus; import org.springframework.http.observation.HttpOutcome; +import org.springframework.util.StringUtils; import org.springframework.web.util.pattern.PathPattern; /** @@ -92,6 +93,9 @@ public class DefaultHttpRequestsObservationConvention implements HttpRequestsObs } protected KeyValue status(HttpRequestsObservationContext context) { + if (context.isConnectionAborted()) { + return STATUS_UNKNOWN; + } return (context.getResponse() != null) ? KeyValue.of(HttpRequestsObservation.LowCardinalityKeyNames.STATUS, Integer.toString(context.getResponse().getStatusCode().value())) : STATUS_UNKNOWN; } @@ -120,16 +124,19 @@ public class DefaultHttpRequestsObservationConvention implements HttpRequestsObs } protected KeyValue exception(HttpRequestsObservationContext context) { - return context.getError().map(throwable -> - KeyValue.of(HttpRequestsObservation.LowCardinalityKeyNames.EXCEPTION, throwable.getClass().getSimpleName())) - .orElse(EXCEPTION_NONE); + return context.getError().map(throwable -> { + String simpleName = throwable.getClass().getSimpleName(); + return KeyValue.of(HttpRequestsObservation.LowCardinalityKeyNames.EXCEPTION, + StringUtils.hasText(simpleName) ? simpleName : throwable.getClass().getName()); + }) + .orElse(EXCEPTION_NONE); } protected KeyValue outcome(HttpRequestsObservationContext context) { if (context.isConnectionAborted()) { return HttpOutcome.UNKNOWN.asKeyValue(); } - else if (context.getResponse() != null) { + if (context.getResponse() != null) { HttpOutcome httpOutcome = HttpOutcome.forStatus(context.getResponse().getStatusCode()); return httpOutcome.asKeyValue(); } diff --git a/spring-web/src/main/java/org/springframework/web/observation/reactive/HttpRequestsObservation.java b/spring-web/src/main/java/org/springframework/web/observation/reactive/HttpRequestsObservation.java index a6b27d341e3..f62d482d539 100644 --- a/spring-web/src/main/java/org/springframework/web/observation/reactive/HttpRequestsObservation.java +++ b/spring-web/src/main/java/org/springframework/web/observation/reactive/HttpRequestsObservation.java @@ -23,7 +23,7 @@ import io.micrometer.observation.docs.DocumentedObservation; /** * Documented {@link io.micrometer.common.KeyValue KeyValues} for the HTTP server observations - * for Servlet-based web applications. + * for reactive web applications. *

This class is used by automated tools to document KeyValues attached to the HTTP server observations. * @author Brian Clozel * @since 6.0 @@ -54,7 +54,7 @@ public enum HttpRequestsObservation implements DocumentedObservation { public enum LowCardinalityKeyNames implements KeyName { /** - * Name of HTTP request method or {@code "None"} if the request was not received properly. + * Name of HTTP request method or {@code "none"} if the request was not received properly. */ METHOD { @Override @@ -65,7 +65,7 @@ public enum HttpRequestsObservation implements DocumentedObservation { }, /** - * HTTP response raw status code, or {@code "STATUS_UNKNOWN"} if no response was created. + * HTTP response raw status code, or {@code "UNKNOWN"} if no response was created. */ STATUS { @Override @@ -87,7 +87,7 @@ public enum HttpRequestsObservation implements DocumentedObservation { }, /** - * Name of the exception thrown during the exchange, or {@code "None"} if no exception happened. + * Name of the exception thrown during the exchange, or {@code "none"} if no exception happened. */ EXCEPTION { @Override diff --git a/spring-web/src/main/java/org/springframework/web/observation/reactive/HttpRequestsObservationContext.java b/spring-web/src/main/java/org/springframework/web/observation/reactive/HttpRequestsObservationContext.java index 685e1d63601..0a98560f0e7 100644 --- a/spring-web/src/main/java/org/springframework/web/observation/reactive/HttpRequestsObservationContext.java +++ b/spring-web/src/main/java/org/springframework/web/observation/reactive/HttpRequestsObservationContext.java @@ -40,8 +40,8 @@ public class HttpRequestsObservationContext extends RequestReplyReceiverContext< public HttpRequestsObservationContext(ServerWebExchange exchange) { super((request, key) -> request.getHeaders().getFirst(key)); - this.setCarrier(exchange.getRequest()); - this.setResponse(exchange.getResponse()); + setCarrier(exchange.getRequest()); + setResponse(exchange.getResponse()); } /** @@ -66,7 +66,7 @@ public class HttpRequestsObservationContext extends RequestReplyReceiverContext< /** * Whether the current connection was aborted by the client, resulting - * in a {@link reactor.core.publisher.SignalType#CANCEL cancel signal} on te reactive chain, + * in a {@link reactor.core.publisher.SignalType#CANCEL cancel signal} on the reactive chain, * or an {@code AbortedException} when reading the request. * @return if the connection has been aborted */ diff --git a/spring-web/src/main/java/org/springframework/web/observation/reactive/HttpRequestsObservationWebFilter.java b/spring-web/src/main/java/org/springframework/web/observation/reactive/HttpRequestsObservationWebFilter.java index d8182162d70..6f8cfb7c743 100644 --- a/spring-web/src/main/java/org/springframework/web/observation/reactive/HttpRequestsObservationWebFilter.java +++ b/spring-web/src/main/java/org/springframework/web/observation/reactive/HttpRequestsObservationWebFilter.java @@ -60,17 +60,17 @@ public class HttpRequestsObservationWebFilter implements WebFilter { private final HttpRequestsObservationConvention observationConvention; /** - * Create a {@code HttpRequestsObservationWebFilter} that records observations + * Create an {@code HttpRequestsObservationWebFilter} that records observations * against the given {@link ObservationRegistry}. The default * {@link DefaultHttpRequestsObservationConvention convention} will be used. * @param observationRegistry the registry to use for recording observations */ public HttpRequestsObservationWebFilter(ObservationRegistry observationRegistry) { - this(observationRegistry, new DefaultHttpRequestsObservationConvention()); + this(observationRegistry, DEFAULT_OBSERVATION_CONVENTION); } /** - * Create a {@code HttpRequestsObservationWebFilter} that records observations + * Create an {@code HttpRequestsObservationWebFilter} that records observations * against the given {@link ObservationRegistry} with a custom convention. * @param observationRegistry the registry to use for recording observations * @param observationConvention the convention to use for all recorded observations diff --git a/spring-web/src/test/java/org/springframework/http/observation/HttpOutcomeTests.java b/spring-web/src/test/java/org/springframework/http/observation/HttpOutcomeTests.java index 2b2d4880196..e30b6645da1 100644 --- a/spring-web/src/test/java/org/springframework/http/observation/HttpOutcomeTests.java +++ b/spring-web/src/test/java/org/springframework/http/observation/HttpOutcomeTests.java @@ -57,7 +57,7 @@ class HttpOutcomeTests { } @ParameterizedTest - @ValueSource(ints = {404, 404, 405}) + @ValueSource(ints = {400, 404, 405}) void shouldResolveClientError(int code) { HttpOutcome httpOutcome = HttpOutcome.forStatus(HttpStatusCode.valueOf(code)); assertThat(httpOutcome).isEqualTo(HttpOutcome.CLIENT_ERROR); 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 index 7dab6fd7e50..eba388c676f 100644 --- a/spring-web/src/test/java/org/springframework/web/client/RestTemplateObservationTests.java +++ b/spring-web/src/test/java/org/springframework/web/client/RestTemplateObservationTests.java @@ -19,7 +19,6 @@ 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; @@ -27,7 +26,6 @@ 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; @@ -43,6 +41,7 @@ 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.BDDMockito.willThrow; import static org.mockito.Mockito.mock; import static org.springframework.http.HttpMethod.GET; @@ -50,7 +49,7 @@ import static org.springframework.http.HttpMethod.GET; * Tests for the client HTTP observations with {@link RestTemplate}. * @author Brian Clozel */ -public class RestTemplateObservationTests { +class RestTemplateObservationTests { private final TestObservationRegistry observationRegistry = TestObservationRegistry.create(); @@ -102,7 +101,7 @@ public class RestTemplateObservationTests { @Test - void executeAddsSucessAsOutcome() throws Exception { + void executeAddsSuccessAsOutcome() throws Exception { mockSentRequest(GET, "https://example.org"); mockResponseStatus(HttpStatus.OK); mockResponseBody("Hello World", MediaType.TEXT_PLAIN); @@ -117,7 +116,7 @@ public class RestTemplateObservationTests { String url = "https://example.org"; mockSentRequest(GET, url); mockResponseStatus(HttpStatus.INTERNAL_SERVER_ERROR); - BDDMockito.willThrow(new HttpServerErrorException(HttpStatus.INTERNAL_SERVER_ERROR)) + willThrow(new HttpServerErrorException(HttpStatus.INTERNAL_SERVER_ERROR)) .given(errorHandler).handleError(new URI(url), GET, response); assertThatExceptionOfType(HttpServerErrorException.class).isThrownBy(() -> @@ -133,7 +132,7 @@ public class RestTemplateObservationTests { given(converter.canRead(String.class, null)).willReturn(true); MediaType supportedMediaType = new MediaType("test", "supported"); - given(converter.getSupportedMediaTypes()).willReturn(Collections.singletonList(supportedMediaType)); + given(converter.getSupportedMediaTypes()).willReturn(List.of(supportedMediaType)); MediaType other = new MediaType("test", "other"); mockResponseBody("Test Body", other); given(converter.canRead(String.class, other)).willReturn(false); diff --git a/spring-web/src/test/java/org/springframework/web/observation/HttpRequestsObservationFilterTests.java b/spring-web/src/test/java/org/springframework/web/observation/HttpRequestsObservationFilterTests.java index ac989d0cb51..d587f2308fd 100644 --- a/spring-web/src/test/java/org/springframework/web/observation/HttpRequestsObservationFilterTests.java +++ b/spring-web/src/test/java/org/springframework/web/observation/HttpRequestsObservationFilterTests.java @@ -34,7 +34,7 @@ import static org.assertj.core.api.Assertions.assertThatThrownBy; * Tests for {@link HttpRequestsObservationFilter}. * @author Brian Clozel */ -public class HttpRequestsObservationFilterTests { +class HttpRequestsObservationFilterTests { private final TestObservationRegistry observationRegistry = TestObservationRegistry.create(); diff --git a/spring-web/src/test/java/org/springframework/web/observation/reactive/DefaultHttpRequestsObservationConventionTests.java b/spring-web/src/test/java/org/springframework/web/observation/reactive/DefaultHttpRequestsObservationConventionTests.java index 13affb9e99a..bacd9362d86 100644 --- a/spring-web/src/test/java/org/springframework/web/observation/reactive/DefaultHttpRequestsObservationConventionTests.java +++ b/spring-web/src/test/java/org/springframework/web/observation/reactive/DefaultHttpRequestsObservationConventionTests.java @@ -135,7 +135,7 @@ class DefaultHttpRequestsObservationConventionTests { exchange.getResponse().setRawStatusCode(200); assertThat(this.convention.getLowCardinalityKeyValues(context)).hasSize(5) - .contains(KeyValue.of("method", "GET"), KeyValue.of("uri", "UNKNOWN"), KeyValue.of("status", "200"), + .contains(KeyValue.of("method", "GET"), KeyValue.of("uri", "UNKNOWN"), KeyValue.of("status", "UNKNOWN"), KeyValue.of("exception", "none"), KeyValue.of("outcome", "UNKNOWN")); assertThat(this.convention.getHighCardinalityKeyValues(context)).hasSize(1) .contains(KeyValue.of("uri.expanded", "/test/resource")); diff --git a/spring-web/src/test/java/org/springframework/web/observation/reactive/HttpRequestsObservationWebFilterTests.java b/spring-web/src/test/java/org/springframework/web/observation/reactive/HttpRequestsObservationWebFilterTests.java index 9f88b43c260..5fb307b2ccc 100644 --- a/spring-web/src/test/java/org/springframework/web/observation/reactive/HttpRequestsObservationWebFilterTests.java +++ b/spring-web/src/test/java/org/springframework/web/observation/reactive/HttpRequestsObservationWebFilterTests.java @@ -85,7 +85,7 @@ class HttpRequestsObservationWebFilterTests { assertThatHttpObservation().hasLowCardinalityKeyValue("outcome", "UNKNOWN"); } - WebFilterChain createFilterChain(ThrowingConsumer exchangeConsumer) { + private WebFilterChain createFilterChain(ThrowingConsumer exchangeConsumer) { return filterExchange -> { try { exchangeConsumer.accept(filterExchange); diff --git a/spring-webflux/src/main/java/org/springframework/web/reactive/function/client/ClientObservation.java b/spring-webflux/src/main/java/org/springframework/web/reactive/function/client/ClientObservation.java index 6530e790d2b..9a21759f786 100644 --- a/spring-webflux/src/main/java/org/springframework/web/reactive/function/client/ClientObservation.java +++ b/spring-webflux/src/main/java/org/springframework/web/reactive/function/client/ClientObservation.java @@ -64,7 +64,7 @@ public enum ClientObservation implements DocumentedObservation { }, /** - * URI template used for HTTP request, or {@code ""} if none was provided. + * URI template used for HTTP request, or {@code "none"} if none was provided. */ URI { @Override diff --git a/spring-webflux/src/main/java/org/springframework/web/reactive/function/client/DefaultClientObservationConvention.java b/spring-webflux/src/main/java/org/springframework/web/reactive/function/client/DefaultClientObservationConvention.java index 07aefc6b818..fe45796b03d 100644 --- a/spring-webflux/src/main/java/org/springframework/web/reactive/function/client/DefaultClientObservationConvention.java +++ b/spring-webflux/src/main/java/org/springframework/web/reactive/function/client/DefaultClientObservationConvention.java @@ -22,6 +22,7 @@ import io.micrometer.common.KeyValue; import io.micrometer.common.KeyValues; import io.micrometer.observation.ObservationConvention; +import org.springframework.http.client.observation.ClientHttpObservation; import org.springframework.http.observation.HttpOutcome; import org.springframework.util.StringUtils; @@ -40,8 +41,15 @@ public class DefaultClientObservationConvention implements ClientObservationConv private static final KeyValue METHOD_NONE = KeyValue.of(ClientObservation.LowCardinalityKeyNames.METHOD, "none"); + private static final KeyValue STATUS_IO_ERROR = KeyValue.of(ClientHttpObservation.LowCardinalityKeyNames.STATUS, "IO_ERROR"); + + private static final KeyValue STATUS_CLIENT_ERROR = KeyValue.of(ClientHttpObservation.LowCardinalityKeyNames.STATUS, "CLIENT_ERROR"); + private static final KeyValue EXCEPTION_NONE = KeyValue.of(ClientObservation.LowCardinalityKeyNames.EXCEPTION, "none"); + private static final KeyValue URI_EXPANDED_NONE = KeyValue.of(ClientHttpObservation.HighCardinalityKeyNames.URI_EXPANDED, "none"); + + private static final KeyValue CLIENT_NAME_NONE = KeyValue.of(ClientHttpObservation.HighCardinalityKeyNames.CLIENT_NAME, "none"); private final String name; @@ -93,17 +101,17 @@ public class DefaultClientObservationConvention implements ClientObservationConv } protected KeyValue status(ClientObservationContext context) { - return KeyValue.of(ClientObservation.LowCardinalityKeyNames.STATUS, getStatusMessage(context)); - } - - private String getStatusMessage(ClientObservationContext context) { - if (context.getResponse() != null) { - return String.valueOf(context.getResponse().statusCode().value()); + if (context.isAborted()) { + return STATUS_CLIENT_ERROR; } - if (context.getError().isPresent()) { - return (context.getError().get() instanceof IOException) ? "IO_ERROR" : "CLIENT_ERROR"; + ClientResponse response = context.getResponse(); + if (response != null) { + return KeyValue.of(ClientObservation.LowCardinalityKeyNames.STATUS, String.valueOf(response.statusCode().value())); } - return "CLIENT_ERROR"; + if (context.getError().isPresent() && context.getError().get() instanceof IOException) { + return STATUS_IO_ERROR; + } + return STATUS_CLIENT_ERROR; } protected KeyValue exception(ClientObservationContext context) { @@ -114,11 +122,11 @@ public class DefaultClientObservationConvention implements ClientObservationConv }).orElse(EXCEPTION_NONE); } - protected static KeyValue outcome(ClientObservationContext context) { + protected KeyValue outcome(ClientObservationContext context) { if (context.isAborted()) { return HttpOutcome.UNKNOWN.asKeyValue(); } - else if (context.getResponse() != null) { + if (context.getResponse() != null) { HttpOutcome httpOutcome = HttpOutcome.forStatus(context.getResponse().statusCode()); return httpOutcome.asKeyValue(); } @@ -134,15 +142,14 @@ public class DefaultClientObservationConvention implements ClientObservationConv if (context.getCarrier() != null) { return KeyValue.of(ClientObservation.HighCardinalityKeyNames.URI_EXPANDED, context.getCarrier().url().toASCIIString()); } - return KeyValue.of(ClientObservation.HighCardinalityKeyNames.URI_EXPANDED, "none"); + return URI_EXPANDED_NONE; } protected KeyValue clientName(ClientObservationContext context) { - String host = "none"; if (context.getCarrier() != null && context.getCarrier().url().getHost() != null) { - host = context.getCarrier().url().getHost(); + return KeyValue.of(ClientObservation.HighCardinalityKeyNames.CLIENT_NAME, context.getCarrier().url().getHost()); } - return KeyValue.of(ClientObservation.HighCardinalityKeyNames.CLIENT_NAME, host); + return CLIENT_NAME_NONE; } } diff --git a/spring-webflux/src/main/java/org/springframework/web/reactive/function/client/WebClient.java b/spring-webflux/src/main/java/org/springframework/web/reactive/function/client/WebClient.java index 443f3ce9b8c..95291810e45 100644 --- a/spring-webflux/src/main/java/org/springframework/web/reactive/function/client/WebClient.java +++ b/spring-webflux/src/main/java/org/springframework/web/reactive/function/client/WebClient.java @@ -26,7 +26,7 @@ import java.util.function.Function; import java.util.function.IntPredicate; import java.util.function.Predicate; -import io.micrometer.observation.Observation; +import io.micrometer.observation.ObservationConvention; import io.micrometer.observation.ObservationRegistry; import org.reactivestreams.Publisher; import reactor.core.publisher.Flux; @@ -346,7 +346,7 @@ public interface WebClient { Builder observationRegistry(ObservationRegistry observationRegistry); /** - * Provide a {@link Observation.ObservationConvention} to use for collecting + * Provide an {@link ObservationConvention} to use for collecting * metadata for the current observation. Will use {@link DefaultClientObservationConvention} * if none provided. * @param observationConvention the observation convention to use diff --git a/spring-webflux/src/test/java/org/springframework/web/reactive/function/client/DefaultClientObservationConventionTests.java b/spring-webflux/src/test/java/org/springframework/web/reactive/function/client/DefaultClientObservationConventionTests.java index 91d4a949ce2..6340902abdb 100644 --- a/spring-webflux/src/test/java/org/springframework/web/reactive/function/client/DefaultClientObservationConventionTests.java +++ b/spring-webflux/src/test/java/org/springframework/web/reactive/function/client/DefaultClientObservationConventionTests.java @@ -34,7 +34,7 @@ import static org.assertj.core.api.Assertions.assertThat; */ class DefaultClientObservationConventionTests { - private DefaultClientObservationConvention observationConvention = new DefaultClientObservationConvention(); + private final DefaultClientObservationConvention observationConvention = new DefaultClientObservationConvention(); @Test void shouldHaveName() { diff --git a/spring-webflux/src/test/java/org/springframework/web/reactive/function/client/DefaultClientObservationTests.java b/spring-webflux/src/test/java/org/springframework/web/reactive/function/client/WebClientObservationTests.java similarity index 94% rename from spring-webflux/src/test/java/org/springframework/web/reactive/function/client/DefaultClientObservationTests.java rename to spring-webflux/src/test/java/org/springframework/web/reactive/function/client/WebClientObservationTests.java index f397d4423cc..79800678a91 100644 --- a/spring-webflux/src/test/java/org/springframework/web/reactive/function/client/DefaultClientObservationTests.java +++ b/spring-webflux/src/test/java/org/springframework/web/reactive/function/client/WebClientObservationTests.java @@ -39,19 +39,19 @@ import static org.mockito.Mockito.verifyNoMoreInteractions; * Tests for the {@link WebClient} {@link io.micrometer.observation.Observation observations}. * @author Brian Clozel */ -public class DefaultClientObservationTests { +class WebClientObservationTests { private final TestObservationRegistry observationRegistry = TestObservationRegistry.create(); - private ExchangeFunction exchangeFunction = mock(ExchangeFunction.class); + private final ExchangeFunction exchangeFunction = mock(ExchangeFunction.class); - private ArgumentCaptor request = ArgumentCaptor.forClass(ClientRequest.class); + private final ArgumentCaptor request = ArgumentCaptor.forClass(ClientRequest.class); private WebClient.Builder builder; @BeforeEach - public void setup() { + void setup() { ClientResponse mockResponse = mock(ClientResponse.class); when(mockResponse.statusCode()).thenReturn(HttpStatus.OK); when(mockResponse.bodyToMono(Void.class)).thenReturn(Mono.empty());