Make FlowReporter log tld and various other fields

As part of b/36599833, this makes FlowReporter log the tld(s) of every domain
flow it executes, so we can provide ICANN reporting totals on a per-TLD basis.

It also adds several other fields that we're computing anyway and which seem
useful, particularly for debugging any issues we see in production with the data
that we're attempting to record for ICANN reporting.  The full set of fields is:

  - commandType (e.g. "create", "info", "transfer")
  - resourceType* (e.g. "domain", "contact", "host")
  - flowClassName (e.g. "ContactCreateFlow", "DomainRestoreRequestFlow")
  - targetId* (e.g. "ns1.foo.com", "bar.org", "contact-1234")
  - targetIds* - plural of the above, for multi-resource checks
  - tld** (e.g. "com", "co.uk") - extracted from targetId, lowercased
  - tlds** - plural of the above, deduplicated, for multi-resource checks

* = only non-empty for resource flows (not e.g. login, logout, poll)
** = only non-empty for domain flows

Note that TLD extraction is deliberately very lenient to avoid the complexity
overhead of double-validation of the domain names in the common case.

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

java/google/registry/flows/FlowReporter.java

@@ -16,12 +16,18 @@ package google.registry.flows;
 
 import static com.google.common.io.BaseEncoding.base64;
 import static google.registry.xml.XmlTransformer.prettyPrint;
+import static java.util.Collections.EMPTY_LIST;
 
+import com.google.common.base.Ascii;
+import com.google.common.base.Optional;
+import com.google.common.collect.ImmutableList;
 import com.google.common.collect.ImmutableMap;
+import com.google.common.collect.ImmutableSet;
 import google.registry.flows.FlowModule.ClientId;
 import google.registry.flows.FlowModule.InputXml;
 import google.registry.flows.annotations.ReportingSpec;
 import google.registry.model.eppcommon.Trid;
+import google.registry.model.eppinput.EppInput;
 import google.registry.util.FormattingLogger;
 import javax.inject.Inject;
 import org.json.simple.JSONValue;
