You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@skywalking.apache.org by "Chenfx-git (via GitHub)" <gi...@apache.org> on 2023/02/20 13:03:03 UTC
[GitHub] [skywalking-java] Chenfx-git opened a new pull request, #460: Fix servicecomb plugin trace break
Chenfx-git opened a new pull request, #460:
URL: https://github.com/apache/skywalking-java/pull/460
fix while consumer don't use CseRestTemplate in servicecomb(like httpclient、SpringRestTemplate、OKhttp，etc), trace still complete
--
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.
To unsubscribe, e-mail: notifications-unsubscribe@skywalking.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [skywalking-java] wu-sheng commented on a diff in pull request #460: Fix servicecomb plugin trace break
Posted by "wu-sheng (via GitHub)" <gi...@apache.org>.
wu-sheng commented on code in PR #460:
URL: https://github.com/apache/skywalking-java/pull/460#discussion_r1112004667
##########
apm-sniffer/apm-sdk-plugin/servicecomb-plugin/servicecomb-java-chassis-2.x-plugin/src/main/java/org/apache/skywalking/apm/plugin/servicecomb/v2/ProducerOperationHandlerInterceptor.java:
##########
@@ -52,6 +52,13 @@ public void beforeMethod(EnhancedInstance objInst, Method method, Object[] allAr
SpanLayer.asRPCFramework(span);
}
+ private String getHeadValue(Invocation invocation, String key) {
+ if (invocation.getContext().containsKey("sw8")) {
Review Comment:
What do you mean? Are you starting a new discussion?
--
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.
To unsubscribe, e-mail: notifications-unsubscribe@skywalking.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [skywalking-java] Chenfx-git commented on a diff in pull request #460: Fix servicecomb plugin trace break
Posted by "Chenfx-git (via GitHub)" <gi...@apache.org>.
Chenfx-git commented on code in PR #460:
URL: https://github.com/apache/skywalking-java/pull/460#discussion_r1112086024
##########
apm-sniffer/apm-sdk-plugin/servicecomb-plugin/servicecomb-java-chassis-2.x-plugin/src/main/java/org/apache/skywalking/apm/plugin/servicecomb/v2/ProducerOperationHandlerInterceptor.java:
##########
@@ -52,6 +52,13 @@ public void beforeMethod(EnhancedInstance objInst, Method method, Object[] allAr
SpanLayer.asRPCFramework(span);
}
+ private String getHeadValue(Invocation invocation, String key) {
+ if (invocation.getContext().containsKey("sw8")) {
Review Comment:
ok, i have given new push
--
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.
To unsubscribe, e-mail: notifications-unsubscribe@skywalking.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [skywalking-java] wu-sheng commented on pull request #460: Fix servicecomb plugin trace break
Posted by "wu-sheng (via GitHub)" <gi...@apache.org>.
wu-sheng commented on PR #460:
URL: https://github.com/apache/skywalking-java/pull/460#issuecomment-1436999680
@WillemJiang Could you ping a ServiceComb committer to confirm this API use change?
@Chenfx-git You need to update the changes.md in the root.
--
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.
To unsubscribe, e-mail: notifications-unsubscribe@skywalking.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [skywalking-java] wu-sheng commented on a diff in pull request #460: Fix servicecomb plugin trace break
Posted by "wu-sheng (via GitHub)" <gi...@apache.org>.
wu-sheng commented on code in PR #460:
URL: https://github.com/apache/skywalking-java/pull/460#discussion_r1112090082
##########
apm-sniffer/apm-sdk-plugin/servicecomb-plugin/servicecomb-java-chassis-2.x-plugin/src/main/java/org/apache/skywalking/apm/plugin/servicecomb/v2/ProducerOperationHandlerInterceptor.java:
##########
@@ -52,6 +52,13 @@ public void beforeMethod(EnhancedInstance objInst, Method method, Object[] allAr
SpanLayer.asRPCFramework(span);
}
+ private String getHeadValue(Invocation invocation, String key) {
+ if (invocation.getContext().containsKey("sw8")) {
Review Comment:
I am still no following what is the issue. Please provide a detailed explanation. Right now, the codes don't make sense.
--
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.
To unsubscribe, e-mail: notifications-unsubscribe@skywalking.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [skywalking-java] Chenfx-git commented on a diff in pull request #460: Fix servicecomb plugin trace break
Posted by "Chenfx-git (via GitHub)" <gi...@apache.org>.
Chenfx-git commented on code in PR #460:
URL: https://github.com/apache/skywalking-java/pull/460#discussion_r1115207240
##########
apm-sniffer/apm-sdk-plugin/servicecomb-plugin/servicecomb-java-chassis-2.x-plugin/src/main/java/org/apache/skywalking/apm/plugin/servicecomb/v2/ProducerOperationHandlerInterceptor.java:
##########
@@ -42,7 +42,12 @@ public void beforeMethod(EnhancedInstance objInst, Method method, Object[] allAr
CarrierItem next = contextCarrier.items();
while (next.hasNext()) {
next = next.next();
- next.setHeadValue(invocation.getContext().get(next.getHeadKey()));
+ String headKey = next.getHeadKey();
+ if (invocation.getContext().containsKey(headKey)) {
+ next.setHeadValue(invocation.getContext().get(headKey));
+ } else {
+ next.setHeadValue(invocation.getRequestEx().getHeader(headKey));
Review Comment:
Add not cse invoke scenario. I have test it was ok.
--
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.
To unsubscribe, e-mail: notifications-unsubscribe@skywalking.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [skywalking-java] Chenfx-git commented on a diff in pull request #460: Fix servicecomb plugin trace break
Posted by "Chenfx-git (via GitHub)" <gi...@apache.org>.
Chenfx-git commented on code in PR #460:
URL: https://github.com/apache/skywalking-java/pull/460#discussion_r1113224842
##########
apm-sniffer/apm-sdk-plugin/servicecomb-plugin/servicecomb-java-chassis-2.x-plugin/src/main/java/org/apache/skywalking/apm/plugin/servicecomb/v2/ProducerOperationHandlerInterceptor.java:
##########
@@ -40,9 +39,14 @@ public void beforeMethod(EnhancedInstance objInst, Method method, Object[] allAr
Invocation invocation = (Invocation) allArguments[0];
ContextCarrier contextCarrier = new ContextCarrier();
CarrierItem next = contextCarrier.items();
+ boolean inContext = isInContext(invocation);
while (next.hasNext()) {
next = next.next();
- next.setHeadValue(invocation.getContext().get(next.getHeadKey()));
+ if (inContext) {
+ next.setHeadValue(invocation.getContext().get(next.getHeadKey()));
+ }else {
+ invocation.getRequestEx().getHeader(next.getHeadKey());
+ }
Review Comment:
Client side was determined by the developer. if use cseRestemplate invocation context and http header contains trace message, but in other methods are only in http header.
--
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.
To unsubscribe, e-mail: notifications-unsubscribe@skywalking.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [skywalking-java] wu-sheng commented on a diff in pull request #460: Fix servicecomb plugin trace break
Posted by "wu-sheng (via GitHub)" <gi...@apache.org>.
wu-sheng commented on code in PR #460:
URL: https://github.com/apache/skywalking-java/pull/460#discussion_r1111953323
##########
apm-sniffer/apm-sdk-plugin/servicecomb-plugin/servicecomb-java-chassis-2.x-plugin/src/main/java/org/apache/skywalking/apm/plugin/servicecomb/v2/ProducerOperationHandlerInterceptor.java:
##########
@@ -52,6 +52,13 @@ public void beforeMethod(EnhancedInstance objInst, Method method, Object[] allAr
SpanLayer.asRPCFramework(span);
}
+ private String getHeadValue(Invocation invocation, String key) {
+ if (invocation.getContext().containsKey("sw8")) {
Review Comment:
```suggestion
if (invocation.getContext().containsKey(key)) {
```
Hard code `sw8` is wrong. We have more headers than this. Notice, your method is in the `while`.
--
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.
To unsubscribe, e-mail: notifications-unsubscribe@skywalking.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [skywalking-java] wu-sheng commented on a diff in pull request #460: Fix servicecomb plugin trace break
Posted by "wu-sheng (via GitHub)" <gi...@apache.org>.
wu-sheng commented on code in PR #460:
URL: https://github.com/apache/skywalking-java/pull/460#discussion_r1114266071
##########
apm-sniffer/apm-sdk-plugin/servicecomb-plugin/servicecomb-java-chassis-2.x-plugin/src/main/java/org/apache/skywalking/apm/plugin/servicecomb/v2/ProducerOperationHandlerInterceptor.java:
##########
@@ -40,9 +39,14 @@ public void beforeMethod(EnhancedInstance objInst, Method method, Object[] allAr
Invocation invocation = (Invocation) allArguments[0];
ContextCarrier contextCarrier = new ContextCarrier();
CarrierItem next = contextCarrier.items();
+ boolean inContext = isInContext(invocation);
while (next.hasNext()) {
next = next.next();
- next.setHeadValue(invocation.getContext().get(next.getHeadKey()));
+ if (inContext) {
+ next.setHeadValue(invocation.getContext().get(next.getHeadKey()));
+ }else {
+ invocation.getRequestEx().getHeader(next.getHeadKey());
+ }
Review Comment:
Make sense. I just want to wait for one or two days, to see whether @WillemJiang could have some ServiceComb ppl to confirm this strange behaviour.
@Chenfx-git If you know anyone from their team, please ask them to confirm here.
The key to confirm is, `some request headers are in invocation.getContext, others are in getRequestEx`.
--
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.
To unsubscribe, e-mail: notifications-unsubscribe@skywalking.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [skywalking-java] WillemJiang commented on a diff in pull request #460: Fix servicecomb plugin trace break
Posted by "WillemJiang (via GitHub)" <gi...@apache.org>.
WillemJiang commented on code in PR #460:
URL: https://github.com/apache/skywalking-java/pull/460#discussion_r1115127337
##########
apm-sniffer/apm-sdk-plugin/servicecomb-plugin/servicecomb-java-chassis-2.x-plugin/src/main/java/org/apache/skywalking/apm/plugin/servicecomb/v2/ProducerOperationHandlerInterceptor.java:
##########
@@ -42,7 +42,12 @@ public void beforeMethod(EnhancedInstance objInst, Method method, Object[] allAr
CarrierItem next = contextCarrier.items();
while (next.hasNext()) {
next = next.next();
- next.setHeadValue(invocation.getContext().get(next.getHeadKey()));
+ String headKey = next.getHeadKey();
+ if (invocation.getContext().containsKey(headKey)) {
+ next.setHeadValue(invocation.getContext().get(headKey));
+ } else {
+ next.setHeadValue(invocation.getRequestEx().getHeader(headKey));
Review Comment:
Please add a comment for this fallback solution.
##########
CHANGES.md:
##########
@@ -16,6 +16,7 @@ Release Notes.
* Refactor kotlin coroutine plugin with CoroutineContext.
* Fix OracleURLParser ignoring actual port when :SID is absent.
* Change gRPC instrumentation point to fix plugin not working for server side.
+* Fix servicecomb plugin trace break.
Review Comment:
Please add the GitHub issue link in this CHANGES
--
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.
To unsubscribe, e-mail: notifications-unsubscribe@skywalking.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [skywalking-java] wu-sheng commented on a diff in pull request #460: Fix servicecomb plugin trace break
Posted by "wu-sheng (via GitHub)" <gi...@apache.org>.
wu-sheng commented on code in PR #460:
URL: https://github.com/apache/skywalking-java/pull/460#discussion_r1115128570
##########
CHANGES.md:
##########
@@ -16,6 +16,7 @@ Release Notes.
* Refactor kotlin coroutine plugin with CoroutineContext.
* Fix OracleURLParser ignoring actual port when :SID is absent.
* Change gRPC instrumentation point to fix plugin not working for server side.
+* Fix servicecomb plugin trace break.
Review Comment:
We don't recommend this, as the git commit shows the relationship.
--
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.
To unsubscribe, e-mail: notifications-unsubscribe@skywalking.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [skywalking-java] wu-sheng commented on pull request #460: Fix servicecomb plugin trace break
Posted by "wu-sheng (via GitHub)" <gi...@apache.org>.
wu-sheng commented on PR #460:
URL: https://github.com/apache/skywalking-java/pull/460#issuecomment-1439250156
Please follow codestyle.xml in the root, and fix CI which fails due to code style issues.
--
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.
To unsubscribe, e-mail: notifications-unsubscribe@skywalking.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [skywalking-java] Chenfx-git commented on a diff in pull request #460: Fix servicecomb plugin trace break
Posted by "Chenfx-git (via GitHub)" <gi...@apache.org>.
Chenfx-git commented on code in PR #460:
URL: https://github.com/apache/skywalking-java/pull/460#discussion_r1114049353
##########
apm-sniffer/apm-sdk-plugin/servicecomb-plugin/servicecomb-java-chassis-2.x-plugin/src/main/java/org/apache/skywalking/apm/plugin/servicecomb/v2/ProducerOperationHandlerInterceptor.java:
##########
@@ -40,9 +39,14 @@ public void beforeMethod(EnhancedInstance objInst, Method method, Object[] allAr
Invocation invocation = (Invocation) allArguments[0];
ContextCarrier contextCarrier = new ContextCarrier();
CarrierItem next = contextCarrier.items();
+ boolean inContext = isInContext(invocation);
while (next.hasNext()) {
next = next.next();
- next.setHeadValue(invocation.getContext().get(next.getHeadKey()));
+ if (inContext) {
+ next.setHeadValue(invocation.getContext().get(next.getHeadKey()));
+ }else {
+ invocation.getRequestEx().getHeader(next.getHeadKey());
+ }
Review Comment:
no CseRestTemplate plugin.
Here are two invoke process:
1、RestTemplateBuilder.create().postForObject("http://127.0.0.1:8080/springmvchello/hi", null, String.class); // trace break
consumer --> **RestTemplate** --> provider --> ProducerOperationHandler
2、RestTemplateBuilder.create().postForObject("cse://test/springmvchello/hello", person, String.class); // trace ok
consumer --> **CseRestTemplate --> TransportClientHandler** --> provider --> ProducerOperationHandler
--
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.
To unsubscribe, e-mail: notifications-unsubscribe@skywalking.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [skywalking-java] wu-sheng merged pull request #460: Fix servicecomb plugin trace break
Posted by "wu-sheng (via GitHub)" <gi...@apache.org>.
wu-sheng merged PR #460:
URL: https://github.com/apache/skywalking-java/pull/460
--
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.
To unsubscribe, e-mail: notifications-unsubscribe@skywalking.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [skywalking-java] Chenfx-git commented on a diff in pull request #460: Fix servicecomb plugin trace break
Posted by "Chenfx-git (via GitHub)" <gi...@apache.org>.
Chenfx-git commented on code in PR #460:
URL: https://github.com/apache/skywalking-java/pull/460#discussion_r1112475104
##########
apm-sniffer/apm-sdk-plugin/servicecomb-plugin/servicecomb-java-chassis-2.x-plugin/src/main/java/org/apache/skywalking/apm/plugin/servicecomb/v2/ProducerOperationHandlerInterceptor.java:
##########
@@ -52,6 +56,13 @@ public void beforeMethod(EnhancedInstance objInst, Method method, Object[] allAr
SpanLayer.asRPCFramework(span);
}
+ private boolean isInContext(Invocation invocation) {
+ Map<String, String> context = invocation.getContext();
+ return context.containsKey(SW8CarrierItem.HEADER_NAME) ||
+ context.containsKey(SW8CorrelationCarrierItem.HEADER_NAME) ||
+ context.containsKey(SW8ExtensionCarrierItem.HEADER_NAME);
Review Comment:
if invocation context don't contains trace message, try to get from http header
--
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.
To unsubscribe, e-mail: notifications-unsubscribe@skywalking.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [skywalking-java] wu-sheng commented on a diff in pull request #460: Fix servicecomb plugin trace break
Posted by "wu-sheng (via GitHub)" <gi...@apache.org>.
wu-sheng commented on code in PR #460:
URL: https://github.com/apache/skywalking-java/pull/460#discussion_r1112484372
##########
apm-sniffer/apm-sdk-plugin/servicecomb-plugin/servicecomb-java-chassis-2.x-plugin/src/main/java/org/apache/skywalking/apm/plugin/servicecomb/v2/ProducerOperationHandlerInterceptor.java:
##########
@@ -40,9 +39,14 @@ public void beforeMethod(EnhancedInstance objInst, Method method, Object[] allAr
Invocation invocation = (Invocation) allArguments[0];
ContextCarrier contextCarrier = new ContextCarrier();
CarrierItem next = contextCarrier.items();
+ boolean inContext = isInContext(invocation);
while (next.hasNext()) {
next = next.next();
- next.setHeadValue(invocation.getContext().get(next.getHeadKey()));
+ if (inContext) {
+ next.setHeadValue(invocation.getContext().get(next.getHeadKey()));
+ }else {
+ invocation.getRequestEx().getHeader(next.getHeadKey());
+ }
Review Comment:
If you want to fail back, don't try to on your own, try to get from context, if empty or null, fall back to from request ext
But still, your codes don't show logic about why this is putting in two places, because client side code doesn't show difference.
--
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.
To unsubscribe, e-mail: notifications-unsubscribe@skywalking.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [skywalking-java] wu-sheng commented on a diff in pull request #460: Fix servicecomb plugin trace break
Posted by "wu-sheng (via GitHub)" <gi...@apache.org>.
wu-sheng commented on code in PR #460:
URL: https://github.com/apache/skywalking-java/pull/460#discussion_r1112087772
##########
apm-sniffer/apm-sdk-plugin/servicecomb-plugin/servicecomb-java-chassis-2.x-plugin/src/main/java/org/apache/skywalking/apm/plugin/servicecomb/v2/ProducerOperationHandlerInterceptor.java:
##########
@@ -52,6 +56,13 @@ public void beforeMethod(EnhancedInstance objInst, Method method, Object[] allAr
SpanLayer.asRPCFramework(span);
}
+ private boolean isInContext(Invocation invocation) {
+ Map<String, String> context = invocation.getContext();
+ return context.containsKey(SW8CarrierItem.HEADER_NAME) ||
+ context.containsKey(SW8CorrelationCarrierItem.HEADER_NAME) ||
+ context.containsKey(SW8ExtensionCarrierItem.HEADER_NAME);
Review Comment:
Please explain what you mean by this. It is very confused.
--
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.
To unsubscribe, e-mail: notifications-unsubscribe@skywalking.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [skywalking-java] wu-sheng commented on a diff in pull request #460: Fix servicecomb plugin trace break
Posted by "wu-sheng (via GitHub)" <gi...@apache.org>.
wu-sheng commented on code in PR #460:
URL: https://github.com/apache/skywalking-java/pull/460#discussion_r1115214194
##########
apm-sniffer/apm-sdk-plugin/servicecomb-plugin/servicecomb-java-chassis-2.x-plugin/src/main/java/org/apache/skywalking/apm/plugin/servicecomb/v2/ProducerOperationHandlerInterceptor.java:
##########
@@ -42,7 +42,12 @@ public void beforeMethod(EnhancedInstance objInst, Method method, Object[] allAr
CarrierItem next = contextCarrier.items();
while (next.hasNext()) {
next = next.next();
- next.setHeadValue(invocation.getContext().get(next.getHeadKey()));
+ String headKey = next.getHeadKey();
+ if (invocation.getContext().containsKey(headKey)) {
+ next.setHeadValue(invocation.getContext().get(headKey));
+ } else {
+ next.setHeadValue(invocation.getRequestEx().getHeader(headKey));
Review Comment:
I think the request to code 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.
To unsubscribe, e-mail: notifications-unsubscribe@skywalking.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [skywalking-java] wu-sheng commented on a diff in pull request #460: Fix servicecomb plugin trace break
Posted by "wu-sheng (via GitHub)" <gi...@apache.org>.
wu-sheng commented on code in PR #460:
URL: https://github.com/apache/skywalking-java/pull/460#discussion_r1113692310
##########
apm-sniffer/apm-sdk-plugin/servicecomb-plugin/servicecomb-java-chassis-2.x-plugin/src/main/java/org/apache/skywalking/apm/plugin/servicecomb/v2/ProducerOperationHandlerInterceptor.java:
##########
@@ -40,9 +39,14 @@ public void beforeMethod(EnhancedInstance objInst, Method method, Object[] allAr
Invocation invocation = (Invocation) allArguments[0];
ContextCarrier contextCarrier = new ContextCarrier();
CarrierItem next = contextCarrier.items();
+ boolean inContext = isInContext(invocation);
while (next.hasNext()) {
next = next.next();
- next.setHeadValue(invocation.getContext().get(next.getHeadKey()));
+ if (inContext) {
+ next.setHeadValue(invocation.getContext().get(next.getHeadKey()));
+ }else {
+ invocation.getRequestEx().getHeader(next.getHeadKey());
+ }
Review Comment:
My question is, where is `cseRestemplate`? I can't see the plugin for it. How it works?
--
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.
To unsubscribe, e-mail: notifications-unsubscribe@skywalking.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [skywalking-java] Chenfx-git commented on a diff in pull request #460: Fix servicecomb plugin trace break
Posted by "Chenfx-git (via GitHub)" <gi...@apache.org>.
Chenfx-git commented on code in PR #460:
URL: https://github.com/apache/skywalking-java/pull/460#discussion_r1112473092
##########
apm-sniffer/apm-sdk-plugin/servicecomb-plugin/servicecomb-java-chassis-2.x-plugin/src/main/java/org/apache/skywalking/apm/plugin/servicecomb/v2/ProducerOperationHandlerInterceptor.java:
##########
@@ -52,6 +52,13 @@ public void beforeMethod(EnhancedInstance objInst, Method method, Object[] allAr
SpanLayer.asRPCFramework(span);
}
+ private String getHeadValue(Invocation invocation, String key) {
+ if (invocation.getContext().containsKey("sw8")) {
Review Comment:
issues: https://github.com/apache/skywalking/issues/10422
--
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.
To unsubscribe, e-mail: notifications-unsubscribe@skywalking.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [skywalking-java] wu-sheng commented on a diff in pull request #460: Fix servicecomb plugin trace break
Posted by "wu-sheng (via GitHub)" <gi...@apache.org>.
wu-sheng commented on code in PR #460:
URL: https://github.com/apache/skywalking-java/pull/460#discussion_r1112482884
##########
apm-sniffer/apm-sdk-plugin/servicecomb-plugin/servicecomb-java-chassis-2.x-plugin/src/main/java/org/apache/skywalking/apm/plugin/servicecomb/v2/ProducerOperationHandlerInterceptor.java:
##########
@@ -52,6 +56,13 @@ public void beforeMethod(EnhancedInstance objInst, Method method, Object[] allAr
SpanLayer.asRPCFramework(span);
}
+ private boolean isInContext(Invocation invocation) {
+ Map<String, String> context = invocation.getContext();
+ return context.containsKey(SW8CarrierItem.HEADER_NAME) ||
+ context.containsKey(SW8CorrelationCarrierItem.HEADER_NAME) ||
+ context.containsKey(SW8ExtensionCarrierItem.HEADER_NAME);
Review Comment:
Headers should not listed, it should use next.getHeaderKey.
The kernel controls thd header keys, not plugin.
--
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.
To unsubscribe, e-mail: notifications-unsubscribe@skywalking.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [skywalking-java] wu-sheng commented on a diff in pull request #460: Fix servicecomb plugin trace break
Posted by "wu-sheng (via GitHub)" <gi...@apache.org>.
wu-sheng commented on code in PR #460:
URL: https://github.com/apache/skywalking-java/pull/460#discussion_r1112482277
##########
apm-sniffer/apm-sdk-plugin/servicecomb-plugin/servicecomb-java-chassis-2.x-plugin/src/main/java/org/apache/skywalking/apm/plugin/servicecomb/v2/ProducerOperationHandlerInterceptor.java:
##########
@@ -52,6 +52,13 @@ public void beforeMethod(EnhancedInstance objInst, Method method, Object[] allAr
SpanLayer.asRPCFramework(span);
}
+ private String getHeadValue(Invocation invocation, String key) {
+ if (invocation.getContext().containsKey("sw8")) {
Review Comment:
I don't mean the issue of GitHub. I mean what is the issue of codes.
You just mentioned trace broken, but what you changed is very strange.
--
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.
To unsubscribe, e-mail: notifications-unsubscribe@skywalking.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [skywalking-java] Chenfx-git commented on a diff in pull request #460: Fix servicecomb plugin trace break
Posted by "Chenfx-git (via GitHub)" <gi...@apache.org>.
Chenfx-git commented on code in PR #460:
URL: https://github.com/apache/skywalking-java/pull/460#discussion_r1111991291
##########
apm-sniffer/apm-sdk-plugin/servicecomb-plugin/servicecomb-java-chassis-2.x-plugin/src/main/java/org/apache/skywalking/apm/plugin/servicecomb/v2/ProducerOperationHandlerInterceptor.java:
##########
@@ -52,6 +52,13 @@ public void beforeMethod(EnhancedInstance objInst, Method method, Object[] allAr
SpanLayer.asRPCFramework(span);
}
+ private String getHeadValue(Invocation invocation, String key) {
+ if (invocation.getContext().containsKey("sw8")) {
Review Comment:
How can i know trace message in http header or servicecomb context? How many kind of key in skywalking like sw8
--
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.
To unsubscribe, e-mail: notifications-unsubscribe@skywalking.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org