Skip to content

Commit

Permalink
finagle-core: Add descriptions to stack modules
Browse files Browse the repository at this point in the history
Problem:
These stack modules do not have descriptions because the only have a role name:
RequestDraining, PrepFactory, PrepConn, protoTracing, Preparer, Failure. In
TwitterServer admin UI page, it is hard for users to know what these stack
modules do.

Solution:
Wrap the module role in a stackable service factory modules so that the
description is attached.

JIRA Issues: CSL-11015

Differential Revision: https://phabricator.twitter.biz/D685887
  • Loading branch information
tigerlily-he authored and jenkins committed Jun 17, 2021
1 parent bd69498 commit 1ea1a3e
Show file tree
Hide file tree
Showing 4 changed files with 95 additions and 15 deletions.
4 changes: 4 additions & 0 deletions CHANGELOG.rst
Expand Up @@ -17,6 +17,10 @@ New Features
defined in both, and both take precedence over the `Dtab.base`. The existing
`Dtab.local` request propagation behavior remains unchanged. ``PHAB_ID=D677860``

* finagle-core: Add descriptions to RequestDraining, PrepFactory, PrepConn, and
protoTracing modules in StackClient. Add descriptions to preparer and
protoTracing modules in StackServer. ``PHAB_ID=D685887``

Breaking API Changes
~~~~~~~~~~~~~~~~~~~~

