kezhenxu94 commented on a change in pull request #4327: Tag annotation supports 
returned expression
URL: https://github.com/apache/skywalking/pull/4327#discussion_r377997300
 
 

 ##########
 File path: 
apm-sniffer/apm-toolkit-activation/apm-toolkit-trace-activation/src/main/java/org/apache/skywalking/apm/toolkit/activation/trace/TraceAnnotationMethodInterceptor.java
 ##########
 @@ -56,24 +56,41 @@ public void beforeMethod(EnhancedInstance objInst, Method 
method, Object[] allAr
         final Tags tags = method.getAnnotation(Tags.class);
         if (tags != null && tags.value().length > 0) {
             for (final Tag tag : tags.value()) {
-                tagSpan(localSpan, tag, context);
+                if (!TagUtil.isReturnTag(tag.value())) {
+                    TagUtil.tagParamsSpan(localSpan, context, tag);
+                }
             }
         }
-
         final Tag tag = method.getAnnotation(Tag.class);
-        if (tag != null) {
-            tagSpan(localSpan, tag, context);
+        if (tag != null && !TagUtil.isReturnTag(tag.value())) {
+            TagUtil.tagParamsSpan(localSpan, context, tag);
         }
     }
 
-    private void tagSpan(final AbstractSpan span, final Tag tag, final 
Map<String, Object> context) {
-        new StringTag(tag.key()).set(span, 
CustomizeExpression.parseExpression(tag.value(), context));
-    }
-
     @Override
     public Object afterMethod(EnhancedInstance objInst, Method method, 
Object[] allArguments, Class<?>[] argumentsTypes,
         Object ret) throws Throwable {
-        ContextManager.stopSpan();
+        try {
+            if (ret == null) {
+                return ret;
+            }
+            final AbstractSpan localSpan = ContextManager.activeSpan();
+            final Map<String, Object> context = 
CustomizeExpression.evaluationReturnContext(ret);
+            final Tags tags = method.getAnnotation(Tags.class);
+            if (tags != null && tags.value().length > 0) {
+                for (final Tag tag : tags.value()) {
+                    if (TagUtil.isReturnTag(tag.value())) {
+                        TagUtil.tagReturnSpanSpan(localSpan, context, tag);
+                    }
+                }
+            }
+            final Tag tag = method.getAnnotation(Tag.class);
+            if (tag != null && TagUtil.isReturnTag(tag.value())) {
+                TagUtil.tagReturnSpanSpan(localSpan, context, tag);
+            }
+        } finally {
+            ContextManager.stopSpan();
 
 Review comment:
   > Same here.
   
   There is no conditional creation of span in the `#beforeMethod`, so it's 
right here

----------------------------------------------------------------
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:
us...@infra.apache.org


With regards,
Apache Git Services

Reply via email to