finagle-core: Add failures as an metric expression

Problem/Solution We would like to have failures as an expression, for client and server, to be auto rendered in the dashboards. JIRA Issues: CSL-11437 Differential Revision: https://phabricator.twitter.biz/D783013
twitter · Nov 16, 2021 · 4f7d874 · 4f7d874
1 parent 82677f8
commit 4f7d874
Show file tree

Hide file tree

Showing 2 changed files with 31 additions and 6 deletions.
diff --git a/server/src/main/scala/com/twitter/server/handler/MetricExpressionHandler.scala b/server/src/main/scala/com/twitter/server/handler/MetricExpressionHandler.scala
@@ -28,6 +28,7 @@ import com.twitter.util.Future
 object MetricExpressionHandler {
   private val Version = 1.1
   private val statsFormatter = StatsFormatter.default
+  private val Wildcard = "/*"
 
   /**
    * Translate the [[Expression]] object to a single line string which represents generic
@@ -43,7 +44,8 @@ object MetricExpressionHandler {
   ): String =
     expr match {
       case HistogramExpression(schema, _) => getHisto(schema, labels)
-      case MetricExpression(schema) => getMetric(schema, shouldRate, sourceLatched)
+      case MetricExpression(schema, showRollup) =>
+        getMetric(schema, showRollup, shouldRate, sourceLatched)
       case ConstantExpression(repr) => repr
       case FunctionExpression(funcName, exprs) =>
         s"$funcName(${exprs
@@ -66,15 +68,20 @@ object MetricExpressionHandler {
   // Form metrics other than histograms, rate() for unlatched counters
   private def getMetric(
     metricBuilder: MetricBuilder,
+    showRollUp: Boolean,
     shouldRate: Boolean,
     sourceLatched: Boolean
   ): String = {
+    val metric = metricBuilder.name.mkString(metadataScopeSeparator()) + {
+      if (showRollUp) Wildcard
+      else ""
+    }
     metricBuilder.metricType match {
       case CounterType if shouldRate && !sourceLatched =>
-        s"rate(${metricBuilder.name.mkString(metadataScopeSeparator())})"
+        s"rate($metric)"
       case CounterishGaugeType if shouldRate =>
-        s"rate(${metricBuilder.name.mkString(metadataScopeSeparator())})"
-      case other => metricBuilder.name.mkString(metadataScopeSeparator())
+        s"rate($metric)"
+      case _ => metric
     }
   }
 }

diff --git a/server/src/test/scala/com/twitter/server/handler/exp/MetricExpressionHandlerTest.scala b/server/src/test/scala/com/twitter/server/handler/exp/MetricExpressionHandlerTest.scala
@@ -49,10 +49,14 @@ class MetricExpressionHandlerTest extends AnyFunSuite {
   val latencyP99 =
     ExpressionSchema("latency_p99", Expression(latencyMb, Right(0.99))).withNamespace("tenantName")
 
+  val failureExpression =
+    ExpressionSchema("failures", Expression(failuresMb, true))
+
   val expressionSchemaMap: Map[ExpressionSchemaKey, ExpressionSchema] = Map(
     ExpressionSchemaKey("success_rate", Map(), Seq()) -> successRateExpression,
     ExpressionSchemaKey("throughput", Map(), Seq()) -> throughputExpression,
-    ExpressionSchemaKey("latency", Map(), Seq("path", "to", "tenantName")) -> latencyP99
+    ExpressionSchemaKey("latency", Map(), Seq("path", "to", "tenantName")) -> latencyP99,
+    ExpressionSchemaKey("failures", Map(), Seq()) -> failureExpression
   )
 
   val expressionRegistry = new SchemaRegistry {
@@ -150,7 +154,21 @@ class MetricExpressionHandlerTest extends AnyFunSuite {
         |        "bucket": "p99"
         |      },
         |      "namespaces" : ["tenantName"],
-        |      "expression" :  "latency.p99",
+        |      "expression" : "latency.p99",
+        |      "bounds" : {
+        |        "kind" : "unbounded"
+        |      },
+        |      "description" : "Unspecified",
+        |      "unit" : "Unspecified"
+        |    },
+        |    {
+        |      "name" : "failures",
+        |      "labels" : {
+        |        "process_path" : "Unspecified",
+        |        "service_name" : "Unspecified",
+        |        "role" : "NoRoleSpecified"
+        |      },
+        |      "expression" : "failures/*",
         |      "bounds" : {
         |        "kind" : "unbounded"
         |      },