Skip to content

Commit 3879a75

Browse files
bclozelbclozel
committed
Record cancelled client requests in WebClient
Prior to this commit, cancelled client requests (for example as a result of a `timeout()` reactor operator would not be recorded by Micrometer. This commit instruments the cancelled signal for outgoing client requests and assigns a status `CLIENT_ERROR`. The cancellation can be intentional (triggering a timeout and falling back on a faster alternative) or considered as an error. The intent cannot be derived from the signal itself so we're considering it as a client error. Closes gh-18444
1 parent a6d1f1c commit 3879a75

File tree

6 files changed

+85
-25
lines changed

6 files changed

+85
-25
lines changed

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

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright 2012-2019 the original author or authors.
2+
* Copyright 2012-2020 the original author or authors.
33
*
44
* Licensed under the Apache License, Version 2.0 (the "License");
55
* you may not use this file except in compliance with the License.
@@ -37,9 +37,9 @@ public Iterable<Tag> tags(ClientRequest request, ClientResponse response, Throwa
3737
Tag method = WebClientExchangeTags.method(request);
3838
Tag uri = WebClientExchangeTags.uri(request);
3939
Tag clientName = WebClientExchangeTags.clientName(request);
40-
return Arrays.asList(method, uri, clientName,
41-
(response != null) ? WebClientExchangeTags.status(response) : WebClientExchangeTags.status(throwable),
42-
WebClientExchangeTags.outcome(response));
40+
Tag status = WebClientExchangeTags.status(response, throwable);
41+
Tag outcome = WebClientExchangeTags.outcome(response);
42+
return Arrays.asList(method, uri, clientName, status, outcome);
4343
}
4444

4545
}

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

Lines changed: 21 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@
2121
import io.micrometer.core.instrument.MeterRegistry;
2222
import io.micrometer.core.instrument.Tag;
2323
import reactor.core.publisher.Mono;
24+
import reactor.core.publisher.SignalType;
2425
import reactor.util.context.Context;
2526

2627
import org.springframework.boot.actuate.metrics.AutoTimer;
@@ -71,16 +72,27 @@ public Mono<ClientResponse> filter(ClientRequest request, ExchangeFunction next)
7172
if (!this.autoTimer.isEnabled()) {
7273
return next.exchange(request);
7374
}
74-
return next.exchange(request).doOnEach((signal) -> {
75-
if (!signal.isOnComplete()) {
76-
Long startTime = getStartTime(signal.getContext());
77-
ClientResponse response = signal.get();
78-
Throwable throwable = signal.getThrowable();
79-
Iterable<Tag> tags = this.tagProvider.tags(request, response, throwable);
80-
this.autoTimer.builder(this.metricName).tags(tags).description("Timer of WebClient operation")
81-
.register(this.meterRegistry).record(System.nanoTime() - startTime, TimeUnit.NANOSECONDS);
75+
return next.exchange(request).as((responseMono) -> instrumentResponse(request, responseMono))
76+
.subscriberContext(this::putStartTime);
77+
}
78+
79+
private Mono<ClientResponse> instrumentResponse(ClientRequest request, Mono<ClientResponse> responseMono) {
80+
return Mono.deferWithContext((ctx) -> responseMono.doOnEach((signal) -> {
81+
if (signal.isOnNext() || signal.isOnError()) {
82+
Iterable<Tag> tags = this.tagProvider.tags(request, signal.get(), signal.getThrowable());
83+
recordTimer(tags, getStartTime(ctx));
84+
}
85+
}).doFinally((signalType) -> {
86+
if (SignalType.CANCEL.equals(signalType)) {
87+
Iterable<Tag> tags = this.tagProvider.tags(request, null, null);
88+
recordTimer(tags, getStartTime(ctx));
8289
}
83-
}).subscriberContext(this::putStartTime);
90+
}));
91+
}
92+
93+
private void recordTimer(Iterable<Tag> tags, Long startTime) {
94+
this.autoTimer.builder(this.metricName).tags(tags).description("Timer of WebClient operation")
95+
.register(this.meterRegistry).record(System.nanoTime() - startTime, TimeUnit.NANOSECONDS);
8496
}
8597

