Change EppMetric.Builder to use @AutoValue.Builder

Getting rid of builder boilerplate makes my heart sing.  Since we can no
longer @Inject the Builder() constructor, this change adds a provider
in WhiteboxModule that calls a special builderForRequest() factory method,
which gets passed a request ID and Clock and preserves the existing
EppMetric magic that sets the start and end time for you.

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

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;

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 {

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",

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();

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 {
-
-    // Required values
-    private final String requestId;
-    private final DateTime startTimestamp;
-    private int attempts = 0;
-
-    // Optional values
-    private String commandName;
-    private String clientId;
-    private String privilegeLevel;
-    private String eppTarget;
-    private Code status;
+  /** Create an {@link EppMetric.Builder}. */
+  public static Builder builder() {
+    return new AutoValue_EppMetric.Builder();
+  }
 
   /**
-     * Create an {@link EppMetric.Builder}.
+   * 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.
    *
-     * <p>The start timestamp of metrics created via this instance's {@link Builder#build()} will be
-     * the time that this builder was created.
+   * <p>The start timestamp is recorded now, and the end timestamp at {@code build()}.
    */
-    @Inject
-    public Builder() {
-      this(DateTime.now(UTC));
+  public static Builder builderForRequest(String requestId, Clock clock) {
+      return builder()
+          .setRequestId(requestId)
+          .setStartTimestamp(clock.nowUtc())
+          .setClock(clock);
   }
 
-    @VisibleForTesting
-    Builder(DateTime startTimestamp) {
-      this.requestId = getCurrentEnvironment().getAttributes().get(REQUEST_LOG_ID).toString();
-      this.startTimestamp = startTimestamp;
-      this.attempts = 0;
-    }
+  /** A builder to create instances of {@link EppMetric}. */
+  @AutoValue.Builder
+  public abstract static class Builder {
 
-    public Builder setCommandName(String value) {
-      commandName = value;
-      return this;
-    }
+    /** Builder-only counter of the number of attempts, to support {@link #incrementAttempts()}. */
+    private int attempts = 0;
 
-    public Builder setClientId(String value) {
-      clientId = value;
-      return this;
-    }
+    /** Builder-only clock to support automatic recording of endTimestamp on {@link #build()}. */
+    private Clock clock = null;
 
-    public Builder setPrivilegeLevel(String value) {
-      privilegeLevel = value;
-      return this;
-    }
+    abstract Builder setRequestId(String requestId);
 
-    public Builder setEppTarget(String value) {
-      eppTarget = value;
-      return this;
-    }
+    abstract Builder setStartTimestamp(DateTime startTimestamp);
 
-    public Builder setStatus(Code value) {
-      status = value;
-      return this;
-    }
+    abstract Builder setEndTimestamp(DateTime endTimestamp);
+
+    public abstract Builder setCommandName(String commandName);
+
+    public abstract Builder setClientId(String clientId);
+
+    public abstract Builder setClientId(Optional<String> clientId);
+
+    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.
      *
-     * <p>The end timestamp of the metric will be the current time.
+     * <p>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();
   }
 }

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);
+  }
 }

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);

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

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.<String>of());
     flowRunner.trid = Trid.create("client-123", "server-456");

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<String, String>()
-                .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<String> schemaFieldNames = new ImmutableSet.Builder<>();
     for (TableFieldSchema schemaField : metric.getSchemaFields()) {
       schemaFieldNames.add(schemaField.getName());