-
Notifications
You must be signed in to change notification settings - Fork 8.1k
[dashboard/1.8] feat: 新增基于Nacos的规则持久化支持 #3535
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
base: 1.8
Are you sure you want to change the base?
Conversation
官方是不是不维护了啊? |
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 PR adds Nacos-based rule persistence support to the Sentinel Dashboard, addressing the issue where rules are lost after dashboard restarts. The implementation includes both rule providers and publishers for various rule types, along with configuration for Nacos service discovery.
- Integration of Nacos as a persistence layer for Sentinel rules
- Support for multiple rule types (flow, degrade, authority, system, param flow, cluster flow)
- Modified controllers to use Nacos providers/publishers instead of direct client communication
- Added service discovery registration to Nacos
Reviewed Changes
Copilot reviewed 31 out of 31 changed files in this pull request and generated 8 comments.
Show a summary per file
File | Description |
---|---|
pom.xml | Added Spring Cloud and Nacos dependencies |
application.properties | Added Nacos discovery configuration |
DashboardApplication.java | Enabled discovery client for Nacos registration |
NacosConfig.java | Configuration bean for Nacos ConfigService |
*RuleNacosProvider.java | Rule providers for reading from Nacos |
*RuleNacosPublisher.java | Rule publishers for writing to Nacos |
Various controllers | Modified to use Nacos providers/publishers and import optimizations |
Comments suppressed due to low confidence (1)
sentinel-dashboard/src/main/java/com/alibaba/csp/sentinel/dashboard/controller/DegradeController.java:158
- The publishRulesToNacos call should be wrapped in a try-catch block to handle exceptions properly and avoid breaking the rule delete operation.
publishRulesToNacos(oldEntity.getApp());
public void init() throws Exception { | ||
Properties properties = new Properties(); | ||
properties.setProperty(PropertyKeyConst.SERVER_ADDR, serverAddr); | ||
if (namespace != null && !namespace.isEmpty()) { |
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.
[nitpick] Use StringUtil.isNotBlank() for consistency with other parts of the codebase instead of null check and isEmpty().
if (namespace != null && !namespace.isEmpty()) { | |
if (StringUtils.isNotBlank(namespace)) { |
Copilot uses AI. Check for mistakes.
} | ||
|
||
// === 新增:发布规则到 Nacos 的方法 === | ||
private void publishRulesToNacos(String app) throws Exception { |
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.
Method throws generic Exception. Consider catching and wrapping with a more specific exception or logging the error instead of propagating.
private void publishRulesToNacos(String app) throws Exception { | |
private void publishRulesToNacos(String app) { |
Copilot uses AI. Check for mistakes.
private void publishRulesToNacos(String app) throws Exception { | ||
List<ParamFlowRuleEntity> rules = repository.findAllByApp(app); | ||
rulePublisher.publish(app, rules); |
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.
Method throws generic Exception. Consider catching and wrapping with a more specific exception or logging the error instead of propagating.
private void publishRulesToNacos(String app) throws Exception { | |
List<ParamFlowRuleEntity> rules = repository.findAllByApp(app); | |
rulePublisher.publish(app, rules); | |
private void publishRulesToNacos(String app) { | |
try { | |
List<ParamFlowRuleEntity> rules = repository.findAllByApp(app); | |
rulePublisher.publish(app, rules); | |
} catch (Exception e) { | |
logger.error("Error publishing parameter flow rules to Nacos for app: {}", app, e); | |
throw new RuntimeException("Failed to publish parameter flow rules to Nacos for app: " + app, e); | |
} |
Copilot uses AI. Check for mistakes.
private void publishRulesToNacos(String app) throws Exception { | ||
List<FlowRuleEntity> rules = repository.findAllByApp(app); | ||
rulePublisher.publish(app, rules); |
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.
Method throws generic Exception. Consider catching and wrapping with a more specific exception or logging the error instead of propagating.
private void publishRulesToNacos(String app) throws Exception { | |
List<FlowRuleEntity> rules = repository.findAllByApp(app); | |
rulePublisher.publish(app, rules); | |
private void publishRulesToNacos(String app) { | |
try { | |
List<FlowRuleEntity> rules = repository.findAllByApp(app); | |
rulePublisher.publish(app, rules); | |
} catch (Exception e) { | |
logger.error("Error publishing rules to Nacos for app: {}", app, e); | |
throw new RuntimeException("Failed to publish rules to Nacos for app: " + app, e); | |
} |
Copilot uses AI. Check for mistakes.
private void publishRulesToNacos(String app) throws Exception { | ||
List<DegradeRuleEntity> rules = repository.findAllByApp(app); | ||
rulePublisher.publish(app, rules); |
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.
Method throws generic Exception. Consider catching and wrapping with a more specific exception or logging the error instead of propagating.
private void publishRulesToNacos(String app) throws Exception { | |
List<DegradeRuleEntity> rules = repository.findAllByApp(app); | |
rulePublisher.publish(app, rules); | |
private void publishRulesToNacos(String app) { | |
try { | |
List<DegradeRuleEntity> rules = repository.findAllByApp(app); | |
rulePublisher.publish(app, rules); | |
} catch (Exception e) { | |
logger.error("Failed to publish degrade rules to Nacos for app: {}", app, e); | |
throw new RuntimeException("Failed to publish degrade rules to Nacos for app: " + app, e); | |
} |
Copilot uses AI. Check for mistakes.
private void publishRulesToNacos(String app) throws Exception { | ||
List<AuthorityRuleEntity> rules = repository.findAllByApp(app); | ||
rulePublisher.publish(app, rules); |
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.
Method throws generic Exception. Consider catching and wrapping with a more specific exception or logging the error instead of propagating.
private void publishRulesToNacos(String app) throws Exception { | |
List<AuthorityRuleEntity> rules = repository.findAllByApp(app); | |
rulePublisher.publish(app, rules); | |
private void publishRulesToNacos(String app) { | |
try { | |
List<AuthorityRuleEntity> rules = repository.findAllByApp(app); | |
rulePublisher.publish(app, rules); | |
} catch (Exception e) { | |
logger.error("Failed to publish authority rules to Nacos for app: {}", app, e); | |
throw new RuntimeException("Failed to publish authority rules to Nacos for app: " + app, e); | |
} |
Copilot uses AI. Check for mistakes.
entity.setGmtModified(date); | ||
try { | ||
entity = repository.save(entity); | ||
publishRulesToNacos(entity.getApp()); |
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.
The publishRulesToNacos call should be wrapped in a try-catch block to handle exceptions properly and avoid breaking the rule save operation.
Copilot uses AI. Check for mistakes.
entity.setGmtModified(new Date()); | ||
try { | ||
entity = repository.save(entity); | ||
publishRulesToNacos(entity.getApp()); |
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.
The publishRulesToNacos call should be wrapped in a try-catch block to handle exceptions properly and avoid breaking the rule update operation.
Copilot uses AI. Check for mistakes.
<!-- Nacos 服务发现 --> | ||
<dependency> | ||
<groupId>com.alibaba.cloud</groupId> | ||
<artifactId>spring-cloud-starter-alibaba-nacos-discovery</artifactId> |
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.
Should we consider add nacos-discovery?
|
||
private final Logger logger = LoggerFactory.getLogger(AuthorityRuleController.class); | ||
|
||
// === 新增:注入 Nacos Provider/Publisher === |
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.
This type of comment seems unnecessary - we can safely remove it to keep the code concise. (the same applies to other similar comments below)
What changes were made?
Why are these changes needed?
How to verify these changes?