Update metrics with feedback from Ryan and Jon. Metrics are now reported before we send the response back to the client. Hence no need for sleeps in the client side test.

This commit is contained in:
Tony Clarke
2018-07-25 15:57:38 -04:00
parent 22312fb2c1
commit ca915c9b5a
2 changed files with 14 additions and 31 deletions

View File

@@ -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<Void> 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<Tag> 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));
}
}

View File

@@ -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<String> 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");