New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

@hystrixcommand doesn't work for private methods #1020

Closed
shivangshah opened this Issue Dec 18, 2015 · 10 comments

Comments

Projects
None yet
7 participants
@shivangshah

shivangshah commented Dec 18, 2015

I am providing a test case here: https://github.com/shivangshah/test-service.git

It seems that if they method that is called by a service is annotated with @hystrixcommand it works good but class level private methods with @hystrixcommand does not seem to work. Is this by design?

@spencergibb

This comment has been minimized.

Show comment
Hide comment
@spencergibb

spencergibb Dec 18, 2015

Contributor

@shivangshah I believe it is a limitation of aspectj (which is how @HystrixCommand works) rather than @HystrixCommand.

Contributor

spencergibb commented Dec 18, 2015

@shivangshah I believe it is a limitation of aspectj (which is how @HystrixCommand works) rather than @HystrixCommand.

@dmgcodevil

This comment has been minimized.

Show comment
Hide comment
@dmgcodevil

dmgcodevil Dec 18, 2015

Contributor

Hi, I didn't try to use @hystrixcommand together with private methods but
it works with fallback methods even if fallback method annotated with
@hystrixcommand

On Fri, Dec 18, 2015 at 2:32 PM, Shivang Shah notifications@github.com
wrote:

I am providing a test case here:
https://github.com/shivangshah/test-service.git

It seems that if they method that is called by a service is annotated with
@hystrixcommand it works good but class level private methods with
@hystrixcommand does not seem to work. Is this by design?


Reply to this email directly or view it on GitHub
#1020.

Contributor

dmgcodevil commented Dec 18, 2015

Hi, I didn't try to use @hystrixcommand together with private methods but
it works with fallback methods even if fallback method annotated with
@hystrixcommand

On Fri, Dec 18, 2015 at 2:32 PM, Shivang Shah notifications@github.com
wrote:

I am providing a test case here:
https://github.com/shivangshah/test-service.git

It seems that if they method that is called by a service is annotated with
@hystrixcommand it works good but class level private methods with
@hystrixcommand does not seem to work. Is this by design?


Reply to this email directly or view it on GitHub
#1020.

@shivangshah

This comment has been minimized.

Show comment
Hide comment
@shivangshah

shivangshah Dec 18, 2015

@dmgcodevil did you get a chance to look at my test case? I annotated only my private methods which are not called as a part of the root service call and that doesn't work. The original call that is made to the service is the one that can be annotated and than it can fallback to other private methods. BUT if you have a call is made to a service method that doesn't have the annotation and it calls other private methods that have annotations for fallback, that doesn't work.
@spencergibb any documentations on this limitation?

shivangshah commented Dec 18, 2015

@dmgcodevil did you get a chance to look at my test case? I annotated only my private methods which are not called as a part of the root service call and that doesn't work. The original call that is made to the service is the one that can be annotated and than it can fallback to other private methods. BUT if you have a call is made to a service method that doesn't have the annotation and it calls other private methods that have annotations for fallback, that doesn't work.
@spencergibb any documentations on this limitation?

@tianhongbo

This comment has been minimized.

Show comment
Hide comment
@tianhongbo

tianhongbo Mar 21, 2016

I also got the same situation as @shivangshah did. @HystrixCommand does NOT work, if the original call is made to the service method without direct @HystrixCommand. For example: the original call => service() => doSomething(). Here, service() does NOT has @HystrixCommand, but it calls another method doSomething(), which has @HystrixCommand.

public class A {

    public void service() {
        ...
        doSomething();
        ...
    }

    @HystrixCommand
    public void doSomething() {
        ...
    }
}

On the other side, if the original call invokes doSomething() directly, then @HystrixCommand works well.

I believe @spencergibb gave a very good direction for this issue: it is all about AOP rather than @HystrixCommand self. I did a very quick search about AOP, and found a very good Posted on 26th July 2009 by Denis Zhdanov. Please check it out via the link:
Spring AOP top problem #1 - aspects are not applied

tianhongbo commented Mar 21, 2016

I also got the same situation as @shivangshah did. @HystrixCommand does NOT work, if the original call is made to the service method without direct @HystrixCommand. For example: the original call => service() => doSomething(). Here, service() does NOT has @HystrixCommand, but it calls another method doSomething(), which has @HystrixCommand.

public class A {

    public void service() {
        ...
        doSomething();
        ...
    }

    @HystrixCommand
    public void doSomething() {
        ...
    }
}

On the other side, if the original call invokes doSomething() directly, then @HystrixCommand works well.

I believe @spencergibb gave a very good direction for this issue: it is all about AOP rather than @HystrixCommand self. I did a very quick search about AOP, and found a very good Posted on 26th July 2009 by Denis Zhdanov. Please check it out via the link:
Spring AOP top problem #1 - aspects are not applied

