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.

Sign up for GitHub

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

Jump to bottom

Optimize dynamic config: integrate Zookeeper & Nacos, support interface-level dynamic config #1430

Merged
merged 9 commits into from
Oct 31, 2024
+1,793 −40
Merged

Optimize dynamic config: integrate Zookeeper & Nacos, support interface-level dynamic config #1430

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
528d0fb
integrate Zookeeper and Nacos as configuration centers
Narzisss Jul 21, 2024
853ed3f
support dynamic config at the interface level
Narzisss Sep 23, 2024
92351f2
Optimize interface-level dynamic config
Narzisss Sep 29, 2024
d841de9
Merge branch 'sofastack:master' into master
Narzisss Oct 15, 2024
7a354b3
Modify dynamic config test
Narzisss Oct 15, 2024
b8c60ae
Add DynamicUrl
Narzisss Oct 29, 2024
08a0a85
Optimize config update process
Narzisss Oct 30, 2024
0f613cf
Modify config update process
Narzisss Oct 31, 2024
117b33c
Modify ApolloDynamicConfigManagerTest
Narzisss Oct 31, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 12 additions & 0 deletions all/pom.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -310,6 +310,16 @@
<artifactId>sofa-rpc-config-apollo</artifactId>
<version>${project.version}</version>
</dependency>
<dependency>
<groupId>com.alipay.sofa</groupId>
<artifactId>sofa-rpc-config-zk</artifactId>
<version>${project.version}</version>
</dependency>
<dependency>
<groupId>com.alipay.sofa</groupId>
<artifactId>sofa-rpc-config-nacos</artifactId>
<version>${project.version}</version>
</dependency>
<dependency>
<groupId>com.alipay.sofa</groupId>
<artifactId>bolt</artifactId>
Expand Down Expand Up @@ -565,6 +575,8 @@
<include>com.alipay.sofa:sofa-rpc-tracer-opentracing-resteasy</include>
<include>com.alipay.sofa:sofa-rpc-tracer-opentracing-triple</include>
<include>com.alipay.sofa:sofa-rpc-config-apollo</include>
<include>com.alipay.sofa:sofa-rpc-config-zk</include>
<include>com.alipay.sofa:sofa-rpc-config-nacos</include>
<include>com.alipay.sofa:sofa-rpc-doc-swagger</include>
<!-- TODO -->
</includes>
Expand Down
19 changes: 12 additions & 7 deletions ...ollo/src/test/java/com/alipay/sofa/rpc/dynamic/apollo/ApolloDynamicConfigManagerTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,8 @@
*/
package com.alipay.sofa.rpc.dynamic.apollo;

import com.alipay.sofa.rpc.dynamic.DynamicConfigManager;
import com.alipay.sofa.rpc.dynamic.DynamicConfigManagerFactory;
import com.alipay.sofa.rpc.dynamic.DynamicHelper;
import com.alipay.sofa.rpc.log.Logger;
import com.alipay.sofa.rpc.log.LoggerFactory;
Expand All @@ -24,10 +26,11 @@

