You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@skywalking.apache.org by GitBox <gi...@apache.org> on 2019/12/30 14:20:41 UTC

[GitHub] [skywalking] kezhenxu94 opened a new pull request #4152: [Feature] Add tag annotation to allow tagging span with annotation

kezhenxu94 opened a new pull request #4152: [Feature] Add tag annotation to allow tagging span with annotation
URL: https://github.com/apache/skywalking/pull/4152
 
 
   Please answer these questions before submitting pull request
   
   - Why submit this pull request?
   - [ ] Bug fix
   - [x] New feature provided
   - [ ] Improve performance
   
   This PR provides another annotation `Tag` (similar to `Trace`) to allow users to apply a tag to the current active span, it's more convenient to tag a trace in companion with `@Trace`:
   
   ```java
   @Trace
   @Tag(key = "tag", value = "arg[0]")
   public void method(String param) {
   }
   ```
   
   or multiple tags
   
   
   ```java
   @Trace
   @Tags({
       @Tag(key = "tag1", value = "arg[0]"),
       @Tag(key = "tag2", value = "arg[1]"),
   })
   public void method(String param1, String param2) {
   }
   ```
   

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


With regards,
Apache Git Services

[GitHub] [skywalking] dmsolr commented on a change in pull request #4152: [Feature] Add tag annotation to allow tagging span with annotation

Posted by GitBox <gi...@apache.org>.
dmsolr commented on a change in pull request #4152: [Feature] Add tag annotation to allow tagging span with annotation
URL: https://github.com/apache/skywalking/pull/4152#discussion_r362137870
 
 

 ##########
 File path: apm-application-toolkit/apm-toolkit-trace/src/main/java/org/apache/skywalking/apm/toolkit/trace/Tag.java
 ##########
 @@ -0,0 +1,47 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ *
+ */
+
+package org.apache.skywalking.apm.toolkit.trace;
+
+import java.lang.annotation.ElementType;
+import java.lang.annotation.Retention;
+import java.lang.annotation.RetentionPolicy;
+import java.lang.annotation.Target;
+
+/**
+ * Tag the current active span with key {@link #key()} and value {@link #value()},
+ * if there is no active span, this annotation takes no effect.
+ *
+ * @author kezhenxu94
+ * @see Tags
+ */
+@Target(ElementType.METHOD)
+@Retention(RetentionPolicy.RUNTIME)
+public @interface Tag {
 
 Review comment:
   Miss `@Repeatable`?

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


With regards,
Apache Git Services

[GitHub] [skywalking] dmsolr commented on issue #4152: [Feature] Add tag annotation to allow tagging span with annotation

Posted by GitBox <gi...@apache.org>.
dmsolr commented on issue #4152: [Feature] Add tag annotation to allow tagging span with annotation
URL: https://github.com/apache/skywalking/pull/4152#issuecomment-569852064
 
 
   > Yes, you got what i thought. I think it's meaningful & If we do, make our developer write plugins simplfy. @kezhenxu94
   
   It supports LocalSpan only now! :)

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


With regards,
Apache Git Services

[GitHub] [skywalking] kezhenxu94 commented on issue #4152: [Feature] Add tag annotation to allow tagging span with annotation

Posted by GitBox <gi...@apache.org>.
kezhenxu94 commented on issue #4152: [Feature] Add tag annotation to allow tagging span with annotation
URL: https://github.com/apache/skywalking/pull/4152#issuecomment-569853785
 
 
   > > Yes, you got what i thought. I think it's meaningful & If we do, make our developer write plugins simplfy. @kezhenxu94
   > 
   > It supports LocalSpan only now! :)
   
   @dmsolr nope, it supports any kinds of span (it is not related to specific kind of span), except for those annotated by `RequestMapping` and similar annotations that are intercepted by other agent plugins

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


With regards,
Apache Git Services

[GitHub] [skywalking] kezhenxu94 commented on a change in pull request #4152: [Feature] Add tag annotation to allow tagging span with annotation