@mattrjacobs

This comment has been minimized.

Show comment
Hide comment
@mattrjacobs

mattrjacobs Apr 4, 2016

Contributor

Closing due to inactivity. Please re-open if there's more to discuss

Contributor

mattrjacobs commented Apr 4, 2016

Closing due to inactivity. Please re-open if there's more to discuss

@mattrjacobs mattrjacobs closed this Apr 4, 2016

@dmgcodevil

This comment has been minimized.

Show comment
Hide comment
@dmgcodevil

dmgcodevil Apr 5, 2016

Contributor

I think we have to change point cut in the aspect class, something like
execution(private * *(..))
But also make some tweaks for fallback methods
On 5 Apr 2016 12:39 a.m., "Matt Jacobs" notifications@github.com wrote:

Closing due to inactivity. Please re-open if there's more to discuss


You are receiving this because you were mentioned.
Reply to this email directly or view it on GitHub
#1020 (comment)

Contributor

dmgcodevil commented Apr 5, 2016

I think we have to change point cut in the aspect class, something like
execution(private * *(..))
But also make some tweaks for fallback methods
On 5 Apr 2016 12:39 a.m., "Matt Jacobs" notifications@github.com wrote:

Closing due to inactivity. Please re-open if there's more to discuss


You are receiving this because you were mentioned.
Reply to this email directly or view it on GitHub
#1020 (comment)

@mattrjacobs mattrjacobs reopened this Apr 5, 2016

@jjathman

This comment has been minimized.

Show comment
Hide comment
@jjathman

jjathman Jun 28, 2016

It makes sense why this wouldn't work on private methods, but I would update the documentation then. There are a few examples that use the HystrixCommand annotation on private methods, but that doesn't seem to actually be possible.

Ideally, I would love it if this was possible, but I know with other things in Spring like transactional support it can only be done on public methods.

jjathman commented Jun 28, 2016

It makes sense why this wouldn't work on private methods, but I would update the documentation then. There are a few examples that use the HystrixCommand annotation on private methods, but that doesn't seem to actually be possible.

Ideally, I would love it if this was possible, but I know with other things in Spring like transactional support it can only be done on public methods.

@dmgcodevil

This comment has been minimized.

Show comment
Hide comment
@dmgcodevil

dmgcodevil Jun 29, 2016

Contributor

@jjathman as I already mentioned it's possible with aspectj because it uses CGLIB to create proxies. We need to add corresponding pointcut in the aspect class @Pointcut("execution(private * *(..))"). I believe Spring doesn't allow it because particular aspect doesn't have a poincut for private methods

Contributor

dmgcodevil commented Jun 29, 2016

@jjathman as I already mentioned it's possible with aspectj because it uses CGLIB to create proxies. We need to add corresponding pointcut in the aspect class @Pointcut("execution(private * *(..))"). I believe Spring doesn't allow it because particular aspect doesn't have a poincut for private methods

@davidkarlsen

This comment has been minimized.

Show comment
Hide comment
@davidkarlsen

davidkarlsen Jun 29, 2016

Contributor

http://docs.spring.io/spring/docs/current/spring-framework-reference/html/aop.html

Due to the proxy-based nature of Spring’s AOP framework, protected methods are by definition not intercepted, neither for JDK proxies (where this isn’t applicable) nor for CGLIB proxies (where this is technically possible but not recommendable for AOP purposes). As a consequence, any given pointcut will be matched against public methods only!
If your interception needs include protected/private methods or even constructors, consider the use of Spring-driven native AspectJ weaving instead of Spring’s proxy-based AOP framework. This constitutes a different mode of AOP usage with different characteristics, so be sure to make yourself familiar with weaving first before making a decision.
Contributor

davidkarlsen commented Jun 29, 2016

http://docs.spring.io/spring/docs/current/spring-framework-reference/html/aop.html

Due to the proxy-based nature of Spring’s AOP framework, protected methods are by definition not intercepted, neither for JDK proxies (where this isn’t applicable) nor for CGLIB proxies (where this is technically possible but not recommendable for AOP purposes). As a consequence, any given pointcut will be matched against public methods only!
If your interception needs include protected/private methods or even constructors, consider the use of Spring-driven native AspectJ weaving instead of Spring’s proxy-based AOP framework. This constitutes a different mode of AOP usage with different characteristics, so be sure to make yourself familiar with weaving first before making a decision.
@mattrjacobs

This comment has been minimized.

Show comment
Hide comment
@mattrjacobs

mattrjacobs Oct 27, 2016

Contributor

Closing due to inactivity. Please re-open if there's more to discuss

Contributor

mattrjacobs commented Oct 27, 2016

Closing due to inactivity. Please re-open if there's more to discuss

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment