Polish gh-11514

As pointed out by Rossen in gh-11514 comments, a handler might commit
the response and then send an error signal in the pipeline. In this
case, adding a callback to `beforeCommit` is useless because it won't be
triggered. In those cases, we need to collect metrics right away.
pull/12890/head
Brian Clozel 7 years ago
parent 9c7612e681
commit b2e7be17ab

@ -62,8 +62,17 @@ public class MetricsWebFilter implements WebFilter {
long start = System.nanoTime(); long start = System.nanoTime();
ServerHttpResponse response = exchange.getResponse(); ServerHttpResponse response = exchange.getResponse();
return call.doOnSuccess((done) -> success(exchange, start)) return call.doOnSuccess((done) -> success(exchange, start))
.doOnError((cause) -> response .doOnError((cause) -> {
.beforeCommit(() -> error(exchange, start, cause))); if (response.isCommitted()) {
error(exchange, start, cause);
}
else {
response.beforeCommit(() -> {
error(exchange, start, cause);
return Mono.empty();
});
}
});
} }
private void success(ServerWebExchange exchange, long start) { private void success(ServerWebExchange exchange, long start) {
@ -72,11 +81,10 @@ public class MetricsWebFilter implements WebFilter {
TimeUnit.NANOSECONDS); TimeUnit.NANOSECONDS);
} }
private Mono<Void> error(ServerWebExchange exchange, long start, Throwable cause) { private void error(ServerWebExchange exchange, long start, Throwable cause) {
Iterable<Tag> tags = this.tagsProvider.httpRequestTags(exchange, cause); Iterable<Tag> tags = this.tagsProvider.httpRequestTags(exchange, cause);
this.registry.timer(this.metricName, tags).record(System.nanoTime() - start, this.registry.timer(this.metricName, tags).record(System.nanoTime() - start,
TimeUnit.NANOSECONDS); TimeUnit.NANOSECONDS);
return Mono.empty();
} }
} }

@ -74,6 +74,22 @@ public class MetricsWebFilterTests {
assertMetricsContainsTag("status", "500"); assertMetricsContainsTag("status", "500");
} }
@Test
public void filterAddsTagsToRegistryForExceptionsAndCommittedResponse() {
MockServerWebExchange exchange = createExchange("/projects/spring-boot",
"/projects/{project}");
this.webFilter.filter(exchange,
serverWebExchange -> {
exchange.getResponse().setStatusCodeValue(500);
return exchange.getResponse().setComplete()
.then(Mono.error(new IllegalStateException("test error")));
})
.onErrorResume(t -> Mono.empty())
.block();
assertMetricsContainsTag("uri", "/projects/{project}");
assertMetricsContainsTag("status", "500");
}
private MockServerWebExchange createExchange(String path, String pathPattern) { private MockServerWebExchange createExchange(String path, String pathPattern) {
PathPatternParser parser = new PathPatternParser(); PathPatternParser parser = new PathPatternParser();
MockServerWebExchange exchange = MockServerWebExchange MockServerWebExchange exchange = MockServerWebExchange

Loading…
Cancel
Save