You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@skywalking.apache.org by "darkness-2nd (via GitHub)" <gi...@apache.org> on 2024/07/21 17:17:00 UTC
[PR] Feature/repairkafka3.7 [skywalking-java]
darkness-2nd opened a new pull request, #707:
URL: https://github.com/apache/skywalking-java/pull/707
<!--
⚠️ 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
Re: [PR] Feature/repairkafka3.7 [skywalking-java]
Posted by "darkness-2nd (via GitHub)" <gi...@apache.org>.
darkness-2nd commented on code in PR #707:
URL: https://github.com/apache/skywalking-java/pull/707#discussion_r1690760806
##########
apm-sniffer/apm-sdk-plugin/kafka-plugin/src/main/java/org/apache/skywalking/apm/plugin/kafka/Kafka37ConsumerInterceptor.java:
##########
@@ -0,0 +1,57 @@
+/*
+ * 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.kafka;
+
+import org.apache.kafka.clients.consumer.ConsumerRecord;
+import org.apache.kafka.clients.consumer.ConsumerRecords;
+import org.apache.kafka.common.TopicPartition;
+
+import java.util.ArrayList;
+import java.util.HashMap;
+import java.util.List;
+import java.util.Map;
+
+/**
+ * kafak3.7 removed the method named pollForFetches with return type Fetch.
+ * When the kafka version < 3.7, the enhance method could be: org.apache.kafka.clients.consumer.KafkaConsumer#pollForFetches.
+ * But when the kafka version >= 3.7, the enhance method should be: org.apache.kafka.clients.consumer.KafkaConsumer#poll(java.time.Duration).
+ * And the return type was also changed, from org.apache.kafka.clients.consumer.internals.Fetch to org.apache.kafka.clients.consumer.ConsumerRecords.
+ */
+public class Kafka37ConsumerInterceptor extends KafkaConsumerInterceptor {
+
+ @Override
+ protected Map<TopicPartition, List<ConsumerRecord<?, ?>>> fetchRecords(Object retObj) {
+ Map<TopicPartition, List<ConsumerRecord<?, ?>>> rsp = new HashMap<>();
+ if (retObj instanceof ConsumerRecords) {
Review Comment:
This class should be removed, I missed it, I'll delete it at next commit
--
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
Re: [PR] Feature/repairkafka3.7 [skywalking-java]
Posted by "darkness-2nd (via GitHub)" <gi...@apache.org>.
darkness-2nd commented on PR #707:
URL: https://github.com/apache/skywalking-java/pull/707#issuecomment-2242402344
> Logically same. But we should test.
OK, Should I create modules kafka-3.6.x-scenario and kafka-3.7.x-scenario like kafka-2.3.x-scenario in dir /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
Re: [PR] Feature/repairkafka3.7 [skywalking-java]
Posted by "lujiajing1126 (via GitHub)" <gi...@apache.org>.
lujiajing1126 commented on code in PR #707:
URL: https://github.com/apache/skywalking-java/pull/707#discussion_r1690850098
##########
apm-sniffer/apm-sdk-plugin/kafka-plugin/src/main/java/org/apache/skywalking/apm/plugin/kafka/define/Kafka37AsyncConsumerInstrumentation.java:
##########
@@ -0,0 +1,42 @@
+/*
+ * 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.kafka.define;
+
+import org.apache.skywalking.apm.agent.core.plugin.match.ClassMatch;
+import static org.apache.skywalking.apm.agent.core.plugin.match.NameMatch.byName;
+
+/**
Review Comment:
> Yes, the difference is the method named pollForFetchs was removed from KafkaConsumer to another two classes, so the original interceptor can not intercept it. Because of the enhance class is changed, so I create two new classes to repair the uncompatible problem.
I meant you can modify the comment for this 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
Re: [PR] Feature/repairkafka3.7 [skywalking-java]
Posted by "wu-sheng (via GitHub)" <gi...@apache.org>.
wu-sheng commented on code in PR #707:
URL: https://github.com/apache/skywalking-java/pull/707#discussion_r1685833702
##########
apm-sniffer/apm-sdk-plugin/kafka-plugin/src/main/java/org/apache/skywalking/apm/plugin/kafka/define/KafkaConsumerInstrumentation.java:
##########
@@ -62,104 +64,121 @@ public class KafkaConsumerInstrumentation extends AbstractKafkaInstrumentation {
@Override
public ConstructorInterceptPoint[] getConstructorsInterceptPoints() {
- return new ConstructorInterceptPoint[] {
- new ConstructorInterceptPoint() {
- @Override
- public ElementMatcher<MethodDescription> getConstructorMatcher() {
- return takesArgumentWithType(0, CONSTRUCTOR_INTERCEPT_TYPE);
- }
+ return new ConstructorInterceptPoint[]{
+ new ConstructorInterceptPoint() {
+ @Override
+ public ElementMatcher<MethodDescription> getConstructorMatcher() {
+ return takesArgumentWithType(0, CONSTRUCTOR_INTERCEPT_TYPE);
+ }
Review Comment:
You should not reformat, thr code style file is in the source codes.
--
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
Re: [PR] Feature/repairkafka3.7 [skywalking-java]
Posted by "wu-sheng (via GitHub)" <gi...@apache.org>.
wu-sheng merged PR #707:
URL: https://github.com/apache/skywalking-java/pull/707
--
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
Re: [PR] Feature/repairkafka3.7 [skywalking-java]
Posted by "darkness-2nd (via GitHub)" <gi...@apache.org>.
darkness-2nd commented on PR #707:
URL: https://github.com/apache/skywalking-java/pull/707#issuecomment-2242326514
> Please pick one for 3.4, 3.5, 3.6 as well.
3.4, 3.5, 3.6 are same to 3.2.3
--
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
Re: [PR] Feature/repairkafka3.7 [skywalking-java]
Posted by "wu-sheng (via GitHub)" <gi...@apache.org>.
wu-sheng commented on PR #707:
URL: https://github.com/apache/skywalking-java/pull/707#issuecomment-2249286147
> --- /home/runner/work/skywalking-java/skywalking-java/tools/plugin/md-javaagent-plugin-list.txt 2024-07-25 03:04:25.186078041 +0000
+++ /home/runner/work/skywalking-java/skywalking-java/tools/plugin/genernate-javaagent-plugin-list.txt 2024-0[7](https://github.com/apache/skywalking-java/actions/runs/10087411570/job/27891606111?pr=707#step:5:8)-25 03:04:25.178078015 +0000
I think you did not update docs accordingly, Plugin-list.md and Supported-list.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
Re: [PR] Feature/repairkafka3.7 [skywalking-java]
Posted by "darkness-2nd (via GitHub)" <gi...@apache.org>.
darkness-2nd commented on code in PR #707:
URL: https://github.com/apache/skywalking-java/pull/707#discussion_r1690854260
##########
apm-sniffer/apm-sdk-plugin/kafka-plugin/src/main/java/org/apache/skywalking/apm/plugin/kafka/define/KafkaConsumerInstrumentation.java:
##########
@@ -63,103 +63,103 @@ public class KafkaConsumerInstrumentation extends AbstractKafkaInstrumentation {
@Override
public ConstructorInterceptPoint[] getConstructorsInterceptPoints() {
return new ConstructorInterceptPoint[] {
- new ConstructorInterceptPoint() {
- @Override
- public ElementMatcher<MethodDescription> getConstructorMatcher() {
- return takesArgumentWithType(0, CONSTRUCTOR_INTERCEPT_TYPE);
- }
+ new ConstructorInterceptPoint() {
+ @Override
+ public ElementMatcher<MethodDescription> getConstructorMatcher() {
+ return takesArgumentWithType(0, CONSTRUCTOR_INTERCEPT_TYPE);
+ }
- @Override
- public String getConstructorInterceptor() {
- return CONSUMER_CONFIG_CONSTRUCTOR_INTERCEPTOR_CLASS;
- }
- },
- new ConstructorInterceptPoint() {
- @Override
- public ElementMatcher<MethodDescription> getConstructorMatcher() {
- return takesArgumentWithType(0, CONSTRUCTOR_INTERCEPT_MAP_TYPE);
- }
-
- @Override
- public String getConstructorInterceptor() {
- return MAP_CONSTRUCTOR_INTERCEPTOR_CLASS;
- }
- },
+ @Override
+ public String getConstructorInterceptor() {
+ return CONSUMER_CONFIG_CONSTRUCTOR_INTERCEPTOR_CLASS;
+ }
+ },
+ new ConstructorInterceptPoint() {
+ @Override
+ public ElementMatcher<MethodDescription> getConstructorMatcher() {
+ return takesArgumentWithType(0, CONSTRUCTOR_INTERCEPT_MAP_TYPE);
+ }
+
+ @Override
+ public String getConstructorInterceptor() {
+ return MAP_CONSTRUCTOR_INTERCEPTOR_CLASS;
+ }
+ },
Review Comment:
> > I have already recerted it, this class dosen't change, I have already rollback it to the original version
>
> No, you didn't. Change is still there.
Those changes are compare to the changes which are not correct I commit before...
--
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
Re: [PR] Feature/repairkafka3.7 [skywalking-java]
Posted by "wu-sheng (via GitHub)" <gi...@apache.org>.
wu-sheng commented on PR #707:
URL: https://github.com/apache/skywalking-java/pull/707#issuecomment-2241725810
Please explain what is changing here, and if this is a plugin update, you need to update test scenarios too.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@skywalking.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Feature/repairkafka3.7 [skywalking-java]
Posted by "wu-sheng (via GitHub)" <gi...@apache.org>.
wu-sheng commented on PR #707:
URL: https://github.com/apache/skywalking-java/pull/707#issuecomment-2242405594
And please fix CIs.
--
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
Re: [PR] Feature/repairkafka3.7 [skywalking-java]
Posted by "wu-sheng (via GitHub)" <gi...@apache.org>.
wu-sheng commented on code in PR #707:
URL: https://github.com/apache/skywalking-java/pull/707#discussion_r1690733039
##########
docs/en/setup/service-agent/java-agent/Bootstrap-plugins.md:
##########
@@ -19,4 +19,3 @@ The plugin of JDK HttpURLConnection depended on `sun.net.*`. When using Java 9+,
For more information
1. [JEP 403: Strongly Encapsulate JDK Internals](https://openjdk.org/jeps/403)
-2. [A peek into Java 17: Encapsulating the Java runtime internals](https://blogs.oracle.com/javamagazine/post/a-peek-into-java-17-continuing-the-drive-to-encapsulate-the-java-runtime-internals)
Review Comment:
You should not remove the docs, just because a CI took says otherwise.
--
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
Re: [PR] Feature/repairkafka3.7 [skywalking-java]
Posted by "wu-sheng (via GitHub)" <gi...@apache.org>.
wu-sheng commented on code in PR #707:
URL: https://github.com/apache/skywalking-java/pull/707#discussion_r1690731077
##########
docs/en/setup/service-agent/java-agent/Bootstrap-plugins.md:
##########
@@ -19,4 +19,3 @@ The plugin of JDK HttpURLConnection depended on `sun.net.*`. When using Java 9+,
For more information
1. [JEP 403: Strongly Encapsulate JDK Internals](https://openjdk.org/jeps/403)
-2. [A peek into Java 17: Encapsulating the Java runtime internals](https://blogs.oracle.com/javamagazine/post/a-peek-into-java-17-continuing-the-drive-to-encapsulate-the-java-runtime-internals)
Review Comment:
I think you made a mistake to remove this line?
--
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
Re: [PR] Feature/repairkafka3.7 [skywalking-java]
Posted by "darkness-2nd (via GitHub)" <gi...@apache.org>.
darkness-2nd commented on code in PR #707:
URL: https://github.com/apache/skywalking-java/pull/707#discussion_r1690851500
##########
apm-sniffer/apm-sdk-plugin/kafka-plugin/src/main/java/org/apache/skywalking/apm/plugin/kafka/define/Kafka37AsyncConsumerInstrumentation.java:
##########
@@ -0,0 +1,42 @@
+/*
+ * 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.kafka.define;
+
+import org.apache.skywalking.apm.agent.core.plugin.match.ClassMatch;
+import static org.apache.skywalking.apm.agent.core.plugin.match.NameMatch.byName;
+
+/**
Review Comment:
You are right, I forget to change the comment
--
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
Re: [PR] Feature/repairkafka3.7 [skywalking-java]
Posted by "darkness-2nd (via GitHub)" <gi...@apache.org>.
darkness-2nd commented on code in PR #707:
URL: https://github.com/apache/skywalking-java/pull/707#discussion_r1690881990
##########
apm-sniffer/apm-sdk-plugin/kafka-plugin/src/main/java/org/apache/skywalking/apm/plugin/kafka/define/KafkaConsumerInstrumentation.java:
##########
@@ -63,103 +63,103 @@ public class KafkaConsumerInstrumentation extends AbstractKafkaInstrumentation {
@Override
public ConstructorInterceptPoint[] getConstructorsInterceptPoints() {
return new ConstructorInterceptPoint[] {
- new ConstructorInterceptPoint() {
- @Override
- public ElementMatcher<MethodDescription> getConstructorMatcher() {
- return takesArgumentWithType(0, CONSTRUCTOR_INTERCEPT_TYPE);
- }
+ new ConstructorInterceptPoint() {
+ @Override
+ public ElementMatcher<MethodDescription> getConstructorMatcher() {
+ return takesArgumentWithType(0, CONSTRUCTOR_INTERCEPT_TYPE);
+ }
- @Override
- public String getConstructorInterceptor() {
- return CONSUMER_CONFIG_CONSTRUCTOR_INTERCEPTOR_CLASS;
- }
- },
- new ConstructorInterceptPoint() {
- @Override
- public ElementMatcher<MethodDescription> getConstructorMatcher() {
- return takesArgumentWithType(0, CONSTRUCTOR_INTERCEPT_MAP_TYPE);
- }
-
- @Override
- public String getConstructorInterceptor() {
- return MAP_CONSTRUCTOR_INTERCEPTOR_CLASS;
- }
- },
+ @Override
+ public String getConstructorInterceptor() {
+ return CONSUMER_CONFIG_CONSTRUCTOR_INTERCEPTOR_CLASS;
+ }
+ },
+ new ConstructorInterceptPoint() {
+ @Override
+ public ElementMatcher<MethodDescription> getConstructorMatcher() {
+ return takesArgumentWithType(0, CONSTRUCTOR_INTERCEPT_MAP_TYPE);
+ }
+
+ @Override
+ public String getConstructorInterceptor() {
+ return MAP_CONSTRUCTOR_INTERCEPTOR_CLASS;
+ }
+ },
Review Comment:
> > I have already recerted it, this class dosen't change, I have already rollback it to the original version
>
> No, you didn't. Change is still there.
Resolved
--
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
Re: [PR] Feature/repairkafka3.7 [skywalking-java]
Posted by "wu-sheng (via GitHub)" <gi...@apache.org>.
wu-sheng commented on code in PR #707:
URL: https://github.com/apache/skywalking-java/pull/707#discussion_r1690750382
##########
apm-sniffer/apm-sdk-plugin/kafka-plugin/src/main/java/org/apache/skywalking/apm/plugin/kafka/Kafka37ConsumerInterceptor.java:
##########
@@ -0,0 +1,57 @@
+/*
+ * 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.kafka;
+
+import org.apache.kafka.clients.consumer.ConsumerRecord;
+import org.apache.kafka.clients.consumer.ConsumerRecords;
+import org.apache.kafka.common.TopicPartition;
+
+import java.util.ArrayList;
+import java.util.HashMap;
+import java.util.List;
+import java.util.Map;
+
+/**
+ * kafak3.7 removed the method named pollForFetches with return type Fetch.
+ * When the kafka version < 3.7, the enhance method could be: org.apache.kafka.clients.consumer.KafkaConsumer#pollForFetches.
+ * But when the kafka version >= 3.7, the enhance method should be: org.apache.kafka.clients.consumer.KafkaConsumer#poll(java.time.Duration).
+ * And the return type was also changed, from org.apache.kafka.clients.consumer.internals.Fetch to org.apache.kafka.clients.consumer.ConsumerRecords.
+ */
+public class Kafka37ConsumerInterceptor extends KafkaConsumerInterceptor {
+
+ @Override
+ protected Map<TopicPartition, List<ConsumerRecord<?, ?>>> fetchRecords(Object retObj) {
+ Map<TopicPartition, List<ConsumerRecord<?, ?>>> rsp = new HashMap<>();
+ if (retObj instanceof ConsumerRecords) {
Review Comment:
Why `instanceof`? Is there another case to have another parameter? The parent class has `Map<TopicPartition, List<ConsumerRecord<?, ?>>>` type as the parameter, but it should not affect this new Kafka37ConsumerInterceptor, right?
--
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
Re: [PR] Feature/repairkafka3.7 [skywalking-java]
Posted by "darkness-2nd (via GitHub)" <gi...@apache.org>.
darkness-2nd commented on code in PR #707:
URL: https://github.com/apache/skywalking-java/pull/707#discussion_r1686127385
##########
apm-sniffer/apm-sdk-plugin/kafka-plugin/src/main/java/org/apache/skywalking/apm/plugin/kafka/define/KafkaConsumerInstrumentation.java:
##########
@@ -62,104 +64,121 @@ public class KafkaConsumerInstrumentation extends AbstractKafkaInstrumentation {
@Override
public ConstructorInterceptPoint[] getConstructorsInterceptPoints() {
- return new ConstructorInterceptPoint[] {
- new ConstructorInterceptPoint() {
- @Override
- public ElementMatcher<MethodDescription> getConstructorMatcher() {
- return takesArgumentWithType(0, CONSTRUCTOR_INTERCEPT_TYPE);
- }
+ return new ConstructorInterceptPoint[]{
+ new ConstructorInterceptPoint() {
+ @Override
+ public ElementMatcher<MethodDescription> getConstructorMatcher() {
+ return takesArgumentWithType(0, CONSTRUCTOR_INTERCEPT_TYPE);
+ }
Review Comment:
> You should not reformat, thr code style file is in the source codes.
I reset the java file which I reformat
--
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
Re: [PR] Feature/repairkafka3.7 [skywalking-java]
Posted by "wu-sheng (via GitHub)" <gi...@apache.org>.
wu-sheng commented on PR #707:
URL: https://github.com/apache/skywalking-java/pull/707#issuecomment-2249496898
@kezhenxu94 As 403 respond code, should we add this into ignore list?
--
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
Re: [PR] Feature/repairkafka3.7 [skywalking-java]
Posted by "wu-sheng (via GitHub)" <gi...@apache.org>.
wu-sheng commented on code in PR #707:
URL: https://github.com/apache/skywalking-java/pull/707#discussion_r1690955075
##########
docs/en/setup/service-agent/java-agent/Supported-list.md:
##########
@@ -76,7 +76,7 @@ metrics based on the tracing data.
* MQ
* [RocketMQ](https://github.com/apache/rocketmq) 3.x-> 5.x
* [RocketMQ-gRPC](http://github.com/apache/rocketmq-clients) 5.x
- * [Kafka](http://kafka.apache.org) 0.11.0.0 -> 3.2.3
+ * [Kafka](http://kafka.apache.org) 0.11.0.0 -> 3.2.3 -> 3.7.1
Review Comment:
```suggestion
* [Kafka](http://kafka.apache.org) 0.11.0.0 -> 3.7.1
```
--
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
Re: [PR] Feature/repairkafka3.7 [skywalking-java]
Posted by "wu-sheng (via GitHub)" <gi...@apache.org>.
wu-sheng commented on PR #707:
URL: https://github.com/apache/skywalking-java/pull/707#issuecomment-2242323585
Please pick one for 3.4, 3.5, 3.6 as well.
--
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
Re: [PR] Feature/repairkafka3.7 [skywalking-java]
Posted by "wu-sheng (via GitHub)" <gi...@apache.org>.
wu-sheng commented on PR #707:
URL: https://github.com/apache/skywalking-java/pull/707#issuecomment-2241813975
If the new use cases are same here, you should add more versions to support your new implementations for newer versions, and keep compatibility with previous.
--
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
Re: [PR] Feature/repairkafka3.7 [skywalking-java]
Posted by "darkness-2nd (via GitHub)" <gi...@apache.org>.
darkness-2nd commented on code in PR #707:
URL: https://github.com/apache/skywalking-java/pull/707#discussion_r1690768521
##########
apm-sniffer/apm-sdk-plugin/kafka-plugin/src/main/java/org/apache/skywalking/apm/plugin/kafka/define/KafkaConsumerInstrumentation.java:
##########
@@ -63,103 +63,103 @@ public class KafkaConsumerInstrumentation extends AbstractKafkaInstrumentation {
@Override
public ConstructorInterceptPoint[] getConstructorsInterceptPoints() {
return new ConstructorInterceptPoint[] {
- new ConstructorInterceptPoint() {
- @Override
- public ElementMatcher<MethodDescription> getConstructorMatcher() {
- return takesArgumentWithType(0, CONSTRUCTOR_INTERCEPT_TYPE);
- }
+ new ConstructorInterceptPoint() {
+ @Override
+ public ElementMatcher<MethodDescription> getConstructorMatcher() {
+ return takesArgumentWithType(0, CONSTRUCTOR_INTERCEPT_TYPE);
+ }
- @Override
- public String getConstructorInterceptor() {
- return CONSUMER_CONFIG_CONSTRUCTOR_INTERCEPTOR_CLASS;
- }
- },
- new ConstructorInterceptPoint() {
- @Override
- public ElementMatcher<MethodDescription> getConstructorMatcher() {
- return takesArgumentWithType(0, CONSTRUCTOR_INTERCEPT_MAP_TYPE);
- }
-
- @Override
- public String getConstructorInterceptor() {
- return MAP_CONSTRUCTOR_INTERCEPTOR_CLASS;
- }
- },
+ @Override
+ public String getConstructorInterceptor() {
+ return CONSUMER_CONFIG_CONSTRUCTOR_INTERCEPTOR_CLASS;
+ }
+ },
+ new ConstructorInterceptPoint() {
+ @Override
+ public ElementMatcher<MethodDescription> getConstructorMatcher() {
+ return takesArgumentWithType(0, CONSTRUCTOR_INTERCEPT_MAP_TYPE);
+ }
+
+ @Override
+ public String getConstructorInterceptor() {
+ return MAP_CONSTRUCTOR_INTERCEPTOR_CLASS;
+ }
+ },
Review Comment:
> Please revert re-format.
I have already recerted it, this class dosen't change, I have already rollback it to the original version
--
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
Re: [PR] Feature/repairkafka3.7 [skywalking-java]
Posted by "darkness-2nd (via GitHub)" <gi...@apache.org>.
darkness-2nd commented on code in PR #707:
URL: https://github.com/apache/skywalking-java/pull/707#discussion_r1690848907
##########
apm-sniffer/apm-sdk-plugin/kafka-plugin/src/main/java/org/apache/skywalking/apm/plugin/kafka/define/Kafka37AsyncConsumerInstrumentation.java:
##########
@@ -0,0 +1,42 @@
+/*
+ * 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.kafka.define;
+
+import org.apache.skywalking.apm.agent.core.plugin.match.ClassMatch;
+import static org.apache.skywalking.apm.agent.core.plugin.match.NameMatch.byName;
+
+/**
Review Comment:
> For the comment, I think we can focus on the difference instead of repeating what is already written in the parent class.
Yes, the difference is the method named pollForFetchs was removed from KafkaConsumer to another two classes, so the original interceptor can not intercept it. Because of the enhance class is changed, so I create two new classes to repair the uncompatible problem.
--
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
Re: [PR] Feature/repairkafka3.7 [skywalking-java]
Posted by "wu-sheng (via GitHub)" <gi...@apache.org>.
wu-sheng commented on PR #707:
URL: https://github.com/apache/skywalking-java/pull/707#issuecomment-2242405190
> > Logically same. But we should test.
>
>
>
> OK, Should I create modules kafka-3.6.x-scenario and kafka-3.7.x-scenario like kafka-2.3.x-scenario in dir /test?
Those exist because the test codes are different. If you just need to change dependency versions, you don't need to do that.
--
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
Re: [PR] Feature/repairkafka3.7 [skywalking-java]
Posted by "wu-sheng (via GitHub)" <gi...@apache.org>.
wu-sheng commented on code in PR #707:
URL: https://github.com/apache/skywalking-java/pull/707#discussion_r1690732900
##########
docs/en/setup/service-agent/java-agent/Bootstrap-plugins.md:
##########
@@ -19,4 +19,3 @@ The plugin of JDK HttpURLConnection depended on `sun.net.*`. When using Java 9+,
For more information
1. [JEP 403: Strongly Encapsulate JDK Internals](https://openjdk.org/jeps/403)
-2. [A peek into Java 17: Encapsulating the Java runtime internals](https://blogs.oracle.com/javamagazine/post/a-peek-into-java-17-continuing-the-drive-to-encapsulate-the-java-runtime-internals)
Review Comment:
The blog is there. What is the deadlink check saying?
--
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
Re: [PR] Feature/repairkafka3.7 [skywalking-java]
Posted by "wu-sheng (via GitHub)" <gi...@apache.org>.
wu-sheng commented on code in PR #707:
URL: https://github.com/apache/skywalking-java/pull/707#discussion_r1690745753
##########
apm-sniffer/apm-sdk-plugin/kafka-plugin/src/main/java/org/apache/skywalking/apm/plugin/kafka/define/KafkaConsumerInstrumentation.java:
##########
@@ -63,103 +63,103 @@ public class KafkaConsumerInstrumentation extends AbstractKafkaInstrumentation {
@Override
public ConstructorInterceptPoint[] getConstructorsInterceptPoints() {
return new ConstructorInterceptPoint[] {
- new ConstructorInterceptPoint() {
- @Override
- public ElementMatcher<MethodDescription> getConstructorMatcher() {
- return takesArgumentWithType(0, CONSTRUCTOR_INTERCEPT_TYPE);
- }
+ new ConstructorInterceptPoint() {
+ @Override
+ public ElementMatcher<MethodDescription> getConstructorMatcher() {
+ return takesArgumentWithType(0, CONSTRUCTOR_INTERCEPT_TYPE);
+ }
- @Override
- public String getConstructorInterceptor() {
- return CONSUMER_CONFIG_CONSTRUCTOR_INTERCEPTOR_CLASS;
- }
- },
- new ConstructorInterceptPoint() {
- @Override
- public ElementMatcher<MethodDescription> getConstructorMatcher() {
- return takesArgumentWithType(0, CONSTRUCTOR_INTERCEPT_MAP_TYPE);
- }
-
- @Override
- public String getConstructorInterceptor() {
- return MAP_CONSTRUCTOR_INTERCEPTOR_CLASS;
- }
- },
+ @Override
+ public String getConstructorInterceptor() {
+ return CONSUMER_CONFIG_CONSTRUCTOR_INTERCEPTOR_CLASS;
+ }
+ },
+ new ConstructorInterceptPoint() {
+ @Override
+ public ElementMatcher<MethodDescription> getConstructorMatcher() {
+ return takesArgumentWithType(0, CONSTRUCTOR_INTERCEPT_MAP_TYPE);
+ }
+
+ @Override
+ public String getConstructorInterceptor() {
+ return MAP_CONSTRUCTOR_INTERCEPTOR_CLASS;
+ }
+ },
Review Comment:
Please revert re-format.
--
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
Re: [PR] Feature/repairkafka3.7 [skywalking-java]
Posted by "darkness-2nd (via GitHub)" <gi...@apache.org>.
darkness-2nd commented on PR #707:
URL: https://github.com/apache/skywalking-java/pull/707#issuecomment-2249629671
> One more. changes.md should be updated as well.
Changed
--
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
Re: [PR] Feature/repairkafka3.7 [skywalking-java]
Posted by "lujiajing1126 (via GitHub)" <gi...@apache.org>.
lujiajing1126 commented on code in PR #707:
URL: https://github.com/apache/skywalking-java/pull/707#discussion_r1690845088
##########
apm-sniffer/apm-sdk-plugin/kafka-plugin/src/main/java/org/apache/skywalking/apm/plugin/kafka/define/Kafka37AsyncConsumerInstrumentation.java:
##########
@@ -0,0 +1,42 @@
+/*
+ * 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.kafka.define;
+
+import org.apache.skywalking.apm.agent.core.plugin.match.ClassMatch;
+import static org.apache.skywalking.apm.agent.core.plugin.match.NameMatch.byName;
+
+/**
Review Comment:
For the comment, I think we can focus on the difference instead of repeating what is already written in the parent class.
##########
apm-sniffer/apm-sdk-plugin/kafka-plugin/src/main/java/org/apache/skywalking/apm/plugin/kafka/define/KafkaConsumerInstrumentation.java:
##########
@@ -63,103 +63,103 @@ public class KafkaConsumerInstrumentation extends AbstractKafkaInstrumentation {
@Override
public ConstructorInterceptPoint[] getConstructorsInterceptPoints() {
return new ConstructorInterceptPoint[] {
- new ConstructorInterceptPoint() {
- @Override
- public ElementMatcher<MethodDescription> getConstructorMatcher() {
- return takesArgumentWithType(0, CONSTRUCTOR_INTERCEPT_TYPE);
- }
+ new ConstructorInterceptPoint() {
+ @Override
+ public ElementMatcher<MethodDescription> getConstructorMatcher() {
+ return takesArgumentWithType(0, CONSTRUCTOR_INTERCEPT_TYPE);
+ }
- @Override
- public String getConstructorInterceptor() {
- return CONSUMER_CONFIG_CONSTRUCTOR_INTERCEPTOR_CLASS;
- }
- },
- new ConstructorInterceptPoint() {
- @Override
- public ElementMatcher<MethodDescription> getConstructorMatcher() {
- return takesArgumentWithType(0, CONSTRUCTOR_INTERCEPT_MAP_TYPE);
- }
-
- @Override
- public String getConstructorInterceptor() {
- return MAP_CONSTRUCTOR_INTERCEPTOR_CLASS;
- }
- },
+ @Override
+ public String getConstructorInterceptor() {
+ return CONSUMER_CONFIG_CONSTRUCTOR_INTERCEPTOR_CLASS;
+ }
+ },
+ new ConstructorInterceptPoint() {
+ @Override
+ public ElementMatcher<MethodDescription> getConstructorMatcher() {
+ return takesArgumentWithType(0, CONSTRUCTOR_INTERCEPT_MAP_TYPE);
+ }
+
+ @Override
+ public String getConstructorInterceptor() {
+ return MAP_CONSTRUCTOR_INTERCEPTOR_CLASS;
+ }
+ },
Review Comment:
> I have already recerted it, this class dosen't change, I have already rollback it to the original version
No, you didn't. Change is still there.
--
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
Re: [PR] Feature/repairkafka3.7 [skywalking-java]
Posted by "darkness-2nd (via GitHub)" <gi...@apache.org>.
darkness-2nd commented on PR #707:
URL: https://github.com/apache/skywalking-java/pull/707#issuecomment-2249496863
> > --- /home/runner/work/skywalking-java/skywalking-java/tools/plugin/md-javaagent-plugin-list.txt 2024-07-25 03:04:25.186078041 +0000
> > +++ /home/runner/work/skywalking-java/skywalking-java/tools/plugin/genernate-javaagent-plugin-list.txt 2024-0[7](https://github.com/apache/skywalking-java/actions/runs/10087411570/job/27891606111?pr=707#step:5:8)-25 03:04:25.178078015 +0000
>
> I think you did not update docs accordingly, Plugin-list.md and Supported-list.md
> > --- /home/runner/work/skywalking-java/skywalking-java/tools/plugin/md-javaagent-plugin-list.txt 2024-07-25 03:04:25.186078041 +0000
> > +++ /home/runner/work/skywalking-java/skywalking-java/tools/plugin/genernate-javaagent-plugin-list.txt 2024-0[7](https://github.com/apache/skywalking-java/actions/runs/10087411570/job/27891606111?pr=707#step:5:8)-25 03:04:25.178078015 +0000
>
> I think you did not update docs accordingly, Plugin-list.md and Supported-list.md
> And please fix CIs.
> And please fix CIs.
> And please fix CIs.
All the CIs were successfully executed but the DeadLink check
--
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
Re: [PR] Feature/repairkafka3.7 [skywalking-java]
Posted by "darkness-2nd (via GitHub)" <gi...@apache.org>.
darkness-2nd commented on PR #707:
URL: https://github.com/apache/skywalking-java/pull/707#issuecomment-2241880439
> ase explain what is changing here, and if th
> If the new use cases are same here, you should add more versions to support your new implementations for newer versions, and keep compatibility with previous.
Ok, I'll commit later
--
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
Re: [PR] Feature/repairkafka3.7 [skywalking-java]
Posted by "wu-sheng (via GitHub)" <gi...@apache.org>.
wu-sheng commented on PR #707:
URL: https://github.com/apache/skywalking-java/pull/707#issuecomment-2242110660
About https://github.com/apache/skywalking-java/pull/708#issuecomment-2242063529 I mean the test versions
demo codes https://github.com/apache/skywalking-java/tree/main/test/plugin/scenarios/kafka-scenario
tested versions https://github.com/apache/skywalking-java/blob/main/test/plugin/scenarios/kafka-scenario/support-version.list
--
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
Re: [PR] Feature/repairkafka3.7 [skywalking-java]
Posted by "darkness-2nd (via GitHub)" <gi...@apache.org>.
darkness-2nd commented on PR #707:
URL: https://github.com/apache/skywalking-java/pull/707#issuecomment-2246138300
> You should verify all locally rather than costing too much on CI.
Sry, it's my first time to create PR in github, I'm doing test CI in my private branch 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
Re: [PR] Feature/repairkafka3.7 [skywalking-java]
Posted by "wu-sheng (via GitHub)" <gi...@apache.org>.
wu-sheng commented on PR #707:
URL: https://github.com/apache/skywalking-java/pull/707#issuecomment-2244249281
You should verify all locally rather than costing too much on CI.
--
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
Re: [PR] Feature/repairkafka3.7 [skywalking-java]
Posted by "wu-sheng (via GitHub)" <gi...@apache.org>.
wu-sheng commented on PR #707:
URL: https://github.com/apache/skywalking-java/pull/707#issuecomment-2242330562
Logically same. But we should 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
Re: [PR] Feature/repairkafka3.7 [skywalking-java]
Posted by "darkness-2nd (via GitHub)" <gi...@apache.org>.
darkness-2nd commented on code in PR #707:
URL: https://github.com/apache/skywalking-java/pull/707#discussion_r1690867660
##########
apm-sniffer/apm-sdk-plugin/kafka-plugin/src/main/java/org/apache/skywalking/apm/plugin/kafka/define/KafkaConsumerInstrumentation.java:
##########
@@ -63,103 +63,103 @@ public class KafkaConsumerInstrumentation extends AbstractKafkaInstrumentation {
@Override
public ConstructorInterceptPoint[] getConstructorsInterceptPoints() {
return new ConstructorInterceptPoint[] {
- new ConstructorInterceptPoint() {
- @Override
- public ElementMatcher<MethodDescription> getConstructorMatcher() {
- return takesArgumentWithType(0, CONSTRUCTOR_INTERCEPT_TYPE);
- }
+ new ConstructorInterceptPoint() {
+ @Override
+ public ElementMatcher<MethodDescription> getConstructorMatcher() {
+ return takesArgumentWithType(0, CONSTRUCTOR_INTERCEPT_TYPE);
+ }
- @Override
- public String getConstructorInterceptor() {
- return CONSUMER_CONFIG_CONSTRUCTOR_INTERCEPTOR_CLASS;
- }
- },
- new ConstructorInterceptPoint() {
- @Override
- public ElementMatcher<MethodDescription> getConstructorMatcher() {
- return takesArgumentWithType(0, CONSTRUCTOR_INTERCEPT_MAP_TYPE);
- }
-
- @Override
- public String getConstructorInterceptor() {
- return MAP_CONSTRUCTOR_INTERCEPTOR_CLASS;
- }
- },
+ @Override
+ public String getConstructorInterceptor() {
+ return CONSUMER_CONFIG_CONSTRUCTOR_INTERCEPTOR_CLASS;
+ }
+ },
+ new ConstructorInterceptPoint() {
+ @Override
+ public ElementMatcher<MethodDescription> getConstructorMatcher() {
+ return takesArgumentWithType(0, CONSTRUCTOR_INTERCEPT_MAP_TYPE);
+ }
+
+ @Override
+ public String getConstructorInterceptor() {
+ return MAP_CONSTRUCTOR_INTERCEPTOR_CLASS;
+ }
+ },
Review Comment:
> > I have already recerted it, this class dosen't change, I have already rollback it to the original version
>
> No, you didn't. Change is still there.
I dont't know why.... I check out the original branch and copy the codes into the class file, but the IDE says that there is no difference.....
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@skywalking.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Feature/repairkafka3.7 [skywalking-java]
Posted by "wu-sheng (via GitHub)" <gi...@apache.org>.
wu-sheng commented on PR #707:
URL: https://github.com/apache/skywalking-java/pull/707#issuecomment-2249623005
One more. changes.md should be updated as well.
--
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
Re: [PR] Feature/repairkafka3.7 [skywalking-java]
Posted by "darkness-2nd (via GitHub)" <gi...@apache.org>.
darkness-2nd commented on code in PR #707:
URL: https://github.com/apache/skywalking-java/pull/707#discussion_r1690732121
##########
docs/en/setup/service-agent/java-agent/Bootstrap-plugins.md:
##########
@@ -19,4 +19,3 @@ The plugin of JDK HttpURLConnection depended on `sun.net.*`. When using Java 9+,
For more information
1. [JEP 403: Strongly Encapsulate JDK Internals](https://openjdk.org/jeps/403)
-2. [A peek into Java 17: Encapsulating the Java runtime internals](https://blogs.oracle.com/javamagazine/post/a-peek-into-java-17-continuing-the-drive-to-encapsulate-the-java-runtime-internals)
Review Comment:
> I think you made a mistake to remove this line?
No, the CI thiks this line is a deadlink, though it can be access in browser
--
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
Re: [PR] Feature/repairkafka3.7 [skywalking-java]
Posted by "darkness-2nd (via GitHub)" <gi...@apache.org>.
darkness-2nd commented on code in PR #707:
URL: https://github.com/apache/skywalking-java/pull/707#discussion_r1690734241
##########
docs/en/setup/service-agent/java-agent/Bootstrap-plugins.md:
##########
@@ -19,4 +19,3 @@ The plugin of JDK HttpURLConnection depended on `sun.net.*`. When using Java 9+,
For more information
1. [JEP 403: Strongly Encapsulate JDK Internals](https://openjdk.org/jeps/403)
-2. [A peek into Java 17: Encapsulating the Java runtime internals](https://blogs.oracle.com/javamagazine/post/a-peek-into-java-17-continuing-the-drive-to-encapsulate-the-java-runtime-internals)
Review Comment:
> You should not remove the docs, just because a CI took says otherwise.
OK, I will rollback it, thank 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