From 8c24e8c034e2a7b26021b319b9d4486a23bfb265 Mon Sep 17 00:00:00 2001 From: Brian Clozel Date: Fri, 30 Sep 2022 18:14:10 +0200 Subject: [PATCH] Add HttpOutcome for HTTP observations Prior to this commit, the HTTP Observations would use `HttpStatus.Series` as a value source for the "outcome" key value in recorded observations. This would work for most cases, but would not align in the 2xx HTTP status cases: the series would provide a "SUCESSFUL" value whereas the heritage metrics support in Spring Boot would give "SUCESS". This commit introduces a dedicated `HttpOutcome` concept for this and applies it to all HTTP observations. Fixes gh-29232 --- ...efaultClientHttpObservationConvention.java | 12 +-- .../http/observation/HttpOutcome.java | 101 ++++++++++++++++++ .../http/observation/package-info.java | 9 ++ ...aultHttpRequestsObservationConvention.java | 13 ++- ...aultHttpRequestsObservationConvention.java | 13 +-- ...tClientHttpObservationConventionTests.java | 2 +- .../http/observation/HttpOutcomeTests.java | 83 ++++++++++++++ .../client/RestTemplateObservationTests.java | 2 +- ...ttpRequestsObservationConventionTests.java | 4 +- .../HttpRequestsObservationFilterTests.java | 4 +- ...ttpRequestsObservationConventionTests.java | 4 +- ...HttpRequestsObservationWebFilterTests.java | 2 +- .../DefaultClientObservationConvention.java | 13 +-- ...faultClientObservationConventionTests.java | 2 +- .../client/DefaultClientObservationTests.java | 2 +- 15 files changed, 224 insertions(+), 42 deletions(-) create mode 100644 spring-web/src/main/java/org/springframework/http/observation/HttpOutcome.java create mode 100644 spring-web/src/main/java/org/springframework/http/observation/package-info.java create mode 100644 spring-web/src/test/java/org/springframework/http/observation/HttpOutcomeTests.java 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 edb2c4303f..c1c40d7e32 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 @@ -21,8 +21,8 @@ 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.http.observation.HttpOutcome; import org.springframework.lang.Nullable; import org.springframework.util.StringUtils; @@ -43,8 +43,6 @@ public class DefaultClientHttpObservationConvention implements ClientHttpObserva 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; @@ -122,16 +120,14 @@ public class DefaultClientHttpObservationConvention implements ClientHttpObserva 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()); - } + HttpOutcome httpOutcome = HttpOutcome.forStatus(context.getResponse().getStatusCode()); + return httpOutcome.asKeyValue(); } } catch (IOException ex) { // Continue } - return OUTCOME_UNKNOWN; + return HttpOutcome.UNKNOWN.asKeyValue(); } @Override 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 new file mode 100644 index 0000000000..fd66bf94a2 --- /dev/null +++ b/spring-web/src/main/java/org/springframework/http/observation/HttpOutcome.java @@ -0,0 +1,101 @@ +/* + * 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.observation; + +import io.micrometer.common.KeyValue; + +import org.springframework.http.HttpStatusCode; + +/** + * The outcome of an HTTP request. + *

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 + */ +public enum HttpOutcome { + + /** + * Outcome of the request was informational. + */ + INFORMATIONAL, + + /** + * Outcome of the request was success. + */ + SUCCESS, + + /** + * Outcome of the request was redirection. + */ + REDIRECTION, + + /** + * Outcome of the request was client error. + */ + CLIENT_ERROR, + + /** + * Outcome of the request was server error. + */ + SERVER_ERROR, + + /** + * Outcome of the request was unknown. + */ + UNKNOWN; + + private final KeyValue keyValue; + + HttpOutcome() { + this.keyValue = KeyValue.of("outcome", name()); + } + + /** + * Returns the {@code Outcome} as a {@link KeyValue} named {@code outcome}. + * @return the {@code outcome} {@code KeyValue} + */ + public KeyValue asKeyValue() { + return this.keyValue; + } + + /** + * Return the {@code HttpOutcome} for the given HTTP {@code status} code. + * @param status the HTTP status code + * @return the matching HttpOutcome + */ + public static HttpOutcome forStatus(HttpStatusCode status) { + if (status.is1xxInformational()) { + return INFORMATIONAL; + } + else if (status.is2xxSuccessful()) { + return SUCCESS; + } + else if (status.is3xxRedirection()) { + return REDIRECTION; + } + else if (status.is4xxClientError()) { + return CLIENT_ERROR; + } + else if (status.is5xxServerError()) { + return SERVER_ERROR; + } + return UNKNOWN; + } +} diff --git a/spring-web/src/main/java/org/springframework/http/observation/package-info.java b/spring-web/src/main/java/org/springframework/http/observation/package-info.java new file mode 100644 index 0000000000..87a33f2d27 --- /dev/null +++ b/spring-web/src/main/java/org/springframework/http/observation/package-info.java @@ -0,0 +1,9 @@ +/** + * Base support for HTTP {@link io.micrometer.observation.Observation}. + */ +@NonNullApi +@NonNullFields +package org.springframework.http.observation; + +import org.springframework.lang.NonNullApi; +import org.springframework.lang.NonNullFields; 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 e8c51f7169..43360b8428 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 @@ -20,6 +20,8 @@ import io.micrometer.common.KeyValue; import io.micrometer.common.KeyValues; import org.springframework.http.HttpStatus; +import org.springframework.http.HttpStatusCode; +import org.springframework.http.observation.HttpOutcome; /** * Default {@link HttpRequestsObservationConvention}. @@ -44,8 +46,6 @@ public class DefaultHttpRequestsObservationConvention implements HttpRequestsObs private static final KeyValue EXCEPTION_NONE = KeyValue.of(HttpRequestsObservation.LowCardinalityKeyNames.EXCEPTION, "none"); - private static final KeyValue OUTCOME_UNKNOWN = KeyValue.of(HttpRequestsObservation.LowCardinalityKeyNames.OUTCOME, "UNKNOWN"); - private static final KeyValue URI_EXPANDED_UNKNOWN = KeyValue.of(HttpRequestsObservation.HighCardinalityKeyNames.URI_EXPANDED, "UNKNOWN"); private final String name; @@ -125,12 +125,11 @@ public class DefaultHttpRequestsObservationConvention implements HttpRequestsObs protected KeyValue outcome(HttpRequestsObservationContext context) { if (context.getResponse() != null) { - HttpStatus status = HttpStatus.resolve(context.getResponse().getStatus()); - if (status != null) { - return KeyValue.of(HttpRequestsObservation.LowCardinalityKeyNames.OUTCOME, status.series().name()); - } + HttpStatusCode statusCode = HttpStatusCode.valueOf(context.getResponse().getStatus()); + HttpOutcome httpOutcome = HttpOutcome.forStatus(statusCode); + return httpOutcome.asKeyValue(); } - return OUTCOME_UNKNOWN; + return HttpOutcome.UNKNOWN.asKeyValue(); } protected KeyValue uriExpanded(HttpRequestsObservationContext context) { 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 45361707dc..ca44f78be3 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 @@ -20,6 +20,7 @@ import io.micrometer.common.KeyValue; import io.micrometer.common.KeyValues; import org.springframework.http.HttpStatus; +import org.springframework.http.observation.HttpOutcome; import org.springframework.web.util.pattern.PathPattern; /** @@ -46,8 +47,6 @@ public class DefaultHttpRequestsObservationConvention implements HttpRequestsObs private static final KeyValue EXCEPTION_NONE = KeyValue.of(HttpRequestsObservation.LowCardinalityKeyNames.EXCEPTION, "none"); - private static final KeyValue OUTCOME_UNKNOWN = KeyValue.of(HttpRequestsObservation.LowCardinalityKeyNames.OUTCOME, "UNKNOWN"); - private static final KeyValue URI_EXPANDED_UNKNOWN = KeyValue.of(HttpRequestsObservation.HighCardinalityKeyNames.URI_EXPANDED, "UNKNOWN"); private final String name; @@ -128,15 +127,13 @@ public class DefaultHttpRequestsObservationConvention implements HttpRequestsObs protected KeyValue outcome(HttpRequestsObservationContext context) { if (context.isConnectionAborted()) { - return OUTCOME_UNKNOWN; + return HttpOutcome.UNKNOWN.asKeyValue(); } else if (context.getResponse() != null) { - HttpStatus status = HttpStatus.resolve(context.getResponse().getStatusCode().value()); - if (status != null) { - return KeyValue.of(HttpRequestsObservation.LowCardinalityKeyNames.OUTCOME, status.series().name()); - } + HttpOutcome httpOutcome = HttpOutcome.forStatus(context.getResponse().getStatusCode()); + return httpOutcome.asKeyValue(); } - return OUTCOME_UNKNOWN; + return HttpOutcome.UNKNOWN.asKeyValue(); } protected KeyValue uriExpanded(HttpRequestsObservationContext context) { 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 index 9c405e7df8..989aec4cb7 100644 --- 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 @@ -88,7 +88,7 @@ class DefaultClientHttpObservationConventionTests { 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")); + KeyValue.of("status", "200"), KeyValue.of("outcome", "SUCCESS")); assertThat(this.observationConvention.getHighCardinalityKeyValues(context)).hasSize(2) .contains(KeyValue.of("client.name", "none"), KeyValue.of("uri.expanded", "/resource/42")); } 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 new file mode 100644 index 0000000000..2b2d488019 --- /dev/null +++ b/spring-web/src/test/java/org/springframework/http/observation/HttpOutcomeTests.java @@ -0,0 +1,83 @@ +/* + * 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.observation; + + +import io.micrometer.common.KeyValue; +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.ValueSource; + +import org.springframework.http.HttpStatusCode; + +import static org.assertj.core.api.Assertions.assertThat; + +/** + * Tests for {@link HttpOutcome}. + * + * @author Brian Clozel + */ +class HttpOutcomeTests { + + @ParameterizedTest + @ValueSource(ints = {100, 101, 102}) + void shouldResolveInformational(int code) { + HttpOutcome httpOutcome = HttpOutcome.forStatus(HttpStatusCode.valueOf(code)); + assertThat(httpOutcome).isEqualTo(HttpOutcome.INFORMATIONAL); + assertThat(httpOutcome.asKeyValue()).isEqualTo(KeyValue.of("outcome", "INFORMATIONAL")); + } + + @ParameterizedTest + @ValueSource(ints = {200, 202, 226}) + void shouldResolveSuccess(int code) { + HttpOutcome httpOutcome = HttpOutcome.forStatus(HttpStatusCode.valueOf(code)); + assertThat(httpOutcome).isEqualTo(HttpOutcome.SUCCESS); + assertThat(httpOutcome.asKeyValue()).isEqualTo(KeyValue.of("outcome", "SUCCESS")); + } + + @ParameterizedTest + @ValueSource(ints = {300, 302, 303}) + void shouldResolveRedirection(int code) { + HttpOutcome httpOutcome = HttpOutcome.forStatus(HttpStatusCode.valueOf(code)); + assertThat(httpOutcome).isEqualTo(HttpOutcome.REDIRECTION); + assertThat(httpOutcome.asKeyValue()).isEqualTo(KeyValue.of("outcome", "REDIRECTION")); + } + + @ParameterizedTest + @ValueSource(ints = {404, 404, 405}) + void shouldResolveClientError(int code) { + HttpOutcome httpOutcome = HttpOutcome.forStatus(HttpStatusCode.valueOf(code)); + assertThat(httpOutcome).isEqualTo(HttpOutcome.CLIENT_ERROR); + assertThat(httpOutcome.asKeyValue()).isEqualTo(KeyValue.of("outcome", "CLIENT_ERROR")); + } + + @ParameterizedTest + @ValueSource(ints = {500, 502, 503}) + void shouldResolveServerError(int code) { + HttpOutcome httpOutcome = HttpOutcome.forStatus(HttpStatusCode.valueOf(code)); + assertThat(httpOutcome).isEqualTo(HttpOutcome.SERVER_ERROR); + assertThat(httpOutcome.asKeyValue()).isEqualTo(KeyValue.of("outcome", "SERVER_ERROR")); + } + + @ParameterizedTest + @ValueSource(ints = {600, 799, 855}) + void shouldResolveUnknown(int code) { + HttpOutcome httpOutcome = HttpOutcome.forStatus(HttpStatusCode.valueOf(code)); + assertThat(httpOutcome).isEqualTo(HttpOutcome.UNKNOWN); + assertThat(httpOutcome.asKeyValue()).isEqualTo(KeyValue.of("outcome", "UNKNOWN")); + } + +} 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 10b5eab75f..7dab6fd7e5 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 @@ -109,7 +109,7 @@ public class RestTemplateObservationTests { template.execute("https://example.org", GET, null, null); - assertThatHttpObservation().hasLowCardinalityKeyValue("outcome", "SUCCESSFUL"); + assertThatHttpObservation().hasLowCardinalityKeyValue("outcome", "SUCCESS"); } @Test diff --git a/spring-web/src/test/java/org/springframework/web/observation/DefaultHttpRequestsObservationConventionTests.java b/spring-web/src/test/java/org/springframework/web/observation/DefaultHttpRequestsObservationConventionTests.java index c516f372d2..1628c1b0e9 100644 --- a/spring-web/src/test/java/org/springframework/web/observation/DefaultHttpRequestsObservationConventionTests.java +++ b/spring-web/src/test/java/org/springframework/web/observation/DefaultHttpRequestsObservationConventionTests.java @@ -64,7 +64,7 @@ class DefaultHttpRequestsObservationConventionTests { assertThat(this.convention.getLowCardinalityKeyValues(this.context)).hasSize(5) .contains(KeyValue.of("method", "POST"), KeyValue.of("uri", "UNKNOWN"), KeyValue.of("status", "200"), - KeyValue.of("exception", "none"), KeyValue.of("outcome", "SUCCESSFUL")); + KeyValue.of("exception", "none"), KeyValue.of("outcome", "SUCCESS")); assertThat(this.convention.getHighCardinalityKeyValues(this.context)).hasSize(1) .contains(KeyValue.of("uri.expanded", "/test/resource")); } @@ -77,7 +77,7 @@ class DefaultHttpRequestsObservationConventionTests { assertThat(this.convention.getLowCardinalityKeyValues(this.context)).hasSize(5) .contains(KeyValue.of("method", "GET"), KeyValue.of("uri", "/test/{name}"), KeyValue.of("status", "200"), - KeyValue.of("exception", "none"), KeyValue.of("outcome", "SUCCESSFUL")); + KeyValue.of("exception", "none"), KeyValue.of("outcome", "SUCCESS")); assertThat(this.convention.getHighCardinalityKeyValues(this.context)).hasSize(1) .contains(KeyValue.of("uri.expanded", "/test/resource")); } 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 a093317500..ac989d0cb5 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 @@ -57,7 +57,7 @@ public class HttpRequestsObservationFilterTests { assertThat(context.getCarrier()).isEqualTo(this.request); assertThat(context.getResponse()).isEqualTo(this.response); assertThat(context.getPathPattern()).isNull(); - assertThatHttpObservation().hasLowCardinalityKeyValue("outcome", "SUCCESSFUL"); + assertThatHttpObservation().hasLowCardinalityKeyValue("outcome", "SUCCESS"); } @Test @@ -84,7 +84,7 @@ public class HttpRequestsObservationFilterTests { HttpRequestsObservationContext context = (HttpRequestsObservationContext) this.request .getAttribute(HttpRequestsObservationFilter.CURRENT_OBSERVATION_CONTEXT_ATTRIBUTE); assertThat(context.getError()).get().isEqualTo(customError); - assertThatHttpObservation().hasLowCardinalityKeyValue("outcome", "SUCCESSFUL"); + assertThatHttpObservation().hasLowCardinalityKeyValue("outcome", "SUCCESS"); } private TestObservationRegistryAssert.TestObservationRegistryAssertReturningObservationContextAssert assertThatHttpObservation() { 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 417ad78a20..13affb9e99 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 @@ -65,7 +65,7 @@ class DefaultHttpRequestsObservationConventionTests { assertThat(this.convention.getLowCardinalityKeyValues(context)).hasSize(5) .contains(KeyValue.of("method", "POST"), KeyValue.of("uri", "UNKNOWN"), KeyValue.of("status", "201"), - KeyValue.of("exception", "none"), KeyValue.of("outcome", "SUCCESSFUL")); + KeyValue.of("exception", "none"), KeyValue.of("outcome", "SUCCESS")); assertThat(this.convention.getHighCardinalityKeyValues(context)).hasSize(1) .contains(KeyValue.of("uri.expanded", "/test/resource")); } @@ -80,7 +80,7 @@ class DefaultHttpRequestsObservationConventionTests { assertThat(this.convention.getLowCardinalityKeyValues(context)).hasSize(5) .contains(KeyValue.of("method", "GET"), KeyValue.of("uri", "/test/{name}"), KeyValue.of("status", "200"), - KeyValue.of("exception", "none"), KeyValue.of("outcome", "SUCCESSFUL")); + KeyValue.of("exception", "none"), KeyValue.of("outcome", "SUCCESS")); 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 b7a7a4964d..9f88b43c26 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 @@ -55,7 +55,7 @@ class HttpRequestsObservationWebFilterTests { assertThat(observationContext.get().getResponse()).isEqualTo(exchange.getResponse()); }); this.filter.filter(exchange, filterChain).block(); - assertThatHttpObservation().hasLowCardinalityKeyValue("outcome", "SUCCESSFUL"); + assertThatHttpObservation().hasLowCardinalityKeyValue("outcome", "SUCCESS"); } @Test 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 53375081ac..07aefc6b81 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,7 +22,7 @@ import io.micrometer.common.KeyValue; import io.micrometer.common.KeyValues; import io.micrometer.observation.ObservationConvention; -import org.springframework.http.HttpStatus; +import org.springframework.http.observation.HttpOutcome; import org.springframework.util.StringUtils; /** @@ -42,7 +42,6 @@ public class DefaultClientObservationConvention implements ClientObservationConv private static final KeyValue EXCEPTION_NONE = KeyValue.of(ClientObservation.LowCardinalityKeyNames.EXCEPTION, "none"); - private static final KeyValue OUTCOME_UNKNOWN = KeyValue.of(ClientObservation.LowCardinalityKeyNames.OUTCOME, "UNKNOWN"); private final String name; @@ -117,15 +116,13 @@ public class DefaultClientObservationConvention implements ClientObservationConv protected static KeyValue outcome(ClientObservationContext context) { if (context.isAborted()) { - return OUTCOME_UNKNOWN; + return HttpOutcome.UNKNOWN.asKeyValue(); } else if (context.getResponse() != null) { - HttpStatus status = HttpStatus.resolve(context.getResponse().statusCode().value()); - if (status != null) { - return KeyValue.of(ClientObservation.LowCardinalityKeyNames.OUTCOME, status.series().name()); - } + HttpOutcome httpOutcome = HttpOutcome.forStatus(context.getResponse().statusCode()); + return httpOutcome.asKeyValue(); } - return OUTCOME_UNKNOWN; + return HttpOutcome.UNKNOWN.asKeyValue(); } @Override 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 7efb49fe56..91d4a949ce 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 @@ -83,7 +83,7 @@ class DefaultClientObservationConventionTests { 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")); + KeyValue.of("status", "200"), KeyValue.of("outcome", "SUCCESS")); assertThat(this.observationConvention.getHighCardinalityKeyValues(context)).hasSize(2) .contains(KeyValue.of("client.name", "none"), KeyValue.of("uri.expanded", "/resource/42")); } 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/DefaultClientObservationTests.java index a59f7e9d36..f397d4423c 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/DefaultClientObservationTests.java @@ -66,7 +66,7 @@ public class DefaultClientObservationTests { .retrieve().bodyToMono(Void.class).block(Duration.ofSeconds(10)); verifyAndGetRequest(); - assertThatHttpObservation().hasLowCardinalityKeyValue("outcome", "SUCCESSFUL") + assertThatHttpObservation().hasLowCardinalityKeyValue("outcome", "SUCCESS") .hasLowCardinalityKeyValue("uri", "/resource/{id}"); }