public class ApolloDynamicConfigManagerTest {

private final static Logger logger = LoggerFactory
.getLogger(ApolloDynamicConfigManagerTest.class);
private final static Logger logger = LoggerFactory
.getLogger(ApolloDynamicConfigManagerTest.class);

private ApolloDynamicConfigManager apolloDynamicConfigManager = new ApolloDynamicConfigManager("test");
private DynamicConfigManager apolloDynamicConfigManager = DynamicConfigManagerFactory.getDynamicManager("test",
"apollo");

@Test
public void getProviderServiceProperty() {
Expand All @@ -37,17 +40,19 @@ public void getProviderServiceProperty() {

@Test
public void getConsumerServiceProperty() {
String result = apolloDynamicConfigManager.getConsumerServiceProperty("serviceName", "timeout");
Assert.assertEquals(DynamicHelper.DEFAULT_DYNAMIC_VALUE, result);
}

@Test
public void getProviderMethodProperty() {
String result = apolloDynamicConfigManager.getProviderMethodProperty("serviceName", "methodName", "timeout");
Assert.assertEquals(DynamicHelper.DEFAULT_DYNAMIC_VALUE, result);
}

@Test
public void getConsumerMethodProperty() {
}

@Test
public void getServiceAuthRule() {
String result = apolloDynamicConfigManager.getConsumerMethodProperty("serviceName", "methodName", "timeout");
Assert.assertEquals(DynamicHelper.DEFAULT_DYNAMIC_VALUE, result);
Comment on lines +43 to +56
Copy link
Contributor

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Enhance test coverage with additional test scenarios.

The current tests only verify default values. Consider adding:

  1. Tests with actual configuration values
  2. Negative test cases (null/invalid service names, methods)
  3. Tests verifying dynamic config changes
  4. Tests for initialization failures

Example test to add:

@Test
public void testDynamicConfigUpdate() {
    // Setup initial value
    String serviceName = "testService";
    String property = "timeout";
    String initialValue = apolloDynamicConfigManager.getConsumerServiceProperty(serviceName, property);
    
    // Simulate config change
    // TODO: Add mechanism to trigger config change
    
    // Verify updated value
    String updatedValue = apolloDynamicConfigManager.getConsumerServiceProperty(serviceName, property);
    Assert.assertNotEquals("Config should be updated", initialValue, updatedValue);
}

@Test(expected = IllegalArgumentException.class)
public void testInvalidServiceName() {
    apolloDynamicConfigManager.getConsumerServiceProperty(null, "timeout");
}

}
}
107 changes: 107 additions & 0 deletions config/config-nacos/pom.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,107 @@
<?xml version="1.0" encoding="UTF-8"?>
<project xmlns="http://maven.apache.org/POM/4.0.0"
xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
xsi:schemaLocation="http://maven.apache.org/POM/4.0.0 http://maven.apache.org/xsd/maven-4.0.0.xsd">
<modelVersion>4.0.0</modelVersion>

<parent>
<groupId>com.alipay.sofa</groupId>
<artifactId>sofa-rpc-config</artifactId>
<version>${revision}</version>
</parent>

<artifactId>sofa-rpc-config-nacos</artifactId>

<dependencies>
<dependency>
<groupId>com.alipay.sofa</groupId>
<artifactId>sofa-rpc-log-common-tools</artifactId>
</dependency>
<dependency>
<groupId>com.alipay.sofa</groupId>
<artifactId>sofa-rpc-log</artifactId>
</dependency>
<dependency>
<groupId>com.alipay.sofa</groupId>
<artifactId>sofa-rpc-api</artifactId>
</dependency>

<dependency>
<groupId>com.alibaba.nacos</groupId>
<artifactId>nacos-client</artifactId>
</dependency>
<dependency>
<groupId>org.slf4j</groupId>
<artifactId>slf4j-log4j12</artifactId>
<scope>test</scope>
</dependency>
<dependency>
<groupId>junit</groupId>
<artifactId>junit</artifactId>
<scope>test</scope>
</dependency>

</dependencies>

<build>
<sourceDirectory>src/main/java</sourceDirectory>
<resources>
<resource>
<directory>src/main/resources</directory>
<filtering>false</filtering>
<includes>
<include>**/**</include>
</includes>
</resource>
</resources>
<testSourceDirectory>src/test/java</testSourceDirectory>
<testResources>
<testResource>
<directory>src/test/resources</directory>
<filtering>false</filtering>
<includes>
<include>**/**</include>
</includes>
</testResource>
</testResources>

<plugins>
<plugin>
<groupId>org.apache.maven.plugins</groupId>
<artifactId>maven-compiler-plugin</artifactId>
<configuration>
<source>${maven.compiler.source}</source>
<target>${maven.compiler.source}</target>
<encoding>${project.build.sourceEncoding}</encoding>
</configuration>
</plugin>
<plugin>
<groupId>org.apache.maven.plugins</groupId>
<artifactId>maven-install-plugin</artifactId>
<configuration>
<skip>${module.install.skip}</skip>
</configuration>
</plugin>
<plugin>
<groupId>org.apache.maven.plugins</groupId>
<artifactId>maven-deploy-plugin</artifactId>
<configuration>
<skip>${module.deploy.skip}</skip>
</configuration>
</plugin>
<plugin>
<groupId>org.apache.maven.plugins</groupId>
<artifactId>maven-surefire-plugin</artifactId>
<configuration>
<skipTests>${skipTests}</skipTests>
<includes>
<!-- 这里需要根据自己的需要指定要跑的单元测试 -->
<include>**/*Test.java</include>
</includes>
<!-- 如无特殊需求，将forkMode设置为once -->
<forkMode>once</forkMode>
</configuration>
</plugin>
</plugins>
</build>
</project>
129 changes: 129 additions & 0 deletions ...nfig-nacos/src/main/java/com/alipay/sofa/rpc/dynamic/nacos/NacosDynamicConfigManager.java
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,129 @@
/*
* Licensed to the Apache Software Foundation (ASF) under one or more
* contributor license agreements. See the NOTICE file distributed with
* this work for additional information regarding copyright ownership.
* The ASF licenses this file to You under the Apache License, Version 2.0
* (the "License"); you may not use this file except in compliance with
* the License. You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/
package com.alipay.sofa.rpc.dynamic.nacos;

import com.alibaba.nacos.api.PropertyKeyConst;
import com.alipay.sofa.common.config.SofaConfigs;
import com.alipay.sofa.rpc.auth.AuthRuleGroup;
import com.alipay.sofa.rpc.common.config.RpcConfigKeys;
import com.alipay.sofa.rpc.common.utils.StringUtils;
import com.alipay.sofa.rpc.dynamic.DynamicConfigKeyHelper;
import com.alipay.sofa.rpc.dynamic.DynamicConfigManager;
import com.alipay.sofa.rpc.dynamic.DynamicHelper;
import com.alipay.sofa.rpc.ext.Extension;
import com.alibaba.nacos.api.config.ConfigService;
import com.alibaba.nacos.api.exception.NacosException;
import com.alibaba.nacos.api.config.ConfigFactory;
import com.alipay.sofa.rpc.log.Logger;
import com.alipay.sofa.rpc.log.LoggerFactory;

import java.util.Properties;

/**
* @author Narziss
* @version NaocsDynamicConfigManager.java, v 0.1 2024年07月26日 09:37 Narziss
*/

@Extension(value = "nacos", override = true)
public class NacosDynamicConfigManager extends DynamicConfigManager {

private final static Logger LOGGER = LoggerFactory.getLogger(NacosDynamicConfigManager.class);
private static final String DEFAULT_NAMESPACE = "sofa-rpc";
private static final String ADDRESS = SofaConfigs.getOrDefault(RpcConfigKeys.NACOS_ADDRESS);
private static final String DEFAULT_GROUP = "sofa-rpc";
private static final long DEFAULT_TIMEOUT = 5000;
private ConfigService configService;
private Properties nacosConfig = new Properties();
private final String appName;
Copy link
Contributor

Choose a reason for hiding this comment

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

Consider using SofaConfigs#getOrDefault for default values.

The DEFAULT_NAMESPACE and DEFAULT_GROUP values are hardcoded. Consider using SofaConfigs#getOrDefault for these values to allow for configuration flexibility.

- private static final String DEFAULT_NAMESPACE = "sofa-rpc";
- private static final String DEFAULT_GROUP = "sofa-rpc";
+ private static final String DEFAULT_NAMESPACE = SofaConfigs.getOrDefault(RpcConfigKeys.NACOS_NAMESPACE, "sofa-rpc");
+ private static final String DEFAULT_GROUP = SofaConfigs.getOrDefault(RpcConfigKeys.NACOS_GROUP, "sofa-rpc");
Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
@Extension(value = "nacos", override = true)
public class NacosDynamicConfigManager extends DynamicConfigManager {
private final static Logger LOGGER = LoggerFactory.getLogger(NacosDynamicConfigManager.class);
private static final String DEFAULT_NAMESPACE = "sofa-rpc";
private static final String ADDRESS = SofaConfigs.getOrDefault(RpcConfigKeys.NACOS_ADDRESS);
private static final String DEFAULT_GROUP = "sofa-rpc";
private static final long DEFAULT_TIMEOUT = 5000;
private ConfigService configService;
private Properties nacosConfig = new Properties();
private final String appName;
private static final String DEFAULT_NAMESPACE = SofaConfigs.getOrDefault(RpcConfigKeys.NACOS_NAMESPACE, "sofa-rpc");
private static final String DEFAULT_GROUP = SofaConfigs.getOrDefault(RpcConfigKeys.NACOS_GROUP, "sofa-rpc");


protected NacosDynamicConfigManager(String appName) {
super(appName);
if (StringUtils.isEmpty(appName)) {
this.appName = DEFAULT_GROUP;
} else {
this.appName = appName;
}
try {
nacosConfig.put(PropertyKeyConst.SERVER_ADDR, ADDRESS);
nacosConfig.put(PropertyKeyConst.NAMESPACE, DEFAULT_NAMESPACE);
configService = ConfigFactory.createConfigService(nacosConfig);

} catch (NacosException e) {
LOGGER.error("Failed to create ConfigService", e);
}
Copy link
Contributor

Choose a reason for hiding this comment

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

Improve exception handling in the constructor.

The exception handling in the constructor could be improved by providing more context in the log message.

- LOGGER.error("Failed to create ConfigService", e);
+ LOGGER.error("Failed to create ConfigService for appName: " + appName, e);
Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
protected NacosDynamicConfigManager(String appName) {
super(appName);
if (StringUtils.isEmpty(appName)) {
this.appName = DEFAULT_GROUP;
} else {
this.appName = appName;
}
try {
nacosConfig.put(PropertyKeyConst.SERVER_ADDR, ADDRESS);
nacosConfig.put(PropertyKeyConst.NAMESPACE, DEFAULT_NAMESPACE);
configService = ConfigFactory.createConfigService(nacosConfig);
} catch (NacosException e) {
LOGGER.error("Failed to create ConfigService", e);
}
protected NacosDynamicConfigManager(String appName) {
super(appName);
if (StringUtils.isEmpty(appName)) {
this.appName = DEFAULT_GROUP;
} else {
this.appName = appName;
}
try {
nacosConfig.put(PropertyKeyConst.SERVER_ADDR, ADDRESS);
nacosConfig.put(PropertyKeyConst.NAMESPACE, DEFAULT_NAMESPACE);
configService = ConfigFactory.createConfigService(nacosConfig);
} catch (NacosException e) {
LOGGER.error("Failed to create ConfigService for appName: " + appName, e);
}

}

@Override
public void initServiceConfiguration(String service) {
//TODO not now

}
Copy link
Contributor

Choose a reason for hiding this comment

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

Implement service configuration initialization.

The initServiceConfiguration method is currently a placeholder. Ensure to implement the service configuration initialization in the future.

Do you want me to generate the implementation code or open a GitHub issue to track this task?


@Override
public String getProviderServiceProperty(String service, String key) {
try {
String configValue = configService.getConfig(
DynamicConfigKeyHelper.buildProviderServiceProKey(service, key),
appName, DEFAULT_TIMEOUT);
return configValue != null ? configValue : DynamicHelper.DEFAULT_DYNAMIC_VALUE;
} catch (NacosException e) {
return DynamicHelper.DEFAULT_DYNAMIC_VALUE;
}
}

@Override
public String getConsumerServiceProperty(String service, String key) {
try {
String configValue = configService.getConfig(
DynamicConfigKeyHelper.buildConsumerServiceProKey(service, key),
appName, DEFAULT_TIMEOUT);
return configValue != null ? configValue : DynamicHelper.DEFAULT_DYNAMIC_VALUE;
} catch (NacosException e) {
return DynamicHelper.DEFAULT_DYNAMIC_VALUE;
}
}

@Override
public String getProviderMethodProperty(String service, String method, String key) {
try {
String configValue = configService.getConfig(
DynamicConfigKeyHelper.buildProviderMethodProKey(service, method, key),
appName, DEFAULT_TIMEOUT);
return configValue != null ? configValue : DynamicHelper.DEFAULT_DYNAMIC_VALUE;
} catch (NacosException e) {
return DynamicHelper.DEFAULT_DYNAMIC_VALUE;
}
}

@Override
public String getConsumerMethodProperty(String service, String method, String key) {
try {
String configValue = configService.getConfig(
DynamicConfigKeyHelper.buildConsumerMethodProKey(service, method, key),
appName, DEFAULT_TIMEOUT);
return configValue != null ? configValue : DynamicHelper.DEFAULT_DYNAMIC_VALUE;
} catch (NacosException e) {
return DynamicHelper.DEFAULT_DYNAMIC_VALUE;
}
}

@Override
public AuthRuleGroup getServiceAuthRule(String service) {
//TODO 暂不支持
return null;
Comment on lines +140 to +142
Copy link
Contributor

Choose a reason for hiding this comment

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

Implement service authentication rule retrieval.

The getServiceAuthRule method is currently a placeholder. Ensure to implement the service authentication rule retrieval in the future.

Do you want me to generate the implementation code or open a GitHub issue to track this task?

}
Comment on lines +140 to +143
Copy link
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue

Handle unsupported operation in getServiceAuthRule

Similarly, the getServiceAuthRule(String service) method is not implemented. Throwing an UnsupportedOperationException would make it clear to users that this functionality is not available yet.

Apply this diff to handle the unsupported operation:

@Override
public AuthRuleGroup getServiceAuthRule(String service) {
-    //TODO 暂不支持
-    return null;
+    throw new UnsupportedOperationException("getServiceAuthRule is not supported yet.");
}
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
public AuthRuleGroup getServiceAuthRule(String service) {
//TODO 暂不支持
return null;
}
public AuthRuleGroup getServiceAuthRule(String service) {
throw new UnsupportedOperationException("getServiceAuthRule is not supported yet.");
}

}
2 changes: 2 additions & 0 deletions ...ain/resources/META-INF/services/sofa-rpc/com.alipay.sofa.rpc.dynamic.DynamicConfigManager
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
nacos=com.alipay.sofa.rpc.dynamic.nacos.NacosDynamicConfigManager

58 changes: 58 additions & 0 deletions ...-nacos/src/test/java/com/alipay/sofa/rpc/dynamic/nacos/NacosDynamicConfigManagerTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,58 @@
/*
* Licensed to the Apache Software Foundation (ASF) under one or more
* contributor license agreements. See the NOTICE file distributed with
* this work for additional information regarding copyright ownership.
* The ASF licenses this file to You under the Apache License, Version 2.0
* (the "License"); you may not use this file except in compliance with
* the License. You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/
package com.alipay.sofa.rpc.dynamic.nacos;

import com.alipay.sofa.rpc.dynamic.DynamicConfigManager;
import com.alipay.sofa.rpc.dynamic.DynamicConfigManagerFactory;
import com.alipay.sofa.rpc.dynamic.DynamicHelper;
import com.alipay.sofa.rpc.log.Logger;
import com.alipay.sofa.rpc.log.LoggerFactory;
import org.junit.Assert;
import org.junit.Test;

public class NacosDynamicConfigManagerTest {

private final static Logger logger = LoggerFactory
.getLogger(NacosDynamicConfigManagerTest.class);

private DynamicConfigManager nacosDynamicConfigManager = DynamicConfigManagerFactory.getDynamicManager("test",
"nacos");

@Test
public void getProviderServiceProperty() {
String result = nacosDynamicConfigManager.getProviderServiceProperty("serviceName", "timeout");
Assert.assertEquals(DynamicHelper.DEFAULT_DYNAMIC_VALUE, result);
}

@Test
public void getConsumerServiceProperty() {
String result = nacosDynamicConfigManager.getConsumerServiceProperty("serviceName", "timeout");
Assert.assertEquals(DynamicHelper.DEFAULT_DYNAMIC_VALUE, result);
}

@Test
public void getProviderMethodProperty() {
String result = nacosDynamicConfigManager.getProviderMethodProperty("serviceName", "methodName", "timeout");
Assert.assertEquals(DynamicHelper.DEFAULT_DYNAMIC_VALUE, result);
}

@Test
public void getConsumerMethodProperty() {
String result = nacosDynamicConfigManager.getConsumerMethodProperty("serviceName", "methodName", "timeout");
Assert.assertEquals(DynamicHelper.DEFAULT_DYNAMIC_VALUE, result);
}
}
16 changes: 16 additions & 0 deletions config/config-nacos/src/test/resources/log4j.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
<?xml version="1.0" encoding="UTF-8"?>
<!DOCTYPE log4j:configuration SYSTEM "log4j.dtd">
<log4j:configuration xmlns:log4j="http://jakarta.apache.org/log4j/" debug="false">

<appender name="CONSOLE" class="org.apache.log4j.ConsoleAppender">
<layout class="org.apache.log4j.PatternLayout">
<param name="ConversionPattern" value="%d %t %5p [%c:%M:%L] - %m%n"/>
</layout>
</appender>

<root>
<level value="INFO"/>
<appender-ref ref="CONSOLE"/>
</root>

</log4j:configuration>
Loading
Loading