thrift: Add Thrift service class to registry

Problem

We'd like to add the Thrift interface being implemented
to the server's registry. This will appear under the
key: `library/finatra/thrift/service_class`. Additionally,
we want to set the Finagle `ServiceClass` Stack param in
the case where it isn't automatically set by calling
`server.serveIface`.

Solution

Update the routers to register the `service_class` entry
into the global registry and update where we serve with
`server.serve` to set the `ServiceClass` Stack param to
opt these services into any available Finagle instrumentation.

Result

Ability to inspect the class name of the Thrift service
the server is implementing.

JIRA Issues: CSL-10989

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

CHANGELOG.rst

@@ -10,6 +10,8 @@ Unreleased
 Changed
 ~~~~~~~
 
+* thrift: Add `service_class` to Finatra library thrift registry entry. ``PHAB_ID=D687117``
+
 * finatra (BREAKING API CHANGE): Update to use the new util/util-jackson `ScalaObjectMapper` for
   case class object mapping. We've removed the custom Finatra `c.t.finatra.jackson.ScalaObjectMapper`
   and instead now use the `c.t.util.jackson.ScalaObjectMapper`. Since the `c.t.util.jackson.ScalaObjectMapper`

thrift/src/main/scala/com/twitter/finatra/thrift/routing/routers.scala

@@ -27,12 +27,20 @@ private[routing] abstract class BaseThriftRouter[Router <: BaseThriftRouter[Rout
   exceptionManager: ExceptionManager)
     extends Logging { this: Router =>
 
-  def isConfigured: Boolean = configurationComplete
-
   // There is no guarantee that this is always accessed from the same thread
   @volatile
   private[this] var configurationComplete: Boolean = false
 
+  // There is no guarantee that this is always accessed from the same thread
+  @volatile
+  protected[this] var serviceClazzStackParam: Option[Class[_]] = None
+
+  /** Returns true is this router is configured */
+  def isConfigured: Boolean = configurationComplete
+
+  /** Returns the currently set `ServiceClassStackParam` value. */
+  def getServiceClazzStackParam: Option[Class[_]] = serviceClazzStackParam
+
   /**
    * Add exception mapper used for the corresponding exceptions.
    *
@@ -109,15 +117,28 @@ private[routing] abstract class BaseThriftRouter[Router <: BaseThriftRouter[Rout
   }
 
   protected[this] def registerGlobalFilter(
-    thriftFilter: Object,
-    registry: LibraryRegistry
+    registry: LibraryRegistry,
+    thriftFilter: Object
   ): Unit = {
     if (thriftFilter ne Filter.TypeAgnostic.Identity) {
       registry
         .withSection("thrift")
         .put("filters", thriftFilter.toString)
     }
   }
+
+  protected[this] def registerService(
+    registry: LibraryRegistry,
+    serviceName: String
+  ): Unit = {
+    val name =
+      if (serviceName.endsWith("$")) {
+        serviceName.substring(0, serviceName.length - 1)
+      } else serviceName
+    registry
+      .withSection("thrift")
+      .put("service_class", name)
+  }
 }
 
 private object ThriftRouter {
@@ -248,11 +269,19 @@ class ThriftRouter @Inject() (
         .instance[LibraryRegistry]
         .withSection("thrift", "methods")
 
-      registerGlobalFilter(filterStack, reg)
-
-      underlying = controller.config match {
-        case c: Controller.ControllerConfig => addController(controller, c)
-        case c: Controller.LegacyConfig => addLegacyController(controller, c)
+      registerGlobalFilter(reg, filterStack)
+
+      this.underlying = controller.config match {
+        case c: Controller.ControllerConfig =>
+          val controllerClazz = c.gen.getClass
+          this.serviceClazzStackParam = Some(controllerClazz)
+          registerService(reg, controllerClazz.getName)
+          addController(controller, c)
+        case c: Controller.LegacyConfig =>
+          val controllerClazz = controller.getClass.getInterfaces.head
+          this.serviceClazzStackParam = Some(controllerClazz)
+          registerService(reg, controllerClazz.getName)
+          addLegacyController(controller, c)
       }
     }
   }
@@ -478,7 +507,9 @@ class JavaThriftRouter @Inject() (injector: Injector, exceptionManager: Exceptio
               declaredMethods.map(method => s"$serviceName.${method.getName}").mkString("\n")
           )
 
-          registerGlobalFilter(filters, libraryRegistry)
+          registerGlobalFilter(libraryRegistry, filters)
+          serviceClazzStackParam = Some(serviceClazz)
+          registerService(libraryRegistry, serviceClazz.getName)
           registerMethods(serviceName, controllerClazz, declaredMethods.toSeq)
 
           serviceInstance

thrift/src/main/scala/com/twitter/finatra/thrift/servers.scala

@@ -10,7 +10,6 @@ import com.twitter.finatra.thrift.modules.{ExceptionManagerModule, ThriftRespons
 import com.twitter.finatra.thrift.response.ThriftResponseClassifier
 import com.twitter.finatra.thrift.routing.{JavaThriftRouter, ThriftRouter}
 import com.twitter.inject.annotations.Lifecycle
-import com.twitter.inject.internal.LibraryRegistry
 import com.twitter.inject.modules.StackTransformerModule
 import com.twitter.inject.server.{AbstractTwitterServer, PortUtils, TwitterServer}
 import com.twitter.util.{Await, Duration}
@@ -317,8 +316,14 @@ abstract class AbstractThriftServer extends AbstractTwitterServer with ThriftSer
     server: ThriftMux.Server
   ): ListeningServer = {
     val router = injector.instance[JavaThriftRouter]
-    val service = registerService(configureService(router.createService(server.serverParam)))
-    server.serve(addr, service)
+    router.getServiceClazzStackParam match {
+      case Some(serviceClazz) =>
+        server
+          .withServiceClass(serviceClazz)
+          .serve(addr, configureService(router.createService(server.serverParam)))
+      case _ =>
+        server.serve(addr, configureService(router.createService(server.serverParam)))
+    }
   }
 
   /** Users are expected to use [[configureThrift(router: JavaThriftRouter)]] */
@@ -364,23 +369,4 @@ abstract class AbstractThriftServer extends AbstractTwitterServer with ThriftSer
   override private[thrift] final def configureRouter(): Unit = {
     configureThrift(injector.instance[JavaThriftRouter])
   }
-
-  /* Private */
-
-  /**
-   * The service may be filtered and thus we want to fully capture in the registry the
-   * resultant service.
-   *
-   * @note we register the service given here because it may not have been configured
-   *       by a router (which registers methods and applied filters).
-   */
-  private[this] final def registerService[Req, Rep](
-    service: Service[Req, Rep]
-  ): Service[Req, Rep] = {
-    injector
-      .instance[LibraryRegistry]
-      .withSection("thrift")
-      .put("service", service.toString)
-    service
-  }
 }

thrift/src/test/scala/com/twitter/finatra/thrift/tests/DoEverythingThriftServerFeatureTest.scala

@@ -309,6 +309,7 @@ class DoEverythingThriftServerFeatureTest extends FeatureTest {
     val thrift = finatra("thrift").asInstanceOf[Map[String, Any]]
     thrift.contains("filters") should be(true)
     thrift.contains("methods") should be(true)
+    thrift.contains("service_class") should be(true)
 
     val methods = thrift("methods").asInstanceOf[Map[String, Any]]
     methods.size should be > 0