Skip to content

Commit

Permalink
Support to use empty string to override existing config. (apache#4635)
Browse files Browse the repository at this point in the history
  • Loading branch information
@wu-sheng
wu-sheng authored Apr 11, 2020
1 parent 44cae97 commit 3138515
Show file tree
Hide file tree
Showing 4 changed files with 58 additions and 60 deletions.
22 changes: 14 additions & 8 deletions ...mons/apm-util/src/main/java/org/apache/skywalking/apm/util/PropertyPlaceholderHelper.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,11 @@
*/
public enum PropertyPlaceholderHelper {

INSTANCE(PlaceholderConfigurerSupport.DEFAULT_PLACEHOLDER_PREFIX, PlaceholderConfigurerSupport.DEFAULT_PLACEHOLDER_SUFFIX, PlaceholderConfigurerSupport.DEFAULT_VALUE_SEPARATOR, true);
INSTANCE(
PlaceholderConfigurerSupport.DEFAULT_PLACEHOLDER_PREFIX,
PlaceholderConfigurerSupport.DEFAULT_PLACEHOLDER_SUFFIX, PlaceholderConfigurerSupport.DEFAULT_VALUE_SEPARATOR,
true
);

private final String placeholderPrefix;

Expand All @@ -54,7 +58,7 @@ public enum PropertyPlaceholderHelper {
* true}) or cause an exception ({@code false})
*/
PropertyPlaceholderHelper(String placeholderPrefix, String placeholderSuffix, String valueSeparator,
boolean ignoreUnresolvablePlaceholders) {
boolean ignoreUnresolvablePlaceholders) {
if (StringUtil.isEmpty(placeholderPrefix) || StringUtil.isEmpty(placeholderSuffix)) {
throw new UnsupportedOperationException("'placeholderPrefix or placeholderSuffix' must not be null");
}
Expand Down Expand Up @@ -89,17 +93,17 @@ public String replacePlaceholders(String value, final Properties properties) {
return replacePlaceholders(value, new PlaceholderResolver() {
@Override
public String resolvePlaceholder(String placeholderName) {
return PropertyPlaceholderHelper.this.getConfigValue(placeholderName, properties);
return getConfigValue(placeholderName, properties);
}
});
}

private String getConfigValue(String key, final Properties properties) {
String value = System.getProperty(key);
if (StringUtil.isEmpty(value)) {
if (value == null) {
value = System.getenv(key);
}
if (StringUtil.isEmpty(value)) {
if (value == null) {
value = properties.getProperty(key);
}
return value;
Expand All @@ -118,7 +122,7 @@ public String replacePlaceholders(String value, PlaceholderResolver placeholderR
}

protected String parseStringValue(String value, PlaceholderResolver placeholderResolver,
Set<String> visitedPlaceholders) {
Set<String> visitedPlaceholders) {

StringBuilder result = new StringBuilder(value);

Expand All @@ -129,7 +133,8 @@ protected String parseStringValue(String value, PlaceholderResolver placeholderR
String placeholder = result.substring(startIndex + this.placeholderPrefix.length(), endIndex);
String originalPlaceholder = placeholder;
if (!visitedPlaceholders.add(originalPlaceholder)) {
throw new IllegalArgumentException("Circular placeholder reference '" + originalPlaceholder + "' in property definitions");
throw new IllegalArgumentException(
"Circular placeholder reference '" + originalPlaceholder + "' in property definitions");
}
// Recursive invocation, parsing placeholders contained in the placeholder key.
placeholder = parseStringValue(placeholder, placeholderResolver, visitedPlaceholders);
Expand All @@ -156,7 +161,8 @@ protected String parseStringValue(String value, PlaceholderResolver placeholderR
// Proceed with unprocessed value.
startIndex = result.indexOf(this.placeholderPrefix, endIndex + this.placeholderSuffix.length());
} else {
throw new IllegalArgumentException("Could not resolve placeholder '" + placeholder + "'" + " in value \"" + value + "\"");
throw new IllegalArgumentException(
"Could not resolve placeholder '" + placeholder + "'" + " in value \"" + value + "\"");
}
visitedPlaceholders.remove(originalPlaceholder);
} else {
Expand Down
57 changes: 40 additions & 17 deletions ...rc/main/java/org/apache/skywalking/oap/server/starter/config/ApplicationConfigLoader.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -24,13 +24,12 @@
import java.util.Map;
import java.util.Properties;
import java.util.Set;
import lombok.extern.slf4j.Slf4j;
import org.apache.skywalking.apm.util.PropertyPlaceholderHelper;
import org.apache.skywalking.oap.server.library.module.ApplicationConfiguration;
import org.apache.skywalking.oap.server.library.module.ProviderNotFoundException;
import org.apache.skywalking.oap.server.library.util.CollectionUtils;
import org.apache.skywalking.oap.server.library.util.ResourceUtils;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
import org.yaml.snakeyaml.Yaml;

/**
Expand All @@ -39,10 +38,8 @@
* <p>
* At last, override setting by system.properties and system.envs if the key matches moduleName.provideName.settingKey.
*/
@Slf4j
public class ApplicationConfigLoader implements ConfigLoader<ApplicationConfiguration> {

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

private static final String DISABLE_SELECTOR = "-";
private static final String SELECTOR = "selector";

Expand All @@ -65,10 +62,14 @@ private void loadConfig(ApplicationConfiguration configuration) throws ConfigFil
selectConfig(moduleConfig);
moduleConfig.forEach((moduleName, providerConfig) -> {
if (providerConfig.size() > 0) {
logger.info("Get a module define from application.yml, module name: {}", moduleName);
ApplicationConfiguration.ModuleConfiguration moduleConfiguration = configuration.addModule(moduleName);
log.info("Get a module define from application.yml, module name: {}", moduleName);
ApplicationConfiguration.ModuleConfiguration moduleConfiguration = configuration.addModule(
moduleName);
providerConfig.forEach((providerName, config) -> {
logger.info("Get a provider define belong to {} module, provider name: {}", moduleName, providerName);
log.info(
"Get a provider define belong to {} module, provider name: {}", moduleName,
providerName
);
final Map<String, ?> propertiesConfig = (Map<String, ?>) config;
final Properties properties = new Properties();
if (propertiesConfig != null) {
Expand All @@ -89,7 +90,10 @@ private void loadConfig(ApplicationConfiguration configuration) throws ConfigFil
moduleConfiguration.addProviderConfiguration(providerName, properties);
});
} else {
logger.warn("Get a module define from application.yml, but no provider define, use default, module name: {}", moduleName);
log.warn(
"Get a module define from application.yml, but no provider define, use default, module name: {}",
moduleName
);
}
});
}
Expand All @@ -99,11 +103,26 @@ private void loadConfig(ApplicationConfiguration configuration) throws ConfigFil
}

private void replacePropertyAndLog(final Object propertyName, final Object propertyValue, final Properties target,
final Object providerName) {
final Object replaceValue = yaml.load(PropertyPlaceholderHelper.INSTANCE.replacePlaceholders(propertyValue + "", target));
if (replaceValue != null) {
target.replace(propertyName, replaceValue);
logger.info("The property with key: {}, value: {}, in {} provider", propertyName, replaceValue.toString(), providerName);
final Object providerName) {
final String valueString = PropertyPlaceholderHelper.INSTANCE
.replacePlaceholders(propertyValue + "", target);
if (valueString != null) {
if (valueString.trim().length() == 0) {
target.replace(propertyName, valueString);
log.info("Provider={} config={} has been set as an empty string", providerName, propertyName);
} else {
// Use YAML to do data type conversion.
final Object replaceValue = yaml.load(valueString);
if (replaceValue != null) {
target.replace(propertyName, replaceValue);
log.info(
"Provider={} config={} has been set as {}",
providerName,
propertyName,
replaceValue.toString()
);
}
}
}
}

Expand Down Expand Up @@ -148,7 +167,7 @@ private void selectConfig(final Map<String, Map<String, Object>> moduleConfigura
final boolean shouldBeRemoved = modulesWithoutProvider.contains(module);

if (shouldBeRemoved) {
logger.info("Remove module {} without any provider", module);
log.info("Remove module {} without any provider", module);
}

return shouldBeRemoved;
Expand All @@ -162,7 +181,8 @@ private void overrideModuleSettings(ApplicationConfiguration configuration, Stri
}
String moduleName = key.substring(0, moduleAndConfigSeparator);
String providerSettingSubKey = key.substring(moduleAndConfigSeparator + 1);
ApplicationConfiguration.ModuleConfiguration moduleConfiguration = configuration.getModuleConfiguration(moduleName);
ApplicationConfiguration.ModuleConfiguration moduleConfiguration = configuration.getModuleConfiguration(
moduleName);
if (moduleConfiguration == null) {
return;
}
Expand Down Expand Up @@ -193,6 +213,9 @@ else if (type.equals(boolean.class) || type.equals(Boolean.class)) {
return;
}

logger.info("The setting has been override by key: {}, value: {}, in {} provider of {} module through {}", settingKey, value, providerName, moduleName, "System.properties");
log.info(
"The setting has been override by key: {}, value: {}, in {} provider of {} module through {}", settingKey,
value, providerName, moduleName, "System.properties"
);
}
}
8 changes: 4 additions & 4 deletions oap-server/server-bootstrap/src/main/resources/application.yml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -92,8 +92,8 @@ storage:
nameSpace: ${SW_NAMESPACE:""}
clusterNodes: ${SW_STORAGE_ES_CLUSTER_NODES:localhost:9200}
protocol: ${SW_STORAGE_ES_HTTP_PROTOCOL:"http"}
trustStorePath: ${SW_SW_STORAGE_ES_SSL_JKS_PATH:""}
trustStorePass: ${SW_SW_STORAGE_ES_SSL_JKS_PASS:""}
trustStorePath: ${SW_STORAGE_ES_SSL_JKS_PATH:""}
trustStorePass: ${SW_STORAGE_ES_SSL_JKS_PASS:""}
user: ${SW_ES_USER:""}
password: ${SW_ES_PASSWORD:""}
secretsManagementFile: ${SW_ES_SECRETS_MANAGEMENT_FILE:""} # Secrets management file in the properties format includes the username, password, which are managed by 3rd party tool.
Expand All @@ -113,8 +113,8 @@ storage:
nameSpace: ${SW_NAMESPACE:""}
clusterNodes: ${SW_STORAGE_ES_CLUSTER_NODES:localhost:9200}
protocol: ${SW_STORAGE_ES_HTTP_PROTOCOL:"http"}
trustStorePath: ${SW_SW_STORAGE_ES_SSL_JKS_PATH:"../es_keystore.jks"}
trustStorePass: ${SW_SW_STORAGE_ES_SSL_JKS_PASS:""}
trustStorePath: ${SW_STORAGE_ES_SSL_JKS_PATH:""}
trustStorePass: ${SW_STORAGE_ES_SSL_JKS_PASS:""}
dayStep: ${SW_STORAGE_DAY_STEP:1} # Represent the number of days in the one minute/hour/day index.
user: ${SW_ES_USER:""}
password: ${SW_ES_PASSWORD:""}
Expand Down
31 changes: 0 additions & 31 deletions .../skywalking/oap/server/storage/plugin/elasticsearch/StorageModuleElasticsearchConfig.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -89,37 +89,6 @@ public class StorageModuleElasticsearchConfig extends ModuleConfig {
@Setter
private int profileTaskQueryMaxSize = 200;
@Setter
private int recordDataTTL = 7;
@Setter
private int minuteMetricsDataTTL = 2;
@Setter
private int hourMetricsDataTTL = 2;
@Setter
private int dayMetricsDataTTL = 2;
private int otherMetricsDataTTL = 0;
@Setter
private int monthMetricsDataTTL = 18;
@Setter
private String advanced;

public int getMinuteMetricsDataTTL() {
if (otherMetricsDataTTL > 0) {
return otherMetricsDataTTL;
}
return minuteMetricsDataTTL;
}

public int getHourMetricsDataTTL() {
if (otherMetricsDataTTL > 0) {
return otherMetricsDataTTL;
}
return hourMetricsDataTTL;
}

public int getDayMetricsDataTTL() {
if (otherMetricsDataTTL > 0) {
return otherMetricsDataTTL;
}
return dayMetricsDataTTL;
}
}

0 comments on commit 3138515

Please sign in to comment.