From 61c525076fb057c73a18f3b3ea00ef738fc9c839 Mon Sep 17 00:00:00 2001 From: Eric Zhao Date: Mon, 25 Mar 2019 14:14:31 +0800 Subject: [PATCH] Improve SentinelResourceAspect and update test cases / document of annotation support Signed-off-by: Eric Zhao --- .../sentinel/annotation/SentinelResource.java | 3 ++- .../sentinel-annotation-aspectj/README.md | 1 + .../AbstractSentinelAspectSupport.java | 26 +++++++++++++++++++ .../aspectj/SentinelResourceAspect.java | 24 +---------------- .../SentinelAnnotationIntegrationTest.java | 10 ++++++- .../integration/service/FooService.java | 7 ++++- 6 files changed, 45 insertions(+), 26 deletions(-) diff --git a/sentinel-core/src/main/java/com/alibaba/csp/sentinel/annotation/SentinelResource.java b/sentinel-core/src/main/java/com/alibaba/csp/sentinel/annotation/SentinelResource.java index 37e613b0..10631006 100644 --- a/sentinel-core/src/main/java/com/alibaba/csp/sentinel/annotation/SentinelResource.java +++ b/sentinel-core/src/main/java/com/alibaba/csp/sentinel/annotation/SentinelResource.java @@ -61,7 +61,8 @@ public @interface SentinelResource { String fallback() default ""; /** - * @return the exception classes to trace, Throwable.class by default + * @return the list of exception classes to trace, {@link Throwable} by default + * @since 1.5.1 */ Class[] exceptionsToTrace() default {Throwable.class}; } diff --git a/sentinel-extension/sentinel-annotation-aspectj/README.md b/sentinel-extension/sentinel-annotation-aspectj/README.md index 75576460..15e7e0fb 100644 --- a/sentinel-extension/sentinel-annotation-aspectj/README.md +++ b/sentinel-extension/sentinel-annotation-aspectj/README.md @@ -11,6 +11,7 @@ The `@SentinelResource` annotation indicates a resource definition, including: - `entryType`: Resource entry type (inbound or outbound), `EntryType.OUT` by default - `fallback`: Fallback method when degraded (optional). The fallback method should be located in the same class with original method. The signature of the fallback method should match the original method (parameter types and return type). - `blockHandler`: Handler method that handles `BlockException` when blocked. The signature should match original method, with the last additional parameter type `BlockException`. The block handler method should be located in the same class with original method by default. If you want to use method in other classes, you can set the `blockHandlerClass` with corresponding `Class` (Note the method in other classes must be *static*). +- `exceptionsToTrace`: List of business exception classes to trace and record (since 1.5.1). For example: diff --git a/sentinel-extension/sentinel-annotation-aspectj/src/main/java/com/alibaba/csp/sentinel/annotation/aspectj/AbstractSentinelAspectSupport.java b/sentinel-extension/sentinel-annotation-aspectj/src/main/java/com/alibaba/csp/sentinel/annotation/aspectj/AbstractSentinelAspectSupport.java index 6a13765d..90277736 100644 --- a/sentinel-extension/sentinel-annotation-aspectj/src/main/java/com/alibaba/csp/sentinel/annotation/aspectj/AbstractSentinelAspectSupport.java +++ b/sentinel-extension/sentinel-annotation-aspectj/src/main/java/com/alibaba/csp/sentinel/annotation/aspectj/AbstractSentinelAspectSupport.java @@ -15,6 +15,7 @@ */ package com.alibaba.csp.sentinel.annotation.aspectj; +import com.alibaba.csp.sentinel.Tracer; import com.alibaba.csp.sentinel.annotation.SentinelResource; import com.alibaba.csp.sentinel.log.RecordLog; import com.alibaba.csp.sentinel.slots.block.BlockException; @@ -76,6 +77,31 @@ public abstract class AbstractSentinelAspectSupport { throw ex; } + protected void traceException(Throwable ex, SentinelResource annotation) { + if (isTracedException(ex, annotation.exceptionsToTrace())) { + Tracer.trace(ex); + } + } + + /** + * Check whether the exception is in tracked list of exception classes. + * + * @param ex provided throwable + * @param exceptionsToTrace list of exceptions to trace + * @return true if it should be traced, otherwise false + */ + private boolean isTracedException(Throwable ex, Class[] exceptionsToTrace) { + if (exceptionsToTrace == null) { + return false; + } + for (Class exceptionToTrace : exceptionsToTrace) { + if (exceptionToTrace.isAssignableFrom(ex.getClass())) { + return true; + } + } + return false; + } + private boolean isDegradeFailure(/*@NonNull*/ BlockException ex) { return ex instanceof DegradeException; } diff --git a/sentinel-extension/sentinel-annotation-aspectj/src/main/java/com/alibaba/csp/sentinel/annotation/aspectj/SentinelResourceAspect.java b/sentinel-extension/sentinel-annotation-aspectj/src/main/java/com/alibaba/csp/sentinel/annotation/aspectj/SentinelResourceAspect.java index a4a78ec9..ae22ef6c 100644 --- a/sentinel-extension/sentinel-annotation-aspectj/src/main/java/com/alibaba/csp/sentinel/annotation/aspectj/SentinelResourceAspect.java +++ b/sentinel-extension/sentinel-annotation-aspectj/src/main/java/com/alibaba/csp/sentinel/annotation/aspectj/SentinelResourceAspect.java @@ -18,7 +18,6 @@ package com.alibaba.csp.sentinel.annotation.aspectj; import com.alibaba.csp.sentinel.Entry; import com.alibaba.csp.sentinel.EntryType; import com.alibaba.csp.sentinel.SphU; -import com.alibaba.csp.sentinel.Tracer; import com.alibaba.csp.sentinel.annotation.SentinelResource; import com.alibaba.csp.sentinel.slots.block.BlockException; import org.aspectj.lang.ProceedingJoinPoint; @@ -59,9 +58,7 @@ public class SentinelResourceAspect extends AbstractSentinelAspectSupport { } catch (BlockException ex) { return handleBlockException(pjp, annotation, ex); } catch (Throwable ex) { - if (isTrackedException(ex, annotation.exceptionsToTrace())) { - Tracer.trace(ex); - } + traceException(ex, annotation); throw ex; } finally { if (entry != null) { @@ -69,23 +66,4 @@ public class SentinelResourceAspect extends AbstractSentinelAspectSupport { } } } - - /** - * whether the exception is in tracked list of exception classes - * - * @param ex - * @param exceptionsToTrace - * @return - */ - private boolean isTrackedException(Throwable ex, Class[] exceptionsToTrace) { - if (exceptionsToTrace == null) { - return false; - } - for (Class exceptionToTrace : exceptionsToTrace) { - if (exceptionToTrace.isAssignableFrom(ex.getClass())) { - return true; - } - } - return false; - } } diff --git a/sentinel-extension/sentinel-annotation-aspectj/src/test/java/com/alibaba/csp/sentinel/annotation/aspectj/integration/SentinelAnnotationIntegrationTest.java b/sentinel-extension/sentinel-annotation-aspectj/src/test/java/com/alibaba/csp/sentinel/annotation/aspectj/integration/SentinelAnnotationIntegrationTest.java index 547994ff..ed17ee28 100644 --- a/sentinel-extension/sentinel-annotation-aspectj/src/test/java/com/alibaba/csp/sentinel/annotation/aspectj/integration/SentinelAnnotationIntegrationTest.java +++ b/sentinel-extension/sentinel-annotation-aspectj/src/test/java/com/alibaba/csp/sentinel/annotation/aspectj/integration/SentinelAnnotationIntegrationTest.java @@ -98,7 +98,15 @@ public class SentinelAnnotationIntegrationTest extends AbstractJUnit4SpringConte try { fooService.foo(5758); fail("should not reach here"); - } catch (IllegalAccessException ex) { + } catch (Exception ex) { + // Should not be traced. + assertThat(cn.exceptionQps()).isZero(); + } + + try { + fooService.foo(5763); + fail("should not reach here"); + } catch (Exception ex) { assertThat(cn.exceptionQps()).isPositive(); } diff --git a/sentinel-extension/sentinel-annotation-aspectj/src/test/java/com/alibaba/csp/sentinel/annotation/aspectj/integration/service/FooService.java b/sentinel-extension/sentinel-annotation-aspectj/src/test/java/com/alibaba/csp/sentinel/annotation/aspectj/integration/service/FooService.java index 3d30adf6..f9afef49 100644 --- a/sentinel-extension/sentinel-annotation-aspectj/src/test/java/com/alibaba/csp/sentinel/annotation/aspectj/integration/service/FooService.java +++ b/sentinel-extension/sentinel-annotation-aspectj/src/test/java/com/alibaba/csp/sentinel/annotation/aspectj/integration/service/FooService.java @@ -18,6 +18,7 @@ package com.alibaba.csp.sentinel.annotation.aspectj.integration.service; import com.alibaba.csp.sentinel.annotation.SentinelResource; import com.alibaba.csp.sentinel.slots.block.BlockException; import com.alibaba.csp.sentinel.slots.block.degrade.DegradeException; + import org.springframework.stereotype.Service; import java.util.concurrent.ThreadLocalRandom; @@ -28,7 +29,8 @@ import java.util.concurrent.ThreadLocalRandom; @Service public class FooService { - @SentinelResource(value = "apiFoo", blockHandler = "fooBlockHandler", fallback = "fooFallbackFunc") + @SentinelResource(value = "apiFoo", blockHandler = "fooBlockHandler", fallback = "fooFallbackFunc", + exceptionsToTrace = {IllegalArgumentException.class}) public String foo(int i) throws Exception { if (i == 9527) { throw new DegradeException("ggg"); @@ -36,6 +38,9 @@ public class FooService { if (i == 5758) { throw new IllegalAccessException(); } + if (i == 5763) { + throw new IllegalArgumentException(); + } return "Hello for " + i; }