diff --git a/CHANGELOG.md b/CHANGELOG.md index 9b22a89975b46..2e10d8d0337bf 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -48,6 +48,7 @@ Inspired from [Keep a Changelog](https://keepachangelog.com/en/1.0.0/) ### Deprecated ### Removed +- Remove deprecated code to add node name into log pattern of log4j property file ([#4568](https://github.com/opensearch-project/OpenSearch/pull/4568)) ### Fixed - `opensearch-service.bat start` and `opensearch-service.bat manager` failing to run ([#4289](https://github.com/opensearch-project/OpenSearch/pull/4289)) diff --git a/qa/evil-tests/src/test/java/org/opensearch/common/logging/EvilLoggerTests.java b/qa/evil-tests/src/test/java/org/opensearch/common/logging/EvilLoggerTests.java index 9319d11ebae98..e6548885a8d10 100644 --- a/qa/evil-tests/src/test/java/org/opensearch/common/logging/EvilLoggerTests.java +++ b/qa/evil-tests/src/test/java/org/opensearch/common/logging/EvilLoggerTests.java @@ -285,29 +285,6 @@ public void testProperties() throws IOException, UserException { assertThat(System.getProperty("opensearch.logs.node_name"), equalTo(Node.NODE_NAME_SETTING.get(settings))); } - public void testNoNodeNameInPatternWarning() throws IOException, UserException { - String nodeName = randomAlphaOfLength(16); - LogConfigurator.setNodeName(nodeName); - setupLogging("no_node_name"); - final String path = - System.getProperty("opensearch.logs.base_path") + - System.getProperty("file.separator") + - System.getProperty("opensearch.logs.cluster_name") + ".log"; - final List events = Files.readAllLines(PathUtils.get(path)); - assertThat(events.size(), equalTo(2)); - final String location = "org.opensearch.common.logging.LogConfigurator"; - // the first message is a warning for unsupported configuration files - assertLogLine(events.get(0), Level.WARN, location, "\\[" + nodeName + "\\] Some logging configurations have " - + "%marker but don't have %node_name. We will automatically add %node_name to the pattern to ease the " - + "migration for users who customize log4j2.properties but will stop this behavior in 7.0. You should " - + "manually replace `%node_name` with `\\[%node_name\\]%marker ` in these locations:"); - if (Constants.WINDOWS) { - assertThat(events.get(1), endsWith("no_node_name\\log4j2.properties")); - } else { - assertThat(events.get(1), endsWith("no_node_name/log4j2.properties")); - } - } - private void setupLogging(final String config) throws IOException, UserException { setupLogging(config, Settings.EMPTY); } diff --git a/server/src/main/java/org/opensearch/common/logging/LogConfigurator.java b/server/src/main/java/org/opensearch/common/logging/LogConfigurator.java index e70692808a641..e42727b2aa2af 100644 --- a/server/src/main/java/org/opensearch/common/logging/LogConfigurator.java +++ b/server/src/main/java/org/opensearch/common/logging/LogConfigurator.java @@ -36,8 +36,6 @@ import org.apache.logging.log4j.LogManager; import org.apache.logging.log4j.core.LoggerContext; import org.apache.logging.log4j.core.config.AbstractConfiguration; -import org.apache.logging.log4j.core.config.ConfigurationException; -import org.apache.logging.log4j.core.config.ConfigurationSource; import org.apache.logging.log4j.core.config.Configurator; import org.apache.logging.log4j.core.config.builder.api.ConfigurationBuilder; import org.apache.logging.log4j.core.config.builder.api.ConfigurationBuilderFactory; @@ -45,7 +43,6 @@ import org.apache.logging.log4j.core.config.composite.CompositeConfiguration; import org.apache.logging.log4j.core.config.plugins.util.PluginManager; import org.apache.logging.log4j.core.config.properties.PropertiesConfiguration; -import org.apache.logging.log4j.core.config.properties.PropertiesConfigurationBuilder; import org.apache.logging.log4j.core.config.properties.PropertiesConfigurationFactory; import org.apache.logging.log4j.status.StatusConsoleListener; import org.apache.logging.log4j.status.StatusData; @@ -60,7 +57,6 @@ import org.opensearch.node.Node; import java.io.IOException; -import java.io.InputStream; import java.io.PrintStream; import java.nio.charset.StandardCharsets; import java.nio.file.FileVisitOption; @@ -75,7 +71,6 @@ import java.util.HashSet; import java.util.List; import java.util.Objects; -import java.util.Properties; import java.util.Set; import java.util.concurrent.atomic.AtomicBoolean; import java.util.stream.StreamSupport; @@ -189,50 +184,7 @@ private static void configure(final Settings settings, final Path configsPath, f final Set locationsWithDeprecatedPatterns = Collections.synchronizedSet(new HashSet<>()); final List configurations = new ArrayList<>(); - /* - * Subclass the properties configurator to hack the new pattern in - * place so users don't have to change log4j2.properties in - * a minor release. In 7.0 we'll remove this and force users to - * change log4j2.properties. If they don't customize log4j2.properties - * then they won't have to do anything anyway. - * - * Everything in this subclass that isn't marked as a hack is copied - * from log4j2's source. - */ - final PropertiesConfigurationFactory factory = new PropertiesConfigurationFactory() { - @Override - public PropertiesConfiguration getConfiguration(final LoggerContext loggerContext, final ConfigurationSource source) { - final Properties properties = new Properties(); - try (InputStream configStream = source.getInputStream()) { - properties.load(configStream); - } catch (final IOException ioe) { - throw new ConfigurationException("Unable to load " + source.toString(), ioe); - } - // Hack the new pattern into place - for (String name : properties.stringPropertyNames()) { - if (false == name.endsWith(".pattern")) continue; - // Null is weird here but we can't do anything with it so ignore it - String value = properties.getProperty(name); - if (value == null) continue; - // Tests don't need to be changed - if (value.contains("%test_thread_info")) continue; - /* - * Patterns without a marker are sufficiently customized - * that we don't have an opinion about them. - */ - if (false == value.contains("%marker")) continue; - if (false == value.contains("%node_name")) { - locationsWithDeprecatedPatterns.add(source.getLocation()); - properties.setProperty(name, value.replace("%marker", "[%node_name]%marker ")); - } - } - // end hack - return new PropertiesConfigurationBuilder().setConfigurationSource(source) - .setRootProperties(properties) - .setLoggerContext(loggerContext) - .build(); - } - }; + final PropertiesConfigurationFactory factory = new PropertiesConfigurationFactory(); final Set options = EnumSet.of(FileVisitOption.FOLLOW_LINKS); Files.walkFileTree(configsPath, options, Integer.MAX_VALUE, new SimpleFileVisitor() { @Override @@ -252,18 +204,6 @@ public FileVisitResult visitFile(final Path file, final BasicFileAttributes attr configureLoggerLevels(settings); - final String deprecatedLocationsString = String.join("\n ", locationsWithDeprecatedPatterns); - if (deprecatedLocationsString.length() > 0) { - LogManager.getLogger(LogConfigurator.class) - .warn( - "Some logging configurations have %marker but don't have %node_name. " - + "We will automatically add %node_name to the pattern to ease the migration for users who customize " - + "log4j2.properties but will stop this behavior in 7.0. You should manually replace `%node_name` with " - + "`[%node_name]%marker ` in these locations:\n {}", - deprecatedLocationsString - ); - } - // Redirect stdout/stderr to log4j. While we ensure Elasticsearch code does not write to those streams, // third party libraries may do that System.setOut(