diff --git a/proxy/build.gradle b/proxy/build.gradle index c545e8240..1cd16e118 100644 --- a/proxy/build.gradle +++ b/proxy/build.gradle @@ -18,7 +18,8 @@ task buildProxyImage(dependsOn: deployJar, type: Exec) { commandLine 'docker', 'build', '-t', 'proxy', '.' } -task deployProxy(dependsOn: [buildProxyImage, ':verifyDeployment']) { +task deployProxy(dependsOn: buildProxyImage) { + configure verifyDeploymentConfig doLast { exec { commandLine 'docker', 'tag', 'proxy', "gcr.io/${rootProject.gcpProject}/proxy" diff --git a/proxy/src/main/java/google/registry/proxy/EppProtocolModule.java b/proxy/src/main/java/google/registry/proxy/EppProtocolModule.java index 819138855..d93b75fd8 100644 --- a/proxy/src/main/java/google/registry/proxy/EppProtocolModule.java +++ b/proxy/src/main/java/google/registry/proxy/EppProtocolModule.java @@ -24,6 +24,7 @@ import google.registry.proxy.HttpsRelayProtocolModule.HttpsRelayProtocol; import google.registry.proxy.Protocol.BackendProtocol; import google.registry.proxy.Protocol.FrontendProtocol; import google.registry.proxy.handler.EppServiceHandler; +import google.registry.proxy.handler.FrontendMetricsHandler; import google.registry.proxy.handler.ProxyProtocolHandler; import google.registry.proxy.handler.QuotaHandler.EppQuotaHandler; import google.registry.proxy.handler.RelayHandler.FullHttpRequestRelayHandler; @@ -85,6 +86,7 @@ public class EppProtocolModule { Provider lengthFieldBasedFrameDecoderProvider, Provider lengthFieldPrependerProvider, Provider eppServiceHandlerProvider, + Provider frontendMetricsHandlerProvider, Provider eppQuotaHandlerProvider, Provider relayHandlerProvider) { return ImmutableList.of( @@ -94,6 +96,7 @@ public class EppProtocolModule { lengthFieldBasedFrameDecoderProvider, lengthFieldPrependerProvider, eppServiceHandlerProvider, + frontendMetricsHandlerProvider, eppQuotaHandlerProvider, relayHandlerProvider); } diff --git a/proxy/src/main/java/google/registry/proxy/WhoisProtocolModule.java b/proxy/src/main/java/google/registry/proxy/WhoisProtocolModule.java index 5e967e306..a8f573df3 100644 --- a/proxy/src/main/java/google/registry/proxy/WhoisProtocolModule.java +++ b/proxy/src/main/java/google/registry/proxy/WhoisProtocolModule.java @@ -21,6 +21,7 @@ import dagger.multibindings.IntoSet; import google.registry.proxy.HttpsRelayProtocolModule.HttpsRelayProtocol; import google.registry.proxy.Protocol.BackendProtocol; import google.registry.proxy.Protocol.FrontendProtocol; +import google.registry.proxy.handler.FrontendMetricsHandler; import google.registry.proxy.handler.ProxyProtocolHandler; import google.registry.proxy.handler.QuotaHandler.WhoisQuotaHandler; import google.registry.proxy.handler.RelayHandler.FullHttpRequestRelayHandler; @@ -74,6 +75,7 @@ public class WhoisProtocolModule { @WhoisProtocol Provider readTimeoutHandlerProvider, Provider lineBasedFrameDecoderProvider, Provider whoisServiceHandlerProvider, + Provider frontendMetricsHandlerProvider, Provider whoisQuotaHandlerProvider, Provider relayHandlerProvider) { return ImmutableList.of( @@ -81,6 +83,7 @@ public class WhoisProtocolModule { readTimeoutHandlerProvider, lineBasedFrameDecoderProvider, whoisServiceHandlerProvider, + frontendMetricsHandlerProvider, whoisQuotaHandlerProvider, relayHandlerProvider); } diff --git a/proxy/src/main/java/google/registry/proxy/handler/BackendMetricsHandler.java b/proxy/src/main/java/google/registry/proxy/handler/BackendMetricsHandler.java index 5371deae1..236258db2 100644 --- a/proxy/src/main/java/google/registry/proxy/handler/BackendMetricsHandler.java +++ b/proxy/src/main/java/google/registry/proxy/handler/BackendMetricsHandler.java @@ -36,14 +36,16 @@ import java.util.Optional; import java.util.Queue; import javax.inject.Inject; import org.joda.time.DateTime; +import org.joda.time.Duration; /** - * Handler that records metrics a backend channel. + * Handler that records metrics for a backend channel. * - *

This handler is added right before {@link FullHttpResponseRelayHandler} in the backend - * protocol handler provider method. {@link FullHttpRequest} outbound messages encounter this first - * before being handed over to HTTP related handler. {@link FullHttpResponse} inbound messages are - * first constructed (from plain bytes) by preceding handlers and then logged in this handler. + *

This handler is added before the {@link FullHttpResponseRelayHandler} in the backend protocol + * handler provider method. {@link FullHttpRequest} outbound messages encounter this first before + * being handed over to HTTP related handler. {@link FullHttpResponse} inbound messages are first + * constructed (from plain bytes) by preceding handlers and then related metrics are instrumented in + * this handler. */ public class BackendMetricsHandler extends ChannelDuplexHandler { @@ -95,7 +97,7 @@ public class BackendMetricsHandler extends ChannelDuplexHandler { relayedProtocolName, clientCertHash, (FullHttpResponse) msg, - clock.nowUtc().getMillis() - requestSentTimeQueue.remove().getMillis()); + new Duration(requestSentTimeQueue.remove().getMillis(), clock.nowUtc().getMillis())); super.channelRead(ctx, msg); } @@ -118,8 +120,6 @@ public class BackendMetricsHandler extends ChannelDuplexHandler { // 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 = ctx.write(msg, promise) .addListener( @@ -127,7 +127,7 @@ public class BackendMetricsHandler extends ChannelDuplexHandler { if (future.isSuccess()) { // Only instrument request metrics when the request is actually sent to GAE. metrics.requestSent(relayedProtocolName, clientCertHash, bytes); - requestSentTimeQueue.add(sentTime); + requestSentTimeQueue.add(clock.nowUtc()); } }); } diff --git a/proxy/src/main/java/google/registry/proxy/handler/FrontendMetricsHandler.java b/proxy/src/main/java/google/registry/proxy/handler/FrontendMetricsHandler.java new file mode 100644 index 000000000..582c0c6e1 --- /dev/null +++ b/proxy/src/main/java/google/registry/proxy/handler/FrontendMetricsHandler.java @@ -0,0 +1,121 @@ +// Copyright 2017 The Nomulus Authors. All Rights Reserved. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package google.registry.proxy.handler; + +import static com.google.common.base.Preconditions.checkState; +import static google.registry.proxy.Protocol.PROTOCOL_KEY; +import static google.registry.proxy.handler.EppServiceHandler.CLIENT_CERTIFICATE_HASH_KEY; + +import google.registry.proxy.metric.FrontendMetrics; +import google.registry.util.Clock; +import io.netty.channel.ChannelDuplexHandler; +import io.netty.channel.ChannelFuture; +import io.netty.channel.ChannelHandlerContext; +import io.netty.channel.ChannelPromise; +import java.util.ArrayDeque; +import java.util.Optional; +import java.util.Queue; +import javax.inject.Inject; +import org.joda.time.DateTime; +import org.joda.time.Duration; + +/** + * Handler that records metrics for a fronend channel. + * + *

This handler is added before the {@link RelayHandler} in the frontend protocol handler + * provider method. Outbound messages encounter this first before being handed over to + * protocol-specific handlers. Inbound messages are first constructed (from plain bytes) by + * preceding handlers and then related metrics are instrumented in this handler. + */ +public class FrontendMetricsHandler extends ChannelDuplexHandler { + + private final Clock clock; + private final FrontendMetrics metrics; + + private String protocolName; + private String clientCertHash; + + /** + * A queue that saves the time at which a request is received from the client. + * + *

This queue is used to calculate frontend request-response latency. + * + *

For the WHOIS protocol, the TCP connection closes after one request-response round trip and + * the request always comes first. The queue for WHOIS therefore only need to store one value. + * + *

For the EPP protocol, the specification allows for pipelining, in which a client can sent + * multiple requests without waiting for each responses. Therefore a queue is needed to record all + * the requests that are sent but have not yet received a response. + * + *

A server must send its response in the same order it receives requests. This invariance + * guarantees that the request time at the head of the queue always corresponds to the response + * received in {@link #channelRead}. + * + * @see RFC 3912 WHOIS Protocol Specification + * @see RFC 5734 Extensible Provisioning + * Protocol (EPP) Transport over TCP + */ + private final Queue requestReceivedTimeQueue = new ArrayDeque<>(); + + @Inject + FrontendMetricsHandler(Clock clock, FrontendMetrics metrics) { + this.clock = clock; + this.metrics = metrics; + } + + @Override + public void channelRegistered(ChannelHandlerContext ctx) throws Exception { + protocolName = ctx.channel().attr(PROTOCOL_KEY).get().name(); + super.channelRegistered(ctx); + } + + @Override + public void channelRead(ChannelHandlerContext ctx, Object msg) throws Exception { + requestReceivedTimeQueue.add(clock.nowUtc()); + super.channelRead(ctx, msg); + } + + @Override + public void write(ChannelHandlerContext ctx, Object msg, ChannelPromise promise) + throws Exception { + // Only instrument request metrics when the response is actually sent to client. + // It is OK to check the queue size preemptively here, not when the front element of the queue + // is acutally removed after the write to the client is successful, because responses are + // written to the client in order. Hence there cannot be any response succsssfully sent to the + // client (which reduces the queue size) before this current request is sent. The queue *can* + // increase in size if more requests are received from the client, but that does not invalidate + // this check. + checkState(!requestReceivedTimeQueue.isEmpty(), "Response sent before request is received."); + // For WHOIS, client certificate hash is always set to "none". + // For EPP, the client hash attribute is set upon handshake completion, before the first HELLO + // is sent to the server. Therefore the first call to write() with HELLO payload has access to + // the hash in its channel attribute. + if (clientCertHash == null) { + clientCertHash = + Optional.ofNullable(ctx.channel().attr(CLIENT_CERTIFICATE_HASH_KEY).get()).orElse("none"); + } + ChannelFuture unusedFuture = + ctx.write(msg, promise) + .addListener( + future -> { + if (future.isSuccess()) { + metrics.responseSent( + protocolName, + clientCertHash, + new Duration(requestReceivedTimeQueue.remove(), clock.nowUtc())); + } + }); + } +} diff --git a/proxy/src/main/java/google/registry/proxy/metric/BackendMetrics.java b/proxy/src/main/java/google/registry/proxy/metric/BackendMetrics.java index 3682a910e..3653d2c3f 100644 --- a/proxy/src/main/java/google/registry/proxy/metric/BackendMetrics.java +++ b/proxy/src/main/java/google/registry/proxy/metric/BackendMetrics.java @@ -15,10 +15,7 @@ package google.registry.proxy.metric; import com.google.common.collect.ImmutableSet; -import com.google.monitoring.metrics.CustomFitter; import com.google.monitoring.metrics.EventMetric; -import com.google.monitoring.metrics.ExponentialFitter; -import com.google.monitoring.metrics.FibonacciFitter; import com.google.monitoring.metrics.IncrementableMetric; import com.google.monitoring.metrics.LabelDescriptor; import com.google.monitoring.metrics.MetricRegistryImpl; @@ -26,26 +23,11 @@ import google.registry.util.NonFinalForTesting; import io.netty.handler.codec.http.FullHttpResponse; import javax.inject.Inject; import javax.inject.Singleton; +import org.joda.time.Duration; /** Backend metrics instrumentation. */ @Singleton -public class BackendMetrics { - - // Maximum request size is defined in the config file, this is not realistic and we'd be out of - // memory when the size approach 1 GB. - private static final CustomFitter DEFAULT_SIZE_FITTER = FibonacciFitter.create(1073741824); - - // Maximum 1 hour latency, this is not specified by the spec, but given we have a one hour idle - // timeout, it seems reasonable that maximum latency is set to 1 hour as well. If we are - // approaching anywhere near 1 hour latency, we'd be way out of SLO anyway. - private static final ExponentialFitter DEFAULT_LATENCY_FITTER = - ExponentialFitter.create(22, 2, 1.0); - - private static final ImmutableSet LABELS = - ImmutableSet.of( - LabelDescriptor.create("protocol", "Name of the protocol."), - LabelDescriptor.create( - "client_cert_hash", "SHA256 hash of the client certificate, if available.")); +public class BackendMetrics extends BaseMetrics { static final IncrementableMetric requestsCounter = MetricRegistryImpl.getDefault() @@ -96,13 +78,8 @@ public class BackendMetrics { @Inject BackendMetrics() {} - /** - * Resets all backend metrics. - * - *

This should only used in tests to clear out states. No production code should call this - * function. - */ - void resetMetric() { + @Override + void resetMetrics() { requestBytes.reset(); requestsCounter.reset(); responseBytes.reset(); @@ -118,8 +95,8 @@ public class BackendMetrics { @NonFinalForTesting public void responseReceived( - String protocol, String certHash, FullHttpResponse response, long latency) { - latencyMs.record(latency, protocol, certHash); + String protocol, String certHash, FullHttpResponse response, Duration latency) { + latencyMs.record(latency.getMillis(), protocol, certHash); responseBytes.record(response.content().readableBytes(), protocol, certHash); responsesCounter.increment(protocol, certHash, response.status().toString()); } diff --git a/proxy/src/main/java/google/registry/proxy/metric/BaseMetrics.java b/proxy/src/main/java/google/registry/proxy/metric/BaseMetrics.java new file mode 100644 index 000000000..dd62251fb --- /dev/null +++ b/proxy/src/main/java/google/registry/proxy/metric/BaseMetrics.java @@ -0,0 +1,62 @@ +// Copyright 2019 The Nomulus Authors. All Rights Reserved. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package google.registry.proxy.metric; + +import com.google.common.annotations.VisibleForTesting; +import com.google.common.collect.ImmutableSet; +import com.google.monitoring.metrics.CustomFitter; +import com.google.monitoring.metrics.ExponentialFitter; +import com.google.monitoring.metrics.FibonacciFitter; +import com.google.monitoring.metrics.LabelDescriptor; + +/** Base class for metrics. */ +public abstract class BaseMetrics { + + /** + * Labels to register metrics with. + * + *

The client certificate hash value is only used for EPP metrics. For WHOIS metrics, it will + * always be {@code "none"}. In order to get the actual registrar name, one can use the {@code + * nomulus} tool: + * + * 

+   * nomulus -e production list_registrars -f clientCertificateHash | grep $HASH
+   *
+ */ + protected static final ImmutableSet LABELS = + ImmutableSet.of( + LabelDescriptor.create("protocol", "Name of the protocol."), + LabelDescriptor.create( + "client_cert_hash", "SHA256 hash of the client certificate, if available.")); + + // Maximum request size is defined in the config file, this is not realistic and we'd be out of + // memory when the size approaches 1 GB. + protected static final CustomFitter DEFAULT_SIZE_FITTER = FibonacciFitter.create(1073741824); + + // Maximum 1 hour latency, this is not specified by the spec, but given we have a one hour idle + // timeout, it seems reasonable that maximum latency is set to 1 hour as well. If we are + // approaching anywhere near 1 hour latency, we'd be way out of SLO anyway. + protected static final ExponentialFitter DEFAULT_LATENCY_FITTER = + ExponentialFitter.create(22, 2, 1.0); + + /** + * Resets all metrics. + * + *

This should only be used in tests to reset states. Production code should not call this + * method. + */ + @VisibleForTesting + abstract void resetMetrics(); +} diff --git a/proxy/src/main/java/google/registry/proxy/metric/FrontendMetrics.java b/proxy/src/main/java/google/registry/proxy/metric/FrontendMetrics.java index feb414e83..2a2f9aa7e 100644 --- a/proxy/src/main/java/google/registry/proxy/metric/FrontendMetrics.java +++ b/proxy/src/main/java/google/registry/proxy/metric/FrontendMetrics.java @@ -14,12 +14,10 @@ package google.registry.proxy.metric; -import com.google.common.annotations.VisibleForTesting; import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableMap; -import com.google.common.collect.ImmutableSet; +import com.google.monitoring.metrics.EventMetric; import com.google.monitoring.metrics.IncrementableMetric; -import com.google.monitoring.metrics.LabelDescriptor; import com.google.monitoring.metrics.Metric; import com.google.monitoring.metrics.MetricRegistryImpl; import google.registry.util.NonFinalForTesting; @@ -32,27 +30,11 @@ import java.util.concurrent.ConcurrentHashMap; import java.util.concurrent.ConcurrentMap; import javax.inject.Inject; import javax.inject.Singleton; +import org.joda.time.Duration; /** Frontend metrics instrumentation. */ @Singleton -public class FrontendMetrics { - - /** - * Labels to register front metrics with. - * - *

The client certificate hash value is only used for EPP metrics. For WHOIS metrics, it will - * always be {@code "none"}. In order to get the actual registrar name, one can use the {@code - * nomulus} tool: - * - * 

-   * nomulus -e production list_registrars -f clientCertificateHash | grep $HASH
-   *
- */ - private static final ImmutableSet LABELS = - ImmutableSet.of( - LabelDescriptor.create("protocol", "Name of the protocol."), - LabelDescriptor.create( - "client_cert_hash", "SHA256 hash of the client certificate, if available.")); +public class FrontendMetrics extends BaseMetrics { private static final ConcurrentMap, ChannelGroup> activeConnections = new ConcurrentHashMap<>(); @@ -65,9 +47,7 @@ public class FrontendMetrics { "Active Connections", LABELS, () -> - activeConnections - .entrySet() - .stream() + activeConnections.entrySet().stream() .collect( ImmutableMap.toImmutableMap( Map.Entry::getKey, entry -> (long) entry.getValue().size())), @@ -89,19 +69,23 @@ public class FrontendMetrics { "Quota Rejections", LABELS); + static final EventMetric latencyMs = + MetricRegistryImpl.getDefault() + .newEventMetric( + "/proxy/frontend/latency_ms", + "Round-trip time between a request received and its corresponding response is sent.", + "Latency Milliseconds", + LABELS, + DEFAULT_LATENCY_FITTER); + @Inject public FrontendMetrics() {} - /** - * Resets all frontend metrics. - * - *

This should only be used in tests to reset states. Production code should not call this - * method. - */ - @VisibleForTesting + @Override void resetMetrics() { totalConnectionsCounter.reset(); activeConnections.clear(); + latencyMs.reset(); } @NonFinalForTesting @@ -122,4 +106,9 @@ public class FrontendMetrics { public void registerQuotaRejection(String protocol, String certHash) { quotaRejectionsCounter.increment(protocol, certHash); } + + @NonFinalForTesting + public void responseSent(String protocol, String certHash, Duration latency) { + latencyMs.record(latency.getMillis(), protocol, certHash); + } } diff --git a/proxy/src/test/java/google/registry/proxy/ProtocolModuleTest.java b/proxy/src/test/java/google/registry/proxy/ProtocolModuleTest.java index dcc2630b0..c7b550f76 100644 --- a/proxy/src/test/java/google/registry/proxy/ProtocolModuleTest.java +++ b/proxy/src/test/java/google/registry/proxy/ProtocolModuleTest.java @@ -32,6 +32,7 @@ import google.registry.proxy.ProxyConfig.Environment; import google.registry.proxy.WebWhoisProtocolsModule.HttpWhoisProtocol; import google.registry.proxy.WhoisProtocolModule.WhoisProtocol; import google.registry.proxy.handler.BackendMetricsHandler; +import google.registry.proxy.handler.FrontendMetricsHandler; import google.registry.proxy.handler.ProxyProtocolHandler; import google.registry.proxy.handler.QuotaHandler.EppQuotaHandler; import google.registry.proxy.handler.QuotaHandler.WhoisQuotaHandler; @@ -74,7 +75,7 @@ import org.junit.Before; */ public abstract class ProtocolModuleTest { - protected static final ProxyConfig PROXY_CONFIG = getProxyConfig(LOCAL); + protected static final ProxyConfig PROXY_CONFIG = getProxyConfig(Environment.LOCAL); protected TestComponent testComponent; @@ -106,6 +107,7 @@ public abstract class ProtocolModuleTest { LoggingHandler.class, // Metrics instrumentation is tested separately. BackendMetricsHandler.class, + FrontendMetricsHandler.class, // Quota management is tested separately. WhoisQuotaHandler.class, EppQuotaHandler.class, diff --git a/proxy/src/test/java/google/registry/proxy/handler/BackendMetricsHandlerTest.java b/proxy/src/test/java/google/registry/proxy/handler/BackendMetricsHandlerTest.java index 1a34644e4..f2f325c75 100644 --- a/proxy/src/test/java/google/registry/proxy/handler/BackendMetricsHandlerTest.java +++ b/proxy/src/test/java/google/registry/proxy/handler/BackendMetricsHandlerTest.java @@ -133,7 +133,8 @@ public class BackendMetricsHandlerTest { verify(metrics) .requestSent(RELAYED_PROTOCOL_NAME, CLIENT_CERT_HASH, request.content().readableBytes()); - verify(metrics).responseReceived(RELAYED_PROTOCOL_NAME, CLIENT_CERT_HASH, response, 1); + verify(metrics) + .responseReceived(RELAYED_PROTOCOL_NAME, CLIENT_CERT_HASH, response, Duration.millis(1)); verifyNoMoreInteractions(metrics); } @@ -154,7 +155,8 @@ public class BackendMetricsHandlerTest { verify(metrics) .requestSent(RELAYED_PROTOCOL_NAME, CLIENT_CERT_HASH, request.content().readableBytes()); - verify(metrics).responseReceived(RELAYED_PROTOCOL_NAME, CLIENT_CERT_HASH, response, 1); + verify(metrics) + .responseReceived(RELAYED_PROTOCOL_NAME, CLIENT_CERT_HASH, response, Duration.millis(1)); verifyNoMoreInteractions(metrics); } @@ -178,46 +180,46 @@ public class BackendMetricsHandlerTest { // First request, time = 0 assertThat(channel.writeOutbound(request1)).isTrue(); assertHttpRequestEquivalent(request1, channel.readOutbound()); - DateTime sentTime1 = fakeClock.nowUtc(); + DateTime requestTime1 = fakeClock.nowUtc(); fakeClock.advanceBy(Duration.millis(5)); // Second request, time = 5 assertThat(channel.writeOutbound(request2)).isTrue(); assertHttpRequestEquivalent(request2, channel.readOutbound()); - DateTime sentTime2 = fakeClock.nowUtc(); + DateTime requestTime2 = fakeClock.nowUtc(); fakeClock.advanceBy(Duration.millis(7)); // First response, time = 12, latency = 12 - 0 = 12 assertThat(channel.writeInbound(response1)).isTrue(); assertHttpResponseEquivalent(response1, channel.readInbound()); - DateTime receivedTime1 = fakeClock.nowUtc(); + DateTime responseTime1 = fakeClock.nowUtc(); fakeClock.advanceBy(Duration.millis(11)); // Third request, time = 23 assertThat(channel.writeOutbound(request3)).isTrue(); assertHttpRequestEquivalent(request3, channel.readOutbound()); - DateTime sentTime3 = fakeClock.nowUtc(); + DateTime requestTime3 = fakeClock.nowUtc(); fakeClock.advanceBy(Duration.millis(2)); // Second response, time = 25, latency = 25 - 5 = 20 assertThat(channel.writeInbound(response2)).isTrue(); assertHttpResponseEquivalent(response2, channel.readInbound()); - DateTime receivedTime2 = fakeClock.nowUtc(); + DateTime responseTime2 = fakeClock.nowUtc(); fakeClock.advanceBy(Duration.millis(4)); // Third response, time = 29, latency = 29 - 23 = 6 assertThat(channel.writeInbound(response3)).isTrue(); assertHttpResponseEquivalent(response3, channel.readInbound()); - DateTime receivedTime3 = fakeClock.nowUtc(); + DateTime responseTime3 = fakeClock.nowUtc(); - long latency1 = new Duration(sentTime1, receivedTime1).getMillis(); - long latency2 = new Duration(sentTime2, receivedTime2).getMillis(); - long latency3 = new Duration(sentTime3, receivedTime3).getMillis(); + Duration latency1 = new Duration(requestTime1, responseTime1); + Duration latency2 = new Duration(requestTime2, responseTime2); + Duration latency3 = new Duration(requestTime3, responseTime3); verify(metrics) .requestSent(RELAYED_PROTOCOL_NAME, CLIENT_CERT_HASH, request1.content().readableBytes()); diff --git a/proxy/src/test/java/google/registry/proxy/handler/FrontendMetricsHandlerTest.java b/proxy/src/test/java/google/registry/proxy/handler/FrontendMetricsHandlerTest.java new file mode 100644 index 000000000..e26481e45 --- /dev/null +++ b/proxy/src/test/java/google/registry/proxy/handler/FrontendMetricsHandlerTest.java @@ -0,0 +1,169 @@ +// Copyright 2017 The Nomulus Authors. All Rights Reserved. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package google.registry.proxy.handler; + +import static com.google.common.truth.Truth.assertThat; +import static google.registry.proxy.Protocol.PROTOCOL_KEY; +import static google.registry.proxy.handler.EppServiceHandler.CLIENT_CERTIFICATE_HASH_KEY; +import static google.registry.testing.JUnitBackports.assertThrows; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.verifyNoMoreInteractions; +import static org.mockito.Mockito.verifyZeroInteractions; + +import com.google.common.collect.ImmutableList; +import google.registry.proxy.Protocol; +import google.registry.proxy.Protocol.FrontendProtocol; +import google.registry.proxy.metric.FrontendMetrics; +import google.registry.testing.FakeClock; +import io.netty.channel.ChannelInitializer; +import io.netty.channel.embedded.EmbeddedChannel; +import org.joda.time.DateTime; +import org.joda.time.Duration; +import org.junit.Before; +import org.junit.Test; +import org.junit.runner.RunWith; +import org.junit.runners.JUnit4; + +/** Unit tests for {@link FrontendMetricsHandler}. */ +@RunWith(JUnit4.class) +public class FrontendMetricsHandlerTest { + + private static final String CLIENT_CERT_HASH = "blah12345"; + private static final String PROTOCOL_NAME = "frontend protocol"; + + private final FakeClock fakeClock = new FakeClock(); + private final FrontendMetrics metrics = mock(FrontendMetrics.class); + private final FrontendMetricsHandler handler = new FrontendMetricsHandler(fakeClock, metrics); + + private final FrontendProtocol frontendProtocol = + Protocol.frontendBuilder() + .name(PROTOCOL_NAME) + .port(2) + .hasBackend(false) + .handlerProviders(ImmutableList.of()) + .build(); + + private EmbeddedChannel channel; + + @Before + public void setUp() { + channel = + new EmbeddedChannel( + new ChannelInitializer() { + @Override + protected void initChannel(EmbeddedChannel ch) throws Exception { + ch.attr(PROTOCOL_KEY).set(frontendProtocol); + ch.attr(CLIENT_CERTIFICATE_HASH_KEY).set(CLIENT_CERT_HASH); + ch.pipeline().addLast(handler); + } + }); + } + + @Test + public void testSuccess_oneRequest() { + // Inbound message passed to the next handler. + Object request = new Object(); + assertThat(channel.writeInbound(request)).isTrue(); + assertThat((Object) channel.readInbound()).isEqualTo(request); + verifyZeroInteractions(metrics); + } + + @Test + public void testSuccess_oneRequest_oneResponse() { + Object request = new Object(); + Object response = new Object(); + // Inbound message passed to the next handler. + assertThat(channel.writeInbound(request)).isTrue(); + assertThat((Object) channel.readInbound()).isEqualTo(request); + fakeClock.advanceOneMilli(); + // Outbound message passed to the next handler. + assertThat(channel.writeOutbound(response)).isTrue(); + assertThat((Object) channel.readOutbound()).isEqualTo(response); + // Verify that latency is recorded. + verify(metrics).responseSent(PROTOCOL_NAME, CLIENT_CERT_HASH, Duration.millis(1)); + verifyNoMoreInteractions(metrics); + } + + @Test + public void testFailure_responseBeforeRequest() { + Object response = new Object(); + IllegalStateException e = + assertThrows(IllegalStateException.class, () -> channel.writeOutbound(response)); + assertThat(e).hasMessageThat().isEqualTo("Response sent before request is received."); + } + + @Test + public void testSuccess_pipelinedResponses() { + Object request1 = new Object(); + Object response1 = new Object(); + Object request2 = new Object(); + Object response2 = new Object(); + Object request3 = new Object(); + Object response3 = new Object(); + + // First request, time = 0 + assertThat(channel.writeInbound(request1)).isTrue(); + assertThat((Object) channel.readInbound()).isEqualTo(request1); + DateTime requestTime1 = fakeClock.nowUtc(); + + fakeClock.advanceBy(Duration.millis(5)); + + // Second request, time = 5 + assertThat(channel.writeInbound(request2)).isTrue(); + assertThat((Object) channel.readInbound()).isEqualTo(request2); + DateTime requestTime2 = fakeClock.nowUtc(); + + fakeClock.advanceBy(Duration.millis(7)); + + // First response, time = 12, latency = 12 - 0 = 12 + assertThat(channel.writeOutbound(response1)).isTrue(); + assertThat((Object) channel.readOutbound()).isEqualTo(response1); + DateTime responseTime1 = fakeClock.nowUtc(); + + fakeClock.advanceBy(Duration.millis(11)); + + // Third request, time = 23 + assertThat(channel.writeInbound(request3)).isTrue(); + assertThat((Object) channel.readInbound()).isEqualTo(request3); + DateTime requestTime3 = fakeClock.nowUtc(); + + fakeClock.advanceBy(Duration.millis(2)); + + // Second response, time = 25, latency = 25 - 5 = 20 + assertThat(channel.writeOutbound(response2)).isTrue(); + assertThat((Object) channel.readOutbound()).isEqualTo(response2); + DateTime responseTime2 = fakeClock.nowUtc(); + + fakeClock.advanceBy(Duration.millis(4)); + + // Third response, time = 29, latency = 29 - 23 = 6 + assertThat(channel.writeOutbound(response3)).isTrue(); + assertThat((Object) channel.readOutbound()).isEqualTo(response3); + DateTime responseTime3 = fakeClock.nowUtc(); + + Duration latency1 = new Duration(requestTime1, responseTime1); + Duration latency2 = new Duration(requestTime2, responseTime2); + Duration latency3 = new Duration(requestTime3, responseTime3); + + verify(metrics) + .responseSent(PROTOCOL_NAME, CLIENT_CERT_HASH, latency1); + verify(metrics) + .responseSent(PROTOCOL_NAME, CLIENT_CERT_HASH, latency2); + verify(metrics) + .responseSent(PROTOCOL_NAME, CLIENT_CERT_HASH, latency3); + verifyNoMoreInteractions(metrics); + } +} diff --git a/proxy/src/test/java/google/registry/proxy/metric/BackendMetricsTest.java b/proxy/src/test/java/google/registry/proxy/metric/BackendMetricsTest.java index ea6dfd7b9..77f43fd3f 100644 --- a/proxy/src/test/java/google/registry/proxy/metric/BackendMetricsTest.java +++ b/proxy/src/test/java/google/registry/proxy/metric/BackendMetricsTest.java @@ -23,6 +23,7 @@ import com.google.common.collect.ImmutableSet; import io.netty.handler.codec.http.FullHttpRequest; import io.netty.handler.codec.http.FullHttpResponse; import io.netty.handler.codec.http.HttpResponseStatus; +import org.joda.time.Duration; import org.junit.Before; import org.junit.Test; import org.junit.runner.RunWith; @@ -40,7 +41,7 @@ public class BackendMetricsTest { @Before public void setUp() { - metrics.resetMetric(); + metrics.resetMetrics(); } @Test @@ -89,7 +90,7 @@ public class BackendMetricsTest { public void testSuccess_oneResponse() { String content = "some response"; FullHttpResponse response = makeHttpResponse(content, HttpResponseStatus.OK); - metrics.responseReceived(protocol, certHash, response, 5); + metrics.responseReceived(protocol, certHash, response, Duration.millis(5)); assertThat(BackendMetrics.requestsCounter).hasNoOtherValues(); assertThat(BackendMetrics.requestBytes).hasNoOtherValues(); @@ -115,9 +116,9 @@ public class BackendMetricsTest { FullHttpResponse response1 = makeHttpResponse(content1, HttpResponseStatus.OK); FullHttpResponse response2 = makeHttpResponse(content2, HttpResponseStatus.OK); FullHttpResponse response3 = makeHttpResponse(content3, HttpResponseStatus.BAD_REQUEST); - metrics.responseReceived(protocol, certHash, response1, 5); - metrics.responseReceived(protocol, certHash, response2, 8); - metrics.responseReceived(protocol, certHash, response3, 2); + metrics.responseReceived(protocol, certHash, response1, Duration.millis(5)); + metrics.responseReceived(protocol, certHash, response2, Duration.millis(8)); + metrics.responseReceived(protocol, certHash, response3, Duration.millis(2)); assertThat(BackendMetrics.requestsCounter).hasNoOtherValues(); assertThat(BackendMetrics.requestBytes).hasNoOtherValues(); @@ -147,7 +148,7 @@ public class BackendMetricsTest { FullHttpRequest request = makeHttpPostRequest(requestContent, host, "/"); FullHttpResponse response = makeHttpResponse(responseContent, HttpResponseStatus.OK); metrics.requestSent(protocol, certHash, request.content().readableBytes()); - metrics.responseReceived(protocol, certHash, response, 10); + metrics.responseReceived(protocol, certHash, response, Duration.millis(10)); assertThat(BackendMetrics.requestsCounter) .hasValueForLabels(1, protocol, certHash)