Reduce cardinality of reserved list processing time metric (#2542)

This single metric currently accounts for 22.2% of our total metrics bill,
almost double the size of our EPP requests metric, while also simultaneously
being much less useful. This change reduces the cardinality by removing two
parameters we don't care that much about, which should significantly reduce the
size and thus the cost. If after this change the metric is still too large, I'll
also then remove the matchCount parameter from this metric. We could possibly
even consider deleting the metric in its entirety, as we hardly ever use it.

This PR also removes unused code for premium list metrics that have never
actually been written out (and that we won't bother with at this point).

core/src/main/java/google/registry/model/tld/label/DomainLabelMetrics.java

@@ -50,8 +50,7 @@ class DomainLabelMetrics {
   }
 
   /**
-   * Labels attached to {@link #reservedListChecks} and {@link #reservedListProcessingTime}
-   * metrics.
+   * Labels attached to {@link #reservedListChecks} metrics.
    *
    * <p>A domain name can be matched by multiple reserved lists. To keep the metrics useful by
    * emitting only one metric result for each check, while avoiding potential combinatorial
@@ -67,6 +66,13 @@ class DomainLabelMetrics {
           LabelDescriptor.create("most_severe_reserved_list", "Reserved list name, if any."),
           LabelDescriptor.create("most_severe_reservation_type", "Type of reservation found."));
 
+  /** Labels attached to {@link #reservedListProcessingTime} metrics. */
+  private static final ImmutableSet<LabelDescriptor>
+      RESERVED_LIST_PROCESSING_TIME_LABEL_DESCRIPTORS =
+          ImmutableSet.of(
+              LabelDescriptor.create("tld", "TLD"),
+              LabelDescriptor.create("reserved_list_count", "Number of matching reserved lists."));
+
   /** Labels attached to {@link #reservedListHits} metric. */
   private static final ImmutableSet<LabelDescriptor> RESERVED_LIST_HIT_LABEL_DESCRIPTORS =
       ImmutableSet.of(
@@ -74,15 +80,6 @@ class DomainLabelMetrics {
           LabelDescriptor.create("reserved_list", "Reserved list name."),
           LabelDescriptor.create("reservation_type", "Type of reservation found."));
 
-  /**
-   * Labels attached to {@link #premiumListChecks} and {@link #premiumListProcessingTime} metrics.
-   */
-  private static final ImmutableSet<LabelDescriptor> PREMIUM_LIST_LABEL_DESCRIPTORS =
-      ImmutableSet.of(
-          LabelDescriptor.create("tld", "TLD"),
-          LabelDescriptor.create("premium_list", "Premium list name."),
-          LabelDescriptor.create("outcome", "Outcome of the premium list check."));
-
   /** Metric counting the number of times a label was checked against all reserved lists. */
   @VisibleForTesting
   static final IncrementableMetric reservedListChecks =
@@ -101,7 +98,7 @@ class DomainLabelMetrics {
               "/domain_label/reserved/processing_time",
               "Reserved list check processing time",
               "milliseconds",
-              RESERVED_LIST_LABEL_DESCRIPTORS,
+              RESERVED_LIST_PROCESSING_TIME_LABEL_DESCRIPTORS,
               EventMetric.DEFAULT_FITTER);
 
   /**
@@ -123,28 +120,6 @@ class DomainLabelMetrics {
               "count",
               RESERVED_LIST_HIT_LABEL_DESCRIPTORS);
 
-
-  /** Metric recording the result of each premium list check. */
-  @VisibleForTesting
-  static final IncrementableMetric premiumListChecks =
-      MetricRegistryImpl.getDefault()
-          .newIncrementableMetric(
-              "/domain_label/premium/checks",
-              "Count of premium list checks",
-              "count",
-              PREMIUM_LIST_LABEL_DESCRIPTORS);
-
-  /** Metric recording the time required to process each premium list check. */
-  @VisibleForTesting
-  static final EventMetric premiumListProcessingTime =
-      MetricRegistryImpl.getDefault()
-          .newEventMetric(
-              "/domain_label/premium/processing_time",
-              "Premium list check processing time",
-              "milliseconds",
-              PREMIUM_LIST_LABEL_DESCRIPTORS,
-              EventMetric.DEFAULT_FITTER);
-
   /** Update all three reserved list metrics. */
   static void recordReservedListCheckOutcome(
       String tld, ImmutableSet<MetricsReservedListMatch> matches, double elapsedMillis) {
@@ -163,14 +138,6 @@ class DomainLabelMetrics {
         (matches.isEmpty() ? "(none)" : mostSevereMatch.reservationType()).toString();
     reservedListChecks.increment(
         tld, matchCount, mostSevereReservedList, mostSevereReservationType);
-    reservedListProcessingTime.record(
-        elapsedMillis, tld, matchCount, mostSevereReservedList, mostSevereReservationType);
-  }
-
-  /** Update both premium list metrics. */
-  static void recordPremiumListCheckOutcome(
-      String tld, String premiumList, PremiumListCheckOutcome outcome, double elapsedMillis) {
-    premiumListChecks.increment(tld, premiumList, outcome.name());
-    premiumListProcessingTime.record(elapsedMillis, tld, premiumList, outcome.name());
+    reservedListProcessingTime.record(elapsedMillis, tld, matchCount);
   }
 }

core/src/test/java/google/registry/model/tld/label/ReservedListTest.java

@@ -65,7 +65,7 @@ class ReservedListTest {
         .and()
         .hasNoOtherValues();
     assertThat(reservedListProcessingTime)
-        .hasAnyValueForLabels("tld", "0", "(none)", "(none)")
+        .hasAnyValueForLabels("tld", "0")
         .and()
         .hasNoOtherValues();
     assertThat(reservedListHits).hasNoOtherValues();
@@ -130,11 +130,11 @@ class ReservedListTest {
         .and()
         .hasNoOtherValues();
     assertThat(reservedListProcessingTime)
-        .hasAnyValueForLabels("tld", "0", "(none)", "(none)")
+        .hasAnyValueForLabels("tld", "0")
         .and()
-        .hasAnyValueForLabels("tld", "1", "reserved1", FULLY_BLOCKED.toString())
+        .hasAnyValueForLabels("tld", "1")
         .and()
-        .hasAnyValueForLabels("tld", "1", "reserved2", FULLY_BLOCKED.toString())
+        .hasAnyValueForLabels("tld", "1")
         .and()
         .hasNoOtherValues();
     assertThat(reservedListHits)
@@ -182,9 +182,9 @@ class ReservedListTest {
         .and()
         .hasNoOtherValues();
     assertThat(reservedListProcessingTime)
-        .hasAnyValueForLabels("tld", "1", "reserved2", FULLY_BLOCKED.toString())
+        .hasAnyValueForLabels("tld", "1")
         .and()
-        .hasAnyValueForLabels("tld", "0", "(none)", "(none)")
+        .hasAnyValueForLabels("tld", "0")
         .and()
         .hasNoOtherValues();
     assertThat(reservedListHits)
@@ -209,9 +209,9 @@ class ReservedListTest {
         .and()
         .hasNoOtherValues();
     assertThat(reservedListProcessingTime)
-        .hasAnyValueForLabels("tld", "1", "reserved1", ALLOWED_IN_SUNRISE.toString())
+        .hasAnyValueForLabels("tld", "1")
         .and()
-        .hasAnyValueForLabels("tld", "2", "reserved2", FULLY_BLOCKED.toString())
+        .hasAnyValueForLabels("tld", "2")
         .and()
         .hasNoOtherValues();
     assertThat(reservedListHits)

docs/operational-procedures.md

@@ -20,7 +20,7 @@ metrics monitored are as follows:
     name, client id, and return status code.
 *   `/custom/epp/processing_time` -- A [Distribution][distribution] representing
     the processing time for EPP requests, described by command name, client id,
-    and retujrn status code.
+    and return status code.
 *   `/custom/whois/requests` -- A count of WHOIS requests, described by command
     name, number of returned results, and return status code.
 *   `/custom/whois/processing_time` -- A [Distribution][distribution]