You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@servicecomb.apache.org by wu...@apache.org on 2020/12/23 18:44:01 UTC

[servicecomb-java-chassis] branch master updated: [SCB-2170] add configuration to control log invocation exception stack trace

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

wujimin 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 4546c6d  [SCB-2170] add configuration to control log invocation exception stack trace
4546c6d is described below

commit 4546c6d6ef7a06e138cd38eb7922537599917977
Author: wujimin <wu...@huawei.com>
AuthorDate: Thu Dec 24 01:40:38 2020 +0800

    [SCB-2170] add configuration to control log invocation exception stack trace
---
 .../servicecomb/core/exception/Exceptions.java     | 23 ++++++++++++++--
 .../core/invocation/ProducerInvocationFlow.java    | 32 ++++++++++++++++++++--
 .../core/provider/consumer/InvokerUtils.java       | 30 ++++++++++++++++----
 core/src/main/resources/microservice.yaml          |  3 ++
 .../highway/HighwayProducerInvocationFlow.java     | 14 +++++++++-
 5 files changed, 90 insertions(+), 12 deletions(-)

diff --git a/core/src/main/java/org/apache/servicecomb/core/exception/Exceptions.java b/core/src/main/java/org/apache/servicecomb/core/exception/Exceptions.java
index 51b6e8a..7166d6b 100644
--- a/core/src/main/java/org/apache/servicecomb/core/exception/Exceptions.java
+++ b/core/src/main/java/org/apache/servicecomb/core/exception/Exceptions.java
@@ -32,6 +32,7 @@ import javax.ws.rs.core.Response.StatusType;
 
 import org.apache.servicecomb.core.Invocation;
 import org.apache.servicecomb.foundation.common.concurrent.ConcurrentHashMapEx;
+import org.apache.servicecomb.foundation.common.utils.ExceptionUtils;
 import org.apache.servicecomb.foundation.common.utils.SPIServiceUtils;
 import org.apache.servicecomb.swagger.invocation.Response;
 import org.apache.servicecomb.swagger.invocation.exception.ExceptionFactory;
