Merge pull request #724 from axiak/check-request-log-enabled

Check if the request log is enabled before requiring the context is initialized.
Netflix · Mar 25, 2015 · e4b71da · e4b71da
2 parents 75f5c3b + 099e9e2
commit e4b71da
Show file tree

Hide file tree

Showing 2 changed files with 42 additions and 17 deletions.
diff --git a/hystrix-core/src/main/java/com/netflix/hystrix/AbstractCommand.java b/hystrix-core/src/main/java/com/netflix/hystrix/AbstractCommand.java
@@ -257,21 +257,25 @@ protected AbstractCommand(HystrixCommandGroupKey group, HystrixCommandKey key, H
         /* setup the request cache for this instance */
         this.requestCache = HystrixRequestCache.getInstance(this.commandKey, this.concurrencyStrategy);
 
-        /* store reference to request log regardless of which thread later hits it */
-        if (concurrencyStrategy instanceof HystrixConcurrencyStrategyDefault) {
-            // if we're using the default we support only optionally using a request context
-            if (HystrixRequestContext.isCurrentThreadInitialized()) {
+        if (properties.requestLogEnabled().get()) {
+            /* store reference to request log regardless of which thread later hits it */
+            if (concurrencyStrategy instanceof HystrixConcurrencyStrategyDefault) {
+              // if we're using the default we support only optionally using a request context
+              if (HystrixRequestContext.isCurrentThreadInitialized()) {
                 currentRequestLog = HystrixRequestLog.getCurrentRequest(concurrencyStrategy);
-            } else {
+              } else {
                 currentRequestLog = null;
-            }
-        } else {
-            // if it's a custom strategy it must ensure the context is initialized
-            if (HystrixRequestLog.getCurrentRequest(concurrencyStrategy) != null) {
-                currentRequestLog = HystrixRequestLog.getCurrentRequest(concurrencyStrategy);
+              }
             } else {
+              // if it's a custom strategy it must ensure the context is initialized
+              if (HystrixRequestLog.getCurrentRequest(concurrencyStrategy) != null) {
+                currentRequestLog = HystrixRequestLog.getCurrentRequest(concurrencyStrategy);
+              } else {
                 currentRequestLog = null;
+              }
             }
+        } else {
+            currentRequestLog = null;
         }
     }
 

diff --git a/...rc/test/java/com/netflix/hystrix/strategy/concurrency/HystrixConcurrencyStrategyTest.java b/...rc/test/java/com/netflix/hystrix/strategy/concurrency/HystrixConcurrencyStrategyTest.java
@@ -16,9 +16,13 @@
 package com.netflix.hystrix.strategy.concurrency;
 
 import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertTrue;
 
+import java.lang.IllegalStateException;
 import java.util.concurrent.atomic.AtomicBoolean;
 
+import com.netflix.hystrix.strategy.concurrency.HystrixConcurrencyStrategy;
+import com.netflix.hystrix.strategy.concurrency.HystrixRequestContext;
 import org.junit.After;
 import org.junit.Before;
 import org.junit.Test;
@@ -41,11 +45,7 @@ public void prepareForTest() {
 
     @After
     public void cleanup() {
-        // instead of storing the reference from initialize we'll just get the current state and shutdown
-        if (HystrixRequestContext.getContextForCurrentThread() != null) {
-            // it could have been set NULL by the test
-            HystrixRequestContext.getContextForCurrentThread().shutdown();
-        }
+        shutdownContextIfExists();
 
         // force properties to be clean as well
         ConfigurationManager.getConfigInstance().clear();
@@ -82,7 +82,9 @@ public SimpleCommand() {
 
         @Override
         protected String run() throws Exception {
-            System.out.println("Executing => Commands: " + HystrixRequestLog.getCurrentRequest().getAllExecutedCommands());
+            if (HystrixRequestContext.isCurrentThreadInitialized()) {
+                System.out.println("Executing => Commands: " + HystrixRequestLog.getCurrentRequest().getAllExecutedCommands());
+            }
             return "Hello";
         }
 
@@ -106,7 +108,26 @@ public void call(Throwable throwable) {
         assertEquals(true, isInitialized.get());
     }
 
-    public static class TimeoutCommand extends HystrixCommand<Void> {
+    @Test
+    public void testNoRequestContextOnSimpleConcurencyStrategyWithoutException() throws Exception {
+        shutdownContextIfExists();
+        ConfigurationManager.getConfigInstance().setProperty("hystrix.command.default.requestLog.enabled", "false");
+
+        new SimpleCommand().execute();
+
+        assertTrue("We are able to run the simple command without a context initialization error.", true);
+    }
+
+    private void shutdownContextIfExists() {
+        // instead of storing the reference from initialize we'll just get the current state and shutdown
+        if (HystrixRequestContext.getContextForCurrentThread() != null) {
+            // it could have been set NULL by the test
+            HystrixRequestContext.getContextForCurrentThread().shutdown();
+        }
+    }
+    private static class DummyHystrixConcurrencyStrategy extends HystrixConcurrencyStrategy {}
+
+  public static class TimeoutCommand extends HystrixCommand<Void> {
         static final HystrixCommand.Setter properties = HystrixCommand.Setter
                 .withGroupKey(HystrixCommandGroupKey.Factory.asKey("TimeoutTest"))
                 .andCommandPropertiesDefaults(HystrixCommandProperties.Setter()