Remove TLD @parameter from inside the DnsMetrics

Currently, the TLD is a "hidden" part of the metric - we record it, but by looking at the call site you can't see that we record it.

Also, it's injected from the query / POST parameter, so we might not even be aware of what the value is and it might not be the value we wanted.

Instead, making it explicit in the Metric call. That way it's also more similar to the "logging" statements that record the same data but have to explicitly output the TLD.

It also makes the tests better, as we test that we indeed record the correct TLD

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

java/google/registry/dns/DnsMetrics.java

@@ -14,8 +14,6 @@
 
 package google.registry.dns;
 
-import static google.registry.request.RequestParameters.PARAM_TLD;
-
 import com.google.common.collect.ImmutableSet;
 import com.google.monitoring.metrics.DistributionFitter;
 import com.google.monitoring.metrics.EventMetric;
@@ -25,7 +23,6 @@
 import com.google.monitoring.metrics.LabelDescriptor;
 import com.google.monitoring.metrics.MetricRegistryImpl;
 import google.registry.config.RegistryEnvironment;
-import google.registry.request.Parameter;
 import javax.inject.Inject;
 import org.joda.time.Duration;
 
@@ -184,7 +181,6 @@ public enum ActionStatus { SUCCESS, COMMIT_FAILURE, LOCK_FAILURE, BAD_WRITER, BA
               EXPONENTIAL_FITTER);
 
   @Inject RegistryEnvironment registryEnvironment;
-  @Inject @Parameter(PARAM_TLD) String tld;
 
   @Inject
   DnsMetrics() {}
