EvanLjp commented on pull request #5426:
URL: https://github.com/apache/skywalking/pull/5426#issuecomment-688886435


   > If there is an exception, then the status could be error=true, what is the 
issue of that?
   
   For our thinking, errorOccurred and log should come in pairs. For example
   ``
   ContextManager.activeSpan().errorOccurred().log(t);
   ``
   But the author implement is like following:
   ```
   @Override
       public void cancel(@Nullable String message, @Nullable Throwable cause) {
           final AbstractSpan span = 
ContextManager.createLocalSpan(operationPrefix + 
REQUEST_ON_CANCEL_OPERATION_NAME);
           span.setComponent(ComponentsDefine.GRPC);
           span.setLayer(SpanLayer.RPC_FRAMEWORK);
           ContextManager.continued(snapshot);
   
           if (cause != null) {
               span.log(cause);
           }
   
           try {
               super.cancel(message, cause);
           } catch (Throwable t) {
               ContextManager.activeSpan().errorOccurred().log(t);
               throw t;
           } finally {
               ContextManager.stopSpan();
           }
       }
   ```
   The local span only triggers the log function rather than trigger the 2 
functions together. So the local span should always be a normal span. It seems 
that the author did this on purpose.
   
   


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
[email protected]


Reply via email to