finagle-core: Move SourceRole subtypes into its object

Problem

We have about a million things named `Client` or `Server` and that now
includes the subtypes of `SourceRole`. This can make things really
difficult to navigate.

Solution

Move the subtypes to the object of `SourceRole` to make the relationship
more clear.

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

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: SourceRole subtypes have been moved into the SourceRole object to make their
+  relationship to the parent type more clear. ``PHAB_ID=D909744``
+
 Runtime Behavior Changes
 ~~~~~~~~~~~~~~~~~~~~~~~~
 

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

@@ -16,16 +16,28 @@ import scala.annotation.varargs
  * request to another service). In many cases, there is no relevant role for a metric, in which
  * case NoRole should be used.
  */
-sealed trait SourceRole {
+sealed abstract class SourceRole private () {
 
   /**
    * Java-friendly helper for accessing the object itself.
    */
   def getInstance(): SourceRole = this
 }
-case object NoRoleSpecified extends SourceRole
-case object Client extends SourceRole
-case object Server extends SourceRole
+
+object SourceRole {
+  case object NoRoleSpecified extends SourceRole
+  case object Client extends SourceRole
+  case object Server extends SourceRole
+
+  /**
+   * Java-friendly accessors
+   */
+  def noRoleSpecified(): SourceRole = NoRoleSpecified
+
+  def client(): SourceRole = Client
+
+  def server(): SourceRole = Server
+}
 
 /**
  * finagle-stats has configurable scope separators. As this package is wrapped by finagle-stats, we
@@ -66,7 +78,7 @@ object MetricBuilder {
     keyIndicator: Boolean = false,
     description: String = "No description provided",
     units: MetricUnit = Unspecified,
-    role: SourceRole = NoRoleSpecified,
+    role: SourceRole = SourceRole.NoRoleSpecified,
     verbosity: Verbosity = Verbosity.Default,
     sourceClass: Option[String] = None,
     name: Seq[String] = Seq.empty,
@@ -102,7 +114,6 @@ object MetricBuilder {
    * than being forced to use the mega apply method.
    *
    * @param metricType the type of the metric being constructed.
-   * @param statsReceiver used for the actual metric creation, set by the StatsReceiver when creating a MetricBuilder.
    */
   def newBuilder(metricType: MetricType): MetricBuilder =
     apply(metricType = metricType)

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

@@ -8,7 +8,7 @@
 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.SourceRole;
 import com.twitter.finagle.stats.StatsReceiver;
 import com.twitter.finagle.stats.Verbosity;
 
@@ -37,7 +37,7 @@ public void testWithMethods() {
       .withSourceClass(new Some<>("com.twitter.finagle.AwesomeClass"))
       .withIdentifier(new Some<>("/p/foo/bar"))
       .withUnits(Microseconds.getInstance())
-      .withRole(NoRoleSpecified.getInstance())
+      .withRole(SourceRole.noRoleSpecified())
       .withName("my", "very", "cool", "name")
       .withRelativeName("cool", "name")
       .withPercentiles(0.99, 0.88, 0.77);

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

@@ -5,30 +5,30 @@ import org.scalatest.funsuite.AnyFunSuite
 class RoleConfiguredStatsReceiverTest extends AnyFunSuite {
   test("RoleConfiguredSR configures metadata role correctly") {
     val mem = new InMemoryStatsReceiver
-    val clientSR = new RoleConfiguredStatsReceiver(mem, Client)
-    val serverSR = new RoleConfiguredStatsReceiver(mem, Server)
+    val clientSR = new RoleConfiguredStatsReceiver(mem, SourceRole.Client)
+    val serverSR = new RoleConfiguredStatsReceiver(mem, SourceRole.Server)
 
     clientSR.counter("foo")
-    assert(mem.schemas.get(Seq("foo")).get.role == Client)
+    assert(mem.schemas.get(Seq("foo")).get.role == SourceRole.Client)
     clientSR.addGauge("bar")(3)
-    assert(mem.schemas.get(Seq("bar")).get.role == Client)
+    assert(mem.schemas.get(Seq("bar")).get.role == SourceRole.Client)
     clientSR.stat("baz")
-    assert(mem.schemas.get(Seq("baz")).get.role == Client)
+    assert(mem.schemas.get(Seq("baz")).get.role == SourceRole.Client)
 
     serverSR.counter("fee")
-    assert(mem.schemas.get(Seq("fee")).get.role == Server)
+    assert(mem.schemas.get(Seq("fee")).get.role == SourceRole.Server)
     serverSR.addGauge("fi")(3)
-    assert(mem.schemas.get(Seq("fi")).get.role == Server)
+    assert(mem.schemas.get(Seq("fi")).get.role == SourceRole.Server)
     serverSR.stat("foe")
-    assert(mem.schemas.get(Seq("foe")).get.role == Server)
+    assert(mem.schemas.get(Seq("foe")).get.role == SourceRole.Server)
   }
 
   test("RoleConfiguredSR equality") {
     val mem = new InMemoryStatsReceiver
-    val clientA = RoleConfiguredStatsReceiver(mem, Client)
-    val clientB = RoleConfiguredStatsReceiver(mem, Client)
-    val serverA = RoleConfiguredStatsReceiver(mem, Server)
-    val serverB = RoleConfiguredStatsReceiver(mem, Server)
+    val clientA = RoleConfiguredStatsReceiver(mem, SourceRole.Client)
+    val clientB = RoleConfiguredStatsReceiver(mem, SourceRole.Client)
+    val serverA = RoleConfiguredStatsReceiver(mem, SourceRole.Server)
+    val serverB = RoleConfiguredStatsReceiver(mem, SourceRole.Server)
 
     assert(clientA == clientB)
     assert(clientA != serverA)

util-stats/src/test/scala/com/twitter/finagle/stats/exp/ExpressionSchemaTest.scala

@@ -14,7 +14,7 @@ import org.scalatest.funsuite.AnyFunSuite
 class ExpressionSchemaTest extends AnyFunSuite {
   trait Ctx {
     val sr = new InMemoryStatsReceiver
-    val clientSR = RoleConfiguredStatsReceiver(sr, Client, Some("downstream"))
+    val clientSR = RoleConfiguredStatsReceiver(sr, SourceRole.Client, Some("downstream"))
 
     val successMb =
       MetricBuilder(name = Seq("success"), metricType = CounterType)
@@ -32,7 +32,9 @@ class ExpressionSchemaTest extends AnyFunSuite {
     }
 
     val downstreamLabel =
-      Map(ExpressionSchema.Role -> Client.toString, ExpressionSchema.ServiceName -> "downstream")
+      Map(
+        ExpressionSchema.Role -> SourceRole.Client.toString,
+        ExpressionSchema.ServiceName -> "downstream")
 
     protected[this] def nameToKey(
       name: String,
@@ -88,7 +90,8 @@ class ExpressionSchemaTest extends AnyFunSuite {
       assert(sr.stats(Seq("latency")) == Seq(50, 50))
 
       assert(
-        sr.expressions(nameToKey("success_rate")).labels(ExpressionSchema.Role) == Client.toString)
+        sr.expressions(nameToKey("success_rate")).labels(
+            ExpressionSchema.Role) == SourceRole.Client.toString)
     }
   }
 
@@ -179,7 +182,7 @@ class ExpressionSchemaTest extends AnyFunSuite {
           Expression(Seq("clnt", "aclient", "success"), isCounter = true).divide(
             Expression(Seq("clnt", "aclient", "success"), isCounter = true).plus(
               Expression(Seq("clnt", "aclient", "failures"), isCounter = true))))
-      ).withRole(Client).withServiceName("aclient"))
+      ).withRole(SourceRole.Client).withServiceName("aclient"))
 
     assert(sr.expressions.values.size == 1)
     assert(
@@ -203,7 +206,7 @@ class ExpressionSchemaTest extends AnyFunSuite {
             .counter(sr, Seq("clnt", "aclient", "success")).get.divide(Expression
               .counter(sr, Seq("clnt", "aclient", "success")).get.plus(
                 Expression.counter(sr, Seq("clnt", "aclient", "failures"), showRollup = true).get)))
-      ).withRole(Client).withServiceName("aclient"))
+      ).withRole(SourceRole.Client).withServiceName("aclient"))
 
     assert(sr.expressions.values.size == 1)
     assert(
@@ -222,7 +225,8 @@ class ExpressionSchemaTest extends AnyFunSuite {
       Expression.stats(sr, Seq("clnt", "aclient", "latencies"))
     expressions.map { expression =>
       sr.registerExpression(
-        ExpressionSchema("latencies", expression).withRole(Client).withServiceName("aclient"))
+        ExpressionSchema("latencies", expression)
+          .withRole(SourceRole.Client).withServiceName("aclient"))
     }
 
     assert(sr.expressions.values.size == expressions.size)