! statsd,datadog: reformat metric names to get a better display of automatically named traces, closes #35

After a bit of testing it seems like instead of  generating more friendly names by default, we need to do
changes that are specific to each module (StatsD and Datadog) in order to make it work properly. Here is
a list of the introduced changes:
  - StatsD
    - Change dots (.) to underscores (_) on the host name string.
    - Change ': ' to single underscore and spaced to underscore. Userful for automatically generated http
      trace names.
    - Change '/' to underscore. Ideally the dashboard could replace '_' with '/' to display it like a path.

  - Datadog
    - Change ': ' to ':'. Userful for automatically generated http trace names.

6 files changed, 94 insertions, 39 deletions

diff --git a/kamon-datadog/src/main/scala/kamon/datadog/Datadog.scala b/kamon-datadog/src/main/scala/kamon/datadog/Datadog.scala
index b4358ce7..1ae493f7 100644
--- a/kamon-datadog/src/main/scala/kamon/datadog/Datadog.scala
+++ b/kamon-datadog/src/main/scala/kamon/datadog/Datadog.scala
@@ -70,12 +70,12 @@ class DatadogExtension(system: ExtendedActorSystem) extends Kamon.Extension {
   def buildMetricsListener(tickInterval: Long, flushInterval: Long): ActorRef = {
     assert(flushInterval >= tickInterval, "Datadog flush-interval needs to be equal or greater to the tick-interval")
 
-    val metricsTranslator = system.actorOf(DatadogMetricsSender.props(datadogHost, maxPacketSizeInBytes), "datadog-metrics-sender")
+    val metricsSender = system.actorOf(DatadogMetricsSender.props(datadogHost, maxPacketSizeInBytes), "datadog-metrics-sender")
     if (flushInterval == tickInterval) {
       // No need to buffer the metrics, let's go straight to the metrics sender.
-      metricsTranslator
+      metricsSender
     } else {
-      system.actorOf(TickMetricSnapshotBuffer.props(flushInterval.toInt.millis, metricsTranslator), "datadog-metrics-buffer")
+      system.actorOf(TickMetricSnapshotBuffer.props(flushInterval.toInt.millis, metricsSender), "datadog-metrics-buffer")
     }
   }
 }