8698
private Long getStartTime(Context context) {

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

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -75,12 +75,33 @@ private static String extractPath(String url) {
7575
return (path.startsWith("/") ? path : "/" + path);
7676
}
7777

78+
/**
79+
* Creates a {@code status} {@code Tag} derived from the
80+
* {@link ClientResponse#statusCode()} of the given {@code response} if available, the
81+
* thrown exception otherwise, or considers the request as Cancelled as a last resort.
82+
* @param response the response
83+
* @param throwable the exception
84+
* @return the status tag
85+
* @since 2.3.0
86+
*/
87+
public static Tag status(ClientResponse response, Throwable throwable) {
88+
if (response != null) {
89+
return Tag.of("status", String.valueOf(response.rawStatusCode()));
90+
}
91+
else if (throwable != null) {
92+
return (throwable instanceof IOException) ? IO_ERROR : CLIENT_ERROR;
93+
}
94+
return CLIENT_ERROR;
95+
}
96+
7897
/**
7998
* Creates a {@code status} {@code Tag} derived from the
8099
* {@link ClientResponse#statusCode()} of the given {@code response}.
81100
* @param response the response
82101
* @return the status tag
102+
* @deprecated since 2.3.0 in favor of {@link #status(ClientResponse, Throwable)}
83103
*/
104+
@Deprecated
84105
public static Tag status(ClientResponse response) {
85106
return Tag.of("status", String.valueOf(response.rawStatusCode()));
86107
}
@@ -90,7 +111,9 @@ public static Tag status(ClientResponse response) {
90111
* client.
91112
* @param throwable the exception
92113
* @return the status tag
114+
* @deprecated since 2.3.0 in favor of {@link #status(ClientResponse, Throwable)}
93115
*/
116+
@Deprecated
94117
public static Tag status(Throwable throwable) {
95118
return (throwable instanceof IOException) ? IO_ERROR : CLIENT_ERROR;
96119
}

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

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright 2012-2019 the original author or authors.
2+
* Copyright 2012-2020 the original author or authors.
33
*
44
* Licensed under the Apache License, Version 2.0 (the "License");
55
* you may not use this file except in compliance with the License.
@@ -87,4 +87,11 @@ void tagsWhenExceptionShouldReturnClientErrorStatus() {
8787
Tag.of("clientName", "example.org"), Tag.of("status", "CLIENT_ERROR"), Tag.of("outcome", "UNKNOWN"));
8888
}
8989

90+
@Test
91+
void tagsWhenCancelledRequestShouldReturnClientErrorStatus() {
92+
Iterable<Tag> tags = this.tagsProvider.tags(this.request, null, null);
93+
assertThat(tags).containsExactlyInAnyOrder(Tag.of("method", "GET"), Tag.of("uri", "/projects/{project}"),
94+
Tag.of("clientName", "example.org"), Tag.of("status", "CLIENT_ERROR"), Tag.of("outcome", "UNKNOWN"));
95+
}
96+
9097
}

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

Lines changed: 19 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright 2012-2019 the original author or authors.
2+
* Copyright 2012-2020 the original author or authors.
33
*
44
* Licensed under the Apache License, Version 2.0 (the "License");
55
* you may not use this file except in compliance with the License.
@@ -29,6 +29,7 @@
2929
import org.junit.jupiter.api.BeforeEach;
3030
import org.junit.jupiter.api.Test;
3131
import reactor.core.publisher.Mono;
32+
import reactor.test.StepVerifier;
3233

3334
import org.springframework.boot.actuate.metrics.AutoTimer;
3435
import org.springframework.http.HttpMethod;
@@ -73,7 +74,7 @@ void filterShouldRecordTimer() {
7374
ClientRequest request = ClientRequest
7475
.create(HttpMethod.GET, URI.create("https://example.com/projects/spring-boot")).build();
7576
given(this.response.rawStatusCode()).willReturn(HttpStatus.OK.value());
76-
this.filterFunction.filter(request, this.exchange).block(Duration.ofSeconds(30));
77+
this.filterFunction.filter(request, this.exchange).block(Duration.ofSeconds(5));
7778
assertThat(this.registry.get("http.client.requests")
7879
.tags("method", "GET", "uri", "/projects/spring-boot", "status", "200").timer().count()).isEqualTo(1);
7980
}
@@ -84,7 +85,7 @@ void filterWhenUriTemplatePresentShouldRecordTimer() {
8485
.create(HttpMethod.GET, URI.create("https://example.com/projects/spring-boot"))
8586
.attribute(URI_TEMPLATE_ATTRIBUTE, "/projects/{project}").build();
8687
given(this.response.rawStatusCode()).willReturn(HttpStatus.OK.value());
87-
this.filterFunction.filter(request, this.exchange).block(Duration.ofSeconds(30));
88+
this.filterFunction.filter(request, this.exchange).block(Duration.ofSeconds(5));
8889
assertThat(this.registry.get("http.client.requests")
8990
.tags("method", "GET", "uri", "/projects/{project}", "status", "200").timer().count()).isEqualTo(1);
9091
}
@@ -95,7 +96,7 @@ void filterWhenIoExceptionThrownShouldRecordTimer() {
9596
.create(HttpMethod.GET, URI.create("https://example.com/projects/spring-boot")).build();
9697
ExchangeFunction errorExchange = (r) -> Mono.error(new IOException());
9798
this.filterFunction.filter(request, errorExchange).onErrorResume(IOException.class, (t) -> Mono.empty())
98-
.block(Duration.ofSeconds(30));
99+
.block(Duration.ofSeconds(5));
99100
assertThat(this.registry.get("http.client.requests")
100101
.tags("method", "GET", "uri", "/projects/spring-boot", "status", "IO_ERROR").timer().count())
101102
.isEqualTo(1);
@@ -107,7 +108,19 @@ void filterWhenExceptionThrownShouldRecordTimer() {
107108
.create(HttpMethod.GET, URI.create("https://example.com/projects/spring-boot")).build();
108109
ExchangeFunction exchange = (r) -> Mono.error(new IllegalArgumentException());
109110
this.filterFunction.filter(request, exchange).onErrorResume(IllegalArgumentException.class, (t) -> Mono.empty())
110-
.block(Duration.ofSeconds(30));
111+
.block(Duration.ofSeconds(5));
112+
assertThat(this.registry.get("http.client.requests")
113+
.tags("method", "GET", "uri", "/projects/spring-boot", "status", "CLIENT_ERROR").timer().count())
114+
.isEqualTo(1);
115+
}
116+
117+
@Test
118+
void filterWhenCancelThrownShouldRecordTimer() {
119+
ClientRequest request = ClientRequest
120+
.create(HttpMethod.GET, URI.create("https://example.com/projects/spring-boot")).build();
121+
given(this.response.rawStatusCode()).willReturn(HttpStatus.OK.value());
122+
Mono<ClientResponse> filter = this.filterFunction.filter(request, this.exchange);
123+
StepVerifier.create(filter).thenCancel().verify(Duration.ofSeconds(5));
111124
assertThat(this.registry.get("http.client.requests")
112125
.tags("method", "GET", "uri", "/projects/spring-boot", "status", "CLIENT_ERROR").timer().count())
113126
.isEqualTo(1);
@@ -120,7 +133,7 @@ void filterWhenExceptionAndRetryShouldNotCumulateRecordTime() {
120133
ExchangeFunction exchange = (r) -> Mono.error(new IllegalArgumentException())
121134
.delaySubscription(Duration.ofMillis(300)).cast(ClientResponse.class);
122135
this.filterFunction.filter(request, exchange).retry(1)
123-
.onErrorResume(IllegalArgumentException.class, (t) -> Mono.empty()).block(Duration.ofSeconds(30));
136+
.onErrorResume(IllegalArgumentException.class, (t) -> Mono.empty()).block(Duration.ofSeconds(5));
124137
Timer timer = this.registry.get("http.client.requests")
125138
.tags("method", "GET", "uri", "/projects/spring-boot", "status", "CLIENT_ERROR").timer();
126139
assertThat(timer.count()).isEqualTo(2);

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

Lines changed: 10 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright 2012-2019 the original author or authors.
2+
* Copyright 2012-2020 the original author or authors.
33
*
44
* Licensed under the Apache License, Version 2.0 (the "License");
55
* you may not use this file except in compliance with the License.
@@ -86,24 +86,29 @@ void clientName() {
8686
@Test
8787
void status() {
8888
given(this.response.rawStatusCode()).willReturn(HttpStatus.OK.value());
89-
assertThat(WebClientExchangeTags.status(this.response)).isEqualTo(Tag.of("status", "200"));
89+
assertThat(WebClientExchangeTags.status(this.response, null)).isEqualTo(Tag.of("status", "200"));
9090
}
9191

9292
@Test
9393
void statusWhenIOException() {
94-
assertThat(WebClientExchangeTags.status(new IOException())).isEqualTo(Tag.of("status", "IO_ERROR"));
94+
assertThat(WebClientExchangeTags.status(null, new IOException())).isEqualTo(Tag.of("status", "IO_ERROR"));
9595
}
9696

9797
@Test
9898
void statusWhenClientException() {
99-
assertThat(WebClientExchangeTags.status(new IllegalArgumentException()))
99+
assertThat(WebClientExchangeTags.status(null, new IllegalArgumentException()))
100100
.isEqualTo(Tag.of("status", "CLIENT_ERROR"));
101101
}
102102

103103
@Test
104104
void statusWhenNonStandard() {
105105
given(this.response.rawStatusCode()).willReturn(490);
106-
assertThat(WebClientExchangeTags.status(this.response)).isEqualTo(Tag.of("status", "490"));
106+
assertThat(WebClientExchangeTags.status(this.response, null)).isEqualTo(Tag.of("status", "490"));
107+
}
108+
109+
@Test
110+
void statusWhenCancelled() {
111+
assertThat(WebClientExchangeTags.status(null, null)).isEqualTo(Tag.of("status", "CLIENT_ERROR"));
107112
}
108113

109114
@Test

0 commit comments

Comments
 (0)