diff --git a/core/src/main/java/org/elasticsearch/common/logging/ESLoggerFactory.java b/core/src/main/java/org/elasticsearch/common/logging/ESLoggerFactory.java index a0645c6a4d3..4fdde3db895 100644 --- a/core/src/main/java/org/elasticsearch/common/logging/ESLoggerFactory.java +++ b/core/src/main/java/org/elasticsearch/common/logging/ESLoggerFactory.java @@ -22,12 +22,23 @@ package org.elasticsearch.common.logging; import org.elasticsearch.common.logging.jdk.JdkESLoggerFactory; import org.elasticsearch.common.logging.log4j.Log4jESLoggerFactory; import org.elasticsearch.common.logging.slf4j.Slf4jESLoggerFactory; +import org.elasticsearch.common.settings.AbstractScopedSettings; +import org.elasticsearch.common.settings.Setting; +import org.elasticsearch.common.settings.Settings; + +import java.util.Locale; +import java.util.Map; +import java.util.function.Consumer; +import java.util.regex.Pattern; /** * Factory to get {@link ESLogger}s */ public abstract class ESLoggerFactory { + public static final Setting LOG_DEFAULT_LEVEL_SETTING = new Setting<>("logger.level", LogLevel.INFO.name(), LogLevel::parse, false, Setting.Scope.CLUSTER); + public static final Setting LOG_LEVEL_SETTING = Setting.dynamicKeySetting("logger.", LogLevel.INFO.name(), LogLevel::parse, true, Setting.Scope.CLUSTER); + private static volatile ESLoggerFactory defaultFactory = new JdkESLoggerFactory(); static { @@ -85,4 +96,11 @@ public abstract class ESLoggerFactory { protected abstract ESLogger rootLogger(); protected abstract ESLogger newInstance(String prefix, String name); + + public enum LogLevel { + WARN, TRACE, INFO, DEBUG, ERROR; + public static LogLevel parse(String level) { + return valueOf(level.toUpperCase(Locale.ROOT)); + } + } } diff --git a/core/src/main/java/org/elasticsearch/common/settings/AbstractScopedSettings.java b/core/src/main/java/org/elasticsearch/common/settings/AbstractScopedSettings.java index 65db6d155d3..b30178857e1 100644 --- a/core/src/main/java/org/elasticsearch/common/settings/AbstractScopedSettings.java +++ b/core/src/main/java/org/elasticsearch/common/settings/AbstractScopedSettings.java @@ -166,7 +166,11 @@ public abstract class AbstractScopedSettings extends AbstractComponent { if (setting != get(setting.getKey())) { throw new IllegalArgumentException("Setting is not registered for key [" + setting.getKey() + "]"); } - this.settingUpdaters.add(setting.newUpdater(consumer, logger, validator)); + addSettingsUpdater(setting.newUpdater(consumer, logger, validator)); + } + + synchronized void addSettingsUpdater(SettingUpdater updater) { + this.settingUpdaters.add(updater); } /** @@ -184,7 +188,7 @@ public abstract class AbstractScopedSettings extends AbstractComponent { if (b != get(b.getKey())) { throw new IllegalArgumentException("Setting is not registered for key [" + b.getKey() + "]"); } - this.settingUpdaters.add(Setting.compoundUpdater(consumer, a, b, logger)); + addSettingsUpdater(Setting.compoundUpdater(consumer, a, b, logger)); } /** @@ -288,7 +292,7 @@ public abstract class AbstractScopedSettings extends AbstractComponent { } for (Map.Entry> entry : complexMatchers.entrySet()) { if (entry.getValue().match(key)) { - return entry.getValue(); + return entry.getValue().getConcreteSetting(key); } } return null; diff --git a/core/src/main/java/org/elasticsearch/common/settings/ClusterSettings.java b/core/src/main/java/org/elasticsearch/common/settings/ClusterSettings.java index 58196549575..e66afc0c49b 100644 --- a/core/src/main/java/org/elasticsearch/common/settings/ClusterSettings.java +++ b/core/src/main/java/org/elasticsearch/common/settings/ClusterSettings.java @@ -82,50 +82,61 @@ import org.elasticsearch.transport.netty.NettyTransport; import java.util.Arrays; import java.util.Collections; import java.util.HashSet; -import java.util.Map; import java.util.Set; +import java.util.function.Predicate; /** * Encapsulates all valid cluster level settings. */ public final class ClusterSettings extends AbstractScopedSettings { - - public ClusterSettings(Settings settings, Set> settingsSet) { - super(settings, settingsSet, Setting.Scope.CLUSTER); + public ClusterSettings(Settings nodeSettings, Set> settingsSet) { + super(nodeSettings, settingsSet, Setting.Scope.CLUSTER); + addSettingsUpdater(new LoggingSettingUpdater(nodeSettings)); } - @Override - public synchronized Settings applySettings(Settings newSettings) { - Settings settings = super.applySettings(newSettings); - try { - for (Map.Entry entry : settings.getAsMap().entrySet()) { - if (entry.getKey().startsWith("logger.")) { - String component = entry.getKey().substring("logger.".length()); - if ("_root".equals(component)) { - ESLoggerFactory.getRootLogger().setLevel(entry.getValue()); + private static final class LoggingSettingUpdater implements SettingUpdater { + final Predicate loggerPredicate = ESLoggerFactory.LOG_LEVEL_SETTING::match; + private final Settings settings; + + LoggingSettingUpdater(Settings settings) { + this.settings = settings; + } + + @Override + public boolean hasChanged(Settings current, Settings previous) { + return current.filter(loggerPredicate).getAsMap().equals(previous.filter(loggerPredicate).getAsMap()) == false; + } + + @Override + public Settings getValue(Settings current, Settings previous) { + Settings.Builder builder = Settings.builder(); + builder.put(current.filter(loggerPredicate).getAsMap()); + for (String key : previous.getAsMap().keySet()) { + if (loggerPredicate.test(key) && builder.internalMap().containsKey(key) == false) { + if (ESLoggerFactory.LOG_LEVEL_SETTING.getConcreteSetting(key).exists(settings) == false) { + builder.putNull(key); } else { - ESLoggerFactory.getLogger(component).setLevel(entry.getValue()); + builder.put(key, ESLoggerFactory.LOG_LEVEL_SETTING.getConcreteSetting(key).get(settings).name()); } } } - } catch (Exception e) { - logger.warn("failed to refresh settings for [{}]", e, "logger"); + return builder.build(); } - return settings; - } - - @Override - public boolean hasDynamicSetting(String key) { - return isLoggerSetting(key) || super.hasDynamicSetting(key); - } - - /** - * Returns true if the settings is a logger setting. - */ - public boolean isLoggerSetting(String key) { - return key.startsWith("logger."); - } + @Override + public void apply(Settings value, Settings current, Settings previous) { + for (String key : value.getAsMap().keySet()) { + assert loggerPredicate.test(key); + String component = key.substring("logger.".length()); + if ("_root".equals(component)) { + final String rootLevel = value.get(key); + ESLoggerFactory.getRootLogger().setLevel(rootLevel == null ? ESLoggerFactory.LOG_DEFAULT_LEVEL_SETTING.get(settings).name() : rootLevel); + } else { + ESLoggerFactory.getLogger(component).setLevel(value.get(key)); + } + } + } + }; public static Set> BUILT_IN_CLUSTER_SETTINGS = Collections.unmodifiableSet(new HashSet<>( Arrays.asList(AwarenessAllocationDecider.CLUSTER_ROUTING_ALLOCATION_AWARENESS_ATTRIBUTE_SETTING, @@ -300,5 +311,7 @@ public final class ClusterSettings extends AbstractScopedSettings { InternalSettingsPreparer.IGNORE_SYSTEM_PROPERTIES_SETTING, ClusterModule.SHARDS_ALLOCATOR_TYPE_SETTING, EsExecutors.PROCESSORS_SETTING, - ThreadContext.DEFAULT_HEADERS_SETTING))); + ThreadContext.DEFAULT_HEADERS_SETTING, + ESLoggerFactory.LOG_DEFAULT_LEVEL_SETTING, + ESLoggerFactory.LOG_LEVEL_SETTING))); } diff --git a/core/src/main/java/org/elasticsearch/common/settings/Setting.java b/core/src/main/java/org/elasticsearch/common/settings/Setting.java index a76c5c73e1b..96b0a238a7f 100644 --- a/core/src/main/java/org/elasticsearch/common/settings/Setting.java +++ b/core/src/main/java/org/elasticsearch/common/settings/Setting.java @@ -200,6 +200,11 @@ public class Setting extends ToXContentToBytes { return get(secondary); } + public Setting getConcreteSetting(String key) { + assert key.startsWith(this.getKey()) : "was " + key + " expected: " + getKey(); // we use startsWith here since the key might be foo.bar.0 if it's an array + return this; + } + /** * The settings scope - settings can either be cluster settings or per index settings. */ @@ -449,8 +454,6 @@ public class Setting extends ToXContentToBytes { } } - - public static Setting groupSetting(String key, boolean dynamic, Scope scope) { if (key.endsWith(".") == false) { throw new IllegalArgumentException("key must end with a '.'"); @@ -558,4 +561,38 @@ public class Setting extends ToXContentToBytes { public int hashCode() { return Objects.hash(key); } + + /** + * This setting type allows to validate settings that have the same type and a common prefix. For instance feature.${type}=[true|false] + * can easily be added with this setting. Yet, dynamic key settings don't support updaters our of the box unless {@link #getConcreteSetting(String)} + * is used to pull the updater. + */ + public static Setting dynamicKeySetting(String key, String defaultValue, Function parser, boolean dynamic, Scope scope) { + return new Setting(key, defaultValue, parser, dynamic, scope) { + + @Override + boolean isGroupSetting() { + return true; + } + + @Override + public boolean match(String toTest) { + return toTest.startsWith(getKey()); + } + + @Override + AbstractScopedSettings.SettingUpdater newUpdater(Consumer consumer, ESLogger logger, Consumer validator) { + throw new UnsupportedOperationException("dynamic settings can't be updated use #getConcreteSetting for updating"); + } + + @Override + public Setting getConcreteSetting(String key) { + if (match(key)) { + return new Setting<>(key, defaultValue, parser, dynamic, scope); + } else { + throw new IllegalArgumentException("key must match setting but didn't ["+key +"]"); + } + } + }; + } } diff --git a/core/src/test/java/org/elasticsearch/cluster/settings/ClusterSettingsIT.java b/core/src/test/java/org/elasticsearch/cluster/settings/ClusterSettingsIT.java index cf99879c4d4..fb0ff372551 100644 --- a/core/src/test/java/org/elasticsearch/cluster/settings/ClusterSettingsIT.java +++ b/core/src/test/java/org/elasticsearch/cluster/settings/ClusterSettingsIT.java @@ -21,13 +21,11 @@ package org.elasticsearch.cluster.settings; import org.elasticsearch.action.admin.cluster.settings.ClusterUpdateSettingsRequestBuilder; import org.elasticsearch.action.admin.cluster.settings.ClusterUpdateSettingsResponse; -import org.elasticsearch.cluster.ClusterName; -import org.elasticsearch.cluster.metadata.IndexMetaData; import org.elasticsearch.cluster.metadata.MetaData; import org.elasticsearch.cluster.routing.allocation.decider.EnableAllocationDecider; +import org.elasticsearch.common.logging.ESLoggerFactory; import org.elasticsearch.common.settings.Settings; import org.elasticsearch.common.unit.ByteSizeUnit; -import org.elasticsearch.common.util.concurrent.EsExecutors; import org.elasticsearch.discovery.DiscoverySettings; import org.elasticsearch.index.store.IndexStoreConfig; import org.elasticsearch.test.ESIntegTestCase; @@ -329,16 +327,30 @@ public class ClusterSettingsIT extends ESIntegTestCase { } } - private void createNode(Settings settings) { - internalCluster().startNode(Settings.builder() - .put(ClusterName.CLUSTER_NAME_SETTING.getKey(), "ClusterSettingsIT") - .put("node.name", "ClusterSettingsIT") - .put(IndexMetaData.SETTING_NUMBER_OF_SHARDS, 1) - .put(IndexMetaData.SETTING_NUMBER_OF_REPLICAS, 0) - .put(EsExecutors.PROCESSORS_SETTING.getKey(), 1) // limit the number of threads created - .put("http.enabled", false) - .put("config.ignore_system_properties", true) // make sure we get what we set :) - .put(settings) - ); + public void testLoggerLevelUpdate() { + assertAcked(prepareCreate("test")); + final String rootLevel = ESLoggerFactory.getRootLogger().getLevel(); + final String testLevel = ESLoggerFactory.getLogger("test").getLevel(); + try { + client().admin().cluster().prepareUpdateSettings().setTransientSettings(Settings.builder().put("logger._root", "BOOM")).execute().actionGet(); + fail("Expected IllegalArgumentException"); + } catch (IllegalArgumentException e) { + assertEquals("No enum constant org.elasticsearch.common.logging.ESLoggerFactory.LogLevel.BOOM", e.getMessage()); + } + + try { + client().admin().cluster().prepareUpdateSettings().setTransientSettings(Settings.builder().put("logger.test", "TRACE").put("logger._root", "trace")).execute().actionGet(); + assertEquals("TRACE", ESLoggerFactory.getLogger("test").getLevel()); + assertEquals("TRACE", ESLoggerFactory.getRootLogger().getLevel()); + } finally { + if (randomBoolean()) { + client().admin().cluster().prepareUpdateSettings().setTransientSettings(Settings.builder().putNull("logger.test").putNull("logger._root")).execute().actionGet(); + } else { + client().admin().cluster().prepareUpdateSettings().setTransientSettings(Settings.builder().putNull("logger.*")).execute().actionGet(); + } + assertEquals(testLevel, ESLoggerFactory.getLogger("test").getLevel()); + assertEquals(rootLevel, ESLoggerFactory.getRootLogger().getLevel()); + } } + } diff --git a/core/src/test/java/org/elasticsearch/common/settings/ScopedSettingsTests.java b/core/src/test/java/org/elasticsearch/common/settings/ScopedSettingsTests.java index 86a37723b57..58f5cde65ce 100644 --- a/core/src/test/java/org/elasticsearch/common/settings/ScopedSettingsTests.java +++ b/core/src/test/java/org/elasticsearch/common/settings/ScopedSettingsTests.java @@ -22,6 +22,7 @@ import org.elasticsearch.Version; import org.elasticsearch.cluster.metadata.IndexMetaData; import org.elasticsearch.cluster.routing.allocation.decider.FilterAllocationDecider; import org.elasticsearch.cluster.routing.allocation.decider.ShardsLimitAllocationDecider; +import org.elasticsearch.common.logging.ESLoggerFactory; import org.elasticsearch.index.IndexModule; import org.elasticsearch.test.ESTestCase; import org.elasticsearch.transport.TransportService; @@ -254,4 +255,48 @@ public class ScopedSettingsTests extends ESTestCase { Settings.EMPTY, Collections.singleton(Setting.boolSetting("boo", true, false, Setting.Scope.INDEX))); } + public void testLoggingUpdates() { + final String level = ESLoggerFactory.getRootLogger().getLevel(); + final String testLevel = ESLoggerFactory.getLogger("test").getLevel(); + String property = System.getProperty("es.logger.level"); + Settings.Builder builder = Settings.builder(); + if (property != null) { + builder.put("logger.level", property); + } + try { + ClusterSettings settings = new ClusterSettings(builder.build(), ClusterSettings.BUILT_IN_CLUSTER_SETTINGS); + try { + settings.validate(Settings.builder().put("logger._root", "boom").build()); + fail(); + } catch (IllegalArgumentException ex) { + assertEquals("No enum constant org.elasticsearch.common.logging.ESLoggerFactory.LogLevel.BOOM", ex.getMessage()); + } + assertEquals(level, ESLoggerFactory.getRootLogger().getLevel()); + settings.applySettings(Settings.builder().put("logger._root", "TRACE").build()); + assertEquals("TRACE", ESLoggerFactory.getRootLogger().getLevel()); + settings.applySettings(Settings.builder().build()); + assertEquals(level, ESLoggerFactory.getRootLogger().getLevel()); + settings.applySettings(Settings.builder().put("logger.test", "TRACE").build()); + assertEquals("TRACE", ESLoggerFactory.getLogger("test").getLevel()); + settings.applySettings(Settings.builder().build()); + assertEquals(testLevel, ESLoggerFactory.getLogger("test").getLevel()); + } finally { + ESLoggerFactory.getRootLogger().setLevel(level); + ESLoggerFactory.getLogger("test").setLevel(testLevel); + } + } + + public void testFallbackToLoggerLevel() { + final String level = ESLoggerFactory.getRootLogger().getLevel(); + try { + ClusterSettings settings = new ClusterSettings(Settings.builder().put("logger.level", "ERROR").build(), ClusterSettings.BUILT_IN_CLUSTER_SETTINGS); + assertEquals(level, ESLoggerFactory.getRootLogger().getLevel()); + settings.applySettings(Settings.builder().put("logger._root", "TRACE").build()); + assertEquals("TRACE", ESLoggerFactory.getRootLogger().getLevel()); + settings.applySettings(Settings.builder().build()); // here we fall back to 'logger.level' which is our default. + assertEquals("ERROR", ESLoggerFactory.getRootLogger().getLevel()); + } finally { + ESLoggerFactory.getRootLogger().setLevel(level); + } + } } diff --git a/core/src/test/java/org/elasticsearch/common/settings/SettingTests.java b/core/src/test/java/org/elasticsearch/common/settings/SettingTests.java index e5ae9bd6296..c4386179af9 100644 --- a/core/src/test/java/org/elasticsearch/common/settings/SettingTests.java +++ b/core/src/test/java/org/elasticsearch/common/settings/SettingTests.java @@ -346,6 +346,22 @@ public class SettingTests extends ESTestCase { assertFalse(listSetting.match("foo_bar.1")); assertTrue(listSetting.match("foo.bar")); assertTrue(listSetting.match("foo.bar." + randomIntBetween(0,10000))); + } + public void testDynamicKeySetting() { + Setting setting = Setting.dynamicKeySetting("foo.", "false", Boolean::parseBoolean, false, Setting.Scope.CLUSTER); + assertTrue(setting.hasComplexMatcher()); + assertTrue(setting.match("foo.bar")); + assertFalse(setting.match("foo")); + Setting concreteSetting = setting.getConcreteSetting("foo.bar"); + assertTrue(concreteSetting.get(Settings.builder().put("foo.bar", "true").build())); + assertFalse(concreteSetting.get(Settings.builder().put("foo.baz", "true").build())); + + try { + setting.getConcreteSetting("foo"); + fail(); + } catch (IllegalArgumentException ex) { + assertEquals("key must match setting but didn't [foo]", ex.getMessage()); + } } } diff --git a/core/src/test/java/org/elasticsearch/common/settings/SettingsModuleTests.java b/core/src/test/java/org/elasticsearch/common/settings/SettingsModuleTests.java index 731957cba06..290eec0c0bb 100644 --- a/core/src/test/java/org/elasticsearch/common/settings/SettingsModuleTests.java +++ b/core/src/test/java/org/elasticsearch/common/settings/SettingsModuleTests.java @@ -78,4 +78,24 @@ public class SettingsModuleTests extends ModuleTestCase { } } } + + public void testLoggerSettings() { + { + Settings settings = Settings.builder().put("logger._root", "TRACE").put("logger.transport", "INFO").build(); + SettingsModule module = new SettingsModule(settings, new SettingsFilter(Settings.EMPTY)); + assertInstanceBinding(module, Settings.class, (s) -> s == settings); + } + + { + Settings settings = Settings.builder().put("logger._root", "BOOM").put("logger.transport", "WOW").build(); + SettingsModule module = new SettingsModule(settings, new SettingsFilter(Settings.EMPTY)); + try { + assertInstanceBinding(module, Settings.class, (s) -> s == settings); + fail(); + } catch (IllegalArgumentException ex) { + assertEquals("No enum constant org.elasticsearch.common.logging.ESLoggerFactory.LogLevel.BOOM", ex.getMessage()); + } + } + + } }