root
/
spring-framework
mirror of https://github.com/spring-projects/spring-framework.git
1
0
Fork 0
Code Issues Actions 4 Packages Projects Releases Wiki Activity

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
This commit is contained in:
Brian Clozel 2022-09-30 18:14:10 +02:00
parent b9070ae752
commit 8c24e8c034
15 changed files with 224 additions and 42 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

12
spring-web/src/main/java/org/springframework/http/client/observation/DefaultClientHttpObservationConvention.java
View File

@ -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

101
spring-web/src/main/java/org/springframework/http/observation/HttpOutcome.java Normal file
View File

@ -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.
* <p>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;
}
}

9
spring-web/src/main/java/org/springframework/http/observation/package-info.java Normal file
View File

@ -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;

13
spring-web/src/main/java/org/springframework/web/observation/DefaultHttpRequestsObservationConvention.java
View File

@ -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) {

13
spring-web/src/main/java/org/springframework/web/observation/reactive/DefaultHttpRequestsObservationConvention.java
View File

@ -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) {

2
spring-web/src/test/java/org/springframework/http/client/observation/DefaultClientHttpObservationConventionTests.java
View File

@ -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"));
}

83
spring-web/src/test/java/org/springframework/http/observation/HttpOutcomeTests.java Normal file
View File

@ -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"));
}
}

2
spring-web/src/test/java/org/springframework/web/client/RestTemplateObservationTests.java
View File

@ -109,7 +109,7 @@ public class RestTemplateObservationTests {
template.execute("https://example.org", GET, null, null);
assertThatHttpObservation().hasLowCardinalityKeyValue("outcome", "SUCCESSFUL");
assertThatHttpObservation().hasLowCardinalityKeyValue("outcome", "SUCCESS");
}
@Test

4
spring-web/src/test/java/org/springframework/web/observation/DefaultHttpRequestsObservationConventionTests.java
View File

@ -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"));
}

4
spring-web/src/test/java/org/springframework/web/observation/HttpRequestsObservationFilterTests.java
View File

@ -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() {

4
spring-web/src/test/java/org/springframework/web/observation/reactive/DefaultHttpRequestsObservationConventionTests.java
View File

@ -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"));
}

2
spring-web/src/test/java/org/springframework/web/observation/reactive/HttpRequestsObservationWebFilterTests.java
View File

@ -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

13
spring-webflux/src/main/java/org/springframework/web/reactive/function/client/DefaultClientObservationConvention.java
View File

@ -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

2
spring-webflux/src/test/java/org/springframework/web/reactive/function/client/DefaultClientObservationConventionTests.java
View File

@ -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"));
}

2
spring-webflux/src/test/java/org/springframework/web/reactive/function/client/DefaultClientObservationTests.java
View File

@ -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}");
}