From e9fcb21d55c586062b7b74798a23e0dd283f22d0 Mon Sep 17 00:00:00 2001 From: Brian Clozel Date: Wed, 11 Oct 2023 11:17:38 +0200 Subject: [PATCH] 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 --- ...faultServerRequestObservationConvention.java | 2 +- ...ServerRequestObservationConventionTests.java | 17 ++++++++++++++++- 2 files changed, 17 insertions(+), 2 deletions(-) diff --git a/spring-web/src/main/java/org/springframework/http/server/reactive/observation/DefaultServerRequestObservationConvention.java b/spring-web/src/main/java/org/springframework/http/server/reactive/observation/DefaultServerRequestObservationConvention.java index b93e73abd017..43fe7fce325f 100644 --- a/spring-web/src/main/java/org/springframework/http/server/reactive/observation/DefaultServerRequestObservationConvention.java +++ b/spring-web/src/main/java/org/springframework/http/server/reactive/observation/DefaultServerRequestObservationConvention.java @@ -108,7 +108,7 @@ protected KeyValue method(ServerRequestObservationContext context) { } 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) ? diff --git a/spring-web/src/test/java/org/springframework/http/server/reactive/observation/DefaultServerRequestObservationConventionTests.java b/spring-web/src/test/java/org/springframework/http/server/reactive/observation/DefaultServerRequestObservationConventionTests.java index 138b52737ad6..fb0e11e90c92 100644 --- a/spring-web/src/test/java/org/springframework/http/server/reactive/observation/DefaultServerRequestObservationConventionTests.java +++ b/spring-web/src/test/java/org/springframework/http/server/reactive/observation/DefaultServerRequestObservationConventionTests.java @@ -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 @@ void addsKeyValuesForCancelledExchange() { .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"));