[GitHub] ScienJus opened a new issue #1892: Custom HystrixConcurrencyStrategy doesn't work when using hystrix plugin.

[GitBox <gi...@apache.org>]

ScienJus opened a new issue #1892: Custom HystrixConcurrencyStrategy doesn't work when using hystrix plugin.
URL: https://github.com/apache/incubator-skywalking/issues/1892
 
 
   Please answer these questions before submitting your issue.
   
   - Why do you submit this issue?
   - [ ] Question or discussion
   - [x] Bug
   - [ ] Requirement
   - [ ] Feature or performance improvement
   
   ___
   ### Bug
   - Which version of SkyWalking, OS and JRE?
   
   SkyWalking 6.0.0-alpha, hystrix-1.x-plugin 6.0.0-alpha
   
   - What happen?
   
   In spring cloud, people like to programmatically set HystrixConcurrencyStrategy through a delegate pattern, let me just give two examples:
   
   [SecurityContextConcurrencyStrategy in spring-cloud-netflix](https://github.com/spring-cloud/spring-cloud-netflix/blob/6859b7a83588f33c8a946de40cfbab0917de3d05/spring-cloud-netflix-hystrix/src/main/java/org/springframework/cloud/netflix/hystrix/security/HystrixSecurityAutoConfiguration.java#L48-L69)
   
   [SleuthHystrixConcurrencyStrategy in spring-cloud-sleuth](https://github.com/spring-cloud/spring-cloud-sleuth/blob/99d38afc6993685653c102216f964fa1dec8c95d/spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/instrument/hystrix/SleuthHystrixConcurrencyStrategy.java#L62-L92)
   
   But hystrix plugin will make them unable to work because the result of `HystrixPlugins#getConcurrencyStrategy` is cached in the HystrixConcurrencyStrategyInterceptor, although after that other HystrixConcurrencyStrategy will reset and register itself. Then HystrixConcurrencyStrategyInterceptor will return the wrong result.
   
   We have several ways to fix this problem:
   
   1. Believe that all HystrixConcurrencyStrategy adheres to the delegate pattern, only need to wrap and register SWHystrixConcurrencyStrategyWrapper in the first call.
   
   ```
   @Override
   public Object afterMethod(EnhancedInstance objInst, Method method, Object[] allArguments, Class<?>[] argumentsTypes,
                             Object ret) throws Throwable {
       SWHystrixPluginsWrapperCache wrapperCache = (SWHystrixPluginsWrapperCache) objInst.getSkyWalkingDynamicField();
       if (wrapperCache == null || wrapperCache.getSwHystrixConcurrencyStrategyWrapper() == null) {
           synchronized (objInst) {
               if (wrapperCache == null) {
                   wrapperCache = new SWHystrixPluginsWrapperCache();
                   objInst.setSkyWalkingDynamicField(wrapperCache);
               }
               if (wrapperCache.getSwHystrixConcurrencyStrategyWrapper() == null) {
                   // Return and register wrapper only for the first time
                   // try to believe that all other strategies will use the their delegates
                   SWHystrixConcurrencyStrategyWrapper strategy = new SWHystrixConcurrencyStrategyWrapper((HystrixConcurrencyStrategy) ret);
                   wrapperCache.setSwHystrixConcurrencyStrategyWrapper(strategy);
   
                   registerSWHystrixConcurrencyStrategyWrapper(strategy);
   
                   return strategy;
               }
           }
       }
       return ret;
   }
   
   private void registerSWHystrixConcurrencyStrategyWrapper(SWHystrixConcurrencyStrategyWrapper swHystrixConcurrencyStrategyWrapper) {
       // Copy from Spring Cloud Sleuth
       HystrixCommandExecutionHook commandExecutionHook = HystrixPlugins
               .getInstance().getCommandExecutionHook();
       HystrixEventNotifier eventNotifier = HystrixPlugins.getInstance()
               .getEventNotifier();
       HystrixMetricsPublisher metricsPublisher = HystrixPlugins.getInstance()
               .getMetricsPublisher();
       HystrixPropertiesStrategy propertiesStrategy = HystrixPlugins.getInstance()
               .getPropertiesStrategy();
       HystrixPlugins.reset();
       HystrixPlugins.getInstance().registerConcurrencyStrategy(swHystrixConcurrencyStrategyWrapper);
       HystrixPlugins.getInstance()
               .registerCommandExecutionHook(commandExecutionHook);
       HystrixPlugins.getInstance().registerEventNotifier(eventNotifier);
       HystrixPlugins.getInstance().registerMetricsPublisher(metricsPublisher);
       HystrixPlugins.getInstance().registerPropertiesStrategy(propertiesStrategy);
   }
   ```
   
   2. If we don't trust the delegate pattern and must make
   the SWHystrixConcurrencyStrategy exists in the delegate chain, it will be difficult, not only to intercept get/reset/register, but also there is no good way to confirm if a class is already on a delegate chain.
   
   What do you think? Do you need a demo project to reproduce this issue?

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services