Merge pull request #39 from derjust/master

Improve MetricRegistryFactory configuration

money-core/src/main/resources/reference.conf

@@ -41,6 +41,8 @@ money {
 
   metrics-registry {
     class-name = "com.comcast.money.metrics.DefaultMetricRegistryFactory"
+    configuration = {
+    }
   }
 
   emitter {

money-core/src/main/scala/com/comcast/money/metrics/MetricRegistryFactory.scala

@@ -17,7 +17,7 @@
 package com.comcast.money.metrics
 
 import com.typesafe.config.Config
-import akka.actor.{ ActorSystem }
+import akka.actor.{ ActorSystem, ExtendedActorSystem, Extension, ExtensionId, ExtensionIdProvider }
 import com.codahale.metrics.{ MetricRegistry, JmxReporter }
 import org.slf4j.LoggerFactory
 import scala.util.{ Try, Success, Failure }
@@ -35,18 +35,21 @@ import scala.util.{ Try, Success, Failure }
  * Note: This trait should be kept as simple as possible so that the resulting interface can also be implemented
  * by a Java client custom factory.
  */
-object MetricRegistryFactory {
-  private val logger = LoggerFactory.getLogger("com.comcast.money.metrics.MetricRegistryFactory")
+class MetricRegistryExtensionImpl(config: Config) extends Extension {
+  private val logger = LoggerFactory.getLogger("com.comcast.money.metrics.MetricRegistryExtensionImpl")
 
-  def metricRegistry(config: Config): MetricRegistry = {
+  val metricRegistry: MetricRegistry = {
     Try({
       // Try to create an instance of the custom factory, configured via 'metricRegistryFactory'
       lazy val realFactory = Class.forName(config.getString("metrics-registry.class-name"))
         .newInstance.asInstanceOf[MetricRegistryFactory]
 
-      // Ask the custom factory for an MetricRegistry - and pass in our configuration so that an implementation
-      // can add their settings in the application.conf, too.
-      realFactory.metricRegistry(config)
+      /* 
+       * Ask the custom factory for an MetricRegistry - and pass in the 
+       * configuration so that an implementation can add their settings in the 
+       * application.conf, too.
+       */
+      realFactory.metricRegistry(config.getConfig("metrics-registry.configuration"))
     }) match {
       case Success(metricRegistry) => metricRegistry
       case Failure(e) => {
@@ -59,6 +62,22 @@ object MetricRegistryFactory {
   }
 }
 
+object MetricRegistryExtension extends ExtensionId[MetricRegistryExtensionImpl] with ExtensionIdProvider {
+  /*
+   * The lookup method is required by ExtensionIdProvider,
+   * so we return ourselves here, this allows us
+   * to configure our extension to be loaded when
+   * the ActorSystem starts up
+   */
+  override def lookup = MetricRegistryExtension
+
+  // This method will be called by Akka to instantiate our Extension
+  override def createExtension(system: ExtendedActorSystem) = new MetricRegistryExtensionImpl(system.settings.config)
+
+  // Java API: retrieve the extension for the given system.
+  override def get(system: ActorSystem): MetricRegistryExtensionImpl = super.get(system)
+}
+
 class DefaultMetricRegistryFactory extends MetricRegistryFactory {
   override def metricRegistry(config: Config): MetricRegistry = {
     val registry = new MetricRegistry

money-core/src/main/scala/com/comcast/money/metrics/MoneyMetrics.scala

@@ -50,7 +50,7 @@ object MoneyMetrics
   // to instantiate our Extension
   override def createExtension(system: ExtendedActorSystem) = {
 
-    val registry = MetricRegistryFactory.metricRegistry(system.settings.config)
+    val registry = MetricRegistryExtension(system).metricRegistry
     // register metrics
     val activeSpans = registry.counter("active.spans")
     val timedOutSpans = registry.counter("timed.out.spans")

money-core/src/main/scala/com/comcast/money/metrics/SpanMetrics.scala

@@ -52,10 +52,9 @@ class SpanMetricsCollector(conf: Config) extends Actor with ActorMaker with Acto
         case Some(spanMetrics) =>
           spanMetrics forward t
         case None =>
-          val metricRegistry = MetricRegistryFactory.metricRegistry(conf)
           val escapedName = t.spanName.replace(' ', '.')
           log.debug(s"Creating span metrics for span $escapedName")
-          makeActor(SpanMetrics.props(escapedName, metricRegistry), escapedName) forward t
+          makeActor(SpanMetrics.props(escapedName, MetricRegistryExtension(context.system).metricRegistry), escapedName) forward t
       }
   }
 }

money-core/src/test/scala/com/comcast/money/metrics/MetricRegistryFactorySpec.scala

@@ -33,7 +33,7 @@ class MockMetricRegistryFactory extends MetricRegistryFactory {
   def metricRegistry(config: Config): MetricRegistry = MockMetricRegistryFactory.mockRegistry
 }
 
-class MetricRegistryFactorySpec extends WordSpecLike with BeforeAndAfter with MockitoSugar {
+class MetricRegistryExtensionSpec extends WordSpecLike with BeforeAndAfter with MockitoSugar {
 
   private val conf = mock[Config]
 
@@ -43,33 +43,33 @@ class MetricRegistryFactorySpec extends WordSpecLike with BeforeAndAfter with Mo
 
         doReturn("com.comcast.money.metrics.DefaultMetricRegistryFactory").when(conf).getString("metrics-registry.class-name")
 
-        val registry = MetricRegistryFactory.metricRegistry(conf)
+        val registry = new MetricRegistryExtensionImpl(conf).metricRegistry
 
         registry shouldNot be(null)
       }
     }
-  }
 
-  "fall back to the DefaultMetricRegistryFactory" should {
-    "when the config is broken" in {
+    "fall back to the DefaultMetricRegistryFactory" should {
+      "when the config is broken" in {
 
-      doReturn("lorem ipsum").when(conf).getString("metrics-registry.class-name")
+        doReturn("lorem ipsum").when(conf).getString("metrics-registry.class-name")
 
-      intercept[ClassNotFoundException] {
-        val registry = MetricRegistryFactory.metricRegistry(conf)
+        intercept[ClassNotFoundException] {
+          val registry = new MetricRegistryExtensionImpl(conf).metricRegistry
+        }
       }
     }
-  }
 
-  "use the MockMetricRegistryFactory" should {
-    "when configured so" in {
+    "use the MockMetricRegistryFactory" should {
+      "when configured so" in {
+        doReturn("com.comcast.money.metrics.MockMetricRegistryFactory").when(conf).getString("metrics-registry.class-name")
 
-      doReturn("com.comcast.money.metrics.MockMetricRegistryFactory").when(conf).getString("metrics-registry.class-name")
+        val ext = new MetricRegistryExtensionImpl(conf)
+        val registry1 = ext.metricRegistry
+        val registry2 = ext.metricRegistry
 
-      val registry1 = MetricRegistryFactory.metricRegistry(conf)
-      val registry2 = MetricRegistryFactory.metricRegistry(conf)
-
-      registry1 should be theSameInstanceAs registry2
+        registry1 should be theSameInstanceAs registry2
+      }
     }
   }
 }