Skip to content
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

Add plugin to support clickhouse jdbc #455

Merged
merged 6 commits into from
Feb 12, 2023
Merged

Conversation

yanye666
Copy link
Contributor

@yanye666 yanye666 commented Feb 12, 2023

Add an agent plugin to support <clickhouse-jdbc-0.3.2.*>

The new version of clickhouse-jdbc has major changes and needs to be re-compatible.

The author reconstructs in the new version, and the code structure changes, so compatibility is not recommended. Later versions of clickhouse-jdbc will remove the code from previous versions

@yanye666
Copy link
Contributor Author

yanye666 commented Feb 12, 2023

Hello, I'm having a problem compiling the project.

类文件具有错误的版本 55.0, 应为 52.0

image

There is more than one place for such problems,How is this compatible? This should be supported by jdk1.8,How can I package support for jdk1.8

@yanye666 yanye666 changed the title Clickhouse jdbc Add plugin to support clickhouse jdbc Feb 12, 2023
@wu-sheng
Copy link
Member

Your local env should be JDK17, the class file could be jdk8 like most plugins.
Also, we should plugin test doc to write tests to verify plugin with a demo application.

https://skywalking.apache.org/docs/skywalking-java/next/en/setup/service-agent/java-agent/java-plugin-development-guide/#contribute-plugins-to-the-apache-skywalking-repository

@yanye666
Copy link
Contributor Author

Your local env should be JDK17, the class file could be jdk8 like most plugins. Also, we should plugin test doc to write tests to verify plugin with a demo application.

https://skywalking.apache.org/docs/skywalking-java/next/en/setup/service-agent/java-agent/java-plugin-development-guide/#contribute-plugins-to-the-apache-skywalking-repository

please check

@wu-sheng
Copy link
Member

I am confused, clickhouse-jdbc plugin exists for 0.3.0 and 0.3.1. What does actually change in 0.3.2? Why do we have to write all rather than doing some compatible work on the original one?

@yanye666
Copy link
Contributor Author

yanye666 commented Feb 12, 2023

I am confused, clickhouse-jdbc plugin exists for 0.3.0 and 0.3.1. What does actually change in 0.3.2? Why do we have to write all rather than doing some compatible work on the original one?

The author reconstructs in the new version, and the code structure changes, so compatibility is not recommended. Later versions of clickhouse-jdbc will remove the code from previous versions

ClickHouse/clickhouse-java#768

@wu-sheng
Copy link
Member

I am confused, clickhouse-jdbc plugin exists for 0.3.0 and 0.3.1. What does actually change in 0.3.2? Why do we have to write all rather than doing some compatible work on the original one?

The author refactored the new version, and the version-related packages will be removed later

ClickHouse/clickhouse-java#768

If so, please update the previous clickhouse-0.3.x-plugin to clickhouse-0.3.1-plugin(same for scenario name). And you should add your test into GitHub Action like the clickhouse-0.3.x-scenario.

# See the License for the specific language governing permissions and
# limitations under the License.

clickhouse-0.3.2.x=org.apache.skywalking.apm.plugin.jdbc.clickhouse.v32.define.ConnectionInstrumentation
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The plugin of 0.3.x uses clickhouse-0.3.x. As 0.3.2 is an incompatible change, that keys should be updated to clickhouse-0.3.1.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

clickhouse-0.3.x-plugin to clickhouse-0.3.1-plugin.

@wu-sheng
Copy link
Member

Run tools/plugin/check-javaagent-plugin-list.sh
--- /home/runner/work/skywalking-java/skywalking-java/tools/plugin/md-javaagent-plugin-list.txt 2023-02-12 13:26:52.050277302 +0000
+++ /home/runner/work/skywalking-java/skywalking-java/tools/plugin/genernate-javaagent-plugin-list.txt 2023-02-12 13:26:52.042277371 +0000

Plugin-list.md should be updated according to your new plugin key.

@wu-sheng wu-sheng added this to the 8.15.0 milestone Feb 12, 2023
Copy link
Member

@wu-sheng wu-sheng left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. Thanks for keeping update.

@wu-sheng wu-sheng merged commit 9cb5a13 into apache:main Feb 12, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants