util-stats: Remove metric instantiation methods from MetricBuilder

Problem

We have a StatsReceiver that lives on MetricBuilder. It doesn't need
to be there. It's currently used mostly for supporting expressions and
their convenient `.build()` method which hunts for a stats receiver in
the MetricSchema to register the expression with.

Solution

In order to unwind this we first need to remove the metric creation methods
from MetricBuilder. In the future we'll make expressions register themselves
with a global instance of a StatsReceiver or even a new interface that
feels more purpose driven.

Differential Revision: https://phabricator.twitter.biz/D859036

Author: Bryce Anderson

CHANGELOG.rst

@@ -7,6 +7,12 @@ Note that ``PHAB_ID=#`` and ``RB_ID=#`` correspond to associated messages in com
 Unreleased
 ----------
 
+Breaking API Changes
+~~~~~~~~~~~~~~~~~~~~
+
+* util-stats: The metric instantiation methods have been removed from `MetricBuilder`. Use the methods on
+  `StatsReceiver` to instantiate metrics instead. ``PHAB_ID=D859036``
+
 22.3.0
 ------
 

util-jvm/src/main/scala/com/twitter/jvm/JvmStats.scala

@@ -80,19 +80,22 @@ object JvmStats {
       case compilation =>
         val compilationStats = stats.scope("compilation")
         gauges.add(
-          compilationStats.metricBuilder(GaugeType).withCounterishGauge.gauge("time_msec") {
+          compilationStats.addGauge(
+            compilationStats.metricBuilder(GaugeType).withCounterishGauge.withName("time_msec")) {
             compilation.getTotalCompilationTime().toFloat
           })
     }
 
     val classes = ManagementFactory.getClassLoadingMXBean()
     val classLoadingStats = stats.scope("classes")
     gauges.add(
-      classLoadingStats.metricBuilder(GaugeType).withCounterishGauge.gauge("total_loaded") {
+      classLoadingStats.addGauge(
+        classLoadingStats.metricBuilder(GaugeType).withCounterishGauge.withName("total_loaded")) {
         classes.getTotalLoadedClassCount().toFloat
       })
     gauges.add(
-      classLoadingStats.metricBuilder(GaugeType).withCounterishGauge.gauge("total_unloaded") {
+      classLoadingStats.addGauge(
+        classLoadingStats.metricBuilder(GaugeType).withCounterishGauge.withName("total_unloaded")) {
         classes.getUnloadedClassCount().toFloat
       })
     gauges.add(
@@ -142,15 +145,20 @@ object JvmStats {
     }
 
     val spStats = stats.scope("safepoint")
-    gauges.add(spStats.metricBuilder(GaugeType).withCounterishGauge.gauge("sync_time_millis") {
-      jvm.safepoint.syncTimeMillis.toFloat
-    })
-    gauges.add(spStats.metricBuilder(GaugeType).withCounterishGauge.gauge("total_time_millis") {
-      jvm.safepoint.totalTimeMillis.toFloat
-    })
-    gauges.add(spStats.metricBuilder(GaugeType).withCounterishGauge.gauge("count") {
-      jvm.safepoint.count.toFloat
-    })
+    gauges.add(
+      spStats.addGauge(
+        spStats.metricBuilder(GaugeType).withCounterishGauge.withName("sync_time_millis")) {
+        jvm.safepoint.syncTimeMillis.toFloat
+      })
+    gauges.add(
+      spStats.addGauge(
+        spStats.metricBuilder(GaugeType).withCounterishGauge.withName("total_time_millis")) {
+        jvm.safepoint.totalTimeMillis.toFloat
+      })
+    gauges.add(
+      spStats.addGauge(spStats.metricBuilder(GaugeType).withCounterishGauge.withName("count")) {
+        jvm.safepoint.count.toFloat
+      })
 
     ManagementFactory.getPlatformMXBeans(classOf[BufferPoolMXBean]) match {
       case null =>
@@ -169,10 +177,12 @@ object JvmStats {
     gcPool.foreach { gc =>
       val name = gc.getName.regexSub("""[^\w]""".r) { m => "_" }
       val poolCycles =
-        gcStats.metricBuilder(GaugeType).withCounterishGauge.gauge(name, "cycles") {
+        gcStats.addGauge(
+          gcStats.metricBuilder(GaugeType).withCounterishGauge.withName(name, "cycles")) {
           gc.getCollectionCount.toFloat
         }
-      val poolMsec = gcStats.metricBuilder(GaugeType).withCounterishGauge.gauge(name, "msec") {
+      val poolMsec = gcStats.addGauge(
+        gcStats.metricBuilder(GaugeType).withCounterishGauge.withName(name, "msec")) {
         gc.getCollectionTime.toFloat
       }
 
@@ -195,12 +205,14 @@ object JvmStats {
     }
 
     // note, these could be -1 if the collector doesn't have support for it.
-    val cycles = gcStats.metricBuilder(GaugeType).withCounterishGauge.gauge("cycles") {
-      gcPool.map(_.getCollectionCount).filter(_ > 0).sum.toFloat
-    }
-    val msec = gcStats.metricBuilder(GaugeType).withCounterishGauge.gauge("msec") {
-      gcPool.map(_.getCollectionTime).filter(_ > 0).sum.toFloat
-    }
+    val cycles =
+      gcStats.addGauge(gcStats.metricBuilder(GaugeType).withCounterishGauge.withName("cycles")) {
+        gcPool.map(_.getCollectionCount).filter(_ > 0).sum.toFloat
+      }
+    val msec =
+      gcStats.addGauge(gcStats.metricBuilder(GaugeType).withCounterishGauge.withName("msec")) {
+        gcPool.map(_.getCollectionTime).filter(_ > 0).sum.toFloat
+      }
 
     gauges.add(cycles)
     gauges.add(msec)

util-stats/src/main/scala/com/twitter/finagle/stats/MetricBuilder.scala

@@ -6,7 +6,6 @@ import com.twitter.finagle.stats.MetricBuilder.GaugeType
 import com.twitter.finagle.stats.MetricBuilder.HistogramType
 import com.twitter.finagle.stats.MetricBuilder.MetricType
 import com.twitter.finagle.stats.MetricBuilder.UnlatchedCounter
-import java.util.function.Supplier
 import scala.annotation.varargs
 
 /**
@@ -303,50 +302,6 @@ class MetricBuilder private (
   private[MetricBuilder] final def counterishGaugeSchema: MetricBuilder =
     this.copy(metricType = CounterishGaugeType)
 
-  /**
-   * Produce a counter as described by the builder inside the underlying StatsReceiver.
-   * @return the counter created.
-   */
-  @deprecated("Use the StatsReceiver interface for instantiation.", "2022-03-28")
-  @varargs
-  final def counter(name: String*): Counter = {
-    this.statsReceiver.validateMetricType(this, CounterType)
-    this.statsReceiver.counter(this.withName(name: _*))
-  }
-
-  /**
-   * Produce a gauge as described by the builder inside the underlying StatsReceiver.
-   * @return the gauge created.
-   */
-  @deprecated("Use the StatsReceiver interface for instantiation.", "2022-03-28")
-  final def gauge(name: String*)(f: => Float): Gauge = {
-    this.statsReceiver.validateMetricType(this, GaugeType)
-    this.statsReceiver.addGauge(this.withName(name: _*))(f)
-  }
-
-  /**
-   * Produce a gauge as described by the builder inside the underlying StatsReceiver.
-   * This API is for Java compatibility
-   * @return the gauge created.
-   */
-  @deprecated("Use the StatsReceiver interface for instantiation.", "2022-03-28")
-  @varargs
-  final def gauge(f: Supplier[Float], name: String*): Gauge = {
-    this.statsReceiver.validateMetricType(this, GaugeType)
-    this.statsReceiver.addGauge(this.withName(name: _*))(f.get())
-  }
-
-  /**
-   * Produce a histogram as described by the builder inside the underlying StatsReceiver.
-   * @return the histogram created.
-   */
-  @deprecated("Use the StatsReceiver interface for instantiation.", "2022-03-28")
-  @varargs
-  final def histogram(name: String*): Stat = {
-    this.statsReceiver.validateMetricType(this, HistogramType)
-    this.statsReceiver.stat(this.withName(name: _*))
-  }
-
   def canEqual(other: Any): Boolean = other.isInstanceOf[MetricBuilder]
 
   override def equals(other: Any): Boolean = other match {

util-stats/src/test/java/com/twitter/compiletest/MetricBuilderCompilationTest.java

@@ -4,14 +4,11 @@
 
 import scala.Some;
 
-import com.twitter.finagle.stats.Counter;
-import com.twitter.finagle.stats.Gauge;
 import com.twitter.finagle.stats.InMemoryStatsReceiver;
 import com.twitter.finagle.stats.MetricBuilder;
 import com.twitter.finagle.stats.MetricTypes;
 import com.twitter.finagle.stats.Microseconds;
 import com.twitter.finagle.stats.NoRoleSpecified;
-import com.twitter.finagle.stats.Stat;
 import com.twitter.finagle.stats.StatsReceiver;
 import com.twitter.finagle.stats.Verbosity;
 
@@ -45,16 +42,4 @@ public void testWithMethods() {
       .withRelativeName("cool", "name")
       .withPercentiles(0.99, 0.88, 0.77);
   }
-
-  @Test
-  public void testConstructingMetrics() {
-    StatsReceiver sr = new InMemoryStatsReceiver();
-    MetricBuilder gmb = sr.metricBuilder(MetricTypes.GAUGE_TYPE);
-    MetricBuilder smb = sr.metricBuilder(MetricTypes.HISTOGRAM_TYPE);
-    MetricBuilder cmb = sr.metricBuilder(MetricTypes.COUNTER_TYPE);
-
-    Gauge g = gmb.gauge(() -> 3.0f, "my", "cool", "gauge");
-    Stat s = smb.histogram("my", "cool", "histogram");
-    Counter c = cmb.counter("my", "cool", "counter");
-  }
 }

util-stats/src/test/scala/com/twitter/finagle/stats/MetricBuilderTest.scala

@@ -1,8 +1,6 @@
 package com.twitter.finagle.stats
 
 import com.twitter.finagle.stats.MetricBuilder.CounterType
-import com.twitter.finagle.stats.MetricBuilder.CounterishGaugeType
-import com.twitter.finagle.stats.MetricBuilder.GaugeType
 import org.scalatest.funsuite.AnyFunSuite
 
 class MetricBuilderTest extends AnyFunSuite {
@@ -18,18 +16,4 @@ class MetricBuilderTest extends AnyFunSuite {
     metadataScopeSeparator.setSeparator("_aTerriblyLongStringForSomeReason_")
     assert(mb.toString.contains("foo_aTerriblyLongStringForSomeReason_bar"))
   }
-
-  test("the .gauge() api builds CounterishGauge vs Gauge where appropriate") {
-    val mb = MetricBuilder(
-      name = Seq("c_gauge"),
-      metricType = GaugeType,
-      statsReceiver = new InMemoryStatsReceiver
-    ).withCounterishGauge
-
-    assert(mb.metricType == CounterishGaugeType)
-
-    val g = mb.gauge("c_gauge") { 1.0.toFloat }
-
-    assert(g.metadata.toMetricBuilder.get.metricType == CounterishGaugeType)
-  }
 }