-
Notifications
You must be signed in to change notification settings - Fork 666
Support kafka-clients-3.9.x intercept & Upgrade kafka-clients version in optional-reporter-plugins to 3.9.1 #780
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
|
To add kafka new client version support, you should add new version into test scenarios. As we have 3.7.1, you should add newer versions. |
|
As we don't have Spring Kafka 3+, please add them as well. I am not sure whether you need a new scenario project. If so, please add one, and make it in the proper GHA control file. |
I have not modified the kafka-client plugin, so there is no need to add new test scenarios. This PR enhances the spring-kafka plugin by adding support for spring-kafka 3.1.0+ versions. The reason is that spring-kafka 3.1.0+ introduces an internal class named ExtendedKafkaConsumer in DefaultKafkaConsumerFactory, which caused the original spring-kafka 2.x version plugin to become ineffective. In spring-kafka 2.x, span injection was implemented by intercepting the class org.apache.kafka.clients.consumer.KafkaConsumer in the kafka-plugin. However, in spring-kafka 3.1.0+, the consumer class has been replaced by ExtendedKafkaConsumer. This change necessitates the enhancement to ensure compatibility with the newer versions of spring-kafka. |
Ok, I'll add a new scenario. |
This makes sense to me. |
|
Your new test is not added into GitHub action control file. Please fix. |
Fixed |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This pull request enhances the Spring Kafka plugin to support newer versions of kafka-clients (3.7.1+) and spring-kafka (3.1.0+), while maintaining backward compatibility with version 2.x. The PR renames the plugin module from spring-kafka-2.x-plugin to spring-kafka-2.x-3.x-plugin and upgrades the kafka-clients dependency to version 3.9.1.
Key changes include:
- Adding support for Spring Kafka 3.x by instrumenting
ExtendedKafkaConsumerclass introduced in newer versions - Creating a new test scenario for spring-kafka-3.3.x to verify the enhanced functionality
- Upgrading kafka-clients version from 2.4.1 to 3.9.1 in optional-reporter-plugins
Reviewed Changes
Copilot reviewed 21 out of 26 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
| test/plugin/scenarios/spring-kafka-3.3.x-scenario/* | New test scenario for spring-kafka 3.3.x with controller, configuration, and expected data |
| apm-sniffer/apm-sdk-plugin/spring-plugins/spring-kafka-2.x-3.x-plugin/src/main/java/org/apache/skywalking/apm/plugin/spring/kafka/* | New interceptors and instrumentation for ExtendedKafkaConsumer to support spring-kafka 3.x |
| apm-sniffer/apm-sdk-plugin/spring-plugins/spring-kafka-2.x-3.x-plugin/src/test/java/* | Unit tests for the new ExtendedKafkaConsumer interceptor |
| apm-sniffer/apm-sdk-plugin/spring-plugins/spring-kafka-2.x-3.x-plugin/pom.xml | Renamed artifact ID and added kafka-clients 3.9.1 dependency with test dependencies |
| apm-sniffer/optional-reporter-plugins/pom.xml | Upgraded kafka-clients version from 2.4.1 to 3.9.1 |
| docs/en/setup/service-agent/java-agent/Plugin-list.md | Updated documentation to reflect support for spring-kafka 2.x/3.x |
| CHANGES.md | Added changelog entries for the enhancements |
| .github/workflows/plugins-test.3.yaml | Added spring-kafka-3.3.x-scenario to test workflow |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
...rc/main/java/test/apache/skywalking/apm/testcase/spring/kafka/controller/CaseController.java
Outdated
Show resolved
Hide resolved
test/plugin/scenarios/spring-kafka-3.3.x-scenario/config/expectedData.yaml
Outdated
Show resolved
Hide resolved
test/plugin/scenarios/spring-kafka-3.3.x-scenario/config/expectedData.yaml
Outdated
Show resolved
Hide resolved
...ain/java/org/apache/skywalking/apm/plugin/spring/kafka/ExtendedKafkaConsumerInterceptor.java
Outdated
Show resolved
Hide resolved
162e088 to
e7c145b
Compare
| import org.apache.skywalking.apm.agent.core.plugin.interceptor.enhance.EnhancedInstance; | ||
| import org.apache.skywalking.apm.agent.core.plugin.interceptor.enhance.InstanceConstructorInterceptor; | ||
|
|
||
| public class ExtendedConstructorInterceptPoint implements InstanceConstructorInterceptor { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| public class ExtendedConstructorInterceptPoint implements InstanceConstructorInterceptor { | |
| public class ExtendedConstructorInterceptor implements InstanceConstructorInterceptor { |
| @Override | ||
| public void onConstruct(final EnhancedInstance objInst, final Object[] allArguments) throws Throwable { | ||
| ExtendedConsumerEnhanceRequiredInfo requiredInfo = new ExtendedConsumerEnhanceRequiredInfo(); | ||
| extractConsumerConfig(allArguments, requiredInfo); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We should not check argument type and arg list in the interceptor. The more proper way is setting the ElementMatcher for the specific method(s).
|
For your new case
This compiling error should indicate JVM version incorrect. For newer JDK is requried, you should put this case into another other GHA control file. |
Yes, I've identified this error. Spring Boot 3.x requires JDK 17, and I'll fix this issue. |
It seems your demo app can't boot successfully. |
|
Did you find the issue of testing failue? |
Yes, the reason is the Spring version matching issue; I'll verify it thoroughly locally first |
|
After local testing, I found the issue wasn't caused by spring-kafka's new |
…on in optional-reporter-plugins to 3.9.1
Support kafka-clients-3.9.x intercept
Upgrade kafka-clients version in optional-reporter-plugins to 3.9.1
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.
Closes [Feature] Support kafka-clients-3.9.x intercept & Upgrade kafka-clients version in optional-reporter-plugins to 3.9.1 skywalking#13560