Skip to content

Commit

Permalink
show the right error message when metric redefinitions happen, fixes k…
Browse files Browse the repository at this point in the history 
…amon-io#608
  • Loading branch information
@ivantopo
ivantopo committed Oct 16, 2019
1 parent 03539a9 commit 4c31ba9
Show file tree
Hide file tree
Showing 2 changed files with 71 additions and 15 deletions.
35 changes: 35 additions & 0 deletions kamon-core-tests/src/test/scala/kamon/metric/MetricLookupSpec.scala
View file
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,38 @@ class MetricLookupSpec extends WordSpec with Matchers {
val rangeSamplerTwo = Kamon.rangeSampler("range-sampler-lookup")
rangeSamplerOne shouldBe theSameInstanceAs(rangeSamplerTwo)
}

"throw an IllegalArgumentException when a metric redefinition is attempted" in {
the[IllegalArgumentException] thrownBy {
Kamon.counter("original-counter")
Kamon.gauge("original-counter")

} should have message(redefinitionError("original-counter", "counter", "gauge"))

the[IllegalArgumentException] thrownBy {
Kamon.counter("original-counter")
Kamon.histogram("original-counter")

} should have message(redefinitionError("original-counter", "counter", "histogram"))

the[IllegalArgumentException] thrownBy {
Kamon.counter("original-counter")
Kamon.rangeSampler("original-counter")

} should have message(redefinitionError("original-counter", "counter", "rangeSampler"))

the[IllegalArgumentException] thrownBy {
Kamon.counter("original-counter")
Kamon.timer("original-counter")

} should have message(redefinitionError("original-counter", "counter", "timer"))

the[IllegalArgumentException] thrownBy {
Kamon.histogram("original-histogram")
Kamon.counter("original-histogram")

} should have message(redefinitionError("original-histogram", "histogram", "counter"))
}
}

"refine a metric with tags and" should {
Expand Down Expand Up @@ -88,4 +120,7 @@ class MetricLookupSpec extends WordSpec with Matchers {
}
}
}

def redefinitionError(name: String, currentType: String, newType: String): String =
s"Cannot redefine metric [$name] as a [$newType], it was already registered as a [$currentType]"
}
51 changes: 36 additions & 15 deletions kamon-core/src/main/scala/kamon/metric/MetricRegistry.scala
View file
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ import kamon.util.Clock
import org.slf4j.LoggerFactory

import scala.collection.concurrent.TrieMap
import scala.reflect.ClassTag

