diff --git a/java/google/registry/proxy/handler/BackendMetricsHandler.java b/java/google/registry/proxy/handler/BackendMetricsHandler.java index 7f0d81844..640118aad 100644 --- a/java/google/registry/proxy/handler/BackendMetricsHandler.java +++ b/java/google/registry/proxy/handler/BackendMetricsHandler.java @@ -113,6 +113,10 @@ public class BackendMetricsHandler extends ChannelDuplexHandler { } FullHttpRequest request = (FullHttpRequest) msg; + // Record request size now because the content would have read by the time the listener is + // called and the readable bytes would be zero by then. + int bytes = request.content().readableBytes(); + // Record sent time before write finishes allows us to take network latency into account. DateTime sentTime = clock.nowUtc(); ChannelFuture unusedFuture = @@ -121,7 +125,7 @@ public class BackendMetricsHandler extends ChannelDuplexHandler { future -> { if (future.isSuccess()) { // Only instrument request metrics when the request is actually sent to GAE. - metrics.requestSent(relayedProtocolName, clientCertHash, request); + metrics.requestSent(relayedProtocolName, clientCertHash, bytes); requestSentTimeQueue.add(sentTime); } }); diff --git a/java/google/registry/proxy/metric/BackendMetrics.java b/java/google/registry/proxy/metric/BackendMetrics.java index c6fa4310d..3682a910e 100644 --- a/java/google/registry/proxy/metric/BackendMetrics.java +++ b/java/google/registry/proxy/metric/BackendMetrics.java @@ -23,7 +23,6 @@ import com.google.monitoring.metrics.IncrementableMetric; import com.google.monitoring.metrics.LabelDescriptor; import com.google.monitoring.metrics.MetricRegistryImpl; import google.registry.util.NonFinalForTesting; -import io.netty.handler.codec.http.FullHttpRequest; import io.netty.handler.codec.http.FullHttpResponse; import javax.inject.Inject; import javax.inject.Singleton; @@ -112,9 +111,9 @@ public class BackendMetrics { } @NonFinalForTesting - public void requestSent(String protocol, String certHash, FullHttpRequest request) { + public void requestSent(String protocol, String certHash, int bytes) { requestsCounter.increment(protocol, certHash); - requestBytes.record(request.content().readableBytes(), protocol, certHash); + requestBytes.record(bytes, protocol, certHash); } @NonFinalForTesting diff --git a/javatests/google/registry/proxy/handler/BackendMetricsHandlerTest.java b/javatests/google/registry/proxy/handler/BackendMetricsHandlerTest.java index 58a09495a..1a34644e4 100644 --- a/javatests/google/registry/proxy/handler/BackendMetricsHandlerTest.java +++ b/javatests/google/registry/proxy/handler/BackendMetricsHandlerTest.java @@ -114,7 +114,8 @@ public class BackendMetricsHandlerTest { // outbound message passed to the next handler. assertThat(channel.writeOutbound(request)).isTrue(); assertHttpRequestEquivalent(request, channel.readOutbound()); - verify(metrics).requestSent(RELAYED_PROTOCOL_NAME, CLIENT_CERT_HASH, request); + verify(metrics) + .requestSent(RELAYED_PROTOCOL_NAME, CLIENT_CERT_HASH, request.content().readableBytes()); verifyNoMoreInteractions(metrics); } @@ -130,7 +131,8 @@ public class BackendMetricsHandlerTest { assertThat(channel.writeInbound(response)).isTrue(); assertHttpResponseEquivalent(response, channel.readInbound()); - verify(metrics).requestSent(RELAYED_PROTOCOL_NAME, CLIENT_CERT_HASH, request); + verify(metrics) + .requestSent(RELAYED_PROTOCOL_NAME, CLIENT_CERT_HASH, request.content().readableBytes()); verify(metrics).responseReceived(RELAYED_PROTOCOL_NAME, CLIENT_CERT_HASH, response, 1); verifyNoMoreInteractions(metrics); } @@ -150,7 +152,8 @@ public class BackendMetricsHandlerTest { assertThat(channel.writeInbound(response)).isTrue(); assertHttpResponseEquivalent(response, channel.readInbound()); - verify(metrics).requestSent(RELAYED_PROTOCOL_NAME, CLIENT_CERT_HASH, request); + verify(metrics) + .requestSent(RELAYED_PROTOCOL_NAME, CLIENT_CERT_HASH, request.content().readableBytes()); verify(metrics).responseReceived(RELAYED_PROTOCOL_NAME, CLIENT_CERT_HASH, response, 1); verifyNoMoreInteractions(metrics); } @@ -167,10 +170,10 @@ public class BackendMetricsHandlerTest { public void testSuccess_pipelinedResponses() { FullHttpRequest request1 = makeHttpPostRequest("request 1", HOST, "/"); FullHttpResponse response1 = makeHttpResponse("response 1", HttpResponseStatus.OK); - FullHttpRequest request2 = makeHttpPostRequest("request 2", HOST, "/"); - FullHttpResponse response2 = makeHttpResponse("response 2", HttpResponseStatus.OK); - FullHttpRequest request3 = makeHttpPostRequest("request 3", HOST, "/"); - FullHttpResponse response3 = makeHttpResponse("response 3", HttpResponseStatus.OK); + FullHttpRequest request2 = makeHttpPostRequest("request 22", HOST, "/"); + FullHttpResponse response2 = makeHttpResponse("response 22", HttpResponseStatus.OK); + FullHttpRequest request3 = makeHttpPostRequest("request 333", HOST, "/"); + FullHttpResponse response3 = makeHttpResponse("response 333", HttpResponseStatus.OK); // First request, time = 0 assertThat(channel.writeOutbound(request1)).isTrue(); @@ -216,9 +219,12 @@ public class BackendMetricsHandlerTest { long latency2 = new Duration(sentTime2, receivedTime2).getMillis(); long latency3 = new Duration(sentTime3, receivedTime3).getMillis(); - verify(metrics).requestSent(RELAYED_PROTOCOL_NAME, CLIENT_CERT_HASH, request1); - verify(metrics).requestSent(RELAYED_PROTOCOL_NAME, CLIENT_CERT_HASH, request2); - verify(metrics).requestSent(RELAYED_PROTOCOL_NAME, CLIENT_CERT_HASH, request3); + verify(metrics) + .requestSent(RELAYED_PROTOCOL_NAME, CLIENT_CERT_HASH, request1.content().readableBytes()); + verify(metrics) + .requestSent(RELAYED_PROTOCOL_NAME, CLIENT_CERT_HASH, request2.content().readableBytes()); + verify(metrics) + .requestSent(RELAYED_PROTOCOL_NAME, CLIENT_CERT_HASH, request3.content().readableBytes()); verify(metrics).responseReceived(RELAYED_PROTOCOL_NAME, CLIENT_CERT_HASH, response1, latency1); verify(metrics).responseReceived(RELAYED_PROTOCOL_NAME, CLIENT_CERT_HASH, response2, latency2); verify(metrics).responseReceived(RELAYED_PROTOCOL_NAME, CLIENT_CERT_HASH, response3, latency3); diff --git a/javatests/google/registry/proxy/metric/BackendMetricsTest.java b/javatests/google/registry/proxy/metric/BackendMetricsTest.java index db7d021fe..ea6dfd7b9 100644 --- a/javatests/google/registry/proxy/metric/BackendMetricsTest.java +++ b/javatests/google/registry/proxy/metric/BackendMetricsTest.java @@ -47,7 +47,7 @@ public class BackendMetricsTest { public void testSuccess_oneRequest() { String content = "some content"; FullHttpRequest request = makeHttpPostRequest(content, host, "/"); - metrics.requestSent(protocol, certHash, request); + metrics.requestSent(protocol, certHash, request.content().readableBytes()); assertThat(BackendMetrics.requestsCounter) .hasValueForLabels(1, protocol, certHash) @@ -68,8 +68,8 @@ public class BackendMetricsTest { String content2 = "some other content"; FullHttpRequest request1 = makeHttpPostRequest(content1, host, "/"); FullHttpRequest request2 = makeHttpPostRequest(content2, host, "/"); - metrics.requestSent(protocol, certHash, request1); - metrics.requestSent(protocol, certHash, request2); + metrics.requestSent(protocol, certHash, request1.content().readableBytes()); + metrics.requestSent(protocol, certHash, request2.content().readableBytes()); assertThat(BackendMetrics.requestsCounter) .hasValueForLabels(2, protocol, certHash) @@ -146,7 +146,7 @@ public class BackendMetricsTest { String responseContent = "the only response"; FullHttpRequest request = makeHttpPostRequest(requestContent, host, "/"); FullHttpResponse response = makeHttpResponse(responseContent, HttpResponseStatus.OK); - metrics.requestSent(protocol, certHash, request); + metrics.requestSent(protocol, certHash, request.content().readableBytes()); metrics.responseReceived(protocol, certHash, response, 10); assertThat(BackendMetrics.requestsCounter)