Polish Tracer with entry.setError(ex) mechanism

Signed-off-by: Eric Zhao <[email protected]>
alibaba · Apr 28, 2020 · 516e36f · 516e36f
1 parent 7f31657
commit 516e36f
Show file tree

Hide file tree

Showing 3 changed files with 30 additions and 87 deletions.
diff --git a/sentinel-core/src/main/java/com/alibaba/csp/sentinel/Tracer.java b/sentinel-core/src/main/java/com/alibaba/csp/sentinel/Tracer.java
@@ -18,11 +18,7 @@
 import com.alibaba.csp.sentinel.context.Context;
 import com.alibaba.csp.sentinel.context.ContextUtil;
 import com.alibaba.csp.sentinel.context.NullContext;
-import com.alibaba.csp.sentinel.metric.extension.MetricExtensionProvider;
-import com.alibaba.csp.sentinel.node.ClusterNode;
-import com.alibaba.csp.sentinel.node.DefaultNode;
 import com.alibaba.csp.sentinel.slots.block.BlockException;
-import com.alibaba.csp.sentinel.metric.extension.MetricExtension;
 import com.alibaba.csp.sentinel.util.AssertUtil;
 
 /**
@@ -39,82 +35,81 @@ public class Tracer {
     protected Tracer() {}
 
     /**
-     * Trace provided {@link Throwable} and increment exception count to entry in current context.
+     * Trace provided {@link Throwable} to the resource entry in current context.
      *
      * @param e exception to record
      */
     public static void trace(Throwable e) {
-        trace(e, 1);
+        traceContext(e, ContextUtil.getContext());
     }
 
     /**
-     * Trace provided {@link Throwable} and add exception count to entry in current context.
+     * Trace provided {@link Throwable} to current entry in current context.
      *
      * @param e     exception to record
      * @param count exception count to add
      */
+    @Deprecated
     public static void trace(Throwable e, int count) {
         traceContext(e, count, ContextUtil.getContext());
     }
 
     /**
-     * Trace provided {@link Throwable} and add exception count to current entry in provided context.
+     * Trace provided {@link Throwable} to current entry of given entrance context.
      *
      * @param e     exception to record
-     * @param count exception count to add
-     * @since 1.4.2
+     * @param context target entrance context
+     * @since 1.8.0
      */
