Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.asciidoc
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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}.
* <p>
* 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<Logger> 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();
Expand All @@ -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);
Comment on lines -57 to +64

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@eyalkoren I do need your review on this as I'm not 100% sure it's the right way to fix it.
Without this, the added anonymous class makes loggingLevelChangeTest test fail.

}

}
Original file line number Diff line number Diff line change
Expand Up @@ -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();

Expand Down Expand Up @@ -231,7 +236,7 @@ private static class TestContextSelector implements ContextSelector {
private final Map<ClassLoader, LoggerContext> 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();
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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<Logger>() {
@Override
public Logger run() {
return iLoggerFactory.getLogger(name);
}
});
}

/**
Expand Down