Record metrics for WHOIS commands

Note that this does not write out metrics for invocations of the
nomulus tool.

This requires a slight refactoring of the existing WhoisResponse
interface so as to also support returning the number of results found
by the WHOIS query.

-------------
Created by MOE: https://github.com/google/moe
MOE_MIGRATED_REVID=149461208

java/google/registry/whois/DomainWhoisResponse.java

@@ -62,9 +62,9 @@ final class DomainWhoisResponse extends WhoisResponseImpl {
   }
 
   @Override
-  public String getPlainTextOutput(final boolean preferUnicode, String disclaimer) {
+  public WhoisResponseResults getResponse(final boolean preferUnicode, String disclaimer) {
     Registrar registrar = getRegistrar(domain.getCurrentSponsorClientId());
-    return new DomainEmitter()
+    String plaintext = new DomainEmitter()
         .emitField(
             "Domain Name", maybeFormatHostname(domain.getFullyQualifiedDomainName(), preferUnicode))
         .emitField("Domain ID", domain.getRepoId())
@@ -97,6 +97,7 @@ final class DomainWhoisResponse extends WhoisResponseImpl {
         .emitAwipMessage()
         .emitFooter(disclaimer)
         .toString();
+    return WhoisResponseResults.create(plaintext, 1);
   }
 
   /** Returns the contact of the given type, or null if it does not exist. */

java/google/registry/whois/NameserverWhoisResponse.java

@@ -42,7 +42,7 @@ final class NameserverWhoisResponse extends WhoisResponseImpl {
   }
 
   @Override
-  public String getPlainTextOutput(boolean preferUnicode, String disclaimer) {
+  public WhoisResponseResults getResponse(boolean preferUnicode, String disclaimer) {
     BasicEmitter emitter = new BasicEmitter();
     for (int i = 0; i < hosts.size(); i++) {
       HostResource host = hosts.get(i);
@@ -63,6 +63,7 @@ final class NameserverWhoisResponse extends WhoisResponseImpl {
         emitter.emitNewline();
       }
     }
-    return emitter.emitLastUpdated(getTimestamp()).emitFooter(disclaimer).toString();
+    String plaintext = emitter.emitLastUpdated(getTimestamp()).emitFooter(disclaimer).toString();
+    return WhoisResponseResults.create(plaintext, hosts.size());
   }
 }

java/google/registry/whois/RegistrarWhoisResponse.java

@@ -43,9 +43,9 @@ class RegistrarWhoisResponse extends WhoisResponseImpl {
   }
 
   @Override
-  public String getPlainTextOutput(boolean preferUnicode, String disclaimer) {
+  public WhoisResponseResults getResponse(boolean preferUnicode, String disclaimer) {
     Set<RegistrarContact> contacts = registrar.getContacts();
-    return new RegistrarEmitter()
+    String plaintext = new RegistrarEmitter()
         .emitField("Registrar Name", registrar.getRegistrarName())
         .emitAddress(
             null,
@@ -62,6 +62,7 @@ class RegistrarWhoisResponse extends WhoisResponseImpl {
         .emitLastUpdated(getTimestamp())
         .emitFooter(disclaimer)
         .toString();
+    return WhoisResponseResults.create(plaintext, 1);
   }
 
   /** An emitter with logic for registrars. */

java/google/registry/whois/Whois.java

@@ -46,9 +46,10 @@ public final class Whois {
           .create(now)
           .readCommand(new StringReader(query))
           .executeQuery(now)
-          .getPlainTextOutput(preferUnicode, disclaimer);
+          .getResponse(preferUnicode, disclaimer)
+          .plainTextOutput();
     } catch (WhoisException e) {
-      return e.getPlainTextOutput(preferUnicode, disclaimer);
+      return e.getResponse(preferUnicode, disclaimer).plainTextOutput();
     } catch (IOException e) {
       throw new RuntimeException(e);
     }

java/google/registry/whois/WhoisException.java

@@ -60,11 +60,12 @@ public final class WhoisException extends Exception implements WhoisResponse {
   }
 
   @Override
-  public String getPlainTextOutput(boolean preferUnicode, String disclaimer) {
+  public WhoisResponseResults getResponse(boolean preferUnicode, String disclaimer) {
-    return new WhoisResponseImpl.BasicEmitter()
+    String plaintext = new WhoisResponseImpl.BasicEmitter()
         .emitRawLine(getMessage())
         .emitLastUpdated(getTimestamp())
         .emitFooter(disclaimer)
         .toString();
+    return WhoisResponseResults.create(plaintext, 0);
   }
 }

java/google/registry/whois/WhoisHttpServer.java

@@ -23,6 +23,7 @@ import static com.google.common.net.HttpHeaders.LAST_MODIFIED;
 import static com.google.common.net.HttpHeaders.X_CONTENT_TYPE_OPTIONS;
 import static com.google.common.net.MediaType.PLAIN_TEXT_UTF_8;
 import static javax.servlet.http.HttpServletResponse.SC_BAD_REQUEST;
+import static javax.servlet.http.HttpServletResponse.SC_INTERNAL_SERVER_ERROR;
 import static javax.servlet.http.HttpServletResponse.SC_OK;
 
 import com.google.common.base.Joiner;
@@ -33,6 +34,8 @@ import google.registry.request.RequestPath;
 import google.registry.request.Response;
 import google.registry.util.Clock;
 import google.registry.util.FormattingLogger;
+import google.registry.whois.WhoisMetrics.WhoisMetric;
+import google.registry.whois.WhoisResponse.WhoisResponseResults;
 import java.io.IOException;
 import java.io.StringReader;
 import java.io.UnsupportedEncodingException;
@@ -133,6 +136,8 @@ public final class WhoisHttpServer implements Runnable {
   @Inject @Config("whoisHttpExpires") Duration expires;
   @Inject WhoisReaderFactory whoisReaderFactory;
   @Inject @RequestPath String requestPath;
+  @Inject WhoisMetric.Builder metricBuilder;
+  @Inject WhoisMetrics whoisMetrics;
   @Inject WhoisHttpServer() {}
 
   @Override
@@ -141,27 +146,38 @@ public final class WhoisHttpServer implements Runnable {
     String path = nullToEmpty(requestPath);
     try {
       // Extremely permissive parsing that turns stuff like "/hello/world/" into "hello world".
-      String command = decode(JOINER.join(SLASHER.split(path.substring(PATH.length())))) + "\r\n";
+      String commandText =
+          decode(JOINER.join(SLASHER.split(path.substring(PATH.length())))) + "\r\n";
       DateTime now = clock.nowUtc();
-      sendResponse(
+      WhoisCommand command =
-          SC_OK,
+          whoisReaderFactory.create(now).readCommand(new StringReader(commandText));
-          whoisReaderFactory.create(now).readCommand(new StringReader(command)).executeQuery(now));
+      metricBuilder.setCommand(command);
+      sendResponse(SC_OK, command.executeQuery(now));
     } catch (WhoisException e) {
+      metricBuilder.setStatus(e.getStatus());
+      metricBuilder.setNumResults(0);
       sendResponse(e.getStatus(), e);
     } catch (IOException e) {
+      metricBuilder.setStatus(SC_INTERNAL_SERVER_ERROR);
+      metricBuilder.setNumResults(0);
       throw new RuntimeException(e);
+    } finally {
+      whoisMetrics.recordWhoisMetric(metricBuilder.build());
     }
   }
 
   private void sendResponse(int status, WhoisResponse whoisResponse) {
     response.setStatus(status);
+    metricBuilder.setStatus(status);
     response.setDateHeader(LAST_MODIFIED, whoisResponse.getTimestamp());
     response.setDateHeader(EXPIRES, whoisResponse.getTimestamp().plus(expires));
     response.setHeader(CACHE_CONTROL, CACHE_CONTROL_VALUE);
     response.setHeader(ACCESS_CONTROL_ALLOW_ORIGIN, CORS_ALLOW_ORIGIN);
     response.setHeader(X_CONTENT_TYPE_OPTIONS, X_CONTENT_NO_SNIFF);
     response.setContentType(PLAIN_TEXT_UTF_8);
-    response.setPayload(whoisResponse.getPlainTextOutput(true, disclaimer));
+    WhoisResponseResults results = whoisResponse.getResponse(true, disclaimer);
+    metricBuilder.setNumResults(results.numResults());
+    response.setPayload(results.plainTextOutput());
   }
 
   /** Removes {@code %xx} escape codes from request path components. */

java/google/registry/whois/WhoisMetrics.java

@@ -14,15 +14,18 @@
 
 package google.registry.whois;
 
+import static com.google.common.base.Preconditions.checkState;
 import static google.registry.monitoring.metrics.EventMetric.DEFAULT_FITTER;
 
 import com.google.auto.value.AutoValue;
+import com.google.common.base.Optional;
 import com.google.common.collect.ImmutableSet;
 import google.registry.monitoring.metrics.EventMetric;
 import google.registry.monitoring.metrics.IncrementableMetric;
 import google.registry.monitoring.metrics.LabelDescriptor;
 import google.registry.monitoring.metrics.MetricRegistryImpl;
 import google.registry.util.Clock;
+import google.registry.whois.WhoisMetrics.WhoisMetric;
 import javax.inject.Inject;
 import org.joda.time.DateTime;
 
@@ -57,36 +60,28 @@ public class WhoisMetrics {
   @Inject
   public WhoisMetrics() {}
 
-  /**
+  /** Records the given {@link WhoisMetric} and its associated processing time. */
-   * Increment a counter which tracks WHOIS requests.
+  public void recordWhoisMetric(WhoisMetric metric) {
-   *
-   * @see WhoisServer
-   */
-  public void incrementWhoisRequests(WhoisMetric metric) {
     whoisRequests.increment(
-        metric.commandName(),
+        metric.commandName().or(""),
-        metric.numResults().toString(),
+        Integer.toString(metric.numResults()),
-        metric.status());
+        Integer.toString(metric.status()));
-  }
-
-  /** Record the server-side processing time for a WHOIS request. */
-  public void recordProcessingTime(WhoisMetric metric) {
     processingTime.record(
         metric.endTimestamp().getMillis() - metric.startTimestamp().getMillis(),
-        metric.commandName(),
+        metric.commandName().or(""),
-        metric.numResults().toString(),
+        Integer.toString(metric.numResults()),
-        metric.status());
+        Integer.toString(metric.status()));
   }
 
   /** A value class for recording attributes of a WHOIS metric. */
   @AutoValue
   public abstract static class WhoisMetric {
 
-    public abstract String commandName();
+    public abstract Optional<String> commandName();
 
-    public abstract Integer numResults();
+    public abstract int numResults();
 
-    public abstract String status();
+    public abstract int status();
 
     public abstract DateTime startTimestamp();
 
@@ -111,14 +106,20 @@ public class WhoisMetrics {
     @AutoValue.Builder
     public abstract static class Builder {
 
+      boolean wasBuilt = false;
+
       /** Builder-only clock to support automatic recording of endTimestamp on {@link #build()}. */
       private Clock clock = null;
 
+      public Builder setCommand(WhoisCommand command) {
+        return setCommandName(command.getClass().getSimpleName());
+      }
+
       public abstract Builder setCommandName(String commandName);
 
-      public abstract Builder setNumResults(Integer numResults);
+      public abstract Builder setNumResults(int numResults);
 
-      public abstract Builder setStatus(String status);
+      public abstract Builder setStatus(int status);
 
       abstract Builder setStartTimestamp(DateTime startTimestamp);
 
@@ -136,9 +137,12 @@ public class WhoisMetrics {
        * current timestamp of the clock; otherwise end timestamp must have been previously set.
        */
       public WhoisMetric build() {
+        checkState(
+            !wasBuilt, "WhoisMetric was already built; building it again is most likely an error");
         if (clock != null) {
           setEndTimestamp(clock.nowUtc());
         }
+        wasBuilt = true;
         return autoBuild();
       }
 

java/google/registry/whois/WhoisReader.java

@@ -62,7 +62,7 @@ import org.joda.time.DateTime;
  * @see <a href="http://tools.ietf.org/html/rfc3912">RFC 3912</a>
  * @see <a href="http://www.iana.org/assignments/registrar-ids">IANA Registrar IDs</a>
  */
-@AutoFactory
+@AutoFactory(allowSubclasses = true)
 class WhoisReader {
 
   /**

java/google/registry/whois/WhoisResponse.java

@@ -14,13 +14,14 @@
 
 package google.registry.whois;
 
+import com.google.auto.value.AutoValue;
 import org.joda.time.DateTime;
 
 /** Representation of a WHOIS query response. */
 public interface WhoisResponse {
 
   /**
-   * Returns a plain text WHOIS response.
+   * Returns the WHOIS response.
    *
    * @param preferUnicode if {@code false} will cause the output to be converted to ASCII whenever
    *     possible; for example, converting IDN hostname labels to punycode. However certain things
@@ -29,10 +30,19 @@ public interface WhoisResponse {
    *     be set to {@code true}.
    * @param disclaimer text to show at bottom of output
    */
-  String getPlainTextOutput(boolean preferUnicode, String disclaimer);
+  WhoisResponseResults getResponse(boolean preferUnicode, String disclaimer);
 
-  /**
+  /** Returns the time at which this response was created. */
-   * Returns the time at which this response was created.
-   */
   DateTime getTimestamp();
+
+  /** A wraper class for the plaintext response of a WHOIS command and its number of results. */
+  @AutoValue
+  abstract static class WhoisResponseResults {
+    public abstract String plainTextOutput();
+    public abstract int numResults();
+
+    static WhoisResponseResults create(String plainTextOutput, int numResults) {
+      return new AutoValue_WhoisResponse_WhoisResponseResults(plainTextOutput, numResults);
+    }
+  }
 }

java/google/registry/whois/WhoisServer.java

@@ -15,6 +15,7 @@
 package google.registry.whois;
 
 import static google.registry.request.Action.Method.POST;
+import static javax.servlet.http.HttpServletResponse.SC_INTERNAL_SERVER_ERROR;
 import static javax.servlet.http.HttpServletResponse.SC_OK;
 
 import com.google.common.net.MediaType;
@@ -23,6 +24,8 @@ import google.registry.request.Action;
 import google.registry.request.Response;
 import google.registry.util.Clock;
 import google.registry.util.FormattingLogger;
+import google.registry.whois.WhoisMetrics.WhoisMetric;
+import google.registry.whois.WhoisResponse.WhoisResponseResults;
 import java.io.Reader;
 import javax.inject.Inject;
 import org.joda.time.DateTime;
@@ -60,6 +63,8 @@ public class WhoisServer implements Runnable {
   @Inject Response response;
   @Inject WhoisReaderFactory whoisReaderFactory;
   @Inject @Config("whoisDisclaimer") String disclaimer;
+  @Inject WhoisMetric.Builder metricBuilder;
+  @Inject WhoisMetrics whoisMetrics;
   @Inject WhoisServer() {}
 
   @Override
@@ -67,17 +72,23 @@ public class WhoisServer implements Runnable {
     String responseText;
     DateTime now = clock.nowUtc();
     try {
-      responseText =
+      WhoisCommand command = whoisReaderFactory.create(now).readCommand(input);
-          whoisReaderFactory
+      metricBuilder.setCommand(command);
-              .create(now)
+      WhoisResponseResults results =
-              .readCommand(input)
+          command.executeQuery(now).getResponse(PREFER_UNICODE, disclaimer);
-              .executeQuery(now)
+      responseText = results.plainTextOutput();
-              .getPlainTextOutput(PREFER_UNICODE, disclaimer);
+      metricBuilder.setNumResults(results.numResults());
+      metricBuilder.setStatus(SC_OK);
     } catch (WhoisException e) {
-      responseText = e.getPlainTextOutput(PREFER_UNICODE, disclaimer);
+      WhoisResponseResults results = e.getResponse(PREFER_UNICODE, disclaimer);
+      responseText = results.plainTextOutput();
+      metricBuilder.setNumResults(0);
+      metricBuilder.setStatus(e.getStatus());
     } catch (Throwable t) {
       logger.severe(t, "WHOIS request crashed");
       responseText = "Internal Server Error";
+      metricBuilder.setNumResults(0);
+      metricBuilder.setStatus(SC_INTERNAL_SERVER_ERROR);
     }
     // Note that we always return 200 (OK) even if an error was hit. This is because returning an
     // non-OK HTTP status code will cause the proxy server to silently close the connection. Since
@@ -86,5 +97,6 @@ public class WhoisServer implements Runnable {
     response.setStatus(SC_OK);
     response.setContentType(CONTENT_TYPE);
     response.setPayload(responseText);
+    whoisMetrics.recordWhoisMetric(metricBuilder.build());
   }
 }

javatests/google/registry/whois/DomainWhoisResponseTest.java

@@ -37,6 +37,7 @@ import google.registry.model.host.HostResource;
 import google.registry.model.registrar.Registrar;
 import google.registry.testing.AppEngineRule;
 import google.registry.testing.FakeClock;
+import google.registry.whois.WhoisResponse.WhoisResponseResults;
 import org.joda.time.DateTime;
 import org.junit.Before;
 import org.junit.Rule;
@@ -239,16 +240,16 @@ public class DomainWhoisResponseTest {
   public void getPlainTextOutputTest() {
     DomainWhoisResponse domainWhoisResponse =
         new DomainWhoisResponse(domainResource, clock.nowUtc());
-    assertThat(domainWhoisResponse.getPlainTextOutput(false, "Doodle Disclaimer"))
+    assertThat(domainWhoisResponse.getResponse(false, "Doodle Disclaimer"))
-        .isEqualTo(loadWhoisTestFile("whois_domain.txt"));
+        .isEqualTo(WhoisResponseResults.create(loadWhoisTestFile("whois_domain.txt"), 1));
   }
 
   @Test
   public void addImplicitOkStatusTest() {
-    DomainWhoisResponse domainWhoisResponse = new DomainWhoisResponse(
+    DomainWhoisResponse domainWhoisResponse =
-        domainResource.asBuilder().setStatusValues(null).build(),
+        new DomainWhoisResponse(
-        clock.nowUtc());
+            domainResource.asBuilder().setStatusValues(null).build(), clock.nowUtc());
-    assertThat(domainWhoisResponse.getPlainTextOutput(false, "Doodle Disclaimer"))
+    assertThat(domainWhoisResponse.getResponse(false, "Doodle Disclaimer").plainTextOutput())
         .contains("Domain Status: ok");
   }
 }

javatests/google/registry/whois/NameserverWhoisResponseTest.java

@@ -26,6 +26,7 @@ import google.registry.model.host.HostResource;
 import google.registry.model.registrar.Registrar;
 import google.registry.testing.AppEngineRule;
 import google.registry.testing.FakeClock;
+import google.registry.whois.WhoisResponse.WhoisResponseResults;
 import org.joda.time.DateTime;
 import org.junit.Before;
 import org.junit.Rule;
@@ -81,15 +82,16 @@ public class NameserverWhoisResponseTest {
   public void testGetTextOutput() {
     NameserverWhoisResponse nameserverWhoisResponse =
         new NameserverWhoisResponse(hostResource1, clock.nowUtc());
-    assertThat(nameserverWhoisResponse.getPlainTextOutput(false, "Doodle Disclaimer"))
+    assertThat(nameserverWhoisResponse.getResponse(false, "Doodle Disclaimer"))
-        .isEqualTo(loadWhoisTestFile("whois_nameserver.txt"));
+        .isEqualTo(WhoisResponseResults.create(loadWhoisTestFile("whois_nameserver.txt"), 1));
   }
 
   @Test
   public void testGetMultipleNameserversResponse() {
     NameserverWhoisResponse nameserverWhoisResponse =
         new NameserverWhoisResponse(ImmutableList.of(hostResource1, hostResource2), clock.nowUtc());
-    assertThat(nameserverWhoisResponse.getPlainTextOutput(false, "Doodle Disclaimer"))
+    assertThat(nameserverWhoisResponse.getResponse(false, "Doodle Disclaimer"))
-        .isEqualTo(loadWhoisTestFile("whois_multiple_nameservers.txt"));
+        .isEqualTo(
+            WhoisResponseResults.create(loadWhoisTestFile("whois_multiple_nameservers.txt"), 2));
   }
 }

javatests/google/registry/whois/RegistrarWhoisResponseTest.java

@@ -26,6 +26,7 @@ import google.registry.model.registrar.RegistrarAddress;
 import google.registry.model.registrar.RegistrarContact;
 import google.registry.testing.AppEngineRule;
 import google.registry.testing.FakeClock;
+import google.registry.whois.WhoisResponse.WhoisResponseResults;
 import org.joda.time.DateTime;
 import org.junit.Rule;
 import org.junit.Test;
@@ -113,8 +114,8 @@ public class RegistrarWhoisResponseTest {
 
     RegistrarWhoisResponse registrarWhoisResponse =
         new RegistrarWhoisResponse(registrar, clock.nowUtc());
-    assertThat(registrarWhoisResponse.getPlainTextOutput(false, "Doodle Disclaimer"))
+    assertThat(registrarWhoisResponse.getResponse(false, "Doodle Disclaimer"))
-        .isEqualTo(loadWhoisTestFile("whois_registrar.txt"));
+        .isEqualTo(WhoisResponseResults.create(loadWhoisTestFile("whois_registrar.txt"), 1));
   }
 
   @Test
@@ -129,6 +130,6 @@ public class RegistrarWhoisResponseTest {
     RegistrarWhoisResponse registrarWhoisResponse =
         new RegistrarWhoisResponse(registrar, clock.nowUtc());
     // Just make sure this doesn't NPE.
-    registrarWhoisResponse.getPlainTextOutput(false, "Doodle Disclaimer");
+    registrarWhoisResponse.getResponse(false, "Doodle Disclaimer");
   }
 }

javatests/google/registry/whois/WhoisHttpServerTest.java

@@ -16,6 +16,7 @@ package google.registry.whois;
 
 import static com.google.common.net.MediaType.PLAIN_TEXT_UTF_8;
 import static com.google.common.truth.Truth.assertThat;
+import static com.google.common.truth.Truth.assert_;
 import static google.registry.testing.DatastoreHelper.createTlds;
 import static google.registry.testing.DatastoreHelper.persistResource;
 import static google.registry.testing.DatastoreHelper.persistSimpleResources;
@@ -25,6 +26,14 @@ import static google.registry.testing.FullFieldsTestEntityHelper.makeHostResourc
 import static google.registry.testing.FullFieldsTestEntityHelper.makeRegistrar;
 import static google.registry.testing.FullFieldsTestEntityHelper.makeRegistrarContacts;
 import static google.registry.whois.WhoisHelper.loadWhoisTestFile;
+import static javax.servlet.http.HttpServletResponse.SC_BAD_REQUEST;
+import static javax.servlet.http.HttpServletResponse.SC_INTERNAL_SERVER_ERROR;
+import static javax.servlet.http.HttpServletResponse.SC_OK;
+import static org.mockito.Matchers.any;
+import static org.mockito.Matchers.eq;
+import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.verify;
+import static org.mockito.Mockito.when;
 
 import google.registry.model.contact.ContactResource;
 import google.registry.model.ofy.Ofy;
@@ -35,6 +44,9 @@ import google.registry.testing.FakeClock;
 import google.registry.testing.FakeResponse;
 import google.registry.testing.InjectRule;
 import google.registry.testing.Providers;
+import google.registry.whois.WhoisMetrics.WhoisMetric;
+import java.io.IOException;
+import java.io.Reader;
 import org.joda.time.DateTime;
 import org.joda.time.Duration;
 import org.junit.After;
@@ -68,6 +80,8 @@ public class WhoisHttpServerTest {
     whoisServer.response = response;
     whoisServer.whoisReaderFactory =
         new WhoisReaderFactory(Providers.of(new WhoisCommandFactory()));
+    whoisServer.whoisMetrics = new WhoisMetrics();
+    whoisServer.metricBuilder = WhoisMetric.builderForRequest(clock);
     whoisServer.disclaimer = "Doodle Disclaimer";
     return whoisServer;
   }
@@ -331,4 +345,57 @@ public class WhoisHttpServerTest {
     assertThat(response.getPayload())
         .isEqualTo(loadWhoisTestFile("whois_server_registrar_not_found.txt"));
   }
+
+  @Test
+  public void testRun_metricsLoggedForSuccessfulCommand() throws Exception {
+    persistResource(makeHostResource("ns1.cat.lol", "1.2.3.4"));
+    WhoisHttpServer server = newWhoisHttpServer("/nameserver/ns1.cat.lol");
+    server.whoisMetrics = mock(WhoisMetrics.class);
+    server.run();
+    WhoisMetric expected =
+        WhoisMetric.builderForRequest(clock)
+            .setCommandName("NameserverLookupByHostCommand")
+            .setNumResults(1)
+            .setStatus(SC_OK)
+            .build();
+    verify(server.whoisMetrics).recordWhoisMetric(eq(expected));
+  }
+
+  @Test
+  public void testRun_metricsLoggedForUnsuccessfulCommand() throws Exception {
+    WhoisHttpServer server = newWhoisHttpServer("nic.%u307F%u3093%u306A");
+    server.whoisMetrics = mock(WhoisMetrics.class);
+    server.run();
+    WhoisMetric expected =
+        WhoisMetric.builderForRequest(clock).setNumResults(0).setStatus(SC_BAD_REQUEST).build();
+    verify(server.whoisMetrics).recordWhoisMetric(eq(expected));
+  }
+
+  @Test
+  public void testRun_metricsLoggedForInternalServerError() throws Exception {
+    persistResource(makeHostResource("ns1.cat.lol", "1.2.3.4"));
+    WhoisHttpServer server = newWhoisHttpServer("ns1.cat.lol");
+    final WhoisReader reader = mock(WhoisReader.class);
+    when(reader.readCommand(any(Reader.class))).thenThrow(new IOException("missing cat interface"));
+
+    server.whoisReaderFactory = new WhoisReaderFactory(Providers.of(new WhoisCommandFactory())) {
+      @Override
+      WhoisReader create(DateTime now) {
+        return reader;
+      }};
+    server.whoisMetrics = mock(WhoisMetrics.class);
+
+    try {
+      server.run();
+      assert_().fail("Should have thrown RuntimeException");
+    } catch (RuntimeException e) {
+      assertThat(e.getCause().getMessage()).isEqualTo("missing cat interface");
+    }
+    WhoisMetric expected =
+        WhoisMetric.builderForRequest(clock)
+            .setNumResults(0)
+            .setStatus(SC_INTERNAL_SERVER_ERROR)
+            .build();
+    verify(server.whoisMetrics).recordWhoisMetric(eq(expected));
+  }
 }

javatests/google/registry/whois/WhoisServerTest.java

@@ -27,6 +27,14 @@ import static google.registry.testing.FullFieldsTestEntityHelper.makeHostResourc
 import static google.registry.testing.FullFieldsTestEntityHelper.makeRegistrar;
 import static google.registry.testing.FullFieldsTestEntityHelper.makeRegistrarContacts;
 import static google.registry.whois.WhoisHelper.loadWhoisTestFile;
+import static javax.servlet.http.HttpServletResponse.SC_INTERNAL_SERVER_ERROR;
+import static javax.servlet.http.HttpServletResponse.SC_NOT_FOUND;
+import static javax.servlet.http.HttpServletResponse.SC_OK;
+import static org.mockito.Matchers.any;
+import static org.mockito.Matchers.eq;
+import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.verify;
+import static org.mockito.Mockito.when;
 
 import google.registry.model.domain.DomainResource;
 import google.registry.model.ofy.Ofy;
@@ -37,6 +45,9 @@ import google.registry.testing.FakeClock;
 import google.registry.testing.FakeResponse;
 import google.registry.testing.InjectRule;
 import google.registry.testing.Providers;
+import google.registry.whois.WhoisMetrics.WhoisMetric;
+import java.io.IOException;
+import java.io.Reader;
 import java.io.StringReader;
 import org.joda.time.DateTime;
 import org.junit.Before;
@@ -63,6 +74,8 @@ public class WhoisServerTest {
     whoisServer.response = response;
     whoisServer.whoisReaderFactory =
         new WhoisReaderFactory(Providers.of(new WhoisCommandFactory()));
+    whoisServer.whoisMetrics = new WhoisMetrics();
+    whoisServer.metricBuilder = WhoisMetric.builderForRequest(clock);
     whoisServer.disclaimer = "Doodle Disclaimer";
     return whoisServer;
   }
@@ -430,4 +443,58 @@ public class WhoisServerTest {
     assertThat(response.getPayload()).contains("ns1.cat.1.test");
     assertThat(response.getPayload()).contains("1.2.3.4");
   }
+
+  @Test
+  public void testRun_metricsLoggedForSuccessfulCommand() throws Exception {
+    persistResource(makeHostResource("ns1.cat.lol", "1.2.3.4"));
+    persistResource(makeHostResource("ns2.cat.lol", "1.2.3.4"));
+    WhoisServer server = newWhoisServer("nameserver 1.2.3.4");
+    server.whoisMetrics = mock(WhoisMetrics.class);
+    server.run();
+    WhoisMetric expected =
+        WhoisMetric.builderForRequest(clock)
+            .setCommandName("NameserverLookupByIpCommand")
+            .setNumResults(2)
+            .setStatus(SC_OK)
+            .build();
+    verify(server.whoisMetrics).recordWhoisMetric(eq(expected));
+  }
+
+  @Test
+  public void testRun_metricsLoggedForUnsuccessfulCommand() throws Exception {
+    WhoisServer server = newWhoisServer("domain cat.lol\r\n");
+    server.whoisMetrics = mock(WhoisMetrics.class);
+    server.run();
+    WhoisMetric expected =
+        WhoisMetric.builderForRequest(clock)
+            .setCommandName("DomainLookupCommand")
+            .setNumResults(0)
+            .setStatus(SC_NOT_FOUND)
+            .build();
+    verify(server.whoisMetrics).recordWhoisMetric(eq(expected));
+  }
+
+  @Test
+  public void testRun_metricsLoggedForInternalServerError() throws Exception {
+    persistResource(makeHostResource("ns1.cat.lol", "1.2.3.4"));
+    WhoisServer server = newWhoisServer("ns1.cat.lol");
+    final WhoisReader reader = mock(WhoisReader.class);
+    when(reader.readCommand(any(Reader.class))).thenThrow(new IOException("missing cat interface"));
+
+    server.whoisReaderFactory = new WhoisReaderFactory(Providers.of(new WhoisCommandFactory())) {
+      @Override
+      WhoisReader create(DateTime now) {
+        return reader;
+      }};
+    server.whoisMetrics = mock(WhoisMetrics.class);
+
+    server.run();
+    WhoisMetric expected =
+        WhoisMetric.builderForRequest(clock)
+            .setNumResults(0)
+            .setStatus(SC_INTERNAL_SERVER_ERROR)
+            .build();
+    verify(server.whoisMetrics).recordWhoisMetric(eq(expected));
+    assertThat(response.getPayload()).isEqualTo("Internal Server Error");
+  }
 }