@@ -193,7 +189,7 @@ public enum ActionStatus { SUCCESS, COMMIT_FAILURE, LOCK_FAILURE, BAD_WRITER, BA
    * Increment a monotonic counter that tracks calls to {@link
    * google.registry.dns.writer.DnsWriter#publishDomain(String)}, per TLD.
    */
-  public void incrementPublishDomainRequests(long numRequests, PublishStatus status) {
+  public void incrementPublishDomainRequests(String tld, long numRequests, PublishStatus status) {
     if (numRequests > 0) {
       publishDomainRequests.incrementBy(numRequests, tld, status.name());
     }
@@ -203,7 +199,7 @@ public void incrementPublishDomainRequests(long numRequests, PublishStatus statu
    * Increment a monotonic counter that tracks calls to {@link
    * google.registry.dns.writer.DnsWriter#publishHost(String)}, per TLD.
    */
-  public void incrementPublishHostRequests(long numRequests, PublishStatus status) {
+  public void incrementPublishHostRequests(String tld, long numRequests, PublishStatus status) {
     if (numRequests > 0) {
       publishHostRequests.incrementBy(numRequests, tld, status.name());
     }
@@ -212,12 +208,13 @@ public void incrementPublishHostRequests(long numRequests, PublishStatus status)
   /**
    * Measures information about the entire batched commit, per TLD.
    *
-   * The information includes running times (per item and per commit), and batch sizes (per item and
-   * per commit)
+   * <p>The information includes running times (per item and per commit), and batch sizes (per item
+   * and per commit)
    *
-   * This is to be used for load testing the system, and will not measure anything in prod.
+   * <p>This is to be used for load testing the system, and will not measure anything in prod.
    */
   void recordCommit(
+      String tld,
       String dnsWriter,
       CommitStatus status,
       Duration processingDuration,
@@ -254,6 +251,7 @@ void recordCommit(
   }
 
   void recordActionResult(
+      String tld,
       String dnsWriter,
       ActionStatus status,
       int numberOfItems,

java/google/registry/dns/PublishDnsUpdatesAction.java

@@ -96,6 +96,7 @@ private void recordActionResult(ActionStatus status) {
     DateTime now = clock.nowUtc();
 
     dnsMetrics.recordActionResult(
+        tld,
         dnsWriter,
         status,
         nullToEmpty(domains).size() + nullToEmpty(hosts).size(),
@@ -197,8 +198,8 @@ private void processBatch() {
         domainsPublished += 1;
       }
     }
-    dnsMetrics.incrementPublishDomainRequests(domainsPublished, PublishStatus.ACCEPTED);
-    dnsMetrics.incrementPublishDomainRequests(domainsRejected, PublishStatus.REJECTED);
+    dnsMetrics.incrementPublishDomainRequests(tld, domainsPublished, PublishStatus.ACCEPTED);
+    dnsMetrics.incrementPublishDomainRequests(tld, domainsRejected, PublishStatus.REJECTED);
 
     int hostsPublished = 0;
     int hostsRejected = 0;
@@ -213,8 +214,8 @@ private void processBatch() {
         hostsPublished += 1;
       }
     }
-    dnsMetrics.incrementPublishHostRequests(hostsPublished, PublishStatus.ACCEPTED);
-    dnsMetrics.incrementPublishHostRequests(hostsRejected, PublishStatus.REJECTED);
+    dnsMetrics.incrementPublishHostRequests(tld, hostsPublished, PublishStatus.ACCEPTED);
+    dnsMetrics.incrementPublishHostRequests(tld, hostsRejected, PublishStatus.REJECTED);
 
     // If we got here it means we managed to stage the entire batch without any errors.
     // Next we will commit the batch.
@@ -229,11 +230,7 @@ private void processBatch() {
       recordActionResult(actionStatus);
       Duration duration = new Duration(timeAtStart, clock.nowUtc());
       dnsMetrics.recordCommit(
-          dnsWriter,
-          commitStatus,
-          duration,
-          domainsPublished,
-          hostsPublished);
+          tld, dnsWriter, commitStatus, duration, domainsPublished, hostsPublished);
       logger.atInfo().log(
           "writer.commit() statistics: TLD: %s, dnsWriter: %s, commitStatus: %s, duration: %s, "
               + "domainsPublished: %d, domainsRejected: %d, hostsPublished: %d, hostsRejected: %d",

javatests/google/registry/dns/PublishDnsUpdatesActionTest.java

@@ -115,13 +115,15 @@ public void testHost_published() {
     verify(dnsWriter).publishHost("ns1.example.xn--q9jyb4c");
     verify(dnsWriter).commit();
     verifyNoMoreInteractions(dnsWriter);
-    verify(dnsMetrics).incrementPublishDomainRequests(0, PublishStatus.ACCEPTED);
-    verify(dnsMetrics).incrementPublishDomainRequests(0, PublishStatus.REJECTED);
-    verify(dnsMetrics).incrementPublishHostRequests(1, PublishStatus.ACCEPTED);
-    verify(dnsMetrics).incrementPublishHostRequests(0, PublishStatus.REJECTED);
-    verify(dnsMetrics).recordCommit("correctWriter", CommitStatus.SUCCESS, Duration.ZERO, 0, 1);
+    verify(dnsMetrics).incrementPublishDomainRequests("xn--q9jyb4c", 0, PublishStatus.ACCEPTED);
+    verify(dnsMetrics).incrementPublishDomainRequests("xn--q9jyb4c", 0, PublishStatus.REJECTED);
+    verify(dnsMetrics).incrementPublishHostRequests("xn--q9jyb4c", 1, PublishStatus.ACCEPTED);
+    verify(dnsMetrics).incrementPublishHostRequests("xn--q9jyb4c", 0, PublishStatus.REJECTED);
+    verify(dnsMetrics)
+        .recordCommit("xn--q9jyb4c", "correctWriter", CommitStatus.SUCCESS, Duration.ZERO, 0, 1);
     verify(dnsMetrics)
         .recordActionResult(
+            "xn--q9jyb4c",
             "correctWriter",
             ActionStatus.SUCCESS,
             1,
@@ -141,13 +143,15 @@ public void testDomain_published() {
     verify(dnsWriter).publishDomain("example.xn--q9jyb4c");
     verify(dnsWriter).commit();
     verifyNoMoreInteractions(dnsWriter);
-    verify(dnsMetrics).incrementPublishDomainRequests(1, PublishStatus.ACCEPTED);
-    verify(dnsMetrics).incrementPublishDomainRequests(0, PublishStatus.REJECTED);
-    verify(dnsMetrics).incrementPublishHostRequests(0, PublishStatus.ACCEPTED);
-    verify(dnsMetrics).incrementPublishHostRequests(0, PublishStatus.REJECTED);
-    verify(dnsMetrics).recordCommit("correctWriter", CommitStatus.SUCCESS, Duration.ZERO, 1, 0);
+    verify(dnsMetrics).incrementPublishDomainRequests("xn--q9jyb4c", 1, PublishStatus.ACCEPTED);
+    verify(dnsMetrics).incrementPublishDomainRequests("xn--q9jyb4c", 0, PublishStatus.REJECTED);
+    verify(dnsMetrics).incrementPublishHostRequests("xn--q9jyb4c", 0, PublishStatus.ACCEPTED);
+    verify(dnsMetrics).incrementPublishHostRequests("xn--q9jyb4c", 0, PublishStatus.REJECTED);
+    verify(dnsMetrics)
+        .recordCommit("xn--q9jyb4c", "correctWriter", CommitStatus.SUCCESS, Duration.ZERO, 1, 0);
     verify(dnsMetrics)
         .recordActionResult(
+            "xn--q9jyb4c",
             "correctWriter",
             ActionStatus.SUCCESS,
             1,
@@ -186,13 +190,15 @@ public void testPublish_commitFails() {
 
     assertThrows(RuntimeException.class, action::run);
 
-    verify(dnsMetrics).incrementPublishDomainRequests(2, PublishStatus.ACCEPTED);
-    verify(dnsMetrics).incrementPublishDomainRequests(0, PublishStatus.REJECTED);
-    verify(dnsMetrics).incrementPublishHostRequests(3, PublishStatus.ACCEPTED);
-    verify(dnsMetrics).incrementPublishHostRequests(0, PublishStatus.REJECTED);
-    verify(dnsMetrics).recordCommit("correctWriter", CommitStatus.FAILURE, Duration.ZERO, 2, 3);
+    verify(dnsMetrics).incrementPublishDomainRequests("xn--q9jyb4c", 2, PublishStatus.ACCEPTED);
+    verify(dnsMetrics).incrementPublishDomainRequests("xn--q9jyb4c", 0, PublishStatus.REJECTED);
+    verify(dnsMetrics).incrementPublishHostRequests("xn--q9jyb4c", 3, PublishStatus.ACCEPTED);
+    verify(dnsMetrics).incrementPublishHostRequests("xn--q9jyb4c", 0, PublishStatus.REJECTED);
+    verify(dnsMetrics)
+        .recordCommit("xn--q9jyb4c", "correctWriter", CommitStatus.FAILURE, Duration.ZERO, 2, 3);
     verify(dnsMetrics)
         .recordActionResult(
+            "xn--q9jyb4c",
             "correctWriter",
             ActionStatus.COMMIT_FAILURE,
             5,
@@ -218,13 +224,15 @@ public void testHostAndDomain_published() {
     verify(dnsWriter).publishHost("ns1.example2.xn--q9jyb4c");
     verify(dnsWriter).commit();
     verifyNoMoreInteractions(dnsWriter);
-    verify(dnsMetrics).incrementPublishDomainRequests(2, PublishStatus.ACCEPTED);
-    verify(dnsMetrics).incrementPublishDomainRequests(0, PublishStatus.REJECTED);
-    verify(dnsMetrics).incrementPublishHostRequests(3, PublishStatus.ACCEPTED);
-    verify(dnsMetrics).incrementPublishHostRequests(0, PublishStatus.REJECTED);
-    verify(dnsMetrics).recordCommit("correctWriter", CommitStatus.SUCCESS, Duration.ZERO, 2, 3);
+    verify(dnsMetrics).incrementPublishDomainRequests("xn--q9jyb4c", 2, PublishStatus.ACCEPTED);
+    verify(dnsMetrics).incrementPublishDomainRequests("xn--q9jyb4c", 0, PublishStatus.REJECTED);
+    verify(dnsMetrics).incrementPublishHostRequests("xn--q9jyb4c", 3, PublishStatus.ACCEPTED);
+    verify(dnsMetrics).incrementPublishHostRequests("xn--q9jyb4c", 0, PublishStatus.REJECTED);
+    verify(dnsMetrics)
+        .recordCommit("xn--q9jyb4c", "correctWriter", CommitStatus.SUCCESS, Duration.ZERO, 2, 3);
     verify(dnsMetrics)
         .recordActionResult(
+            "xn--q9jyb4c",
             "correctWriter",
             ActionStatus.SUCCESS,
             5,
@@ -244,13 +252,15 @@ public void testWrongTld_notPublished() {
 
     verify(dnsWriter).commit();
     verifyNoMoreInteractions(dnsWriter);
-    verify(dnsMetrics).incrementPublishDomainRequests(0, PublishStatus.ACCEPTED);
-    verify(dnsMetrics).incrementPublishDomainRequests(2, PublishStatus.REJECTED);
-    verify(dnsMetrics).incrementPublishHostRequests(0, PublishStatus.ACCEPTED);
-    verify(dnsMetrics).incrementPublishHostRequests(3, PublishStatus.REJECTED);
-    verify(dnsMetrics).recordCommit("correctWriter", CommitStatus.SUCCESS, Duration.ZERO, 0, 0);
+    verify(dnsMetrics).incrementPublishDomainRequests("xn--q9jyb4c", 0, PublishStatus.ACCEPTED);
+    verify(dnsMetrics).incrementPublishDomainRequests("xn--q9jyb4c", 2, PublishStatus.REJECTED);
+    verify(dnsMetrics).incrementPublishHostRequests("xn--q9jyb4c", 0, PublishStatus.ACCEPTED);
+    verify(dnsMetrics).incrementPublishHostRequests("xn--q9jyb4c", 3, PublishStatus.REJECTED);
+    verify(dnsMetrics)
+        .recordCommit("xn--q9jyb4c", "correctWriter", CommitStatus.SUCCESS, Duration.ZERO, 0, 0);
     verify(dnsMetrics)
         .recordActionResult(
+            "xn--q9jyb4c",
             "correctWriter",
             ActionStatus.SUCCESS,
             5,
@@ -274,6 +284,7 @@ public void testLockIsntAvailable() {
     verifyNoMoreInteractions(dnsWriter);
     verify(dnsMetrics)
         .recordActionResult(
+            "xn--q9jyb4c",
             "correctWriter",
             ActionStatus.LOCK_FAILURE,
             5,
@@ -297,6 +308,7 @@ public void testParam_invalidLockIndex() {
     verifyNoMoreInteractions(dnsWriter);
     verify(dnsMetrics)
         .recordActionResult(
+            "xn--q9jyb4c",
             "correctWriter",
             ActionStatus.BAD_LOCK_INDEX,
             2,
@@ -322,6 +334,7 @@ public void testRegistryParam_mismatchedMaxLocks() {
     verifyNoMoreInteractions(dnsWriter);
     verify(dnsMetrics)
         .recordActionResult(
+            "xn--q9jyb4c",
             "correctWriter",
             ActionStatus.BAD_LOCK_INDEX,
             2,
@@ -345,6 +358,7 @@ public void testWrongDnsWriter() {
     verifyNoMoreInteractions(dnsWriter);
     verify(dnsMetrics)
         .recordActionResult(
+            "xn--q9jyb4c",
             "wrongWriter",
             ActionStatus.BAD_WRITER,
             5,