+ statsd: allow percent-encoding of metric section names, related to #46

5 files changed, 170 insertions, 109 deletions

diff --git a/kamon-statsd/src/main/resources/reference.conf b/kamon-statsd/src/main/resources/reference.conf
index 522f9ca9..bd713f65 100644
--- a/kamon-statsd/src/main/resources/reference.conf
+++ b/kamon-statsd/src/main/resources/reference.conf
@@ -30,18 +30,30 @@ kamon {
     report-system-metrics = false
 
     simple-metric-key-generator {
+
       # Application prefix for all metrics pushed to StatsD. The default namespacing scheme for metrics follows
       # this pattern:
       #    application.host.entity.entity-name.metric-name
       application = "kamon"
+
       # Includes the name of the hostname in the generated metric. When set to false, the scheme for the metrics
       # will look as follows:
       #    application.entity.entity-name.metric-name
       include-hostname = true
+
       # Allow users to override the name of the hostname reported by kamon. When changed, the scheme for the metrics
       # will have the following pattern:
-      # application.myhostname.entity.entity-name.metric-name
-      hostname-override = "none"
+      #   application.hostname-override-value.entity.entity-name.metric-name
+      hostname-override = none
+
+      # When the sections that make up the metric names have special characters like dots (very common in dispatcher
+      # names) or forward slashes (all actor metrics) we need to sanitize those values before sending them to StatsD
+      # with one of the following strategies:
+      #   - normalize: changes ': ' to '-' and ' ', '/' and '.' to '_'.
+      #   - percent-encode: percent encode the section on the metric name. Please note that StatsD doesn't support
+      #     percent encoded metric names, this option is only useful if using our docker image which has a patched
+      #     version of StatsD or if you are running your own, customized version of StatsD that supports this.
+      metric-name-normalization-strategy = normalize
     }
   }
 }
\ No newline at end of file
diff --git a/kamon-statsd/src/main/scala/kamon/statsd/SimpleMetricKeyGenerator.scala b/kamon-statsd/src/main/scala/kamon/statsd/SimpleMetricKeyGenerator.scala
new file mode 100644
index 00000000..f0bc6d64
--- /dev/null
+++ b/kamon-statsd/src/main/scala/kamon/statsd/SimpleMetricKeyGenerator.scala
@@ -0,0 +1,69 @@
+package kamon.statsd
+
+import java.lang.management.ManagementFactory
+
+import com.typesafe.config.Config
+import kamon.metric.UserMetrics.UserMetricGroup
+import kamon.metric.{ MetricIdentity, MetricGroupIdentity }
+
+class SimpleMetricKeyGenerator(config: Config) extends StatsD.MetricKeyGenerator {
+  type Normalizer = String ⇒ String
+
+  val configSettings = config.getConfig("kamon.statsd.simple-metric-key-generator")
+  val application = configSettings.getString("application")
+  val includeHostname = configSettings.getBoolean("include-hostname")
+  val hostnameOverride = configSettings.getString("hostname-override")
+  val normalizer = createNormalizer(configSettings.getString("metric-name-normalization-strategy"))
+
+  val normalizedHostname =
+    if (hostnameOverride.equals("none")) normalizer(hostName)
+    else normalizer(hostnameOverride)
+
+  val baseName: String =
+    if (includeHostname) s"$application.$normalizedHostname"
+    else application
+
+  def generateKey(groupIdentity: MetricGroupIdentity, metricIdentity: MetricIdentity): String = {
+    val normalizedGroupName = normalizer(groupIdentity.name)
+    val key = s"${baseName}.${groupIdentity.category.name}.${normalizedGroupName}"
+
+    if (isUserMetric(groupIdentity)) key
+    else s"${key}.${metricIdentity.name}"
+  }
+
+  def isUserMetric(groupIdentity: MetricGroupIdentity): Boolean = groupIdentity.isInstanceOf[UserMetricGroup]
+
+  def hostName: String = ManagementFactory.getRuntimeMXBean.getName.split('@')(1)
+
+  def createNormalizer(strategy: String): Normalizer = strategy match {
+    case "percent-encode" ⇒ PercentEncoder.encode
+    case "normalize"      ⇒ (s: String) ⇒ s.replace(": ", "-").replace(" ", "_").replace("/", "_").replace(".", "_")
+  }
+}
+
+object PercentEncoder {
+
+  def encode(originalString: String): String = {
+    val encodedString = new StringBuilder()
+
+    for (character ← originalString) {
+      if (shouldEncode(character)) {
+        encodedString.append('%')
+        val charHexValue = Integer.toHexString(character).toUpperCase
+        if (charHexValue.length < 2)
+          encodedString.append('0')
+
+        encodedString.append(charHexValue)
+
+      } else {
+        encodedString.append(character)
+      }
+    }
+    encodedString.toString()
+  }
+
+  def shouldEncode(ch: Char): Boolean = {
+    if (ch > 128 || ch < 0) true
+    else " %$&+,./:;=?@<>#%".indexOf(ch) >= 0;
+  }
+}
diff --git a/kamon-statsd/src/main/scala/kamon/statsd/StatsD.scala b/kamon-statsd/src/main/scala/kamon/statsd/StatsD.scala
index 9c1ccbb0..c8f647a8 100644
--- a/kamon-statsd/src/main/scala/kamon/statsd/StatsD.scala
+++ b/kamon-statsd/src/main/scala/kamon/statsd/StatsD.scala
@@ -34,8 +34,6 @@ 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
   }
 }
