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 2022/02/11 04:02:13 UTC
[GitHub] [skywalking-java] honganan opened a new pull request #110: fix Dubbo3.x Context problem #8525
honganan opened a new pull request #110:
URL: https://github.com/apache/skywalking-java/pull/110
<!--
β οΈ Please make sure to read this template first, pull requests that don't accord with this template
maybe closed without notice.
Texts surrounded by `<` and `>` are meant to be replaced by you, e.g. <framework name>, <issue number>.
Put an `x` in the `[ ]` to mark the item as CHECKED. `[x]`
-->
<!-- ==== π Remove this line WHEN AND ONLY WHEN you're fixing a bug, follow the checklist π ====
### Fix <bug description or the bug issue number or bug issue link>
- [ ] Add a unit test to verify that the fix works.
- [ ] Explain briefly why the bug exists and how to fix it.
==== π Remove this line WHEN AND ONLY WHEN you're fixing a bug, follow the checklist π ==== -->
<!-- ==== π Remove this line WHEN AND ONLY WHEN you're adding a new plugin, follow the checklist π ====
### Add an agent plugin to support <framework name>
- [ ] Add a test case for the new plugin, refer to [the doc](https://github.com/apache/skywalking-java/blob/main/docs/en/setup/service-agent/java-agent/Plugin-test.md)
- [ ] Add a component id in [the component-libraries.yml](https://github.com/apache/skywalking/blob/master/oap-server/server-starter/src/main/resources/component-libraries.yml)
- [ ] Add a logo in [the UI repo](https://github.com/apache/skywalking-rocketbot-ui/tree/master/src/views/components/topology/assets)
==== π Remove this line WHEN AND ONLY WHEN you're adding a new plugin, follow the checklist π ==== -->
<!-- ==== π Remove this line WHEN AND ONLY WHEN you're improving the performance, follow the checklist π ====
### Improve the performance of <class or module or ...>
- [ ] Add a benchmark for the improvement, refer to [the existing ones](https://github.com/apache/skywalking-java/blob/main/apm-commons/apm-datacarrier/src/test/java/org/apache/skywalking/apm/commons/datacarrier/LinkedArrayBenchmark.java)
- [ ] The benchmark result.
```text
<Paste the benchmark results here>
```
- [ ] Links/URLs to the theory proof or discussion articles/blogs. <links/URLs here>
==== π Remove this line WHEN AND ONLY WHEN you're improving the performance, follow the checklist π ==== -->
<!-- ==== π Remove this line WHEN AND ONLY WHEN you're adding a new feature, follow the checklist π ====
### <Feature description>
- [ ] If this is non-trivial feature, paste the links/URLs to the design doc.
- [ ] Update the documentation to include this new feature.
- [ ] Tests(including UT, IT, E2E) are added to verify the new feature.
- [ ] If it's UI related, attach the screenshots below.
==== π Remove this line WHEN AND ONLY WHEN you're adding a new feature, follow the checklist π ==== -->
- [ ] If this pull request closes/resolves/fixes an existing issue, replace the issue number. Closes #<issue number>.
- [ ] Update the [`CHANGES` log](https://github.com/apache/skywalking-java/blob/main/CHANGES.md).
--
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 change in pull request #110: fix Dubbo3.x Context problem #8525
Posted by GitBox <gi...@apache.org>.
wu-sheng commented on a change in pull request #110:
URL: https://github.com/apache/skywalking-java/pull/110#discussion_r813692112
##########
File path: apm-sniffer/apm-sdk-plugin/dubbo-3.x-plugin/src/main/java/org/apache/skywalking/apm/plugin/asf/dubbo3/DubboInterceptor.java
##########
@@ -151,6 +137,19 @@ private void dealException(Throwable throwable) {
span.log(throwable);
}
+ /**
+ * to judge if current is in provider side.
+ *
+ * @param rpcInvocation
+ * @return
+ */
+ private static boolean isProvider(RpcInvocation rpcInvocation) {
+ Invoker<?> invoker = rpcInvocation.getInvoker();
+ return invoker.getUrl()
+ .getParameter("side", "consumer")
+ .equals("provider");
Review comment:
Check instrumentation definition, `DubboInstrumentationBase`.
```java
public static final String CONSUMER_ENHANCE_CLASS = "org.apache.dubbo.monitor.support.MonitorClusterFilter";
```
--
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] honganan commented on a change in pull request #110: fix Dubbo3.x Context problem #8525
Posted by GitBox <gi...@apache.org>.
honganan commented on a change in pull request #110:
URL: https://github.com/apache/skywalking-java/pull/110#discussion_r813504146
##########
File path: apm-sniffer/apm-sdk-plugin/dubbo-3.x-plugin/src/main/java/org/apache/skywalking/apm/plugin/asf/dubbo3/DubboInterceptor.java
##########
@@ -151,6 +137,19 @@ private void dealException(Throwable throwable) {
span.log(throwable);
}
+ /**
+ * to judge if current is in provider side.
+ *
+ * @param rpcInvocation
+ * @return
+ */
+ private static boolean isProvider(RpcInvocation rpcInvocation) {
+ Invoker<?> invoker = rpcInvocation.getInvoker();
+ return invoker.getUrl()
+ .getParameter("side", "consumer")
+ .equals("provider");
Review comment:
Seems no need to `try/catch`, just deal the `result.getException` look 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] wu-sheng commented on pull request #110: fix Dubbo3.x Context problem #8525
Posted by GitBox <gi...@apache.org>.
wu-sheng commented on pull request #110:
URL: https://github.com/apache/skywalking-java/pull/110#issuecomment-1046196601
@honganan Could you make this PR works? I am not sure why Dubbocommunity seems not responsing. Let's try at our side.
--
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] honganan commented on a change in pull request #110: fix Dubbo3.x Context problem #8525
Posted by GitBox <gi...@apache.org>.
honganan commented on a change in pull request #110:
URL: https://github.com/apache/skywalking-java/pull/110#discussion_r813706865
##########
File path: apm-sniffer/apm-sdk-plugin/dubbo-3.x-plugin/src/main/java/org/apache/skywalking/apm/plugin/asf/dubbo3/DubboInterceptor.java
##########
@@ -151,6 +137,19 @@ private void dealException(Throwable throwable) {
span.log(throwable);
}
+ /**
+ * to judge if current is in provider side.
+ *
+ * @param rpcInvocation
+ * @return
+ */
+ private static boolean isProvider(RpcInvocation rpcInvocation) {
+ Invoker<?> invoker = rpcInvocation.getInvoker();
+ return invoker.getUrl()
+ .getParameter("side", "consumer")
+ .equals("provider");
Review comment:
I see `MonitorClusterFilter` is extends `MonitorFilter` and `invoke()` method is defined in parent `MonitorFilter` class, at this time SW will enhance `MonitorFilter#invoke()` method twice? One for the subclass, another for parent class, as we specified both `MonitorClusterFilter#invoke()` and `MonitorFilter#invoke()`.
--
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 change in pull request #110: fix Dubbo3.x Context problem #8525
Posted by GitBox <gi...@apache.org>.
wu-sheng commented on a change in pull request #110:
URL: https://github.com/apache/skywalking-java/pull/110#discussion_r813787665
##########
File path: apm-sniffer/apm-sdk-plugin/dubbo-3.x-plugin/src/main/java/org/apache/skywalking/apm/plugin/asf/dubbo3/DubboInterceptor.java
##########
@@ -65,15 +63,7 @@ public void beforeMethod(EnhancedInstance objInst, Method method, Object[] allAr
Invoker invoker = (Invoker) allArguments[0];
Invocation invocation = (Invocation) allArguments[1];
- RpcServiceContext serviceContext = RpcContext.getServiceContext();
- boolean isConsumer;
- URL url = serviceContext.getUrl();
- if (url == null) {
- url = serviceContext.getConsumerUrl();
- isConsumer = url != null;
- } else {
- isConsumer = serviceContext.isConsumerSide();
- }
+ boolean isConsumer = !isProvider(invocation);
Review comment:
Is better to change method name to isConsumer? Assigning Not isProvider to a variable seems 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] honganan commented on a change in pull request #110: fix Dubbo3.x Context problem #8525
Posted by GitBox <gi...@apache.org>.
honganan commented on a change in pull request #110:
URL: https://github.com/apache/skywalking-java/pull/110#discussion_r813487723
##########
File path: apm-sniffer/apm-sdk-plugin/dubbo-3.x-plugin/src/main/java/org/apache/skywalking/apm/plugin/asf/dubbo3/DubboInterceptor.java
##########
@@ -151,6 +137,19 @@ private void dealException(Throwable throwable) {
span.log(throwable);
}
+ /**
+ * to judge if current is in provider side.
+ *
+ * @param rpcInvocation
+ * @return
+ */
+ private static boolean isProvider(RpcInvocation rpcInvocation) {
+ Invoker<?> invoker = rpcInvocation.getInvoker();
+ return invoker.getUrl()
+ .getParameter("side", "consumer")
+ .equals("provider");
Review comment:
OK. And I know the problem now, Dubbo will wrap exception so deal with it here(`#afterMethod`) have no problem.
It log error twice because both `MonitorFilter` and `MonitorClusterFilter` invoked when it is consumer side. It looks we enhance classes not precisely enough, here is the stack:
```
afterMethod:120, DubboInterceptor (org.apache.skywalking.apm.plugin.asf.dubbo3)
**intercept:97, InstMethodsInter (org.apache.skywalking.apm.agent.core.plugin.interceptor.enhance)
invoke:-1, MonitorFilter (org.apache.dubbo.monitor.support)**
invoke$accessor$Jzsj124D:-1, MonitorClusterFilter (org.apache.dubbo.monitor.support)
call:-1, MonitorClusterFilter$auxiliary$xlYbM1kf (org.apache.dubbo.monitor.support)
**intercept:86, InstMethodsInter (org.apache.skywalking.apm.agent.core.plugin.interceptor.enhance)
invoke:-1, MonitorClusterFilter (org.apache.dubbo.monitor.support)**
invoke:84, FilterChainBuilder$FilterChainNode (org.apache.dubbo.rpc.cluster.filter)
```
We can filter the `method.getDeclaringClass()` if it's `MonitorClusterFilter` to avoid this, if no more graceful way.
--
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] AlbumenJ commented on a change in pull request #110: fix Dubbo3.x Context problem #8525
Posted by GitBox <gi...@apache.org>.
AlbumenJ commented on a change in pull request #110:
URL: https://github.com/apache/skywalking-java/pull/110#discussion_r820416737
##########
File path: apm-sniffer/apm-sdk-plugin/dubbo-3.x-plugin/src/main/java/org/apache/skywalking/apm/plugin/asf/dubbo3/DubboInterceptor.java
##########
@@ -65,15 +63,7 @@ public void beforeMethod(EnhancedInstance objInst, Method method, Object[] allAr
Invoker invoker = (Invoker) allArguments[0];
Invocation invocation = (Invocation) allArguments[1];
- RpcServiceContext serviceContext = RpcContext.getServiceContext();
- boolean isConsumer;
- URL url = serviceContext.getUrl();
- if (url == null) {
- url = serviceContext.getConsumerUrl();
- isConsumer = url != null;
- } else {
- isConsumer = serviceContext.isConsumerSide();
- }
+ boolean isConsumer = isConsumer(invocation);
Review comment:
@wu-sheng got it!
--
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] honganan commented on a change in pull request #110: fix Dubbo3.x Context problem #8525
Posted by GitBox <gi...@apache.org>.
honganan commented on a change in pull request #110:
URL: https://github.com/apache/skywalking-java/pull/110#discussion_r813487723
##########
File path: apm-sniffer/apm-sdk-plugin/dubbo-3.x-plugin/src/main/java/org/apache/skywalking/apm/plugin/asf/dubbo3/DubboInterceptor.java
##########
@@ -151,6 +137,19 @@ private void dealException(Throwable throwable) {
span.log(throwable);
}
+ /**
+ * to judge if current is in provider side.
+ *
+ * @param rpcInvocation
+ * @return
+ */
+ private static boolean isProvider(RpcInvocation rpcInvocation) {
+ Invoker<?> invoker = rpcInvocation.getInvoker();
+ return invoker.getUrl()
+ .getParameter("side", "consumer")
+ .equals("provider");
Review comment:
OK. And I know the problem, Dubbo will wrap exception so deal with it here(`#afterMethod`) have no problem.
It log error twice because both `MonitorFilter` and `MonitorClusterFilter` invoked when it is consumer side. It look we enhance classes not precisely enough, here is the stack:
```
afterMethod:120, DubboInterceptor (org.apache.skywalking.apm.plugin.asf.dubbo3)
**intercept:97, InstMethodsInter (org.apache.skywalking.apm.agent.core.plugin.interceptor.enhance)
invoke:-1, MonitorFilter (org.apache.dubbo.monitor.support)**
invoke$accessor$Jzsj124D:-1, MonitorClusterFilter (org.apache.dubbo.monitor.support)
call:-1, MonitorClusterFilter$auxiliary$xlYbM1kf (org.apache.dubbo.monitor.support)
**intercept:86, InstMethodsInter (org.apache.skywalking.apm.agent.core.plugin.interceptor.enhance)
invoke:-1, MonitorClusterFilter (org.apache.dubbo.monitor.support)**
invoke:84, FilterChainBuilder$FilterChainNode (org.apache.dubbo.rpc.cluster.filter)
```
We can filter the `method.getDeclaringClass()` if it's `MonitorClusterFilter` to avoid this, if no more graceful way.
--
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] honganan commented on a change in pull request #110: fix Dubbo3.x Context problem #8525
Posted by GitBox <gi...@apache.org>.
honganan commented on a change in pull request #110:
URL: https://github.com/apache/skywalking-java/pull/110#discussion_r813487723
##########
File path: apm-sniffer/apm-sdk-plugin/dubbo-3.x-plugin/src/main/java/org/apache/skywalking/apm/plugin/asf/dubbo3/DubboInterceptor.java
##########
@@ -151,6 +137,19 @@ private void dealException(Throwable throwable) {
span.log(throwable);
}
+ /**
+ * to judge if current is in provider side.
+ *
+ * @param rpcInvocation
+ * @return
+ */
+ private static boolean isProvider(RpcInvocation rpcInvocation) {
+ Invoker<?> invoker = rpcInvocation.getInvoker();
+ return invoker.getUrl()
+ .getParameter("side", "consumer")
+ .equals("provider");
Review comment:
OK. And I know the problem now, Dubbo will wrap exception so deal with it here(`#afterMethod`) have no problem.
It log error twice because both `MonitorFilter` and `MonitorClusterFilter` invoked when it is consumer side. It look we enhance classes not precisely enough, here is the stack:
```
afterMethod:120, DubboInterceptor (org.apache.skywalking.apm.plugin.asf.dubbo3)
**intercept:97, InstMethodsInter (org.apache.skywalking.apm.agent.core.plugin.interceptor.enhance)
invoke:-1, MonitorFilter (org.apache.dubbo.monitor.support)**
invoke$accessor$Jzsj124D:-1, MonitorClusterFilter (org.apache.dubbo.monitor.support)
call:-1, MonitorClusterFilter$auxiliary$xlYbM1kf (org.apache.dubbo.monitor.support)
**intercept:86, InstMethodsInter (org.apache.skywalking.apm.agent.core.plugin.interceptor.enhance)
invoke:-1, MonitorClusterFilter (org.apache.dubbo.monitor.support)**
invoke:84, FilterChainBuilder$FilterChainNode (org.apache.dubbo.rpc.cluster.filter)
```
We can filter the `method.getDeclaringClass()` if it's `MonitorClusterFilter` to avoid this, if no more graceful way.
--
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 change in pull request #110: fix Dubbo3.x Context problem #8525
Posted by GitBox <gi...@apache.org>.
wu-sheng commented on a change in pull request #110:
URL: https://github.com/apache/skywalking-java/pull/110#discussion_r813502862
##########
File path: apm-sniffer/apm-sdk-plugin/dubbo-3.x-plugin/src/main/java/org/apache/skywalking/apm/plugin/asf/dubbo3/DubboInterceptor.java
##########
@@ -151,6 +137,19 @@ private void dealException(Throwable throwable) {
span.log(throwable);
}
+ /**
+ * to judge if current is in provider side.
+ *
+ * @param rpcInvocation
+ * @return
+ */
+ private static boolean isProvider(RpcInvocation rpcInvocation) {
+ Invoker<?> invoker = rpcInvocation.getInvoker();
+ return invoker.getUrl()
+ .getParameter("side", "consumer")
+ .equals("provider");
Review comment:
Agree, targeting a specific one class seems a good idea to avoid duplicated logging. But the original `try/catch` for `dealException` method itself seems not reasonable, WDYT?
--
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 change in pull request #110: fix Dubbo3.x Context problem #8525
Posted by GitBox <gi...@apache.org>.
wu-sheng commented on a change in pull request #110:
URL: https://github.com/apache/skywalking-java/pull/110#discussion_r820358793
##########
File path: apm-sniffer/apm-sdk-plugin/dubbo-3.x-plugin/src/main/java/org/apache/skywalking/apm/plugin/asf/dubbo3/DubboInterceptor.java
##########
@@ -65,15 +63,7 @@ public void beforeMethod(EnhancedInstance objInst, Method method, Object[] allAr
Invoker invoker = (Invoker) allArguments[0];
Invocation invocation = (Invocation) allArguments[1];
- RpcServiceContext serviceContext = RpcContext.getServiceContext();
- boolean isConsumer;
- URL url = serviceContext.getUrl();
- if (url == null) {
- url = serviceContext.getConsumerUrl();
- isConsumer = url != null;
- } else {
- isConsumer = serviceContext.isConsumerSide();
- }
+ boolean isConsumer = isConsumer(invocation);
Review comment:
@AlbumenJ Agree with @honganan , as a byte code manipulator, once we change the parent class, and the sub classes don't override the changed method, this enhancement actually changes the behavior of all classes on this hierarchy tree.
--
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 #110: fix Dubbo3.x Context problem #8525
Posted by GitBox <gi...@apache.org>.
wu-sheng commented on pull request #110:
URL: https://github.com/apache/skywalking-java/pull/110#issuecomment-1048425103
@honganan Any update?
--
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] honganan commented on pull request #110: fix Dubbo3.x Context problem #8525
Posted by GitBox <gi...@apache.org>.
honganan commented on pull request #110:
URL: https://github.com/apache/skywalking-java/pull/110#issuecomment-1048566288
I can, in these two days. I need more time to make it clear how Dubbo handle exception.
--
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] honganan commented on pull request #110: fix Dubbo3.x Context problem #8525
Posted by GitBox <gi...@apache.org>.
honganan commented on pull request #110:
URL: https://github.com/apache/skywalking-java/pull/110#issuecomment-1050470889
It's strange the Unit Test fails with NPE, unless `@Before` annotated method not invoked. Can you help me check code of `DubboInterceptorTest`
--
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] honganan commented on a change in pull request #110: fix Dubbo3.x Context problem #8525
Posted by GitBox <gi...@apache.org>.
honganan commented on a change in pull request #110:
URL: https://github.com/apache/skywalking-java/pull/110#discussion_r813487723
##########
File path: apm-sniffer/apm-sdk-plugin/dubbo-3.x-plugin/src/main/java/org/apache/skywalking/apm/plugin/asf/dubbo3/DubboInterceptor.java
##########
@@ -151,6 +137,19 @@ private void dealException(Throwable throwable) {
span.log(throwable);
}
+ /**
+ * to judge if current is in provider side.
+ *
+ * @param rpcInvocation
+ * @return
+ */
+ private static boolean isProvider(RpcInvocation rpcInvocation) {
+ Invoker<?> invoker = rpcInvocation.getInvoker();
+ return invoker.getUrl()
+ .getParameter("side", "consumer")
+ .equals("provider");
Review comment:
OK. And I know the problem now, Dubbo will wrap exception so deal with it here(`#afterMethod`) have no problem.
It log error twice because both `MonitorFilter` and `MonitorClusterFilter` invoked when it is consumer side. It looks we enhance classes not precisely enough, here is the stack:
afterMethod:120, DubboInterceptor (org.apache.skywalking.apm.plugin.asf.dubbo3)
**intercept:97, InstMethodsInter (org.apache.skywalking.apm.agent.core.plugin.interceptor.enhance)
invoke:-1, MonitorFilter (org.apache.dubbo.monitor.support)**
invoke$accessor$Jzsj124D:-1, MonitorClusterFilter (org.apache.dubbo.monitor.support)
call:-1, MonitorClusterFilter$auxiliary$xlYbM1kf (org.apache.dubbo.monitor.support)
**intercept:86, InstMethodsInter (org.apache.skywalking.apm.agent.core.plugin.interceptor.enhance)
invoke:-1, MonitorClusterFilter (org.apache.dubbo.monitor.support)**
invoke:84, FilterChainBuilder$FilterChainNode (org.apache.dubbo.rpc.cluster.filter)
We can filter the `method.getDeclaringClass()` if it's `MonitorClusterFilter` to avoid this, if no more graceful way.
--
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] AlbumenJ commented on a change in pull request #110: fix Dubbo3.x Context problem #8525
Posted by GitBox <gi...@apache.org>.
AlbumenJ commented on a change in pull request #110:
URL: https://github.com/apache/skywalking-java/pull/110#discussion_r820351611
##########
File path: apm-sniffer/apm-sdk-plugin/dubbo-3.x-plugin/src/main/java/org/apache/skywalking/apm/plugin/asf/dubbo3/DubboInterceptor.java
##########
@@ -65,15 +63,7 @@ public void beforeMethod(EnhancedInstance objInst, Method method, Object[] allAr
Invoker invoker = (Invoker) allArguments[0];
Invocation invocation = (Invocation) allArguments[1];
- RpcServiceContext serviceContext = RpcContext.getServiceContext();
- boolean isConsumer;
- URL url = serviceContext.getUrl();
- if (url == null) {
- url = serviceContext.getConsumerUrl();
- isConsumer = url != null;
- } else {
- isConsumer = serviceContext.isConsumerSide();
- }
+ boolean isConsumer = isConsumer(invocation);
Review comment:
I'm a little confused about this. Why not judge whether it is the consumer side directly from `EnhancedInstance objInst`. If objInst is a MonitorFilter object, it should be on the provider side. If objInst is a MonitorClusterFilter object, it should be on the provider side.
--
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 change in pull request #110: fix Dubbo3.x Context problem #8525
Posted by GitBox <gi...@apache.org>.
wu-sheng commented on a change in pull request #110:
URL: https://github.com/apache/skywalking-java/pull/110#discussion_r813513728
##########
File path: apm-sniffer/apm-sdk-plugin/dubbo-3.x-plugin/src/main/java/org/apache/skywalking/apm/plugin/asf/dubbo3/DubboInterceptor.java
##########
@@ -151,6 +137,19 @@ private void dealException(Throwable throwable) {
span.log(throwable);
}
+ /**
+ * to judge if current is in provider side.
+ *
+ * @param rpcInvocation
+ * @return
+ */
+ private static boolean isProvider(RpcInvocation rpcInvocation) {
+ Invoker<?> invoker = rpcInvocation.getInvoker();
+ return invoker.getUrl()
+ .getParameter("side", "consumer")
+ .equals("provider");
Review comment:
For tracing perspective, only consumer side span's peer matters. But it is strange to be `null`.
--
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 change in pull request #110: fix Dubbo3.x Context problem #8525
Posted by GitBox <gi...@apache.org>.
wu-sheng commented on a change in pull request #110:
URL: https://github.com/apache/skywalking-java/pull/110#discussion_r813505179
##########
File path: apm-sniffer/apm-sdk-plugin/dubbo-3.x-plugin/src/main/java/org/apache/skywalking/apm/plugin/asf/dubbo3/DubboInterceptor.java
##########
@@ -151,6 +137,19 @@ private void dealException(Throwable throwable) {
span.log(throwable);
}
+ /**
+ * to judge if current is in provider side.
+ *
+ * @param rpcInvocation
+ * @return
+ */
+ private static boolean isProvider(RpcInvocation rpcInvocation) {
+ Invoker<?> invoker = rpcInvocation.getInvoker();
+ return invoker.getUrl()
+ .getParameter("side", "consumer")
+ .equals("provider");
Review comment:
+1 from me.
--
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] honganan commented on a change in pull request #110: fix Dubbo3.x Context problem #8525
Posted by GitBox <gi...@apache.org>.
honganan commented on a change in pull request #110:
URL: https://github.com/apache/skywalking-java/pull/110#discussion_r813680622
##########
File path: apm-sniffer/apm-sdk-plugin/dubbo-3.x-plugin/src/main/java/org/apache/skywalking/apm/plugin/asf/dubbo3/DubboInterceptor.java
##########
@@ -151,6 +137,19 @@ private void dealException(Throwable throwable) {
span.log(throwable);
}
+ /**
+ * to judge if current is in provider side.
+ *
+ * @param rpcInvocation
+ * @return
+ */
+ private static boolean isProvider(RpcInvocation rpcInvocation) {
+ Invoker<?> invoker = rpcInvocation.getInvoker();
+ return invoker.getUrl()
+ .getParameter("side", "consumer")
+ .equals("provider");
Review comment:
@wu-sheng Can you have a look at this, I want to filter by `objInst`'s class name, but found it's still `MonitorClusterFilter`. The stack is below, from my understanding, we are now intercepting `MonitorFilter` nested in `MonitorClusterFilter`, should the `objInst` be `MonitorFilter` class? How did SW internal works 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.
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 change in pull request #110: fix Dubbo3.x Context problem #8525
Posted by GitBox <gi...@apache.org>.
wu-sheng commented on a change in pull request #110:
URL: https://github.com/apache/skywalking-java/pull/110#discussion_r813720109
##########
File path: apm-sniffer/apm-sdk-plugin/dubbo-3.x-plugin/src/main/java/org/apache/skywalking/apm/plugin/asf/dubbo3/DubboInterceptor.java
##########
@@ -151,6 +137,19 @@ private void dealException(Throwable throwable) {
span.log(throwable);
}
+ /**
+ * to judge if current is in provider side.
+ *
+ * @param rpcInvocation
+ * @return
+ */
+ private static boolean isProvider(RpcInvocation rpcInvocation) {
+ Invoker<?> invoker = rpcInvocation.getInvoker();
+ return invoker.getUrl()
+ .getParameter("side", "consumer")
+ .equals("provider");
Review comment:
`MonitorClusterFilter` is a shell from the source code level. So, you only could intercept `MonitorFilter#invode`, no matter how you define this.
--
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] honganan commented on a change in pull request #110: fix Dubbo3.x Context problem #8525
Posted by GitBox <gi...@apache.org>.
honganan commented on a change in pull request #110:
URL: https://github.com/apache/skywalking-java/pull/110#discussion_r812695561
##########
File path: apm-sniffer/apm-sdk-plugin/dubbo-2.7.x-plugin/src/main/java/org/apache/skywalking/apm/plugin/asf/dubbo/DubboInterceptor.java
##########
@@ -110,14 +109,6 @@ public void beforeMethod(EnhancedInstance objInst, Method method, Object[] allAr
public Object afterMethod(EnhancedInstance objInst, Method method, Object[] allArguments, Class<?>[] argumentsTypes,
Object ret) throws Throwable {
Result result = (Result) ret;
- try {
- if (result != null && result.getException() != null) {
- dealException(result.getException());
- }
Review comment:
I haven't received notifications these days, I will debug and make it clear about how it process Exceptions.
--
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] honganan commented on pull request #110: fix Dubbo3.x Context problem #8525
Posted by GitBox <gi...@apache.org>.
honganan commented on pull request #110:
URL: https://github.com/apache/skywalking-java/pull/110#issuecomment-1048623250
OK
εθͺζηiPhone
------------------ Original ------------------
From: ε΄ζ Wu Sheng ***@***.***>
Date: Wed,Feb 23,2022 5:32 PM
To: apache/skywalking-java ***@***.***>
Cc: ζ΄ͺιΏε ***@***.***>, Mention ***@***.***>
Subject: Re: [apache/skywalking-java] fix Dubbo3.x Context problem #8525 (PR#110)
@wu-sheng commented on this pull request.
In apm-sniffer/apm-sdk-plugin/dubbo-3.x-plugin/src/main/java/org/apache/skywalking/apm/plugin/asf/dubbo3/DubboInterceptor.java:
> + return invoker.getUrl() + .getParameter("side", "consumer") + .equals("provider");
Oh, I see. Could you add this as comments on the codes? This is important to keep this in mind why we change now.
β
Reply to this email directly, view it on GitHub, or unsubscribe.
Triage notifications on the go with GitHub Mobile for iOS or Android.
You are receiving this because you were mentioned.Message ID: ***@***.***>
--
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] honganan commented on a change in pull request #110: fix Dubbo3.x Context problem #8525
Posted by GitBox <gi...@apache.org>.
honganan commented on a change in pull request #110:
URL: https://github.com/apache/skywalking-java/pull/110#discussion_r813730530
##########
File path: apm-sniffer/apm-sdk-plugin/dubbo-3.x-plugin/src/main/java/org/apache/skywalking/apm/plugin/asf/dubbo3/DubboInterceptor.java
##########
@@ -151,6 +137,19 @@ private void dealException(Throwable throwable) {
span.log(throwable);
}
+ /**
+ * to judge if current is in provider side.
+ *
+ * @param rpcInvocation
+ * @return
+ */
+ private static boolean isProvider(RpcInvocation rpcInvocation) {
+ Invoker<?> invoker = rpcInvocation.getInvoker();
+ return invoker.getUrl()
+ .getParameter("side", "consumer")
+ .equals("provider");
Review comment:
OK, same as I see. I will modify it and pass the test.
--
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 change in pull request #110: fix Dubbo3.x Context problem #8525
Posted by GitBox <gi...@apache.org>.
wu-sheng commented on a change in pull request #110:
URL: https://github.com/apache/skywalking-java/pull/110#discussion_r812539632
##########
File path: apm-sniffer/apm-sdk-plugin/dubbo-3.x-plugin/src/main/java/org/apache/skywalking/apm/plugin/asf/dubbo3/DubboInterceptor.java
##########
@@ -151,6 +137,19 @@ private void dealException(Throwable throwable) {
span.log(throwable);
}
+ /**
+ * to judge if current is in provider side.
+ *
+ * @param rpcInvocation
+ * @return
+ */
+ private static boolean isProvider(RpcInvocation rpcInvocation) {
+ Invoker<?> invoker = rpcInvocation.getInvoker();
+ return invoker.getUrl()
+ .getParameter("side", "consumer")
+ .equals("provider");
Review comment:
If this is already official, could we use `RpcServiceContext#isConsumerSide()` directly?
--
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] honganan commented on pull request #110: fix Dubbo3.x Context problem #8525
Posted by GitBox <gi...@apache.org>.
honganan commented on pull request #110:
URL: https://github.com/apache/skywalking-java/pull/110#issuecomment-1050469110
> I think this mock preparation is not enough, should be enhanced with the URL you expect.
Do you mean add clean `side` param in the URL in `DubboInterceptorTest`?
--
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] honganan commented on a change in pull request #110: fix Dubbo3.x Context problem #8525
Posted by GitBox <gi...@apache.org>.
honganan commented on a change in pull request #110:
URL: https://github.com/apache/skywalking-java/pull/110#discussion_r813724027
##########
File path: apm-sniffer/apm-sdk-plugin/dubbo-3.x-plugin/src/main/java/org/apache/skywalking/apm/plugin/asf/dubbo3/DubboInterceptor.java
##########
@@ -151,6 +137,19 @@ private void dealException(Throwable throwable) {
span.log(throwable);
}
+ /**
+ * to judge if current is in provider side.
+ *
+ * @param rpcInvocation
+ * @return
+ */
+ private static boolean isProvider(RpcInvocation rpcInvocation) {
+ Invoker<?> invoker = rpcInvocation.getInvoker();
+ return invoker.getUrl()
+ .getParameter("side", "consumer")
+ .equals("provider");
Review comment:
I don't know why here[https://github.com/apache/skywalking-java/pull/73#issuecomment-987712730](url) says it fails as Consumer not invoking `MonitorFilter`. As I see, logically, the Consumer did not use `MonitorFilter` and it uses `MonitorClusterFilter`, but from our class bytes perspective, both class bytes are defined in `MonitorFilter#invoke()`, we just need to enhance it once. Am I in the right way?
--
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] honganan commented on a change in pull request #110: fix Dubbo3.x Context problem #8525
Posted by GitBox <gi...@apache.org>.
honganan commented on a change in pull request #110:
URL: https://github.com/apache/skywalking-java/pull/110#discussion_r813512139
##########
File path: apm-sniffer/apm-sdk-plugin/dubbo-3.x-plugin/src/main/java/org/apache/skywalking/apm/plugin/asf/dubbo3/DubboInterceptor.java
##########
@@ -151,6 +137,19 @@ private void dealException(Throwable throwable) {
span.log(throwable);
}
+ /**
+ * to judge if current is in provider side.
+ *
+ * @param rpcInvocation
+ * @return
+ */
+ private static boolean isProvider(RpcInvocation rpcInvocation) {
+ Invoker<?> invoker = rpcInvocation.getInvoker();
+ return invoker.getUrl()
+ .getParameter("side", "consumer")
+ .equals("provider");
Review comment:
I will simplify it in 3.x plugin and run the test.
There are more problem beyond these, the peer of Span is not correct, ip is `null` when in consumer side and port is `0` in provider side. I will try to fix it today.
--
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] honganan commented on a change in pull request #110: fix Dubbo3.x Context problem #8525
Posted by GitBox <gi...@apache.org>.
honganan commented on a change in pull request #110:
URL: https://github.com/apache/skywalking-java/pull/110#discussion_r820353736
##########
File path: apm-sniffer/apm-sdk-plugin/dubbo-3.x-plugin/src/main/java/org/apache/skywalking/apm/plugin/asf/dubbo3/DubboInterceptor.java
##########
@@ -65,15 +63,7 @@ public void beforeMethod(EnhancedInstance objInst, Method method, Object[] allAr
Invoker invoker = (Invoker) allArguments[0];
Invocation invocation = (Invocation) allArguments[1];
- RpcServiceContext serviceContext = RpcContext.getServiceContext();
- boolean isConsumer;
- URL url = serviceContext.getUrl();
- if (url == null) {
- url = serviceContext.getConsumerUrl();
- isConsumer = url != null;
- } else {
- isConsumer = serviceContext.isConsumerSide();
- }
+ boolean isConsumer = isConsumer(invocation);
Review comment:
We just enhanced `MonitorFilter` class finally, as `MonitorClusterFilter ` extends `MonitorFilter` and enhance both of them will cause problem. We have talked above.
--
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 #110: fix Dubbo3.x Context problem #8525
Posted by GitBox <gi...@apache.org>.
wu-sheng commented on pull request #110:
URL: https://github.com/apache/skywalking-java/pull/110#issuecomment-1050477840
<img width="906" alt="image" src="https://user-images.githubusercontent.com/5441976/155646596-ffc43fb5-3470-43ef-8016-e5cd9af36a0b.png">
I can't see you mocked the attachment in `prepare` stage. Only `testProviderWithAttachment` method has this.
This is easy to reproduce locally.
```
java.lang.NullPointerException
at org.apache.skywalking.apm.plugin.asf.dubbo3.DubboInterceptor.beforeMethod(DubboInterceptor.java:99)
at org.apache.skywalking.apm.plugin.dubbo3.DubboInterceptorTest.testConsumerWithException(DubboInterceptorTest.java:156)
at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62)
at java.base/jdk.internal.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
at java.base/java.lang.reflect.Method.invoke(Method.java:566)
at org.junit.runners.model.FrameworkMethod$1.runReflectiveCall(FrameworkMethod.java:50)
at org.junit.internal.runners.model.ReflectiveCallable.run(ReflectiveCallable.java:12)
at org.junit.runners.model.FrameworkMethod.invokeExplosively(FrameworkMethod.java:47)
at org.junit.internal.runners.statements.InvokeMethod.evaluate(InvokeMethod.java:17)
at org.junit.internal.runners.statements.RunBefores.evaluate(RunBefores.java:26)
at org.apache.skywalking.apm.agent.test.tools.TracingSegmentRunner$1.evaluate(TracingSegmentRunner.java:85)
at org.junit.rules.ExternalResource$1.evaluate(ExternalResource.java:48)
at org.junit.rules.RunRules.evaluate(RunRules.java:20)
at org.junit.runners.ParentRunner.runLeaf(ParentRunner.java:325)
at org.junit.runners.BlockJUnit4ClassRunner.runChild(BlockJUnit4ClassRunner.java:78)
at org.junit.runners.BlockJUnit4ClassRunner.runChild(BlockJUnit4ClassRunner.java:57)
at org.junit.runners.ParentRunner$3.run(ParentRunner.java:290)
at org.junit.runners.ParentRunner$1.schedule(ParentRunner.java:71)
at org.junit.runners.ParentRunner.runChildren(ParentRunner.java:288)
at org.junit.runners.ParentRunner.access$000(ParentRunner.java:58)
at org.junit.runners.ParentRunner$2.evaluate(ParentRunner.java:268)
at org.junit.runners.ParentRunner.run(ParentRunner.java:363)
at org.powermock.modules.junit4.internal.impl.DelegatingPowerMockRunner$2.call(DelegatingPowerMockRunner.java:149)
at org.powermock.modules.junit4.internal.impl.DelegatingPowerMockRunner$2.call(DelegatingPowerMockRunner.java:141)
at org.powermock.modules.junit4.internal.impl.DelegatingPowerMockRunner.withContextClassLoader(DelegatingPowerMockRunner.java:132)
at org.powermock.modules.junit4.internal.impl.DelegatingPowerMockRunner.run(DelegatingPowerMockRunner.java:141)
```
--
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 #110: fix Dubbo3.x Context problem #8525
Posted by GitBox <gi...@apache.org>.
wu-sheng merged pull request #110:
URL: https://github.com/apache/skywalking-java/pull/110
--
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 change in pull request #110: fix Dubbo3.x Context problem #8525
Posted by GitBox <gi...@apache.org>.
wu-sheng commented on a change in pull request #110:
URL: https://github.com/apache/skywalking-java/pull/110#discussion_r813730612
##########
File path: apm-sniffer/apm-sdk-plugin/dubbo-3.x-plugin/src/main/java/org/apache/skywalking/apm/plugin/asf/dubbo3/DubboInterceptor.java
##########
@@ -151,6 +137,19 @@ private void dealException(Throwable throwable) {
span.log(throwable);
}
+ /**
+ * to judge if current is in provider side.
+ *
+ * @param rpcInvocation
+ * @return
+ */
+ private static boolean isProvider(RpcInvocation rpcInvocation) {
+ Invoker<?> invoker = rpcInvocation.getInvoker();
+ return invoker.getUrl()
+ .getParameter("side", "consumer")
+ .equals("provider");
Review comment:
Yes, we just need to declare the parent class only.
--
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] honganan commented on a change in pull request #110: fix Dubbo3.x Context problem #8525
Posted by GitBox <gi...@apache.org>.
honganan commented on a change in pull request #110:
URL: https://github.com/apache/skywalking-java/pull/110#discussion_r813739041
##########
File path: apm-sniffer/apm-sdk-plugin/dubbo-3.x-plugin/src/main/java/org/apache/skywalking/apm/plugin/asf/dubbo3/DubboInterceptor.java
##########
@@ -151,6 +137,19 @@ private void dealException(Throwable throwable) {
span.log(throwable);
}
+ /**
+ * to judge if current is in provider side.
+ *
+ * @param rpcInvocation
+ * @return
+ */
+ private static boolean isProvider(RpcInvocation rpcInvocation) {
+ Invoker<?> invoker = rpcInvocation.getInvoker();
+ return invoker.getUrl()
+ .getParameter("side", "consumer")
+ .equals("provider");
Review comment:
https://github.com/apache/skywalking-java/pull/73#issuecomment-987712730
I reduced 2 `Instrumentation` to 1 and testing it.
--
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] honganan commented on a change in pull request #110: fix Dubbo3.x Context problem #8525
Posted by GitBox <gi...@apache.org>.
honganan commented on a change in pull request #110:
URL: https://github.com/apache/skywalking-java/pull/110#discussion_r813895690
##########
File path: apm-sniffer/apm-sdk-plugin/dubbo-3.x-plugin/src/main/java/org/apache/skywalking/apm/plugin/asf/dubbo3/DubboInterceptor.java
##########
@@ -65,15 +63,7 @@ public void beforeMethod(EnhancedInstance objInst, Method method, Object[] allAr
Invoker invoker = (Invoker) allArguments[0];
Invocation invocation = (Invocation) allArguments[1];
- RpcServiceContext serviceContext = RpcContext.getServiceContext();
- boolean isConsumer;
- URL url = serviceContext.getUrl();
- if (url == null) {
- url = serviceContext.getConsumerUrl();
- isConsumer = url != null;
- } else {
- isConsumer = serviceContext.isConsumerSide();
- }
+ boolean isConsumer = !isProvider(invocation);
Review comment:
Agree
--
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 change in pull request #110: fix Dubbo3.x Context problem #8525
Posted by GitBox <gi...@apache.org>.
wu-sheng commented on a change in pull request #110:
URL: https://github.com/apache/skywalking-java/pull/110#discussion_r812539305
##########
File path: apm-sniffer/apm-sdk-plugin/dubbo-2.7.x-plugin/src/main/java/org/apache/skywalking/apm/plugin/asf/dubbo/DubboInterceptor.java
##########
@@ -110,14 +109,6 @@ public void beforeMethod(EnhancedInstance objInst, Method method, Object[] allAr
public Object afterMethod(EnhancedInstance objInst, Method method, Object[] allArguments, Class<?>[] argumentsTypes,
Object ret) throws Throwable {
Result result = (Result) ret;
- try {
- if (result != null && result.getException() != null) {
- dealException(result.getException());
- }
Review comment:
Do we need to process `result#getExcetpion`?
--
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] honganan commented on pull request #110: fix Dubbo3.x Context problem #8525
Posted by GitBox <gi...@apache.org>.
honganan commented on pull request #110:
URL: https://github.com/apache/skywalking-java/pull/110#issuecomment-1049956627
I will figure this out tomorrow morning.
------------------ Original ------------------
From: ε΄ζ Wu Sheng ***@***.***>
Date: Thu,Feb 24,2022 10:53 PM
To: apache/skywalking-java ***@***.***>
Cc: ζ΄ͺιΏε ***@***.***>, Mention ***@***.***>
Subject: Re: [apache/skywalking-java] fix Dubbo3.x Context problem #8525 (PR#110)
Error: testConsumerWithException(org.apache.skywalking.apm.plugin.dubbo3.DubboInterceptorTest) Time elapsed: 2.085 s <<< ERROR! java.lang.NullPointerException at org.apache.skywalking.apm.plugin.dubbo3.DubboInterceptorTest.testConsumerWithException(DubboInterceptorTest.java:156) Error: testConsumerWithResultHasException(org.apache.skywalking.apm.plugin.dubbo3.DubboInterceptorTest) Time elapsed: 0.013 s <<< ERROR! java.lang.NullPointerException at org.apache.skywalking.apm.plugin.dubbo3.DubboInterceptorTest.testConsumerWithResultHasException(DubboInterceptorTest.java:169) Error: testConsumerWithAttachment(org.apache.skywalking.apm.plugin.dubbo3.DubboInterceptorTest) Time elapsed: 0.008 s <<< ERROR! java.lang.NullPointerException at org.apache.skywalking.apm.plugin.dubbo3.DubboInterceptorTest.testConsumerWithAttachment(DubboInterceptorTest.java:144)
@honganan I think you missed some UTs.
β
Reply to this email directly, view it on GitHub, or unsubscribe.
Triage notifications on the go with GitHub Mobile for iOS or Android.
You are receiving this because you were mentioned.Message ID: ***@***.***>
--
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] honganan commented on pull request #110: fix Dubbo3.x Context problem #8525
Posted by GitBox <gi...@apache.org>.
honganan commented on pull request #110:
URL: https://github.com/apache/skywalking-java/pull/110#issuecomment-1050483424
> <img alt="image" width="906" src="https://user-images.githubusercontent.com/5441976/155646596-ffc43fb5-3470-43ef-8016-e5cd9af36a0b.png">
>
> I can't see you mocked the attachment in `prepare` stage. Only `testProviderWithAttachment` method has this.
>
> This is easy to reproduce locally.
>
> ```
> java.lang.NullPointerException
> at org.apache.skywalking.apm.plugin.asf.dubbo3.DubboInterceptor.beforeMethod(DubboInterceptor.java:99)
> at org.apache.skywalking.apm.plugin.dubbo3.DubboInterceptorTest.testConsumerWithException(DubboInterceptorTest.java:156)
> at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
> at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62)
> at java.base/jdk.internal.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
> at java.base/java.lang.reflect.Method.invoke(Method.java:566)
> at org.junit.runners.model.FrameworkMethod$1.runReflectiveCall(FrameworkMethod.java:50)
> at org.junit.internal.runners.model.ReflectiveCallable.run(ReflectiveCallable.java:12)
> at org.junit.runners.model.FrameworkMethod.invokeExplosively(FrameworkMethod.java:47)
> at org.junit.internal.runners.statements.InvokeMethod.evaluate(InvokeMethod.java:17)
> at org.junit.internal.runners.statements.RunBefores.evaluate(RunBefores.java:26)
> at org.apache.skywalking.apm.agent.test.tools.TracingSegmentRunner$1.evaluate(TracingSegmentRunner.java:85)
> at org.junit.rules.ExternalResource$1.evaluate(ExternalResource.java:48)
> at org.junit.rules.RunRules.evaluate(RunRules.java:20)
> at org.junit.runners.ParentRunner.runLeaf(ParentRunner.java:325)
> at org.junit.runners.BlockJUnit4ClassRunner.runChild(BlockJUnit4ClassRunner.java:78)
> at org.junit.runners.BlockJUnit4ClassRunner.runChild(BlockJUnit4ClassRunner.java:57)
> at org.junit.runners.ParentRunner$3.run(ParentRunner.java:290)
> at org.junit.runners.ParentRunner$1.schedule(ParentRunner.java:71)
> at org.junit.runners.ParentRunner.runChildren(ParentRunner.java:288)
> at org.junit.runners.ParentRunner.access$000(ParentRunner.java:58)
> at org.junit.runners.ParentRunner$2.evaluate(ParentRunner.java:268)
> at org.junit.runners.ParentRunner.run(ParentRunner.java:363)
> at org.powermock.modules.junit4.internal.impl.DelegatingPowerMockRunner$2.call(DelegatingPowerMockRunner.java:149)
> at org.powermock.modules.junit4.internal.impl.DelegatingPowerMockRunner$2.call(DelegatingPowerMockRunner.java:141)
> at org.powermock.modules.junit4.internal.impl.DelegatingPowerMockRunner.withContextClassLoader(DelegatingPowerMockRunner.java:132)
> at org.powermock.modules.junit4.internal.impl.DelegatingPowerMockRunner.run(DelegatingPowerMockRunner.java:141)
> ```
I found the problem, as changed `isProvider()` name and the default value last night and did not update code today.
I will add 2 mock URL separately for Consumer and Provider.
--
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] honganan commented on a change in pull request #110: fix Dubbo3.x Context problem #8525
Posted by GitBox <gi...@apache.org>.
honganan commented on a change in pull request #110:
URL: https://github.com/apache/skywalking-java/pull/110#discussion_r813504146
##########
File path: apm-sniffer/apm-sdk-plugin/dubbo-3.x-plugin/src/main/java/org/apache/skywalking/apm/plugin/asf/dubbo3/DubboInterceptor.java
##########
@@ -151,6 +137,19 @@ private void dealException(Throwable throwable) {
span.log(throwable);
}
+ /**
+ * to judge if current is in provider side.
+ *
+ * @param rpcInvocation
+ * @return
+ */
+ private static boolean isProvider(RpcInvocation rpcInvocation) {
+ Invoker<?> invoker = rpcInvocation.getInvoker();
+ return invoker.getUrl()
+ .getParameter("side", "consumer")
+ .equals("provider");
Review comment:
Seems no need to `try/catch`, just deal the `result.getException` looks 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] wu-sheng commented on a change in pull request #110: fix Dubbo3.x Context problem #8525
Posted by GitBox <gi...@apache.org>.
wu-sheng commented on a change in pull request #110:
URL: https://github.com/apache/skywalking-java/pull/110#discussion_r813731043
##########
File path: apm-sniffer/apm-sdk-plugin/dubbo-3.x-plugin/src/main/java/org/apache/skywalking/apm/plugin/asf/dubbo3/DubboInterceptor.java
##########
@@ -151,6 +137,19 @@ private void dealException(Throwable throwable) {
span.log(throwable);
}
+ /**
+ * to judge if current is in provider side.
+ *
+ * @param rpcInvocation
+ * @return
+ */
+ private static boolean isProvider(RpcInvocation rpcInvocation) {
+ Invoker<?> invoker = rpcInvocation.getInvoker();
+ return invoker.getUrl()
+ .getParameter("side", "consumer")
+ .equals("provider");
Review comment:
But I can't see the link from your comments, but the conclusion is certain.
--
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 #110: fix Dubbo3.x Context problem #8525
Posted by GitBox <gi...@apache.org>.
wu-sheng commented on pull request #110:
URL: https://github.com/apache/skywalking-java/pull/110#issuecomment-1050467921
I think this mock preparation is not enough, should be enhanced with the URL you expect.
--
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 change in pull request #110: fix Dubbo3.x Context problem #8525
Posted by GitBox <gi...@apache.org>.
wu-sheng commented on a change in pull request #110:
URL: https://github.com/apache/skywalking-java/pull/110#discussion_r813756362
##########
File path: apm-sniffer/apm-sdk-plugin/dubbo-3.x-plugin/src/main/java/org/apache/skywalking/apm/plugin/asf/dubbo3/DubboInterceptor.java
##########
@@ -151,6 +137,19 @@ private void dealException(Throwable throwable) {
span.log(throwable);
}
+ /**
+ * to judge if current is in provider side.
+ *
+ * @param rpcInvocation
+ * @return
+ */
+ private static boolean isProvider(RpcInvocation rpcInvocation) {
+ Invoker<?> invoker = rpcInvocation.getInvoker();
+ return invoker.getUrl()
+ .getParameter("side", "consumer")
+ .equals("provider");
Review comment:
Got it. Let's see.
--
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 change in pull request #110: fix Dubbo3.x Context problem #8525
Posted by GitBox <gi...@apache.org>.
wu-sheng commented on a change in pull request #110:
URL: https://github.com/apache/skywalking-java/pull/110#discussion_r812697608
##########
File path: apm-sniffer/apm-sdk-plugin/dubbo-2.7.x-plugin/src/main/java/org/apache/skywalking/apm/plugin/asf/dubbo/DubboInterceptor.java
##########
@@ -110,14 +109,6 @@ public void beforeMethod(EnhancedInstance objInst, Method method, Object[] allAr
public Object afterMethod(EnhancedInstance objInst, Method method, Object[] allArguments, Class<?>[] argumentsTypes,
Object ret) throws Throwable {
Result result = (Result) ret;
- try {
- if (result != null && result.getException() != null) {
- dealException(result.getException());
- }
Review comment:
Usually, we don't need to catch exceptions, as the `handleMethodException` method exists already. But we should process `result#getExcetpion`, because this exception may throw after the whole intercepting is finished or even wouldn't, depending on Dubbo's implementation.
--
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] honganan removed a comment on pull request #110: fix Dubbo3.x Context problem #8525
Posted by GitBox <gi...@apache.org>.
honganan removed a comment on pull request #110:
URL: https://github.com/apache/skywalking-java/pull/110#issuecomment-1048623250
OK
εθͺζηiPhone
------------------ Original ------------------
From: ε΄ζ Wu Sheng ***@***.***>
Date: Wed,Feb 23,2022 5:32 PM
To: apache/skywalking-java ***@***.***>
Cc: ζ΄ͺιΏε ***@***.***>, Mention ***@***.***>
Subject: Re: [apache/skywalking-java] fix Dubbo3.x Context problem #8525 (PR#110)
@wu-sheng commented on this pull request.
In apm-sniffer/apm-sdk-plugin/dubbo-3.x-plugin/src/main/java/org/apache/skywalking/apm/plugin/asf/dubbo3/DubboInterceptor.java:
> + return invoker.getUrl() + .getParameter("side", "consumer") + .equals("provider");
Oh, I see. Could you add this as comments on the codes? This is important to keep this in mind why we change now.
β
Reply to this email directly, view it on GitHub, or unsubscribe.
Triage notifications on the go with GitHub Mobile for iOS or Android.
You are receiving this because you were mentioned.Message ID: ***@***.***>
--
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] honganan commented on a change in pull request #110: fix Dubbo3.x Context problem #8525
Posted by GitBox <gi...@apache.org>.
honganan commented on a change in pull request #110:
URL: https://github.com/apache/skywalking-java/pull/110#discussion_r812699033
##########
File path: apm-sniffer/apm-sdk-plugin/dubbo-3.x-plugin/src/main/java/org/apache/skywalking/apm/plugin/asf/dubbo3/DubboInterceptor.java
##########
@@ -151,6 +137,19 @@ private void dealException(Throwable throwable) {
span.log(throwable);
}
+ /**
+ * to judge if current is in provider side.
+ *
+ * @param rpcInvocation
+ * @return
+ */
+ private static boolean isProvider(RpcInvocation rpcInvocation) {
+ Invoker<?> invoker = rpcInvocation.getInvoker();
+ return invoker.getUrl()
+ .getParameter("side", "consumer")
+ .equals("provider");
Review comment:
The problem is `RpcServiceContext ` not been reset when it's role switched, but `RpcInvocation` is always correctly bounded to the current request or serve request
--
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 change in pull request #110: fix Dubbo3.x Context problem #8525
Posted by GitBox <gi...@apache.org>.
wu-sheng commented on a change in pull request #110:
URL: https://github.com/apache/skywalking-java/pull/110#discussion_r812701922
##########
File path: apm-sniffer/apm-sdk-plugin/dubbo-3.x-plugin/src/main/java/org/apache/skywalking/apm/plugin/asf/dubbo3/DubboInterceptor.java
##########
@@ -151,6 +137,19 @@ private void dealException(Throwable throwable) {
span.log(throwable);
}
+ /**
+ * to judge if current is in provider side.
+ *
+ * @param rpcInvocation
+ * @return
+ */
+ private static boolean isProvider(RpcInvocation rpcInvocation) {
+ Invoker<?> invoker = rpcInvocation.getInvoker();
+ return invoker.getUrl()
+ .getParameter("side", "consumer")
+ .equals("provider");
Review comment:
Oh, I see. Could you add this as comments on the codes? This is important to keep this in mind why we change 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.
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 #110: fix Dubbo3.x Context problem #8525
Posted by GitBox <gi...@apache.org>.
wu-sheng commented on pull request #110:
URL: https://github.com/apache/skywalking-java/pull/110#issuecomment-1049939514
```
Error: testConsumerWithException(org.apache.skywalking.apm.plugin.dubbo3.DubboInterceptorTest) Time elapsed: 2.085 s <<< ERROR!
java.lang.NullPointerException
at org.apache.skywalking.apm.plugin.dubbo3.DubboInterceptorTest.testConsumerWithException(DubboInterceptorTest.java:156)
Error: testConsumerWithResultHasException(org.apache.skywalking.apm.plugin.dubbo3.DubboInterceptorTest) Time elapsed: 0.013 s <<< ERROR!
java.lang.NullPointerException
at org.apache.skywalking.apm.plugin.dubbo3.DubboInterceptorTest.testConsumerWithResultHasException(DubboInterceptorTest.java:169)
Error: testConsumerWithAttachment(org.apache.skywalking.apm.plugin.dubbo3.DubboInterceptorTest) Time elapsed: 0.008 s <<< ERROR!
java.lang.NullPointerException
at org.apache.skywalking.apm.plugin.dubbo3.DubboInterceptorTest.testConsumerWithAttachment(DubboInterceptorTest.java:144)
```
@honganan I think you missed some UTs.
--
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] AlbumenJ commented on a change in pull request #110: fix Dubbo3.x Context problem #8525
Posted by GitBox <gi...@apache.org>.
AlbumenJ commented on a change in pull request #110:
URL: https://github.com/apache/skywalking-java/pull/110#discussion_r820351611
##########
File path: apm-sniffer/apm-sdk-plugin/dubbo-3.x-plugin/src/main/java/org/apache/skywalking/apm/plugin/asf/dubbo3/DubboInterceptor.java
##########
@@ -65,15 +63,7 @@ public void beforeMethod(EnhancedInstance objInst, Method method, Object[] allAr
Invoker invoker = (Invoker) allArguments[0];
Invocation invocation = (Invocation) allArguments[1];
- RpcServiceContext serviceContext = RpcContext.getServiceContext();
- boolean isConsumer;
- URL url = serviceContext.getUrl();
- if (url == null) {
- url = serviceContext.getConsumerUrl();
- isConsumer = url != null;
- } else {
- isConsumer = serviceContext.isConsumerSide();
- }
+ boolean isConsumer = isConsumer(invocation);
Review comment:
@wu-sheng I'm a little confused about this. Why not judge whether it is the consumer side directly from `EnhancedInstance objInst`. If objInst is a MonitorFilter object, it should be on the provider side. If objInst is a MonitorClusterFilter object, it should be on the provider side.
--
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 change in pull request #110: fix Dubbo3.x Context problem #8525
Posted by GitBox <gi...@apache.org>.
wu-sheng commented on a change in pull request #110:
URL: https://github.com/apache/skywalking-java/pull/110#discussion_r804346150
##########
File path: apm-sniffer/apm-sdk-plugin/dubbo-3.x-plugin/src/main/java/org/apache/skywalking/apm/plugin/asf/dubbo3/DubboInterceptor.java
##########
@@ -151,6 +137,19 @@ private void dealException(Throwable throwable) {
span.log(throwable);
}
+ /**
+ * to judge if current is in provider side.
+ *
+ * @param rpcInvocation
+ * @return
+ */
+ private static boolean isProvider(RpcInvocation rpcInvocation) {
+ Invoker<?> invoker = rpcInvocation.getInvoker();
+ return invoker.getUrl()
+ .getParameter("side", "consumer")
+ .equals("provider");
Review comment:
Is this an official way? It seems strange. I am waiting for Dubbo team to response.
--
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] honganan commented on a change in pull request #110: fix Dubbo3.x Context problem #8525
Posted by GitBox <gi...@apache.org>.
honganan commented on a change in pull request #110:
URL: https://github.com/apache/skywalking-java/pull/110#discussion_r804369957
##########
File path: apm-sniffer/apm-sdk-plugin/dubbo-3.x-plugin/src/main/java/org/apache/skywalking/apm/plugin/asf/dubbo3/DubboInterceptor.java
##########
@@ -151,6 +137,19 @@ private void dealException(Throwable throwable) {
span.log(throwable);
}
+ /**
+ * to judge if current is in provider side.
+ *
+ * @param rpcInvocation
+ * @return
+ */
+ private static boolean isProvider(RpcInvocation rpcInvocation) {
+ Invoker<?> invoker = rpcInvocation.getInvoker();
+ return invoker.getUrl()
+ .getParameter("side", "consumer")
+ .equals("provider");
Review comment:
We can wait for the Dubbo team...
I write this from my understanding to the scope of `Rpc*Context` and `RpcInvocation`.
The `RpcServiceContext#isConsumerSide()` implements like:
```java
public boolean isConsumerSide() {
return this.getUrl().getSide("provider").equals("consumer");
}
```
--
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