Use the actual EPP command flow name for EppMetrics

It was previously only using the name of the inner command XML element,
e.g. "Create", "Delete", "Update", etc. This wasn't very useful because
there was no way to discriminate between operations on different types
of EPP resources.

-------------
Created by MOE: https://github.com/google/moe
MOE_MIGRATED_REVID=151131491
This commit is contained in:
mcilwain 2017-03-24 09:03:01 -07:00 committed by Ben McIlwain
parent 0d32b6b7b2
commit 4260fb573f
5 changed files with 36 additions and 15 deletions

View file

@ -85,7 +85,6 @@ public final class EppController {
metricBuilder.setStatus(e.getResult().getCode()); metricBuilder.setStatus(e.getResult().getCode());
return getErrorResponse(e.getResult(), Trid.create(null)); return getErrorResponse(e.getResult(), Trid.create(null));
} }
metricBuilder.setCommandName(eppInput.getCommandName());
if (!eppInput.getTargetIds().isEmpty()) { if (!eppInput.getTargetIds().isEmpty()) {
metricBuilder.setEppTarget(Joiner.on(',').join(eppInput.getTargetIds())); metricBuilder.setEppTarget(Joiner.on(',').join(eppInput.getTargetIds()));
} }

View file

@ -56,6 +56,7 @@ public class FlowRunner {
@Inject TransportCredentials credentials; @Inject TransportCredentials credentials;
@Inject EppRequestSource eppRequestSource; @Inject EppRequestSource eppRequestSource;
@Inject Provider<Flow> flowProvider; @Inject Provider<Flow> flowProvider;
@Inject Class<? extends Flow> flowClass;
@Inject @InputXml byte[] inputXmlBytes; @Inject @InputXml byte[] inputXmlBytes;
@Inject @DryRun boolean isDryRun; @Inject @DryRun boolean isDryRun;
@Inject @Superuser boolean isSuperuser; @Inject @Superuser boolean isSuperuser;
@ -92,6 +93,7 @@ public class FlowRunner {
"clientId", clientId, "clientId", clientId,
"xml", prettyXml, "xml", prettyXml,
"xmlBytes", xmlBase64))); "xmlBytes", xmlBase64)));
metric.setCommandNameFromFlow(flowClass.getSimpleName());
if (!isTransactional) { if (!isTransactional) {
metric.incrementAttempts(); metric.incrementAttempts();
return EppOutput.create(flowProvider.get().run()); return EppOutput.create(flowProvider.get().run());

View file

@ -14,6 +14,7 @@
package google.registry.monitoring.whitebox; package google.registry.monitoring.whitebox;
import static com.google.common.base.Preconditions.checkArgument;
import static google.registry.bigquery.BigqueryUtils.toBigqueryTimestamp; import static google.registry.bigquery.BigqueryUtils.toBigqueryTimestamp;
import com.google.api.services.bigquery.model.TableFieldSchema; import com.google.api.services.bigquery.model.TableFieldSchema;
@ -141,7 +142,14 @@ public abstract class EppMetric implements BigQueryMetric {
abstract Builder setEndTimestamp(DateTime endTimestamp); 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); public abstract Builder setClientId(String clientId);

View file

@ -138,7 +138,6 @@ public class EppControllerTest extends ShardableTestCase {
assertThat(metric.getClientId()).hasValue("some-client"); assertThat(metric.getClientId()).hasValue("some-client");
assertThat(metric.getPrivilegeLevel()).hasValue("SUPERUSER"); assertThat(metric.getPrivilegeLevel()).hasValue("SUPERUSER");
assertThat(metric.getStatus()).hasValue(Code.SUCCESS_WITH_NO_MESSAGES); assertThat(metric.getStatus()).hasValue(Code.SUCCESS_WITH_NO_MESSAGES);
assertThat(metric.getCommandName()).hasValue("Create");
assertThat(metric.getEppTarget()).hasValue("example.tld"); assertThat(metric.getEppTarget()).hasValue("example.tld");
} }
} }

View file

@ -30,6 +30,7 @@ import com.google.common.collect.ImmutableSet;
import com.google.common.collect.Iterables; import com.google.common.collect.Iterables;
import com.google.common.testing.TestLogHandler; import com.google.common.testing.TestLogHandler;
import google.registry.model.eppcommon.Trid; import google.registry.model.eppcommon.Trid;
import google.registry.model.eppoutput.EppOutput.ResponseOrGreeting;
import google.registry.model.eppoutput.EppResponse; import google.registry.model.eppoutput.EppResponse;
import google.registry.monitoring.whitebox.EppMetric; import google.registry.monitoring.whitebox.EppMetric;
import google.registry.testing.AppEngineRule; import google.registry.testing.AppEngineRule;
@ -59,22 +60,21 @@ public class FlowRunnerTest extends ShardableTestCase {
private final TestLogHandler handler = new TestLogHandler(); private final TestLogHandler handler = new TestLogHandler();
static class TestCommandFlow implements Flow {
@Override
public ResponseOrGreeting run() throws EppException {
return mock(EppResponse.class);
}
}
@Before @Before
public void before() { public void before() {
Logger.getLogger(FlowRunner.class.getCanonicalName()).addHandler(handler); Logger.getLogger(FlowRunner.class.getCanonicalName()).addHandler(handler);
final EppResponse eppResponse = mock(EppResponse.class);
flowRunner.clientId = "TheRegistrar"; flowRunner.clientId = "TheRegistrar";
flowRunner.credentials = new PasswordOnlyTransportCredentials(); flowRunner.credentials = new PasswordOnlyTransportCredentials();
flowRunner.eppRequestSource = EppRequestSource.UNIT_TEST; flowRunner.eppRequestSource = EppRequestSource.UNIT_TEST;
flowRunner.flowProvider = flowRunner.flowProvider = Providers.<Flow>of(new TestCommandFlow());
Providers.<Flow>of( flowRunner.flowClass = TestCommandFlow.class;
new Flow() {
@Override
public EppResponse run() {
return eppResponse;
}});
flowRunner.inputXmlBytes = "<xml/>".getBytes(UTF_8); flowRunner.inputXmlBytes = "<xml/>".getBytes(UTF_8);
flowRunner.isDryRun = false; flowRunner.isDryRun = false;
flowRunner.isSuperuser = false; flowRunner.isSuperuser = false;
@ -98,18 +98,31 @@ public class FlowRunnerTest extends ShardableTestCase {
} }
@Test @Test
public void testRun_notIsTransactional_incrementsMetricAttempts() throws Exception { public void testRun_nonTransactionalCommand_incrementsMetricAttempts() throws Exception {
flowRunner.run(); flowRunner.run();
assertThat(flowRunner.metric.build().getAttempts()).isEqualTo(1); assertThat(flowRunner.metric.build().getAttempts()).isEqualTo(1);
} }
@Test @Test
public void testRun_isTransactional_incrementsMetricAttempts() throws Exception { public void testRun_transactionalCommand_incrementsMetricAttempts() throws Exception {
flowRunner.isTransactional = true; flowRunner.isTransactional = true;
flowRunner.run(); flowRunner.run();
assertThat(flowRunner.metric.build().getAttempts()).isEqualTo(1); 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 @Test
public void testRun_reportingLogStatement_noClientId() throws Exception { public void testRun_reportingLogStatement_noClientId() throws Exception {
flowRunner.clientId = ""; flowRunner.clientId = "";