diff --git a/spring-cloud-gateway-core/src/main/java/org/springframework/cloud/gateway/filter/GatewayMetricsFilter.java b/spring-cloud-gateway-core/src/main/java/org/springframework/cloud/gateway/filter/GatewayMetricsFilter.java index 3788b32d..11ac4981 100644 --- a/spring-cloud-gateway-core/src/main/java/org/springframework/cloud/gateway/filter/GatewayMetricsFilter.java +++ b/spring-cloud-gateway-core/src/main/java/org/springframework/cloud/gateway/filter/GatewayMetricsFilter.java @@ -19,8 +19,6 @@ package org.springframework.cloud.gateway.filter; import static org.springframework.cloud.gateway.support.ServerWebExchangeUtils.GATEWAY_ROUTE_ATTR; -import java.util.Arrays; - import org.springframework.cloud.gateway.route.Route; import org.springframework.core.Ordered; import org.springframework.http.HttpStatus; @@ -29,7 +27,7 @@ import org.springframework.http.server.reactive.ServerHttpResponse; import org.springframework.web.server.ServerWebExchange; import io.micrometer.core.instrument.MeterRegistry; -import io.micrometer.core.instrument.Tag; +import io.micrometer.core.instrument.Tags; import io.micrometer.core.instrument.Timer; import io.micrometer.core.instrument.Timer.Sample; import reactor.core.publisher.Mono; @@ -44,17 +42,18 @@ public class GatewayMetricsFilter implements GlobalFilter, Ordered { @Override public int getOrder() { - return NettyWriteResponseFilter.WRITE_RESPONSE_FILTER_ORDER - 1; + // start the timer as soon as possible and report the metric event before we write + // response to client + return NettyWriteResponseFilter.WRITE_RESPONSE_FILTER_ORDER + 1; } @Override public Mono filter(ServerWebExchange exchange, GatewayFilterChain chain) { Sample sample = Timer.start(meterRegistry); - return chain.filter(exchange).then(Mono.fromRunnable(() -> { + + return chain.filter(exchange).doOnSuccessOrError((aVoid, ex) -> { endTimerRespectingCommit(exchange, sample); - })).doOnError(t -> { // needed for example when netty routing filter times out - endTimerRespectingCommit(exchange, sample); - }).then(); + }); } private void endTimerRespectingCommit(ServerWebExchange exchange, Sample sample) { @@ -69,7 +68,6 @@ public class GatewayMetricsFilter implements GlobalFilter, Ordered { return Mono.empty(); }); } - } private void endTimerInner(ServerWebExchange exchange, Sample sample) { @@ -93,9 +91,8 @@ public class GatewayMetricsFilter implements GlobalFilter, Ordered { } } Route route = exchange.getAttribute(GATEWAY_ROUTE_ATTR); - Iterable iterableTags = Arrays.asList(Tag.of("outcome", outcome), - Tag.of("status", status), Tag.of("routeId", route.getId()), - Tag.of("routeUri", route.getUri().toString())); - sample.stop(meterRegistry.timer("gateway.requests", iterableTags)); + Tags tags = Tags.of("outcome", outcome, "status", status, "routeId", + route.getId(), "routeUri", route.getUri().toString()); + sample.stop(meterRegistry.timer("gateway.requests", tags)); } } diff --git a/spring-cloud-gateway-core/src/test/java/org/springframework/cloud/gateway/filter/GatewayMetricFilterTests.java b/spring-cloud-gateway-core/src/test/java/org/springframework/cloud/gateway/filter/GatewayMetricFilterTests.java index 7c77ba64..46a957c9 100644 --- a/spring-cloud-gateway-core/src/test/java/org/springframework/cloud/gateway/filter/GatewayMetricFilterTests.java +++ b/spring-cloud-gateway-core/src/test/java/org/springframework/cloud/gateway/filter/GatewayMetricFilterTests.java @@ -18,7 +18,6 @@ package org.springframework.cloud.gateway.filter; import static org.assertj.core.api.Assertions.assertThat; -import static org.junit.Assert.assertEquals; import static org.springframework.boot.test.context.SpringBootTest.WebEnvironment.RANDOM_PORT; import org.junit.Test; @@ -29,7 +28,6 @@ import org.springframework.boot.SpringBootConfiguration; import org.springframework.boot.autoconfigure.EnableAutoConfiguration; import org.springframework.boot.test.context.SpringBootTest; import org.springframework.boot.test.web.client.TestRestTemplate; -import org.springframework.cloud.gateway.config.GatewayProperties; import org.springframework.cloud.gateway.route.RouteLocator; import org.springframework.cloud.gateway.route.builder.RouteLocatorBuilder; import org.springframework.cloud.gateway.test.BaseWebClientTests; @@ -44,7 +42,6 @@ import org.springframework.test.annotation.DirtiesContext; import org.springframework.test.context.junit4.SpringRunner; import org.springframework.web.bind.annotation.RequestMapping; import org.springframework.web.bind.annotation.RestController; -import org.springframework.web.reactive.function.client.ClientResponse; import io.micrometer.core.instrument.MeterRegistry; @@ -55,9 +52,6 @@ public class GatewayMetricFilterTests extends BaseWebClientTests { private static final String REQUEST_METRICS_NAME = "gateway.requests"; - @Autowired - private GatewayProperties properties; - @Autowired private MeterRegistry meterRegistry; @@ -66,11 +60,7 @@ public class GatewayMetricFilterTests extends BaseWebClientTests { @Test public void gatewayRequestsMeterFilterHasTags() throws InterruptedException { - assertThat(this.properties.getDefaultFilters()).isNotEmpty(); - ClientResponse clientResponse = webClient.get().uri("/headers").exchange() - .block(); - assertEquals(HttpStatus.OK, clientResponse.statusCode()); - Thread.sleep(1000); // allow metrics to complete in the mono following the then + testClient.get().uri("/headers").exchange().expectStatus().isOk(); assertMetricsContainsTag("outcome", HttpStatus.Series.SUCCESSFUL.name()); assertMetricsContainsTag("status", HttpStatus.OK.name()); assertMetricsContainsTag("routeId", "default_path_to_httpbin"); @@ -80,12 +70,8 @@ public class GatewayMetricFilterTests extends BaseWebClientTests { @Test public void gatewayRequestsMeterFilterHasTagsForBadTargetUri() throws InterruptedException { - assertThat(this.properties.getDefaultFilters()).isNotEmpty(); - ClientResponse clientResponse = webClient.get().uri("/badtargeturi").exchange() - .block(); - assertEquals("Expecting request to fail with http status internal server error", - HttpStatus.INTERNAL_SERVER_ERROR, clientResponse.statusCode()); - Thread.sleep(1000); // allow metrics to complete in the mono following the then + testClient.get().uri("/badtargeturi").exchange().expectStatus() + .is5xxServerError(); assertMetricsContainsTag("outcome", HttpStatus.Series.SERVER_ERROR.name()); assertMetricsContainsTag("status", HttpStatus.INTERNAL_SERVER_ERROR.name()); assertMetricsContainsTag("routeId", "default_path_to_httpbin"); @@ -96,11 +82,11 @@ public class GatewayMetricFilterTests extends BaseWebClientTests { public void hasMetricsForSetStatusFilter() throws InterruptedException { HttpHeaders headers = new HttpHeaders(); headers.set(HttpHeaders.HOST, "www.setcustomstatus.org"); + // cannot use netty client since we cannot read custom http status ResponseEntity response = new TestRestTemplate().exchange( baseUri + "/headers", HttpMethod.GET, new HttpEntity<>(headers), String.class); assertThat(response.getStatusCodeValue()).isEqualTo(432); - Thread.sleep(1000); // allow metrics to complete in the mono following the then assertMetricsContainsTag("outcome", "CUSTOM"); assertMetricsContainsTag("status", "432"); assertMetricsContainsTag("routeId", "test_custom_http_status");