From 99100ed635522fb5142230e7a7c03d8011c2f8a1 Mon Sep 17 00:00:00 2001 From: kezhenxu94 Date: Fri, 3 Jan 2020 21:44:25 +0800 Subject: [PATCH] Split the `collect_http_params` as per plugins for better performance (#4170) * Split the `collect_http_params` as per plugins for better performance * Add missing change --- .../apm/agent/core/conf/Config.java | 24 +++++++++++++------ .../AbstractMethodInterceptor.java | 2 +- .../tomcat78x/TomcatInvokeInterceptor.java | 2 +- .../setup/service-agent/java-agent/README.md | 3 ++- .../armeria-0.96minus-scenario/bin/startup.sh | 2 +- .../httpclient-3.x-scenario/configuration.yml | 2 +- .../spring-3.1.x-scenario/configuration.yml | 2 +- 7 files changed, 24 insertions(+), 13 deletions(-) diff --git a/apm-sniffer/apm-agent-core/src/main/java/org/apache/skywalking/apm/agent/core/conf/Config.java b/apm-sniffer/apm-agent-core/src/main/java/org/apache/skywalking/apm/agent/core/conf/Config.java index 0327b3613..12e020116 100755 --- a/apm-sniffer/apm-agent-core/src/main/java/org/apache/skywalking/apm/agent/core/conf/Config.java +++ b/apm-sniffer/apm-agent-core/src/main/java/org/apache/skywalking/apm/agent/core/conf/Config.java @@ -252,12 +252,26 @@ public class Config { public static Map CONTEXT = new HashMap(); } + public static class Tomcat { + /** + * This config item controls that whether the Tomcat plugin should + * collect the parameters of the request. + */ + public static boolean COLLECT_HTTP_PARAMS = false; + } + public static class SpringMVC { /** * If true, the fully qualified method name will be used as the endpoint name instead of the request URL, * default is false. */ public static boolean USE_QUALIFIED_NAME_AS_ENDPOINT_NAME = false; + + /** + * This config item controls that whether the SpringMVC plugin should + * collect the parameters of the request. + */ + public static boolean COLLECT_HTTP_PARAMS = false; } public static class Toolkit { @@ -356,13 +370,9 @@ public class Config { public static class Http { /** - * This config item controls that whether the plugins related to HTTP should - * collect the parameters of the request. - */ - public static boolean COLLECT_HTTP_PARAMS = false; - /** - * When {@link #COLLECT_HTTP_PARAMS} is enabled, how many characters to keep and - * send to the OAP backend, use negative values to keep and send the complete parameters, + * When either {@link Tomcat#COLLECT_HTTP_PARAMS} or {@link SpringMVC#COLLECT_HTTP_PARAMS} + * is enabled, how many characters to keep and send to the OAP backend, + * use negative values to keep and send the complete parameters, * NB. this config item is added for the sake of performance */ public static int HTTP_PARAMS_LENGTH_THRESHOLD = 1024; diff --git a/apm-sniffer/apm-sdk-plugin/spring-plugins/mvc-annotation-commons/src/main/java/org/apache/skywalking/apm/plugin/spring/mvc/commons/interceptor/AbstractMethodInterceptor.java b/apm-sniffer/apm-sdk-plugin/spring-plugins/mvc-annotation-commons/src/main/java/org/apache/skywalking/apm/plugin/spring/mvc/commons/interceptor/AbstractMethodInterceptor.java index b046eb459..800894113 100644 --- a/apm-sniffer/apm-sdk-plugin/spring-plugins/mvc-annotation-commons/src/main/java/org/apache/skywalking/apm/plugin/spring/mvc/commons/interceptor/AbstractMethodInterceptor.java +++ b/apm-sniffer/apm-sdk-plugin/spring-plugins/mvc-annotation-commons/src/main/java/org/apache/skywalking/apm/plugin/spring/mvc/commons/interceptor/AbstractMethodInterceptor.java @@ -107,7 +107,7 @@ public abstract class AbstractMethodInterceptor implements InstanceMethodsAround span.setComponent(ComponentsDefine.SPRING_MVC_ANNOTATION); SpanLayer.asHttp(span); - if (Config.Plugin.Http.COLLECT_HTTP_PARAMS) { + if (Config.Plugin.SpringMVC.COLLECT_HTTP_PARAMS) { final Map parameterMap = request.getParameterMap(); if (parameterMap != null && !parameterMap.isEmpty()) { String tagValue = CollectionUtil.toString(parameterMap); diff --git a/apm-sniffer/apm-sdk-plugin/tomcat-7.x-8.x-plugin/src/main/java/org/apache/skywalking/apm/plugin/tomcat78x/TomcatInvokeInterceptor.java b/apm-sniffer/apm-sdk-plugin/tomcat-7.x-8.x-plugin/src/main/java/org/apache/skywalking/apm/plugin/tomcat78x/TomcatInvokeInterceptor.java index 112d1c1c5..09751f7ed 100644 --- a/apm-sniffer/apm-sdk-plugin/tomcat-7.x-8.x-plugin/src/main/java/org/apache/skywalking/apm/plugin/tomcat78x/TomcatInvokeInterceptor.java +++ b/apm-sniffer/apm-sdk-plugin/tomcat-7.x-8.x-plugin/src/main/java/org/apache/skywalking/apm/plugin/tomcat78x/TomcatInvokeInterceptor.java @@ -87,7 +87,7 @@ public class TomcatInvokeInterceptor implements InstanceMethodsAroundInterceptor span.setComponent(ComponentsDefine.TOMCAT); SpanLayer.asHttp(span); - if (Config.Plugin.Http.COLLECT_HTTP_PARAMS) { + if (Config.Plugin.Tomcat.COLLECT_HTTP_PARAMS) { final Map parameterMap = new HashMap<>(); final org.apache.coyote.Request coyoteRequest = request.getCoyoteRequest(); final Parameters parameters = coyoteRequest.getParameters(); diff --git a/docs/en/setup/service-agent/java-agent/README.md b/docs/en/setup/service-agent/java-agent/README.md index fbea28897..2b66f6999 100755 --- a/docs/en/setup/service-agent/java-agent/README.md +++ b/docs/en/setup/service-agent/java-agent/README.md @@ -118,7 +118,8 @@ property key | Description | Default | `plugin.opgroup.*`|Support operation name customize group rules in different plugins. Read [Group rule supported plugins](op_name_group_rule.md)|Not set| `plugin.springtransaction.simplify_transaction_definition_name`|If true, the transaction definition name will be simplified.|false| `plugin.jdkthreading.threading_class_prefixes` | Threading classes (`java.lang.Runnable` and `java.util.concurrent.Callable`) and their subclasses, including anonymous inner classes whose name match any one of the `THREADING_CLASS_PREFIXES` (splitted by `,`) will be instrumented, make sure to only specify as narrow prefixes as what you're expecting to instrument, (`java.` and `javax.` will be ignored due to safety issues) | Not set | -`plugin.http.collect_http_params`| This config item controls that whether the plugins related to HTTP should collect the parameters of the request. The name `plugin.http.collect_http_params` is rather general, but it doesn't guarantee that all http plugins should be under its control, unless the plugin itself takes this config item into consideration, such as Tomcat, Spring MVC, and Armeria plugin. | `false` | +`plugin.tomcat.collect_http_params`| This config item controls that whether the Tomcat plugin should collect the parameters of the request. | `false` | +`plugin.springmvc.collect_http_params`| This config item controls that whether the SpringMVC plugin should collect the parameters of the request, when your Spring application is based on Tomcat, consider only setting either `plugin.tomcat.collect_http_params` or `plugin.springmvc.collect_http_params`. | `false` | `plugin.http.http_params_length_threshold`| When `COLLECT_HTTP_PARAMS` is enabled, how many characters to keep and send to the OAP backend, use negative values to keep and send the complete parameters, NB. this config item is added for the sake of performance. | `1024` | ## Optional Plugins diff --git a/test/plugin/scenarios/armeria-0.96minus-scenario/bin/startup.sh b/test/plugin/scenarios/armeria-0.96minus-scenario/bin/startup.sh index 073590256..0298af56d 100644 --- a/test/plugin/scenarios/armeria-0.96minus-scenario/bin/startup.sh +++ b/test/plugin/scenarios/armeria-0.96minus-scenario/bin/startup.sh @@ -18,4 +18,4 @@ home="$(cd "$(dirname $0)"; pwd)" -java -jar -Dskywalking.plugin.http.collect_http_params=true ${agent_opts} ${home}/../libs/armeria-0.96minus-scenario.jar & \ No newline at end of file +java -jar -Dskywalking.plugin.tomcat.collect_http_params=true ${agent_opts} ${home}/../libs/armeria-0.96minus-scenario.jar & \ No newline at end of file diff --git a/test/plugin/scenarios/httpclient-3.x-scenario/configuration.yml b/test/plugin/scenarios/httpclient-3.x-scenario/configuration.yml index e9e2644d7..9c5db9c7b 100644 --- a/test/plugin/scenarios/httpclient-3.x-scenario/configuration.yml +++ b/test/plugin/scenarios/httpclient-3.x-scenario/configuration.yml @@ -19,4 +19,4 @@ entryService: '"http://localhost:8080/httpclient-3.x-scenario/case/httpclient?q1 healthCheck: http://localhost:8080/httpclient-3.x-scenario/healthCheck framework: httpclient environment: - - CATALINA_OPTS="-Dskywalking.plugin.http.collect_http_params=true" \ No newline at end of file + - CATALINA_OPTS="-Dskywalking.plugin.tomcat.collect_http_params=true" \ No newline at end of file diff --git a/test/plugin/scenarios/spring-3.1.x-scenario/configuration.yml b/test/plugin/scenarios/spring-3.1.x-scenario/configuration.yml index bb95c255d..cbe2958cc 100644 --- a/test/plugin/scenarios/spring-3.1.x-scenario/configuration.yml +++ b/test/plugin/scenarios/spring-3.1.x-scenario/configuration.yml @@ -21,4 +21,4 @@ runningMode: with_optional withPlugins: apm-spring-annotation-plugin-*.jar framework: spring environment: - - CATALINA_OPTS="-Dskywalking.plugin.http.collect_http_params=true" \ No newline at end of file + - CATALINA_OPTS="-Dskywalking.plugin.springmvc.collect_http_params=true" \ No newline at end of file