diff --git a/java/google/registry/flows/EppController.java b/java/google/registry/flows/EppController.java index e9faca719..cb5fe2742 100644 --- a/java/google/registry/flows/EppController.java +++ b/java/google/registry/flows/EppController.java @@ -85,7 +85,6 @@ public final class EppController { metricBuilder.setStatus(e.getResult().getCode()); return getErrorResponse(e.getResult(), Trid.create(null)); } - metricBuilder.setCommandName(eppInput.getCommandName()); if (!eppInput.getTargetIds().isEmpty()) { metricBuilder.setEppTarget(Joiner.on(',').join(eppInput.getTargetIds())); } diff --git a/java/google/registry/flows/FlowRunner.java b/java/google/registry/flows/FlowRunner.java index 5162fe426..46c5ca711 100644 --- a/java/google/registry/flows/FlowRunner.java +++ b/java/google/registry/flows/FlowRunner.java @@ -56,6 +56,7 @@ public class FlowRunner { @Inject TransportCredentials credentials; @Inject EppRequestSource eppRequestSource; @Inject Provider flowProvider; + @Inject Class flowClass; @Inject @InputXml byte[] inputXmlBytes; @Inject @DryRun boolean isDryRun; @Inject @Superuser boolean isSuperuser; @@ -92,6 +93,7 @@ public class FlowRunner { "clientId", clientId, "xml", prettyXml, "xmlBytes", xmlBase64))); + metric.setCommandNameFromFlow(flowClass.getSimpleName()); if (!isTransactional) { metric.incrementAttempts(); return EppOutput.create(flowProvider.get().run()); diff --git a/java/google/registry/monitoring/whitebox/EppMetric.java b/java/google/registry/monitoring/whitebox/EppMetric.java index f2955d7e9..a43e45364 100644 --- a/java/google/registry/monitoring/whitebox/EppMetric.java +++ b/java/google/registry/monitoring/whitebox/EppMetric.java @@ -14,6 +14,7 @@ package google.registry.monitoring.whitebox; +import static com.google.common.base.Preconditions.checkArgument; import static google.registry.bigquery.BigqueryUtils.toBigqueryTimestamp; import com.google.api.services.bigquery.model.TableFieldSchema; @@ -141,7 +142,14 @@ public abstract class EppMetric implements BigQueryMetric { abstract Builder setEndTimestamp(DateTime endTimestamp); - public abstract Builder setCommandName(String commandName); + abstract Builder setCommandName(String commandName); + + public Builder setCommandNameFromFlow(String flowSimpleClassName) { + checkArgument( + flowSimpleClassName.endsWith("Flow"), + "Must pass in the simple class name of a flow class"); + return setCommandName(flowSimpleClassName.replaceFirst("Flow$", "")); + } public abstract Builder setClientId(String clientId); diff --git a/javatests/google/registry/flows/EppControllerTest.java b/javatests/google/registry/flows/EppControllerTest.java index ca9130909..30c2a8ddb 100644 --- a/javatests/google/registry/flows/EppControllerTest.java +++ b/javatests/google/registry/flows/EppControllerTest.java @@ -138,7 +138,6 @@ public class EppControllerTest extends ShardableTestCase { assertThat(metric.getClientId()).hasValue("some-client"); assertThat(metric.getPrivilegeLevel()).hasValue("SUPERUSER"); assertThat(metric.getStatus()).hasValue(Code.SUCCESS_WITH_NO_MESSAGES); - assertThat(metric.getCommandName()).hasValue("Create"); assertThat(metric.getEppTarget()).hasValue("example.tld"); } } diff --git a/javatests/google/registry/flows/FlowRunnerTest.java b/javatests/google/registry/flows/FlowRunnerTest.java index 1267f64a2..afa214070 100644 --- a/javatests/google/registry/flows/FlowRunnerTest.java +++ b/javatests/google/registry/flows/FlowRunnerTest.java @@ -30,6 +30,7 @@ import com.google.common.collect.ImmutableSet; import com.google.common.collect.Iterables; import com.google.common.testing.TestLogHandler; import google.registry.model.eppcommon.Trid; +import google.registry.model.eppoutput.EppOutput.ResponseOrGreeting; import google.registry.model.eppoutput.EppResponse; import google.registry.monitoring.whitebox.EppMetric; import google.registry.testing.AppEngineRule; @@ -59,22 +60,21 @@ public class FlowRunnerTest extends ShardableTestCase { private final TestLogHandler handler = new TestLogHandler(); + static class TestCommandFlow implements Flow { + @Override + public ResponseOrGreeting run() throws EppException { + return mock(EppResponse.class); + } + } + @Before public void before() { Logger.getLogger(FlowRunner.class.getCanonicalName()).addHandler(handler); - - final EppResponse eppResponse = mock(EppResponse.class); - flowRunner.clientId = "TheRegistrar"; flowRunner.credentials = new PasswordOnlyTransportCredentials(); flowRunner.eppRequestSource = EppRequestSource.UNIT_TEST; - flowRunner.flowProvider = - Providers.of( - new Flow() { - @Override - public EppResponse run() { - return eppResponse; - }}); + flowRunner.flowProvider = Providers.of(new TestCommandFlow()); + flowRunner.flowClass = TestCommandFlow.class; flowRunner.inputXmlBytes = "".getBytes(UTF_8); flowRunner.isDryRun = false; flowRunner.isSuperuser = false; @@ -98,18 +98,31 @@ public class FlowRunnerTest extends ShardableTestCase { } @Test - public void testRun_notIsTransactional_incrementsMetricAttempts() throws Exception { + public void testRun_nonTransactionalCommand_incrementsMetricAttempts() throws Exception { flowRunner.run(); assertThat(flowRunner.metric.build().getAttempts()).isEqualTo(1); } @Test - public void testRun_isTransactional_incrementsMetricAttempts() throws Exception { + public void testRun_transactionalCommand_incrementsMetricAttempts() throws Exception { flowRunner.isTransactional = true; flowRunner.run(); assertThat(flowRunner.metric.build().getAttempts()).isEqualTo(1); } + @Test + public void testRun_nonTransactionalCommand_setsCommandNameOnMetric() throws Exception { + flowRunner.isTransactional = true; + flowRunner.run(); + assertThat(flowRunner.metric.build().getCommandName()).hasValue("TestCommand"); + } + + @Test + public void testRun_transactionalCommand_setsCommandNameOnMetric() throws Exception { + flowRunner.run(); + assertThat(flowRunner.metric.build().getCommandName()).hasValue("TestCommand"); + } + @Test public void testRun_reportingLogStatement_noClientId() throws Exception { flowRunner.clientId = "";