Refactor the mechanism of recording error (on completed)

sentinel-core/src/main/java/com/alibaba/csp/sentinel/Entry.java

@@ -15,6 +15,7 @@
  */
 package com.alibaba.csp.sentinel;
 
+import com.alibaba.csp.sentinel.slots.block.BlockException;
 import com.alibaba.csp.sentinel.util.TimeUtil;
 import com.alibaba.csp.sentinel.context.ContextUtil;
 import com.alibaba.csp.sentinel.node.Node;
@@ -44,6 +45,7 @@
  * @author qinan.qn
  * @author jialiang.linjl
  * @author leyou(lihao)
+ * @author Eric Zhao
  * @see SphU
  * @see Context
  * @see ContextUtil
@@ -52,18 +54,23 @@ public abstract class Entry implements AutoCloseable {
 
     private static final Object[] OBJECTS0 = new Object[0];
 
-    private long createTime;
+    private final long createTimestamp;
+    private long completeTimestamp;
+
     private Node curNode;
     /**
      * {@link Node} of the specific origin, Usually the origin is the Service Consumer.
      */
     private Node originNode;
+
     private Throwable error;
-    protected ResourceWrapper resourceWrapper;
+    private BlockException blockError;
+
+    protected final ResourceWrapper resourceWrapper;
 
     public Entry(ResourceWrapper resourceWrapper) {
         this.resourceWrapper = resourceWrapper;
-        this.createTime = TimeUtil.currentTimeMillis();
+        this.createTimestamp = TimeUtil.currentTimeMillis();
     }
 
     public ResourceWrapper getResourceWrapper() {
@@ -119,8 +126,17 @@ public void close() {
      */
     public abstract Node getLastNode();
 
-    public long getCreateTime() {
-        return createTime;
+    public long getCreateTimestamp() {
+        return createTimestamp;
+    }
+
+    public long getCompleteTimestamp() {
+        return completeTimestamp;
+    }
+
+    public Entry setCompleteTimestamp(long completeTimestamp) {
+        this.completeTimestamp = completeTimestamp;
+        return this;
     }
 
     public Node getCurNode() {
@@ -131,6 +147,15 @@ public void setCurNode(Node node) {
         this.curNode = node;
     }
 
+    public BlockException getBlockError() {
+        return blockError;
+    }
+
+    public Entry setBlockError(BlockException blockError) {
+        this.blockError = blockError;
+        return this;
+    }
+
     public Throwable getError() {
         return error;
     }

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;
     }
-}
+}

sentinel-core/src/main/java/com/alibaba/csp/sentinel/metric/extension/callback/MetricExitCallback.java

@@ -14,14 +14,22 @@
  * @since 1.6.1
  */
 public class MetricExitCallback implements ProcessorSlotExitCallback {
+
     @Override
     public void onExit(Context context, ResourceWrapper resourceWrapper, int count, Object... args) {
         for (MetricExtension m : MetricExtensionProvider.getMetricExtensions()) {
-            if (context.getCurEntry().getError() == null) {
-                long realRt = TimeUtil.currentTimeMillis() - context.getCurEntry().getCreateTime();
-                m.addRt(resourceWrapper.getName(), realRt, args);
-                m.addSuccess(resourceWrapper.getName(), count, args);
-                m.decreaseThreadNum(resourceWrapper.getName(), args);
+            if (context.getCurEntry().getBlockError() != null) {
+                continue;
+            }
+            String resource = resourceWrapper.getName();
+            long realRt = TimeUtil.currentTimeMillis() - context.getCurEntry().getCreateTimestamp();
+            m.addRt(resource, realRt, args);
+            m.addSuccess(resource, count, args);
+            m.decreaseThreadNum(resource, args);
+
+            Throwable ex = context.getCurEntry().getError();
+            if (ex != null) {
+                m.addException(resource, count, ex);
             }
         }
     }

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("-");

sentinel-core/src/main/java/com/alibaba/csp/sentinel/slots/statistic/StatisticSlot.java

@@ -17,7 +17,7 @@
 
 import java.util.Collection;
 
-import com.alibaba.csp.sentinel.config.SentinelConfig;
+import com.alibaba.csp.sentinel.node.Node;
 import com.alibaba.csp.sentinel.slotchain.ProcessorSlotEntryCallback;
 import com.alibaba.csp.sentinel.slotchain.ProcessorSlotExitCallback;
 import com.alibaba.csp.sentinel.slots.block.flow.PriorityWaitException;
@@ -95,7 +95,7 @@ public void entry(Context context, ResourceWrapper resourceWrapper, DefaultNode
             }
         } catch (BlockException e) {
             // Blocked, set block exception to current entry.
-            context.getCurEntry().setError(e);
+            context.getCurEntry().setBlockError(e);
 
             // Add block count.
             node.increaseBlockQps(count);
@@ -115,52 +115,31 @@ public void entry(Context context, ResourceWrapper resourceWrapper, DefaultNode
 
             throw e;
         } catch (Throwable e) {
-            // Unexpected error, set error to current entry.
+            // Unexpected internal error, set error to current entry.
             context.getCurEntry().setError(e);
 
-            // This should not happen.
-            node.increaseExceptionQps(count);
-            if (context.getCurEntry().getOriginNode() != null) {
-                context.getCurEntry().getOriginNode().increaseExceptionQps(count);
-            }
-
-            if (resourceWrapper.getEntryType() == EntryType.IN) {
-                Constants.ENTRY_NODE.increaseExceptionQps(count);
-            }
             throw e;
         }
     }
 
     @Override
     public void exit(Context context, ResourceWrapper resourceWrapper, int count, Object... args) {
-        DefaultNode node = (DefaultNode)context.getCurNode();
-
-        if (context.getCurEntry().getError() == null) {
-            // Calculate response time (max RT is statisticMaxRt from SentinelConfig).
-            long rt = TimeUtil.currentTimeMillis() - context.getCurEntry().getCreateTime();
-            int maxStatisticRt = SentinelConfig.statisticMaxRt();
-            if (rt > maxStatisticRt) {
-                rt = maxStatisticRt;
-            }
+        Node node = context.getCurNode();
 
-            // Record response time and success count.
-            node.addRtAndSuccess(rt, count);
-            if (context.getCurEntry().getOriginNode() != null) {
-                context.getCurEntry().getOriginNode().addRtAndSuccess(rt, count);
-            }
+        if (context.getCurEntry().getBlockError() == null) {
+            // Calculate response time (use completeStatTime as the time of completion).
+            long completeStatTime = TimeUtil.currentTimeMillis();
+            context.getCurEntry().setCompleteTimestamp(completeStatTime);
+            long rt = completeStatTime - context.getCurEntry().getCreateTimestamp();
 
-            node.decreaseThreadNum();
-
-            if (context.getCurEntry().getOriginNode() != null) {
-                context.getCurEntry().getOriginNode().decreaseThreadNum();
-            }
+            Throwable error = context.getCurEntry().getError();
 
+            // Record response time and success count.
+            recordCompleteFor(node, count, rt, error);
+            recordCompleteFor(context.getCurEntry().getOriginNode(), count, rt, error);
             if (resourceWrapper.getEntryType() == EntryType.IN) {
-                Constants.ENTRY_NODE.addRtAndSuccess(rt, count);
-                Constants.ENTRY_NODE.decreaseThreadNum();
+                recordCompleteFor(Constants.ENTRY_NODE, count, rt, error);
             }
-        } else {
-            // Error may happen.
         }
 
         // Handle exit event with registered exit callback handlers.
@@ -171,4 +150,16 @@ public void exit(Context context, ResourceWrapper resourceWrapper, int count, Ob
 
         fireExit(context, resourceWrapper, count);
     }
+
+    private void recordCompleteFor(Node node, int batchCount, long rt, Throwable error) {
+        if (node == null) {
+            return;
+        }
+        node.addRtAndSuccess(rt, batchCount);
+        node.decreaseThreadNum();
+
+        if (error != null && !(error instanceof BlockException)) {
+            node.increaseExceptionQps(batchCount);
+        }
+    }
 }

sentinel-core/src/test/java/com/alibaba/csp/sentinel/metric/extension/callback/MetricExitCallbackTest.java

@@ -55,7 +55,7 @@ public void onExit() {
 
         int deltaMs = 100;
         when(entry.getError()).thenReturn(null);
-        when(entry.getCreateTime()).thenReturn(curMillis - deltaMs);
+        when(entry.getCreateTimestamp()).thenReturn(curMillis - deltaMs);
         when(context.getCurEntry()).thenReturn(entry);
         exitCallback.onExit(context, resourceWrapper, count, args);
         Assert.assertEquals(prevRt + deltaMs, extension.rt);