From 5f748c1c74bb71ba4125644b2afec7d4e58bcb9f Mon Sep 17 00:00:00 2001 From: Ivan Topolnjak Date: Sat, 10 Feb 2018 01:32:53 +0100 Subject: apply metric tags on spans, even if the span is not sampled, fixes #513 --- .../src/test/scala/kamon/trace/SpanMetrics.scala | 166 ------------------- .../test/scala/kamon/trace/SpanMetricsSpec.scala | 178 +++++++++++++++++++++ kamon-core/src/main/scala/kamon/trace/Span.scala | 33 ++-- 3 files changed, 198 insertions(+), 179 deletions(-) delete mode 100644 kamon-core-tests/src/test/scala/kamon/trace/SpanMetrics.scala create mode 100644 kamon-core-tests/src/test/scala/kamon/trace/SpanMetricsSpec.scala diff --git a/kamon-core-tests/src/test/scala/kamon/trace/SpanMetrics.scala b/kamon-core-tests/src/test/scala/kamon/trace/SpanMetrics.scala deleted file mode 100644 index 2a04024c..00000000 --- a/kamon-core-tests/src/test/scala/kamon/trace/SpanMetrics.scala +++ /dev/null @@ -1,166 +0,0 @@ -/* ========================================================================================= - * Copyright © 2013-2017 the kamon project - * - * Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file - * except in compliance with the License. You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software distributed under the - * License is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, - * either express or implied. See the License for the specific language governing permissions - * and limitations under the License. - * ========================================================================================= - */ - -package kamon.trace - -import kamon.Kamon.buildSpan -import kamon.testkit.{MetricInspection, Reconfigure} -import org.scalatest.{Matchers, WordSpecLike} - -import scala.util.control.NoStackTrace - -class SpanMetrics extends WordSpecLike with Matchers with MetricInspection with Reconfigure { - - sampleAlways() - - "Span Metrics" should { - "be recorded for successful execution" in { - val operation = "span-success" - val operationTag = "operation" -> operation - - buildSpan(operation) - .start() - .finish() - - val histogram = Span.Metrics.ProcessingTime.refine(Map(operationTag, noErrorTag)) - histogram.distribution().count shouldBe 1 - - val errorHistogram = Span.Metrics.ProcessingTime.refine(Map(operationTag, errorTag)) - errorHistogram.distribution().count shouldBe 0 - - } - - "not be recorded when disableMetrics() is called on the SpanBuilder or the Span" in { - val operation = "span-with-disabled-metrics" - buildSpan(operation) - .start() - .disableMetrics() - .finish() - - buildSpan(operation) - .disableMetrics() - .start() - .finish() - - Span.Metrics.ProcessingTime.valuesForTag("operation") shouldNot contain(operation) - } - - "be recorded if metrics are enabled by calling enableMetrics() on the Span" in { - val operation = "span-with-re-enabled-metrics" - buildSpan(operation) - .start() - .disableMetrics() - .enableMetrics() - .finish() - - Span.Metrics.ProcessingTime.valuesForTag("operation") should contain(operation) - } - - "record correctly error latency and count" in { - val operation = "span-failure" - val operationTag = "operation" -> operation - - buildSpan(operation) - .start() - .addError("Terrible Error") - .finish() - - buildSpan(operation) - .start() - .addError("Terrible Error with Throwable", new Throwable with NoStackTrace) - .finish() - - val histogram = Span.Metrics.ProcessingTime.refine(Map(operationTag, noErrorTag)) - histogram.distribution().count shouldBe 0 - - val errorHistogram = Span.Metrics.ProcessingTime.refine(Map(operationTag, errorTag)) - errorHistogram.distribution().count shouldBe 2 - } - - "add a parentOperation tag to the metrics if span metrics scoping is enabled" in { - val parent = buildSpan("parent").start() - val parentOperationTag = "parentOperation" -> "parent" - - val operation = "span-with-parent" - val operationTag = "operation" -> operation - - buildSpan(operation) - .asChildOf(parent) - .start() - .finish() - - buildSpan(operation) - .asChildOf(parent) - .start() - .addError("Terrible Error") - .finish() - - buildSpan(operation) - .asChildOf(parent) - .start() - .addError("Terrible Error with Throwable", new Throwable with NoStackTrace) - .finish() - - val histogram = Span.Metrics.ProcessingTime.refine(Map(operationTag, noErrorTag, parentOperationTag)) - histogram.distribution().count shouldBe 1 - - val errorHistogram = Span.Metrics.ProcessingTime.refine(Map(operationTag, errorTag, parentOperationTag)) - errorHistogram.distribution().count shouldBe 2 - } - - "not add any parentOperation tag to the metrics if span metrics scoping is disabled" in withoutSpanScopingEnabled { - val parent = buildSpan("parent").start() - val parentOperationTag = "parentOperation" -> "parent" - - val operation = "span-with-parent" - val operationTag = "operation" -> operation - - buildSpan(operation) - .asChildOf(parent) - .start() - .finish() - - buildSpan(operation) - .asChildOf(parent) - .start() - .addError("Terrible Error") - .finish() - - buildSpan(operation) - .asChildOf(parent) - .start() - .addError("Terrible Error with Throwable", new Throwable with NoStackTrace) - .finish() - - val histogram = Span.Metrics.ProcessingTime.refine(Map(operationTag, noErrorTag, parentOperationTag)) - histogram.distribution().count shouldBe 0 - - val errorHistogram = Span.Metrics.ProcessingTime.refine(Map(operationTag, errorTag, parentOperationTag)) - errorHistogram.distribution().count shouldBe 0 - } - } - - val errorTag = "error" -> "true" - val noErrorTag = "error" -> "false" - - private def withoutSpanScopingEnabled[T](f: => T): T = { - disableSpanMetricScoping() - val evaluated = f - enableSpanMetricScoping() - evaluated - } -} - - diff --git a/kamon-core-tests/src/test/scala/kamon/trace/SpanMetricsSpec.scala b/kamon-core-tests/src/test/scala/kamon/trace/SpanMetricsSpec.scala new file mode 100644 index 00000000..1dfe24a3 --- /dev/null +++ b/kamon-core-tests/src/test/scala/kamon/trace/SpanMetricsSpec.scala @@ -0,0 +1,178 @@ +/* ========================================================================================= + * Copyright © 2013-2017 the kamon project + * + * Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file + * except in compliance with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software distributed under the + * License is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, + * either express or implied. See the License for the specific language governing permissions + * and limitations under the License. + * ========================================================================================= + */ + +package kamon.trace + +import kamon.Kamon.buildSpan +import kamon.testkit.{MetricInspection, Reconfigure} +import org.scalatest.{Matchers, WordSpecLike} + +import scala.util.control.NoStackTrace + +class SpanMetricsSpec extends WordSpecLike with Matchers with MetricInspection with Reconfigure { + + sampleNever() + + "Span Metrics" should { + "be recorded for successful execution" in { + val operation = "span-success" + val operationTag = "operation" -> operation + + buildSpan(operation) + .start() + .finish() + + val histogram = Span.Metrics.ProcessingTime.refine(Map(operationTag, noErrorTag)) + histogram.distribution().count shouldBe 1 + + val errorHistogram = Span.Metrics.ProcessingTime.refine(Map(operationTag, errorTag)) + errorHistogram.distribution().count shouldBe 0 + + } + + "not be recorded when disableMetrics() is called on the SpanBuilder or the Span" in { + val operation = "span-with-disabled-metrics" + buildSpan(operation) + .start() + .disableMetrics() + .finish() + + buildSpan(operation) + .disableMetrics() + .start() + .finish() + + 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) + .start() + .disableMetrics() + .enableMetrics() + .finish() + + Span.Metrics.ProcessingTime.valuesForTag("operation") should contain(operation) + } + + "record error latency and count" in { + val operation = "span-failure" + val operationTag = "operation" -> operation + + buildSpan(operation) + .start() + .addError("Terrible Error") + .finish() + + buildSpan(operation) + .start() + .addError("Terrible Error with Throwable", new Throwable with NoStackTrace) + .finish() + + val histogram = Span.Metrics.ProcessingTime.refine(Map(operationTag, noErrorTag)) + histogram.distribution().count shouldBe 0 + + val errorHistogram = Span.Metrics.ProcessingTime.refine(Map(operationTag, errorTag)) + errorHistogram.distribution().count shouldBe 2 + } + + "add a parentOperation tag to the metrics if span metrics scoping is enabled" in { + val parent = buildSpan("parent").start() + val parentOperationTag = "parentOperation" -> "parent" + + val operation = "span-with-parent" + val operationTag = "operation" -> operation + + buildSpan(operation) + .asChildOf(parent) + .start() + .finish() + + buildSpan(operation) + .asChildOf(parent) + .start() + .addError("Terrible Error") + .finish() + + buildSpan(operation) + .asChildOf(parent) + .start() + .addError("Terrible Error with Throwable", new Throwable with NoStackTrace) + .finish() + + val histogram = Span.Metrics.ProcessingTime.refine(Map(operationTag, noErrorTag, parentOperationTag)) + histogram.distribution().count shouldBe 1 + + val errorHistogram = Span.Metrics.ProcessingTime.refine(Map(operationTag, errorTag, parentOperationTag)) + errorHistogram.distribution().count shouldBe 2 + } + + "not add any parentOperation tag to the metrics if span metrics scoping is disabled" in withoutSpanScopingEnabled { + val parent = buildSpan("parent").start() + val parentOperationTag = "parentOperation" -> "parent" + + val operation = "span-with-parent" + val operationTag = "operation" -> operation + + buildSpan(operation) + .asChildOf(parent) + .start() + .finish() + + buildSpan(operation) + .asChildOf(parent) + .start() + .addError("Terrible Error") + .finish() + + buildSpan(operation) + .asChildOf(parent) + .start() + .addError("Terrible Error with Throwable", new Throwable with NoStackTrace) + .finish() + + val histogram = Span.Metrics.ProcessingTime.refine(Map(operationTag, noErrorTag, parentOperationTag)) + histogram.distribution().count shouldBe 0 + + val errorHistogram = Span.Metrics.ProcessingTime.refine(Map(operationTag, errorTag, parentOperationTag)) + errorHistogram.distribution().count shouldBe 0 + } + } + + val errorTag = "error" -> "true" + val noErrorTag = "error" -> "false" + + private def withoutSpanScopingEnabled[T](f: => T): T = { + disableSpanMetricScoping() + val evaluated = f + enableSpanMetricScoping() + evaluated + } +} + + diff --git a/kamon-core/src/main/scala/kamon/trace/Span.scala b/kamon-core/src/main/scala/kamon/trace/Span.scala index 690efeb4..9778373a 100644 --- a/kamon-core/src/main/scala/kamon/trace/Span.scala +++ b/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 } -- cgit v1.2.3