From 9337d992cbb1fe2236d79df507a34956812d1c1a Mon Sep 17 00:00:00 2001 From: Spencer Gibb Date: Mon, 1 Apr 2019 18:23:12 -0400 Subject: [PATCH] Updates metrics filter to use int status first. --- .../gateway/filter/GatewayMetricsFilter.java | 48 ++++++++++++------- .../support/ServerWebExchangeUtils.java | 3 ++ .../filter/GatewayMetricsFilterTests.java | 9 ++-- 3 files changed, 40 insertions(+), 20 deletions(-) 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 e105f6a9..a7ca7215 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 @@ -20,6 +20,8 @@ import io.micrometer.core.instrument.MeterRegistry; import io.micrometer.core.instrument.Tags; import io.micrometer.core.instrument.Timer; import io.micrometer.core.instrument.Timer.Sample; +import org.apache.commons.logging.Log; +import org.apache.commons.logging.LogFactory; import reactor.core.publisher.Mono; import org.springframework.cloud.gateway.route.Route; @@ -36,6 +38,8 @@ import static org.springframework.cloud.gateway.support.ServerWebExchangeUtils.G */ public class GatewayMetricsFilter implements GlobalFilter, Ordered { + private static final Log log = LogFactory.getLog(GatewayMetricsFilter.class); + private MeterRegistry meterRegistry; public GatewayMetricsFilter(MeterRegistry meterRegistry) { @@ -78,30 +82,42 @@ public class GatewayMetricsFilter implements GlobalFilter, Ordered { String httpStatusCodeStr = "NA"; String httpMethod = exchange.getRequest().getMethodValue(); - HttpStatus statusCode = exchange.getResponse().getStatusCode(); - if (statusCode != null) { - httpStatusCodeStr = String.valueOf(statusCode.value()); - outcome = statusCode.series().name(); - status = statusCode.name(); - } - else { // a non standard HTTPS status could be used. Let's be defensive here - if (exchange.getResponse() instanceof AbstractServerHttpResponse) { - Integer statusInt = ((AbstractServerHttpResponse) exchange.getResponse()) - .getStatusCodeValue(); - if (statusInt != null) { - status = String.valueOf(statusInt); - httpStatusCodeStr = status; - } - else { - status = "NA"; + + // a non standard HTTPS status could be used. Let's be defensive here + // it needs to be checked for first, otherwise the delegate response + // who's status DIDN"T change, will be used + if (exchange.getResponse() instanceof AbstractServerHttpResponse) { + Integer statusInt = ((AbstractServerHttpResponse) exchange.getResponse()) + .getStatusCodeValue(); + if (statusInt != null) { + status = String.valueOf(statusInt); + httpStatusCodeStr = status; + HttpStatus resolved = HttpStatus.resolve(statusInt); + if (resolved != null) { + // this is not a CUSTOM status, so use series here. + outcome = resolved.series().name(); + status = resolved.name(); } } } + else { + HttpStatus statusCode = exchange.getResponse().getStatusCode(); + if (statusCode != null) { + httpStatusCodeStr = String.valueOf(statusCode.value()); + outcome = statusCode.series().name(); + status = statusCode.name(); + } + } + // TODO refactor to allow Tags provider like in MetricsWebFilter Route route = exchange.getAttribute(GATEWAY_ROUTE_ATTR); Tags tags = Tags.of("outcome", outcome, "status", status, "httpStatusCode", httpStatusCodeStr, "routeId", route.getId(), "routeUri", route.getUri().toString(), "httpMethod", httpMethod); + + if (log.isTraceEnabled()) { + log.trace("gateway.requests tags: " + tags); + } sample.stop(meterRegistry.timer("gateway.requests", tags)); } diff --git a/spring-cloud-gateway-core/src/main/java/org/springframework/cloud/gateway/support/ServerWebExchangeUtils.java b/spring-cloud-gateway-core/src/main/java/org/springframework/cloud/gateway/support/ServerWebExchangeUtils.java index 90214c5b..d795193f 100644 --- a/spring-cloud-gateway-core/src/main/java/org/springframework/cloud/gateway/support/ServerWebExchangeUtils.java +++ b/spring-cloud-gateway-core/src/main/java/org/springframework/cloud/gateway/support/ServerWebExchangeUtils.java @@ -163,6 +163,9 @@ public final class ServerWebExchangeUtils { if (exchange.getResponse().isCommitted()) { return false; } + if (logger.isDebugEnabled()) { + logger.debug("Setting response status to " + statusHolder); + } if (statusHolder.getHttpStatus() != null) { return setResponseStatus(exchange, statusHolder.getHttpStatus()); } diff --git a/spring-cloud-gateway-core/src/test/java/org/springframework/cloud/gateway/filter/GatewayMetricsFilterTests.java b/spring-cloud-gateway-core/src/test/java/org/springframework/cloud/gateway/filter/GatewayMetricsFilterTests.java index 81619696..c1e93b11 100644 --- a/spring-cloud-gateway-core/src/test/java/org/springframework/cloud/gateway/filter/GatewayMetricsFilterTests.java +++ b/spring-cloud-gateway-core/src/test/java/org/springframework/cloud/gateway/filter/GatewayMetricsFilterTests.java @@ -85,7 +85,7 @@ public class GatewayMetricsFilterTests extends BaseWebClientTests { @Test public void hasMetricsForSetStatusFilter() throws InterruptedException { HttpHeaders headers = new HttpHeaders(); - headers.set(HttpHeaders.HOST, "www.setcustomstatus.org"); + headers.set(HttpHeaders.HOST, "www.setcustomstatusmetrics.org"); // cannot use netty client since we cannot read custom http status ResponseEntity response = new TestRestTemplate().exchange( baseUri + "/headers", HttpMethod.POST, new HttpEntity<>(headers), @@ -93,7 +93,7 @@ public class GatewayMetricsFilterTests extends BaseWebClientTests { assertThat(response.getStatusCodeValue()).isEqualTo(432); assertMetricsContainsTag("outcome", "CUSTOM"); assertMetricsContainsTag("status", "432"); - assertMetricsContainsTag("routeId", "test_custom_http_status"); + assertMetricsContainsTag("routeId", "test_custom_http_status_metrics"); assertMetricsContainsTag("routeUri", testUri); assertMetricsContainsTag("httpStatusCode", "432"); assertMetricsContainsTag("httpMethod", HttpMethod.POST.toString()); @@ -116,8 +116,9 @@ public class GatewayMetricsFilterTests extends BaseWebClientTests { @Bean public RouteLocator myRouteLocator(RouteLocatorBuilder builder) { return builder.routes() - .route("test_custom_http_status", r -> r.host("*.setcustomstatus.org") - .filters(f -> f.setStatus(432)).uri(testUri)) + .route("test_custom_http_status_metrics", + r -> r.host("*.setcustomstatusmetrics.org") + .filters(f -> f.setStatus(432)).uri(testUri)) .build(); }