From 42c423cbf2206446ed9890e1daabff9a4a52c2b3 Mon Sep 17 00:00:00 2001
From: Brian Harrington <brharrington@gmail.com>
Date: Fri, 17 May 2024 20:08:24 -0500
Subject: [PATCH] eval: use parens for simple legends

When generating a simple legend with variable substitutions
use the `$(var)` for instead of `$var`. This avoid issues
in some cases where the data source is using invalid key
characters.
---
 .../netflix/atlas/eval/graph/SimpleLegends.scala   |  4 ++--
 .../atlas/eval/graph/SimpleLegendsSuite.scala      | 14 +++++++++-----
 2 files changed, 11 insertions(+), 7 deletions(-)

diff --git a/atlas-eval/src/main/scala/com/netflix/atlas/eval/graph/SimpleLegends.scala b/atlas-eval/src/main/scala/com/netflix/atlas/eval/graph/SimpleLegends.scala
index 9ca1487dc..e7126ab9b 100644
--- a/atlas-eval/src/main/scala/com/netflix/atlas/eval/graph/SimpleLegends.scala
+++ b/atlas-eval/src/main/scala/com/netflix/atlas/eval/graph/SimpleLegends.scala
@@ -73,7 +73,7 @@ object SimpleLegends extends StrictLogging {
   }
 
   private def withLegend(expr: StyleExpr, legend: String): StyleExpr = {
-    val label = if (expr.offset > 0L) s"$legend (offset=$$atlas.offset)" else legend
+    val label = if (expr.offset > 0L) s"$legend (offset=$$(atlas.offset))" else legend
     expr.copy(settings = expr.settings + ("legend" -> label))
   }
 
@@ -94,7 +94,7 @@ object SimpleLegends extends StrictLogging {
 
   private def generateLegend(expr: StyleExpr, kv: Map[String, String]): StyleExpr = {
     if (expr.expr.isGrouped) {
-      val fmt = expr.expr.finalGrouping.mkString("$", " $", "")
+      val fmt = expr.expr.finalGrouping.mkString("$(", ") $(", ")")
       withLegend(expr, fmt)
     } else if (kv.contains("name")) {
       withLegend(expr, kv("name"))
diff --git a/atlas-eval/src/test/scala/com/netflix/atlas/eval/graph/SimpleLegendsSuite.scala b/atlas-eval/src/test/scala/com/netflix/atlas/eval/graph/SimpleLegendsSuite.scala
index af38dbd3c..a4332cfd5 100644
--- a/atlas-eval/src/test/scala/com/netflix/atlas/eval/graph/SimpleLegendsSuite.scala
+++ b/atlas-eval/src/test/scala/com/netflix/atlas/eval/graph/SimpleLegendsSuite.scala
@@ -62,7 +62,7 @@ class SimpleLegendsSuite extends FunSuite {
   }
 
   test("use group by keys") {
-    assertEquals(legends("name,cpu,:eq,:sum,(,app,id,),:by"), List("$app $id"))
+    assertEquals(legends("name,cpu,:eq,:sum,(,app,id,),:by"), List("$(app) $(id)"))
   }
 
   test("name with math") {
@@ -79,7 +79,7 @@ class SimpleLegendsSuite extends FunSuite {
 
   test("name with offsets") {
     val expr = "name,cpu,:eq,:sum,(,0h,1w,),:offset"
-    assertEquals(legends(expr), List("cpu", "cpu (offset=$atlas.offset)"))
+    assertEquals(legends(expr), List("cpu", "cpu (offset=$(atlas.offset))"))
   }
 
   test("name with avg") {
@@ -103,17 +103,21 @@ class SimpleLegendsSuite extends FunSuite {
   }
 
   test("name with node avg and grouping") {
-    assertEquals(legends("name,cpu,:eq,:node-avg,(,app,),:by"), List("$app"))
+    assertEquals(legends("name,cpu,:eq,:node-avg,(,app,),:by"), List("$(app)"))
+  }
+
+  test("name with node avg and grouping with special chars") {
+    assertEquals(legends("name,cpu,:eq,:node-avg,(,foo:bar,),:by"), List("$(foo:bar)"))
   }
 
   test("name with node avg and nested grouping") {
     val expr = "name,cpu,:eq,:node-avg,(,app,region,),:by,:max,(,region,),:by"
-    assertEquals(legends(expr), List("$region"))
+    assertEquals(legends(expr), List("$(region)"))
   }
 
   test("group by with offsets") {
     val expr = "name,cpu,:eq,:sum,(,id,),:by,(,0h,1w,),:offset"
-    assertEquals(legends(expr), List("$id", "$id (offset=$atlas.offset)"))
+    assertEquals(legends(expr), List("$(id)", "$(id) (offset=$(atlas.offset))"))
   }
 
   test("complex: same name and math") {