Instrument proxy frontend latency metric (#320)

* Instrument proxy frontend latency metric

Backend latency only captures the time spent waiting for the GAE backend
to respond to a request, which is not representitive of what clients
experience. For instance it does not take into account the time the
proxy spends on processing the requests and the time it takes to send a
response to the client.

This PR adds a metric for frontend latency, which is the time spent on
answering a client request. This should serve as a better proxy for the
latency clients observe.

* Rename AbstractMetrics to BaseMetrics

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"

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<LengthFieldBasedFrameDecoder> lengthFieldBasedFrameDecoderProvider,
       Provider<LengthFieldPrepender> lengthFieldPrependerProvider,
       Provider<EppServiceHandler> eppServiceHandlerProvider,
+      Provider<FrontendMetricsHandler> frontendMetricsHandlerProvider,
       Provider<EppQuotaHandler> eppQuotaHandlerProvider,
       Provider<FullHttpRequestRelayHandler> relayHandlerProvider) {
     return ImmutableList.of(
@@ -94,6 +96,7 @@ public class EppProtocolModule {
         lengthFieldBasedFrameDecoderProvider,
         lengthFieldPrependerProvider,
         eppServiceHandlerProvider,
+        frontendMetricsHandlerProvider,
         eppQuotaHandlerProvider,
         relayHandlerProvider);
   }

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<ReadTimeoutHandler> readTimeoutHandlerProvider,
       Provider<LineBasedFrameDecoder> lineBasedFrameDecoderProvider,
       Provider<WhoisServiceHandler> whoisServiceHandlerProvider,
+      Provider<FrontendMetricsHandler> frontendMetricsHandlerProvider,
       Provider<WhoisQuotaHandler> whoisQuotaHandlerProvider,
       Provider<FullHttpRequestRelayHandler> relayHandlerProvider) {
     return ImmutableList.of(
@@ -81,6 +83,7 @@ public class WhoisProtocolModule {
         readTimeoutHandlerProvider,
         lineBasedFrameDecoderProvider,
         whoisServiceHandlerProvider,
+        frontendMetricsHandlerProvider,
         whoisQuotaHandlerProvider,
         relayHandlerProvider);
   }

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

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.
+ *
+ * <p>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.
+   *
+   * <p>This queue is used to calculate frontend request-response latency.
+   *
+   * <p>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.
+   *
+   * <p>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.
+   *
+   * <p>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 <a href="https://tools.ietf.org/html/rfc3912">RFC 3912 WHOIS Protocol Specification</a>
+   * @see <a href="https://tools.ietf.org/html/rfc5734#section-3">RFC 5734 Extensible Provisioning
+   *     Protocol (EPP) Transport over TCP</a>
+   */
+  private final Queue<DateTime> 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()));
+                  }
+                });
+  }
+}

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 {
+public class BackendMetrics extends BaseMetrics {
-
-  // 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<LabelDescriptor> LABELS =
-      ImmutableSet.of(
-          LabelDescriptor.create("protocol", "Name of the protocol."),
-          LabelDescriptor.create(
-              "client_cert_hash", "SHA256 hash of the client certificate, if available."));
 
   static final IncrementableMetric requestsCounter =
       MetricRegistryImpl.getDefault()
@@ -96,13 +78,8 @@ public class BackendMetrics {
   @Inject
   BackendMetrics() {}
 
-  /**
+  @Override
-   * Resets all backend metrics.
+  void resetMetrics() {
-   *
-   * <p>This should only used in tests to clear out states. No production code should call this
-   * function.
-   */
-  void resetMetric() {
     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) {
+      String protocol, String certHash, FullHttpResponse response, Duration latency) {
-    latencyMs.record(latency, protocol, certHash);
+    latencyMs.record(latency.getMillis(), protocol, certHash);
     responseBytes.record(response.content().readableBytes(), protocol, certHash);
     responsesCounter.increment(protocol, certHash, response.status().toString());
   }

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.
+   *
+   * <p>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:
+   *
+   * <pre>
+   * nomulus -e production list_registrars -f clientCertificateHash | grep $HASH
+   * </pre>
+   */
+   protected static final ImmutableSet<LabelDescriptor> 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.
+   *
+   * <p>This should only be used in tests to reset states. Production code should not call this
+   * method.
+   */
+  @VisibleForTesting
+  abstract void resetMetrics();
+}

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 {
+public class FrontendMetrics extends BaseMetrics {
-
-  /**
-   * Labels to register front metrics with.
-   *
-   * <p>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:
-   *
-   * <pre>
-   * nomulus -e production list_registrars -f clientCertificateHash | grep $HASH
-   * </pre>
-   */
-  private static final ImmutableSet<LabelDescriptor> LABELS =
-      ImmutableSet.of(
-          LabelDescriptor.create("protocol", "Name of the protocol."),
-          LabelDescriptor.create(
-              "client_cert_hash", "SHA256 hash of the client certificate, if available."));
 
   private static final ConcurrentMap<ImmutableList<String>, ChannelGroup> activeConnections =
       new ConcurrentHashMap<>();
@@ -65,9 +47,7 @@ public class FrontendMetrics {
               "Active Connections",
               LABELS,
               () ->
-                  activeConnections
+                  activeConnections.entrySet().stream()
-                      .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() {}
 
-  /**
+  @Override
-   * Resets all frontend metrics.
-   *
-   * <p>This should only be used in tests to reset states. Production code should not call this
-   * method.
-   */
-  @VisibleForTesting
   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);
+  }
 }

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,

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();
+    Duration latency1 = new Duration(requestTime1, responseTime1);
-    long latency2 = new Duration(sentTime2, receivedTime2).getMillis();
+    Duration latency2 = new Duration(requestTime2, responseTime2);
-    long latency3 = new Duration(sentTime3, receivedTime3).getMillis();
+    Duration latency3 = new Duration(requestTime3, responseTime3);
 
     verify(metrics)
         .requestSent(RELAYED_PROTOCOL_NAME, CLIENT_CERT_HASH, request1.content().readableBytes());

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<EmbeddedChannel>() {
+              @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);
+  }
+}

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, response1, Duration.millis(5));
-    metrics.responseReceived(protocol, certHash, response2, 8);
+    metrics.responseReceived(protocol, certHash, response2, Duration.millis(8));
-    metrics.responseReceived(protocol, certHash, response3, 2);
+    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)