diff --git a/CHANGELOG.asciidoc b/CHANGELOG.asciidoc index 1ce3981324..3b10cf3fcf 100644 --- a/CHANGELOG.asciidoc +++ b/CHANGELOG.asciidoc @@ -29,6 +29,7 @@ endif::[] * Fixed service name discovery based on MANIFEST.MF file through `ServletContainerInitializer#onStartup` on Jakarta Servlet containers - {pull}2546[#2546] * Fix shaded classloader package definition - {pull}2566[#2566] +* Fix logging initialization with Security Manager - {pull}2568[#2568] [[release-notes-1.x]] === Java Agent version 1.x diff --git a/apm-agent-core/src/main/java/co/elastic/apm/agent/logging/Log4jLoggerFactoryBridge.java b/apm-agent-core/src/main/java/co/elastic/apm/agent/logging/Log4jLoggerFactoryBridge.java index fe338a33a0..b569d75901 100644 --- a/apm-agent-core/src/main/java/co/elastic/apm/agent/logging/Log4jLoggerFactoryBridge.java +++ b/apm-agent-core/src/main/java/co/elastic/apm/agent/logging/Log4jLoggerFactoryBridge.java @@ -28,12 +28,15 @@ import org.apache.logging.log4j.util.StackLocatorUtil; /** - * Based on {@code org.apache.logging.slf4j.Log4jLoggerFactory} + * Based on {@code org.apache.logging.slf4j.Log4jLoggerFactory}. + *

+ * This class does not implement {@link ILoggerFactory} directly but through the super class implementation that has + * a method matching {@link ILoggerFactory#getLogger(String)}. Given this method is caller-sensitive then we can't + * implement it directly without side effects. */ public class Log4jLoggerFactoryBridge extends AbstractLoggerAdapter implements ILoggerFactory { - private static final String FQCN = Log4jLoggerFactoryBridge.class.getName(); - private static final String PACKAGE = "co.elastic.apm.agent.sdk.logging"; + private static final String LOGGER_FACTORY = "co.elastic.apm.agent.sdk.logging.LoggerFactory"; public static void shutdown() { LoggerContextFactory factory = LogManager.getFactory(); @@ -53,7 +56,12 @@ protected Logger newLogger(final String name, final LoggerContext context) { @Override protected LoggerContext getContext() { - final Class anchor = StackLocatorUtil.getCallerClass(FQCN, PACKAGE); - return anchor == null ? LogManager.getContext() : getContext(StackLocatorUtil.getCallerClass(anchor)); + // the logger context is defined by the class that calls LoggerFactory + final Class factoryCaller = StackLocatorUtil.getCallerClass(LOGGER_FACTORY); + if (factoryCaller == null) { + return LogManager.getContext(); + } + return getContext(factoryCaller); } + } diff --git a/apm-agent-core/src/test/java/co/elastic/apm/agent/logging/LoggingConfigurationTest.java b/apm-agent-core/src/test/java/co/elastic/apm/agent/logging/LoggingConfigurationTest.java index 4ccb208435..0811579f42 100644 --- a/apm-agent-core/src/test/java/co/elastic/apm/agent/logging/LoggingConfigurationTest.java +++ b/apm-agent-core/src/test/java/co/elastic/apm/agent/logging/LoggingConfigurationTest.java @@ -136,11 +136,16 @@ void loggingLevelChangeTest() throws IOException { // stagemonitor relies on slf4j, which should be bridged to the same log4j registries and get the same configuration assertThat(configOptionLogger.isTraceEnabled()).isFalse(); - assertThat(testLog4jContextFactory.getContext(configOptionLogger.getName())).isEqualTo(agentLoggerContext); + assertThat(testLog4jContextFactory.getContext(configOptionLogger.getName())) + .describedAs("configuration logger context should be the same as the agent logger context") + .isEqualTo(agentLoggerContext); LoggerContext pluginLoggerContext = testLog4jContextFactory.getContext(pluginLogger.getName()); assertThat(pluginLoggerContext).isNotNull(); - assertThat(pluginLoggerContext).isNotEqualTo(agentLoggerContext); + assertThat(pluginLoggerContext) + .describedAs("plugin logger context should be distinct from agent logger context") + .isNotEqualTo(agentLoggerContext); + assertThat(pluginLoggerContext.getName()).startsWith(IndyPluginClassLoader.class.getName()); assertThat(pluginLogger.isTraceEnabled()).isFalse(); @@ -231,7 +236,7 @@ private static class TestContextSelector implements ContextSelector { private final Map contextMap = new HashMap<>(); @Override - public LoggerContext getContext(String fqcn, ClassLoader loader, boolean currentContext) { + public LoggerContext getContext(String fqcn, @Nullable ClassLoader loader, boolean currentContext) { if (loader == null) { loader = ClassLoader.getSystemClassLoader(); } diff --git a/apm-agent-plugin-sdk/src/main/java/co/elastic/apm/agent/sdk/logging/LoggerFactory.java b/apm-agent-plugin-sdk/src/main/java/co/elastic/apm/agent/sdk/logging/LoggerFactory.java index c61257d21e..493e78f3e9 100644 --- a/apm-agent-plugin-sdk/src/main/java/co/elastic/apm/agent/sdk/logging/LoggerFactory.java +++ b/apm-agent-plugin-sdk/src/main/java/co/elastic/apm/agent/sdk/logging/LoggerFactory.java @@ -18,6 +18,9 @@ */ package co.elastic.apm.agent.sdk.logging; +import java.security.AccessController; +import java.security.PrivilegedAction; + public class LoggerFactory { private static volatile ILoggerFactory iLoggerFactory; @@ -32,11 +35,16 @@ public static void initialize(ILoggerFactory iLoggerFactory) { * @param name The name of the logger. * @return logger */ - public static Logger getLogger(String name) { + public static Logger getLogger(final String name) { if (iLoggerFactory == null) { return NoopLogger.INSTANCE; } - return iLoggerFactory.getLogger(name); + return AccessController.doPrivileged(new PrivilegedAction() { + @Override + public Logger run() { + return iLoggerFactory.getLogger(name); + } + }); } /**