@@ -39,6 +40,8 @@ import org.apache.servicecomb.swagger.invocation.exception.InvocationException;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
+import com.netflix.config.DynamicPropertyFactory;
+
 public final class Exceptions {
   private static final Logger LOGGER = LoggerFactory.getLogger(Exceptions.class);
 
@@ -113,13 +116,24 @@ public final class Exceptions {
       StatusType genericStatus) {
     InvocationException invocationException = Exceptions.convert(invocation, exception, genericStatus);
     if (invocation != null) {
-      LOGGER.error("failed to process {} invocation, operation={}.",
-          invocation.getInvocationType(), invocation.getMicroserviceQualifiedName(), invocationException);
+      logException(invocation, invocationException);
     }
     return Response.status(invocationException.getStatus())
         .entity(invocationException.getErrorData());
   }
 
+  private static void logException(@Nonnull Invocation invocation, InvocationException invocationException) {
+    if (isPrintInvocationStackTrace()) {
+      LOGGER.error("failed to process {} invocation, operation={}.",
+          invocation.getInvocationType(), invocation.getMicroserviceQualifiedName(), invocationException);
+      return;
+    }
+
+    LOGGER.error("failed to process {} invocation, operation={}, message={}.",
+        invocation.getInvocationType(), invocation.getMicroserviceQualifiedName(),
+        ExceptionUtils.getExceptionMessageWithoutTrace(invocationException));
+  }
+
   public static InvocationException convert(@Nonnull Invocation invocation, Throwable throwable) {
     StatusType genericStatus = getGenericStatus(invocation);
     return convert(invocation, throwable, genericStatus);
@@ -141,4 +155,9 @@ public final class Exceptions {
 
     throw new IllegalStateException("never happened: can not find converter for " + throwable.getClass().getName());
   }
+
+  public static boolean isPrintInvocationStackTrace() {
+    return DynamicPropertyFactory.getInstance()
+        .getBooleanProperty("servicecomb.exception.invocation.print-stack-trace", false).get();
+  }
 }
diff --git a/core/src/main/java/org/apache/servicecomb/core/invocation/ProducerInvocationFlow.java b/core/src/main/java/org/apache/servicecomb/core/invocation/ProducerInvocationFlow.java
index 847936d..93af06c 100644
--- a/core/src/main/java/org/apache/servicecomb/core/invocation/ProducerInvocationFlow.java
+++ b/core/src/main/java/org/apache/servicecomb/core/invocation/ProducerInvocationFlow.java
@@ -19,6 +19,7 @@ package org.apache.servicecomb.core.invocation;
 import org.apache.servicecomb.core.Invocation;
 import org.apache.servicecomb.core.exception.Exceptions;
 import org.apache.servicecomb.foundation.common.utils.AsyncUtils;
+import org.apache.servicecomb.foundation.common.utils.ExceptionUtils;
 import org.apache.servicecomb.foundation.vertx.http.HttpServletRequestEx;
 import org.apache.servicecomb.foundation.vertx.http.HttpServletResponseEx;
 import org.apache.servicecomb.swagger.invocation.Response;
@@ -68,18 +69,43 @@ public abstract class ProducerInvocationFlow {
   private void finishInvocation(Invocation invocation, Response response, Throwable throwable) {
     invocation.onFinish(response);
 
+    tryLogException(invocation, throwable);
+  }
+
+  private void tryLogException(Invocation invocation, Throwable throwable) {
     if (throwable == null) {
       return;
     }
 
     throwable = Exceptions.unwrap(throwable);
     if (requestEx == null) {
-      LOGGER.error("Failed to finish invocation, operation:{}", invocation.getMicroserviceQualifiedName(), throwable);
+      logException(invocation, throwable);
+      return;
+    }
+
+    logException(invocation, requestEx, throwable);
+  }
+
+  private void logException(Invocation invocation, Throwable throwable) {
+    if (Exceptions.isPrintInvocationStackTrace()) {
+      LOGGER.error("Failed to finish invocation, operation:{}.", invocation.getMicroserviceQualifiedName(), throwable);
+      return;
+    }
+
+    LOGGER.error("Failed to finish invocation, operation:{}, message={}.", invocation.getMicroserviceQualifiedName(),
+        ExceptionUtils.getExceptionMessageWithoutTrace(throwable));
+  }
+
+  private void logException(Invocation invocation, HttpServletRequestEx requestEx, Throwable throwable) {
+    if (Exceptions.isPrintInvocationStackTrace()) {
+      LOGGER.error("Failed to finish invocation, operation:{}, request uri:{}.",
+          invocation.getMicroserviceQualifiedName(), requestEx.getRequestURI(), throwable);
       return;
     }
 
-    LOGGER.error("Failed to finish invocation, operation:{}, request uri:{}",
-        invocation.getMicroserviceQualifiedName(), requestEx.getRequestURI(), throwable);
+    LOGGER.error("Failed to finish invocation, operation:{}, request uri:{}, message={}.",
+        invocation.getMicroserviceQualifiedName(), requestEx.getRequestURI(),
+        ExceptionUtils.getExceptionMessageWithoutTrace(throwable));
   }
 
   protected abstract Invocation sendCreateInvocationException(Throwable throwable);
diff --git a/core/src/main/java/org/apache/servicecomb/core/provider/consumer/InvokerUtils.java b/core/src/main/java/org/apache/servicecomb/core/provider/consumer/InvokerUtils.java
index b804f36..c14cc24 100644
--- a/core/src/main/java/org/apache/servicecomb/core/provider/consumer/InvokerUtils.java
+++ b/core/src/main/java/org/apache/servicecomb/core/provider/consumer/InvokerUtils.java
@@ -33,6 +33,7 @@ import org.apache.servicecomb.core.definition.OperationMeta;
 import org.apache.servicecomb.core.definition.SchemaMeta;
 import org.apache.servicecomb.core.exception.Exceptions;
 import org.apache.servicecomb.core.invocation.InvocationFactory;
+import org.apache.servicecomb.foundation.common.utils.ExceptionUtils;
 import org.apache.servicecomb.swagger.invocation.AsyncResponse;
 import org.apache.servicecomb.swagger.invocation.Response;
 import org.apache.servicecomb.swagger.invocation.context.ContextUtils;
@@ -229,18 +230,35 @@ public final class InvokerUtils {
     return invocation.getMicroserviceMeta().getFilterChain()
         .onFilter(invocation)
         .exceptionally(throwable -> convertException(invocation, throwable))
-        .whenComplete((response, throwable) -> processMetrics(invocation, response));
+        .whenComplete((response, throwable) -> finishInvocation(invocation, response, throwable));
   }
 
   private static Response convertException(Invocation invocation, Throwable throwable) {
-    InvocationException invocationException = Exceptions.convert(invocation, throwable);
+    throw Exceptions.convert(invocation, throwable);
+  }
+
+  private static void finishInvocation(Invocation invocation, Response response, Throwable throwable) {
+    processMetrics(invocation, response);
+    tryLogException(invocation, throwable);
+  }
+
+  private static void tryLogException(Invocation invocation, Throwable throwable) {
+    if (throwable == null) {
+      return;
+    }
+
+    if (Exceptions.isPrintInvocationStackTrace()) {
+      LOGGER.error("failed to invoke {}, endpoint={}.",
+          invocation.getMicroserviceQualifiedName(),
+          invocation.getEndpoint(),
+          throwable);
+      return;
+    }
 
-    LOGGER.error("failed to invoke {}, endpoint={}.",
+    LOGGER.error("failed to invoke {}, endpoint={}, message={}.",
         invocation.getMicroserviceQualifiedName(),
         invocation.getEndpoint(),
-        invocationException);
-
-    throw invocationException;
+        ExceptionUtils.getExceptionMessageWithoutTrace(throwable));
   }
 
   private static void processMetrics(Invocation invocation, Response response) {
diff --git a/core/src/main/resources/microservice.yaml b/core/src/main/resources/microservice.yaml
index 8b505cd..969e8ab 100644
--- a/core/src/main/resources/microservice.yaml
+++ b/core/src/main/resources/microservice.yaml
@@ -18,6 +18,9 @@
 servicecomb-config-order: -500
 
 servicecomb:
+  exception:
+    invocation:
+      print-stack-trace: false
   filter-chains:
     enabled: false
     transport:
diff --git a/transports/transport-highway/src/main/java/org/apache/servicecomb/transport/highway/HighwayProducerInvocationFlow.java b/transports/transport-highway/src/main/java/org/apache/servicecomb/transport/highway/HighwayProducerInvocationFlow.java
index 13f8204..36e0af5 100644
--- a/transports/transport-highway/src/main/java/org/apache/servicecomb/transport/highway/HighwayProducerInvocationFlow.java
+++ b/transports/transport-highway/src/main/java/org/apache/servicecomb/transport/highway/HighwayProducerInvocationFlow.java
@@ -17,8 +17,10 @@
 package org.apache.servicecomb.transport.highway;
 
 import org.apache.servicecomb.core.Invocation;
+import org.apache.servicecomb.core.exception.Exceptions;
 import org.apache.servicecomb.core.invocation.InvocationCreator;
 import org.apache.servicecomb.core.invocation.ProducerInvocationFlow;
+import org.apache.servicecomb.foundation.common.utils.ExceptionUtils;
 import org.apache.servicecomb.foundation.vertx.tcp.TcpConnection;
 import org.apache.servicecomb.swagger.invocation.Response;
 import org.slf4j.Logger;
@@ -39,10 +41,20 @@ public class HighwayProducerInvocationFlow extends ProducerInvocationFlow {
 
   @Override
   protected Invocation sendCreateInvocationException(Throwable throwable) {
-    LOGGER.error("Failed to prepare invocation, msgId={}", msgId, throwable);
+    logException(throwable);
     return null;
   }
 
+  private void logException(Throwable throwable) {
+    if (Exceptions.isPrintInvocationStackTrace()) {
+      LOGGER.error("Failed to prepare invocation, msgId={}.", msgId, throwable);
+      return;
+    }
+
+    LOGGER.error("Failed to prepare invocation, msgId={}, message={}.", msgId,
+        ExceptionUtils.getExceptionMessageWithoutTrace(throwable));
+  }
+
   @Override
   protected void sendResponse(Invocation invocation, Response response) {
     HighwayTransportContext transportContext = invocation.getTransportContext();