Improve SentinelResourceAspect and update test cases / document of an…

…notation support Signed-off-by: Eric Zhao <[email protected]>
LakeNam · Mar 25, 2019 · 61c5250 · 61c5250
1 parent 6e1dfb3
commit 61c5250
Show file tree

Hide file tree

Showing 6 changed files with 45 additions and 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
@@ -61,7 +61,8 @@
     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<? extends Throwable>[] exceptionsToTrace() default {Throwable.class};
 }
diff --git 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/.../main/java/com/alibaba/csp/sentinel/annotation/aspectj/AbstractSentinelAspectSupport.java b/.../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 @@ protected Object handleBlockException(ProceedingJoinPoint pjp, String fallback,
         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<? extends Throwable>[] exceptionsToTrace) {
+        if (exceptionsToTrace == null) {
+            return false;
+        }
+        for (Class<? extends Throwable> exceptionToTrace : exceptionsToTrace) {
+            if (exceptionToTrace.isAssignableFrom(ex.getClass())) {
+                return true;
+            }
+        }
+        return false;
+    }
+
     private boolean isDegradeFailure(/*@NonNull*/ BlockException ex) {
         return ex instanceof DegradeException;
     }

diff --git a/...ctj/src/main/java/com/alibaba/csp/sentinel/annotation/aspectj/SentinelResourceAspect.java b/...ctj/src/main/java/com/alibaba/csp/sentinel/annotation/aspectj/SentinelResourceAspect.java
@@ -18,7 +18,6 @@
 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,33 +58,12 @@ public Object invokeResourceWithSentinel(ProceedingJoinPoint pjp) throws Throwab
         } 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) {
                 entry.exit();
             }
         }
     }
-
-    /**
-     * whether the exception is in tracked list of exception classes
-     *
-     * @param ex
-     * @param exceptionsToTrace
-     * @return
-     */
-    private boolean isTrackedException(Throwable ex, Class<? extends Throwable>[] exceptionsToTrace) {
-        if (exceptionsToTrace == null) {
-            return false;
-        }
-        for (Class exceptionToTrace : exceptionsToTrace) {
-            if (exceptionToTrace.isAssignableFrom(ex.getClass())) {
-                return true;
-            }
-        }
-        return false;
-    }
 }
diff --git a/...libaba/csp/sentinel/annotation/aspectj/integration/SentinelAnnotationIntegrationTest.java b/...libaba/csp/sentinel/annotation/aspectj/integration/SentinelAnnotationIntegrationTest.java
@@ -98,7 +98,15 @@ public void testNormalBlockHandlerAndFallback() throws Exception {
         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/...test/java/com/alibaba/csp/sentinel/annotation/aspectj/integration/service/FooService.java b/...test/java/com/alibaba/csp/sentinel/annotation/aspectj/integration/service/FooService.java
@@ -18,6 +18,7 @@
 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,14 +29,18 @@
 @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");
         }
         if (i == 5758) {
             throw new IllegalAccessException();
         }
+        if (i == 5763) {
+            throw new IllegalArgumentException();
+        }
         return "Hello for " + i;
     }