Expand Down
Expand Up @@ -327,7 +327,9 @@ object Failure {
def module[Req, Rep]: Stackable[ServiceFactory[Req, Rep]] =
new Stack.Module0[ServiceFactory[Req, Rep]] {
val role: Stack.Role = Failure.role
val description: String = "process failures"
val description: String =
"""Converts failures into a format suitable for users by unwrapping inner failures or
|Throwables and stripping out dangerous flags""".stripMargin

private[this] val filter = new ProcessFailures[Req, Rep]

Expand Down
Expand Up @@ -18,6 +18,48 @@ import scala.collection.immutable.Queue

object StackClient {

object RequestDraining {
def module[Req, Rep]: Stackable[ServiceFactory[Req, Rep]] =
new Stack.Module0[ServiceFactory[Req, Rep]] {
val role: Stack.Role = Role.requestDraining
val description: String =
"Ensures that services wait until all outstanding requests complete before closure"
def make(next: ServiceFactory[Req, Rep]): ServiceFactory[Req, Rep] =
new RefcountedFactory(next)
}
}

object PrepFactory {
def module[Req, Rep]: Stackable[ServiceFactory[Req, Rep]] =
new Stack.Module0[ServiceFactory[Req, Rep]] {
val role: Stack.Role = Role.prepFactory
val description: String =
"""A pre-allocated module at the top of the stack (after name resolution).
| Injects codec-specific behavior during service acquisition""".stripMargin
def make(next: ServiceFactory[Req, Rep]): ServiceFactory[Req, Rep] = next
}
}

object PrepConn {
def module[Req, Rep]: Stackable[ServiceFactory[Req, Rep]] =
new Stack.Module0[ServiceFactory[Req, Rep]] {
val role: Stack.Role = Role.prepConn
val description: String =
"Pre-allocated module at the bottom of the stack that handles newly connected sessions"
def make(next: ServiceFactory[Req, Rep]): ServiceFactory[Req, Rep] = next
}
}

object ProtoTracing {
def module[Req, Rep]: Stackable[ServiceFactory[Req, Rep]] =
new Stack.Module0[ServiceFactory[Req, Rep]] {
val role: Stack.Role = Role.protoTracing
val description: String =
"Pre-allocated stack module for protocols to inject tracing"
def make(next: ServiceFactory[Req, Rep]): ServiceFactory[Req, Rep] = next
}
}

/**
* Canonical Roles for some Client-related Stack module. Other roles are defined
* within the companion objects of the respective modules.
Expand Down Expand Up @@ -56,14 +98,14 @@ object StackClient {
val postNameResolutionTimeout = Stack.Role("PostNameResolutionTimeout")

/**
* Defines a preallocated position at the "bottom" of the stack which is
* Defines a pre-allocated position at the "bottom" of the stack which is
* special in that it's the first role before the client sends the request to
* the underlying transport implementation.
*/
val prepConn = Stack.Role("PrepConn")

/**
* Defines a preallocated position in the stack for protocols to inject tracing.
* Defines a pre-allocated position in the stack for protocols to inject tracing.
*/
val protoTracing = Stack.Role("protoTracing")
}
Expand Down Expand Up @@ -124,7 +166,7 @@ object StackClient {
* finagle-thrift uses this role to install session upgrading logic from
* vanilla Thrift to Twitter Thrift.
*/
stk.push(Role.prepConn, identity[ServiceFactory[Req, Rep]](_))
stk.push(PrepConn.module)

/**
* `WriteTracingFilter` annotates traced requests. Annotations are timestamped
Expand Down Expand Up @@ -302,7 +344,7 @@ object StackClient {
* below `FactoryToService` so that it is called on each
* service acquisition.
*
* * `Role.requestDraining` ensures that a service is not closed
* * `RequestDraining` ensures that a service is not closed
* until all outstanding requests on it have completed. It must
* appear below `FactoryToService` so that services are not
* prematurely closed by `FactoryToService`. (However it is
Expand All @@ -316,7 +358,7 @@ object StackClient {
* `FactoryToService` so that it is called on each service
* acquisition.
*
* * `Role.prepFactory` is a hook used to inject codec-specific
* * `PrepFactory` is a hook used to inject codec-specific
* behavior that needs to run for each session before it's been acquired.
*
* * `FactoryToService` acquires a new endpoint service from the
Expand Down Expand Up @@ -344,9 +386,9 @@ object StackClient {
stk.push(ExportSslUsage.module)
stk.push(LoadBalancerFactory.module)
stk.push(StatsFactoryWrapper.module)
stk.push(Role.requestDraining, (fac: ServiceFactory[Req, Rep]) => new RefcountedFactory(fac))
stk.push(RequestDraining.module)
stk.push(TimeoutFactory.module(Role.postNameResolutionTimeout))
stk.push(Role.prepFactory, identity[ServiceFactory[Req, Rep]](_))
stk.push(PrepFactory.module)
stk.push(FactoryToService.module)
stk.push(Retries.moduleRequeueable)
stk.push(ClearContextValueFilter.module(context.Retries))
Expand Down Expand Up @@ -414,7 +456,7 @@ object StackClient {
* These modules set up tracing for the request span and miscellaneous
* actions before a request leaves the client stack:
*
* * `Role.protoTracing` is a hook for protocol-specific tracing
* * `ProtoTracing` is a hook for protocol-specific tracing
*
* * `Failure` processes request failures for external representation
*
Expand Down Expand Up @@ -443,7 +485,7 @@ object StackClient {
* request span encompasses all tracing in the course of a
* request.
*/
stk.push(Role.protoTracing, identity[ServiceFactory[Req, Rep]](_))
stk.push(ProtoTracing.module)
stk.push(Failure.module)
stk.push(ClientTracingFilter.module)
stk.push(ForwardAnnotation.module)
Expand Down
Expand Up @@ -31,12 +31,44 @@ object StackServer {
}
}

object ProtoTracing {
def module[Req, Rep]: Stackable[ServiceFactory[Req, Rep]] =
new Stack.Module0[ServiceFactory[Req, Rep]] {
val role: Stack.Role = Role.protoTracing
val description: String =
"Pre-allocated stack module for protocols to inject tracing"
def make(next: ServiceFactory[Req, Rep]): ServiceFactory[Req, Rep] = next
}
}

object Preparer {
def module[Req, Rep]: Stackable[ServiceFactory[Req, Rep]] =
new Stack.Module0[ServiceFactory[Req, Rep]] {
val role: Stack.Role = Role.preparer
val description: String =
"Prepares the server"
def make(next: ServiceFactory[Req, Rep]): ServiceFactory[Req, Rep] = next
}
}

/**
* Canonical Roles for each Server-related Stack modules.
*/
object Role extends Stack.Role("StackServer") {

/**
* Server-side JVM tracing
*/
val jvmTracing: Stack.Role = Stack.Role("JvmTracing")

/**
* Prepares the server for transport-level connection
*/
val preparer: Stack.Role = Stack.Role("preparer")

/**
* Defines a pre-allocated position in the stack for protocols to inject tracing.
*/
val protoTracing: Stack.Role = Stack.Role("protoTracing")
}

Expand Down Expand Up @@ -121,7 +153,7 @@ object StackServer {
stk.push(ExceptionSourceFilter.module)
stk.push(new JvmTracing)
stk.push(ServerStatsFilter.module)
stk.push(Role.protoTracing, identity[ServiceFactory[Req, Rep]](_))
stk.push(ProtoTracing.module)
// `WriteTracingFilter` annotates traced requests. Annotations are timestamped
// so this should be low in the stack to accurately delineate between wire time
// and handling time. Ideally this would live closer to the "wire" in the netty
Expand All @@ -130,7 +162,7 @@ object StackServer {
// allowing us to provide a complimentary annotation to the Client WR/WS as well
// as measure queueing within the server via ConcurrentRequestFilter.
stk.push(WireTracingFilter.serverModule)
stk.push(Role.preparer, identity[ServiceFactory[Req, Rep]](_))
stk.push(Preparer.module)

// forks the execution if the current scheduler supports forking
stk.push(ForkingSchedulerFilter.server)
Expand Down Expand Up @@ -241,9 +273,9 @@ trait StackServer[Req, Rep]

def withParams(ps: Stack.Params): StackServer[Req, Rep]

override def configured[P: Stack.Param](p: P): StackServer[Req, Rep]
def configured[P: Stack.Param](p: P): StackServer[Req, Rep]

override def configured[P](psp: (P, Stack.Param[P])): StackServer[Req, Rep]
def configured[P](psp: (P, Stack.Param[P])): StackServer[Req, Rep]

override def configuredParams(params: Stack.Params): StackServer[Req, Rep]
def configuredParams(params: Stack.Params): StackServer[Req, Rep]
}

0 comments on commit 1ea1a3e

Please sign in to comment.