Refactor EppMetrics into the EppMetric value type

This change refactors EppMetrics from the mutable self-exporting thing that it
was into a real value type EppMetric, and delegates exporting functionality to the
BigQueryMetricsEnqueuer.

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

java/google/registry/config/ConfigModule.java

@@ -648,7 +648,7 @@ public final class ConfigModule {
   }
 
   /**
-   * The reporting interval, for Metrics to be sent to a {@link
+   * The reporting interval, for BigQueryMetricsEnqueuer to be sent to a {@link
    * google.registry.monitoring.metrics.MetricWriter}.
    *
    * @see google.registry.monitoring.metrics.MetricReporter

java/google/registry/flows/EppController.java

@@ -25,7 +25,8 @@ import google.registry.model.eppoutput.EppOutput;
 import google.registry.model.eppoutput.EppResponse;
 import google.registry.model.eppoutput.Result;
 import google.registry.model.eppoutput.Result.Code;
-import google.registry.monitoring.whitebox.EppMetrics;
+import google.registry.monitoring.whitebox.BigQueryMetricsEnqueuer;
+import google.registry.monitoring.whitebox.EppMetric;
 import google.registry.util.Clock;
 import google.registry.util.FormattingLogger;
 import javax.inject.Inject;
@@ -41,7 +42,8 @@ public final class EppController {
 
   @Inject Clock clock;
   @Inject FlowComponent.Builder flowComponentBuilder;
-  @Inject EppMetrics metrics;
+  @Inject EppMetric.Builder metric;
+  @Inject BigQueryMetricsEnqueuer bigQueryMetricsEnqueuer;
   @Inject EppController() {}
 
   /** Read EPP XML, execute the matching flow, and return an {@link EppOutput}. */
@@ -52,20 +54,20 @@ public final class EppController {
       boolean isDryRun,
       boolean isSuperuser,
       byte[] inputXmlBytes) {
-    metrics.setClientId(sessionMetadata.getClientId());
-    metrics.setPrivilegeLevel(isSuperuser ? "SUPERUSER" : "NORMAL");
+    metric.setClientId(sessionMetadata.getClientId());
+    metric.setPrivilegeLevel(isSuperuser ? "SUPERUSER" : "NORMAL");
     try {
       EppInput eppInput;
       try {
         eppInput = unmarshal(EppInput.class, inputXmlBytes);
       } catch (EppException e) {
         // Send the client an error message, with no clTRID since we couldn't unmarshal it.
-        metrics.setEppStatus(e.getResult().getCode());
+        metric.setStatus(e.getResult().getCode());
         return getErrorResponse(clock, e.getResult(), Trid.create(null));
       }
-      metrics.setCommandName(eppInput.getCommandName());
+      metric.setCommandName(eppInput.getCommandName());
       if (!eppInput.getTargetIds().isEmpty()) {
-        metrics.setEppTarget(Joiner.on(',').join(eppInput.getTargetIds()));
+        metric.setEppTarget(Joiner.on(',').join(eppInput.getTargetIds()));
       }
       EppOutput output = runFlowConvertEppErrors(flowComponentBuilder
           .flowModule(new FlowModule.Builder()
@@ -79,11 +81,11 @@ public final class EppController {
               .build())
           .build());
       if (output.isResponse()) {
-        metrics.setEppStatus(output.getResponse().getResult().getCode());
+        metric.setStatus(output.getResponse().getResult().getCode());
       }
       return output;
     } finally {
-      metrics.export();
+      bigQueryMetricsEnqueuer.export(metric.build());
     }
   }
 

java/google/registry/flows/FlowRunner.java

@@ -31,7 +31,7 @@ import google.registry.flows.FlowModule.Transactional;
 import google.registry.model.eppcommon.Trid;
 import google.registry.model.eppinput.EppInput;
 import google.registry.model.eppoutput.EppOutput;
-import google.registry.monitoring.whitebox.EppMetrics;
+import google.registry.monitoring.whitebox.EppMetric;
 import google.registry.util.Clock;
 import google.registry.util.FormattingLogger;
 import javax.annotation.Nullable;
@@ -67,7 +67,7 @@ public class FlowRunner {
   @Inject @DryRun boolean isDryRun;
   @Inject @Superuser boolean isSuperuser;
   @Inject @Transactional boolean isTransactional;
-  @Inject EppMetrics metrics;
+  @Inject EppMetric.Builder metric;
   @Inject SessionMetadata sessionMetadata;
   @Inject Trid trid;
   @Inject FlowRunner() {}
@@ -100,7 +100,7 @@ public class FlowRunner {
             "xml", prettyXml,
             "xmlBytes", xmlBase64)));
     if (!isTransactional) {
-      metrics.incrementAttempts();
+      metric.incrementAttempts();
       return createAndInitFlow(clock.nowUtc()).run();
     }
     // We log the command in a structured format. Note that we do this before the transaction;
