Fix dynamic configuration watch implementation current value not null when the config is deleted. (#7606)

* Fix dynamic configuration watch implementation current value not null when the config is deleted. * add changes.md * fix LoggingConfigWatcher watch.vale, and when value = null ,config not reset. * fix * fix style * Update oap-server/analyzer/agent-analyzer/src/main/java/org/apache/skywalking/oap/server/analyzer/provider/trace/UninstrumentedGatewaysConfig.java Co-authored-by: kezhenxu94 <kezhenxu94@apache.org> * use Strings.nullToEmpty Co-authored-by: 吴晟 Wu Sheng <wu.sheng@foxmail.com> Co-authored-by: kezhenxu94 <kezhenxu94@apache.org>
2021-08-30 18:20:25 +08:00 · 2021-08-30 18:20:25 +08:00 · eb7fae2ef2
parent 040115daa1
commit eb7fae2ef2
7 changed files with 24 additions and 26 deletions
--- a/CHANGES.md
+++ b/CHANGES.md
@ -40,7 +40,14 @@ Release Notes.
 * Fix NPE when OAP nodes synchronize events with each other in cluster mode.
 * Support k8s configmap grouped dynamic configurations.
 * Add desc sort function in H2 and ElasticSearch implementations of IBrowserLogQueryDAO
-* Support configure sampling policy by `configuration module` dynamically and static configuration file `trace-sampling-policy-settings.yml` for service dimension on the backend side. Dynamic configurations `agent-analyzer.default.sampleRate` and `agent-analyzer.default.slowTraceSegmentThreshold` are replaced by `agent-analyzer.default.traceSamplingPolicy`. Static configurations `agent-analyzer.default.sampleRate` and `agent-analyzer.default.slowTraceSegmentThreshold` are replaced by `agent-analyzer.default.traceSamplingPolicySettingsFile`.
+* Support configure sampling policy by `configuration module` dynamically and static configuration
+  file `trace-sampling-policy-settings.yml` for service dimension on the backend side. Dynamic
+  configurations `agent-analyzer.default.sampleRate` and `agent-analyzer.default.slowTraceSegmentThreshold` are replaced
+  by `agent-analyzer.default.traceSamplingPolicy`. Static configurations `agent-analyzer.default.sampleRate`
+  and `agent-analyzer.default.slowTraceSegmentThreshold` are replaced
+  by `agent-analyzer.default.traceSamplingPolicySettingsFile`.
+* Fix dynamic configuration watch implementation current value not null when the config is deleted.
+* Fix `LoggingConfigWatcher` return `watch.value` would not consistent with the real configuration content.

 #### UI

--- a/oap-server/analyzer/agent-analyzer/src/main/java/org/apache/skywalking/oap/server/analyzer/provider/trace/UninstrumentedGatewaysConfig.java
+++ b/oap-server/analyzer/agent-analyzer/src/main/java/org/apache/skywalking/oap/server/analyzer/provider/trace/UninstrumentedGatewaysConfig.java
@ -17,6 +17,7 @@

 package org.apache.skywalking.oap.server.analyzer.provider.trace;

+import com.google.common.base.Strings;
 import java.io.FileNotFoundException;
 import java.io.Reader;
 import java.util.ArrayList;
@ -35,7 +36,6 @@ import lombok.ToString;
 import lombok.extern.slf4j.Slf4j;
 import org.apache.skywalking.oap.server.analyzer.module.AnalyzerModule;
 import org.apache.skywalking.oap.server.configuration.api.ConfigChangeWatcher;
-import org.apache.skywalking.oap.server.core.Const;
 import org.apache.skywalking.oap.server.library.module.ModuleProvider;
 import org.apache.skywalking.oap.server.library.util.ResourceUtils;
 import org.apache.skywalking.oap.server.library.util.yaml.ClassFilterConstructor;
@ -51,7 +51,7 @@ public class UninstrumentedGatewaysConfig extends ConfigChangeWatcher {

    public UninstrumentedGatewaysConfig(ModuleProvider provider) {
        super(AnalyzerModule.NAME, provider, "uninstrumentedGateways");
-        this.settingsString = new AtomicReference<>(Const.EMPTY_STRING);
+        this.settingsString = new AtomicReference<>(null);
        final GatewayInfos defaultGateways = parseGatewaysFromFile("gateways.yml");
        log.info("Default configured gateways: {}", defaultGateways);
        onGatewaysUpdated(defaultGateways);
@ -62,13 +62,13 @@ public class UninstrumentedGatewaysConfig extends ConfigChangeWatcher {
            log.debug("Updating using new static config: {}", config);
        }
        this.settingsString.set(config);
-        onGatewaysUpdated(parseGatewaysFromYml(config));
+        onGatewaysUpdated(parseGatewaysFromYml(Strings.nullToEmpty(config)));
    }

    @Override
    public void notify(ConfigChangeEvent value) {
        if (EventType.DELETE.equals(value.getEventType())) {
-            activeSetting("");
+            activeSetting(null);
        } else {
            activeSetting(value.getNewValue());
        }
--- a/oap-server/server-alarm-plugin/src/main/java/org/apache/skywalking/oap/server/core/alarm/provider/AlarmRulesWatcher.java
+++ b/oap-server/server-alarm-plugin/src/main/java/org/apache/skywalking/oap/server/core/alarm/provider/AlarmRulesWatcher.java
@ -26,7 +26,6 @@ import java.util.Map;
 import lombok.Getter;
 import lombok.extern.slf4j.Slf4j;
 import org.apache.skywalking.oap.server.configuration.api.ConfigChangeWatcher;
-import org.apache.skywalking.oap.server.core.Const;
 import org.apache.skywalking.oap.server.core.alarm.AlarmModule;
 import org.apache.skywalking.oap.server.core.alarm.provider.dingtalk.DingtalkSettings;
 import org.apache.skywalking.oap.server.core.alarm.provider.expression.Expression;
@ -58,7 +57,7 @@ public class AlarmRulesWatcher extends ConfigChangeWatcher {
        super(AlarmModule.NAME, provider, "alarm-settings");
        this.runningContext = new HashMap<>();
        this.alarmRuleRunningRuleMap = new HashMap<>();
-        this.settingsString = Const.EMPTY_STRING;
+        this.settingsString = null;
        Expression expression = new Expression(new ExpressionContext());
        this.compositeRuleEvaluator = new CompositeRuleEvaluator(expression);
        notify(defaultRules);
@ -67,7 +66,7 @@ public class AlarmRulesWatcher extends ConfigChangeWatcher {
    @Override
    public void notify(ConfigChangeEvent value) {
        if (value.getEventType().equals(EventType.DELETE)) {
-            settingsString = Const.EMPTY_STRING;
+            settingsString = null;
            notify(new Rules());
        } else {
            settingsString = value.getNewValue();
--- a/oap-server/server-core/src/main/java/org/apache/skywalking/oap/server/core/analysis/ApdexThresholdConfig.java
+++ b/oap-server/server-core/src/main/java/org/apache/skywalking/oap/server/core/analysis/ApdexThresholdConfig.java
@ -18,6 +18,7 @@

 package org.apache.skywalking.oap.server.core.analysis;

+import com.google.common.base.Strings;
 import java.io.FileNotFoundException;
 import java.io.Reader;
 import java.io.StringReader;
@ -25,7 +26,6 @@ import java.util.Collections;
 import java.util.Map;
 import lombok.extern.slf4j.Slf4j;
 import org.apache.skywalking.oap.server.configuration.api.ConfigChangeWatcher;
-import org.apache.skywalking.oap.server.core.Const;
 import org.apache.skywalking.oap.server.core.CoreModule;
 import org.apache.skywalking.oap.server.core.CoreModuleProvider;
 import org.apache.skywalking.oap.server.library.util.ResourceUtils;
@ -44,7 +44,7 @@ public class ApdexThresholdConfig extends ConfigChangeWatcher implements Configu

    private Map<String, Integer> dictionary = Collections.emptyMap();

-    private String rawConfig = Const.EMPTY_STRING;
+    private String rawConfig = null;

    public ApdexThresholdConfig(final CoreModuleProvider provider) {
        super(CoreModule.NAME, provider, "apdexThreshold");
@ -74,7 +74,7 @@ public class ApdexThresholdConfig extends ConfigChangeWatcher implements Configu
    @Override
    public void notify(ConfigChangeEvent value) {
        if (EventType.DELETE.equals(value.getEventType())) {
-            activeSetting("");
+            activeSetting(null);
        } else {
            activeSetting(value.getNewValue());
        }
@ -90,7 +90,7 @@ public class ApdexThresholdConfig extends ConfigChangeWatcher implements Configu
            log.debug("Updating using new static config: {}", config);
        }
        rawConfig = config;
-        updateConfig(new StringReader(config));
+        updateConfig(new StringReader(Strings.nullToEmpty(config)));
    }

    @SuppressWarnings("unchecked")
--- a/oap-server/server-core/src/main/java/org/apache/skywalking/oap/server/core/config/group/EndpointNameGroupingRuleWatcher.java
+++ b/oap-server/server-core/src/main/java/org/apache/skywalking/oap/server/core/config/group/EndpointNameGroupingRuleWatcher.java
@ -47,7 +47,7 @@ public class EndpointNameGroupingRuleWatcher extends ConfigChangeWatcher {
    @Override
    public void notify(final ConfigChangeEvent value) {
        if (value.getEventType().equals(EventType.DELETE)) {
-            ruleSetting = "";
+            ruleSetting = null;
            grouping.setEndpointGroupingRule(new EndpointGroupingRule());
        } else {
            ruleSetting = value.getNewValue();
--- a/oap-server/server-core/src/main/java/org/apache/skywalking/oap/server/core/logging/LoggingConfigWatcher.java
+++ b/oap-server/server-core/src/main/java/org/apache/skywalking/oap/server/core/logging/LoggingConfigWatcher.java
@ -50,9 +50,10 @@ public class LoggingConfigWatcher extends ConfigChangeWatcher {
    public void notify(final ConfigChangeEvent value) {
        String newValue;
        if (EventType.DELETE.equals(value.getEventType())) {
-            this.content = "";
-            newValue = "";
+            this.content = null;
+            newValue = null;
        } else {
+            this.content = value.getNewValue();
            newValue = value.getNewValue();
        }
        try {
@ -63,14 +64,6 @@ public class LoggingConfigWatcher extends ConfigChangeWatcher {
            log.error("failed to apply configuration to log4j", t);
            return;
        }
-        StringBuilder builder = new StringBuilder();
-        ctx.getConfiguration().getLoggers().forEach((loggerName, config) -> {
-            builder.append(Strings.isNullOrEmpty(loggerName) ? "Root" : loggerName)
-                   .append(":")
-                   .append(config.getLevel())
-                   .append(",");
-        });
-        this.content = builder.toString();
    }

    @Override
--- a/oap-server/server-receiver-plugin/configuration-discovery-receiver-plugin/src/main/java/org/apache/skywalking/oap/server/recevier/configuration/discovery/AgentConfigurationsWatcher.java
+++ b/oap-server/server-receiver-plugin/configuration-discovery-receiver-plugin/src/main/java/org/apache/skywalking/oap/server/recevier/configuration/discovery/AgentConfigurationsWatcher.java
@ -20,7 +20,6 @@ package org.apache.skywalking.oap.server.recevier.configuration.discovery;

 import org.apache.commons.codec.digest.DigestUtils;
 import org.apache.skywalking.oap.server.configuration.api.ConfigChangeWatcher;
-import org.apache.skywalking.oap.server.core.Const;
 import org.apache.skywalking.oap.server.library.module.ModuleProvider;

 import java.io.StringReader;
@ -36,7 +35,7 @@ public class AgentConfigurationsWatcher extends ConfigChangeWatcher {

    public AgentConfigurationsWatcher(ModuleProvider provider) {
        super(ConfigurationDiscoveryModule.NAME, provider, "agentConfigurations");
-        this.settingsString = Const.EMPTY_STRING;
+        this.settingsString = null;
        this.agentConfigurationsTable = new AgentConfigurationsTable();
        this.emptyAgentConfigurations = new AgentConfigurations(
            null, new HashMap<>(), DigestUtils.sha512Hex("EMPTY")
@ -46,7 +45,7 @@ public class AgentConfigurationsWatcher extends ConfigChangeWatcher {
    @Override
    public void notify(ConfigChangeEvent value) {
        if (value.getEventType().equals(EventType.DELETE)) {
-            settingsString = Const.EMPTY_STRING;
+            settingsString = null;
            this.agentConfigurationsTable = new AgentConfigurationsTable();
        } else {
            settingsString = value.getNewValue();