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

fix: Nacos integration with SpringBootAdmin.(2.2.x) #3308

Merged
merged 4 commits into from
May 27, 2023

Conversation

zhangbinhub
Copy link
Contributor

Describe what this PR does / why we need it

Refactor GatewayLocatorHeartBeatPublisher to enable friendly subscription to HeartBeat events in addition to the gateway

Does this pull request fix one issue?

Refer #3258

Describe how you did it

  1. GatewayLocatorHeartBeatPublisher migrate to NacosDiscoveryHeartBeatPublisher.
  2. Add properties spring.cloud.nacos.discovery.heart-beat.enabled to enable NacosDiscoveryHeartBeatPublisher, default is false .
  3. Optimize the lifecycle of NacosDiscoveryHeartBeatPublisher, set running correctly after start and stop.
  4. Separate NacosDiscoveryHeartBeatConfiguration and use conditional combinations compatible with spring cloud gateway, spring boot admin, and others.

…binations compatible with spring cloud gateway, spring boot admin, and others
@zhangbinhub
Copy link
Contributor Author

@steverao @ruansheng8 May I ask when this PR can be reviewed? this pr is to solve the heartbeat problem for 2.2.x

@@ -90,6 +90,11 @@
<artifactId>spring-cloud-starter-openfeign</artifactId>
<scope>test</scope>
</dependency>
<dependency>
Copy link
Collaborator

Choose a reason for hiding this comment

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

Why do you add it here?

Copy link
Contributor Author

@zhangbinhub zhangbinhub May 23, 2023

Choose a reason for hiding this comment

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

单元测试的时候,这个模块总是测试不通过,抛异常zuul的类找不到,于是我就添加了spring-cloud-netflix-zuul用于测试,测试就通过了

Copy link
Contributor Author

@zhangbinhub zhangbinhub May 23, 2023

Choose a reason for hiding this comment

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

具体可以参看一下昨天执行失败的那个集成测试action,是和此次pr无关的xds-adapter模块测试没通过
https://github.com/alibaba/spring-cloud-alibaba/actions/runs/5016091380

Copy link
Collaborator

Choose a reason for hiding this comment

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

具体可以参看一下昨天执行失败的那个集成测试action,是和此次pr无关的xds-adapter模块测试没通过 https://github.com/alibaba/spring-cloud-alibaba/actions/runs/5016091380

我看了一下这个问题是这次修复新引入的,具体是NacosDiscoveryHeartBeatConfiguration类中的@AutoConfigureAfter注解引发的,需要解决一下,否则后边其他人引入Nacos相关依赖都需要手动添加Zuul依赖。

Copy link
Contributor Author

Choose a reason for hiding this comment

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

具体可以参看一下昨天执行失败的那个集成测试action,是和此次pr无关的xds-adapter模块测试没通过 https://github.com/alibaba/spring-cloud-alibaba/actions/runs/5016091380

我看了一下这个问题是这次修复新引入的,具体是NacosDiscoveryHeartBeatConfiguration类中的@AutoConfigureAfter注解引发的,需要解决一下,否则后边其他人引入Nacos相关依赖都需要手动添加Zuul依赖。

感谢提醒,通过仔细阅读代码我明白问题在哪了。正如您所说,我尽快解决这个问题

@zhangbinhub zhangbinhub requested a review from steverao May 25, 2023 15:02
Copy link
Collaborator

@steverao steverao 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 @zhangbinhub Please add my dingtalk's account: steverao2021

@steverao steverao merged commit 1add626 into alibaba:2.2.x May 27, 2023
@zhangbinhub zhangbinhub deleted the fix-nacos-heartbeat-2.2.x branch July 27, 2023 09:34
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