From 56bebb8bafcdc4c0164ae3682c99a44d00312246 Mon Sep 17 00:00:00 2001 From: liqiangz Date: Mon, 29 Mar 2021 10:50:27 +0800 Subject: [PATCH] Remove the close_before_method and close_after_method parameters of custom-enhance-plugin to avoid memory leaks (#6632) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * Remove the close_before_method and close_after_method parameters of custom-enhance-plugin to avoid memory leaks * refine Co-authored-by: 吴晟 Wu Sheng --- CHANGES.md | 1 + .../conf/CustomizeConfiguration.java | 16 ----- .../customize/conf/MethodConfiguration.java | 16 ----- .../plugin/customize/constants/Constants.java | 8 --- .../interceptor/BaseInterceptorMethods.java | 72 +++++++++---------- 5 files changed, 35 insertions(+), 78 deletions(-) diff --git a/CHANGES.md b/CHANGES.md index a464a654f..d5c1d8188 100644 --- a/CHANGES.md +++ b/CHANGES.md @@ -25,6 +25,7 @@ Release Notes. * Support jedis pipeline in jedis-2.x-plugin. * Fix apm-toolkit-log4j-2.x-activation no trace Id in async log. * Replace hbase-1.x-plugin with hbase-1.x-2.x-plugin to adapt hbase client 2.x +* Remove the close_before_method and close_after_method parameters of custom-enhance-plugin to avoid memory leaks. * Fix bug that springmvn-annotation-4.x-plugin, witness class does not exist in some versions. #### OAP-Backend diff --git a/apm-sniffer/optional-plugins/customize-enhance-plugin/src/main/java/org/apache/skywalking/apm/plugin/customize/conf/CustomizeConfiguration.java b/apm-sniffer/optional-plugins/customize-enhance-plugin/src/main/java/org/apache/skywalking/apm/plugin/customize/conf/CustomizeConfiguration.java index 2c1864b8c..ba48a9280 100644 --- a/apm-sniffer/optional-plugins/customize-enhance-plugin/src/main/java/org/apache/skywalking/apm/plugin/customize/conf/CustomizeConfiguration.java +++ b/apm-sniffer/optional-plugins/customize-enhance-plugin/src/main/java/org/apache/skywalking/apm/plugin/customize/conf/CustomizeConfiguration.java @@ -172,22 +172,6 @@ public enum CustomizeConfiguration { Constants.XML_ELEMENT_OPERATION_NAME) .getNodeValue()); } - if (methodDesc.getAttributes().getNamedItem(Constants.XML_ELEMENT_CLOSE_BEFORE_METHOD) != null) { - MethodConfiguration.setCloseBeforeMethod(configuration, Boolean.valueOf(methodDesc.getAttributes() - .getNamedItem( - Constants.XML_ELEMENT_CLOSE_BEFORE_METHOD) - .getNodeValue())); - } else { - MethodConfiguration.setCloseBeforeMethod(configuration, false); - } - if (methodDesc.getAttributes().getNamedItem(Constants.XML_ELEMENT_CLOSE_AFTER_METHOD) != null) { - MethodConfiguration.setCloseAfterMethod(configuration, Boolean.valueOf(methodDesc.getAttributes() - .getNamedItem( - Constants.XML_ELEMENT_CLOSE_AFTER_METHOD) - .getNodeValue())); - } else { - MethodConfiguration.setCloseAfterMethod(configuration, false); - } if (methodDesc.getAttributes().getNamedItem(Constants.XML_ELEMENT_METHOD_IS_STATIC) != null) { MethodConfiguration.setStatic(configuration, Boolean.valueOf(methodDesc.getAttributes() .getNamedItem( diff --git a/apm-sniffer/optional-plugins/customize-enhance-plugin/src/main/java/org/apache/skywalking/apm/plugin/customize/conf/MethodConfiguration.java b/apm-sniffer/optional-plugins/customize-enhance-plugin/src/main/java/org/apache/skywalking/apm/plugin/customize/conf/MethodConfiguration.java index 89d544b88..375672ed9 100644 --- a/apm-sniffer/optional-plugins/customize-enhance-plugin/src/main/java/org/apache/skywalking/apm/plugin/customize/conf/MethodConfiguration.java +++ b/apm-sniffer/optional-plugins/customize-enhance-plugin/src/main/java/org/apache/skywalking/apm/plugin/customize/conf/MethodConfiguration.java @@ -55,14 +55,6 @@ public class MethodConfiguration { configuration.put(Constants.CONFIGURATION_ATTRIBUTE_OPERATION_NAME, operationName); } - static void setCloseBeforeMethod(Map configuration, Boolean closeBeforeMethod) { - configuration.put(Constants.CONFIGURATION_ATTRIBUTE_CLOSE_BEFORE_METHOD, closeBeforeMethod); - } - - static void setCloseAfterMethod(Map configuration, Boolean closeAfterMethod) { - configuration.put(Constants.CONFIGURATION_ATTRIBUTE_CLOSE_AFTER_METHOD, closeAfterMethod); - } - static void setStatic(Map configuration, Boolean isStatic) { configuration.put(Constants.CONFIGURATION_ATTRIBUTE_IS_STATIC, isStatic); } @@ -123,14 +115,6 @@ public class MethodConfiguration { return (String) configuration.get(Constants.CONFIGURATION_ATTRIBUTE_OPERATION_NAME); } - public static boolean isCloseBeforeMethod(Map configuration) { - return (Boolean) configuration.get(Constants.CONFIGURATION_ATTRIBUTE_CLOSE_BEFORE_METHOD); - } - - public static boolean isCloseAfterMethod(Map configuration) { - return (Boolean) configuration.get(Constants.CONFIGURATION_ATTRIBUTE_CLOSE_AFTER_METHOD); - } - @SuppressWarnings("unchecked") public static Map getTags(Map configuration) { return (Map) configuration.get(Constants.CONFIGURATION_ATTRIBUTE_TAGS); diff --git a/apm-sniffer/optional-plugins/customize-enhance-plugin/src/main/java/org/apache/skywalking/apm/plugin/customize/constants/Constants.java b/apm-sniffer/optional-plugins/customize-enhance-plugin/src/main/java/org/apache/skywalking/apm/plugin/customize/constants/Constants.java index b9f18634d..87e5c8759 100644 --- a/apm-sniffer/optional-plugins/customize-enhance-plugin/src/main/java/org/apache/skywalking/apm/plugin/customize/constants/Constants.java +++ b/apm-sniffer/optional-plugins/customize-enhance-plugin/src/main/java/org/apache/skywalking/apm/plugin/customize/constants/Constants.java @@ -42,10 +42,6 @@ public class Constants { public static final String XML_ELEMENT_OPERATION_NAME = "operation_name"; - public static final String XML_ELEMENT_CLOSE_BEFORE_METHOD = "close_before_method"; - - public static final String XML_ELEMENT_CLOSE_AFTER_METHOD = "close_after_method"; - public static final String XML_ELEMENT_OPERATION_NAME_SUFFIX = "operation_name_suffix"; public static final String XML_ELEMENT_TAG = "tag"; @@ -70,10 +66,6 @@ public class Constants { public static final String CONFIGURATION_ATTRIBUTE_OPERATION_NAME = "CONFIGURATION_ATTRIBUTE_OPERATION_NAME"; - public static final String CONFIGURATION_ATTRIBUTE_CLOSE_BEFORE_METHOD = "CONFIGURATION_ATTRIBUTE_CLOSE_BEFORE_METHOD"; - - public static final String CONFIGURATION_ATTRIBUTE_CLOSE_AFTER_METHOD = "CONFIGURATION_ATTRIBUTE_CLOSE_AFTER_METHOD"; - public static final String CONFIGURATION_ATTRIBUTE_OPERATION_NAME_SUFFIXES = "CONFIGURATION_ATTRIBUTE_OPERATION_NAME_SUFFIXES"; public static final String CONFIGURATION_ATTRIBUTE_TAGS = "CONFIGURATION_ATTRIBUTE_TAGS"; diff --git a/apm-sniffer/optional-plugins/customize-enhance-plugin/src/main/java/org/apache/skywalking/apm/plugin/customize/interceptor/BaseInterceptorMethods.java b/apm-sniffer/optional-plugins/customize-enhance-plugin/src/main/java/org/apache/skywalking/apm/plugin/customize/interceptor/BaseInterceptorMethods.java index 109766349..dcef43ec7 100644 --- a/apm-sniffer/optional-plugins/customize-enhance-plugin/src/main/java/org/apache/skywalking/apm/plugin/customize/interceptor/BaseInterceptorMethods.java +++ b/apm-sniffer/optional-plugins/customize-enhance-plugin/src/main/java/org/apache/skywalking/apm/plugin/customize/interceptor/BaseInterceptorMethods.java @@ -35,56 +35,52 @@ class BaseInterceptorMethods { void beforeMethod(Method method, Object[] allArguments) { Map configuration = CustomizeConfiguration.INSTANCE.getConfiguration(method); - if (!MethodConfiguration.isCloseBeforeMethod(configuration)) { - String operationName = MethodConfiguration.getOperationName(configuration); - Map context = CustomizeExpression.evaluationContext(allArguments); - if (context == null || context.isEmpty()) { - ContextManager.createLocalSpan(operationName); - } else { + String operationName = MethodConfiguration.getOperationName(configuration); + Map context = CustomizeExpression.evaluationContext(allArguments); + if (context == null || context.isEmpty()) { + ContextManager.createLocalSpan(operationName); + } else { - Map tags = MethodConfiguration.getTags(configuration); - Map spanTags = new HashMap(); - Map logs = MethodConfiguration.getLogs(configuration); - Map spanLogs = new HashMap(); + Map tags = MethodConfiguration.getTags(configuration); + Map spanTags = new HashMap(); + Map logs = MethodConfiguration.getLogs(configuration); + Map spanLogs = new HashMap(); - List operationNameSuffixes = MethodConfiguration.getOperationNameSuffixes(configuration); - StringBuilder operationNameSuffix = new StringBuilder(); - if (operationNameSuffixes != null && !operationNameSuffixes.isEmpty()) { - for (String expression : operationNameSuffixes) { - operationNameSuffix.append(Constants.OPERATION_NAME_SEPARATOR); - operationNameSuffix.append(CustomizeExpression.parseExpression(expression, context)); - } + List operationNameSuffixes = MethodConfiguration.getOperationNameSuffixes(configuration); + StringBuilder operationNameSuffix = new StringBuilder(); + if (operationNameSuffixes != null && !operationNameSuffixes.isEmpty()) { + for (String expression : operationNameSuffixes) { + operationNameSuffix.append(Constants.OPERATION_NAME_SEPARATOR); + operationNameSuffix.append(CustomizeExpression.parseExpression(expression, context)); } - if (tags != null && !tags.isEmpty()) { - for (Map.Entry expression: tags.entrySet()) { - spanTags.put(expression.getKey(), CustomizeExpression.parseExpression(expression.getValue(), context)); - } + } + if (tags != null && !tags.isEmpty()) { + for (Map.Entry expression : tags.entrySet()) { + spanTags.put(expression.getKey(), CustomizeExpression.parseExpression(expression.getValue(), context)); } - if (logs != null && !logs.isEmpty()) { - for (Map.Entry entries : logs.entrySet()) { - String expression = logs.get(entries.getKey()); - spanLogs.put(entries.getKey(), CustomizeExpression.parseExpression(expression, context)); - } + } + if (logs != null && !logs.isEmpty()) { + for (Map.Entry entries : logs.entrySet()) { + String expression = logs.get(entries.getKey()); + spanLogs.put(entries.getKey(), CustomizeExpression.parseExpression(expression, context)); } - operationName = operationNameSuffix.insert(0, operationName).toString(); + } + operationName = operationNameSuffix.insert(0, operationName).toString(); - AbstractSpan span = ContextManager.createLocalSpan(operationName); - if (!spanTags.isEmpty()) { - for (Map.Entry tag : spanTags.entrySet()) { - span.tag(Tags.ofKey(tag.getKey()), tag.getValue()); - } - } - if (!spanLogs.isEmpty()) { - span.log(System.currentTimeMillis(), spanLogs); + AbstractSpan span = ContextManager.createLocalSpan(operationName); + if (!spanTags.isEmpty()) { + for (Map.Entry tag : spanTags.entrySet()) { + span.tag(Tags.ofKey(tag.getKey()), tag.getValue()); } } + if (!spanLogs.isEmpty()) { + span.log(System.currentTimeMillis(), spanLogs); + } } } void afterMethod(Method method) { - if (!MethodConfiguration.isCloseAfterMethod(CustomizeConfiguration.INSTANCE.getConfiguration(method))) { - ContextManager.stopSpan(); - } + ContextManager.stopSpan(); } void handleMethodException(Throwable t) {