diff --git a/kamon-datadog/src/main/scala/kamon/datadog/DatadogMetricsSender.scala b/kamon-datadog/src/main/scala/kamon/datadog/DatadogMetricsSender.scala
index 17e19d0b..0f67cc34 100644
--- a/kamon-datadog/src/main/scala/kamon/datadog/DatadogMetricsSender.scala
+++ b/kamon-datadog/src/main/scala/kamon/datadog/DatadogMetricsSender.scala
@@ -90,8 +90,11 @@ class DatadogMetricsSender(remote: InetSocketAddress, maxPacketSizeInBytes: Long
   def buildMetricName(groupIdentity: MetricGroupIdentity, metricIdentity: MetricIdentity): String =
     s"$appName.${groupIdentity.category.name}.${metricIdentity.name}"
 
-  def buildIdentificationTag(groupIdentity: MetricGroupIdentity, metricIdentity: MetricIdentity): String =
-    s"|#${groupIdentity.category.name}:${groupIdentity.name}"
+  def buildIdentificationTag(groupIdentity: MetricGroupIdentity, metricIdentity: MetricIdentity): String = {
+    // Make the automatic HTTP trace names a bit more friendly
+    val normalizedEntityName = groupIdentity.name.replace(": ", ":")
+    s"|#${groupIdentity.category.name}:${normalizedEntityName}"
+  }
 }
 
 object DatadogMetricsSender {
diff --git a/kamon-datadog/src/test/scala/kamon/datadog/DatadogMetricSenderSpec.scala b/kamon-datadog/src/test/scala/kamon/datadog/DatadogMetricSenderSpec.scala
index 91b503e2..713db30d 100644
--- a/kamon-datadog/src/test/scala/kamon/datadog/DatadogMetricSenderSpec.scala
+++ b/kamon-datadog/src/test/scala/kamon/datadog/DatadogMetricSenderSpec.scala
@@ -30,8 +30,19 @@ import java.net.InetSocketAddress
 import com.typesafe.config.ConfigFactory
 
 class DatadogMetricSenderSpec extends TestKitBase with WordSpecLike with Matchers {
-  implicit lazy val system = ActorSystem("datadog-metric-sender-spec",
-    ConfigFactory.parseString("kamon.datadog.max-packet-size = 256 bytes"))
+  implicit lazy val system: ActorSystem = ActorSystem("datadog-metric-sender-spec", ConfigFactory.parseString(
+    """
+      |kamon {
+      |  metrics {
+      |    disable-aspectj-weaver-missing-error = true
+      |  }
+      |
+      |  datadog {
+      |    max-packet-size = 256 bytes
+      |  }
+      |}
+      |
+    """.stripMargin))
 
   val collectionContext = Kamon(Metrics).buildDefaultCollectionContext
 
diff --git a/kamon-statsd/src/main/scala/kamon/statsd/StatsD.scala b/kamon-statsd/src/main/scala/kamon/statsd/StatsD.scala
index dcd78f78..299b1acc 100644
--- a/kamon-statsd/src/main/scala/kamon/statsd/StatsD.scala
+++ b/kamon-statsd/src/main/scala/kamon/statsd/StatsD.scala
@@ -32,6 +32,8 @@ object StatsD extends ExtensionId[StatsDExtension] with ExtensionIdProvider {
   override def createExtension(system: ExtendedActorSystem): StatsDExtension = new StatsDExtension(system)
 
   trait MetricKeyGenerator {
+    def localhostName: String
+    def normalizedLocalhostName: String
     def generateKey(groupIdentity: MetricGroupIdentity, metricIdentity: MetricIdentity): String
   }
 }
@@ -69,22 +71,34 @@ class StatsDExtension(system: ExtendedActorSystem) extends Kamon.Extension {
 
   def buildMetricsListener(tickInterval: Long, flushInterval: Long): ActorRef = {
     assert(flushInterval >= tickInterval, "StatsD flush-interval needs to be equal or greater to the tick-interval")
+    val defaultMetricKeyGenerator = new SimpleMetricKeyGenerator(system.settings.config)
+
+    val metricsSender = system.actorOf(StatsDMetricsSender.props(
+      statsDHost,
+      maxPacketSizeInBytes,
+      defaultMetricKeyGenerator), "statsd-metrics-sender")
 
-    val metricsTranslator = system.actorOf(StatsDMetricsSender.props(statsDHost, maxPacketSizeInBytes), "statsd-metrics-sender")
     if (flushInterval == tickInterval) {
       // No need to buffer the metrics, let's go straight to the metrics sender.
-      metricsTranslator
+      metricsSender
     } else {
-      system.actorOf(TickMetricSnapshotBuffer.props(flushInterval.toInt.millis, metricsTranslator), "statsd-metrics-buffer")
+      system.actorOf(TickMetricSnapshotBuffer.props(flushInterval.toInt.millis, metricsSender), "statsd-metrics-buffer")
     }
   }
 }
 
 class SimpleMetricKeyGenerator(config: Config) extends StatsD.MetricKeyGenerator {
   val application = config.getString("kamon.statsd.simple-metric-key-generator.application")
-  val localhostName = ManagementFactory.getRuntimeMXBean.getName.split('@')(1)
+  val _localhostName = ManagementFactory.getRuntimeMXBean.getName.split('@')(1)
+  val _normalizedLocalhostName = _localhostName.replace('.', '_')
+
+  def localhostName: String = _localhostName
 
-  def generateKey(groupIdentity: MetricGroupIdentity, metricIdentity: MetricIdentity): String =
-    s"${application}.${localhostName}.${groupIdentity.category.name}.${groupIdentity.name}.${metricIdentity.name}"
+  def normalizedLocalhostName: String = _normalizedLocalhostName
+
+  def generateKey(groupIdentity: MetricGroupIdentity, metricIdentity: MetricIdentity): String = {
+    val normalizedGroupName = groupIdentity.name.replace(": ", "-").replace(" ", "_").replace("/", "_")
+    s"${application}.${normalizedLocalhostName}.${groupIdentity.category.name}.${normalizedGroupName}.${metricIdentity.name}"
+  }
 }
 
diff --git a/kamon-statsd/src/main/scala/kamon/statsd/StatsDMetricsSender.scala b/kamon-statsd/src/main/scala/kamon/statsd/StatsDMetricsSender.scala
index 94bab27c..8fbf4fee 100644
--- a/kamon-statsd/src/main/scala/kamon/statsd/StatsDMetricsSender.scala
+++ b/kamon-statsd/src/main/scala/kamon/statsd/StatsDMetricsSender.scala
@@ -26,10 +26,10 @@ import java.util.Locale
 
 import kamon.metric.instrument.{ Counter, Histogram }
 
-class StatsDMetricsSender(remote: InetSocketAddress, maxPacketSizeInBytes: Long) extends Actor with UdpExtensionProvider {
+class StatsDMetricsSender(remote: InetSocketAddress, maxPacketSizeInBytes: Long, metricKeyGenerator: StatsD.MetricKeyGenerator)
+    extends Actor with UdpExtensionProvider {
   import context.system
 
-  val metricKeyGenerator = new SimpleMetricKeyGenerator(context.system.settings.config)
   val symbols = DecimalFormatSymbols.getInstance(Locale.US)
   symbols.setDecimalSeparator('.') // Just in case there is some weird locale config we are not aware of.
 
@@ -80,7 +80,8 @@ class StatsDMetricsSender(remote: InetSocketAddress, maxPacketSizeInBytes: Long)
 }
 
 object StatsDMetricsSender {
-  def props(remote: InetSocketAddress, maxPacketSize: Long): Props = Props(new StatsDMetricsSender(remote, maxPacketSize))
+  def props(remote: InetSocketAddress, maxPacketSize: Long, metricKeyGenerator: StatsD.MetricKeyGenerator): Props =
+    Props(new StatsDMetricsSender(remote, maxPacketSize, metricKeyGenerator))
 }
 
 trait UdpExtensionProvider {
diff --git a/kamon-statsd/src/test/scala/kamon/statsd/StatsDMetricSenderSpec.scala b/kamon-statsd/src/test/scala/kamon/statsd/StatsDMetricSenderSpec.scala
index 60d52491..3bc1364c 100644
--- a/kamon-statsd/src/test/scala/kamon/statsd/StatsDMetricSenderSpec.scala
+++ b/kamon-statsd/src/test/scala/kamon/statsd/StatsDMetricSenderSpec.scala
@@ -30,15 +30,31 @@ import java.net.InetSocketAddress
 import com.typesafe.config.ConfigFactory
 
 class StatsDMetricSenderSpec extends TestKitBase with WordSpecLike with Matchers {
-  implicit lazy val system = ActorSystem("statsd-metric-sender-spec",
-    ConfigFactory.parseString("kamon.statsd.max-packet-size = 256 bytes"))
+  implicit lazy val system: ActorSystem = ActorSystem("statsd-metric-sender-spec", ConfigFactory.parseString(
+    """
+      |kamon {
+      |  metrics {
+      |    disable-aspectj-weaver-missing-error = true
+      |  }
+      |
+      |  statsd {
+      |    max-packet-size = 256 bytes
+      |  }
+      |}
+      |
+    """.stripMargin))
 
   val collectionContext = Kamon(Metrics).buildDefaultCollectionContext
 
   "the StatsDMetricSender" should {
+    "normalize the group entity name to remove spaces, colons and replace '/' with '_'" in new UdpListenerFixture {
+      val testMetricKey = buildMetricKey("trace", "POST: /kamon/example", "elapsed-time")
+      testMetricKey should be(s"kamon.localhost_local.trace.POST-_kamon_example.elapsed-time")
+    }
+
     "flush the metrics data after processing the tick, even if the max-packet-size is not reached" in new UdpListenerFixture {
-      val testMetricName = "test-metric"
-      val testMetricKey = buildMetricKey(testMetricName)
+      val testMetricName = "processing-time"
+      val testMetricKey = buildMetricKey("actor", "/user/kamon", testMetricName)
       val testRecorder = Histogram(1000L, Precision.Normal, Scale.Unit)
       testRecorder.record(10L)
 
@@ -49,8 +65,8 @@ class StatsDMetricSenderSpec extends TestKitBase with WordSpecLike with Matchers
     }
 
     "render several measurements of the same key under a single (key + multiple measurements) packet" in new UdpListenerFixture {
-      val testMetricName = "test-metric"
-      val testMetricKey = buildMetricKey(testMetricName)
+      val testMetricName = "processing-time"
+      val testMetricKey = buildMetricKey("actor", "/user/kamon", testMetricName)
       val testRecorder = Histogram(1000L, Precision.Normal, Scale.Unit)
       testRecorder.record(10L)
       testRecorder.record(11L)
@@ -63,8 +79,8 @@ class StatsDMetricSenderSpec extends TestKitBase with WordSpecLike with Matchers
     }
 
     "include the correspondent sampling rate when rendering multiple occurrences of the same value" in new UdpListenerFixture {
-      val testMetricName = "test-metric"
-      val testMetricKey = buildMetricKey(testMetricName)
+      val testMetricName = "processing-time"
+      val testMetricKey = buildMetricKey("actor", "/user/kamon", testMetricName)
       val testRecorder = Histogram(1000L, Precision.Normal, Scale.Unit)
       testRecorder.record(10L)
       testRecorder.record(10L)
@@ -76,8 +92,8 @@ class StatsDMetricSenderSpec extends TestKitBase with WordSpecLike with Matchers
     }
 
     "flush the packet when the max-packet-size is reached" in new UdpListenerFixture {
-      val testMetricName = "test-metric"
-      val testMetricKey = buildMetricKey(testMetricName)
+      val testMetricName = "processing-time"
+      val testMetricKey = buildMetricKey("actor", "/user/kamon", testMetricName)
       val testRecorder = Histogram(10000L, Precision.Normal, Scale.Unit)
 
       var bytes = testMetricKey.length
@@ -97,9 +113,9 @@ class StatsDMetricSenderSpec extends TestKitBase with WordSpecLike with Matchers
 
     "render multiple keys in the same packet using newline as separator" in new UdpListenerFixture {
       val firstTestMetricName = "first-test-metric"
-      val firstTestMetricKey = buildMetricKey(firstTestMetricName)
+      val firstTestMetricKey = buildMetricKey("actor", "/user/kamon", firstTestMetricName)
       val secondTestMetricName = "second-test-metric"
-      val secondTestMetricKey = buildMetricKey(secondTestMetricName)
+      val secondTestMetricKey = buildMetricKey("actor", "/user/kamon", secondTestMetricName)
 
       val firstTestRecorder = Histogram(1000L, Precision.Normal, Scale.Unit)
       val secondTestRecorder = Histogram(1000L, Precision.Normal, Scale.Unit)
@@ -121,14 +137,32 @@ class StatsDMetricSenderSpec extends TestKitBase with WordSpecLike with Matchers
   }
 
   trait UdpListenerFixture {
-    val localhostName = ManagementFactory.getRuntimeMXBean.getName.split('@')(1)
     val testMaxPacketSize = system.settings.config.getBytes("kamon.statsd.max-packet-size")
+    val metricKeyGenerator = new SimpleMetricKeyGenerator(system.settings.config) {
+      override def normalizedLocalhostName: String = "localhost_local"
+    }
 
-    def buildMetricKey(metricName: String): String = s"kamon.$localhostName.test-metric-category.test-group.$metricName"
+    val testGroupIdentity = new MetricGroupIdentity {
+      val name: String = "/user/kamon"
+      val category: MetricGroupCategory = new MetricGroupCategory {
+        val name: String = "actor"
+      }
+    }
+
+    def buildMetricKey(categoryName: String, entityName: String, metricName: String): String = {
+      val metricIdentity = new MetricIdentity { val name: String = metricName }
+      val groupIdentity = new MetricGroupIdentity {
+        val name: String = entityName
+        val category: MetricGroupCategory = new MetricGroupCategory {
+          val name: String = categoryName
+        }
+      }
+      metricKeyGenerator.generateKey(groupIdentity, metricIdentity)
+    }
 
     def setup(metrics: Map[String, MetricSnapshot]): TestProbe = {
       val udp = TestProbe()
-      val metricsSender = system.actorOf(Props(new StatsDMetricsSender(new InetSocketAddress(localhostName, 0), testMaxPacketSize) {
+      val metricsSender = system.actorOf(Props(new StatsDMetricsSender(new InetSocketAddress("127.0.0.1", 0), testMaxPacketSize, metricKeyGenerator) {
         override def udpExtension(implicit system: ActorSystem): ActorRef = udp.ref
       }))
 
@@ -136,17 +170,9 @@ class StatsDMetricSenderSpec extends TestKitBase with WordSpecLike with Matchers
       udp.expectMsgType[Udp.SimpleSender]
       udp.reply(Udp.SimpleSenderReady)
 
-      val testGroupIdentity = new MetricGroupIdentity {
-        val name: String = "test-group"
-        val category: MetricGroupCategory = new MetricGroupCategory {
-          val name: String = "test-metric-category"
-        }
-      }
-
       val testMetrics = for ((metricName, snapshot) ← metrics) yield {
         val testMetricIdentity = new MetricIdentity {
           val name: String = metricName
-          val tag: String = ""
         }
 
         (testMetricIdentity, snapshot)