Merge pull request #36 from lloydmeta/feature/cacheless-hystrix-prope…

…rties-strategy

Implement a custom HystrixPropertiesStrategy plugin

.travis.yml

@@ -14,4 +14,4 @@ before_script:
   - psql -c "CREATE USER octoparts_app WITH PASSWORD '';" -U postgres
   - psql -c "GRANT ALL PRIVILEGES ON DATABASE octoparts_test to octoparts_app;" -U postgres
 
-script: "sbt coveralls"
+script: "sbt coveralls test"

app/com/m3/octoparts/Global.scala

@@ -7,10 +7,11 @@ import _root_.controllers.ControllersModule
 import com.kenshoo.play.metrics.MetricsFilter
 import com.m3.octoparts.cache.CacheModule
 import com.m3.octoparts.http.HttpModule
-import com.m3.octoparts.hystrix.{ HystrixMetricsLogger, HystrixModule }
+import com.m3.octoparts.hystrix.{ KeyAndBuilderValuesHystrixPropertiesStrategy, HystrixMetricsLogger, HystrixModule }
 import com.m3.octoparts.logging.PartRequestLogger
 import com.beachape.logging.LTSVLogger
 import com.m3.octoparts.repository.RepositoriesModule
+import com.netflix.hystrix.strategy.HystrixPlugins
 import com.typesafe.config.ConfigFactory
 import com.wordnik.swagger.config.{ ConfigFactory => SwaggerConfigFactory }
 import com.wordnik.swagger.model.ApiInfo
@@ -22,6 +23,7 @@ import scaldi.play.ScaldiSupport
 
 import scala.collection.concurrent.TrieMap
 import scala.concurrent.duration._
+import scala.util.control.NonFatal
 
 object Global extends WithFilters(MetricsFilter) with ScaldiSupport {
 
@@ -80,8 +82,8 @@ object Global extends WithFilters(MetricsFilter) with ScaldiSupport {
   }
 
   override def onStart(app: Application) = {
+    setHystrixPropertiesStrategy(app)
     super.onStart(app)
-
     startPeriodicTasks(app)
   }
 
@@ -97,4 +99,25 @@ object Global extends WithFilters(MetricsFilter) with ScaldiSupport {
       HystrixMetricsLogger.logHystrixMetrics()
     }
   }
+
+  /**
+   * Tries to set the Hystrix properties strategy to [[KeyAndBuilderValuesHystrixPropertiesStrategy]]
+   *
+   * Resist the temptation to do a HystrixPlugins.getInstance().getPropertiesStrategy first to do
+   * checking, as that actually also sets the strategy if it isn't already set.
+   */
+  def setHystrixPropertiesStrategy(app: Application): Unit = {
+    // If it's defined, we don't need to set anything
+    if (sys.props.get("hystrix.plugin.HystrixPropertiesStrategy.implementation").isEmpty) {
+      LTSVLogger.info("-Dhystrix.plugin.HystrixPropertiesStrategy.implementation is not set. Defaulting to" -> "com.m3.octoparts.hystrix.KeyAndBuilderValuesHystrixPropertiesStrategy")
+      try {
+        HystrixPlugins.getInstance().registerPropertiesStrategy(new KeyAndBuilderValuesHystrixPropertiesStrategy)
+      } catch {
+        case NonFatal(e) => {
+          val currentStrategy = HystrixPlugins.getInstance().getPropertiesStrategy.getClass
+          LTSVLogger.info(e, "Current Hystrix Properties Strategy:" -> currentStrategy)
+        }
+      }
+    }
+  }
 }

app/com/m3/octoparts/hystrix/KeyAndBuilderValuesHystrixPropertiesStrategy.scala

@@ -0,0 +1,21 @@
+package com.m3.octoparts.hystrix
+
+import com.fasterxml.jackson.annotation.JsonInclude
+import com.fasterxml.jackson.databind.ObjectMapper
+import com.netflix.hystrix.{ HystrixCommandKey, HystrixCommandProperties }
+import com.netflix.hystrix.strategy.properties.HystrixPropertiesStrategy
+
+/**
+ * Custom [[HystrixPropertiesStrategy]] implementation
+ */
+class KeyAndBuilderValuesHystrixPropertiesStrategy extends HystrixPropertiesStrategy {
+  private val mapper = new ObjectMapper().setSerializationInclusion(JsonInclude.Include.NON_NULL)
+
+  /**
+   * Overriden to return a [[String]] that is a combination of the commandKey name and a JSON string
+   * of the builder values
+   */
+  override def getCommandPropertiesCacheKey(commandKey: HystrixCommandKey, builder: HystrixCommandProperties.Setter): String =
+    s"${commandKey.name()}-${mapper.writeValueAsString(builder)}"
+
+}

test/com/m3/octoparts/hystrix/KeyAndBuilderValuesHystrixPropertiesStrategySpec.scala

@@ -0,0 +1,40 @@
+package com.m3.octoparts.hystrix
+
+import com.netflix.hystrix.strategy.HystrixPlugins
+import com.netflix.hystrix.strategy.properties.HystrixPropertiesFactory
+import com.netflix.hystrix.{ HystrixCommandProperties, HystrixCommandKey }
+import org.scalatest.{ Matchers, FunSpec }
+
+class KeyAndBuilderValuesHystrixPropertiesStrategySpec extends FunSpec with Matchers {
+
+  val subject = new KeyAndBuilderValuesHystrixPropertiesStrategy
+  val commandKey = HystrixCommandKey.Factory.asKey("hello")
+  val commandProps = HystrixCommandProperties.Setter()
+  describe("getCommandPropertiesCacheKey") {
+    it("should return a combination of the commandKey name and commandProps JSON value") {
+      val r1 = subject.getCommandPropertiesCacheKey(commandKey, commandProps)
+      r1 should be("""hello-{}""")
+      val r2 = subject.getCommandPropertiesCacheKey(commandKey, HystrixCommandProperties.Setter().withExecutionIsolationThreadTimeoutInMilliseconds(100))
+      r2 should be("""hello-{"executionIsolationThreadTimeoutInMilliseconds":100}""")
+    }
+  }
+
+  // The following works if this test is run by itself, but
+  describe("after registering with HystrixPlugins") {
+
+    it("should allow HystrixPropertiesFactory.getCommandProperties to instantiate different HystrixCommandProperties for the same command key") {
+      if (HystrixPlugins.getInstance().getPropertiesStrategy.getClass != subject.getClass) {
+        fail("HystrixPlugins.getPropertiesStrategy did not return KeyAndBuilderValuesHystrixPropertiesStrategy")
+      }
+      val properties1 = HystrixPropertiesFactory.getCommandProperties(
+        commandKey,
+        HystrixCommandProperties.Setter().withExecutionIsolationThreadTimeoutInMilliseconds(300))
+      val properties2 = HystrixPropertiesFactory.getCommandProperties(
+        commandKey,
+        HystrixCommandProperties.Setter().withExecutionIsolationThreadTimeoutInMilliseconds(600))
+      properties1.executionIsolationThreadTimeoutInMilliseconds.get should be(300)
+      properties2.executionIsolationThreadTimeoutInMilliseconds.get should be(600)
+    }
+  }
+
+}