@@ -48,6 +54,7 @@ public class FlowReporter {
   @Inject Trid trid;
   @Inject @ClientId String clientId;
   @Inject @InputXml byte[] inputXmlBytes;
+  @Inject EppInput eppInput;
   @Inject Class<? extends Flow> flowClass;
   @Inject FlowReporter() {}
 
@@ -64,13 +71,57 @@ public class FlowReporter {
     // Explicitly log flow metadata separately from the EPP XML itself so that it stays compact
     // enough to be sure to fit in a single log entry (the XML part in rare cases could be long
     // enough to overflow into multiple log entries, breaking routine parsing of the JSON format).
+    String resourceType = eppInput.getResourceType().or("");
+    boolean isDomain = "domain".equals(resourceType);
+    String singleTargetId = eppInput.getSingleTargetId().or("");
+    ImmutableList<String> targetIds = eppInput.getTargetIds();
     logger.infofmt(
         "%s: %s",
         METADATA_LOG_SIGNATURE,
-        JSONValue.toJSONString(ImmutableMap.<String, Object>of(
+        JSONValue.toJSONString(new ImmutableMap.Builder<String, Object>()
-            "trid", trid.getServerTransactionId(),
+            .put("serverTrid", trid.getServerTransactionId())
-            "clientId", clientId,
+            .put("clientId", clientId)
-            "icannActivityReportField", extractActivityReportField(flowClass))));
+            .put("commandType", eppInput.getCommandType())
+            .put("resourceType", resourceType)
+            .put("flowClassName", flowClass.getSimpleName())
+            .put("targetId", singleTargetId)
+            .put("targetIds", targetIds)
+            .put("tld", isDomain ? extractTld(singleTargetId).or("") : "")
+            .put("tlds", isDomain ? extractTlds(targetIds).asList() : EMPTY_LIST)
+            .put("icannActivityReportField", extractActivityReportField(flowClass))
+            .build()));
+  }
+
+  /**
+   * Returns the guessed TLD of the given domain name, assuming a second-level domain name, or
+   * absent if no TLD could be detected.
+   *
+   * <p>This method is quick and dirty and doesn't attempt to validate the domain name in any way;
+   * it just takes anything after the first period to be the TLD and converts ASCII to lowercase.
+   * We want quick and dirty here because this will be called on not-yet-validated EPP XML where
+   * just about anything could be supplied, and there's no reason to validate twice when this just
+   * needs to be roughly correct.
+   */
+  private static final Optional<String> extractTld(String domainName) {
+    int index = domainName.indexOf('.');
+    return index == -1
+        ? Optional.absent()
+        : Optional.of(Ascii.toLowerCase(domainName.substring(index + 1)));
+  }
+
+  /**
+   * Returns the set of unique results of {@link #extractTld} applied to each given domain name,
+   * excluding any absent results (i.e. cases where no TLD was detected).
+   */
+  private static final ImmutableSet<String> extractTlds(Iterable<String> domainNames) {
+    ImmutableSet.Builder<String> set = new ImmutableSet.Builder<>();
+    for (String domainName : domainNames) {
+      Optional<String> extractedTld = extractTld(domainName);
+      if (extractedTld.isPresent()) {
+        set.add(extractedTld.get());
+      }
+    }
+    return set.build();
   }
 
   /**

javatests/google/registry/flows/FlowReporterTest.java

@@ -19,13 +19,17 @@ import static com.google.common.truth.Truth.assertThat;
 import static google.registry.testing.TestDataHelper.loadFileWithSubstitutions;
 import static java.nio.charset.StandardCharsets.UTF_8;
 import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.when;
 
+import com.google.common.base.Optional;
 import com.google.common.base.Predicate;
+import com.google.common.collect.ImmutableList;
 import com.google.common.collect.ImmutableMap;
 import com.google.common.collect.Iterables;
 import com.google.common.testing.TestLogHandler;
 import google.registry.flows.annotations.ReportingSpec;
 import google.registry.model.eppcommon.Trid;
+import google.registry.model.eppinput.EppInput;
 import google.registry.model.eppoutput.EppOutput.ResponseOrGreeting;
 import google.registry.model.eppoutput.EppResponse;
 import google.registry.model.reporting.IcannReportingTypes.ActivityReportField;
@@ -68,6 +72,11 @@ public class FlowReporterTest extends ShardableTestCase {
     flowReporter.clientId = "TheRegistrar";
     flowReporter.inputXmlBytes = "<xml/>".getBytes(UTF_8);
     flowReporter.flowClass = TestCommandFlow.class;
+    flowReporter.eppInput = mock(EppInput.class);
+    when(flowReporter.eppInput.getCommandType()).thenReturn("info");
+    when(flowReporter.eppInput.getResourceType()).thenReturn(Optional.of("domain"));
+    when(flowReporter.eppInput.getSingleTargetId()).thenReturn(Optional.of("target.foo"));
+    when(flowReporter.eppInput.getTargetIds()).thenReturn(ImmutableList.of("target.foo"));
   }
 
   @Test
@@ -96,8 +105,15 @@ public class FlowReporterTest extends ShardableTestCase {
     flowReporter.recordToLogs();
     assertThat(parseJsonMap(findLogMessageByPrefix(handler, "FLOW-LOG-SIGNATURE-METADATA: ")))
         .containsExactly(
-              "trid", "server-456",
+              "serverTrid", "server-456",
               "clientId", "TheRegistrar",
+              "commandType", "info",
+              "resourceType", "domain",
+              "flowClassName", "TestCommandFlow",
+              "targetId", "target.foo",
+              "targetIds", ImmutableList.of("target.foo"),
+              "tld", "foo",
+              "tlds", ImmutableList.of("foo"),
               "icannActivityReportField", "");
   }
 
@@ -105,22 +121,109 @@ public class FlowReporterTest extends ShardableTestCase {
   public void testRecordToLogs_metadata_withReportingSpec() throws Exception {
     flowReporter.flowClass = TestReportingSpecCommandFlow.class;
     flowReporter.recordToLogs();
-    assertThat(parseJsonMap(findLogMessageByPrefix(handler, "FLOW-LOG-SIGNATURE-METADATA: ")))
+    Map<String, Object> json =
-        .containsExactly(
+        parseJsonMap(findLogMessageByPrefix(handler, "FLOW-LOG-SIGNATURE-METADATA: "));
-              "trid", "server-456",
+    assertThat(json).containsEntry("flowClassName", "TestReportingSpecCommandFlow");
-              "clientId", "TheRegistrar",
+    assertThat(json).containsEntry("icannActivityReportField", "srs-cont-check");
-              "icannActivityReportField", "srs-cont-check");
   }
 
   @Test
   public void testRecordToLogs_metadata_noClientId() throws Exception {
     flowReporter.clientId = "";
     flowReporter.recordToLogs();
-    assertThat(parseJsonMap(findLogMessageByPrefix(handler, "FLOW-LOG-SIGNATURE-METADATA: ")))
+    Map<String, Object> json =
-        .containsExactly(
+        parseJsonMap(findLogMessageByPrefix(handler, "FLOW-LOG-SIGNATURE-METADATA: "));
-              "trid", "server-456",
+    assertThat(json).containsEntry("clientId", "");
-              "clientId", "",
+  }
-              "icannActivityReportField", "");
+
+  @Test
+  public void testRecordToLogs_metadata_notResourceFlow_noResourceTypeOrTld() throws Exception {
+    when(flowReporter.eppInput.getResourceType()).thenReturn(Optional.absent());
+    flowReporter.recordToLogs();
+    Map<String, Object> json =
+        parseJsonMap(findLogMessageByPrefix(handler, "FLOW-LOG-SIGNATURE-METADATA: "));
+    assertThat(json).containsEntry("resourceType", "");
+    assertThat(json).containsEntry("tld", "");
+    assertThat(json).containsEntry("tlds", ImmutableList.of());
+  }
+
+
+  @Test
+  public void testRecordToLogs_metadata_notDomainFlow_noTld() throws Exception {
+    when(flowReporter.eppInput.getResourceType()).thenReturn(Optional.of("contact"));
+    flowReporter.recordToLogs();
+    Map<String, Object> json =
+        parseJsonMap(findLogMessageByPrefix(handler, "FLOW-LOG-SIGNATURE-METADATA: "));
+    assertThat(json).containsEntry("resourceType", "contact");
+    assertThat(json).containsEntry("tld", "");
+    assertThat(json).containsEntry("tlds", ImmutableList.of());
+  }
+
+  @Test
+  public void testRecordToLogs_metadata_multipartDomainName_multipartTld() throws Exception {
+    when(flowReporter.eppInput.getSingleTargetId()).thenReturn(Optional.of("target.co.uk"));
+    when(flowReporter.eppInput.getTargetIds()).thenReturn(ImmutableList.of("target.co.uk"));
+    flowReporter.recordToLogs();
+    Map<String, Object> json =
+        parseJsonMap(findLogMessageByPrefix(handler, "FLOW-LOG-SIGNATURE-METADATA: "));
+    assertThat(json).containsEntry("targetId", "target.co.uk");
+    assertThat(json).containsEntry("targetIds", ImmutableList.of("target.co.uk"));
+    assertThat(json).containsEntry("tld", "co.uk");
+    assertThat(json).containsEntry("tlds", ImmutableList.of("co.uk"));
+  }
+
+  @Test
+  public void testRecordToLogs_metadata_multipleTargetIds_uniqueTldSet() throws Exception {
+    when(flowReporter.eppInput.getSingleTargetId()).thenReturn(Optional.absent());
+    when(flowReporter.eppInput.getTargetIds())
+        .thenReturn(ImmutableList.of("target.co.uk", "foo.uk", "bar.uk", "baz.com"));
+    flowReporter.recordToLogs();
+    Map<String, Object> json =
+        parseJsonMap(findLogMessageByPrefix(handler, "FLOW-LOG-SIGNATURE-METADATA: "));
+    assertThat(json).containsEntry("targetId", "");
+    assertThat(json).containsEntry(
+        "targetIds", ImmutableList.of("target.co.uk", "foo.uk", "bar.uk", "baz.com"));
+    assertThat(json).containsEntry("tld", "");
+    assertThat(json).containsEntry("tlds", ImmutableList.of("co.uk", "uk", "com"));
+  }
+
+  @Test
+  public void testRecordToLogs_metadata_uppercaseDomainName_lowercaseTld() throws Exception {
+    when(flowReporter.eppInput.getSingleTargetId()).thenReturn(Optional.of("TARGET.FOO"));
+    when(flowReporter.eppInput.getTargetIds()).thenReturn(ImmutableList.of("TARGET.FOO"));
+    flowReporter.recordToLogs();
+    Map<String, Object> json =
+        parseJsonMap(findLogMessageByPrefix(handler, "FLOW-LOG-SIGNATURE-METADATA: "));
+    assertThat(json).containsEntry("targetId", "TARGET.FOO");
+    assertThat(json).containsEntry("targetIds", ImmutableList.of("TARGET.FOO"));
+    assertThat(json).containsEntry("tld", "foo");
+    assertThat(json).containsEntry("tlds", ImmutableList.of("foo"));
+  }
+
+  @Test
+  public void testRecordToLogs_metadata_invalidDomainName_stillGuessesTld() throws Exception {
+    when(flowReporter.eppInput.getSingleTargetId()).thenReturn(Optional.of("<foo@bar.com>"));
+    when(flowReporter.eppInput.getTargetIds()).thenReturn(ImmutableList.of("<foo@bar.com>"));
+    flowReporter.recordToLogs();
+    Map<String, Object> json =
+        parseJsonMap(findLogMessageByPrefix(handler, "FLOW-LOG-SIGNATURE-METADATA: "));
+    assertThat(json).containsEntry("targetId", "<foo@bar.com>");
+    assertThat(json).containsEntry("targetIds", ImmutableList.of("<foo@bar.com>"));
+    assertThat(json).containsEntry("tld", "com>");
+    assertThat(json).containsEntry("tlds", ImmutableList.of("com>"));
+  }
+
+  @Test
+  public void testRecordToLogs_metadata_domainWithoutPeriod_noTld() throws Exception {
+    when(flowReporter.eppInput.getSingleTargetId()).thenReturn(Optional.of("target,foo"));
+    when(flowReporter.eppInput.getTargetIds()).thenReturn(ImmutableList.of("target,foo"));
+    flowReporter.recordToLogs();
+    Map<String, Object> json =
+        parseJsonMap(findLogMessageByPrefix(handler, "FLOW-LOG-SIGNATURE-METADATA: "));
+    assertThat(json).containsEntry("targetId", "target,foo");
+    assertThat(json).containsEntry("targetIds", ImmutableList.of("target,foo"));
+    assertThat(json).containsEntry("tld", "");
+    assertThat(json).containsEntry("tlds", ImmutableList.of());
   }
 
   @SuppressWarnings("unchecked")

javatests/google/registry/flows/ResourceFlowTestCase.java

@@ -45,6 +45,7 @@ import java.util.logging.Level;
 import java.util.logging.Logger;
 import org.joda.time.DateTime;
 import org.joda.time.Duration;
+import org.json.simple.JSONValue;
 import org.junit.Before;
 import org.junit.Test;
 
@@ -173,10 +174,25 @@ public abstract class ResourceFlowTestCase<F extends Flow, R extends EppResource
             .payload(expectedPayload));
   }
 
+
+  protected void assertClientIdFieldLogged(String clientId) {
+    assertAboutLogs().that(logHandler)
+        .hasLogAtLevelWithMessage(Level.INFO, "FLOW-LOG-SIGNATURE-METADATA")
+        .which()
+        .contains("\"clientId\":" + JSONValue.toJSONString(clientId));
+  }
+
+  protected void assertTldsFieldLogged(String... tlds) {
+    assertAboutLogs().that(logHandler)
+        .hasLogAtLevelWithMessage(Level.INFO, "FLOW-LOG-SIGNATURE-METADATA")
+        .which()
+        .contains("\"tlds\":" + JSONValue.toJSONString(ImmutableList.copyOf(tlds)));
+  }
+
   protected void assertIcannReportingActivityFieldLogged(String fieldName) {
     assertAboutLogs().that(logHandler)
         .hasLogAtLevelWithMessage(Level.INFO, "FLOW-LOG-SIGNATURE-METADATA")
         .which()
-        .contains(fieldName);
+        .contains("\"icannActivityReportField\":" + JSONValue.toJSONString(fieldName));
   }
 }

javatests/google/registry/flows/domain/ClaimsCheckFlowTest.java

@@ -158,5 +158,6 @@ public class ClaimsCheckFlowTest extends ResourceFlowTestCase<ClaimsCheckFlow, D
   public void testIcannActivityReportField_getsLogged() throws Exception {
     runFlow();
     assertIcannReportingActivityFieldLogged("srs-dom-check");
+    assertTldsFieldLogged("tld");
   }
 }

javatests/google/registry/flows/domain/DomainAllocateFlowTest.java

@@ -658,5 +658,8 @@ public class DomainAllocateFlowTest
     setupDomainApplication("tld", TldState.QUIET_PERIOD);
     runFlow(CommitMode.LIVE, UserPrivileges.SUPERUSER);
     assertIcannReportingActivityFieldLogged("srs-dom-create");
+    assertTldsFieldLogged("tld");
+    // Ensure we log the client ID for srs-dom-create so we can also use it for attempted-adds.
+    assertClientIdFieldLogged("TheRegistrar");
   }
 }

javatests/google/registry/flows/domain/DomainApplicationCreateFlowTest.java

@@ -1812,5 +1812,8 @@ public class DomainApplicationCreateFlowTest
     clock.advanceOneMilli();
     runFlow();
     assertIcannReportingActivityFieldLogged("srs-dom-create");
+    assertTldsFieldLogged("tld");
+    // Ensure we log the client ID for srs-dom-create so we can also use it for attempted-adds.
+    assertClientIdFieldLogged("TheRegistrar");
   }
 }

javatests/google/registry/flows/domain/DomainApplicationDeleteFlowTest.java

@@ -313,5 +313,6 @@ public class DomainApplicationDeleteFlowTest
     clock.advanceOneMilli();
     runFlow();
     assertIcannReportingActivityFieldLogged("srs-dom-delete");
+    assertTldsFieldLogged("tld");
   }
 }

javatests/google/registry/flows/domain/DomainApplicationInfoFlowTest.java

@@ -363,5 +363,6 @@ public class DomainApplicationInfoFlowTest
     persistTestEntities(HostsState.HOSTS_EXIST, MarksState.NO_MARKS_EXIST);
     runFlow();
     assertIcannReportingActivityFieldLogged("srs-dom-info");
+    assertTldsFieldLogged("tld");
   }
 }

javatests/google/registry/flows/domain/DomainApplicationUpdateFlowTest.java

@@ -862,5 +862,6 @@ public class DomainApplicationUpdateFlowTest
     clock.advanceOneMilli();
     runFlow();
     assertIcannReportingActivityFieldLogged("srs-dom-update");
+    assertTldsFieldLogged("tld");
   }
 }

javatests/google/registry/flows/domain/DomainCheckFlowTest.java

@@ -875,7 +875,12 @@ public class DomainCheckFlowTest
 
   @Test
   public void testIcannActivityReportField_getsLogged() throws Exception {
+    createTld("com", TldState.GENERAL_AVAILABILITY);
+    createTld("net", TldState.GENERAL_AVAILABILITY);
+    createTld("org", TldState.GENERAL_AVAILABILITY);
+    setEppInput("domain_check.xml");
     runFlow();
     assertIcannReportingActivityFieldLogged("srs-dom-check");
+    assertTldsFieldLogged("com", "net", "org");
   }
 }

javatests/google/registry/flows/domain/DomainCreateFlowTest.java

@@ -1996,6 +1996,9 @@ public class DomainCreateFlowTest extends ResourceFlowTestCase<DomainCreateFlow,
     persistContactsAndHosts();
     runFlow();
     assertIcannReportingActivityFieldLogged("srs-dom-create");
+    assertTldsFieldLogged("tld");
+    // Ensure we log the client ID for srs-dom-create so we can also use it for attempted-adds.
+    assertClientIdFieldLogged("TheRegistrar");
   }
 
   @Test

javatests/google/registry/flows/domain/DomainDeleteFlowTest.java

@@ -744,5 +744,6 @@ public class DomainDeleteFlowTest extends ResourceFlowTestCase<DomainDeleteFlow,
     clock.advanceOneMilli();
     runFlow();
     assertIcannReportingActivityFieldLogged("srs-dom-delete");
+    assertTldsFieldLogged("tld");
   }
 }

javatests/google/registry/flows/domain/DomainInfoFlowTest.java

@@ -668,5 +668,6 @@ public class DomainInfoFlowTest extends ResourceFlowTestCase<DomainInfoFlow, Dom
     persistTestEntities(false);
     runFlow();
     assertIcannReportingActivityFieldLogged("srs-dom-info");
+    assertTldsFieldLogged("tld");
   }
 }

javatests/google/registry/flows/domain/DomainRenewFlowTest.java

@@ -646,5 +646,6 @@ public class DomainRenewFlowTest extends ResourceFlowTestCase<DomainRenewFlow, D
     persistDomain();
     runFlow();
     assertIcannReportingActivityFieldLogged("srs-dom-renew");
+    assertTldsFieldLogged("tld");
   }
 }

javatests/google/registry/flows/domain/DomainRestoreRequestFlowTest.java

@@ -553,5 +553,6 @@ public class DomainRestoreRequestFlowTest extends
     persistPendingDeleteDomain();
     runFlow();
     assertIcannReportingActivityFieldLogged("srs-dom-rgp-restore-request");
+    assertTldsFieldLogged("tld");
  }
 }

javatests/google/registry/flows/domain/DomainTransferApproveFlowTest.java

@@ -430,5 +430,6 @@ public class DomainTransferApproveFlowTest
   public void testIcannActivityReportField_getsLogged() throws Exception {
     runFlow();
     assertIcannReportingActivityFieldLogged("srs-dom-transfer-approve");
+    assertTldsFieldLogged("tld");
   }
 }

javatests/google/registry/flows/domain/DomainTransferCancelFlowTest.java

@@ -307,5 +307,6 @@ public class DomainTransferCancelFlowTest
     clock.advanceOneMilli();
     runFlow();
     assertIcannReportingActivityFieldLogged("srs-dom-transfer-cancel");
+    assertTldsFieldLogged("tld");
   }
 }

javatests/google/registry/flows/domain/DomainTransferQueryFlowTest.java

@@ -226,5 +226,6 @@ public class DomainTransferQueryFlowTest
   public void testIcannActivityReportField_getsLogged() throws Exception {
     runFlow();
     assertIcannReportingActivityFieldLogged("srs-dom-transfer-query");
+    assertTldsFieldLogged("tld");
   }
 }

javatests/google/registry/flows/domain/DomainTransferRejectFlowTest.java

@@ -270,5 +270,6 @@ public class DomainTransferRejectFlowTest
   public void testIcannActivityReportField_getsLogged() throws Exception {
     runFlow();
     assertIcannReportingActivityFieldLogged("srs-dom-transfer-reject");
+    assertTldsFieldLogged("tld");
   }
 }

javatests/google/registry/flows/domain/DomainTransferRequestFlowTest.java

@@ -956,5 +956,6 @@ public class DomainTransferRequestFlowTest
     clock.advanceOneMilli();
     runTest("domain_transfer_request.xml", UserPrivileges.NORMAL);
     assertIcannReportingActivityFieldLogged("srs-dom-transfer-request");
+    assertTldsFieldLogged("tld");
   }
 }

javatests/google/registry/flows/domain/DomainUpdateFlowTest.java

@@ -1432,5 +1432,6 @@ public class DomainUpdateFlowTest extends ResourceFlowTestCase<DomainUpdateFlow,
     persistDomain();
     runFlow();
     assertIcannReportingActivityFieldLogged("srs-dom-update");
+    assertTldsFieldLogged("tld");
   }
 }