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

Bump up grpc #650

Merged
merged 3 commits into from
Nov 28, 2023
Merged

Bump up grpc #650

merged 3 commits into from
Nov 28, 2023

Conversation

kezhenxu94
Copy link
Member

@kezhenxu94 kezhenxu94 commented Nov 28, 2023

  • If this pull request closes/resolves/fixes an existing issue, replace the issue number. Closes #.
  • Update the CHANGES log.

@kezhenxu94
Copy link
Member Author

rocketmq plugin is preventing us from upgrading grpc now

@wu-sheng
Copy link
Member

Could you recheck the CI?

@kezhenxu94
Copy link
Member Author

Could you recheck the CI?

#650 (comment)

👆

@wu-sheng
Copy link
Member

What do you mean? How a plugin blocks a lib upgrade?

@kezhenxu94
Copy link
Member Author

The log is better than hundreds of words:

Could not resolve version conflict among [org.apache.rocketmq:rocketmq-client:jar:5.1.1 -> org.apache.rocketmq:rocketmq-remoting:jar:5.1.1 -> org.apache.rocketmq:rocketmq-common:jar:5.1.1 -> io.grpc:grpc-netty-shaded:jar:1.50.0 -> io.grpc:grpc-core:jar:[1.50.0,1.50.0], org.apache.skywalking:apm-agent-core:jar:9.1.0-SNAPSHOT -> org.apache.skywalking:java-agent-network:jar:9.1.0-SNAPSHOT -> io.grpc:grpc-netty:jar:1.53.0 -> io.grpc:grpc-core:jar:[1.53.0,1.53.0]] -> [Help 1]

https://github.com/apache/skywalking-java/actions/runs/7015220764/job/19084175721?pr=650#step:3:431

@kezhenxu94
Copy link
Member Author

rocketmq sets a hard requirement of gRPC 1.53.0, upgrading our core grpc to >1.53.0 breaks maven dependency resolution

Comment on lines +42 to +47
<exclusions>
<exclusion>
<groupId>io.grpc</groupId>
<artifactId>grpc-netty</artifactId>
</exclusion>
</exclusions>
Copy link
Member

Choose a reason for hiding this comment

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

I think we should exclude grpc from agent-core, rather than from this side?

Copy link
Member Author

Choose a reason for hiding this comment

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

Why? GRPC is needed in agent side, how can we exclude it there?

Copy link
Member

Choose a reason for hiding this comment

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

I think the agent plugin depends on the agent core lib, which has the gRPC dependency overriding the version.
We just need the RocketMQ plugin compiled. So, about exclude, I mean for this plugin, it could depend on agent-core with gRPC excluded. In the runtime, this plugin is using the user's gRPC, our gRPC and netty would have been shaded in another package already. So, we should be good by that.

Comment on lines +43 to +53
<dependency>
<groupId>org.apache.skywalking</groupId>
<artifactId>apm-agent-core</artifactId>
<version>${project.version}</version>
<scope>provided</scope>
<exclusions>
<exclusion>
<groupId>io.grpc</groupId>
<artifactId>grpc-netty</artifactId>
</exclusion>
</exclusions>
Copy link
Member

Choose a reason for hiding this comment

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

These are my new changes.

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.

Dep upgrade works.

@wu-sheng wu-sheng merged commit 5063985 into main Nov 28, 2023
182 checks passed
@wu-sheng wu-sheng deleted the grpc branch November 28, 2023 16:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants