util-stats,finagle,twitter-server: Labels want to be free

Problem

Right now, it's difficult to add new labels to metrics, because they're
hardcoded in a case class.

Solution

Make labels just a simple Map[String, String], or a dictionary when we
export it.

JIRA Issues: CSL-11163

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

CHANGELOG.rst

@@ -17,6 +17,11 @@ Admin Endpoint Versions
   "kind": "counterish_gauge", instead of "counterish_gauge": "true"
   ``PHAB_ID=D700353``.
 
+* Bump expressions.json to version 1.1: the `labels` field in the Metric
+  Metadata Expressions output to return a dictionary instead of a well-defined
+  JSON object. However, the existing fields in `labels` will be preserved for
+  now. ``PHAB_ID=D700065``
+
 Runtime Behavior Changes
 ~~~~~~~~~~~~~~~~~~~~~~~~
 

doc/src/sphinx/Admin.rst

@@ -517,17 +517,18 @@ Top-level/instance-wide Fields:
 Expression Object Fields:
 ^^^^^^^^^^^^^^^^^^^^^^^^^
 :name: a string representing the name or significance of the expression.
-:labels: an object which describes where the metric comes from (see next section for labels-level fields).
+:labels: a dictionary (string -> string) with additional metadata about the metric
 :expresssion: a string representation illustrating how to aggregate metrics into the expression value.
 :bounds: an object which describes the bounds for what values resulting from the expression should be considered "healthy" for the service.
 :description: text description of the expression, intended for human consumption.
 :unit: the appropriate unit for this metric (ex, milliseconds, megabytes, count).
 
-Labels Object Fields:
-^^^^^^^^^^^^^^^^^^^^^
-This object is likely to change content and even become more flexible in the future. It is safest to treat it as a Map[String, String].
+Labels Entries:
+^^^^^^^^^^^^^^^
+Although it should be interpreted as a dictionary, not as an object, there are some common fields that are often relevant, especially for Finagle services.
+
 :process_path: a string indicating the relevant downstream, when there is one.
-:service_name: a string representing the finagle service name. particularly helpful for services containing multiple internal servers (ex, thrift and http).
+:service_name: a string representing the Finagle service name. particularly helpful for services containing multiple internal servers (ex, thrift and http).
 :role: either "Server" or "Client" indicating whether the expression pertains to this server or one of its downstreams. Should be treated as a string.
 
 Bounds Object Fields:

server/src/main/scala/com/twitter/server/handler/MetricExpressionHandler.scala

@@ -20,7 +20,7 @@ import com.twitter.server.util.{AdminJsonConverter, MetricSchemaSource}
 import com.twitter.util.Future
 
 object MetricExpressionHandler {
-  private val Version = 1.0
+  private val Version = 1.1
   private val statsFormatter = StatsFormatter.default
 
   /**

server/src/main/scala/com/twitter/server/util/exp/ExpressionJson.scala

@@ -4,6 +4,7 @@ import com.fasterxml.jackson.core.{JsonGenerator, JsonParser}
 import com.fasterxml.jackson.databind.deser.std.StdDeserializer
 import com.fasterxml.jackson.databind.ser.std.StdSerializer
 import com.fasterxml.jackson.databind.{DeserializationContext, SerializerProvider}
+import com.twitter.finagle.stats.NoRoleSpecified
 import com.twitter.finagle.stats.exp._
 
 /**
@@ -26,13 +27,17 @@ object ExpressionJson {
       gen.writeStringField("name", expressionSchema.name)
 
       gen.writeObjectFieldStart("labels")
-      gen.writeStringField(
-        "process_path",
-        expressionSchema.labels.processPath.getOrElse("Unspecified"))
-      gen.writeStringField(
-        "service_name",
-        expressionSchema.labels.serviceName.getOrElse("Unspecified"))
-      gen.writeStringField("role", expressionSchema.labels.role.toString)
+
+      // we need this to evolve the expressions format compatibly
+      // in the future, we will only export labels that are set
+      val labels = Map(
+        ExpressionSchema.ProcessPath -> "Unspecified",
+        ExpressionSchema.ServiceName -> "Unspecified",
+        ExpressionSchema.Role -> NoRoleSpecified.toString
+      ) ++ expressionSchema.labels
+      for ((key, value) <- labels) {
+        gen.writeStringField(key, value)
+      }
       gen.writeEndObject()
 
       if (expressionSchema.namespace.nonEmpty) {

server/src/test/scala/com/twitter/server/handler/exp/MetricExpressionHandlerTest.scala

@@ -44,9 +44,9 @@ class MetricExpressionHandlerTest extends AnyFunSuite {
     ExpressionSchema("latency_p99", Expression(latencyMb, Right(0.99))).withNamespace("tenantName")
 
   val expressionSchemaMap: Map[ExpressionSchemaKey, ExpressionSchema] = Map(
-    ExpressionSchemaKey("success_rate", None, Seq()) -> successRateExpression,
-    ExpressionSchemaKey("throughput", None, Seq()) -> throughputExpression,
-    ExpressionSchemaKey("latency", None, Seq("path", "to", "tenantName")) -> latencyP99
+    ExpressionSchemaKey("success_rate", Map(), Seq()) -> successRateExpression,
+    ExpressionSchemaKey("throughput", Map(), Seq()) -> throughputExpression,
+    ExpressionSchemaKey("latency", Map(), Seq("path", "to", "tenantName")) -> latencyP99
   )
 
   val expressionRegistry = new SchemaRegistry {
@@ -97,7 +97,7 @@ class MetricExpressionHandlerTest extends AnyFunSuite {
     val expectedResponse =
       """
         |{
-        |  "@version" : 1.0,
+        |  "@version" : 1.1,
         |  "counters_latched" : false,
         |  "separator_char" : "/",
         |  "expressions" : [
@@ -212,7 +212,7 @@ class MetricExpressionHandlerTest extends AnyFunSuite {
     val expectedResponse =
       """
         |{
-        |  "@version" : 1.0,
+        |  "@version" : 1.1,
         |  "counters_latched" : false,
         |  "separator_char" : "/",
         |  "expressions" : [
@@ -247,7 +247,7 @@ class MetricExpressionHandlerTest extends AnyFunSuite {
     val expectedResponse =
       """
         |{
-        |  "@version" : 1.0,
+        |  "@version" : 1.1,
         |  "counters_latched" : false,
         |  "separator_char" : "/",
         |  "expressions" : [
@@ -293,7 +293,7 @@ class MetricExpressionHandlerTest extends AnyFunSuite {
     val expectedResponse =
       """
         |{
-        |  "@version" : 1.0,
+        |  "@version" : 1.1,
         |  "counters_latched" : false,
         |  "separator_char" : "/",
         |  "expressions" : [
@@ -324,7 +324,7 @@ class MetricExpressionHandlerTest extends AnyFunSuite {
     val expectedResponse =
       """
         |{
-        |  "@version" : 1.0,
+        |  "@version" : 1.1,
         |  "counters_latched" : false,
         |  "separator_char" : "/",
         |  "expressions" : [