@@ -107,40 +105,4 @@ class StatsDExtension(system: ExtendedActorSystem) extends Kamon.Extension {
       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 includeHostnameInMetrics =
-    config.getBoolean("kamon.statsd.simple-metric-key-generator.include-hostname")
-  val hostnameOverride =
-    config.getString("kamon.statsd.simple-metric-key-generator.hostname-override")
-
-  val _localhostName = ManagementFactory.getRuntimeMXBean.getName.split('@')(1)
-  val _normalizedLocalhostName = _localhostName.replace('.', '_')
-
-  def localhostName: String = _localhostName
-
-  def normalizedLocalhostName: String = _normalizedLocalhostName
-
-  val hostname: String =
-    if (hostnameOverride == "none") normalizedLocalhostName
-    else hostnameOverride
-
-  val baseName: String =
-    if (includeHostnameInMetrics) s"${application}.${hostname}"
-    else application
-
-  def generateKey(groupIdentity: MetricGroupIdentity, metricIdentity: MetricIdentity): String = {
-    val normalizedGroupName = groupIdentity.name.replace(": ", "-").replace(" ", "_").replace("/", "_")
-    val key = s"${baseName}.${groupIdentity.category.name}.${normalizedGroupName}"
-
-    if (isUserMetric(groupIdentity)) key
-    else s"${key}.${metricIdentity.name}"
-  }
-
-  def isUserMetric(groupIdentity: MetricGroupIdentity): Boolean = groupIdentity match {
-    case someUserMetric: UserMetricGroup ⇒ true
-    case everythingElse                  ⇒ false
-  }
-}
+}
\ No newline at end of file
diff --git a/kamon-statsd/src/test/scala/kamon/statsd/SimpleMetricKeyGeneratorSpec.scala b/kamon-statsd/src/test/scala/kamon/statsd/SimpleMetricKeyGeneratorSpec.scala
new file mode 100644
index 00000000..ed3fae5b
--- /dev/null
+++ b/kamon-statsd/src/test/scala/kamon/statsd/SimpleMetricKeyGeneratorSpec.scala
@@ -0,0 +1,80 @@
+package kamon.statsd
+
+import com.typesafe.config.ConfigFactory
+import kamon.metric.{ MetricGroupCategory, MetricGroupIdentity, MetricIdentity }
+import org.scalatest.{ Matchers, WordSpec }
+
+class SimpleMetricKeyGeneratorSpec extends WordSpec with Matchers {
+
+  val defaultConfiguration = ConfigFactory.parseString(
+    """
+      |kamon.statsd.simple-metric-key-generator {
+      |  application = kamon
+      |  hostname-override = none
+      |  include-hostname = true
+      |  metric-name-normalization-strategy = normalize
+      |}
+    """.stripMargin)
+
+  "the StatsDMetricSender" should {
+    "generate metric names that follow the application.host.entity.entity-name.metric-name pattern by default" in {
+      implicit val metricKeyGenerator = new SimpleMetricKeyGenerator(defaultConfiguration) {
+        override def hostName: String = "localhost"
+      }
+
+      buildMetricKey("actor", "/user/example", "processing-time") should be("kamon.localhost.actor._user_example.processing-time")
+      buildMetricKey("trace", "POST: /kamon/example", "elapsed-time") should be("kamon.localhost.trace.POST-_kamon_example.elapsed-time")
+    }
+
+    "allow to override the hostname" in {
+      val hostOverrideConfig = ConfigFactory.parseString("kamon.statsd.simple-metric-key-generator.hostname-override = kamon-host")
+      implicit val metricKeyGenerator = new SimpleMetricKeyGenerator(hostOverrideConfig.withFallback(defaultConfiguration)) {
+        override def hostName: String = "localhost"
+      }
+
+      buildMetricKey("actor", "/user/example", "processing-time") should be("kamon.kamon-host.actor._user_example.processing-time")
+      buildMetricKey("trace", "POST: /kamon/example", "elapsed-time") should be("kamon.kamon-host.trace.POST-_kamon_example.elapsed-time")
+    }
+
+    "removes host name when attribute 'include-hostname' is set to false" in {
+      val hostOverrideConfig = ConfigFactory.parseString("kamon.statsd.simple-metric-key-generator.include-hostname = false")
+      implicit val metricKeyGenerator = new SimpleMetricKeyGenerator(hostOverrideConfig.withFallback(defaultConfiguration)) {
+        override def hostName: String = "localhost"
+      }
+
+      buildMetricKey("actor", "/user/example", "processing-time") should be("kamon.actor._user_example.processing-time")
+      buildMetricKey("trace", "POST: /kamon/example", "elapsed-time") should be("kamon.trace.POST-_kamon_example.elapsed-time")
+    }
+
+    "remove spaces, colons and replace '/' with '_' when the normalization strategy is 'normalize'" in {
+      val hostOverrideConfig = ConfigFactory.parseString("kamon.statsd.simple-metric-key-generator.metric-name-normalization-strategy = normalize")
+      implicit val metricKeyGenerator = new SimpleMetricKeyGenerator(hostOverrideConfig.withFallback(defaultConfiguration)) {
+        override def hostName: String = "localhost.local"
+      }
+
+      buildMetricKey("actor", "/user/example", "processing-time") should be("kamon.localhost_local.actor._user_example.processing-time")
+      buildMetricKey("trace", "POST: /kamon/example", "elapsed-time") should be("kamon.localhost_local.trace.POST-_kamon_example.elapsed-time")
+    }
+
+    "percent-encode special characters in the group name and hostname when the normalization strategy is 'normalize'" in {
+      val hostOverrideConfig = ConfigFactory.parseString("kamon.statsd.simple-metric-key-generator.metric-name-normalization-strategy = percent-encode")
+      implicit val metricKeyGenerator = new SimpleMetricKeyGenerator(hostOverrideConfig.withFallback(defaultConfiguration)) {
+        override def hostName: String = "localhost.local"
+      }
+
+      buildMetricKey("actor", "/user/example", "processing-time") should be("kamon.localhost%2Elocal.actor.%2Fuser%2Fexample.processing-time")
+      buildMetricKey("trace", "POST: /kamon/example", "elapsed-time") should be("kamon.localhost%2Elocal.trace.POST%3A%20%2Fkamon%2Fexample.elapsed-time")
+    }
+  }
+
+  def buildMetricKey(categoryName: String, entityName: String, metricName: String)(implicit metricKeyGenerator: SimpleMetricKeyGenerator): 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)
+  }
+}
diff --git a/kamon-statsd/src/test/scala/kamon/statsd/StatsDMetricSenderSpec.scala b/kamon-statsd/src/test/scala/kamon/statsd/StatsDMetricSenderSpec.scala
index 28ead7dc..5d37bb75 100644
--- a/kamon-statsd/src/test/scala/kamon/statsd/StatsDMetricSenderSpec.scala
+++ b/kamon-statsd/src/test/scala/kamon/statsd/StatsDMetricSenderSpec.scala
@@ -37,85 +37,23 @@ class StatsDMetricSenderSpec extends TestKitBase with WordSpecLike with Matchers
       |    disable-aspectj-weaver-missing-error = true
       |  }
       |
