diff --git a/java/google/registry/flows/EppController.java b/java/google/registry/flows/EppController.java index e49e52406..394592ebb 100644 --- a/java/google/registry/flows/EppController.java +++ b/java/google/registry/flows/EppController.java @@ -18,6 +18,7 @@ import static google.registry.flows.EppXmlTransformer.unmarshal; import com.google.common.annotations.VisibleForTesting; import com.google.common.base.Joiner; +import com.google.common.base.Optional; import google.registry.flows.FlowModule.EppExceptionInProviderException; import google.registry.model.eppcommon.Trid; import google.registry.model.eppinput.EppInput; @@ -55,7 +56,7 @@ public final class EppController { boolean isDryRun, boolean isSuperuser, byte[] inputXmlBytes) { - metricBuilder.setClientId(sessionMetadata.getClientId()); + metricBuilder.setClientId(Optional.fromNullable(sessionMetadata.getClientId())); metricBuilder.setPrivilegeLevel(isSuperuser ? "SUPERUSER" : "NORMAL"); try { EppInput eppInput; diff --git a/java/google/registry/module/frontend/FrontendRequestComponent.java b/java/google/registry/module/frontend/FrontendRequestComponent.java index 158922b1b..84da740f8 100644 --- a/java/google/registry/module/frontend/FrontendRequestComponent.java +++ b/java/google/registry/module/frontend/FrontendRequestComponent.java @@ -21,6 +21,7 @@ import google.registry.flows.EppConsoleAction; import google.registry.flows.EppTlsAction; import google.registry.flows.FlowComponent; import google.registry.flows.TlsCredentials.EppTlsModule; +import google.registry.monitoring.whitebox.WhiteboxModule; import google.registry.rdap.RdapAutnumAction; import google.registry.rdap.RdapDomainAction; import google.registry.rdap.RdapDomainSearchAction; @@ -50,6 +51,7 @@ import google.registry.whois.WhoisServer; RdapModule.class, RegistrarUserModule.class, RequestModule.class, + WhiteboxModule.class, WhoisModule.class, }) interface FrontendRequestComponent { diff --git a/java/google/registry/module/tools/BUILD b/java/google/registry/module/tools/BUILD index fc57ff329..e24b0e0e2 100644 --- a/java/google/registry/module/tools/BUILD +++ b/java/google/registry/module/tools/BUILD @@ -24,6 +24,7 @@ java_library( "//java/google/registry/keyring/api", "//java/google/registry/loadtest", "//java/google/registry/mapreduce", + "//java/google/registry/monitoring/whitebox", "//java/google/registry/request", "//java/google/registry/request:modules", "//java/google/registry/tools/server", diff --git a/java/google/registry/module/tools/ToolsRequestComponent.java b/java/google/registry/module/tools/ToolsRequestComponent.java index 7a10ff928..65b3be5f2 100644 --- a/java/google/registry/module/tools/ToolsRequestComponent.java +++ b/java/google/registry/module/tools/ToolsRequestComponent.java @@ -22,6 +22,7 @@ import google.registry.flows.FlowComponent; import google.registry.loadtest.LoadTestAction; import google.registry.loadtest.LoadTestModule; import google.registry.mapreduce.MapreduceModule; +import google.registry.monitoring.whitebox.WhiteboxModule; import google.registry.request.RequestModule; import google.registry.request.RequestScope; import google.registry.tools.server.CreateGroupsAction; @@ -54,6 +55,7 @@ import google.registry.tools.server.javascrap.RefreshAllDomainsAction; MapreduceModule.class, RequestModule.class, ToolsServerModule.class, + WhiteboxModule.class, }) interface ToolsRequestComponent { BackfillAutorenewBillingFlagAction backfillAutorenewBillingFlagAction(); diff --git a/java/google/registry/monitoring/whitebox/EppMetric.java b/java/google/registry/monitoring/whitebox/EppMetric.java index 6e72d48c8..f3824ceb4 100644 --- a/java/google/registry/monitoring/whitebox/EppMetric.java +++ b/java/google/registry/monitoring/whitebox/EppMetric.java @@ -14,21 +14,18 @@ package google.registry.monitoring.whitebox; -import static com.google.apphosting.api.ApiProxy.getCurrentEnvironment; import static google.registry.bigquery.BigqueryUtils.toBigqueryTimestamp; -import static org.joda.time.DateTimeZone.UTC; import com.google.api.services.bigquery.model.TableFieldSchema; import com.google.auto.value.AutoValue; -import com.google.common.annotations.VisibleForTesting; import com.google.common.base.Optional; import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableMap; import google.registry.bigquery.BigqueryUtils.FieldType; import google.registry.model.eppoutput.Result.Code; import google.registry.request.RequestScope; +import google.registry.util.Clock; import java.util.concurrent.TimeUnit; -import javax.inject.Inject; import org.joda.time.DateTime; /** @@ -53,30 +50,6 @@ public abstract class EppMetric implements BigQueryMetric { new TableFieldSchema().setName("eppStatus").setType(FieldType.INTEGER.name()), new TableFieldSchema().setName("attempts").setType(FieldType.INTEGER.name())); - private static final String REQUEST_LOG_ID = "com.google.appengine.runtime.request_log_id"; - - private static EppMetric create( - String requestId, - DateTime startTimestamp, - DateTime endTimestamp, - String commandName, - String clientId, - String privilegeLevel, - String eppTarget, - Code status, - int attempts) { - return new AutoValue_EppMetric( - requestId, - startTimestamp, - endTimestamp, - Optional.fromNullable(commandName), - Optional.fromNullable(clientId), - Optional.fromNullable(privilegeLevel), - Optional.fromNullable(eppTarget), - Optional.fromNullable(status), - attempts); - } - public abstract String getRequestId(); public abstract DateTime getStartTimestamp(); @@ -141,90 +114,78 @@ public abstract class EppMetric implements BigQueryMetric { } } - /** A builder to create instances of {@link EppMetric}. */ - public static class Builder { + /** Create an {@link EppMetric.Builder}. */ + public static Builder builder() { + return new AutoValue_EppMetric.Builder(); + } - // Required values - private final String requestId; - private final DateTime startTimestamp; + /** + * Create an {@link EppMetric.Builder} for a request context, with the given request ID and + * with start and end timestamps taken from the given clock. + * + *

The start timestamp is recorded now, and the end timestamp at {@code build()}. + */ + public static Builder builderForRequest(String requestId, Clock clock) { + return builder() + .setRequestId(requestId) + .setStartTimestamp(clock.nowUtc()) + .setClock(clock); + } + + /** A builder to create instances of {@link EppMetric}. */ + @AutoValue.Builder + public abstract static class Builder { + + /** Builder-only counter of the number of attempts, to support {@link #incrementAttempts()}. */ private int attempts = 0; - // Optional values - private String commandName; - private String clientId; - private String privilegeLevel; - private String eppTarget; - private Code status; + /** Builder-only clock to support automatic recording of endTimestamp on {@link #build()}. */ + private Clock clock = null; - /** - * Create an {@link EppMetric.Builder}. - * - *

The start timestamp of metrics created via this instance's {@link Builder#build()} will be - * the time that this builder was created. - */ - @Inject - public Builder() { - this(DateTime.now(UTC)); - } + abstract Builder setRequestId(String requestId); - @VisibleForTesting - Builder(DateTime startTimestamp) { - this.requestId = getCurrentEnvironment().getAttributes().get(REQUEST_LOG_ID).toString(); - this.startTimestamp = startTimestamp; - this.attempts = 0; - } + abstract Builder setStartTimestamp(DateTime startTimestamp); - public Builder setCommandName(String value) { - commandName = value; - return this; - } + abstract Builder setEndTimestamp(DateTime endTimestamp); - public Builder setClientId(String value) { - clientId = value; - return this; - } + public abstract Builder setCommandName(String commandName); - public Builder setPrivilegeLevel(String value) { - privilegeLevel = value; - return this; - } + public abstract Builder setClientId(String clientId); - public Builder setEppTarget(String value) { - eppTarget = value; - return this; - } + public abstract Builder setClientId(Optional clientId); - public Builder setStatus(Code value) { - status = value; - return this; - } + public abstract Builder setPrivilegeLevel(String privilegeLevel); + + public abstract Builder setEppTarget(String eppTarget); + + public abstract Builder setStatus(Code code); + + abstract Builder setAttempts(Integer attempts); public Builder incrementAttempts() { attempts++; return this; } - @VisibleForTesting - EppMetric build(DateTime endTimestamp) { - return EppMetric.create( - requestId, - startTimestamp, - endTimestamp, - commandName, - clientId, - privilegeLevel, - eppTarget, - status, - attempts); + Builder setClock(Clock clock) { + this.clock = clock; + return this; } /** * Build an instance of {@link EppMetric} using this builder. * - *

The end timestamp of the metric will be the current time. + *

If a clock was provided with {@code setClock()}, the end timestamp will be set to the + * current timestamp of the clock; otherwise end timestamp must have been previously set. */ public EppMetric build() { - return build(DateTime.now(UTC)); + setAttempts(attempts); + if (clock != null) { + setEndTimestamp(clock.nowUtc()); + } + return autoBuild(); } + + abstract EppMetric autoBuild(); } } diff --git a/java/google/registry/monitoring/whitebox/WhiteboxModule.java b/java/google/registry/monitoring/whitebox/WhiteboxModule.java index b8cff04d8..801f12b1f 100644 --- a/java/google/registry/monitoring/whitebox/WhiteboxModule.java +++ b/java/google/registry/monitoring/whitebox/WhiteboxModule.java @@ -17,6 +17,7 @@ package google.registry.monitoring.whitebox; import static google.registry.request.RequestParameters.extractRequiredParameter; import com.google.api.services.bigquery.model.TableFieldSchema; +import com.google.apphosting.api.ApiProxy; import com.google.common.base.Supplier; import com.google.common.collect.ImmutableList; import dagger.Module; @@ -24,7 +25,9 @@ import dagger.Provides; import dagger.multibindings.IntoMap; import dagger.multibindings.StringKey; import google.registry.request.Parameter; +import google.registry.util.Clock; import java.util.UUID; +import javax.inject.Named; import javax.servlet.http.HttpServletRequest; /** @@ -33,6 +36,8 @@ import javax.servlet.http.HttpServletRequest; @Module public class WhiteboxModule { + private static final String REQUEST_LOG_ID = "com.google.appengine.runtime.request_log_id"; + @Provides @IntoMap @StringKey(EppMetric.TABLE_ID) @@ -68,4 +73,17 @@ public class WhiteboxModule { } }; } + + @Provides + @Named("requestLogId") + static String provideRequestLogId() { + return ApiProxy.getCurrentEnvironment().getAttributes().get(REQUEST_LOG_ID).toString(); + } + + /** Provides an EppMetric builder with the request ID and startTimestamp already initialized. */ + @Provides + static EppMetric.Builder provideEppMetricBuilder( + @Named("requestLogId") String requestLogId, Clock clock) { + return EppMetric.builderForRequest(requestLogId, clock); + } } diff --git a/javatests/google/registry/flows/EppControllerTest.java b/javatests/google/registry/flows/EppControllerTest.java index 3f08decb1..7ba6d90e9 100644 --- a/javatests/google/registry/flows/EppControllerTest.java +++ b/javatests/google/registry/flows/EppControllerTest.java @@ -32,8 +32,10 @@ import google.registry.monitoring.whitebox.EppMetric; import google.registry.testing.AppEngineRule; import google.registry.testing.FakeClock; import google.registry.testing.ShardableTestCase; +import google.registry.util.Clock; import google.registry.util.SystemClock; import google.registry.xml.ValidationMode; +import org.joda.time.DateTime; import org.junit.Before; import org.junit.Rule; import org.junit.Test; @@ -61,6 +63,10 @@ public class EppControllerTest extends ShardableTestCase { @Mock EppResponse eppResponse; @Mock Result result; + private static final DateTime startTime = DateTime.parse("2016-09-01T00:00:00Z"); + + private final Clock clock = new FakeClock(startTime); + private EppController eppController; @Before @@ -77,9 +83,9 @@ public class EppControllerTest extends ShardableTestCase { when(result.getCode()).thenReturn(Code.SuccessWithNoMessages); eppController = new EppController(); - eppController.metricBuilder = new EppMetric.Builder(); + eppController.metricBuilder = EppMetric.builderForRequest("request-id-1", clock); eppController.bigQueryMetricsEnqueuer = metricsEnqueuer; - eppController.clock = new FakeClock(); + eppController.clock = clock; eppController.flowComponentBuilder = flowComponentBuilder; eppController.eppMetrics = eppMetrics; } @@ -105,6 +111,9 @@ public class EppControllerTest extends ShardableTestCase { verify(metricsEnqueuer).export(metricCaptor.capture()); EppMetric metric = metricCaptor.getValue(); + assertThat(metric.getRequestId()).isEqualTo("request-id-1"); + assertThat(metric.getStartTimestamp()).isEqualTo(startTime); + assertThat(metric.getEndTimestamp()).isEqualTo(clock.nowUtc()); assertThat(metric.getClientId()).hasValue("some-client"); assertThat(metric.getPrivilegeLevel()).hasValue("NORMAL"); assertThat(metric.getStatus()).hasValue(Code.SyntaxError); @@ -126,6 +135,9 @@ public class EppControllerTest extends ShardableTestCase { verify(metricsEnqueuer).export(metricCaptor.capture()); EppMetric metric = metricCaptor.getValue(); + assertThat(metric.getRequestId()).isEqualTo("request-id-1"); + assertThat(metric.getStartTimestamp()).isEqualTo(startTime); + assertThat(metric.getEndTimestamp()).isEqualTo(clock.nowUtc()); assertThat(metric.getClientId()).hasValue("some-client"); assertThat(metric.getPrivilegeLevel()).hasValue("SUPERUSER"); assertThat(metric.getStatus()).hasValue(Code.SuccessWithNoMessages); diff --git a/javatests/google/registry/flows/EppTestComponent.java b/javatests/google/registry/flows/EppTestComponent.java index af3cf6398..fd8062c7a 100644 --- a/javatests/google/registry/flows/EppTestComponent.java +++ b/javatests/google/registry/flows/EppTestComponent.java @@ -42,13 +42,13 @@ interface EppTestComponent { @Module static class FakesAndMocksModule { final FakeClock clock; - final EppMetric.Builder metrics; + final EppMetric.Builder metricBuilder; final BigQueryMetricsEnqueuer metricsEnqueuer; final ModulesService modulesService; FakesAndMocksModule(FakeClock clock) { this.clock = clock; - this.metrics = new EppMetric.Builder(); + this.metricBuilder = EppMetric.builderForRequest("request-id-1", clock); this.modulesService = mock(ModulesService.class); this.metricsEnqueuer = mock(BigQueryMetricsEnqueuer.class); } @@ -60,7 +60,7 @@ interface EppTestComponent { @Provides EppMetric.Builder provideMetrics() { - return metrics; + return metricBuilder; } @Provides diff --git a/javatests/google/registry/flows/FlowRunnerTest.java b/javatests/google/registry/flows/FlowRunnerTest.java index 0caf5a94e..36aa64a17 100644 --- a/javatests/google/registry/flows/FlowRunnerTest.java +++ b/javatests/google/registry/flows/FlowRunnerTest.java @@ -90,7 +90,7 @@ public class FlowRunnerTest extends ShardableTestCase { flowRunner.isDryRun = false; flowRunner.isSuperuser = false; flowRunner.isTransactional = false; - flowRunner.metric = new EppMetric.Builder(); + flowRunner.metric = EppMetric.builderForRequest("request-id-1", flowRunner.clock); flowRunner.sessionMetadata = new StatelessRequestSessionMetadata("TheRegistrar", ImmutableSet.of()); flowRunner.trid = Trid.create("client-123", "server-456"); diff --git a/javatests/google/registry/monitoring/whitebox/EppMetricTest.java b/javatests/google/registry/monitoring/whitebox/EppMetricTest.java index 69e2a1494..a6ca39f83 100644 --- a/javatests/google/registry/monitoring/whitebox/EppMetricTest.java +++ b/javatests/google/registry/monitoring/whitebox/EppMetricTest.java @@ -36,21 +36,22 @@ public class EppMetricTest { @Test public void testGetBigQueryRowEncoding_encodesCorrectly() throws Exception { EppMetric metric = - new EppMetric.Builder(new DateTime(1337)) - .setEppTarget("target") - .setPrivilegeLevel("level") + EppMetric.builder() + .setRequestId("request-id-1") + .setStartTimestamp(new DateTime(1337)) + .setEndTimestamp(new DateTime(1338)) .setCommandName("command") .setClientId("client") + .setPrivilegeLevel("level") + .setEppTarget("target") .setStatus(Code.CommandUseError) .incrementAttempts() - .build(new DateTime(1338)); + .build(); - // The request_id is randomly generated and hard to mock without a lot of supporting code - // so we just use the tested metric's request_id verbatim. assertThat(metric.getBigQueryRowEncoding()) .containsExactlyEntriesIn( new ImmutableMap.Builder() - .put("requestId", metric.getRequestId()) + .put("requestId", "request-id-1") .put("startTime", "1.337000") .put("endTime", "1.338000") .put("commandName", "command") @@ -65,14 +66,17 @@ public class EppMetricTest { @Test public void testGetBigQueryRowEncoding_hasAllSchemaFields() throws Exception { EppMetric metric = - new EppMetric.Builder(new DateTime(1337)) - .setEppTarget("target") - .setPrivilegeLevel("level") + EppMetric.builder() + .setRequestId("request-id-1") + .setStartTimestamp(new DateTime(1337)) + .setEndTimestamp(new DateTime(1338)) .setCommandName("command") .setClientId("client") + .setPrivilegeLevel("level") + .setEppTarget("target") .setStatus(Code.CommandUseError) .incrementAttempts() - .build(new DateTime(1338)); + .build(); ImmutableSet.Builder schemaFieldNames = new ImmutableSet.Builder<>(); for (TableFieldSchema schemaField : metric.getSchemaFields()) { schemaFieldNames.add(schemaField.getName());