Comcast / jrugged

A Java libary of robustness design patterns
Apache License 2.0
266 stars 93 forks source link

Allow specifying order of CircuitBreakerAspect and PerformanceMonitorAspect #30

Open lite2073 opened 10 years ago

lite2073 commented 10 years ago

When @CircuitBreaker and @PerformanceMonitor are used together (or with other aspects), we need to impose a certain execution order. Currently, both aspects are unordered and an order cannot be specified.

A simple way to do this is to have them implement Ordered interface and also provide a setter for customization.

joercampbell commented 9 years ago

Currently Ordered is a Spring-ism, where as the Aspects in question are making use of aspect J. I believe that it should be straight forward to set the apsects to always have the performance monitor take precedence (to be inside) a circuit breaker when both are set onto a method. This would mean that the performance would only count the performance of the wrapped thing and not the performance of an open circuit on the wrapped thing. Does this make sense and fulfill the need?

I am asking because in order to do what you ask i would have to move classes between packages as the aspects you are referring to currently do not have a dependance on Spring - but what you suggest would bring that about.

lite2073 commented 9 years ago

It is okay if we don't want to bring in Spring dependencies. I'm not sure how straightforward it is to specify order, perhaps you could provide an example of order of aspects that involve @CircuitBreaker and @PerformanceMonitor?

joercampbell commented 9 years ago

Yeah - I think aspect J does allow the specification of order so I would head in that direction. I am going to try to work in and around Jrugged this week and get this and at least one other change into a build in the next week or so.

On Tue, Oct 7, 2014 at 6:20 PM, w!se9uy notifications@github.com wrote:

It is okay if we don't want to bring in Spring dependencies. I'm not sure how straightforward it is to specify order, perhaps you could provide an example of order of aspects that involve @CircuitBreaker https://github.com/CircuitBreaker and @PerformanceMonitor?

— Reply to this email directly or view it on GitHub https://github.com/Comcast/jrugged/issues/30#issuecomment-58272586.

joercampbell commented 9 years ago

Can you pull master and let me know if the ordering matches with your expectations - There doesn't appear to be an easy way with aspect j to allow callers to specify the precedence values, so I ordered them I believe such that breaker opens will not get counted in perf mon stats.

joercampbell commented 9 years ago

This is proving to be a rather vexing thing to implement... Spring doesn't support the aspectJ mechanism for setting precedence. I'll try to see about a change that implements this after the current release this morning.

lite2073 commented 9 years ago

Thanks for the update, Joe!