[#9595] Improve accuracy of statistics of non-sampling in server plugins

pinpoint-apm · Feb 24, 2023 · f5d0b05 · f5d0b05
1 parent f5e20db
commit f5d0b05
Show file tree

Hide file tree

Showing 12 changed files with 116 additions and 166 deletions.
diff --git a/...m/navercorp/pinpoint/bootstrap/interceptor/SpanEventSimpleAroundInterceptorForPlugin.java b/...m/navercorp/pinpoint/bootstrap/interceptor/SpanEventSimpleAroundInterceptorForPlugin.java
@@ -49,7 +49,7 @@ public void before(Object target, Object[] args) {
 
         prepareBeforeTrace(target, args);
 
-        final Trace trace = traceContext.currentTraceObject();
+        final Trace trace = currentTrace();
         if (trace == null) {
             return;
         }
@@ -64,6 +64,10 @@ public void before(Object target, Object[] args) {
         }
     }
 
+    protected Trace currentTrace() {
+        return traceContext.currentTraceObject();
+    }
+
     protected void logBeforeInterceptor(Object target, Object[] args) {
         logger.beforeInterceptor(target, args);
     }
@@ -82,7 +86,7 @@ public void after(Object target, Object[] args, Object result, Throwable throwab
 
         prepareAfterTrace(target, args, result, throwable);
 
-        final Trace trace = traceContext.currentTraceObject();
+        final Trace trace = currentTrace();
         if (trace == null) {
             return;
         }

diff --git a/...com/navercorp/pinpoint/plugin/grpc/interceptor/server/ServerStreamCreatedInterceptor.java b/...com/navercorp/pinpoint/plugin/grpc/interceptor/server/ServerStreamCreatedInterceptor.java
@@ -54,7 +54,7 @@ public ServerStreamCreatedInterceptor(TraceContext traceContext, MethodDescripto
 
         final RequestAdaptor<GrpcServerStreamRequest> requestAdaptor = new GrpcServerStreamRequestAdaptor();
         this.serverRequestRecorder = new ServerRequestRecorder<>(requestAdaptor);
-        this.requestTraceReader = new RequestTraceReader(traceContext, requestAdaptor, true);
+        this.requestTraceReader = new RequestTraceReader<>(traceContext, requestAdaptor, true);
 
         traceContext.cacheApi(GRPC_SERVER_CALL_METHOD_DESCRIPTOR);
     }

diff --git a/...in/java/com/navercorp/pinpoint/plugin/jboss/interceptor/RequestStartAsyncInterceptor.java b/...in/java/com/navercorp/pinpoint/plugin/jboss/interceptor/RequestStartAsyncInterceptor.java
@@ -18,6 +18,7 @@
 
 import com.navercorp.pinpoint.bootstrap.context.MethodDescriptor;
 import com.navercorp.pinpoint.bootstrap.context.SpanEventRecorder;
+import com.navercorp.pinpoint.bootstrap.context.Trace;
 import com.navercorp.pinpoint.bootstrap.context.TraceContext;
 import com.navercorp.pinpoint.bootstrap.interceptor.SpanEventSimpleAroundInterceptorForPlugin;
 import com.navercorp.pinpoint.plugin.jboss.JbossAsyncListener;
@@ -40,6 +41,11 @@ public RequestStartAsyncInterceptor(TraceContext context, MethodDescriptor descr
     protected void doInBeforeTrace(SpanEventRecorder recorder, Object target, Object[] args) {
     }
 
+    @Override
+    protected Trace currentTrace() {
+        return traceContext.currentRawTraceObject();
+    }
+
     @Override
     protected void doInAfterTrace(SpanEventRecorder recorder, Object target, Object[] args, Object result, Throwable throwable) {
         if (validate(target, result, throwable)) {

diff --git a/...in/java/com/navercorp/pinpoint/plugin/jetty/interceptor/RequestStartAsyncInterceptor.java b/...in/java/com/navercorp/pinpoint/plugin/jetty/interceptor/RequestStartAsyncInterceptor.java
@@ -20,9 +20,7 @@
 import com.navercorp.pinpoint.bootstrap.context.SpanEventRecorder;
 import com.navercorp.pinpoint.bootstrap.context.Trace;
 import com.navercorp.pinpoint.bootstrap.context.TraceContext;
-import com.navercorp.pinpoint.bootstrap.interceptor.AroundInterceptor;
-import com.navercorp.pinpoint.bootstrap.logging.PLogger;
-import com.navercorp.pinpoint.bootstrap.logging.PLoggerFactory;
+import com.navercorp.pinpoint.bootstrap.interceptor.SpanEventSimpleAroundInterceptorForPlugin;
 import com.navercorp.pinpoint.plugin.jetty.JettyAsyncListener;
 import com.navercorp.pinpoint.plugin.jetty.JettyConstants;
 
@@ -33,61 +31,37 @@
 /**
  * @author jaehong.kim
  */
-public class RequestStartAsyncInterceptor implements AroundInterceptor {
-    private PLogger logger = PLoggerFactory.getLogger(this.getClass());
-    private boolean isDebug = logger.isDebugEnabled();
-
-    private TraceContext traceContext;
-    private MethodDescriptor descriptor;
+public class RequestStartAsyncInterceptor extends SpanEventSimpleAroundInterceptorForPlugin {
 
     public RequestStartAsyncInterceptor(TraceContext traceContext, MethodDescriptor descriptor) {
-        this.traceContext = traceContext;
-        this.descriptor = descriptor;
+        super(traceContext, descriptor);
     }
 
-    @Override
-    public void before(Object target, Object[] args) {
-        if (isDebug) {
-            logger.beforeInterceptor(target, args);
-        }
 
-        final Trace trace = traceContext.currentTraceObject();
-        if (trace == null) {
-            return;
-        }
-        trace.traceBlockBegin();
+    @Override
+    protected void doInBeforeTrace(SpanEventRecorder recorder, Object target, Object[] args) throws Exception {
+
     }
 
     @Override
-    public void after(Object target, Object[] args, Object result, Throwable throwable) {
-        if (isDebug) {
-            logger.afterInterceptor(target, args, result, throwable);
-        }
-
-        final Trace trace = traceContext.currentTraceObject();
-        if (trace == null) {
-            return;
-        }
+    protected Trace currentTrace() {
+        return traceContext.currentRawTraceObject();
+    }
 
-        try {
-            final SpanEventRecorder recorder = trace.currentSpanEventRecorder();
-            if (validate(target, result, throwable)) {
-                // Add async listener. Servlet 3.0
-                final AsyncContext asyncContext = (AsyncContext) result;
-                final AsyncListener asyncListener = new JettyAsyncListener(this.traceContext, recorder.recordNextAsyncContext(true));
-                asyncContext.addListener(asyncListener);
-                if (isDebug) {
-                    logger.debug("Add async listener {}", asyncListener);
-                }
+    @Override
+    protected void doInAfterTrace(SpanEventRecorder recorder, Object target, Object[] args, Object result, Throwable throwable) throws Exception {
+        if (validate(target, result, throwable)) {
+            // Add async listener. Servlet 3.0
+            final AsyncContext asyncContext = (AsyncContext) result;
+            final AsyncListener asyncListener = new JettyAsyncListener(this.traceContext, recorder.recordNextAsyncContext(true));
+            asyncContext.addListener(asyncListener);
+            if (isDebug) {
+                logger.debug("Add async listener {}", asyncListener);
             }
-            recorder.recordServiceType(JettyConstants.JETTY_METHOD);
-            recorder.recordApi(descriptor);
-            recorder.recordException(throwable);
-        } catch (Throwable t) {
-            logger.warn("Failed to AFTER process. {}", t.getMessage(), t);
-        } finally {
-            trace.traceBlockEnd();
         }
+        recorder.recordServiceType(JettyConstants.JETTY_METHOD);
+        recorder.recordApi(methodDescriptor);
+        recorder.recordException(throwable);
     }
 
     private boolean validate(final Object target, final Object result, final Throwable throwable) {

diff --git a/...ercorp/pinpoint/plugin/reactor/netty/interceptor/AbstractHttpServerHandleInterceptor.java b/...ercorp/pinpoint/plugin/reactor/netty/interceptor/AbstractHttpServerHandleInterceptor.java
@@ -23,14 +23,12 @@
 import com.navercorp.pinpoint.bootstrap.context.AsyncContextUtils;
 import com.navercorp.pinpoint.bootstrap.context.MethodDescriptor;
 import com.navercorp.pinpoint.bootstrap.context.SpanEventRecorder;
-import com.navercorp.pinpoint.bootstrap.context.SpanRecorder;
 import com.navercorp.pinpoint.bootstrap.context.Trace;
 import com.navercorp.pinpoint.bootstrap.context.TraceContext;
 import com.navercorp.pinpoint.bootstrap.interceptor.AroundInterceptor;
 import com.navercorp.pinpoint.bootstrap.logging.PLogger;
 import com.navercorp.pinpoint.bootstrap.logging.PLoggerFactory;
 import com.navercorp.pinpoint.bootstrap.plugin.RequestRecorderFactory;
-import com.navercorp.pinpoint.bootstrap.plugin.http.HttpStatusCodeRecorder;
 import com.navercorp.pinpoint.bootstrap.plugin.request.RequestAdaptor;
 import com.navercorp.pinpoint.bootstrap.plugin.request.ServerCookieRecorder;
 import com.navercorp.pinpoint.bootstrap.plugin.request.ServerHeaderRecorder;
@@ -52,7 +50,6 @@
 public abstract class AbstractHttpServerHandleInterceptor implements AroundInterceptor {
     private final PLogger logger = PLoggerFactory.getLogger(this.getClass());
     private final boolean isDebug = logger.isDebugEnabled();
-    private final boolean isInfo = logger.isInfoEnabled();
     private final TraceContext traceContext;
     private final MethodDescriptor methodDescriptor;
     private final boolean enableAsyncEndPoint;
@@ -103,9 +100,7 @@ public void before(Object target, Object[] args) {
                 closed(args);
             }
         } catch (Throwable t) {
-            if (isInfo) {
-                logger.info("Failed to servlet request event handle.", t);
-            }
+            logger.info("Failed to servlet request event handle.", t);
         }
     }
 
@@ -131,8 +126,7 @@ private void received(final Object[] args) {
         final SpanEventRecorder recorder = trace.currentSpanEventRecorder();
         if (recorder != null) {
             // make asynchronous trace-id
-            final boolean asyncStateSupport = enableAsyncEndPoint;
-            final AsyncContext asyncContext = recorder.recordNextAsyncContext(asyncStateSupport);
+            final AsyncContext asyncContext = recorder.recordNextAsyncContext(enableAsyncEndPoint);
             ((AsyncContextAccessor) args[0])._$PINPOINT$_setAsyncContext(asyncContext);
             if (isDebug) {
                 logger.debug("Set asyncContext to args[0]. asyncContext={}", asyncContext);
@@ -160,9 +154,7 @@ public void after(Object target, Object[] args, Object result, Throwable throwab
                 received(args, throwable);
             }
         } catch (Throwable t) {
-            if (isInfo) {
-                logger.info("Failed to servlet request event handle.", t);
-            }
+            logger.info("Failed to servlet request event handle.", t);
         }
     }
 

diff --git a/...va/com/navercorp/pinpoint/plugin/resin/interceptor/HttpServletRequestImplInterceptor.java b/...va/com/navercorp/pinpoint/plugin/resin/interceptor/HttpServletRequestImplInterceptor.java
@@ -2,6 +2,7 @@
 
 import com.navercorp.pinpoint.bootstrap.context.MethodDescriptor;
 import com.navercorp.pinpoint.bootstrap.context.SpanEventRecorder;
+import com.navercorp.pinpoint.bootstrap.context.Trace;
 import com.navercorp.pinpoint.bootstrap.context.TraceContext;
 import com.navercorp.pinpoint.bootstrap.interceptor.SpanEventSimpleAroundInterceptorForPlugin;
 import com.navercorp.pinpoint.plugin.resin.ResinAsyncListener;
@@ -25,6 +26,11 @@ public HttpServletRequestImplInterceptor(TraceContext context, MethodDescriptor
     protected void doInBeforeTrace(SpanEventRecorder recorder, Object target, Object[] args) {
     }
 
+    @Override
+    protected Trace currentTrace() {
+        return traceContext.currentRawTraceObject();
+    }
+
     @Override
     protected void doInAfterTrace(SpanEventRecorder recorder, Object target, Object[] args, Object result, Throwable throwable) {
         if (validate(target, result, throwable)) {

diff --git a/...om/navercorp/pinpoint/plugin/tomcat/jakarta/interceptor/RequestStartAsyncInterceptor.java b/...om/navercorp/pinpoint/plugin/tomcat/jakarta/interceptor/RequestStartAsyncInterceptor.java
@@ -18,9 +18,7 @@
 import com.navercorp.pinpoint.bootstrap.context.SpanEventRecorder;
 import com.navercorp.pinpoint.bootstrap.context.Trace;
 import com.navercorp.pinpoint.bootstrap.context.TraceContext;
-import com.navercorp.pinpoint.bootstrap.interceptor.AroundInterceptor;
-import com.navercorp.pinpoint.bootstrap.logging.PLogger;
-import com.navercorp.pinpoint.bootstrap.logging.PLoggerFactory;
+import com.navercorp.pinpoint.bootstrap.interceptor.SpanEventSimpleAroundInterceptorForPlugin;
 import com.navercorp.pinpoint.bootstrap.plugin.request.AsyncListenerInterceptorHelper;
 import com.navercorp.pinpoint.plugin.tomcat.TomcatConstants;
 import com.navercorp.pinpoint.plugin.tomcat.jakarta.TomcatAsyncListenerAdaptor;
@@ -30,69 +28,44 @@
 /**
  * @author jaehong.kim
  */
-public class RequestStartAsyncInterceptor implements AroundInterceptor {
-    private final PLogger logger = PLoggerFactory.getLogger(this.getClass());
-    private final boolean isDebug = logger.isDebugEnabled();
-
-    private final TraceContext traceContext;
-    private final MethodDescriptor descriptor;
+public class RequestStartAsyncInterceptor extends SpanEventSimpleAroundInterceptorForPlugin {
 
     public RequestStartAsyncInterceptor(TraceContext context, MethodDescriptor descriptor) {
-        this.traceContext = context;
-        this.descriptor = descriptor;
+        super(context, descriptor);
     }
 
     @Override
-    public void before(Object target, Object[] args) {
-        if (isDebug) {
-            logger.beforeInterceptor(target, args);
-        }
-
-        final Trace trace = traceContext.currentRawTraceObject();
-        if (trace == null) {
-            return;
-        }
-        trace.traceBlockBegin();
+    protected void doInBeforeTrace(SpanEventRecorder recorder, Object target, Object[] args) throws Exception {
+
     }
 
     @Override
-    public void after(Object target, Object[] args, Object result, Throwable throwable) {
-        if (isDebug) {
-            logger.afterInterceptor(target, args, result, throwable);
-        }
-
-        final Trace trace = traceContext.currentRawTraceObject();
-        if (trace == null) {
-            return;
-        }
+    protected Trace currentTrace() {
+        return traceContext.currentRawTraceObject();
+    }
 
-        try {
-            final SpanEventRecorder recorder = trace.currentSpanEventRecorder();
-            if (validate(target, result, throwable)) {
-                com.navercorp.pinpoint.bootstrap.context.AsyncContext nextAsyncContext = recorder.recordNextAsyncContext(true);
+    @Override
+    protected void doInAfterTrace(SpanEventRecorder recorder, Object target, Object[] args, Object result, Throwable throwable) throws Exception {
+        if (validate(target, result, throwable)) {
+            com.navercorp.pinpoint.bootstrap.context.AsyncContext nextAsyncContext = recorder.recordNextAsyncContext(true);
 
-                final AsyncListenerInterceptorHelper listenerInterceptor = new AsyncListenerInterceptorHelper(traceContext, nextAsyncContext);
+            final AsyncListenerInterceptorHelper listenerInterceptor = new AsyncListenerInterceptorHelper(traceContext, nextAsyncContext);
 
-                // Add async listener
-                final AsyncContext asyncContext = (AsyncContext) result;
-                asyncContext.addListener(new TomcatAsyncListenerAdaptor(listenerInterceptor));
+            // Add async listener
+            final AsyncContext asyncContext = (AsyncContext) result;
+            asyncContext.addListener(new TomcatAsyncListenerAdaptor(listenerInterceptor));
 
-                // Set AsyncContext, AsyncListener
-                final HttpServletRequest request = (HttpServletRequest) target;
-                request.setAttribute(com.navercorp.pinpoint.bootstrap.context.AsyncContext.class.getName(), nextAsyncContext);
-                request.setAttribute(TomcatConstants.TOMCAT_SERVLET_REQUEST_TRACE, listenerInterceptor);
-                if (isDebug) {
-                    logger.debug("Add async listener {}", listenerInterceptor);
-                }
+            // Set AsyncContext, AsyncListener
+            final HttpServletRequest request = (HttpServletRequest) target;
+            request.setAttribute(com.navercorp.pinpoint.bootstrap.context.AsyncContext.class.getName(), nextAsyncContext);
+            request.setAttribute(TomcatConstants.TOMCAT_SERVLET_REQUEST_TRACE, listenerInterceptor);
+            if (isDebug) {
+                logger.debug("Add async listener {}", listenerInterceptor);
             }
-            recorder.recordServiceType(TomcatConstants.TOMCAT_METHOD);
-            recorder.recordApi(descriptor);
-            recorder.recordException(throwable);
-        } catch (Throwable t) {
-            logger.warn("Failed to AFTER process. {}", t.getMessage(), t);
-        } finally {
-            trace.traceBlockEnd();
         }
+        recorder.recordServiceType(TomcatConstants.TOMCAT_METHOD);
+        recorder.recordApi(methodDescriptor);
+        recorder.recordException(throwable);
     }
 
     private boolean validate(final Object target, final Object result, final Throwable throwable) {