Added missing methods to override in SleuthHystrixConcurrencyStrategy
according to issue #612: > When 2 or more customized HystrixConcurrencyStrategy are registered (e.g SleuthHystrixConcurrencyStrategy and SecurityContextConcurrencyStrategy) depending on the order of registration the overrides from SecurityContextConcurrencyStrategy might get ignored. I think the override and delegation should be made all methods of HystrixConcurrencyStrategy otherwise the delegation will possibly be incomplete. fixes #612
This commit is contained in:
@@ -16,21 +16,28 @@
|
||||
|
||||
package org.springframework.cloud.sleuth.instrument.hystrix;
|
||||
|
||||
import java.lang.invoke.MethodHandles;
|
||||
import java.util.concurrent.Callable;
|
||||
|
||||
import com.netflix.hystrix.HystrixThreadPoolKey;
|
||||
import com.netflix.hystrix.HystrixThreadPoolProperties;
|
||||
import com.netflix.hystrix.strategy.HystrixPlugins;
|
||||
import com.netflix.hystrix.strategy.concurrency.HystrixConcurrencyStrategy;
|
||||
import com.netflix.hystrix.strategy.concurrency.HystrixRequestVariable;
|
||||
import com.netflix.hystrix.strategy.concurrency.HystrixRequestVariableLifecycle;
|
||||
import com.netflix.hystrix.strategy.eventnotifier.HystrixEventNotifier;
|
||||
import com.netflix.hystrix.strategy.executionhook.HystrixCommandExecutionHook;
|
||||
import com.netflix.hystrix.strategy.metrics.HystrixMetricsPublisher;
|
||||
import com.netflix.hystrix.strategy.properties.HystrixPropertiesStrategy;
|
||||
import com.netflix.hystrix.strategy.properties.HystrixProperty;
|
||||
import org.apache.commons.logging.Log;
|
||||
import org.apache.commons.logging.LogFactory;
|
||||
import org.springframework.cloud.sleuth.Span;
|
||||
import org.springframework.cloud.sleuth.TraceKeys;
|
||||
import org.springframework.cloud.sleuth.Tracer;
|
||||
|
||||
import com.netflix.hystrix.strategy.HystrixPlugins;
|
||||
import com.netflix.hystrix.strategy.concurrency.HystrixConcurrencyStrategy;
|
||||
import com.netflix.hystrix.strategy.eventnotifier.HystrixEventNotifier;
|
||||
import com.netflix.hystrix.strategy.executionhook.HystrixCommandExecutionHook;
|
||||
import com.netflix.hystrix.strategy.metrics.HystrixMetricsPublisher;
|
||||
import com.netflix.hystrix.strategy.properties.HystrixPropertiesStrategy;
|
||||
import java.lang.invoke.MethodHandles;
|
||||
import java.util.concurrent.BlockingQueue;
|
||||
import java.util.concurrent.Callable;
|
||||
import java.util.concurrent.ThreadPoolExecutor;
|
||||
import java.util.concurrent.TimeUnit;
|
||||
|
||||
/**
|
||||
* A {@link HystrixConcurrencyStrategy} that wraps a {@link Callable} in a
|
||||
@@ -107,6 +114,33 @@ public class SleuthHystrixConcurrencyStrategy extends HystrixConcurrencyStrategy
|
||||
return new HystrixTraceCallable<>(this.tracer, this.traceKeys, wrappedCallable);
|
||||
}
|
||||
|
||||
@Override
|
||||
public ThreadPoolExecutor getThreadPool(HystrixThreadPoolKey threadPoolKey,
|
||||
HystrixProperty<Integer> corePoolSize,
|
||||
HystrixProperty<Integer> maximumPoolSize,
|
||||
HystrixProperty<Integer> keepAliveTime, TimeUnit unit,
|
||||
BlockingQueue<Runnable> workQueue) {
|
||||
return this.delegate.getThreadPool(threadPoolKey, corePoolSize, maximumPoolSize,
|
||||
keepAliveTime, unit, workQueue);
|
||||
}
|
||||
|
||||
@Override
|
||||
public ThreadPoolExecutor getThreadPool(HystrixThreadPoolKey threadPoolKey,
|
||||
HystrixThreadPoolProperties threadPoolProperties) {
|
||||
return this.delegate.getThreadPool(threadPoolKey, threadPoolProperties);
|
||||
}
|
||||
|
||||
@Override
|
||||
public BlockingQueue<Runnable> getBlockingQueue(int maxQueueSize) {
|
||||
return this.delegate.getBlockingQueue(maxQueueSize);
|
||||
}
|
||||
|
||||
@Override
|
||||
public <T> HystrixRequestVariable<T> getRequestVariable(
|
||||
HystrixRequestVariableLifecycle<T> rv) {
|
||||
return this.delegate.getRequestVariable(rv);
|
||||
}
|
||||
|
||||
// Visible for testing
|
||||
static class HystrixTraceCallable<S> implements Callable<S> {
|
||||
|
||||
|
||||
@@ -16,12 +16,21 @@
|
||||
|
||||
package org.springframework.cloud.sleuth.instrument.hystrix;
|
||||
|
||||
import java.util.Random;
|
||||
import java.util.concurrent.Callable;
|
||||
|
||||
import com.netflix.hystrix.HystrixThreadPoolKey;
|
||||
import com.netflix.hystrix.HystrixThreadPoolProperties;
|
||||
import com.netflix.hystrix.strategy.HystrixPlugins;
|
||||
import com.netflix.hystrix.strategy.concurrency.HystrixConcurrencyStrategy;
|
||||
import com.netflix.hystrix.strategy.concurrency.HystrixLifecycleForwardingRequestVariable;
|
||||
import com.netflix.hystrix.strategy.eventnotifier.HystrixEventNotifier;
|
||||
import com.netflix.hystrix.strategy.executionhook.HystrixCommandExecutionHook;
|
||||
import com.netflix.hystrix.strategy.metrics.HystrixMetricsPublisher;
|
||||
import com.netflix.hystrix.strategy.properties.HystrixPropertiesStrategy;
|
||||
import com.netflix.hystrix.strategy.properties.HystrixProperty;
|
||||
import org.junit.After;
|
||||
import org.junit.Before;
|
||||
import org.junit.Test;
|
||||
import org.mockito.BDDMockito;
|
||||
import org.mockito.Mockito;
|
||||
import org.springframework.cloud.sleuth.DefaultSpanNamer;
|
||||
import org.springframework.cloud.sleuth.Span;
|
||||
import org.springframework.cloud.sleuth.TraceKeys;
|
||||
@@ -33,12 +42,10 @@ import org.springframework.cloud.sleuth.trace.DefaultTracer;
|
||||
import org.springframework.cloud.sleuth.util.ArrayListSpanAccumulator;
|
||||
import org.springframework.cloud.sleuth.util.ExceptionUtils;
|
||||
|
||||
import com.netflix.hystrix.strategy.HystrixPlugins;
|
||||
import com.netflix.hystrix.strategy.concurrency.HystrixConcurrencyStrategy;
|
||||
import com.netflix.hystrix.strategy.eventnotifier.HystrixEventNotifier;
|
||||
import com.netflix.hystrix.strategy.executionhook.HystrixCommandExecutionHook;
|
||||
import com.netflix.hystrix.strategy.metrics.HystrixMetricsPublisher;
|
||||
import com.netflix.hystrix.strategy.properties.HystrixPropertiesStrategy;
|
||||
import java.util.Random;
|
||||
import java.util.concurrent.BlockingQueue;
|
||||
import java.util.concurrent.Callable;
|
||||
import java.util.concurrent.TimeUnit;
|
||||
|
||||
import static org.springframework.cloud.sleuth.assertions.SleuthAssertions.then;
|
||||
/**
|
||||
@@ -154,6 +161,35 @@ public class SleuthHystrixConcurrencyStrategyTest {
|
||||
.hasATag(asyncKey, "bar");
|
||||
}
|
||||
|
||||
@Test
|
||||
public void should_delegate_work_to_custom_hystrix_concurrency_strategy()
|
||||
throws Exception {
|
||||
HystrixConcurrencyStrategy strategy = Mockito.mock(HystrixConcurrencyStrategy.class);
|
||||
HystrixPlugins.getInstance().registerConcurrencyStrategy(strategy);
|
||||
SleuthHystrixConcurrencyStrategy sleuthStrategy = new SleuthHystrixConcurrencyStrategy(
|
||||
this.tracer, this.traceKeys);
|
||||
|
||||
sleuthStrategy.wrapCallable(() -> "foo");
|
||||
sleuthStrategy.getThreadPool(HystrixThreadPoolKey.Factory.asKey(""), Mockito.mock(
|
||||
HystrixThreadPoolProperties.class));
|
||||
sleuthStrategy.getThreadPool(HystrixThreadPoolKey.Factory.asKey(""),
|
||||
Mockito.mock(HystrixProperty.class), Mockito.mock(HystrixProperty.class),
|
||||
Mockito.mock(HystrixProperty.class), TimeUnit.DAYS, Mockito.mock(
|
||||
BlockingQueue.class));
|
||||
sleuthStrategy.getBlockingQueue(10);
|
||||
sleuthStrategy.getRequestVariable(Mockito.mock(
|
||||
HystrixLifecycleForwardingRequestVariable.class));
|
||||
|
||||
BDDMockito.then(strategy).should().wrapCallable((Callable) BDDMockito.any());
|
||||
BDDMockito.then(strategy).should().getThreadPool(BDDMockito.any(), BDDMockito.any());
|
||||
BDDMockito.then(strategy).should().getThreadPool(BDDMockito.any(), BDDMockito.any(),
|
||||
BDDMockito.any(), BDDMockito.any(), BDDMockito.any(), BDDMockito.any());
|
||||
BDDMockito.then(strategy).should().getThreadPool(BDDMockito.any(), BDDMockito.any(),
|
||||
BDDMockito.any(), BDDMockito.any(), BDDMockito.any(), BDDMockito.any());
|
||||
BDDMockito.then(strategy).should().getBlockingQueue(10);
|
||||
BDDMockito.then(strategy).should().getRequestVariable(BDDMockito.any());
|
||||
}
|
||||
|
||||
static class MyHystrixCommandExecutionHook extends HystrixCommandExecutionHook {}
|
||||
@SuppressWarnings("unchecked")
|
||||
static class MyHystrixConcurrencyStrategy extends HystrixConcurrencyStrategy {
|
||||
|
||||
Reference in New Issue
Block a user