finagle-core: Don't migrate all metrics yet

Problem

We are not sure we want to migrate users metrics to dimensional form
yet, but we do want to start working on finagles metrics.

Solution

Make metrics hierarchical only by default but turn all finagle metrics
into dimensional metrics as well via in a special proxy StatsReceiver.

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

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

@@ -97,7 +97,7 @@ object MetricBuilder {
       sourceClass,
       // For now we're not allowing construction w"ith labels.
       // They need to be added later via `.withLabels`.
-      Identity(name, name, Map.empty, false),
+      Identity(name, name),
       relativeName,
       processPath,
       percentiles,
@@ -160,8 +160,8 @@ object MetricBuilder {
   final case class Identity(
     hierarchicalName: Seq[String],
     dimensionalName: Seq[String],
-    labels: Map[String, String],
-    hierarchicalOnly: Boolean = false) {
+    labels: Map[String, String] = Map.empty,
+    hierarchicalOnly: Boolean = true) {
     override def toString: String = {
       val hname = hierarchicalName.mkString(metadataScopeSeparator())
       val dname = dimensionalName.mkString(DimensionalNameScopeSeparator)
@@ -276,7 +276,7 @@ class MetricBuilder private (
     copy(identity = identity.copy(hierarchicalName = name, dimensionalName = name))
   }
 
-  private[finagle] def withIdentity(identity: Identity): MetricBuilder = copy(identity = identity)
+  private[twitter] def withIdentity(identity: Identity): MetricBuilder = copy(identity = identity)
 
   /**
    * The hierarchical name of the metric

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

@@ -6,7 +6,6 @@ package com.twitter.finagle.stats
  *
  * @param self The underlying StatsReceiver to which translated names are passed
  * @param namespacePrefix the namespace used for translations
- * @param mode determines which names to translate: hierarchical, dimensional, or both.
  */
 abstract class NameTranslatingStatsReceiver(
   self: StatsReceiver,
@@ -17,7 +16,7 @@ abstract class NameTranslatingStatsReceiver(
   import NameTranslatingStatsReceiver._
 
   def this(self: StatsReceiver, namespacePrefix: String) =
-    this(self, namespacePrefix, NameTranslatingStatsReceiver.FullTranslation)
+    this(self, namespacePrefix, NameTranslatingStatsReceiver.HierarchicalOnly)
 
   def this(self: StatsReceiver) = this(self, "<namespacePrefix>")
 
@@ -40,13 +39,15 @@ abstract class NameTranslatingStatsReceiver(
     builder.withIdentity(nextIdentity)
   }
 
-  override def toString: String = mode match {
-    case FullTranslation | HierarchicalOnly => s"$self/$namespacePrefix"
-    case DimensionalOnly => self.toString
+  override def toString: String = {
+    mode match {
+      case FullTranslation | HierarchicalOnly => s"$self/$namespacePrefix"
+      case DimensionalOnly => self.toString
+    }
   }
 }
 
-private object NameTranslatingStatsReceiver {
+object NameTranslatingStatsReceiver {
 
   /** The mode with which to translate stats, either hierarchical, dimensional, or both. */
   sealed trait Mode

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

@@ -382,18 +382,17 @@ trait StatsReceiver {
    */
   def scope(namespace: String): StatsReceiver = {
     if (namespace == "") this
-    else
-      new ScopeTranslatingStatsReceiver(
-        this,
-        namespace,
-        NameTranslatingStatsReceiver.FullTranslation)
+    else new ScopeTranslatingStatsReceiver(this, namespace, scopeTranslation)
   }
 
+  private[finagle] def scopeTranslation: NameTranslatingStatsReceiver.Mode =
+    NameTranslatingStatsReceiver.HierarchicalOnly
+
   /**
    * Create a new `StatsReceiver` that will add a scope that is only used when the metric is
    * emitted in hierarchical form.
    */
-  private[finagle] def hierarchicalScope(namespace: String): StatsReceiver = {
+  private[finagle] final def hierarchicalScope(namespace: String): StatsReceiver = {
     if (namespace == "") this
     else
       new ScopeTranslatingStatsReceiver(
@@ -406,7 +405,7 @@ trait StatsReceiver {
    * Create a new `StatsReceiver` that will add a scope that is only used when the metric is
    * emitted in dimensional form.
    */
-  private[finagle] def dimensionalScope(namespace: String): StatsReceiver = {
+  private[finagle] final def dimensionalScope(namespace: String): StatsReceiver = {
     if (namespace == "") this
     else
       new ScopeTranslatingStatsReceiver(
@@ -418,7 +417,7 @@ trait StatsReceiver {
   /**
    * Create a new `StatsReceiver` that will add the specified label to all created metrics.
    */
-  private[finagle] def label(labelName: String, labelValue: String): StatsReceiver = {
+  private[finagle] final def label(labelName: String, labelValue: String): StatsReceiver = {
     require(labelName.nonEmpty)
     if (labelValue == "") this
     else new LabelTranslatingStatsReceiver(this, labelName, labelValue)

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

@@ -16,6 +16,9 @@ trait StatsReceiverProxy extends StatsReceiver with DelegatingStatsReceiver {
   def stat(metricBuilder: MetricBuilder): Stat = self.stat(metricBuilder)
   def addGauge(metricBuilder: MetricBuilder)(f: => Float): Gauge = self.addGauge(metricBuilder)(f)
 
+  override private[finagle] def scopeTranslation: NameTranslatingStatsReceiver.Mode =
+    self.scopeTranslation
+
   override def registerExpression(
     expressionSchema: ExpressionSchema
   ): Try[Unit] = self.registerExpression(expressionSchema)

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

@@ -362,13 +362,13 @@ class InMemoryStatsReceiverTest extends AnyFunSuite with Eventually with Integra
       assert(parts.length == 3)
       assert(
         parts(
-          0) == "coolGauge MetricBuilder(false, No description provided, Unspecified, NoRoleSpecified, Verbosity(default), None, Identity(coolGauge, coolGauge, Map(), false), List(), None, Vector(), GaugeType)")
+          0) == "coolGauge MetricBuilder(false, No description provided, Unspecified, NoRoleSpecified, Verbosity(default), None, Identity(coolGauge, coolGauge, Map(), true), List(), None, Vector(), GaugeType)")
       assert(
         parts(
-          1) == "rad/histo MetricBuilder(false, No description provided, Unspecified, NoRoleSpecified, Verbosity(default), None, Identity(rad/histo, rad_histo, Map(), false), List(), None, Vector(), HistogramType)")
+          1) == "rad/histo MetricBuilder(false, No description provided, Unspecified, NoRoleSpecified, Verbosity(default), None, Identity(rad/histo, histo, Map(), true), List(), None, Vector(), HistogramType)")
       assert(
         parts(
-          2) == "sweet/counter MetricBuilder(false, No description provided, Unspecified, NoRoleSpecified, Verbosity(default), None, Identity(sweet/counter, sweet_counter, Map(), false), List(), None, Vector(), CounterType)")
+          2) == "sweet/counter MetricBuilder(false, No description provided, Unspecified, NoRoleSpecified, Verbosity(default), None, Identity(sweet/counter, sweet_counter, Map(), true), List(), None, Vector(), CounterType)")
     } finally {
       ps.close()
     }
@@ -390,11 +390,28 @@ class InMemoryStatsReceiverTest extends AnyFunSuite with Eventually with Integra
 
     // what we expected as hydrated metric builders
     val aaSchema =
-      MetricBuilder(name = Seq("test", "a"), metricType = CounterType)
+      MetricBuilder(metricType = CounterType)
+        .withIdentity(
+          MetricBuilder.Identity(
+            hierarchicalName = Seq("test", "a"),
+            dimensionalName = Seq("a"),
+            labels = Map.empty))
+
     val bbSchema =
-      MetricBuilder(name = Seq("test", "b"), metricType = HistogramType)
+      MetricBuilder(metricType = HistogramType)
+        .withIdentity(
+          MetricBuilder.Identity(
+            hierarchicalName = Seq("test", "b"),
+            dimensionalName = Seq("b"),
+            labels = Map.empty))
+
     val ccSchema =
-      MetricBuilder(name = Seq("test", "c"), metricType = GaugeType)
+      MetricBuilder(metricType = GaugeType)
+        .withIdentity(
+          MetricBuilder.Identity(
+            hierarchicalName = Seq("test", "c"),
+            dimensionalName = Seq("c"),
+            labels = Map.empty))
 
     val expected_expression = ExpressionSchema(
       "test_expression",

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

@@ -112,16 +112,16 @@ class StatsReceiverTest extends AnyFunSuite {
     assert(sr.schemas(Seq("bar")).identity.labels.isEmpty)
   }
 
-  test("StatsReceiver.scope: adds a scope to both names and doesn't poison dimensional metrics") {
+  test("StatsReceiver.scope: by default has the same behavior has hierarchicalScope") {
     val sr = new InMemoryStatsReceiver
     val scopedCounter = sr.scope("cool").counter("numbers")
 
     val mb = scopedCounter.metadata.toMetricBuilder.get
 
     assert(mb.identity.labels.isEmpty)
     assert(mb.identity.hierarchicalName == Seq("cool", "numbers"))
-    assert(mb.identity.dimensionalName == Seq("cool", "numbers"))
-    assert(!mb.identity.hierarchicalOnly)
+    assert(mb.identity.dimensionalName == Seq("numbers"))
+    assert(mb.identity.hierarchicalOnly)
   }
 
   test("StatsReceiver.hierarchicalScope: composes with label correctly") {
@@ -137,7 +137,7 @@ class StatsReceiverTest extends AnyFunSuite {
     assert(mb.identity.hierarchicalName == Seq("clnt", "backend", "requests"))
     assert(mb.identity.dimensionalName == Seq("requests"))
     assert(mb.identity.labels == Map("clnt" -> "backend"))
-    assert(!mb.identity.hierarchicalOnly)
+    assert(mb.identity.hierarchicalOnly)
   }
 
   test("StatsReceiver.dimensionalScope: composes with label correctly") {
@@ -154,7 +154,7 @@ class StatsReceiverTest extends AnyFunSuite {
     assert(mb.identity.hierarchicalName == Seq("bar", "requests"))
     assert(mb.identity.dimensionalName == Seq("foo", "baz", "requests"))
     assert(mb.identity.labels == Map("clnt" -> "backend"))
-    assert(!mb.identity.hierarchicalOnly)
+    assert(mb.identity.hierarchicalOnly)
   }
 
   test("StatsReceiver.scope: prefix stats by a scope string") {