Skip to content

Commit ce65305

Browse files
committed
Improve recording of cancellation signal in WebClient
With its initial fix in gh-18444, the `WebClient` instrumentation would record all CANCEL signals, including: * when a `timeout` expires and the response has not been received * when the client partially consumes the response body Since the second use case is arguable intentional, this commit restricts the instrumentation and thus avoids recording two events for a single request in that case. Closes gh-18444
1 parent 73ca703 commit ce65305

File tree

2 files changed

+23
-1
lines changed

2 files changed

+23
-1
lines changed

spring-boot-project/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/metrics/web/reactive/client/MetricsWebClientFilterFunction.java

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@
1717
package org.springframework.boot.actuate.metrics.web.reactive.client;
1818

1919
import java.util.concurrent.TimeUnit;
20+
import java.util.concurrent.atomic.AtomicBoolean;
2021

2122
import io.micrometer.core.instrument.MeterRegistry;
2223
import io.micrometer.core.instrument.Tag;
@@ -77,13 +78,15 @@ public Mono<ClientResponse> filter(ClientRequest request, ExchangeFunction next)
7778
}
7879

7980
private Mono<ClientResponse> instrumentResponse(ClientRequest request, Mono<ClientResponse> responseMono) {
81+
final AtomicBoolean responseReceived = new AtomicBoolean();
8082
return Mono.deferWithContext((ctx) -> responseMono.doOnEach((signal) -> {
8183
if (signal.isOnNext() || signal.isOnError()) {
84+
responseReceived.set(true);
8285
Iterable<Tag> tags = this.tagProvider.tags(request, signal.get(), signal.getThrowable());
8386
recordTimer(tags, getStartTime(ctx));
8487
}
8588
}).doFinally((signalType) -> {
86-
if (SignalType.CANCEL.equals(signalType)) {
89+
if (!responseReceived.get() && SignalType.CANCEL.equals(signalType)) {
8790
Iterable<Tag> tags = this.tagProvider.tags(request, null, null);
8891
recordTimer(tags, getStartTime(ctx));
8992
}

spring-boot-project/spring-boot-actuator/src/test/java/org/springframework/boot/actuate/metrics/web/reactive/client/MetricsWebClientFilterFunctionTests.java

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,7 @@
2424
import io.micrometer.core.instrument.MeterRegistry;
2525
import io.micrometer.core.instrument.MockClock;
2626
import io.micrometer.core.instrument.Timer;
27+
import io.micrometer.core.instrument.search.MeterNotFoundException;
2728
import io.micrometer.core.instrument.simple.SimpleConfig;
2829
import io.micrometer.core.instrument.simple.SimpleMeterRegistry;
2930
import org.junit.jupiter.api.BeforeEach;
@@ -40,6 +41,7 @@
4041
import org.springframework.web.reactive.function.client.WebClient;
4142

4243
import static org.assertj.core.api.Assertions.assertThat;
44+
import static org.assertj.core.api.Assertions.assertThatThrownBy;
4345
import static org.mockito.BDDMockito.given;
4446
import static org.mockito.Mockito.mock;
4547

@@ -124,6 +126,23 @@ void filterWhenCancelThrownShouldRecordTimer() {
124126
assertThat(this.registry.get("http.client.requests")
125127
.tags("method", "GET", "uri", "/projects/spring-boot", "status", "CLIENT_ERROR").timer().count())
126128
.isEqualTo(1);
129+
assertThatThrownBy(() -> this.registry.get("http.client.requests")
130+
.tags("method", "GET", "uri", "/projects/spring-boot", "status", "200").timer())
131+
.isInstanceOf(MeterNotFoundException.class);
132+
}
133+
134+
@Test
135+
void filterWhenCancelAfterResponseThrownShouldNotRecordTimer() {
136+
ClientRequest request = ClientRequest
137+
.create(HttpMethod.GET, URI.create("https://example.com/projects/spring-boot")).build();
138+
given(this.response.rawStatusCode()).willReturn(HttpStatus.OK.value());
139+
Mono<ClientResponse> filter = this.filterFunction.filter(request, this.exchange);
140+
StepVerifier.create(filter).expectNextCount(1).thenCancel().verify(Duration.ofSeconds(5));
141+
assertThat(this.registry.get("http.client.requests")
142+
.tags("method", "GET", "uri", "/projects/spring-boot", "status", "200").timer().count()).isEqualTo(1);
143+
assertThatThrownBy(() -> this.registry.get("http.client.requests")
144+
.tags("method", "GET", "uri", "/projects/spring-boot", "status", "CLIENT_ERROR").timer())
145+
.isInstanceOf(MeterNotFoundException.class);
127146
}
128147

129148
@Test

0 commit comments

Comments
 (0)