This is an automated email from the ASF dual-hosted git repository.

wusheng pushed a commit to branch bug
in repository https://gitbox.apache.org/repos/asf/skywalking.git

commit 6b6093c645b55423bf0355f54fe3a5514efdee60
Author: Wu Sheng <[email protected]>
AuthorDate: Fri Apr 10 22:55:13 2020 +0800

    Support to use empty string to override existing config.
---
 .../apm/util/PropertyPlaceholderHelper.java        | 22 ++++++---
 .../starter/config/ApplicationConfigLoader.java    | 57 +++++++++++++++-------
 .../src/main/resources/application.yml             |  8 +--
 .../StorageModuleElasticsearchConfig.java          | 31 ------------
 4 files changed, 58 insertions(+), 60 deletions(-)

diff --git 
a/apm-commons/apm-util/src/main/java/org/apache/skywalking/apm/util/PropertyPlaceholderHelper.java
 
b/apm-commons/apm-util/src/main/java/org/apache/skywalking/apm/util/PropertyPlaceholderHelper.java
index 4e985ba..c12661c 100644
--- 
a/apm-commons/apm-util/src/main/java/org/apache/skywalking/apm/util/PropertyPlaceholderHelper.java
+++ 
b/apm-commons/apm-util/src/main/java/org/apache/skywalking/apm/util/PropertyPlaceholderHelper.java
@@ -31,7 +31,11 @@ import java.util.Set;
  */
 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;
 
@@ -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");
         }
@@ -89,17 +93,17 @@ public enum PropertyPlaceholderHelper {
         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;
@@ -118,7 +122,7 @@ public enum PropertyPlaceholderHelper {
     }
 
     protected String parseStringValue(String value, PlaceholderResolver 
placeholderResolver,
-        Set<String> visitedPlaceholders) {
+                                      Set<String> visitedPlaceholders) {
 
         StringBuilder result = new StringBuilder(value);
 
@@ -129,7 +133,8 @@ public enum PropertyPlaceholderHelper {
                 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);
@@ -156,7 +161,8 @@ public enum PropertyPlaceholderHelper {
                     // 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 {
diff --git 
a/oap-server/server-bootstrap/src/main/java/org/apache/skywalking/oap/server/starter/config/ApplicationConfigLoader.java
 
b/oap-server/server-bootstrap/src/main/java/org/apache/skywalking/oap/server/starter/config/ApplicationConfigLoader.java
index cd612f3..31c874d 100644
--- 
a/oap-server/server-bootstrap/src/main/java/org/apache/skywalking/oap/server/starter/config/ApplicationConfigLoader.java
+++ 
b/oap-server/server-bootstrap/src/main/java/org/apache/skywalking/oap/server/starter/config/ApplicationConfigLoader.java
@@ -24,13 +24,12 @@ import java.util.HashSet;
 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;
 
 /**
@@ -39,10 +38,8 @@ import org.yaml.snakeyaml.Yaml;
  * <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";
 
@@ -65,10 +62,14 @@ public class ApplicationConfigLoader implements 
ConfigLoader<ApplicationConfigur
                 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) {
@@ -89,7 +90,10 @@ public class ApplicationConfigLoader implements 
ConfigLoader<ApplicationConfigur
                             
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
+                        );
                     }
                 });
             }
@@ -99,11 +103,26 @@ public class ApplicationConfigLoader implements 
ConfigLoader<ApplicationConfigur
     }
 
     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()
+                    );
+                }
+            }
         }
     }
 
@@ -148,7 +167,7 @@ public class ApplicationConfigLoader implements 
ConfigLoader<ApplicationConfigur
             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;
@@ -162,7 +181,8 @@ public class ApplicationConfigLoader implements 
ConfigLoader<ApplicationConfigur
         }
         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;
         }
@@ -193,6 +213,9 @@ public class ApplicationConfigLoader implements 
ConfigLoader<ApplicationConfigur
             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"
+        );
     }
 }
diff --git a/oap-server/server-bootstrap/src/main/resources/application.yml 
b/oap-server/server-bootstrap/src/main/resources/application.yml
index baa1f81..4e715ac 100755
--- a/oap-server/server-bootstrap/src/main/resources/application.yml
+++ b/oap-server/server-bootstrap/src/main/resources/application.yml
@@ -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.
@@ -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:""}
diff --git 
a/oap-server/server-storage-plugin/storage-elasticsearch-plugin/src/main/java/org/apache/skywalking/oap/server/storage/plugin/elasticsearch/StorageModuleElasticsearchConfig.java
 
b/oap-server/server-storage-plugin/storage-elasticsearch-plugin/src/main/java/org/apache/skywalking/oap/server/storage/plugin/elasticsearch/StorageModuleElasticsearchConfig.java
index 4325212..39a6325 100644
--- 
a/oap-server/server-storage-plugin/storage-elasticsearch-plugin/src/main/java/org/apache/skywalking/oap/server/storage/plugin/elasticsearch/StorageModuleElasticsearchConfig.java
+++ 
b/oap-server/server-storage-plugin/storage-elasticsearch-plugin/src/main/java/org/apache/skywalking/oap/server/storage/plugin/elasticsearch/StorageModuleElasticsearchConfig.java
@@ -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;
-    }
 }

Reply via email to