You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@servicecomb.apache.org by li...@apache.org on 2021/11/25 01:15:46 UTC

[servicecomb-java-chassis] branch master updated: SCB-2360 Provides a unified switch to control exception stack printing (#2644)

This is an automated email from the ASF dual-hosted git repository.

liubao pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/servicecomb-java-chassis.git


The following commit(s) were added to refs/heads/master by this push:
     new 8207a76  SCB-2360 Provides a unified switch to control exception stack printing (#2644)
8207a76 is described below

commit 8207a76ecb0620f989173ce5c5a776aaaea0af4d
Author: zyl <72...@users.noreply.github.com>
AuthorDate: Thu Nov 25 09:15:38 2021 +0800

    SCB-2360 Provides a unified switch to control exception stack printing (#2644)
---
 .../core/ConfigurationSpringInitializer.java       | 25 ++++++++++++-------
 .../java/org/apache/servicecomb/core/Const.java    |  3 +++
 .../handler/impl/ProducerOperationHandler.java     | 16 +++++++++---
 .../servicecomb/bizkeeper/BizkeeperCommand.java    |  9 ++++++-
 .../authentication/provider/AccessController.java  | 29 +++++++++++++++-------
 5 files changed, 59 insertions(+), 23 deletions(-)

diff --git a/core/src/main/java/org/apache/servicecomb/core/ConfigurationSpringInitializer.java b/core/src/main/java/org/apache/servicecomb/core/ConfigurationSpringInitializer.java
index f2e95d2..d61d370 100644
--- a/core/src/main/java/org/apache/servicecomb/core/ConfigurationSpringInitializer.java
+++ b/core/src/main/java/org/apache/servicecomb/core/ConfigurationSpringInitializer.java
@@ -55,6 +55,7 @@ import org.springframework.core.env.PropertySource;
 
 import com.google.common.eventbus.Subscribe;
 import com.netflix.config.ConfigurationManager;
+import com.netflix.config.DynamicPropertyFactory;
 import com.netflix.config.WatchedUpdateResult;
 
 /**
@@ -232,15 +233,21 @@ public class ConfigurationSpringInitializer extends PropertySourcesPlaceholderCo
 
   private void addDynamicConfigurationToSpring(Environment environment,
       ConfigCenterConfigurationSource configCenterConfigurationSource) {
-    if (environment instanceof ConfigurableEnvironment) {
-      ConfigurableEnvironment ce = (ConfigurableEnvironment) environment;
-      if (configCenterConfigurationSource != null) {
-        try {
-          ce.getPropertySources()
-              .addFirst(new MapPropertySource("dynamic-source", dynamicData));
-        } catch (Exception e) {
-          LOGGER.warn("set up spring property source failed. msg: {}", e.getMessage());
-        }
+    if (!(environment instanceof ConfigurableEnvironment)) {
+      return;
+    }
+    ConfigurableEnvironment ce = (ConfigurableEnvironment) environment;
+    if (configCenterConfigurationSource == null) {
+      return;
+    }
+    try {
+      ce.getPropertySources().addFirst(new MapPropertySource("dynamic-source", dynamicData));
+    } catch (Exception e) {
+      if (DynamicPropertyFactory.getInstance().getBooleanProperty(Const.PRINT_SENSITIVE_ERROR_MESSAGE,
+          false).get()) {
+        LOGGER.warn("set up spring property source failed.", e);
+      } else {
+        LOGGER.warn("set up spring property source failed. msg: {}", e.getMessage());
       }
     }
   }
diff --git a/core/src/main/java/org/apache/servicecomb/core/Const.java b/core/src/main/java/org/apache/servicecomb/core/Const.java
index cd44333..03c751d 100644
--- a/core/src/main/java/org/apache/servicecomb/core/Const.java
+++ b/core/src/main/java/org/apache/servicecomb/core/Const.java
@@ -52,4 +52,7 @@ public final class Const {
   public static final String AUTH_TOKEN = "x-cse-auth-rsatoken";
 
   public static final String TRACE_ID_NAME = "X-B3-TraceId";
+
+  // controlling whether to print stack information with sensitive errors
+  public static final String PRINT_SENSITIVE_ERROR_MESSAGE = "servicecomb.error.printSensitiveErrorMessage";
 }
diff --git a/core/src/main/java/org/apache/servicecomb/core/handler/impl/ProducerOperationHandler.java b/core/src/main/java/org/apache/servicecomb/core/handler/impl/ProducerOperationHandler.java
index 488a504..a7050f8 100644
--- a/core/src/main/java/org/apache/servicecomb/core/handler/impl/ProducerOperationHandler.java
+++ b/core/src/main/java/org/apache/servicecomb/core/handler/impl/ProducerOperationHandler.java
@@ -20,6 +20,7 @@ package org.apache.servicecomb.core.handler.impl;
 import java.lang.reflect.InvocationTargetException;
 import java.util.concurrent.CompletableFuture;
 
+import org.apache.servicecomb.core.Const;
 import org.apache.servicecomb.core.Handler;
 import org.apache.servicecomb.core.Invocation;
 import org.apache.servicecomb.core.exception.ExceptionUtils;
@@ -34,6 +35,8 @@ import org.apache.servicecomb.swagger.invocation.extension.ProducerInvokeExtensi
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
+import com.netflix.config.DynamicPropertyFactory;
+
 public class ProducerOperationHandler implements Handler {
   private static final Logger LOGGER = LoggerFactory.getLogger(ProducerOperationHandler.class);
 
@@ -125,10 +128,15 @@ public class ProducerOperationHandler implements Handler {
       invocation.onBusinessMethodFinish();
       invocation.onBusinessFinish();
     } catch (Throwable e) {
-      if (shouldPrintErrorLog(e)) {
-        invocation.getTraceIdLogger().error(LOGGER, "unexpected error operation={}, message={}",
-            invocation.getInvocationQualifiedName(),
-            org.apache.servicecomb.foundation.common.utils.ExceptionUtils.getExceptionMessageWithoutTrace(e));
+      if (DynamicPropertyFactory.getInstance().getBooleanProperty(Const.PRINT_SENSITIVE_ERROR_MESSAGE,
+          false).get()) {
+        LOGGER.error("unexpected error operation={}", invocation.getInvocationQualifiedName(), e);
+      } else {
+        if (shouldPrintErrorLog(e)) {
+          invocation.getTraceIdLogger().error(LOGGER, "unexpected error operation={}, message={}",
+              invocation.getInvocationQualifiedName(),
+              org.apache.servicecomb.foundation.common.utils.ExceptionUtils.getExceptionMessageWithoutTrace(e));
+        }
       }
       invocation.onBusinessMethodFinish();
       invocation.onBusinessFinish();
diff --git a/handlers/handler-bizkeeper/src/main/java/org/apache/servicecomb/bizkeeper/BizkeeperCommand.java b/handlers/handler-bizkeeper/src/main/java/org/apache/servicecomb/bizkeeper/BizkeeperCommand.java
index a582041..83818c8 100644
--- a/handlers/handler-bizkeeper/src/main/java/org/apache/servicecomb/bizkeeper/BizkeeperCommand.java
+++ b/handlers/handler-bizkeeper/src/main/java/org/apache/servicecomb/bizkeeper/BizkeeperCommand.java
@@ -17,11 +17,13 @@
 
 package org.apache.servicecomb.bizkeeper;
 
+import org.apache.servicecomb.core.Const;
 import org.apache.servicecomb.core.Invocation;
 import org.apache.servicecomb.swagger.invocation.Response;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
+import com.netflix.config.DynamicPropertyFactory;
 import com.netflix.hystrix.HystrixObservableCommand;
 import com.netflix.hystrix.strategy.concurrency.HystrixRequestContext;
 
@@ -65,7 +67,12 @@ public abstract class BizkeeperCommand extends HystrixObservableCommand<Response
         f.onNext(FallbackPolicyManager.getFallbackResponse(type, cause, invocation));
         f.onCompleted();
       } catch (Exception e) {
-        LOG.warn("fallback failed due to:" + e.getMessage());
+        if (DynamicPropertyFactory.getInstance().getBooleanProperty(Const.PRINT_SENSITIVE_ERROR_MESSAGE,
+            false).get()) {
+          LOG.warn("fallback failed.", e);
+        } else {
+          LOG.warn("fallback failed due to: {}", e.getMessage());
+        }
         throw e;
       }
     });
diff --git a/handlers/handler-publickey-auth/src/main/java/org/apache/servicecomb/authentication/provider/AccessController.java b/handlers/handler-publickey-auth/src/main/java/org/apache/servicecomb/authentication/provider/AccessController.java
index 561d823..e5a991d 100644
--- a/handlers/handler-publickey-auth/src/main/java/org/apache/servicecomb/authentication/provider/AccessController.java
+++ b/handlers/handler-publickey-auth/src/main/java/org/apache/servicecomb/authentication/provider/AccessController.java
@@ -23,6 +23,7 @@ import java.util.Map;
 import java.util.Map.Entry;
 
 import org.apache.commons.lang3.StringUtils;
+import org.apache.servicecomb.core.Const;
 import org.apache.servicecomb.registry.api.registry.Microservice;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
@@ -70,14 +71,14 @@ public class AccessController {
   }
 
   private boolean whiteAllowed(Microservice microservice) {
-    if(whiteList.isEmpty()) {
+    if (whiteList.isEmpty()) {
       return true;
     }
     return matchFound(microservice, whiteList);
   }
 
   private boolean blackDenied(Microservice microservice) {
-    if(blackList.isEmpty()) {
+    if (blackList.isEmpty()) {
       return false;
     }
     return matchFound(microservice, blackList);
@@ -88,8 +89,9 @@ public class AccessController {
     for (ConfigurationItem item : ruleList.values()) {
       if (ConfigurationItem.CATEGORY_PROPERTY.equals(item.category)) {
         // we support to configure properties, e.g. serviceName, appId, environment, alias, version and so on, also support key in properties.
-        if (matchMicroserviceField(microservice, item) || matchMicroserviceProperties(microservice, item))
+        if (matchMicroserviceField(microservice, item) || matchMicroserviceProperties(microservice, item)) {
           return true;
+        }
       }
     }
     return matched;
@@ -98,8 +100,9 @@ public class AccessController {
   private boolean matchMicroserviceProperties(Microservice microservice, ConfigurationItem item) {
     Map<String, String> properties = microservice.getProperties();
     for (Entry<String, String> entry : properties.entrySet()) {
-      if (!entry.getKey().equals(item.propertyName))
+      if (!entry.getKey().equals(item.propertyName)) {
         continue;
+      }
       return isPatternMatch(entry.getValue(), item.rule);
     }
     return false;
@@ -110,11 +113,19 @@ public class AccessController {
     try {
       fieldValue = new PropertyDescriptor(item.propertyName, Microservice.class).getReadMethod().invoke(microservice);
     } catch (Exception e) {
-      LOG.warn("can't find propertyname: {} in microservice field, will search in microservice properties.", item.propertyName);
+      if (DynamicPropertyFactory.getInstance().getBooleanProperty(Const.PRINT_SENSITIVE_ERROR_MESSAGE,
+          false).get()) {
+        LOG.warn("can't find propertyname: {} in microservice field, will search in microservice properties.",
+            item.propertyName, e);
+      } else {
+        LOG.warn("can't find propertyname: {} in microservice field, will search in microservice properties.",
+            item.propertyName);
+      }
       return false;
     }
-    if (fieldValue.getClass().getName().equals(String.class.getName()))
+    if (fieldValue.getClass().getName().equals(String.class.getName())) {
       return isPatternMatch((String) fieldValue, item.rule);
+    }
     return false;
   }
 
@@ -182,9 +193,9 @@ public class AccessController {
     configurations.entrySet().forEach(stringConfigurationItemEntry -> {
       ConfigurationItem item = stringConfigurationItemEntry.getValue();
       LOG.info((isWhite ? "White list " : "Black list ") + "config item: key=" + stringConfigurationItemEntry.getKey()
-              + ";category=" + item.category
-              + ";propertyName=" + item.propertyName
-              + ";rule=" + item.rule);
+          + ";category=" + item.category
+          + ";propertyName=" + item.propertyName
+          + ";rule=" + item.rule);
     });
   }
 }