Posted by GitBox <gi...@apache.org>.
kezhenxu94 commented on a change in pull request #4152: [Feature] Add tag annotation to allow tagging span with annotation
URL: https://github.com/apache/skywalking/pull/4152#discussion_r362138509
 
 

 ##########
 File path: apm-application-toolkit/apm-toolkit-trace/src/main/java/org/apache/skywalking/apm/toolkit/trace/Tag.java
 ##########
 @@ -0,0 +1,47 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ *
+ */
+
+package org.apache.skywalking.apm.toolkit.trace;
+
+import java.lang.annotation.ElementType;
+import java.lang.annotation.Retention;
+import java.lang.annotation.RetentionPolicy;
+import java.lang.annotation.Target;
+
+/**
+ * Tag the current active span with key {@link #key()} and value {@link #value()},
+ * if there is no active span, this annotation takes no effect.
+ *
+ * @author kezhenxu94
+ * @see Tags
+ */
+@Target(ElementType.METHOD)
+@Retention(RetentionPolicy.RUNTIME)
+public @interface Tag {
 
 Review comment:
   > why it's still ref to the user even i add the quotes.
   
   Use backtick instead of quotes 

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


With regards,
Apache Git Services

[GitHub] [skywalking] wayilau edited a comment on issue #4152: [Feature] Add tag annotation to allow tagging span with annotation

Posted by GitBox <gi...@apache.org>.
wayilau edited a comment on issue #4152: [Feature] Add tag annotation to allow tagging span with annotation
URL: https://github.com/apache/skywalking/pull/4152#issuecomment-569848855
 
 
   Does this PR support we can using anntation write our agent plugins ? 

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


With regards,
Apache Git Services

[GitHub] [skywalking] codecov-io commented on issue #4152: [Feature] Add tag annotation to allow tagging span with annotation

Posted by GitBox <gi...@apache.org>.
codecov-io commented on issue #4152: [Feature] Add tag annotation to allow tagging span with annotation
URL: https://github.com/apache/skywalking/pull/4152#issuecomment-569856019
 
 
   # [Codecov](https://codecov.io/gh/apache/skywalking/pull/4152?src=pr&el=h1) Report
   > Merging [#4152](https://codecov.io/gh/apache/skywalking/pull/4152?src=pr&el=desc) into [master](https://codecov.io/gh/apache/skywalking/commit/1ee14fbbc8251a1faa5f3851d4f7d92f9ec1ab15?src=pr&el=desc) will **decrease** coverage by `0.09%`.
   > The diff coverage is `11.62%`.
   
   [![Impacted file tree graph](https://codecov.io/gh/apache/skywalking/pull/4152/graphs/tree.svg?width=650&token=qrILxY5yA8&height=150&src=pr)](https://codecov.io/gh/apache/skywalking/pull/4152?src=pr&el=tree)
   
   ```diff
   @@            Coverage Diff            @@
   ##           master    #4152     +/-   ##
   =========================================
   - Coverage   27.13%   27.03%   -0.1%     
   =========================================
     Files        1145     1147      +2     
     Lines       25105    25141     +36     
     Branches     3631     3641     +10     
   =========================================
   - Hits         6812     6797     -15     
   - Misses      17690    17739     +49     
   - Partials      603      605      +2
   ```
   
   
   | [Impacted Files](https://codecov.io/gh/apache/skywalking/pull/4152?src=pr&el=tree) | Coverage Δ | |
   |---|---|---|
   | [...it/activation/trace/TraceAnnotationActivation.java](https://codecov.io/gh/apache/skywalking/pull/4152/diff?src=pr&el=tree#diff-YXBtLXNuaWZmZXIvYXBtLXRvb2xraXQtYWN0aXZhdGlvbi9hcG0tdG9vbGtpdC10cmFjZS1hY3RpdmF0aW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9za3l3YWxraW5nL2FwbS90b29sa2l0L2FjdGl2YXRpb24vdHJhY2UvVHJhY2VBbm5vdGF0aW9uQWN0aXZhdGlvbi5qYXZh) | `0% <ø> (ø)` | :arrow_up: |
   | [.../customize/interceptor/BaseInterceptorMethods.java](https://codecov.io/gh/apache/skywalking/pull/4152/diff?src=pr&el=tree#diff-YXBtLXNuaWZmZXIvb3B0aW9uYWwtcGx1Z2lucy9jdXN0b21pemUtZW5oYW5jZS1wbHVnaW4vc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL3NreXdhbGtpbmcvYXBtL3BsdWdpbi9jdXN0b21pemUvaW50ZXJjZXB0b3IvQmFzZUludGVyY2VwdG9yTWV0aG9kcy5qYXZh) | `0% <ø> (ø)` | :arrow_up: |
   | [...agent/core/plugin/match/MethodAnnotationMatch.java](https://codecov.io/gh/apache/skywalking/pull/4152/diff?src=pr&el=tree#diff-YXBtLXNuaWZmZXIvYXBtLWFnZW50LWNvcmUvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL3NreXdhbGtpbmcvYXBtL2FnZW50L2NvcmUvcGx1Z2luL21hdGNoL01ldGhvZEFubm90YXRpb25NYXRjaC5qYXZh) | `0% <0%> (ø)` | :arrow_up: |
   | [...lking/apm/agent/core/util/CustomizeExpression.java](https://codecov.io/gh/apache/skywalking/pull/4152/diff?src=pr&el=tree#diff-YXBtLXNuaWZmZXIvYXBtLWFnZW50LWNvcmUvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL3NreXdhbGtpbmcvYXBtL2FnZW50L2NvcmUvdXRpbC9DdXN0b21pemVFeHByZXNzaW9uLmphdmE=) | `0% <0%> (ø)` | |
   | [...re/plugin/match/logical/LogicalMatchOperation.java](https://codecov.io/gh/apache/skywalking/pull/4152/diff?src=pr&el=tree#diff-YXBtLXNuaWZmZXIvYXBtLWFnZW50LWNvcmUvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL3NreXdhbGtpbmcvYXBtL2FnZW50L2NvcmUvcGx1Z2luL21hdGNoL2xvZ2ljYWwvTG9naWNhbE1hdGNoT3BlcmF0aW9uLmphdmE=) | `0% <0%> (ø)` | :arrow_up: |
   | [...tivation/trace/TagAnnotationMethodInterceptor.java](https://codecov.io/gh/apache/skywalking/pull/4152/diff?src=pr&el=tree#diff-YXBtLXNuaWZmZXIvYXBtLXRvb2xraXQtYWN0aXZhdGlvbi9hcG0tdG9vbGtpdC10cmFjZS1hY3RpdmF0aW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9za3l3YWxraW5nL2FwbS90b29sa2l0L2FjdGl2YXRpb24vdHJhY2UvVGFnQW5ub3RhdGlvbk1ldGhvZEludGVyY2VwdG9yLmphdmE=) | `0% <0%> (ø)` | |
   | [...lkit/activation/trace/TagAnnotationActivation.java](https://codecov.io/gh/apache/skywalking/pull/4152/diff?src=pr&el=tree#diff-YXBtLXNuaWZmZXIvYXBtLXRvb2xraXQtYWN0aXZhdGlvbi9hcG0tdG9vbGtpdC10cmFjZS1hY3RpdmF0aW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9za3l3YWxraW5nL2FwbS90b29sa2l0L2FjdGl2YXRpb24vdHJhY2UvVGFnQW5ub3RhdGlvbkFjdGl2YXRpb24uamF2YQ==) | `0% <0%> (ø)` | |
   | [...vation/trace/TraceAnnotationMethodInterceptor.java](https://codecov.io/gh/apache/skywalking/pull/4152/diff?src=pr&el=tree#diff-YXBtLXNuaWZmZXIvYXBtLXRvb2xraXQtYWN0aXZhdGlvbi9hcG0tdG9vbGtpdC10cmFjZS1hY3RpdmF0aW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9za3l3YWxraW5nL2FwbS90b29sa2l0L2FjdGl2YXRpb24vdHJhY2UvVHJhY2VBbm5vdGF0aW9uTWV0aG9kSW50ZXJjZXB0b3IuamF2YQ==) | `50% <45.45%> (-20%)` | :arrow_down: |
   | ... and [2 more](https://codecov.io/gh/apache/skywalking/pull/4152/diff?src=pr&el=tree-more) | |
   
   ------
   
   [Continue to review full report at Codecov](https://codecov.io/gh/apache/skywalking/pull/4152?src=pr&el=continue).
   > **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
   > `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
   > Powered by [Codecov](https://codecov.io/gh/apache/skywalking/pull/4152?src=pr&el=footer). Last update [1ee14fb...e0e3cc2](https://codecov.io/gh/apache/skywalking/pull/4152?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).
   

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


With regards,
Apache Git Services

[GitHub] [skywalking] kezhenxu94 commented on a change in pull request #4152: [Feature] Add tag annotation to allow tagging span with annotation

Posted by GitBox <gi...@apache.org>.
kezhenxu94 commented on a change in pull request #4152: [Feature] Add tag annotation to allow tagging span with annotation
URL: https://github.com/apache/skywalking/pull/4152#discussion_r362138635
 
 

 ##########
 File path: apm-application-toolkit/apm-toolkit-trace/src/main/java/org/apache/skywalking/apm/toolkit/trace/Tag.java
 ##########
 @@ -0,0 +1,47 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ *
+ */
+
+package org.apache.skywalking.apm.toolkit.trace;
+
+import java.lang.annotation.ElementType;
+import java.lang.annotation.Retention;
+import java.lang.annotation.RetentionPolicy;
+import java.lang.annotation.Target;
+
+/**
+ * Tag the current active span with key {@link #key()} and value {@link #value()},
+ * if there is no active span, this annotation takes no effect.
+ *
+ * @author kezhenxu94
+ * @see Tags
+ */
+@Target(ElementType.METHOD)
+@Retention(RetentionPolicy.RUNTIME)
+public @interface Tag {
 
 Review comment:
   > Miss "@repeatable"?
   
   May be optional ? Tests passed

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


With regards,
Apache Git Services

[GitHub] [skywalking] dmsolr commented on issue #4152: [Feature] Add tag annotation to allow tagging span with annotation

Posted by GitBox <gi...@apache.org>.
dmsolr commented on issue #4152: [Feature] Add tag annotation to allow tagging span with annotation
URL: https://github.com/apache/skywalking/pull/4152#issuecomment-569855164
 
 
   I got it. We work with Spring-Annotations plugins.

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


With regards,
Apache Git Services

[GitHub] [skywalking] wayilau commented on issue #4152: [Feature] Add tag annotation to allow tagging span with annotation

Posted by GitBox <gi...@apache.org>.
wayilau commented on issue #4152: [Feature] Add tag annotation to allow tagging span with annotation
URL: https://github.com/apache/skywalking/pull/4152#issuecomment-569851063
 
 
   > > Does this PR support we can using anntation write our agent plugins ?
   > 
   > Do you mean using `@Tag` in the agent plugin methods? No you can't, classes of agent plugins are ignored:
   > 
   > https://github.com/apache/skywalking/blob/1ee14fbbc8251a1faa5f3851d4f7d92f9ec1ab15/apm-sniffer/apm-agent/src/main/java/org/apache/skywalking/apm/agent/SkyWalkingAgent.java#L90-L98
   > 
   > so although you can annotate the plugin method with `@Tag`, it takes no effect
   
   Yes, you got what i thought.  I think it's meaningful & If we do, make our developer write plugins  simplfy. @kezhenxu94 

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


With regards,
Apache Git Services

[GitHub] [skywalking] wu-sheng commented on issue #4152: [Feature] Add tag annotation to allow tagging span with annotation

Posted by GitBox <gi...@apache.org>.
wu-sheng commented on issue #4152: [Feature] Add tag annotation to allow tagging span with annotation
URL: https://github.com/apache/skywalking/pull/4152#issuecomment-569858978
 
 
   Got it now. Maybe browser cache, can't see it when I refreshed the page last time.

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


With regards,
Apache Git Services

[GitHub] [skywalking] wayilau commented on issue #4152: [Feature] Add tag annotation to allow tagging span with annotation

Posted by GitBox <gi...@apache.org>.
wayilau commented on issue #4152: [Feature] Add tag annotation to allow tagging span with annotation
URL: https://github.com/apache/skywalking/pull/4152#issuecomment-569850104
 
 
   > > Does this PR support we can using anntation write our agent plugins ?
   > 
   > In the test, it can't, because they are not in the release :) So they are copied into the test project too.
   
   Great feature!

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


With regards,
Apache Git Services

[GitHub] [skywalking] kezhenxu94 merged pull request #4152: [Feature] Add tag annotation to allow tagging span with annotation

Posted by GitBox <gi...@apache.org>.
kezhenxu94 merged pull request #4152: [Feature] Add tag annotation to allow tagging span with annotation
URL: https://github.com/apache/skywalking/pull/4152
 
 
   

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


With regards,
Apache Git Services

[GitHub] [skywalking] wayilau commented on issue #4152: [Feature] Add tag annotation to allow tagging span with annotation

Posted by GitBox <gi...@apache.org>.
wayilau commented on issue #4152: [Feature] Add tag annotation to allow tagging span with annotation
URL: https://github.com/apache/skywalking/pull/4152#issuecomment-569848855
 
 
   Does this PR support we can using anntation write our agent plugins ?

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


With regards,
Apache Git Services

[GitHub] [skywalking] wu-sheng commented on issue #4152: [Feature] Add tag annotation to allow tagging span with annotation

Posted by GitBox <gi...@apache.org>.
wu-sheng commented on issue #4152: [Feature] Add tag annotation to allow tagging span with annotation
URL: https://github.com/apache/skywalking/pull/4152#issuecomment-569849058
 
 
   > Does this PR support we can using anntation write our agent plugins ?
   
   In the test, it can't, because they are not in the release :) So they are copied into the test project too.

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


With regards,
Apache Git Services

[GitHub] [skywalking] wu-sheng commented on issue #4152: [Feature] Add tag annotation to allow tagging span with annotation

Posted by GitBox <gi...@apache.org>.
wu-sheng commented on issue #4152: [Feature] Add tag annotation to allow tagging span with annotation
URL: https://github.com/apache/skywalking/pull/4152#issuecomment-569714599
 
 
   CI keeps failure. Please recheck.

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


With regards,
Apache Git Services

[GitHub] [skywalking] wayilau commented on a change in pull request #4152: [Feature] Add tag annotation to allow tagging span with annotation

Posted by GitBox <gi...@apache.org>.
wayilau commented on a change in pull request #4152: [Feature] Add tag annotation to allow tagging span with annotation
URL: https://github.com/apache/skywalking/pull/4152#discussion_r362138233
 
 

 ##########
 File path: apm-application-toolkit/apm-toolkit-trace/src/main/java/org/apache/skywalking/apm/toolkit/trace/Tag.java
 ##########
 @@ -0,0 +1,47 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ *
+ */
+
+package org.apache.skywalking.apm.toolkit.trace;
+
+import java.lang.annotation.ElementType;
+import java.lang.annotation.Retention;
+import java.lang.annotation.RetentionPolicy;
+import java.lang.annotation.Target;
+
+/**
+ * Tag the current active span with key {@link #key()} and value {@link #value()},
+ * if there is no active span, this annotation takes no effect.
+ *
+ * @author kezhenxu94
+ * @see Tags
+ */
+@Target(ElementType.METHOD)
+@Retention(RetentionPolicy.RUNTIME)
+public @interface Tag {
 
 Review comment:
   why it's still ref to the user even i add the quotes.

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


With regards,
Apache Git Services

[GitHub] [skywalking] kezhenxu94 commented on issue #4152: [Feature] Add tag annotation to allow tagging span with annotation

Posted by GitBox <gi...@apache.org>.
kezhenxu94 commented on issue #4152: [Feature] Add tag annotation to allow tagging span with annotation
URL: https://github.com/apache/skywalking/pull/4152#issuecomment-569850311
 
 
   > Does this PR support we can using anntation write our agent plugins ?
   
   Do you mean using `@Tag` in the agent plugin methods? No you can't, classes of agent plugins are ignored:
   
   https://github.com/apache/skywalking/blob/1ee14fbbc8251a1faa5f3851d4f7d92f9ec1ab15/apm-sniffer/apm-agent/src/main/java/org/apache/skywalking/apm/agent/SkyWalkingAgent.java#L90-L98
   
   so although you can annotate the plugin method with `@Tag`, it takes no effect

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


With regards,
Apache Git Services

[GitHub] [skywalking] kezhenxu94 commented on issue #4152: [Feature] Add tag annotation to allow tagging span with annotation

Posted by GitBox <gi...@apache.org>.
kezhenxu94 commented on issue #4152: [Feature] Add tag annotation to allow tagging span with annotation
URL: https://github.com/apache/skywalking/pull/4152#issuecomment-569858283
 
 
   > Doc of https://github.com/apache/skywalking/blob/master/docs/en/setup/service-agent/java-agent/Application-toolkit-trace.md should be updated.
   
   @wu-sheng did you find this? https://github.com/apache/skywalking/pull/4152/files#diff-26046b3a31cc7df0b065c9cdc6b03c7d

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


With regards,
Apache Git Services

[GitHub] [skywalking] dmsolr commented on a change in pull request #4152: [Feature] Add tag annotation to allow tagging span with annotation

Posted by GitBox <gi...@apache.org>.
dmsolr commented on a change in pull request #4152: [Feature] Add tag annotation to allow tagging span with annotation
URL: https://github.com/apache/skywalking/pull/4152#discussion_r362137870
 
 

 ##########
 File path: apm-application-toolkit/apm-toolkit-trace/src/main/java/org/apache/skywalking/apm/toolkit/trace/Tag.java
 ##########
 @@ -0,0 +1,47 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ *
+ */
+
+package org.apache.skywalking.apm.toolkit.trace;
+
+import java.lang.annotation.ElementType;
+import java.lang.annotation.Retention;
+import java.lang.annotation.RetentionPolicy;
+import java.lang.annotation.Target;
+
+/**
+ * Tag the current active span with key {@link #key()} and value {@link #value()},
+ * if there is no active span, this annotation takes no effect.
+ *
+ * @author kezhenxu94
+ * @see Tags
+ */
+@Target(ElementType.METHOD)
+@Retention(RetentionPolicy.RUNTIME)
+public @interface Tag {
 
 Review comment:
   Miss @Repeatable?

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


With regards,
Apache Git Services

[GitHub] [skywalking] dmsolr commented on a change in pull request #4152: [Feature] Add tag annotation to allow tagging span with annotation

Posted by GitBox <gi...@apache.org>.
dmsolr commented on a change in pull request #4152: [Feature] Add tag annotation to allow tagging span with annotation
URL: https://github.com/apache/skywalking/pull/4152#discussion_r362137870
 
 

 ##########
 File path: apm-application-toolkit/apm-toolkit-trace/src/main/java/org/apache/skywalking/apm/toolkit/trace/Tag.java
 ##########
 @@ -0,0 +1,47 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ *
+ */
+
+package org.apache.skywalking.apm.toolkit.trace;
+
+import java.lang.annotation.ElementType;
+import java.lang.annotation.Retention;
+import java.lang.annotation.RetentionPolicy;
+import java.lang.annotation.Target;
+
+/**
+ * Tag the current active span with key {@link #key()} and value {@link #value()},
+ * if there is no active span, this annotation takes no effect.
+ *
+ * @author kezhenxu94
+ * @see Tags
+ */
+@Target(ElementType.METHOD)
+@Retention(RetentionPolicy.RUNTIME)
+public @interface Tag {
 
 Review comment:
   Miss "@Repeatable"?

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


With regards,
Apache Git Services

[GitHub] [skywalking] wu-sheng commented on issue #4152: [Feature] Add tag annotation to allow tagging span with annotation

Posted by GitBox <gi...@apache.org>.
wu-sheng commented on issue #4152: [Feature] Add tag annotation to allow tagging span with annotation
URL: https://github.com/apache/skywalking/pull/4152#issuecomment-569853254
 
 
   Making no-local span is more about exact and inject ContextCarrier :) Even we add that, I think it will be closer to OpenTracing APIs, even it is not widely used.

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


With regards,
Apache Git Services