[pinpoint-apm#10220] Add profiler.uri.stat.collect.http.method opti…

…on to collect uri statistics with http methods
smilu97 · Aug 21, 2023 · 2eaa751 · 2eaa751
1 parent 9bbb809
commit 2eaa751
Show file tree

Hide file tree

Showing 30 changed files with 271 additions and 48 deletions.
diff --git a/agent/src/main/resources/profiles/local/pinpoint.config b/agent/src/main/resources/profiles/local/pinpoint.config
@@ -314,6 +314,7 @@ profiler.ignore-error-handler.myErrorHandler.exception-message@contains=
 # URI Stat
 ###########################################################
 profiler.uri.stat.enable=true
+profiler.uri.stat.collect.http.method=false
 profiler.uri.stat.spring.webmvc.enable=true
 profiler.uri.stat.spring.webmvc.useuserinput=false
 profiler.uri.stat.vertx.enable=true

diff --git a/agent/src/main/resources/profiles/release/pinpoint.config b/agent/src/main/resources/profiles/release/pinpoint.config
@@ -314,6 +314,7 @@ profiler.ignore-error-handler.myErrorHandler.exception-message@contains=
 # URI Stat
 ###########################################################
 profiler.uri.stat.enable=true
+profiler.uri.stat.collect.http.method=false
 profiler.uri.stat.spring.webmvc.enable=true
 profiler.uri.stat.spring.webmvc.useuserinput=false
 profiler.uri.stat.vertx.enable=true

diff --git a/...s/bootstrap-core/src/main/java/com/navercorp/pinpoint/bootstrap/context/SpanRecorder.java b/...s/bootstrap-core/src/main/java/com/navercorp/pinpoint/bootstrap/context/SpanRecorder.java
@@ -50,4 +50,6 @@ public interface SpanRecorder extends FrameAttachment, AttributeRecorder {
     boolean recordUriTemplate(String uriTemplate);
 
     boolean recordUriTemplate(String uriTemplate, boolean force);
+
+    boolean recordUriHttpMethod(String httpMethod);
 }
diff --git a/...bflux/src/main/java/com/navercorp/pinpoint/plugin/spring/webflux/SpringWebFluxPlugin.java b/...bflux/src/main/java/com/navercorp/pinpoint/plugin/spring/webflux/SpringWebFluxPlugin.java
@@ -79,8 +79,8 @@ public void setup(ProfilerPluginSetupContext context) {
 
         // uri stat
         if (config.isUriStatEnable()) {
-            transformTemplate.transform("org.springframework.web.reactive.result.method.AbstractHandlerMethodMapping", AbstractHandlerMethodMappingTransform.class);
-            transformTemplate.transform("org.springframework.web.reactive.handler.AbstractUrlHandlerMapping", AbstractUrlHandlerMappingTransform.class);
+            transformTemplate.transform("org.springframework.web.reactive.result.method.AbstractHandlerMethodMapping", AbstractHandlerMethodMappingTransform.class, new Object[]{config.isUriStatCollectMethod()}, new Class[]{Boolean.class});
+            transformTemplate.transform("org.springframework.web.reactive.handler.AbstractUrlHandlerMapping", AbstractUrlHandlerMappingTransform.class, new Object[]{config.isUriStatCollectMethod()}, new Class[]{Boolean.class});
         }
     }
 
@@ -214,28 +214,39 @@ public byte[] doInTransform(Instrumentor instrumentor, ClassLoader loader, Strin
     }
 
     public static class AbstractHandlerMethodMappingTransform implements TransformCallback {
+        private final Boolean uriStatCollectMethod;
+        public AbstractHandlerMethodMappingTransform(Boolean uriStatCollectMethod) {
+            this.uriStatCollectMethod = uriStatCollectMethod;
+        }
+
         @Override
         public byte[] doInTransform(Instrumentor instrumentor, ClassLoader classLoader, String className, Class<?> classBeingRedefined, ProtectionDomain protectionDomain, byte[] classfileBuffer) throws InstrumentException {
             final InstrumentClass target = instrumentor.getInstrumentClass(classLoader, className, classfileBuffer);
 
             // Add attribute listener.
             final InstrumentMethod lookupHandlerMethod = target.getDeclaredMethod("lookupHandlerMethod", "org.springframework.web.server.ServerWebExchange");
             if (lookupHandlerMethod != null) {
-                lookupHandlerMethod.addInterceptor(AbstractHandlerMethodMappingInterceptor.class);
+                lookupHandlerMethod.addInterceptor(AbstractHandlerMethodMappingInterceptor.class, va(uriStatCollectMethod));
             }
             return target.toBytecode();
         }
     }
 
     public static class AbstractUrlHandlerMappingTransform implements TransformCallback {
+        private final Boolean uriStatCollectMethod;
+
+        public AbstractUrlHandlerMappingTransform(Boolean uriStatCollectMethod) {
+            this.uriStatCollectMethod = uriStatCollectMethod;
+        }
+
         @Override
         public byte[] doInTransform(Instrumentor instrumentor, ClassLoader classLoader, String className, Class<?> classBeingRedefined, ProtectionDomain protectionDomain, byte[] classfileBuffer) throws InstrumentException {
             final InstrumentClass target = instrumentor.getInstrumentClass(classLoader, className, classfileBuffer);
 
             // Add attribute listener.
             final InstrumentMethod exposePathWithinMapping = target.getDeclaredMethod("lookupHandler", "org.springframework.http.server.PathContainer", "org.springframework.web.server.ServerWebExchange");
             if (exposePathWithinMapping != null) {
-                exposePathWithinMapping.addInterceptor(AbstractUrlHandlerMappingInterceptor.class);
+                exposePathWithinMapping.addInterceptor(AbstractUrlHandlerMappingInterceptor.class, va(uriStatCollectMethod));
             }
             return target.toBytecode();
         }

diff --git a/...src/main/java/com/navercorp/pinpoint/plugin/spring/webflux/SpringWebFluxPluginConfig.java b/...src/main/java/com/navercorp/pinpoint/plugin/spring/webflux/SpringWebFluxPluginConfig.java
@@ -33,7 +33,7 @@ public class SpringWebFluxPluginConfig {
 
     private final boolean uriStatEnable;
     private final boolean uriStatUseUserInput;
-
+    private final boolean uriStatCollectMethod;
     public SpringWebFluxPluginConfig(ProfilerConfig config) {
         Objects.requireNonNull(config, "config");
 
@@ -49,6 +49,7 @@ public SpringWebFluxPluginConfig(ProfilerConfig config) {
         this.httpDumpConfig = HttpDumpConfig.get(cookie, cookieDumpType, cookieSamplingRate, cookieDumpSize, false, cookieDumpType, 1, 1024);
         this.uriStatEnable = config.readBoolean("profiler.uri.stat.spring.webflux.enable", false);
         this.uriStatUseUserInput = config.readBoolean("profiler.uri.stat.spring.webflux.useuserinput", false);
+        this.uriStatCollectMethod = config.readBoolean("profiler.uri.stat.collect.http.method", false);
     }
 
     public boolean isEnable() {
@@ -71,6 +72,10 @@ public boolean isUriStatEnable() {
         return uriStatEnable;
     }
 
+    public boolean isUriStatCollectMethod() {
+        return uriStatCollectMethod;
+    }
+
     public boolean isUriStatUseUserInput() {
         return uriStatUseUserInput;
     }

diff --git a/...p/pinpoint/plugin/spring/webflux/interceptor/AbstractHandlerMethodMappingInterceptor.java b/...p/pinpoint/plugin/spring/webflux/interceptor/AbstractHandlerMethodMappingInterceptor.java
@@ -13,11 +13,12 @@
 
 public class AbstractHandlerMethodMappingInterceptor implements AroundInterceptor {
     private final PLogger logger = PLoggerFactory.getLogger(getClass());
-
     private final TraceContext traceContext;
+    private final Boolean uriStatCollectMethod;
 
-    public AbstractHandlerMethodMappingInterceptor(final TraceContext traceContext) {
+    public AbstractHandlerMethodMappingInterceptor(final TraceContext traceContext, Boolean uriStatCollectMethod) {
         this.traceContext = traceContext;
+        this.uriStatCollectMethod = uriStatCollectMethod;
     }
 
     @Override
@@ -34,11 +35,19 @@ public void after(Object target, Object[] args, Object result, Throwable throwab
         try {
             final ServerWebExchange webExchange = ArrayArgumentUtils.getArgument(args, 0, ServerWebExchange.class);
             if (webExchange != null) {
+                final SpanRecorder spanRecorder = trace.getSpanRecorder();
+
                 final String uri = ServerWebExchangeAttributeUtils.extractAttribute(webExchange, SpringWebFluxConstants.SPRING_WEBFLUX_DEFAULT_URI_ATTRIBUTE_KEYS);
                 if (StringUtils.hasLength(uri)) {
-                    final SpanRecorder spanRecorder = trace.getSpanRecorder();
                     spanRecorder.recordUriTemplate(uri, false);
                 }
+
+                if (uriStatCollectMethod) {
+                    final String method = webExchange.getRequest().getMethodValue();
+                    if (StringUtils.hasLength(method)) {
+                        spanRecorder.recordUriHttpMethod(method);
+                    }
+                }
             }
         } catch (Throwable th) {
             if (logger.isWarnEnabled()) {

diff --git a/...corp/pinpoint/plugin/spring/webflux/interceptor/AbstractUrlHandlerMappingInterceptor.java b/...corp/pinpoint/plugin/spring/webflux/interceptor/AbstractUrlHandlerMappingInterceptor.java
@@ -13,11 +13,12 @@
 
 public class AbstractUrlHandlerMappingInterceptor implements AroundInterceptor {
     private final PLogger logger = PLoggerFactory.getLogger(getClass());
-
     private final TraceContext traceContext;
+    private final Boolean uriStatCollectMethod;
 
-    public AbstractUrlHandlerMappingInterceptor(TraceContext traceContext) {
+    public AbstractUrlHandlerMappingInterceptor(TraceContext traceContext, Boolean uriStatCollectMethod) {
         this.traceContext = traceContext;
+        this.uriStatCollectMethod = uriStatCollectMethod;
     }
 
     @Override
@@ -34,11 +35,19 @@ public void after(Object target, Object[] args, Object result, Throwable throwab
         try {
             final ServerWebExchange webExchange = ArrayArgumentUtils.getArgument(args, 1, ServerWebExchange.class);
             if (webExchange != null) {
+                final SpanRecorder spanRecorder = trace.getSpanRecorder();
+
                 final String uri = ServerWebExchangeAttributeUtils.extractAttribute(webExchange, SpringWebFluxConstants.SPRING_WEBFLUX_DEFAULT_URI_ATTRIBUTE_KEYS);
                 if (StringUtils.hasLength(uri)) {
-                    final SpanRecorder spanRecorder = trace.getSpanRecorder();
                     spanRecorder.recordUriTemplate(uri, false);
                 }
+
+                if (uriStatCollectMethod) {
+                    final String method = webExchange.getRequest().getMethodValue();
+                    if (StringUtils.hasLength(method)) {
+                        spanRecorder.recordUriHttpMethod(method);
+                    }
+                }
             }
         } catch (Throwable th) {
             if (logger.isWarnEnabled()) {

diff --git a/...ins/spring/src/main/java/com/navercorp/pinpoint/plugin/spring/web/SpringWebMvcConfig.java b/...ins/spring/src/main/java/com/navercorp/pinpoint/plugin/spring/web/SpringWebMvcConfig.java
@@ -28,12 +28,14 @@ public class SpringWebMvcConfig {
     private final boolean enable;
     private final boolean uriStatEnable;
     private final boolean uriStatUseUserInput;
+    private final boolean uriStatCollectMethod;
 
     public SpringWebMvcConfig(ProfilerConfig config) {
         Objects.requireNonNull(config, "config");
         this.enable = config.readBoolean("profiler.spring.webmvc.enable", true);
         this.uriStatEnable = config.readBoolean("profiler.uri.stat.spring.webmvc.enable", false);
         this.uriStatUseUserInput = config.readBoolean("profiler.uri.stat.spring.webmvc.useuserinput", false);
+        this.uriStatCollectMethod = config.readBoolean("profiler.uri.stat.collect.http.method", false);
     }
 
     public boolean isEnable() {
@@ -48,6 +50,10 @@ public boolean isUriStatUseUserInput() {
         return uriStatUseUserInput;
     }
 
+    public boolean isUriStatCollectMethod() {
+        return uriStatCollectMethod;
+    }
+
     @Override
     public String toString() {
         return "SpringWebMvcConfig{" +

diff --git a/...ins/spring/src/main/java/com/navercorp/pinpoint/plugin/spring/web/SpringWebMvcPlugin.java b/...ins/spring/src/main/java/com/navercorp/pinpoint/plugin/spring/web/SpringWebMvcPlugin.java
@@ -27,9 +27,7 @@
 import com.navercorp.pinpoint.bootstrap.logging.PLoggerFactory;
 import com.navercorp.pinpoint.bootstrap.plugin.ProfilerPlugin;
 import com.navercorp.pinpoint.bootstrap.plugin.ProfilerPluginSetupContext;
-import com.navercorp.pinpoint.plugin.spring.web.interceptor.ExposePathWithinMappingInterceptor;
 import com.navercorp.pinpoint.plugin.spring.web.interceptor.InvocableHandlerMethodInvokeForRequestMethodInterceptor;
-import com.navercorp.pinpoint.plugin.spring.web.javax.interceptor.LookupHandlerMethodInterceptor;
 import com.navercorp.pinpoint.plugin.spring.web.javax.interceptor.ProcessRequestInterceptor;
 
 import java.security.ProtectionDomain;
@@ -59,48 +57,58 @@ public void setup(ProfilerPluginSetupContext context) {
 
         // uri stat
         if (config.isUriStatEnable()) {
-            transformTemplate.transform("org.springframework.web.servlet.handler.AbstractHandlerMethodMapping", AbstractHandlerMethodMappingTransform.class);
-            transformTemplate.transform("org.springframework.web.servlet.handler.AbstractUrlHandlerMapping", AbstractUrlHandlerMappingTransform.class);
+            transformTemplate.transform("org.springframework.web.servlet.handler.AbstractHandlerMethodMapping", AbstractHandlerMethodMappingTransform.class, new Object[]{config.isUriStatCollectMethod()}, new Class[]{Boolean.class});
+            transformTemplate.transform("org.springframework.web.servlet.handler.AbstractUrlHandlerMapping", AbstractUrlHandlerMappingTransform.class, new Object[]{config.isUriStatCollectMethod()}, new Class[]{Boolean.class});
         }
 
     }
 
     public static class AbstractHandlerMethodMappingTransform implements TransformCallback {
+        private final Boolean isUriStatCollectMethod;
+
+        public AbstractHandlerMethodMappingTransform(Boolean isUriStatCollectMethod) {
+            this.isUriStatCollectMethod = isUriStatCollectMethod;
+
+        }
         @Override
         public byte[] doInTransform(Instrumentor instrumentor, ClassLoader classLoader, String className, Class<?> classBeingRedefined, ProtectionDomain protectionDomain, byte[] classfileBuffer) throws InstrumentException {
             final InstrumentClass target = instrumentor.getInstrumentClass(classLoader, className, classfileBuffer);
 
             // Add attribute listener.
             final InstrumentMethod lookupHandlerMethod = target.getDeclaredMethod("lookupHandlerMethod", "java.lang.String", "javax.servlet.http.HttpServletRequest");
             if (lookupHandlerMethod != null) {
-                lookupHandlerMethod.addInterceptor(LookupHandlerMethodInterceptor.class);
+                lookupHandlerMethod.addInterceptor(com.navercorp.pinpoint.plugin.spring.web.javax.interceptor.LookupHandlerMethodInterceptor.class, va(isUriStatCollectMethod));
             }
 
             // Spring 6
             final InstrumentMethod lookupHandlerMethodJakarta = target.getDeclaredMethod("lookupHandlerMethod", "java.lang.String", "jakarta.servlet.http.HttpServletRequest");
             if (lookupHandlerMethodJakarta != null) {
-                lookupHandlerMethodJakarta.addInterceptor(com.navercorp.pinpoint.plugin.spring.web.jakarta.interceptor.LookupHandlerMethodInterceptor.class);
+                lookupHandlerMethodJakarta.addInterceptor(com.navercorp.pinpoint.plugin.spring.web.jakarta.interceptor.LookupHandlerMethodInterceptor.class, va(isUriStatCollectMethod));
             }
 
             return target.toBytecode();
         }
     }
 
     public static class AbstractUrlHandlerMappingTransform implements TransformCallback {
+        private final Boolean isUriStatCollectMethod;
+        public AbstractUrlHandlerMappingTransform(Boolean isUriStatCollectMethod) {
+            this.isUriStatCollectMethod = isUriStatCollectMethod;
+        }
         @Override
         public byte[] doInTransform(Instrumentor instrumentor, ClassLoader classLoader, String className, Class<?> classBeingRedefined, ProtectionDomain protectionDomain, byte[] classfileBuffer) throws InstrumentException {
             final InstrumentClass target = instrumentor.getInstrumentClass(classLoader, className, classfileBuffer);
 
             // Add attribute listener.
             final InstrumentMethod exposePathWithinMapping = target.getDeclaredMethod("exposePathWithinMapping", "java.lang.String", "java.lang.String", "javax.servlet.http.HttpServletRequest");
             if (exposePathWithinMapping != null) {
-                exposePathWithinMapping.addInterceptor(ExposePathWithinMappingInterceptor.class);
+                exposePathWithinMapping.addInterceptor(com.navercorp.pinpoint.plugin.spring.web.javax.interceptor.ExposePathWithinMappingInterceptor.class, va(this.isUriStatCollectMethod));
             }
 
             // Spring 6
             final InstrumentMethod exposePathWithinMappingJakarta = target.getDeclaredMethod("exposePathWithinMapping", "java.lang.String", "java.lang.String", "jakarta.servlet.http.HttpServletRequest");
             if (exposePathWithinMappingJakarta != null) {
-                exposePathWithinMappingJakarta.addInterceptor(ExposePathWithinMappingInterceptor.class);
+                exposePathWithinMappingJakarta.addInterceptor(com.navercorp.pinpoint.plugin.spring.web.jakarta.interceptor.ExposePathWithinMappingInterceptor.class, va(this.isUriStatCollectMethod));
             }
 
             return target.toBytecode();

diff --git a/...rp/pinpoint/plugin/spring/web/jakarta/interceptor/ExposePathWithinMappingInterceptor.java b/...rp/pinpoint/plugin/spring/web/jakarta/interceptor/ExposePathWithinMappingInterceptor.java
@@ -0,0 +1,55 @@
+package com.navercorp.pinpoint.plugin.spring.web.jakarta.interceptor;
+
+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.common.util.ArrayArgumentUtils;
+import com.navercorp.pinpoint.common.util.StringUtils;
+import jakarta.servlet.http.HttpServletRequest;
+
+
+public class ExposePathWithinMappingInterceptor implements AroundInterceptor {
+    private final PLogger logger = PLoggerFactory.getLogger(getClass());
+    private final TraceContext traceContext;
+    private final Boolean uriStatCollectMethod;
+
+    public ExposePathWithinMappingInterceptor(final TraceContext traceContext, Boolean uriStatCollectMethod) {
+        this.traceContext = traceContext;
+        this.uriStatCollectMethod = uriStatCollectMethod;
+    }
+
+    @Override
+    public void before(Object target, Object[] args) {
+    }
+
+    @Override
+    public void after(Object target, Object[] args, Object result, Throwable throwable) {
+        final Trace trace = traceContext.currentRawTraceObject();
+        if (trace == null) {
+            return;
+        }
+
+        try {
+            final SpanRecorder spanRecorder = trace.getSpanRecorder();
+            final String url = ArrayArgumentUtils.getArgument(args, 0, String.class);
+            if (StringUtils.hasLength(url)) {
+                spanRecorder.recordUriTemplate(url);
+            }
+
+            if (uriStatCollectMethod) {
+                final HttpServletRequest request = ArrayArgumentUtils.getArgument(args, 2, HttpServletRequest.class);
+                final String method = request.getMethod();
+                if (StringUtils.hasLength(method)) {
+                    spanRecorder.recordUriHttpMethod(method);
+                }
+            }
+        } catch (Throwable th) {
+            if (logger.isWarnEnabled()) {
+                logger.warn("AFTER error. Caused:{}", th.getMessage(), th);
+            }
+        }
+    }
+}
diff --git a/...ercorp/pinpoint/plugin/spring/web/jakarta/interceptor/LookupHandlerMethodInterceptor.java b/...ercorp/pinpoint/plugin/spring/web/jakarta/interceptor/LookupHandlerMethodInterceptor.java
@@ -12,12 +12,16 @@
 
 import jakarta.servlet.ServletRequest;
 
+import jakarta.servlet.http.HttpServletRequest;
+
 public class LookupHandlerMethodInterceptor implements AroundInterceptor {
     private final PLogger logger = PLoggerFactory.getLogger(getClass());
     private final TraceContext traceContext;
+    private final Boolean uriStatCollectMethod;
 
-    public LookupHandlerMethodInterceptor(final TraceContext traceContext) {
+    public LookupHandlerMethodInterceptor(final TraceContext traceContext, Boolean uriStatCollectMethod) {
         this.traceContext = traceContext;
+        this.uriStatCollectMethod = uriStatCollectMethod;
     }
 
     @Override
@@ -34,11 +38,19 @@ public void after(Object target, Object[] args, Object result, Throwable throwab
         try {
             final ServletRequest request = ArrayArgumentUtils.getArgument(args, 1, ServletRequest.class);
             if (request != null) {
+                final SpanRecorder spanRecorder = trace.getSpanRecorder();
+
                 final String uri = ServletRequestAttributeUtils.extractAttribute(request, SpringWebMvcConstants.SPRING_MVC_DEFAULT_URI_ATTRIBUTE_KEYS);
                 if (StringUtils.hasLength(uri)) {
-                    final SpanRecorder spanRecorder = trace.getSpanRecorder();
                     spanRecorder.recordUriTemplate(uri);
                 }
+
+                if (uriStatCollectMethod) {
+                    final String method = ((HttpServletRequest) request).getMethod();
+                    if (StringUtils.hasLength(method)) {
+                        spanRecorder.recordUriHttpMethod(method);
+                    }
+                }
             }
         } catch (Throwable th) {
             if (logger.isWarnEnabled()) {