Merge pull request #516 from ivantopo/issue#513/tagMetric-not-working-on-unsampled-spans

apply metric tags on spans, even if the span is not sampled, fixes #513

Author: ivantopo

kamon-core-tests/src/test/scala/kamon/trace/SpanMetrics.scala renamed to kamon-core-tests/src/test/scala/kamon/trace/SpanMetricsSpec.scala

@@ -21,9 +21,9 @@ import org.scalatest.{Matchers, WordSpecLike}
 
 import scala.util.control.NoStackTrace
 
-class SpanMetrics extends WordSpecLike with Matchers with MetricInspection with Reconfigure {
+class SpanMetricsSpec extends WordSpecLike with Matchers with MetricInspection with Reconfigure {
 
-  sampleAlways()
+  sampleNever()
 
   "Span Metrics" should {
     "be recorded for successful execution" in {
@@ -57,6 +57,18 @@ class SpanMetrics extends WordSpecLike with Matchers with MetricInspection with
       Span.Metrics.ProcessingTime.valuesForTag("operation") shouldNot contain(operation)
     }
 
+    "allow specifying custom Span metric tags" in {
+      val operation = "span-with-custom-metric-tags"
+      buildSpan(operation)
+        .withMetricTag("custom-metric-tag-on-builder", "value")
+        .start()
+        .tagMetric("custom-metric-tag-on-span", "value")
+        .finish()
+
+      Span.Metrics.ProcessingTime.valuesForTag("custom-metric-tag-on-builder") should contain("value")
+      Span.Metrics.ProcessingTime.valuesForTag("custom-metric-tag-on-span") should contain("value")
+    }
+
     "be recorded if metrics are enabled by calling enableMetrics() on the Span" in {
       val operation = "span-with-re-enabled-metrics"
       buildSpan(operation)
@@ -68,7 +80,7 @@ class SpanMetrics extends WordSpecLike with Matchers with MetricInspection with
       Span.Metrics.ProcessingTime.valuesForTag("operation") should contain(operation)
     }
 
-    "record correctly error latency and count" in {
+    "record error latency and count" in {
       val operation = "span-failure"
       val operationTag = "operation" -> operation
 

kamon-core/src/main/scala/kamon/trace/Span.scala

@@ -123,11 +123,12 @@ object Span {
     }
 
     override def tagMetric(key: String, value: String): Span = synchronized {
-      if(sampled && open) {
-        spanTags = spanTags + (key -> TagValue.String(value))
-
+      if(open) {
         if(collectMetrics)
           customMetricTags = customMetricTags + (key -> value)
+
+        if(sampled)
+          spanTags = spanTags + (key -> TagValue.String(value))
       }
       this
     }
@@ -142,23 +143,29 @@ object Span {
     }
 
     override def addError(error: String): Span = synchronized {
-      if(sampled && open) {
+      if(open) {
         hasError = true
-        spanTags = spanTags ++ Map(
-          "error" -> TagValue.True,
-          "error.object" -> TagValue.String(error)
-        )
+
+        if(sampled) {
+          spanTags = spanTags ++ Map(
+            "error" -> TagValue.True,
+            "error.object" -> TagValue.String(error)
+          )
+        }
       }
       this
     }
 
     override def addError(error: String, throwable: Throwable): Span = synchronized {
-      if(sampled && open) {
+      if(open) {
         hasError = true
-        spanTags = spanTags ++ Map(
-          "error" -> TagValue.True,
-          "error.object" -> TagValue.String(throwable.getMessage())
-        )
+
+        if(sampled) {
+          spanTags = spanTags ++ Map(
+            "error" -> TagValue.True,
+            "error.object" -> TagValue.String(throwable.getMessage())
+          )
+        }
       }
       this
     }