@@ -111,20 +111,24 @@ public class FlowRunner {
         .add("privileges", isSuperuser ? "SUPERUSER" : "NORMAL")
         .add("xmlBytes", xmlBase64));
     try {
-      EppOutput flowResult = ofy().transact(new Work<EppOutput>() {
-        @Override
-        public EppOutput run() {
-          metrics.incrementAttempts();
-          try {
-            EppOutput output = createAndInitFlow(ofy().getTransactionTime()).run();
-            if (isDryRun) {
-              throw new DryRunException(output);
-            }
-            return output;
-          } catch (EppException e) {
-            throw new RuntimeException(e);
-          }
-        }});
+      EppOutput flowResult =
+          ofy()
+              .transact(
+                  new Work<EppOutput>() {
+                    @Override
+                    public EppOutput run() {
+                      metric.incrementAttempts();
+                      try {
+                        EppOutput output = createAndInitFlow(ofy().getTransactionTime()).run();
+                        if (isDryRun) {
+                          throw new DryRunException(output);
+                        }
+                        return output;
+                      } catch (EppException e) {
+                        throw new RuntimeException(e);
+                      }
+                    }
+                  });
       logger.info("EPP_Mutation_Committed " + new JsonLogStatement(trid)
           .add("createdRepoId", flowResult.getResponse().getCreatedRepoId())
           .add("executionTime", flowResult.getResponse().getExecutionTime().getMillis()));

java/google/registry/module/tools/ToolsComponent.java

@@ -27,6 +27,7 @@ import google.registry.request.Modules.AppIdentityCredentialModule;
 import google.registry.request.Modules.DatastoreServiceModule;
 import google.registry.request.Modules.GoogleCredentialModule;
 import google.registry.request.Modules.Jackson2Module;
+import google.registry.request.Modules.ModulesServiceModule;
 import google.registry.request.Modules.UrlFetchTransportModule;
 import google.registry.request.Modules.UseAppIdentityCredentialForGoogleApisModule;
 import google.registry.request.RequestModule;
@@ -49,6 +50,7 @@ import javax.inject.Singleton;
         GroupssettingsModule.class,
         Jackson2Module.class,
         KeyModule.class,
+        ModulesServiceModule.class,
         UrlFetchTransportModule.class,
         UseAppIdentityCredentialForGoogleApisModule.class,
         SystemClockModule.class,

java/google/registry/monitoring/whitebox/BUILD

@@ -22,6 +22,7 @@ java_library(
         "//third_party/java/appengine:appengine-api",
         "//third_party/java/appengine_mapreduce2:appengine_mapreduce",
         "//third_party/java/auto:auto_factory",
+        "//third_party/java/auto:auto_value",
         "//third_party/java/dagger",
         "//third_party/java/joda_time",
         "//third_party/java/jsr305_annotations",

java/google/registry/monitoring/whitebox/BigQueryMetric.java

@@ -0,0 +1,36 @@
+// Copyright 2016 The Domain Registry Authors. All Rights Reserved.
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//     http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package google.registry.monitoring.whitebox;
+
+import com.google.api.services.bigquery.model.TableFieldSchema;
+import com.google.common.collect.ImmutableList;
+import com.google.common.collect.ImmutableMap;
+
+/**
+ * A metric which can be encoded into a BigQuery row.
+ *
+ * @see BigQueryMetricsEnqueuer
+ */
+public interface BigQueryMetric {
+
+  /** Get the BigQuery table name for this metric. */
+  String getTableId();
+
+  /** Get the schema description for the BigQuery table. */
+  ImmutableList<TableFieldSchema> getSchemaFields();
+
+  /** Get a map of the row values for this metric instance. */
+  ImmutableMap<String, String> getBigQueryRowEncoding();
+}

java/google/registry/monitoring/whitebox/BigQueryMetricsEnqueuer.java

@@ -16,67 +16,54 @@ package google.registry.monitoring.whitebox;
 
 import static com.google.appengine.api.taskqueue.QueueFactory.getQueue;
 import static com.google.appengine.api.taskqueue.TaskOptions.Builder.withUrl;
-import static google.registry.bigquery.BigqueryUtils.toBigqueryTimestamp;
 
 import com.google.appengine.api.modules.ModulesService;
-import com.google.appengine.api.modules.ModulesServiceFactory;
 import com.google.appengine.api.taskqueue.TaskOptions;
 import com.google.appengine.api.taskqueue.TransientFailureException;
-import com.google.common.base.Supplier;
-import google.registry.util.Clock;
+import com.google.common.annotations.VisibleForTesting;
 import google.registry.util.FormattingLogger;
-import google.registry.util.NonFinalForTesting;
-import google.registry.util.SystemClock;
-import java.util.HashMap;
-import java.util.Map;
 import java.util.Map.Entry;
 import java.util.UUID;
-import java.util.concurrent.TimeUnit;
+import javax.inject.Inject;
 
-/** A collector of metric information. */
-public abstract class Metrics {
+/**
+ * A collector of metric information. Enqueues collected metrics to a task queue to be written to
+ * BigQuery asynchronously.
+ *
+ * @see MetricsExportAction
+ */
+public class BigQueryMetricsEnqueuer {
 
   private static final FormattingLogger logger = FormattingLogger.getLoggerForCallerClass();
 
   public static final String QUEUE = "bigquery-streaming-metrics";
 
-  @NonFinalForTesting
-  private static ModulesService modulesService = ModulesServiceFactory.getModulesService();
+  @Inject ModulesService modulesService;
 
-  @NonFinalForTesting
-  private static Clock clock = new SystemClock();
+  @Inject
+  BigQueryMetricsEnqueuer() {}
 
-  @NonFinalForTesting
-  private static Supplier<String> idGenerator =
-      new Supplier<String>() {
-        @Override
-        public String get() {
-          return UUID.randomUUID().toString();
-        }};
-
-  protected final Map<String, Object> fields = new HashMap<>();
-
-  private final long startTimeMillis = clock.nowUtc().getMillis();
-
-  public void setTableId(String tableId) {
-    fields.put("tableId", tableId);
-  }
-
-  public void export() {
+  @VisibleForTesting
+  void export(BigQueryMetric metric, String insertId) {
     try {
       String hostname = modulesService.getVersionHostname("backend", null);
-      TaskOptions opts = withUrl(MetricsExportAction.PATH)
-          .header("Host", hostname)
-          .param("insertId", idGenerator.get())
-          .param("startTime", toBigqueryTimestamp(startTimeMillis, TimeUnit.MILLISECONDS))
-          .param("endTime", toBigqueryTimestamp(clock.nowUtc().getMillis(), TimeUnit.MILLISECONDS));
-      for (Entry<String, Object> entry : fields.entrySet()) {
-        opts.param(entry.getKey(), String.valueOf(entry.getValue()));
+      TaskOptions opts =
+          withUrl(MetricsExportAction.PATH)
+              .header("Host", hostname)
+              .param("insertId", insertId);
+      for (Entry<String, String> entry : metric.getBigQueryRowEncoding().entrySet()) {
+        opts.param(entry.getKey(), entry.getValue());
       }
+      opts.param("tableId", metric.getTableId());
       getQueue(QUEUE).add(opts);
     } catch (TransientFailureException e) {
       // Log and swallow. We may drop some metrics here but this should be rare.
       logger.info(e, e.getMessage());
     }
   }
+
+  /** Enqueue a metric to be exported to BigQuery. */
+  public void export(BigQueryMetric metric) {
+    export(metric, UUID.randomUUID().toString());
+  }
 }

java/google/registry/monitoring/whitebox/EppMetric.java

@@ -0,0 +1,228 @@
+// Copyright 2016 The Domain Registry Authors. All Rights Reserved.
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//     http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+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 java.util.concurrent.TimeUnit;
+import javax.inject.Inject;
+import org.joda.time.DateTime;
+
+/**
+ * A value class for recording attributes of an EPP metric.
+ *
+ * @see BigQueryMetricsEnqueuer
+ */
+@AutoValue
+@RequestScope
+public abstract class EppMetric implements BigQueryMetric {
+
+  static final String TABLE_ID = "eppMetrics";
+  static final ImmutableList<TableFieldSchema> SCHEMA_FIELDS =
+      ImmutableList.of(
+          new TableFieldSchema().setName("requestId").setType(FieldType.STRING.name()),
+          new TableFieldSchema().setName("startTime").setType(FieldType.TIMESTAMP.name()),
+          new TableFieldSchema().setName("endTime").setType(FieldType.TIMESTAMP.name()),
+          new TableFieldSchema().setName("commandName").setType(FieldType.STRING.name()),
+          new TableFieldSchema().setName("clientId").setType(FieldType.STRING.name()),
+          new TableFieldSchema().setName("privilegeLevel").setType(FieldType.STRING.name()),
+          new TableFieldSchema().setName("eppTarget").setType(FieldType.STRING.name()),
+          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.ofNullable(commandName),
+        Optional.ofNullable(clientId),
+        Optional.ofNullable(privilegeLevel),
+        Optional.ofNullable(eppTarget),
+        Optional.ofNullable(status),
+        attempts);
+  }
+
+  public abstract String getRequestId();
+
+  public abstract DateTime getStartTimestamp();
+
+  public abstract DateTime getEndTimestamp();
+
+  public abstract Optional<String> getCommandName();
+
+  public abstract Optional<String> getClientId();
+
+  public abstract Optional<String> getPrivilegeLevel();
+
+  public abstract Optional<String> getEppTarget();
+
+  public abstract Optional<Code> getStatus();
+
+  public abstract Integer getAttempts();
+
+  @Override
+  public String getTableId() {
+    return TABLE_ID;
+  }
+
+  @Override
+  public ImmutableList<TableFieldSchema> getSchemaFields() {
+    return SCHEMA_FIELDS;
+  }
+
+  @Override
+  public ImmutableMap<String, String> getBigQueryRowEncoding() {
+    // Create map builder, start with required values
+    ImmutableMap.Builder<String, String> map =
+        ImmutableMap.<String, String>builder()
+            .put("requestId", getRequestId())
+            .put(
+                "startTime",
+                toBigqueryTimestamp(getStartTimestamp().getMillis(), TimeUnit.MILLISECONDS))
+            .put(
+                "endTime",
+                toBigqueryTimestamp(getEndTimestamp().getMillis(), TimeUnit.MILLISECONDS))
+            .put("attempts", getAttempts().toString());
+    // Populate optional values, if present
+    addOptional("commandName", getCommandName(), map);
+    addOptional("clientId", getClientId(), map);
+    addOptional("privilegeLevel", getPrivilegeLevel(), map);
+    addOptional("eppTarget", getEppTarget(), map);
+    addOptional("status", getStatus(), map);
+
+    return map.build();
+  }
+
+  /**
+   * Helper method to populate an {@link com.google.common.collect.ImmutableMap.Builder} with an
+   * {@link Optional} value if the value is {@link Optional#isPresent()}.
+   */
+  private static <T> void addOptional(
+      String key, Optional<T> value, ImmutableMap.Builder<String, String> map) {
+    if (value.isPresent()) {
+      map.put(key, value.get().toString());
+    }
+  }
+
+  /** 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}.
+     *
+     * <p>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));
+    }
+
+    @VisibleForTesting
+    Builder(DateTime startTimestamp) {
+      this.requestId = getCurrentEnvironment().getAttributes().get(REQUEST_LOG_ID).toString();
+      this.startTimestamp = startTimestamp;
+      this.attempts = 0;
+    }
+
+    public Builder setCommandName(String value) {
+      commandName = value;
+      return this;
+    }
+
+    public Builder setClientId(String value) {
+      clientId = value;
+      return this;
+    }
+
+    public Builder setPrivilegeLevel(String value) {
+      privilegeLevel = value;
+      return this;
+    }
+
+    public Builder setEppTarget(String value) {
+      eppTarget = value;
+      return this;
+    }
+
+    public Builder setStatus(Code value) {
+      status = value;
+      return this;
+    }
+
+    public Builder incrementAttempts() {
+      attempts++;
+      return this;
+    }
+
+    @VisibleForTesting
+    EppMetric build(DateTime endTimestamp) {
+      return EppMetric.create(
+          requestId,
+          startTimestamp,
+          endTimestamp,
+          commandName,
+          clientId,
+          privilegeLevel,
+          eppTarget,
+          status,
+          attempts);
+    }
+
+    /**
+     * Build an instance of {@link EppMetric} using this builder.
+     *
+     * <p>The end timestamp of the metric will be the current time.
+     */
+    public EppMetric build() {
+      return build(DateTime.now(UTC));
+    }
+  }
+}

java/google/registry/monitoring/whitebox/EppMetrics.java

@@ -1,76 +0,0 @@
-// Copyright 2016 The Domain Registry Authors. All Rights Reserved.
-//
-// Licensed under the Apache License, Version 2.0 (the "License");
-// you may not use this file except in compliance with the License.
-// You may obtain a copy of the License at
-//
-//     http://www.apache.org/licenses/LICENSE-2.0
-//
-// Unless required by applicable law or agreed to in writing, software
-// distributed under the License is distributed on an "AS IS" BASIS,
-// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
-// See the License for the specific language governing permissions and
-// limitations under the License.
-
-package google.registry.monitoring.whitebox;
-
-import static com.google.apphosting.api.ApiProxy.getCurrentEnvironment;
-
-import com.google.api.services.bigquery.model.TableFieldSchema;
-import com.google.common.collect.ImmutableList;
-import google.registry.bigquery.BigqueryUtils.FieldType;
-import google.registry.model.eppoutput.Result.Code;
-import google.registry.request.RequestScope;
-import javax.inject.Inject;
-
-/** The EPP Metrics collector. See {@link Metrics}. */
-@RequestScope
-public class EppMetrics extends Metrics {
-
-  private static final String REQUEST_LOG_ID = "com.google.appengine.runtime.request_log_id";
-
-  static final String TABLE_ID = "eppMetrics";
-
-  static final ImmutableList<TableFieldSchema> SCHEMA_FIELDS =
-      ImmutableList.of(
-          new TableFieldSchema().setName("requestId").setType(FieldType.STRING.name()),
-          new TableFieldSchema().setName("startTime").setType(FieldType.TIMESTAMP.name()),
-          new TableFieldSchema().setName("endTime").setType(FieldType.TIMESTAMP.name()),
-          new TableFieldSchema().setName("commandName").setType(FieldType.STRING.name()),
-          new TableFieldSchema().setName("clientId").setType(FieldType.STRING.name()),
-          new TableFieldSchema().setName("privilegeLevel").setType(FieldType.STRING.name()),
-          new TableFieldSchema().setName("eppTarget").setType(FieldType.STRING.name()),
-          new TableFieldSchema().setName("eppStatus").setType(FieldType.INTEGER.name()),
-          new TableFieldSchema().setName("attempts").setType(FieldType.INTEGER.name()));
-
-  @Inject
-  public EppMetrics() {
-    setTableId(TABLE_ID);
-    fields.put("attempts", 0);
-    fields.put("requestId", getCurrentEnvironment().getAttributes().get(REQUEST_LOG_ID).toString());
-  }
-
-  public void setCommandName(String name) {
-    fields.put("commandName", name);
-  }
-
-  public void setClientId(String clientId) {
-    fields.put("clientId", clientId);
-  }
-
-  public void setPrivilegeLevel(String level) {
-    fields.put("privilegeLevel", level);
-  }
-
-  public void setEppTarget(String eppTarget) {
-    fields.put("eppTarget", eppTarget);
-  }
-
-  public void setEppStatus(Code status) {
-    fields.put("eppStatus", String.valueOf(status.code));
-  }
-
-  public void incrementAttempts() {
-    fields.put("attempts", ((int) fields.get("attempts")) + 1);
-  }
-}

java/google/registry/monitoring/whitebox/WhiteboxModule.java

@@ -35,9 +35,9 @@ public class WhiteboxModule {
 
   @Provides
   @IntoMap
-  @StringKey(EppMetrics.TABLE_ID)
+  @StringKey(EppMetric.TABLE_ID)
   static ImmutableList<TableFieldSchema> provideEppMetricsSchema() {
-    return EppMetrics.SCHEMA_FIELDS;
+    return EppMetric.SCHEMA_FIELDS;
   }
 
   @Provides