-    public static void traceContext(Throwable e, int count, Context context) {
+    public static void traceContext(Throwable e, Context context) {
         if (!shouldTrace(e)) {
             return;
         }
 
         if (context == null || context instanceof NullContext) {
             return;
         }
-
-        DefaultNode curNode = (DefaultNode)context.getCurNode();
-        traceExceptionToNode(e, count, context.getCurEntry(), curNode);
+        traceEntryInternal(e, context.getCurEntry());
     }
 
     /**
-     * Trace provided {@link Throwable} and increment exception count to provided entry.
+     * Trace provided {@link Throwable} and add exception count to current entry in provided context.
      *
-     * @param e exception to record
+     * @param e     exception to record
+     * @param count exception count to add
      * @since 1.4.2
      */
-    public static void traceEntry(Throwable e, Entry entry) {
-        traceEntry(e, 1, entry);
+    @Deprecated
+    public static void traceContext(Throwable e, int count, Context context) {
+        if (!shouldTrace(e)) {
+            return;
+        }
+
+        if (context == null || context instanceof NullContext) {
+            return;
+        }
+        traceEntryInternal(e, context.getCurEntry());
     }
 
     /**
-     * Trace provided {@link Throwable} and add exception count to provided entry.
+     * Trace provided {@link Throwable} to the given resource entry.
      *
-     * @param e     exception to record
-     * @param count exception count to add
+     * @param e exception to record
      * @since 1.4.2
      */
-    public static void traceEntry(Throwable e, int count, Entry entry) {
+    public static void traceEntry(Throwable e, Entry entry) {
         if (!shouldTrace(e)) {
             return;
         }
-        if (entry == null || entry.getCurNode() == null) {
-            return;
-        }
-
-        DefaultNode curNode = (DefaultNode)entry.getCurNode();
-        traceExceptionToNode(e, count, entry, curNode);
+        traceEntryInternal(e, entry);
     }
 
-    private static void traceExceptionToNode(Throwable t, int count, Entry entry, DefaultNode curNode) {
-        if (curNode == null) {
+    private static void traceEntryInternal(/*@NeedToTrace*/ Throwable e, Entry entry) {
+        if (entry == null) {
             return;
         }
-        for (MetricExtension m : MetricExtensionProvider.getMetricExtensions()) {
-            m.addException(entry.getResourceWrapper().getName(), count, t);
-        }
 
-        curNode.trace(t, count);
+        entry.setError(e);
     }
 
     /**
@@ -203,4 +198,4 @@ protected static boolean shouldTrace(Throwable t) {
         }
         return true;
     }
-}
+}
diff --git a/sentinel-core/src/main/java/com/alibaba/csp/sentinel/node/DefaultNode.java b/sentinel-core/src/main/java/com/alibaba/csp/sentinel/node/DefaultNode.java
@@ -23,7 +23,6 @@
 import com.alibaba.csp.sentinel.SphU;
 import com.alibaba.csp.sentinel.context.Context;
 import com.alibaba.csp.sentinel.slotchain.ResourceWrapper;
-import com.alibaba.csp.sentinel.slots.block.BlockException;
 import com.alibaba.csp.sentinel.slots.nodeselector.NodeSelectorSlot;
 
 /**
@@ -147,28 +146,6 @@ public void printDefaultNode() {
         visitTree(0, this);
     }
 
-
-    /**
-     * Add exception count only when given {@code throwable} is not a {@link BlockException}.
-     *
-     * @param throwable target exception
-     * @param count     count to add
-     */
-    public void trace(Throwable throwable, int count) {
-        if (count <= 0) {
-            return;
-        }
-        if (BlockException.isBlockException(throwable)) {
-            return;
-        }
-        super.increaseExceptionQps(count);
-
-        // clusterNode can be null when Constants.ON is false.
-        if (clusterNode != null) {
-            clusterNode.increaseExceptionQps(count);
-        }
-    }
-
     private void visitTree(int level, DefaultNode node) {
         for (int i = 0; i < level; ++i) {
             System.out.print("-");

diff --git a/sentinel-core/src/test/java/com/alibaba/csp/sentinel/node/ClusterNodeTest.java b/sentinel-core/src/test/java/com/alibaba/csp/sentinel/node/ClusterNodeTest.java
@@ -15,10 +15,6 @@
  */
 package com.alibaba.csp.sentinel.node;
 
-import com.alibaba.csp.sentinel.EntryType;
-import com.alibaba.csp.sentinel.slotchain.StringResourceWrapper;
-import com.alibaba.csp.sentinel.slots.block.flow.FlowException;
-
 import org.junit.Test;
 
 import java.util.ArrayList;
@@ -129,29 +125,4 @@ public Object call() throws Exception {
             }
         }
     }
-
-    @Test
-    public void testTraceException() {
-        ClusterNode clusterNode = new ClusterNode("test");
-        DefaultNode defaultNode = new DefaultNode(new StringResourceWrapper("test", EntryType.IN), clusterNode);
-
-        Exception exception = new RuntimeException("test");
-
-        // test count<=0, no exceptionQps added
-        defaultNode.trace(exception, 0);
-        defaultNode.trace(exception, -1);
-        assertEquals(0, defaultNode.exceptionQps(), 0.01);
-        assertEquals(0, defaultNode.totalException());
-
-        // test count=1, not BlockException, 1 exceptionQps added
-        defaultNode.trace(exception, 1);
-        assertEquals(1, defaultNode.exceptionQps(), 0.01);
-        assertEquals(1, defaultNode.totalException());
-
-        // test count=1, BlockException, no exceptionQps added
-        FlowException flowException = new FlowException("flow");
-        defaultNode.trace(flowException, 1);
-        assertEquals(1, defaultNode.exceptionQps(), 0.01);
-        assertEquals(1, defaultNode.totalException());
-    }
 }