Browse Source

Refine status KeyValue for HTTP server observations

Prior to this commit, a cancelled exchange would always result in an
`"status":"UNKNOWN"` KeyValue. This only applied to reactive variants,
as cancelled exchanges are not currently detected for Servlet
implementations.

In some cases, exchanges can be cancelled by clients before they are
completed, but the response was actually received by the client. The
response status information has been set by the application and the
response has been committed. For those cases, we shouldn't assume an
"UNKNOWN" value.

This commit assumes that committed responses have a response status set
by the application and that the observations should reflect that. From
now on, we only assume an "UNKNOWN" status if the response has not been
commited.

Fixes gh-31388
pull/31496/head
Brian Clozel 2 years ago
parent
commit
e9fcb21d55
  1. 2
      spring-web/src/main/java/org/springframework/http/server/reactive/observation/DefaultServerRequestObservationConvention.java
  2. 17
      spring-web/src/test/java/org/springframework/http/server/reactive/observation/DefaultServerRequestObservationConventionTests.java

2
spring-web/src/main/java/org/springframework/http/server/reactive/observation/DefaultServerRequestObservationConvention.java

@ -108,7 +108,7 @@ public class DefaultServerRequestObservationConvention implements ServerRequestO @@ -108,7 +108,7 @@ public class DefaultServerRequestObservationConvention implements ServerRequestO
}
protected KeyValue status(ServerRequestObservationContext context) {
if (context.isConnectionAborted()) {
if (context.isConnectionAborted() && (context.getResponse() == null || !context.getResponse().isCommitted())) {
return STATUS_UNKNOWN;
}
return (context.getResponse() != null && context.getResponse().getStatusCode() != null) ?

17
spring-web/src/test/java/org/springframework/http/server/reactive/observation/DefaultServerRequestObservationConventionTests.java

@ -1,5 +1,5 @@ @@ -1,5 +1,5 @@
/*
* Copyright 2002-2022 the original author or authors.
* Copyright 2002-2023 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.
@ -147,6 +147,21 @@ class DefaultServerRequestObservationConventionTests { @@ -147,6 +147,21 @@ class DefaultServerRequestObservationConventionTests {
.contains(KeyValue.of("http.url", "/test/resource"));
}
@Test
void addsKeyValuesForCancelledExchangeWhenResponseCommitted() {
ServerWebExchange exchange = MockServerWebExchange.from(MockServerHttpRequest.get("/test/resource"));
ServerRequestObservationContext context = new ServerRequestObservationContext(exchange.getRequest(), exchange.getResponse(), exchange.getAttributes());
context.setConnectionAborted(true);
exchange.getResponse().setRawStatusCode(404);
exchange.getResponse().setComplete().block();
assertThat(this.convention.getLowCardinalityKeyValues(context)).hasSize(5)
.contains(KeyValue.of("method", "GET"), KeyValue.of("uri", "NOT_FOUND"), KeyValue.of("status", "404"),
KeyValue.of("exception", "none"), KeyValue.of("outcome", "UNKNOWN"));
assertThat(this.convention.getHighCardinalityKeyValues(context)).hasSize(1)
.contains(KeyValue.of("http.url", "/test/resource"));
}
@Test
void supportsNullStatusCode() {
ServerWebExchange exchange = MockServerWebExchange.from(MockServerHttpRequest.get("/test/resource"));

Loading…
Cancel
Save