diff --git a/java/google/registry/flows/FlowRunner.java b/java/google/registry/flows/FlowRunner.java index 4267d1572..042b2fd05 100644 --- a/java/google/registry/flows/FlowRunner.java +++ b/java/google/registry/flows/FlowRunner.java @@ -35,10 +35,7 @@ import javax.inject.Provider; /** Run a flow, either transactionally or not, with logging and retrying as needed. */ public class FlowRunner { - /** Log format used by legacy ICANN reporting parsing - DO NOT CHANGE. */ - // TODO(b/20725722): remove this log format entirely once we've transitioned to using the - // JSON log line below instead, or change this one to be for human consumption only. - private static final String COMMAND_LOG_FORMAT = "EPP Command" + Strings.repeat("\n\t%s", 7); + private static final String COMMAND_LOG_FORMAT = "EPP Command" + Strings.repeat("\n\t%s", 8); private static final FormattingLogger logger = FormattingLogger.getLoggerForCallerClass(); @@ -59,10 +56,7 @@ public class FlowRunner { /** Runs the EPP flow, and records metrics on the given builder. */ public EppOutput run(final EppMetric.Builder eppMetricBuilder) throws EppException { String prettyXml = prettyPrint(inputXmlBytes); - // This log line is very fragile since it's used for ICANN reporting - DO NOT CHANGE. - // New data to be logged should be added only to the JSON log statement below. - // TODO(b/20725722): remove this log statement entirely once we've transitioned to using the - // log line below instead, or change this one to be for human consumption only. + if (logger.isLoggable(Level.INFO)) { logger.infofmt( COMMAND_LOG_FORMAT, diff --git a/javatests/google/registry/flows/FlowRunnerTest.java b/javatests/google/registry/flows/FlowRunnerTest.java index 750b32f5f..01bb511e4 100644 --- a/javatests/google/registry/flows/FlowRunnerTest.java +++ b/javatests/google/registry/flows/FlowRunnerTest.java @@ -124,7 +124,7 @@ public class FlowRunnerTest extends ShardableTestCase { } @Test - public void testRun_legacyLoggingStatement_basic() throws Exception { + public void testRun_loggingStatement_basic() throws Exception { flowRunner.run(eppMetricBuilder); assertThat(Splitter.on("\n\t").split(findFirstLogMessageByPrefix(handler, "EPP Command\n\t"))) .containsExactly( @@ -136,12 +136,13 @@ public class FlowRunnerTest extends ShardableTestCase { "", // Extra newline at the end of the XML. "PasswordOnlyTransportCredentials{}", "UNIT_TEST", - "LIVE") + "LIVE", + "NORMAL") .inOrder(); } @Test - public void testRun_legacyLoggingStatement_httpSessionMetadata() throws Exception { + public void testRun_loggingStatement_httpSessionMetadata() throws Exception { flowRunner.sessionMetadata = new HttpSessionMetadata(new FakeHttpSession()); flowRunner.sessionMetadata.setClientId("TheRegistrar"); flowRunner.run(eppMetricBuilder); @@ -152,7 +153,7 @@ public class FlowRunnerTest extends ShardableTestCase { } @Test - public void testRun_legacyLoggingStatement_gaeUserCredentials() throws Exception { + public void testRun_loggingStatement_gaeUserCredentials() throws Exception { flowRunner.credentials = GaeUserCredentials.forTestingUser(new User("user@example.com", "authDomain"), false); flowRunner.run(eppMetricBuilder); @@ -161,7 +162,7 @@ public class FlowRunnerTest extends ShardableTestCase { } @Test - public void testRun_legacyLoggingStatement_tlsCredentials() throws Exception { + public void testRun_loggingStatement_tlsCredentials() throws Exception { flowRunner.credentials = new TlsCredentials("abc123def", Optional.of("127.0.0.1"), "sni"); flowRunner.run(eppMetricBuilder); assertThat(Splitter.on("\n\t").split(findFirstLogMessageByPrefix(handler, "EPP Command\n\t"))) @@ -170,7 +171,7 @@ public class FlowRunnerTest extends ShardableTestCase { } @Test - public void testRun_legacyLoggingStatement_dryRun() throws Exception { + public void testRun_loggingStatement_dryRun() throws Exception { flowRunner.isDryRun = true; flowRunner.run(eppMetricBuilder); assertThat(Splitter.on("\n\t").split(findFirstLogMessageByPrefix(handler, "EPP Command\n\t"))) @@ -178,14 +179,22 @@ public class FlowRunnerTest extends ShardableTestCase { } @Test - public void testRun_legacyLoggingStatement_complexEppInput() throws Exception { + public void testRun_loggingStatement_superuser() throws Exception { + flowRunner.isSuperuser = true; + flowRunner.run(eppMetricBuilder); + assertThat(Splitter.on("\n\t").split(findFirstLogMessageByPrefix(handler, "EPP Command\n\t"))) + .contains("SUPERUSER"); + } + + @Test + public void testRun_loggingStatement_complexEppInput() throws Exception { String domainCreateXml = loadFile(getClass(), "domain_create_prettyprinted.xml"); flowRunner.inputXmlBytes = domainCreateXml.getBytes(UTF_8); flowRunner.run(eppMetricBuilder); String logMessage = findFirstLogMessageByPrefix(handler, "EPP Command\n\t"); List lines = Splitter.on("\n\t").splitToList(logMessage); assertThat(lines.size()).named("number of lines in log message").isAtLeast(9); - String xml = Joiner.on('\n').join(lines.subList(3, lines.size() - 3)); + String xml = Joiner.on('\n').join(lines.subList(3, lines.size() - 4)); assertThat(xml).isEqualTo(domainCreateXml); } }