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 d1983e5  [SCB-2158] optimize exception log
d1983e5 is described below

commit d1983e557605ada80172488553dfe6cd15b39147
Author: wujimin <wuji...@huawei.com>
AuthorDate: Thu Dec 10 01:49:57 2020 +0800

    [SCB-2158] optimize exception log
---
 .../servicecomb/core/exception/Exceptions.java       |  8 ++++++++
 .../converter/DefaultExceptionConverter.java         |  4 ++--
 .../core/filter/impl/ProducerOperationFilter.java    | 20 ++------------------
 .../core/provider/consumer/InvokerUtils.java         |  9 ++++++++-
 .../provider/pojo/FilterInvocationCaller.java        | 11 +++++++++++
 5 files changed, 31 insertions(+), 21 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 fa6115f..51b6e8a 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
@@ -36,8 +36,12 @@ import 
org.apache.servicecomb.foundation.common.utils.SPIServiceUtils;
 import org.apache.servicecomb.swagger.invocation.Response;
 import org.apache.servicecomb.swagger.invocation.exception.ExceptionFactory;
 import org.apache.servicecomb.swagger.invocation.exception.InvocationException;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
 
 public final class Exceptions {
+  private static final Logger LOGGER = 
LoggerFactory.getLogger(Exceptions.class);
+
   @SuppressWarnings("unchecked")
   private static final List<ExceptionConverter<Throwable>> CONVERTERS = 
SPIServiceUtils
       .getOrLoadSortedService(ExceptionConverter.class).stream()
@@ -108,6 +112,10 @@ public final class Exceptions {
   public static Response exceptionToResponse(@Nullable Invocation invocation, 
Throwable exception,
       StatusType genericStatus) {
     InvocationException invocationException = Exceptions.convert(invocation, 
exception, genericStatus);
+    if (invocation != null) {
+      LOGGER.error("failed to process {} invocation, operation={}.",
+          invocation.getInvocationType(), 
invocation.getMicroserviceQualifiedName(), invocationException);
+    }
     return Response.status(invocationException.getStatus())
         .entity(invocationException.getErrorData());
   }
diff --git 
a/core/src/main/java/org/apache/servicecomb/core/exception/converter/DefaultExceptionConverter.java
 
b/core/src/main/java/org/apache/servicecomb/core/exception/converter/DefaultExceptionConverter.java
index ef1777f..b48bdb4 100644
--- 
a/core/src/main/java/org/apache/servicecomb/core/exception/converter/DefaultExceptionConverter.java
+++ 
b/core/src/main/java/org/apache/servicecomb/core/exception/converter/DefaultExceptionConverter.java
@@ -54,13 +54,13 @@ public class DefaultExceptionConverter implements 
ExceptionConverter<Throwable>
 
   @Override
   public InvocationException convert(@Nullable Invocation invocation, 
Throwable throwable, StatusType genericStatus) {
-    LOGGER.error("convert unknown exception to InvocationException.", 
throwable);
-
     String msg = throwable.getMessage();
     if (msg == null) {
       msg = "Unexpected exception when processing.";
     }
 
+    LOGGER.error("convert unknown exception({}) to InvocationException, 
message={}.",
+        throwable.getClass().getName(), msg);
     return new InvocationException(genericStatus, 
ExceptionConverter.getGenericCode(genericStatus),
         msg, throwable);
   }
diff --git 
a/core/src/main/java/org/apache/servicecomb/core/filter/impl/ProducerOperationFilter.java
 
b/core/src/main/java/org/apache/servicecomb/core/filter/impl/ProducerOperationFilter.java
index 6b58df4..1695ba8 100644
--- 
a/core/src/main/java/org/apache/servicecomb/core/filter/impl/ProducerOperationFilter.java
+++ 
b/core/src/main/java/org/apache/servicecomb/core/filter/impl/ProducerOperationFilter.java
@@ -30,7 +30,6 @@ import 
org.apache.servicecomb.foundation.common.utils.AsyncUtils;
 import org.apache.servicecomb.swagger.engine.SwaggerProducerOperation;
 import org.apache.servicecomb.swagger.invocation.Response;
 import org.apache.servicecomb.swagger.invocation.context.ContextUtils;
-import org.apache.servicecomb.swagger.invocation.exception.InvocationException;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
@@ -48,7 +47,7 @@ public class ProducerOperationFilter implements Filter {
     Object[] args = invocation.toProducerArguments();
     return invoke(invocation, instance, method, args)
         .thenApply(result -> convertResultToResponse(invocation, 
producerOperation, result))
-        .whenComplete((response, throwable) -> whenComplete(invocation, 
throwable));
+        .whenComplete((response, throwable) -> processMetrics(invocation));
   }
 
   @SuppressWarnings("unchecked")
@@ -74,23 +73,8 @@ public class ProducerOperationFilter implements Filter {
     return 
producerOperation.getResponseMapper().mapResponse(invocation.getStatus(), 
result);
   }
 
-  protected void whenComplete(Invocation invocation, Throwable throwable) {
-    if (throwable != null) {
-      Throwable unwrapped = Exceptions.unwrap(throwable);
-      if (shouldPrintErrorLog(unwrapped)) {
-        LOGGER.error("unexpected error, invocation={},", 
invocation.getInvocationQualifiedName(), unwrapped);
-      }
-    }
-
+  protected void processMetrics(Invocation invocation) {
     invocation.onBusinessMethodFinish();
     invocation.onBusinessFinish();
   }
-
-  protected boolean shouldPrintErrorLog(Throwable throwable) {
-    if (throwable == null) {
-      return false;
-    }
-
-    return !(throwable instanceof InvocationException);
-  }
 }
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 5648f5f..b804f36 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
@@ -233,7 +233,14 @@ public final class InvokerUtils {
   }
 
   private static Response convertException(Invocation invocation, Throwable 
throwable) {
-    throw Exceptions.convert(invocation, throwable);
+    InvocationException invocationException = Exceptions.convert(invocation, 
throwable);
+
+    LOGGER.error("failed to invoke {}, endpoint={}.",
+        invocation.getMicroserviceQualifiedName(),
+        invocation.getEndpoint(),
+        invocationException);
+
+    throw invocationException;
   }
 
   private static void processMetrics(Invocation invocation, Response response) 
{
diff --git 
a/providers/provider-pojo/src/main/java/org/apache/servicecomb/provider/pojo/FilterInvocationCaller.java
 
b/providers/provider-pojo/src/main/java/org/apache/servicecomb/provider/pojo/FilterInvocationCaller.java
index ece166e..ecb4358 100644
--- 
a/providers/provider-pojo/src/main/java/org/apache/servicecomb/provider/pojo/FilterInvocationCaller.java
+++ 
b/providers/provider-pojo/src/main/java/org/apache/servicecomb/provider/pojo/FilterInvocationCaller.java
@@ -23,16 +23,22 @@ import java.util.concurrent.CompletableFuture;
 
 import javax.annotation.Nonnull;
 
+import org.apache.servicecomb.core.exception.Exceptions;
 import org.apache.servicecomb.core.provider.consumer.InvokerUtils;
 import org.apache.servicecomb.foundation.common.utils.AsyncUtils;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
 
 public class FilterInvocationCaller implements InvocationCaller {
+  private static final Logger LOGGER = 
LoggerFactory.getLogger(FilterInvocationCaller.class);
+
   // if not a sync method, should never throw exception directly
   @Override
   public Object call(Method method, PojoConsumerMetaRefresher metaRefresher, 
PojoInvocationCreator invocationCreator,
       Object[] args) {
     CompletableFuture<Object> future = AsyncUtils
         .tryCatchSupplier(() -> invocationCreator.create(method, 
metaRefresher, args))
+        .exceptionally(throwable -> logCreateInvocationException(method, 
throwable))
         .thenCompose(this::doCall);
 
     return isAsyncMethod(method) ? future : AsyncUtils.toSync(future);
@@ -42,4 +48,9 @@ public class FilterInvocationCaller implements 
InvocationCaller {
     return InvokerUtils.invoke(invocation)
         .thenApply(invocation::convertResponse);
   }
+
+  protected PojoInvocation logCreateInvocationException(Method method, 
Throwable throwable) {
+    LOGGER.error("failed to create invocation, method=", method);
+    throw Exceptions.consumer("SCB_PROVIDER_POJO.400000001", "failed to create 
invocation.", throwable);
+  }
 }

Reply via email to