-      |  statsd {
-      |    max-packet-size = 256 bytes
-      |    simple-metric-key-generator.hostname-override = "none"
+      |  statsd.simple-metric-key-generator {
+      |    application = kamon
+      |    hostname-override = kamon-host
+      |    include-hostname = true
+      |    metric-name-normalization-strategy = normalize
       |  }
       |}
       |
     """.stripMargin))
 
   implicit val metricKeyGenerator = new SimpleMetricKeyGenerator(system.settings.config) {
-    override def normalizedLocalhostName: String = "localhost_local"
+    override def hostName: String = "localhost_local"
   }
 
   val collectionContext = Kamon(Metrics).buildDefaultCollectionContext
 
   "the StatsDMetricSender" should {
-    "allows to override the hostname" in new UdpListenerFixture {
-      val config = ConfigFactory.parseString(
-        """
-        |kamon {
-        |  statsd {
-        |    simple-metric-key-generator.application = "api"
-        |    simple-metric-key-generator.hostname-override = "kamonhost"
-        |    simple-metric-key-generator.include-hostname = true
-        |  }
-        |}
-        |
-        """.stripMargin)
-      implicit val metricKeyGenerator = new SimpleMetricKeyGenerator(config) {
-        override def normalizedLocalhostName: String = "localhost_local"
-      }
-
-      val testMetricKey = buildMetricKey("trace", "POST: /kamon/example", "elapsed-time")
-      testMetricKey should be(s"api.kamonhost.trace.POST-_kamon_example.elapsed-time")
-    }
-
-    "removes host name when attribute 'include-hostname' is set to false" in new UdpListenerFixture {
-      val config = ConfigFactory.parseString(
-        """
-        |kamon {
-        |  statsd {
-        |    simple-metric-key-generator.application = "api"
-        |    simple-metric-key-generator.include-hostname = false
-        |    simple-metric-key-generator.hostname-override = "none"
-        |  }
-        |}
-        |
-        """.stripMargin)
-      implicit val metricKeyGenerator = new SimpleMetricKeyGenerator(config) {
-        override def normalizedLocalhostName: String = "localhost_local"
-      }
-
-      val testMetricKey = buildMetricKey("trace", "POST: /kamon/example", "elapsed-time")
-      testMetricKey should be(s"api.trace.POST-_kamon_example.elapsed-time")
-    }
-
-    "uses aplication prefix when present" in new UdpListenerFixture {
-      val config = ConfigFactory.parseString(
-        """
-        |kamon {
-        |  statsd {
-        |    simple-metric-key-generator.application = "api"
-        |    simple-metric-key-generator.include-hostname = true
-        |    simple-metric-key-generator.hostname-override = "none"
-        |  }
-        |}
-        |
-        """.stripMargin)
-      implicit val metricKeyGenerator = new SimpleMetricKeyGenerator(config) {
-        override def normalizedLocalhostName: String = "localhost_local"
-      }
-
-      val testMetricKey = buildMetricKey("trace", "POST: /kamon/example", "elapsed-time")
-      testMetricKey should be(s"api.localhost_local.trace.POST-_kamon_example.elapsed-time")
-    }
-
-    "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 = "processing-time"