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 2021/11/23 06:51:58 UTC
[GitHub] [skywalking-java] tedli opened a new pull request #73: add plugin for dubbo3
tedli opened a new pull request #73:
URL: https://github.com/apache/skywalking-java/pull/73
<!--
⚠️ 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 👆 ==== -->
### Add an agent plugin to support [dubbo3](https://github.com/apache/dubbo)
- [x] 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) `There is already one (for dubbo 2.5 and 2.7)`
- [ ] Add a logo in [the UI repo](https://github.com/apache/skywalking-rocketbot-ui/tree/master/src/views/components/topology/assets) `There is already one (for dubbo 2.5 and 2.7)`
<!-- ==== 📈 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 👆 ==== -->
- [x] If this pull request closes/resolves/fixes an existing issue, replace the issue number. Closes #<issue number>. [skywalking 7203](https://github.com/apache/skywalking/issues/7203)
- [x] 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 pull request #73: add plugin for dubbo3
Posted by GitBox <gi...@apache.org>.
wu-sheng commented on pull request #73:
URL: https://github.com/apache/skywalking-java/pull/73#issuecomment-989580953
> And I didn't find something like startWith, contains, endWith in the skywalking-agent-test-tool, can I simply use not null?
Yes, that is fine.
--
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 #73: add plugin for dubbo3
Posted by GitBox <gi...@apache.org>.
wu-sheng commented on a change in pull request #73:
URL: https://github.com/apache/skywalking-java/pull/73#discussion_r754842455
##########
File path: apm-sniffer/apm-sdk-plugin/dubbo-3-plugin/src/main/java/org/apache/skywalking/apm/plugin/asf/dubbo3/DubboInstrumentation.java
##########
@@ -0,0 +1,68 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ *
+ */
+
+package org.apache.skywalking.apm.plugin.asf.dubbo3;
+
+import net.bytebuddy.description.method.MethodDescription;
+import net.bytebuddy.matcher.ElementMatcher;
+import org.apache.skywalking.apm.agent.core.plugin.interceptor.ConstructorInterceptPoint;
+import org.apache.skywalking.apm.agent.core.plugin.interceptor.InstanceMethodsInterceptPoint;
+import org.apache.skywalking.apm.agent.core.plugin.interceptor.enhance.ClassInstanceMethodsEnhancePluginDefine;
+import org.apache.skywalking.apm.agent.core.plugin.match.ClassMatch;
+import org.apache.skywalking.apm.agent.core.plugin.match.NameMatch;
+
+import static net.bytebuddy.matcher.ElementMatchers.named;
+
+public class DubboInstrumentation extends ClassInstanceMethodsEnhancePluginDefine {
+
+ private static final String ENHANCE_CLASS = "org.apache.dubbo.monitor.support.MonitorFilter";
Review comment:
And this is targeting the same class of Dubbo 2. What do I miss here? I thought Dubbo v3 is different from Dubbo v2?
--
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] tedli commented on a change in pull request #73: add plugin for dubbo3
Posted by GitBox <gi...@apache.org>.
tedli commented on a change in pull request #73:
URL: https://github.com/apache/skywalking-java/pull/73#discussion_r754853333
##########
File path: apm-sniffer/apm-sdk-plugin/dubbo-3-conflict-patch/src/main/java/org/apache/skywalking/apm/plugin/asf/dubbo3/patch/WrapperInstrumentation.java
##########
@@ -0,0 +1,64 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.skywalking.apm.plugin.asf.dubbo3.patch;
+
+import net.bytebuddy.description.method.MethodDescription;
+import net.bytebuddy.matcher.ElementMatcher;
+import org.apache.skywalking.apm.agent.core.plugin.interceptor.StaticMethodsInterceptPoint;
+import org.apache.skywalking.apm.agent.core.plugin.interceptor.enhance.ClassStaticMethodsEnhancePluginDefine;
+import org.apache.skywalking.apm.agent.core.plugin.match.ClassMatch;
+
+import static net.bytebuddy.matcher.ElementMatchers.named;
+import static org.apache.skywalking.apm.agent.core.plugin.match.NameMatch.byName;
+
+/**
+ * The dubbo conflict plugin resolver the problem about the wrapper class generated by Dubbo core cannot be compiled
+ * successfully. As we known, The wrapper class traverses all the methods. In usual, it works unless this class has been
+ * enhanced by Skywalking. The javasist cannot found the `EnhanceInstance` method when generate.
+ * <p>
+ * The plugin excludes {@link org.apache.skywalking.apm.agent.core.plugin.interceptor.enhance.EnhancedInstance} methods
+ * to ensure the correct compilation of the code.
+ */
+public class WrapperInstrumentation extends ClassStaticMethodsEnhancePluginDefine {
+ @Override
+ public StaticMethodsInterceptPoint[] getStaticMethodsInterceptPoints() {
+ return new StaticMethodsInterceptPoint[] {
+ new StaticMethodsInterceptPoint() {
+ @Override
+ public ElementMatcher<MethodDescription> getMethodsMatcher() {
+ return named("makeWrapper");
+ }
+
+ @Override
+ public String getMethodsInterceptor() {
+ return "org.apache.skywalking.apm.plugin.asf.dubbo3.patch.MakeWrapperInterceptor";
+ }
+
+ @Override
+ public boolean isOverrideArgs() {
+ return false;
+ }
+ }
+ };
+ }
+
+ @Override
+ protected ClassMatch enhanceClass() {
+ return byName("org.apache.dubbo.common.bytecode.Wrapper");
Review comment:
Yes, indeed there is no difference at namespace level between v2 and v3. Actually dubbo 3 even gave an announcement that no need to touch a single line of code just update version from 2.x to 3.x in dependency of pom.xml, and the migration to v3 is done. FYI: https://dubbo.apache.org/zh/docs/migration/migration-and-compatibility-guide/
--
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 #73: add plugin for dubbo3
Posted by GitBox <gi...@apache.org>.
wu-sheng commented on a change in pull request #73:
URL: https://github.com/apache/skywalking-java/pull/73#discussion_r755861070
##########
File path: apm-sniffer/apm-sdk-plugin/dubbo-commons/pom.xml
##########
@@ -0,0 +1,41 @@
+<?xml version="1.0" encoding="UTF-8"?>
+<!--
+ ~ Licensed to the Apache Software Foundation (ASF) under one or more
+ ~ contributor license agreements. See the NOTICE file distributed with
+ ~ this work for additional information regarding copyright ownership.
+ ~ The ASF licenses this file to You under the Apache License, Version 2.0
+ ~ (the "License"); you may not use this file except in compliance with
+ ~ the License. You may obtain a copy of the License at
+ ~
+ ~ http://www.apache.org/licenses/LICENSE-2.0
+ ~
+ ~ Unless required by applicable law or agreed to in writing, software
+ ~ distributed under the License is distributed on an "AS IS" BASIS,
+ ~ WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ ~ See the License for the specific language governing permissions and
+ ~ limitations under the License.
+ -->
+
+<project xmlns="http://maven.apache.org/POM/4.0.0" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" xsi:schemaLocation="http://maven.apache.org/POM/4.0.0 http://maven.apache.org/xsd/maven-4.0.0.xsd">
+ <parent>
+ <artifactId>apm-sdk-plugin</artifactId>
+ <groupId>org.apache.skywalking</groupId>
+ <version>8.9.0-SNAPSHOT</version>
+ </parent>
+ <modelVersion>4.0.0</modelVersion>
+
+ <artifactId>apm-dubbo-commons</artifactId>
+
+ <properties>
+ <project.build.sourceEncoding>UTF-8</project.build.sourceEncoding>
+ </properties>
+
+ <build>
+ <plugins>
+ <plugin>
+ <artifactId>maven-deploy-plugin</artifactId>
+ </plugin>
+ </plugins>
+ </build>
Review comment:
What is this about?
--
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] tedli commented on pull request #73: add plugin for dubbo3
Posted by GitBox <gi...@apache.org>.
tedli commented on pull request #73:
URL: https://github.com/apache/skywalking-java/pull/73#issuecomment-987712730
Hi, @wu-sheng
FYI
https://github.com/apache/dubbo/blob/00ca43643d243f0866389abd294c6160e155c8a5/dubbo-monitor/dubbo-monitor-api/src/main/java/org/apache/dubbo/monitor/support/MonitorFilter.java#L54-L55
https://github.com/apache/dubbo/blob/5a5e3f3fd7d0482b6124bd5b52b10f4900856438/dubbo-monitor/dubbo-monitor-api/src/main/java/org/apache/dubbo/monitor/support/MonitorFilter.java#L52-L53
```java
// dubbo 2.7.x
@Activate(group = {PROVIDER, CONSUMER})
public class MonitorFilter implements Filter, Filter.Listener {
}
// dubbo 3.x
@Activate(group = {PROVIDER})
public class MonitorFilter implements Filter, Filter.Listener {
}
```
After some debugging, I found out why the scenario test of dubbo3 failed, it's because the occasion that `MonitorFilter` takes place is different in dubbo 3.x from 2.7.x, only provider side produced spans. So the test failed.
It need to find other filters to adjust instrumentation class intercepting behavior, so that consumer side can also produce spans.
--
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 #73: add plugin for dubbo3
Posted by GitBox <gi...@apache.org>.
wu-sheng commented on a change in pull request #73:
URL: https://github.com/apache/skywalking-java/pull/73#discussion_r754841572
##########
File path: apm-sniffer/apm-sdk-plugin/dubbo-3-conflict-patch/src/main/java/org/apache/skywalking/apm/plugin/asf/dubbo3/patch/WrapperInstrumentation.java
##########
@@ -0,0 +1,64 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.skywalking.apm.plugin.asf.dubbo3.patch;
+
+import net.bytebuddy.description.method.MethodDescription;
+import net.bytebuddy.matcher.ElementMatcher;
+import org.apache.skywalking.apm.agent.core.plugin.interceptor.StaticMethodsInterceptPoint;
+import org.apache.skywalking.apm.agent.core.plugin.interceptor.enhance.ClassStaticMethodsEnhancePluginDefine;
+import org.apache.skywalking.apm.agent.core.plugin.match.ClassMatch;
+
+import static net.bytebuddy.matcher.ElementMatchers.named;
+import static org.apache.skywalking.apm.agent.core.plugin.match.NameMatch.byName;
+
+/**
+ * The dubbo conflict plugin resolver the problem about the wrapper class generated by Dubbo core cannot be compiled
+ * successfully. As we known, The wrapper class traverses all the methods. In usual, it works unless this class has been
+ * enhanced by Skywalking. The javasist cannot found the `EnhanceInstance` method when generate.
+ * <p>
+ * The plugin excludes {@link org.apache.skywalking.apm.agent.core.plugin.interceptor.enhance.EnhancedInstance} methods
+ * to ensure the correct compilation of the code.
+ */
+public class WrapperInstrumentation extends ClassStaticMethodsEnhancePluginDefine {
+ @Override
+ public StaticMethodsInterceptPoint[] getStaticMethodsInterceptPoints() {
+ return new StaticMethodsInterceptPoint[] {
+ new StaticMethodsInterceptPoint() {
+ @Override
+ public ElementMatcher<MethodDescription> getMethodsMatcher() {
+ return named("makeWrapper");
+ }
+
+ @Override
+ public String getMethodsInterceptor() {
+ return "org.apache.skywalking.apm.plugin.asf.dubbo3.patch.MakeWrapperInterceptor";
+ }
+
+ @Override
+ public boolean isOverrideArgs() {
+ return false;
+ }
+ }
+ };
+ }
+
+ @Override
+ protected ClassMatch enhanceClass() {
+ return byName("org.apache.dubbo.common.bytecode.Wrapper");
Review comment:
I can see this is as same as Dubbo v2's `WrapperInstrumentation`. Is this duplicated **conflict patch**?
I can't see `dubbo` `v2->v3` from namespace level in `MakeWrapperInterceptor`, and there is no witness class in this instrumentation. Should we just simple change `dubbo-2.7.x-conflict-patch` to `dubbo-2.7.x-3.x-conflict-patch`
--
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 #73: add plugin for dubbo3
Posted by GitBox <gi...@apache.org>.
wu-sheng commented on a change in pull request #73:
URL: https://github.com/apache/skywalking-java/pull/73#discussion_r754881736
##########
File path: apm-sniffer/apm-sdk-plugin/dubbo-3-plugin/src/main/java/org/apache/skywalking/apm/plugin/asf/dubbo3/DubboInstrumentation.java
##########
@@ -0,0 +1,68 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ *
+ */
+
+package org.apache.skywalking.apm.plugin.asf.dubbo3;
+
+import net.bytebuddy.description.method.MethodDescription;
+import net.bytebuddy.matcher.ElementMatcher;
+import org.apache.skywalking.apm.agent.core.plugin.interceptor.ConstructorInterceptPoint;
+import org.apache.skywalking.apm.agent.core.plugin.interceptor.InstanceMethodsInterceptPoint;
+import org.apache.skywalking.apm.agent.core.plugin.interceptor.enhance.ClassInstanceMethodsEnhancePluginDefine;
+import org.apache.skywalking.apm.agent.core.plugin.match.ClassMatch;
+import org.apache.skywalking.apm.agent.core.plugin.match.NameMatch;
+
+import static net.bytebuddy.matcher.ElementMatchers.named;
+
+public class DubboInstrumentation extends ClassInstanceMethodsEnhancePluginDefine {
+
+ private static final String ENHANCE_CLASS = "org.apache.dubbo.monitor.support.MonitorFilter";
Review comment:
OK then, follow https://github.com/apache/skywalking-java/pull/73#discussion_r754864763, You should be good.
--
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] tedli commented on pull request #73: add plugin for dubbo3
Posted by GitBox <gi...@apache.org>.
tedli commented on pull request #73:
URL: https://github.com/apache/skywalking-java/pull/73#issuecomment-977711679
> > witness classes and methods added, and extract common code of both dubbo 2.7 and 3 into individual maven module.
>
> The dubbo plugin is not very complex like JDBC, do we really need the common package and set up this complex hierarchy structure? Dubbo v2 will dead(EOL) one day, keeping them separate will make us easier to remove Dubbo 2 plugin and keep 3 plugin w/o change.
Ok, rollback.
**TODO:**
- [ ] add witness to tell is dubbo v2.7 or v3
- [ ] a new plugin test(not UT)
--
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 #73: add plugin for dubbo3
Posted by GitBox <gi...@apache.org>.
wu-sheng commented on pull request #73:
URL: https://github.com/apache/skywalking-java/pull/73#issuecomment-989588589
You need to add your case into GHA control file, then I could activate CI for you.
--
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 #73: add plugin for dubbo3
Posted by GitBox <gi...@apache.org>.
wu-sheng commented on pull request #73:
URL: https://github.com/apache/skywalking-java/pull/73#issuecomment-982246561
We had a contributed contributing this tool to help you locate the witness, https://github.com/SkyAPM/uranus.
BTW, it doesn't have to be a witness class, a method signature is also 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] tedli commented on pull request #73: add plugin for dubbo3
Posted by GitBox <gi...@apache.org>.
tedli commented on pull request #73:
URL: https://github.com/apache/skywalking-java/pull/73#issuecomment-978897387
> [2021-11-25 07:08:58:225] [ERROR] - org.apache.skywalking.plugin.test.agent.tool.Main.verify(Main.java:53) -
assert failed.
SegmentSizeNotEqualsException: dubbo-2.7.x-scenario
expected: ge 3
actual: 2
Current state of `main` branch, I can't get plugin test of dubbo 2.7.x pass.
```bash
# by using this command
./test/plugin/run.sh -f dubbo-2.7.x-scenario
```
--
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] tedli commented on pull request #73: add plugin for dubbo3
Posted by GitBox <gi...@apache.org>.
tedli commented on pull request #73:
URL: https://github.com/apache/skywalking-java/pull/73#issuecomment-977703837
> If not, read this, https://skywalking.apache.org/docs/skywalking-java/latest/en/setup/service-agent/java-agent/java-plugin-development-guide/#implement-plugin, search this 4. Set up witnessClasses and/or witnessMethods if the instrumentation has to be activated in specific versions.
witness classes and methods added, and extract common code of both dubbo 2.7 and 3 into individual maven module.
> A new plugin test(not UT) is required like this, https://github.com/apache/skywalking-java/tree/main/test/plugin/scenarios/dubbo-2.7.x-scenario.
This is on the road...
--
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] tedli edited a comment on pull request #73: add plugin for dubbo3
Posted by GitBox <gi...@apache.org>.
tedli edited a comment on pull request #73:
URL: https://github.com/apache/skywalking-java/pull/73#issuecomment-982244736
I got the point why the scenario test failed. It's because the chosen witness classes fit one minor (patch) version but not for other minor (patch) versions.
The dubbo core classes are stable among various major version like `org.apache.dubbo.rpc.RpcContext`, however it can't be used to tell it's dubbo 2.7.x or 3.
Meanwhile other classes like `org.apache.dubbo.rpc.protocol.rest.NettyServer`, exists in 2.7.x not in 3. But in some newer patch version of 2.7.x, it also not exists.
So it takes more time to find classes which could be used to determine it's 2.7.x or 3.x, and such classes intuitively will stay stable among all the 'x' minor or patch versions.
--
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 #73: add plugin for dubbo3
Posted by GitBox <gi...@apache.org>.
wu-sheng commented on a change in pull request #73:
URL: https://github.com/apache/skywalking-java/pull/73#discussion_r765744713
##########
File path: apm-sniffer/apm-sdk-plugin/dubbo-3.x-plugin/src/main/resources/skywalking-plugin.def
##########
@@ -0,0 +1,18 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements. See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership. The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+
+dubbo=org.apache.skywalking.apm.plugin.asf.dubbo3.DubboProviderInstrumentation
+dubbo=org.apache.skywalking.apm.plugin.asf.dubbo3.DubboConsumerInstrumentation
Review comment:
```
Run tools/plugin/check-javaagent-plugin-list.sh
--- /Users/runner/work/skywalking-java/skywalking-java/tools/plugin/md-javaagent-plugin-list.txt 2021-12-09 10:55:36.000000000 +0000
+++ /Users/runner/work/skywalking-java/skywalking-java/tools/plugin/genernate-javaagent-plugin-list.txt 2021-12-09 10:55:36.000000000 +0000
@@ -16 +15,0 @@
-dubbo-2.7.x/3.x
```
Doesn't match 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] tedli edited a comment on pull request #73: add plugin for dubbo3
Posted by GitBox <gi...@apache.org>.
tedli edited a comment on pull request #73:
URL: https://github.com/apache/skywalking-java/pull/73#issuecomment-978897387
> [2021-11-25 07:08:58:225] [ERROR] - org.apache.skywalking.plugin.test.agent.tool.Main.verify(Main.java:53) -
assert failed.
SegmentSizeNotEqualsException: dubbo-2.7.x-scenario
expected: ge 3
actual: 2
Current state of `main` branch (without this pr), I can't get plugin test of dubbo 2.7.x pass.
```bash
# by using this command
./test/plugin/run.sh -f dubbo-2.7.x-scenario
```
--
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 #73: add plugin for dubbo3
Posted by GitBox <gi...@apache.org>.
wu-sheng commented on pull request #73:
URL: https://github.com/apache/skywalking-java/pull/73#issuecomment-1007015400
In this month, maybe. We are waiting for a grpc next release.
--
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 #73: add plugin for dubbo3
Posted by GitBox <gi...@apache.org>.
wu-sheng commented on pull request #73:
URL: https://github.com/apache/skywalking-java/pull/73#issuecomment-976211233
A new plugin test(not UT) is required like this, https://github.com/apache/skywalking-java/tree/main/test/plugin/scenarios/dubbo-2.7.x-scenario.
We have plugin development doc to guide you how to write a plugin 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 pull request #73: add plugin for dubbo3
Posted by GitBox <gi...@apache.org>.
wu-sheng commented on pull request #73:
URL: https://github.com/apache/skywalking-java/pull/73#issuecomment-977706439
The dubbo plugin is not very complex like JDBC, do we really need the common package and set up this complex hierarchy structure? Dubbo v2 will dead(EOL) one day, keeping them separate will make us easier to remove Dubbo 2 plugin and keep 3 plugin w/o change.
--
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 #73: add plugin for dubbo3
Posted by GitBox <gi...@apache.org>.
wu-sheng commented on pull request #73:
URL: https://github.com/apache/skywalking-java/pull/73#issuecomment-979147286
Have you passed maven install and both plugin cases locally?
--
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 #73: add plugin for dubbo3
Posted by GitBox <gi...@apache.org>.
wu-sheng commented on a change in pull request #73:
URL: https://github.com/apache/skywalking-java/pull/73#discussion_r765745484
##########
File path: test/plugin/scenarios/dubbo-3.x-scenario/pom.xml
##########
@@ -0,0 +1,125 @@
+<?xml version="1.0" encoding="UTF-8"?>
+<!--
+ ~ Licensed to the Apache Software Foundation (ASF) under one or more
+ ~ contributor license agreements. See the NOTICE file distributed with
+ ~ this work for additional information regarding copyright ownership.
+ ~ The ASF licenses this file to You under the Apache License, Version 2.0
+ ~ (the "License"); you may not use this file except in compliance with
+ ~ the License. You may obtain a copy of the License at
+ ~
+ ~ http://www.apache.org/licenses/LICENSE-2.0
+ ~
+ ~ Unless required by applicable law or agreed to in writing, software
+ ~ distributed under the License is distributed on an "AS IS" BASIS,
+ ~ WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ ~ See the License for the specific language governing permissions and
+ ~ limitations under the License.
+ ~
+ -->
+<project xmlns="http://maven.apache.org/POM/4.0.0"
+ xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
+ xsi:schemaLocation="http://maven.apache.org/POM/4.0.0 http://maven.apache.org/xsd/maven-4.0.0.xsd">
+ <modelVersion>4.0.0</modelVersion>
+
+ <groupId>org.apache.skywalking</groupId>
+ <artifactId>dubbo-3.x-scenario</artifactId>
+ <version>5.0.0</version>
+
+ <properties>
+ <project.build.sourceEncoding>UTF-8</project.build.sourceEncoding>
+ <compiler.version>1.8</compiler.version>
+ <spring.boot.version>2.1.6.RELEASE</spring.boot.version>
+
+ <test.framework>dubbo</test.framework>
+ <test.framework.version>3.0.0</test.framework.version>
+ </properties>
+
+ <name>skywalking-dubbo-3.x-scenario</name>
+
+ <dependencyManagement>
+ <dependencies>
+ <dependency>
+ <groupId>org.springframework.boot</groupId>
+ <artifactId>spring-boot-dependencies</artifactId>
+ <version>${spring.boot.version}</version>
+ <type>pom</type>
+ <scope>import</scope>
+ </dependency>
+ </dependencies>
+ </dependencyManagement>
+
+ <dependencies>
+ <dependency>
+ <groupId>org.springframework.boot</groupId>
+ <artifactId>spring-boot-starter-web</artifactId>
+ </dependency>
+
+ <!-- dubbo -->
+ <dependency>
+ <groupId>org.apache.dubbo</groupId>
+ <artifactId>dubbo</artifactId>
+ <version>${test.framework.version}</version>
+ </dependency>
+ <dependency>
+ <groupId>org.apache.dubbo</groupId>
+ <artifactId>dubbo-dependencies-zookeeper</artifactId>
+ <version>${test.framework.version}</version>
+ <type>pom</type>
+ </dependency>
+ </dependencies>
+
+ <build>
+ <finalName>dubbo-3.x-scenario</finalName>
+ <plugins>
+ <plugin>
+ <groupId>org.springframework.boot</groupId>
+ <artifactId>spring-boot-maven-plugin</artifactId>
+ <executions>
+ <execution>
+ <goals>
+ <goal>repackage</goal>
+ </goals>
+ </execution>
+ </executions>
+ </plugin>
+ <plugin>
+ <artifactId>maven-compiler-plugin</artifactId>
+ <configuration>
+ <source>${compiler.version}</source>
+ <target>${compiler.version}</target>
+ <encoding>${project.build.sourceEncoding}</encoding>
+ </configuration>
+ </plugin>
+ <plugin>
+ <groupId>org.apache.maven.plugins</groupId>
+ <artifactId>maven-assembly-plugin</artifactId>
+ <executions>
+ <execution>
+ <id>assemble</id>
+ <phase>package</phase>
+ <goals>
+ <goal>single</goal>
+ </goals>
+ <configuration>
+ <descriptors>
+ <descriptor>src/main/assembly/assembly.xml</descriptor>
+ </descriptors>
+ <outputDirectory>./target/</outputDirectory>
+ </configuration>
+ </execution>
+ </executions>
+ </plugin>
+ </plugins>
+ </build>
+
+ <pluginRepositories>
+ <pluginRepository>
+ <id>spring-snapshots</id>
+ <url>https://repo.spring.io/snapshot</url>
+ </pluginRepository>
+ <pluginRepository>
+ <id>spring-milestones</id>
+ <url>https://repo.spring.io/milestone</url>
+ </pluginRepository>
+ </pluginRepositories>
Review comment:
Do we really need this? It is better to use official Maven central, as we don't test snapshot versions.
--
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 #73: add plugin for dubbo3
Posted by GitBox <gi...@apache.org>.
wu-sheng commented on pull request #73:
URL: https://github.com/apache/skywalking-java/pull/73#issuecomment-989623230
`Supported-list.md`, `Plugin-list.md` and `changes.md` should be updated, otherwise, the CI would fail.
--
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] tedli commented on pull request #73: add plugin for dubbo3
Posted by GitBox <gi...@apache.org>.
tedli commented on pull request #73:
URL: https://github.com/apache/skywalking-java/pull/73#issuecomment-987718566
> Is any `CONSUMER` filter available? How monitoring works for `CONSUMER` side Dubbo3?
Yes, there is.
https://github.com/apache/dubbo/blob/dubbo-3.0.4/dubbo-monitor/dubbo-monitor-api/src/main/java/org/apache/dubbo/monitor/support/MonitorClusterFilter.java
I notice that some other plugins has different instrumentation classes for server and client like the grpc plugin.
I'm trying to add an instrumentation class for dubbo 3 consumer in the same 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 pull request #73: add plugin for dubbo3
Posted by GitBox <gi...@apache.org>.
wu-sheng commented on pull request #73:
URL: https://github.com/apache/skywalking-java/pull/73#issuecomment-987716026
Is any `CONSUMER` filter available? How monitoring works for `CONSUMER` side Dubbo3?
--
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] tedli commented on a change in pull request #73: add plugin for dubbo3
Posted by GitBox <gi...@apache.org>.
tedli commented on a change in pull request #73:
URL: https://github.com/apache/skywalking-java/pull/73#discussion_r754864869
##########
File path: apm-sniffer/apm-sdk-plugin/dubbo-3-plugin/src/main/java/org/apache/skywalking/apm/plugin/asf/dubbo3/DubboInstrumentation.java
##########
@@ -0,0 +1,68 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ *
+ */
+
+package org.apache.skywalking.apm.plugin.asf.dubbo3;
+
+import net.bytebuddy.description.method.MethodDescription;
+import net.bytebuddy.matcher.ElementMatcher;
+import org.apache.skywalking.apm.agent.core.plugin.interceptor.ConstructorInterceptPoint;
+import org.apache.skywalking.apm.agent.core.plugin.interceptor.InstanceMethodsInterceptPoint;
+import org.apache.skywalking.apm.agent.core.plugin.interceptor.enhance.ClassInstanceMethodsEnhancePluginDefine;
+import org.apache.skywalking.apm.agent.core.plugin.match.ClassMatch;
+import org.apache.skywalking.apm.agent.core.plugin.match.NameMatch;
+
+import static net.bytebuddy.matcher.ElementMatchers.named;
+
+public class DubboInstrumentation extends ClassInstanceMethodsEnhancePluginDefine {
+
+ private static final String ENHANCE_CLASS = "org.apache.dubbo.monitor.support.MonitorFilter";
Review comment:
Differences: (not all, ones currently I know)
1. RpcContext.getContext() didn't work;
2. attachment divided into `ClientAttachment` and ServerAttachment;
3. dubbo constants definition class divided into multiple classes (like `org.apache.dubbo.common.constants.CommonConstants`);
4. methods added in `org.apache.dubbo.common.URL`
--
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 #73: add plugin for dubbo3
Posted by GitBox <gi...@apache.org>.
wu-sheng commented on pull request #73:
URL: https://github.com/apache/skywalking-java/pull/73#issuecomment-981251262
Thanks for updating. If there is any help we could provide, please let us know.
--
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] tedli commented on pull request #73: add plugin for dubbo3
Posted by GitBox <gi...@apache.org>.
tedli commented on pull request #73:
URL: https://github.com/apache/skywalking-java/pull/73#issuecomment-979592477
> Have you passed maven install and both plugin cases locally?
Yes, it passed (without this pr). With this pr I can pass other scenarios except dubbo2.7 and 3. So it turns out I broke something.
--
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 #73: add plugin for dubbo3
Posted by GitBox <gi...@apache.org>.
wu-sheng commented on pull request #73:
URL: https://github.com/apache/skywalking-java/pull/73#issuecomment-979593196
I think it is better you reset anything back to main branch, then add your new plugin implementation with witness class.
--
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 #73: add plugin for dubbo3
Posted by GitBox <gi...@apache.org>.
wu-sheng commented on pull request #73:
URL: https://github.com/apache/skywalking-java/pull/73#issuecomment-986591580
@tedli Any luck with this 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] tedli commented on pull request #73: add plugin for dubbo3
Posted by GitBox <gi...@apache.org>.
tedli commented on pull request #73:
URL: https://github.com/apache/skywalking-java/pull/73#issuecomment-989712295
> `Supported-list.md`, `Plugin-list.md` and `changes.md` should be updated, otherwise, the CI would fail.
I saw that the checking for `Plugin-list.md` reads `skywalking-plugin.def`, I don't know how this file work. If I add new entry for dubbo3, should I adjust content of `skywalking-plugin.def` to something like this?
```
dubbo3.x=org.apache.skywalking.apm.plugin.asf.dubbo3.DubboProviderInstrumentation
dubbo3.x=org.apache.skywalking.apm.plugin.asf.dubbo3.DubboConsumerInstrumentation
```
By the way, I saw `retransform-class-scenario` failed in previous CI, did I broke 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] tedli edited a comment on pull request #73: add plugin for dubbo3
Posted by GitBox <gi...@apache.org>.
tedli edited a comment on pull request #73:
URL: https://github.com/apache/skywalking-java/pull/73#issuecomment-977711679
--
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] tedli commented on pull request #73: add plugin for dubbo3
Posted by GitBox <gi...@apache.org>.
tedli commented on pull request #73:
URL: https://github.com/apache/skywalking-java/pull/73#issuecomment-989579748
Hi @wu-sheng ,
I found that if using the same scenario test code like dubbo 2.7.x, the consumer proxy object will use an InJvmInvoker, or if explicitly set inJvm to false, a DubboInvoker will be used, neither of them loads filters.
Then I ran a [EmbeddedZooKeeper](https://github.com/apache/dubbo-samples/blob/master/dubbo-samples-echo/src/main/java/org/apache/dubbo/samples/echo/EmbeddedZooKeeper.java), by using a registry,
https://github.com/apache/dubbo/blob/5a5e3f3fd7d0482b6124bd5b52b10f4900856438/dubbo-cluster/src/main/java/org/apache/dubbo/rpc/cluster/filter/ProtocolFilterWrapper.java#L68-L75
```java
@Override
public <T> Invoker<T> refer(Class<T> type, URL url) throws RpcException {
if (UrlUtils.isRegistry(url)) {
return protocol.refer(type, url);
}
FilterChainBuilder builder = getFilterChainBuilder(url);
return builder.buildInvokerChain(protocol.refer(type, url), REFERENCE_FILTER_KEY, CommonConstants.CONSUMER);
}
```
the consumer proxy object loads filters. So skywalking interceptor on consumer side affects. The produced spans are as expect.
```yaml
segmentItems:
- serviceName: dubbo-3.x-scenario
segmentSize: 3
segments:
- segmentId: 42afe68bf5ad435787439867ed63a554.64.16390270335870000
spans:
- operationName: HEAD:/dubbo-3.x-scenario/case/healthCheck
operationId: 0
parentSpanId: -1
spanId: 0
spanLayer: Http
startTime: 1639027033589
endTime: 1639027033684
componentId: 1
isError: false
spanType: Entry
peer: ''
skipAnalysis: false
tags:
- {key: url, value: 'http://localhost:8080/dubbo-3.x-scenario/case/healthCheck'}
- {key: http.method, value: HEAD}
- segmentId: 42afe68bf5ad435787439867ed63a554.86.16390270340630000
spans:
- operationName: org.apache.skywalking.apm.testcase.dubbo3.services.GreetService.doBusiness(String)
operationId: 0
parentSpanId: -1
spanId: 0
spanLayer: RPCFramework
startTime: 1639027034063
endTime: 1639027034068
componentId: 3
isError: false
spanType: Entry
peer: 172.17.0.2:40328
skipAnalysis: false
tags:
- {key: url, value: 'dubbo://172.17.0.2:20080/org.apache.skywalking.apm.testcase.dubbo3.services.GreetService.doBusiness(String)'}
- {key: arguments, value: helloWorld}
refs:
- {parentEndpoint: 'GET:/dubbo-3.x-scenario/case/dubbo', networkAddress: '172.17.0.2:0',
refType: CrossProcess, parentSpanId: 1, parentTraceSegmentId: 42afe68bf5ad435787439867ed63a554.65.16390270336920000,
parentServiceInstance: ae9af82e06fe419097b7483b8e2d6522@172.17.0.2, parentService: dubbo-3.x-scenario,
traceId: 42afe68bf5ad435787439867ed63a554.65.16390270336920001}
- segmentId: 42afe68bf5ad435787439867ed63a554.65.16390270336920000
spans:
- operationName: org.apache.skywalking.apm.testcase.dubbo3.services.GreetService.doBusiness(String)
operationId: 0
parentSpanId: 0
spanId: 1
spanLayer: RPCFramework
startTime: 1639027033946
endTime: 1639027034073
componentId: 3
isError: false
spanType: Exit
peer: 172.17.0.2:0
skipAnalysis: false
tags:
- {key: url, value: 'dubbo://172.17.0.2:0/org.apache.skywalking.apm.testcase.dubbo3.services.GreetService.doBusiness(String)'}
- {key: arguments, value: helloWorld}
- operationName: GET:/dubbo-3.x-scenario/case/dubbo
operationId: 0
parentSpanId: -1
spanId: 0
spanLayer: Http
startTime: 1639027033692
endTime: 1639027034075
componentId: 1
isError: false
spanType: Entry
peer: ''
skipAnalysis: false
tags:
- {key: url, value: 'http://localhost:8080/dubbo-3.x-scenario/case/dubbo'}
- {key: http.method, value: GET}
```
However, because a registry is used, the `address` no longer be the `localhost` and depends. So the `expectedData.yaml` can't keep using `localhost`.
And I didn't find something like `startWith`, `contains`, `endWith` in the skywalking-agent-test-tool, can I simply use `not 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 pull request #73: add plugin for dubbo3
Posted by GitBox <gi...@apache.org>.
wu-sheng commented on pull request #73:
URL: https://github.com/apache/skywalking-java/pull/73#issuecomment-987720977
Sure, you could disable the whole `MonitorFilter`-based instrumentation for Dubbo3, and pick a better one.
--
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 #73: add plugin for dubbo3
Posted by GitBox <gi...@apache.org>.
wu-sheng commented on a change in pull request #73:
URL: https://github.com/apache/skywalking-java/pull/73#discussion_r754860528
##########
File path: apm-sniffer/apm-sdk-plugin/dubbo-3-conflict-patch/src/main/java/org/apache/skywalking/apm/plugin/asf/dubbo3/patch/WrapperInstrumentation.java
##########
@@ -0,0 +1,64 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.skywalking.apm.plugin.asf.dubbo3.patch;
+
+import net.bytebuddy.description.method.MethodDescription;
+import net.bytebuddy.matcher.ElementMatcher;
+import org.apache.skywalking.apm.agent.core.plugin.interceptor.StaticMethodsInterceptPoint;
+import org.apache.skywalking.apm.agent.core.plugin.interceptor.enhance.ClassStaticMethodsEnhancePluginDefine;
+import org.apache.skywalking.apm.agent.core.plugin.match.ClassMatch;
+
+import static net.bytebuddy.matcher.ElementMatchers.named;
+import static org.apache.skywalking.apm.agent.core.plugin.match.NameMatch.byName;
+
+/**
+ * The dubbo conflict plugin resolver the problem about the wrapper class generated by Dubbo core cannot be compiled
+ * successfully. As we known, The wrapper class traverses all the methods. In usual, it works unless this class has been
+ * enhanced by Skywalking. The javasist cannot found the `EnhanceInstance` method when generate.
+ * <p>
+ * The plugin excludes {@link org.apache.skywalking.apm.agent.core.plugin.interceptor.enhance.EnhancedInstance} methods
+ * to ensure the correct compilation of the code.
+ */
+public class WrapperInstrumentation extends ClassStaticMethodsEnhancePluginDefine {
+ @Override
+ public StaticMethodsInterceptPoint[] getStaticMethodsInterceptPoints() {
+ return new StaticMethodsInterceptPoint[] {
+ new StaticMethodsInterceptPoint() {
+ @Override
+ public ElementMatcher<MethodDescription> getMethodsMatcher() {
+ return named("makeWrapper");
+ }
+
+ @Override
+ public String getMethodsInterceptor() {
+ return "org.apache.skywalking.apm.plugin.asf.dubbo3.patch.MakeWrapperInterceptor";
+ }
+
+ @Override
+ public boolean isOverrideArgs() {
+ return false;
+ }
+ }
+ };
+ }
+
+ @Override
+ protected ClassMatch enhanceClass() {
+ return byName("org.apache.dubbo.common.bytecode.Wrapper");
Review comment:
Then, could you share why dubbo 2.x plugin doesn't work on dubbo 3.x?
--
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 #73: add plugin for dubbo3
Posted by GitBox <gi...@apache.org>.
wu-sheng commented on a change in pull request #73:
URL: https://github.com/apache/skywalking-java/pull/73#discussion_r754864763
##########
File path: apm-sniffer/apm-sdk-plugin/dubbo-3-conflict-patch/src/main/java/org/apache/skywalking/apm/plugin/asf/dubbo3/patch/WrapperInstrumentation.java
##########
@@ -0,0 +1,64 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.skywalking.apm.plugin.asf.dubbo3.patch;
+
+import net.bytebuddy.description.method.MethodDescription;
+import net.bytebuddy.matcher.ElementMatcher;
+import org.apache.skywalking.apm.agent.core.plugin.interceptor.StaticMethodsInterceptPoint;
+import org.apache.skywalking.apm.agent.core.plugin.interceptor.enhance.ClassStaticMethodsEnhancePluginDefine;
+import org.apache.skywalking.apm.agent.core.plugin.match.ClassMatch;
+
+import static net.bytebuddy.matcher.ElementMatchers.named;
+import static org.apache.skywalking.apm.agent.core.plugin.match.NameMatch.byName;
+
+/**
+ * The dubbo conflict plugin resolver the problem about the wrapper class generated by Dubbo core cannot be compiled
+ * successfully. As we known, The wrapper class traverses all the methods. In usual, it works unless this class has been
+ * enhanced by Skywalking. The javasist cannot found the `EnhanceInstance` method when generate.
+ * <p>
+ * The plugin excludes {@link org.apache.skywalking.apm.agent.core.plugin.interceptor.enhance.EnhancedInstance} methods
+ * to ensure the correct compilation of the code.
+ */
+public class WrapperInstrumentation extends ClassStaticMethodsEnhancePluginDefine {
+ @Override
+ public StaticMethodsInterceptPoint[] getStaticMethodsInterceptPoints() {
+ return new StaticMethodsInterceptPoint[] {
+ new StaticMethodsInterceptPoint() {
+ @Override
+ public ElementMatcher<MethodDescription> getMethodsMatcher() {
+ return named("makeWrapper");
+ }
+
+ @Override
+ public String getMethodsInterceptor() {
+ return "org.apache.skywalking.apm.plugin.asf.dubbo3.patch.MakeWrapperInterceptor";
+ }
+
+ @Override
+ public boolean isOverrideArgs() {
+ return false;
+ }
+ }
+ };
+ }
+
+ @Override
+ protected ClassMatch enhanceClass() {
+ return byName("org.apache.dubbo.common.bytecode.Wrapper");
Review comment:
OK, then. In theory(you could test and verify), if this patch classes(instrumentation + interceptor) are exactly 100% same as v2, then this is not required.
> for classes and methods expose to user side (rpc level) is compatible with v2, but the internal dosen't.
For example we can't rely on RpcContext.getContext() with v3.
This would make the plugin for Dubbo 3 is required, and you should add `witness class` to dubbo 3 and dubbo 2.7 plugins, because they share the same target, you should make the agent core knows when should activate this instrumentation, and pick the right interceptor in the runtime.
Do you knoow what is the `witness class`? If not, read this, https://skywalking.apache.org/docs/skywalking-java/latest/en/setup/service-agent/java-agent/java-plugin-development-guide/#implement-plugin, search this `4. Set up witnessClasses and/or witnessMethods if the instrumentation has to be activated in specific versions.`
--
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] tedli commented on a change in pull request #73: add plugin for dubbo3
Posted by GitBox <gi...@apache.org>.
tedli commented on a change in pull request #73:
URL: https://github.com/apache/skywalking-java/pull/73#discussion_r754853306
##########
File path: apm-sniffer/apm-sdk-plugin/dubbo-3-plugin/src/main/java/org/apache/skywalking/apm/plugin/asf/dubbo3/DubboInstrumentation.java
##########
@@ -0,0 +1,68 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ *
+ */
+
+package org.apache.skywalking.apm.plugin.asf.dubbo3;
+
+import net.bytebuddy.description.method.MethodDescription;
+import net.bytebuddy.matcher.ElementMatcher;
+import org.apache.skywalking.apm.agent.core.plugin.interceptor.ConstructorInterceptPoint;
+import org.apache.skywalking.apm.agent.core.plugin.interceptor.InstanceMethodsInterceptPoint;
+import org.apache.skywalking.apm.agent.core.plugin.interceptor.enhance.ClassInstanceMethodsEnhancePluginDefine;
+import org.apache.skywalking.apm.agent.core.plugin.match.ClassMatch;
+import org.apache.skywalking.apm.agent.core.plugin.match.NameMatch;
+
+import static net.bytebuddy.matcher.ElementMatchers.named;
+
+public class DubboInstrumentation extends ClassInstanceMethodsEnhancePluginDefine {
+
+ private static final String ENHANCE_CLASS = "org.apache.dubbo.monitor.support.MonitorFilter";
Review comment:
I'm new to skywalking, I didn't dig for a deeper understanding of skywalking. When I finished current state of this pr, only tested using my dubbo3 demo, and it works.
As you pointed out, I believe it won't work as expect when using dubbo 2.7.
Converted to draft.
##########
File path: apm-sniffer/apm-sdk-plugin/dubbo-3-conflict-patch/src/main/java/org/apache/skywalking/apm/plugin/asf/dubbo3/patch/WrapperInstrumentation.java
##########
@@ -0,0 +1,64 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.skywalking.apm.plugin.asf.dubbo3.patch;
+
+import net.bytebuddy.description.method.MethodDescription;
+import net.bytebuddy.matcher.ElementMatcher;
+import org.apache.skywalking.apm.agent.core.plugin.interceptor.StaticMethodsInterceptPoint;
+import org.apache.skywalking.apm.agent.core.plugin.interceptor.enhance.ClassStaticMethodsEnhancePluginDefine;
+import org.apache.skywalking.apm.agent.core.plugin.match.ClassMatch;
+
+import static net.bytebuddy.matcher.ElementMatchers.named;
+import static org.apache.skywalking.apm.agent.core.plugin.match.NameMatch.byName;
+
+/**
+ * The dubbo conflict plugin resolver the problem about the wrapper class generated by Dubbo core cannot be compiled
+ * successfully. As we known, The wrapper class traverses all the methods. In usual, it works unless this class has been
+ * enhanced by Skywalking. The javasist cannot found the `EnhanceInstance` method when generate.
+ * <p>
+ * The plugin excludes {@link org.apache.skywalking.apm.agent.core.plugin.interceptor.enhance.EnhancedInstance} methods
+ * to ensure the correct compilation of the code.
+ */
+public class WrapperInstrumentation extends ClassStaticMethodsEnhancePluginDefine {
+ @Override
+ public StaticMethodsInterceptPoint[] getStaticMethodsInterceptPoints() {
+ return new StaticMethodsInterceptPoint[] {
+ new StaticMethodsInterceptPoint() {
+ @Override
+ public ElementMatcher<MethodDescription> getMethodsMatcher() {
+ return named("makeWrapper");
+ }
+
+ @Override
+ public String getMethodsInterceptor() {
+ return "org.apache.skywalking.apm.plugin.asf.dubbo3.patch.MakeWrapperInterceptor";
+ }
+
+ @Override
+ public boolean isOverrideArgs() {
+ return false;
+ }
+ }
+ };
+ }
+
+ @Override
+ protected ClassMatch enhanceClass() {
+ return byName("org.apache.dubbo.common.bytecode.Wrapper");
Review comment:
Yes, indeed there is no difference at namespace level between v2 and v3. Actually dubbo 3 even gave an announcement that no need to touch a single line of code just update version from 2.x to 3.x in dependency of pom.xml, and the migration to v3 is done.
--
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] tedli edited a comment on pull request #73: add plugin for dubbo3
Posted by GitBox <gi...@apache.org>.
tedli edited a comment on pull request #73:
URL: https://github.com/apache/skywalking-java/pull/73#issuecomment-978897387
> [2021-11-25 07:08:58:225] [ERROR] - org.apache.skywalking.plugin.test.agent.tool.Main.verify(Main.java:53) -
assert failed.
SegmentSizeNotEqualsException: dubbo-2.7.x-scenario
expected: ge 3
actual: 2
~~Current state of `main` branch (without this pr), I can't get plugin test of dubbo 2.7.x pass.~~
```bash
# by using this command
./test/plugin/run.sh -f dubbo-2.7.x-scenario
```
Turns out that I didn't `make build` first. Test (2.7.x) passed.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
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 #73: add plugin for dubbo3
Posted by GitBox <gi...@apache.org>.
wu-sheng commented on a change in pull request #73:
URL: https://github.com/apache/skywalking-java/pull/73#discussion_r754865302
##########
File path: apm-sniffer/apm-sdk-plugin/dubbo-3-conflict-patch/src/main/java/org/apache/skywalking/apm/plugin/asf/dubbo3/patch/WrapperInstrumentation.java
##########
@@ -0,0 +1,64 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.skywalking.apm.plugin.asf.dubbo3.patch;
+
+import net.bytebuddy.description.method.MethodDescription;
+import net.bytebuddy.matcher.ElementMatcher;
+import org.apache.skywalking.apm.agent.core.plugin.interceptor.StaticMethodsInterceptPoint;
+import org.apache.skywalking.apm.agent.core.plugin.interceptor.enhance.ClassStaticMethodsEnhancePluginDefine;
+import org.apache.skywalking.apm.agent.core.plugin.match.ClassMatch;
+
+import static net.bytebuddy.matcher.ElementMatchers.named;
+import static org.apache.skywalking.apm.agent.core.plugin.match.NameMatch.byName;
+
+/**
+ * The dubbo conflict plugin resolver the problem about the wrapper class generated by Dubbo core cannot be compiled
+ * successfully. As we known, The wrapper class traverses all the methods. In usual, it works unless this class has been
+ * enhanced by Skywalking. The javasist cannot found the `EnhanceInstance` method when generate.
+ * <p>
+ * The plugin excludes {@link org.apache.skywalking.apm.agent.core.plugin.interceptor.enhance.EnhancedInstance} methods
+ * to ensure the correct compilation of the code.
+ */
+public class WrapperInstrumentation extends ClassStaticMethodsEnhancePluginDefine {
+ @Override
+ public StaticMethodsInterceptPoint[] getStaticMethodsInterceptPoints() {
+ return new StaticMethodsInterceptPoint[] {
+ new StaticMethodsInterceptPoint() {
+ @Override
+ public ElementMatcher<MethodDescription> getMethodsMatcher() {
+ return named("makeWrapper");
+ }
+
+ @Override
+ public String getMethodsInterceptor() {
+ return "org.apache.skywalking.apm.plugin.asf.dubbo3.patch.MakeWrapperInterceptor";
+ }
+
+ @Override
+ public boolean isOverrideArgs() {
+ return false;
+ }
+ }
+ };
+ }
+
+ @Override
+ protected ClassMatch enhanceClass() {
+ return byName("org.apache.dubbo.common.bytecode.Wrapper");
Review comment:
Then follow [this doc](https://skywalking.apache.org/docs/skywalking-java/latest/en/setup/service-agent/java-agent/plugin-test/) and take a reference from what I shared for dubbo 2.7 plugin test, you could add a new for dubbo 3.0.
Once both of them pass the test, we should be good.
--
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] tedli commented on a change in pull request #73: add plugin for dubbo3
Posted by GitBox <gi...@apache.org>.
tedli commented on a change in pull request #73:
URL: https://github.com/apache/skywalking-java/pull/73#discussion_r754862702
##########
File path: apm-sniffer/apm-sdk-plugin/dubbo-3-conflict-patch/src/main/java/org/apache/skywalking/apm/plugin/asf/dubbo3/patch/WrapperInstrumentation.java
##########
@@ -0,0 +1,64 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.skywalking.apm.plugin.asf.dubbo3.patch;
+
+import net.bytebuddy.description.method.MethodDescription;
+import net.bytebuddy.matcher.ElementMatcher;
+import org.apache.skywalking.apm.agent.core.plugin.interceptor.StaticMethodsInterceptPoint;
+import org.apache.skywalking.apm.agent.core.plugin.interceptor.enhance.ClassStaticMethodsEnhancePluginDefine;
+import org.apache.skywalking.apm.agent.core.plugin.match.ClassMatch;
+
+import static net.bytebuddy.matcher.ElementMatchers.named;
+import static org.apache.skywalking.apm.agent.core.plugin.match.NameMatch.byName;
+
+/**
+ * The dubbo conflict plugin resolver the problem about the wrapper class generated by Dubbo core cannot be compiled
+ * successfully. As we known, The wrapper class traverses all the methods. In usual, it works unless this class has been
+ * enhanced by Skywalking. The javasist cannot found the `EnhanceInstance` method when generate.
+ * <p>
+ * The plugin excludes {@link org.apache.skywalking.apm.agent.core.plugin.interceptor.enhance.EnhancedInstance} methods
+ * to ensure the correct compilation of the code.
+ */
+public class WrapperInstrumentation extends ClassStaticMethodsEnhancePluginDefine {
+ @Override
+ public StaticMethodsInterceptPoint[] getStaticMethodsInterceptPoints() {
+ return new StaticMethodsInterceptPoint[] {
+ new StaticMethodsInterceptPoint() {
+ @Override
+ public ElementMatcher<MethodDescription> getMethodsMatcher() {
+ return named("makeWrapper");
+ }
+
+ @Override
+ public String getMethodsInterceptor() {
+ return "org.apache.skywalking.apm.plugin.asf.dubbo3.patch.MakeWrapperInterceptor";
+ }
+
+ @Override
+ public boolean isOverrideArgs() {
+ return false;
+ }
+ }
+ };
+ }
+
+ @Override
+ protected ClassMatch enhanceClass() {
+ return byName("org.apache.dubbo.common.bytecode.Wrapper");
Review comment:
It because, for classes and methods expose to user side (rpc level) is compatible with v2, but the internal dosen't.
For example we can't rely on `RpcContext.getContext()` with v3.
--
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] tedli commented on pull request #73: add plugin for dubbo3
Posted by GitBox <gi...@apache.org>.
tedli commented on pull request #73:
URL: https://github.com/apache/skywalking-java/pull/73#issuecomment-986607019
> @tedli Any luck with this plugin?
Hi, the witness methods works fine, by adding witness methods like this:
```java
// dubbo 2.7.x
@Override
protected List<WitnessMethod> witnessMethods() {
return Collections.singletonList(new WitnessMethod(
"org.apache.dubbo.rpc.RpcContext",
named("getServerContext").and(returns(named("org.apache.dubbo.rpc.RpcContext")))));
}
// dubbo 3.x
@Override
protected List<WitnessMethod> witnessMethods() {
return Collections.singletonList(new WitnessMethod(
"org.apache.dubbo.rpc.RpcContext",
named("getServerContext").and(returns(named("org.apache.dubbo.rpc.RpcContextAttachment")))));
}
```
the agent mechanism accurately load the correct plugin. And the scenario test of 2.7.x passed as it is.
How ever the scenario test of 3 still failed, it because `request url` got from `invocation` seemed weird, the produced span didn't match the expect value.
It need some further investigation about how to construct a span for dubbo 3.
:sob: It's the end of the year, I have to handle some of my own mess, which has higher priority. I'm very glad to continue working on this issue. I will make some time for 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] wu-sheng commented on pull request #73: add plugin for dubbo3
Posted by GitBox <gi...@apache.org>.
wu-sheng commented on pull request #73:
URL: https://github.com/apache/skywalking-java/pull/73#issuecomment-989714284
The name of skywalking-plugin.def provides the bootstrap configurable capabilities to disable this plugin directly though config file or system env, rather than removing jar files. So it is better to rename preview to dubbo2.x and add dubbo3.x
--
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] tedli edited a comment on pull request #73: add plugin for dubbo3
Posted by GitBox <gi...@apache.org>.
tedli edited a comment on pull request #73:
URL: https://github.com/apache/skywalking-java/pull/73#issuecomment-982244736
I got the point why the scenario test failed. It's because the chosen witness classes fit one minor (patch) version but not for other minor (patch) versions.
The dubbo core classes are stable among various major version like `org.apache.dubbo.rpc.RpcContext`, however it can't be used to tell it's dubbo 2.7.x or 3.
Meanwhile other classes like `org.apache.dubbo.rpc.protocol.rest.NettyServer`, exists in 2.7.x not in 3. But in some newer patch version of 2.7.x, it also not exists.
So it takes more time to find classes which could be used to determine it's 2.7.x or 3.x, and such classes should be intuitively stay stable among all the 'x' minor or patch versions.
--
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] tedli commented on pull request #73: add plugin for dubbo3
Posted by GitBox <gi...@apache.org>.
tedli commented on pull request #73:
URL: https://github.com/apache/skywalking-java/pull/73#issuecomment-981242765
> I think it is better you reset anything back to main branch, then add your new plugin implementation with witness class.
Yes, I did exactly the same thing. Checked out the main branch, add witness class only to dubbo2.7 and it's conflict patch, do nothing with dubbo3.
Then the 2.7 scenario test broke. However, with these witness class, my 2.7 demo and 3 demo works as expect (the log file in logs folder shows that 2.7 demo dosen't meet 3's witness, and vice versa, and agent can produce spans which appear on skywalking ui as expect).
It needs some more time, I will keep on working on 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] wu-sheng edited a comment on pull request #73: add plugin for dubbo3
Posted by GitBox <gi...@apache.org>.
wu-sheng edited a comment on pull request #73:
URL: https://github.com/apache/skywalking-java/pull/73#issuecomment-977706439
> witness classes and methods added, and extract common code of both dubbo 2.7 and 3 into individual maven module.
The dubbo plugin is not very complex like JDBC, do we really need the common package and set up this complex hierarchy structure? Dubbo v2 will dead(EOL) one day, keeping them separate will make us easier to remove Dubbo 2 plugin and keep 3 plugin w/o change.
--
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] tedli commented on a change in pull request #73: add plugin for dubbo3
Posted by GitBox <gi...@apache.org>.
tedli commented on a change in pull request #73:
URL: https://github.com/apache/skywalking-java/pull/73#discussion_r754870522
##########
File path: apm-sniffer/apm-sdk-plugin/dubbo-3-conflict-patch/src/main/java/org/apache/skywalking/apm/plugin/asf/dubbo3/patch/WrapperInstrumentation.java
##########
@@ -0,0 +1,64 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.skywalking.apm.plugin.asf.dubbo3.patch;
+
+import net.bytebuddy.description.method.MethodDescription;
+import net.bytebuddy.matcher.ElementMatcher;
+import org.apache.skywalking.apm.agent.core.plugin.interceptor.StaticMethodsInterceptPoint;
+import org.apache.skywalking.apm.agent.core.plugin.interceptor.enhance.ClassStaticMethodsEnhancePluginDefine;
+import org.apache.skywalking.apm.agent.core.plugin.match.ClassMatch;
+
+import static net.bytebuddy.matcher.ElementMatchers.named;
+import static org.apache.skywalking.apm.agent.core.plugin.match.NameMatch.byName;
+
+/**
+ * The dubbo conflict plugin resolver the problem about the wrapper class generated by Dubbo core cannot be compiled
+ * successfully. As we known, The wrapper class traverses all the methods. In usual, it works unless this class has been
+ * enhanced by Skywalking. The javasist cannot found the `EnhanceInstance` method when generate.
+ * <p>
+ * The plugin excludes {@link org.apache.skywalking.apm.agent.core.plugin.interceptor.enhance.EnhancedInstance} methods
+ * to ensure the correct compilation of the code.
+ */
+public class WrapperInstrumentation extends ClassStaticMethodsEnhancePluginDefine {
+ @Override
+ public StaticMethodsInterceptPoint[] getStaticMethodsInterceptPoints() {
+ return new StaticMethodsInterceptPoint[] {
+ new StaticMethodsInterceptPoint() {
+ @Override
+ public ElementMatcher<MethodDescription> getMethodsMatcher() {
+ return named("makeWrapper");
+ }
+
+ @Override
+ public String getMethodsInterceptor() {
+ return "org.apache.skywalking.apm.plugin.asf.dubbo3.patch.MakeWrapperInterceptor";
+ }
+
+ @Override
+ public boolean isOverrideArgs() {
+ return false;
+ }
+ }
+ };
+ }
+
+ @Override
+ protected ClassMatch enhanceClass() {
+ return byName("org.apache.dubbo.common.bytecode.Wrapper");
Review comment:
Ok, just need some time, won't be too lang.
--
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 #73: add plugin for dubbo3
Posted by GitBox <gi...@apache.org>.
wu-sheng commented on a change in pull request #73:
URL: https://github.com/apache/skywalking-java/pull/73#discussion_r754860244
##########
File path: apm-sniffer/apm-sdk-plugin/dubbo-3-plugin/src/main/java/org/apache/skywalking/apm/plugin/asf/dubbo3/DubboInstrumentation.java
##########
@@ -0,0 +1,68 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ *
+ */
+
+package org.apache.skywalking.apm.plugin.asf.dubbo3;
+
+import net.bytebuddy.description.method.MethodDescription;
+import net.bytebuddy.matcher.ElementMatcher;
+import org.apache.skywalking.apm.agent.core.plugin.interceptor.ConstructorInterceptPoint;
+import org.apache.skywalking.apm.agent.core.plugin.interceptor.InstanceMethodsInterceptPoint;
+import org.apache.skywalking.apm.agent.core.plugin.interceptor.enhance.ClassInstanceMethodsEnhancePluginDefine;
+import org.apache.skywalking.apm.agent.core.plugin.match.ClassMatch;
+import org.apache.skywalking.apm.agent.core.plugin.match.NameMatch;
+
+import static net.bytebuddy.matcher.ElementMatchers.named;
+
+public class DubboInstrumentation extends ClassInstanceMethodsEnhancePluginDefine {
+
+ private static final String ENHANCE_CLASS = "org.apache.dubbo.monitor.support.MonitorFilter";
Review comment:
> As you pointed out, I believe it won't work as expect when using dubbo 2.7.
We have a plugin for Dubbo 2.7, my question is what are the differences between these two. They seems very closing to each other, especially targeting same class(name)?
--
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] tedli commented on pull request #73: add plugin for dubbo3
Posted by GitBox <gi...@apache.org>.
tedli commented on pull request #73:
URL: https://github.com/apache/skywalking-java/pull/73#issuecomment-982244736
I got the point why the scenario test failed. It's because the chosen witness classes fit one minor version but not for other minor versions.
The dubbo core classes are stable among various major version like `org.apache.dubbo.rpc.RpcContext`, however it can't be used to tell it's dubbo 2.7.x or 3.
Meanwhile other classes like `org.apache.dubbo.rpc.protocol.rest.NettyServer`, exists in 2.7.x not in 3. But in some newer patch version of 2.7.x, it also not exists.
So it takes more time to find classes which could be used to determine it's 2.7.x or 3.x, and such classes intuitively will stay stable among all the 'x' minor or patch versions.
--
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 #73: add plugin for dubbo3
Posted by GitBox <gi...@apache.org>.
wu-sheng commented on pull request #73:
URL: https://github.com/apache/skywalking-java/pull/73#issuecomment-986608950
Got it. Make your time, no hurry about this. I just want to check the status, and we will plan to release the next 8.9.0 for agent after we have Dubbo plugin. Because we know people at China love Dubbo project, and would require this 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 pull request #73: add plugin for dubbo3
Posted by GitBox <gi...@apache.org>.
wu-sheng commented on pull request #73:
URL: https://github.com/apache/skywalking-java/pull/73#issuecomment-989714900
retransform case should only fail unrelatedly. I guess. I can rerun if it happens again. Don't need to worry for 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] tedli edited a comment on pull request #73: add plugin for dubbo3
Posted by GitBox <gi...@apache.org>.
tedli edited a comment on pull request #73:
URL: https://github.com/apache/skywalking-java/pull/73#issuecomment-986607019
> @tedli Any luck with this plugin?
Hi, the witness methods works fine, by adding witness methods like this:
```java
// dubbo 2.7.x
@Override
protected List<WitnessMethod> witnessMethods() {
return Collections.singletonList(new WitnessMethod(
"org.apache.dubbo.rpc.RpcContext",
named("getServerContext").and(returns(named("org.apache.dubbo.rpc.RpcContext")))));
}
// dubbo 3.x
@Override
protected List<WitnessMethod> witnessMethods() {
return Collections.singletonList(new WitnessMethod(
"org.apache.dubbo.rpc.RpcContext",
named("getServerContext").and(returns(named("org.apache.dubbo.rpc.RpcContextAttachment")))));
}
```
the agent mechanism accurately load the correct plugin. And the scenario test of 2.7.x passed as it is.
How ever the scenario test of 3 still failed, ~~it because `request url` got from `invocation` seemed weird~~, the produced span didn't match the expect value.
It need some further investigation about how to construct a span for dubbo 3.
:sob: It's the end of the year, I have to handle some of my own mess, which has higher priority. I'm very glad to continue working on this issue. I will make some time for 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] Cuihongsen commented on pull request #73: add plugin for dubbo3
Posted by GitBox <gi...@apache.org>.
Cuihongsen commented on pull request #73:
URL: https://github.com/apache/skywalking-java/pull/73#issuecomment-1006806425
When will a new version support this new feature be released
--
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