diff --git a/java/google/registry/whois/DomainWhoisResponse.java b/java/google/registry/whois/DomainWhoisResponse.java index abdebe7c0..a24e9e17f 100644 --- a/java/google/registry/whois/DomainWhoisResponse.java +++ b/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. */ diff --git a/java/google/registry/whois/NameserverWhoisResponse.java b/java/google/registry/whois/NameserverWhoisResponse.java index fa9e6729f..55855d3de 100644 --- a/java/google/registry/whois/NameserverWhoisResponse.java +++ b/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()); } } diff --git a/java/google/registry/whois/RegistrarWhoisResponse.java b/java/google/registry/whois/RegistrarWhoisResponse.java index cc341da20..9aa27b2c8 100644 --- a/java/google/registry/whois/RegistrarWhoisResponse.java +++ b/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 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. */ diff --git a/java/google/registry/whois/Whois.java b/java/google/registry/whois/Whois.java index 63c66e0b4..5afe55068 100644 --- a/java/google/registry/whois/Whois.java +++ b/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); } diff --git a/java/google/registry/whois/WhoisException.java b/java/google/registry/whois/WhoisException.java index a077dce55..11d4cd1a3 100644 --- a/java/google/registry/whois/WhoisException.java +++ b/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) { - return new WhoisResponseImpl.BasicEmitter() + public WhoisResponseResults getResponse(boolean preferUnicode, String disclaimer) { + String plaintext = new WhoisResponseImpl.BasicEmitter() .emitRawLine(getMessage()) .emitLastUpdated(getTimestamp()) .emitFooter(disclaimer) .toString(); + return WhoisResponseResults.create(plaintext, 0); } } diff --git a/java/google/registry/whois/WhoisHttpServer.java b/java/google/registry/whois/WhoisHttpServer.java index ea7cdb50b..e316f1608 100644 --- a/java/google/registry/whois/WhoisHttpServer.java +++ b/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( - SC_OK, - whoisReaderFactory.create(now).readCommand(new StringReader(command)).executeQuery(now)); + WhoisCommand command = + whoisReaderFactory.create(now).readCommand(new StringReader(commandText)); + 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. */ diff --git a/java/google/registry/whois/WhoisMetrics.java b/java/google/registry/whois/WhoisMetrics.java index d9fde5e2a..93c3ae47a 100644 --- a/java/google/registry/whois/WhoisMetrics.java +++ b/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() {} - /** - * Increment a counter which tracks WHOIS requests. - * - * @see WhoisServer - */ - public void incrementWhoisRequests(WhoisMetric metric) { + /** Records the given {@link WhoisMetric} and its associated processing time. */ + public void recordWhoisMetric(WhoisMetric metric) { whoisRequests.increment( - metric.commandName(), - metric.numResults().toString(), - metric.status()); - } - - /** Record the server-side processing time for a WHOIS request. */ - public void recordProcessingTime(WhoisMetric metric) { + metric.commandName().or(""), + Integer.toString(metric.numResults()), + Integer.toString(metric.status())); processingTime.record( metric.endTimestamp().getMillis() - metric.startTimestamp().getMillis(), - metric.commandName(), - metric.numResults().toString(), - metric.status()); + metric.commandName().or(""), + Integer.toString(metric.numResults()), + 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 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(); } diff --git a/java/google/registry/whois/WhoisReader.java b/java/google/registry/whois/WhoisReader.java index f111d7d01..f12bdc2af 100644 --- a/java/google/registry/whois/WhoisReader.java +++ b/java/google/registry/whois/WhoisReader.java @@ -62,7 +62,7 @@ import org.joda.time.DateTime; * @see RFC 3912 * @see IANA Registrar IDs */ -@AutoFactory +@AutoFactory(allowSubclasses = true) class WhoisReader { /** diff --git a/java/google/registry/whois/WhoisResponse.java b/java/google/registry/whois/WhoisResponse.java index 037889314..5f0972c38 100644 --- a/java/google/registry/whois/WhoisResponse.java +++ b/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); + } + } } diff --git a/java/google/registry/whois/WhoisServer.java b/java/google/registry/whois/WhoisServer.java index 975544770..6ad978007 100644 --- a/java/google/registry/whois/WhoisServer.java +++ b/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 = - whoisReaderFactory - .create(now) - .readCommand(input) - .executeQuery(now) - .getPlainTextOutput(PREFER_UNICODE, disclaimer); + WhoisCommand command = whoisReaderFactory.create(now).readCommand(input); + metricBuilder.setCommand(command); + WhoisResponseResults results = + command.executeQuery(now).getResponse(PREFER_UNICODE, disclaimer); + responseText = results.plainTextOutput(); + 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()); } } diff --git a/javatests/google/registry/whois/DomainWhoisResponseTest.java b/javatests/google/registry/whois/DomainWhoisResponseTest.java index 2bf455371..f83cd3a5e 100644 --- a/javatests/google/registry/whois/DomainWhoisResponseTest.java +++ b/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")) - .isEqualTo(loadWhoisTestFile("whois_domain.txt")); + assertThat(domainWhoisResponse.getResponse(false, "Doodle Disclaimer")) + .isEqualTo(WhoisResponseResults.create(loadWhoisTestFile("whois_domain.txt"), 1)); } @Test public void addImplicitOkStatusTest() { - DomainWhoisResponse domainWhoisResponse = new DomainWhoisResponse( - domainResource.asBuilder().setStatusValues(null).build(), - clock.nowUtc()); - assertThat(domainWhoisResponse.getPlainTextOutput(false, "Doodle Disclaimer")) + DomainWhoisResponse domainWhoisResponse = + new DomainWhoisResponse( + domainResource.asBuilder().setStatusValues(null).build(), clock.nowUtc()); + assertThat(domainWhoisResponse.getResponse(false, "Doodle Disclaimer").plainTextOutput()) .contains("Domain Status: ok"); } } diff --git a/javatests/google/registry/whois/NameserverWhoisResponseTest.java b/javatests/google/registry/whois/NameserverWhoisResponseTest.java index e4a46f3fa..070050247 100644 --- a/javatests/google/registry/whois/NameserverWhoisResponseTest.java +++ b/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")) - .isEqualTo(loadWhoisTestFile("whois_nameserver.txt")); + assertThat(nameserverWhoisResponse.getResponse(false, "Doodle Disclaimer")) + .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")) - .isEqualTo(loadWhoisTestFile("whois_multiple_nameservers.txt")); + assertThat(nameserverWhoisResponse.getResponse(false, "Doodle Disclaimer")) + .isEqualTo( + WhoisResponseResults.create(loadWhoisTestFile("whois_multiple_nameservers.txt"), 2)); } } diff --git a/javatests/google/registry/whois/RegistrarWhoisResponseTest.java b/javatests/google/registry/whois/RegistrarWhoisResponseTest.java index 6de958717..e9b3748c7 100644 --- a/javatests/google/registry/whois/RegistrarWhoisResponseTest.java +++ b/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")) - .isEqualTo(loadWhoisTestFile("whois_registrar.txt")); + assertThat(registrarWhoisResponse.getResponse(false, "Doodle Disclaimer")) + .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"); } } diff --git a/javatests/google/registry/whois/WhoisHttpServerTest.java b/javatests/google/registry/whois/WhoisHttpServerTest.java index 0772f3299..bbd511ec6 100644 --- a/javatests/google/registry/whois/WhoisHttpServerTest.java +++ b/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)); + } } diff --git a/javatests/google/registry/whois/WhoisServerTest.java b/javatests/google/registry/whois/WhoisServerTest.java index 910e9edb7..00d41dc4d 100644 --- a/javatests/google/registry/whois/WhoisServerTest.java +++ b/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"); + } }