/**
* Handles creation and snapshotting of metrics. If a metric is created twice, the very same instance will be returned.
Expand All @@ -47,10 +48,10 @@ class MetricRegistry(config: Config, scheduler: ScheduledExecutorService, clock:
def counter(name: String, description: Option[String], unit: Option[MeasurementUnit], autoUpdateInterval: Option[Duration]):
Metric.Counter = {

val metric = _metrics.atomicGetOrElseUpdate(name, _factory.counter(name, description, unit, autoUpdateInterval))
.asInstanceOf[Metric.Counter]
val metric = validateInstrumentType[Metric.Counter] {
_metrics.atomicGetOrElseUpdate(name, _factory.counter(name, description, unit, autoUpdateInterval))
} (name, Instrument.Type.Counter)

checkInstrumentType(name, Instrument.Type.Counter, metric)
checkDescription(metric.name, metric.description, description)
checkUnit(metric.name, metric.settings.unit, unit)
checkAutoUpdate(metric.name, metric.settings.autoUpdateInterval, autoUpdateInterval)
Expand All @@ -63,10 +64,10 @@ class MetricRegistry(config: Config, scheduler: ScheduledExecutorService, clock:
def gauge(name: String, description: Option[String], unit: Option[MeasurementUnit], autoUpdateInterval: Option[Duration]):
Metric.Gauge = {

val metric = _metrics.atomicGetOrElseUpdate(name, _factory.gauge(name, description, unit, autoUpdateInterval))
.asInstanceOf[Metric.Gauge]
val metric = validateInstrumentType[Metric.Gauge] {
_metrics.atomicGetOrElseUpdate(name, _factory.gauge(name, description, unit, autoUpdateInterval))
} (name, Instrument.Type.Gauge)

checkInstrumentType(name, Instrument.Type.Gauge, metric)
checkDescription(metric.name, metric.description, description)
checkUnit(metric.name, metric.settings.unit, unit)
checkAutoUpdate(metric.name, metric.settings.autoUpdateInterval, autoUpdateInterval)
Expand All @@ -79,10 +80,10 @@ class MetricRegistry(config: Config, scheduler: ScheduledExecutorService, clock:
def histogram(name: String, description: Option[String], unit: Option[MeasurementUnit], dynamicRange: Option[DynamicRange],
autoUpdateInterval: Option[Duration]): Metric.Histogram = {

val metric = _metrics.atomicGetOrElseUpdate(name, _factory.histogram(name, description, unit, dynamicRange,
autoUpdateInterval)).asInstanceOf[Metric.Histogram]
val metric = validateInstrumentType[Metric.Histogram] {
_metrics.atomicGetOrElseUpdate(name, _factory.histogram(name, description, unit, dynamicRange, autoUpdateInterval))
} (name, Instrument.Type.Histogram)

checkInstrumentType(name, Instrument.Type.Histogram, metric)
checkDescription(metric.name, metric.description, description)
checkUnit(metric.name, metric.settings.unit, unit)
checkDynamicRange(metric.name, metric.settings.dynamicRange, dynamicRange)
Expand All @@ -95,10 +96,11 @@ class MetricRegistry(config: Config, scheduler: ScheduledExecutorService, clock:
*/
def timer(name: String, description: Option[String], dynamicRange: Option[DynamicRange], autoUpdateInterval: Option[Duration]): Metric.Timer = {

val metric = _metrics.atomicGetOrElseUpdate(name, _factory.timer(name, description, Some(MeasurementUnit.time.nanoseconds),
dynamicRange, autoUpdateInterval)).asInstanceOf[Metric.Timer]
val metric = validateInstrumentType[Metric.Timer] {
_metrics.atomicGetOrElseUpdate(name, _factory.timer(name, description, Some(MeasurementUnit.time.nanoseconds),
dynamicRange, autoUpdateInterval))
} (name, Instrument.Type.Timer)

checkInstrumentType(name, Instrument.Type.Timer, metric)
checkDescription(metric.name, metric.description, description)
checkDynamicRange(metric.name, metric.settings.dynamicRange, dynamicRange)
checkAutoUpdate(metric.name, metric.settings.autoUpdateInterval, autoUpdateInterval)
Expand All @@ -111,10 +113,10 @@ class MetricRegistry(config: Config, scheduler: ScheduledExecutorService, clock:
def rangeSampler(name: String, description: Option[String], unit: Option[MeasurementUnit], dynamicRange: Option[DynamicRange],
autoUpdateInterval: Option[Duration]): Metric.RangeSampler = {

val metric = _metrics.atomicGetOrElseUpdate(name, _factory.rangeSampler(name, description, unit, dynamicRange,
autoUpdateInterval)).asInstanceOf[Metric.RangeSampler]
val metric = validateInstrumentType[Metric.RangeSampler] {
_metrics.atomicGetOrElseUpdate(name, _factory.rangeSampler(name, description, unit, dynamicRange, autoUpdateInterval))
} (name, Instrument.Type.RangeSampler)

checkInstrumentType(name, Instrument.Type.RangeSampler, metric)
checkDescription(metric.name, metric.description, description)
checkUnit(metric.name, metric.settings.unit, unit)
checkDynamicRange(metric.name, metric.settings.dynamicRange, dynamicRange)
Expand All @@ -129,6 +131,25 @@ class MetricRegistry(config: Config, scheduler: ScheduledExecutorService, clock:
_factory = MetricFactory.from(newConfig, scheduler, clock)
}

private def validateInstrumentType[T](metric: => Metric[_, _])(name: String, instrumentType: Instrument.Type): T = {
val lookedUpMetric = metric
if(instrumentType.implementation.isInstance(lookedUpMetric))
lookedUpMetric.asInstanceOf[T]
else
throw new IllegalArgumentException(
s"Cannot redefine metric [$name] as a [${instrumentType.name}], it was already " +
s"registered as a [${implementationName(metric)}]"
)
}

private def implementationName(metric: Metric[_, _]): String = metric match {
case _: Metric.Counter => Instrument.Type.Counter.name
case _: Metric.Gauge => Instrument.Type.Gauge.name
case _: Metric.Histogram => Instrument.Type.Histogram.name
case _: Metric.RangeSampler => Instrument.Type.RangeSampler.name
case _: Metric.Timer => Instrument.Type.Timer.name
}

private def checkInstrumentType(name: String, instrumentType: Instrument.Type, metric: Metric[_, _]): Unit =
if(!instrumentType.implementation.isInstance(metric))
sys.error(s"Cannot redefine metric [$name] as a [${instrumentType.name}], it was already registered as a [${metric.getClass.getName}]")
Expand Down

0 comments on commit 4c31ba9

Please sign in to comment.