Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add service to attach metrics to Log4j Core #2469

Draft
wants to merge 1 commit into
base: 2.x
Choose a base branch
from
Draft
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
Original file line number Diff line number Diff line change
Expand Up @@ -41,12 +41,14 @@
import org.apache.logging.log4j.core.config.NullConfiguration;
import org.apache.logging.log4j.core.config.Reconfigurable;
import org.apache.logging.log4j.core.impl.Log4jLogEvent;
import org.apache.logging.log4j.core.instrumentation.InstrumentationService;
import org.apache.logging.log4j.core.jmx.Server;
import org.apache.logging.log4j.core.util.Cancellable;
import org.apache.logging.log4j.core.util.ExecutorServices;
import org.apache.logging.log4j.core.util.NetUtils;
import org.apache.logging.log4j.core.util.ShutdownCallbackRegistry;
import org.apache.logging.log4j.message.MessageFactory;
import org.apache.logging.log4j.message.ParameterizedMessageFactory;
import org.apache.logging.log4j.spi.AbstractLogger;
import org.apache.logging.log4j.spi.LoggerContextFactory;
import org.apache.logging.log4j.spi.LoggerContextShutdownAware;
Expand Down Expand Up @@ -800,6 +802,12 @@ private void initApiModule() {

// LOG4J2-151: changed visibility from private to protected
protected Logger newInstance(final LoggerContext ctx, final String name, final MessageFactory messageFactory) {
return new Logger(ctx, name, messageFactory);
// TODO: Adapt after
// https://github.com/apache/logging-log4j2/issues/2379
// is fixed.
final MessageFactory actualMessageFactory = InstrumentationService.getInstance()
Copy link
Member

Choose a reason for hiding this comment

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

I am not sure I agree with this approach. It is basically making InstrumentationService a factory for wrappers. I would rather see things that should be instrumented implement a well defined interface for providing the instrumented data such as https://github.com/apache/logging-flume/tree/trunk/flume-ng-core/src/main/java/org/apache/flume/instrumentation does. I think the temptation to do more than instrumentation is way too great with the current approach. It also means that InstrumentationServcie has to be aware of every type of thing that can be instrumented.

Copy link
Member

Choose a reason for hiding this comment

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

@vy I would NOT object to "intercepting" instantiation so long as a) it can't wrap the object being returned and b) it can only modify the created object if that object permits it. In my mind instrumentation means the ability to capture statistics or perform an allowed operation, not completely replace the thing being instrumented.

.instrumentMessageFactory(
name, messageFactory != null ? messageFactory : ParameterizedMessageFactory.INSTANCE);
return new Logger(ctx, name, actualMessageFactory);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,7 @@
import org.apache.logging.log4j.core.config.plugins.validation.constraints.Required;
import org.apache.logging.log4j.core.filter.AbstractFilterable;
import org.apache.logging.log4j.core.impl.Log4jLogEvent;
import org.apache.logging.log4j.core.instrumentation.InstrumentationService;
import org.apache.logging.log4j.spi.AbstractLogger;

/**
Expand Down Expand Up @@ -121,7 +122,9 @@ public void start() {
} else if (errorRef == null) {
throw new ConfigurationException("No appenders are available for AsyncAppender " + getName());
}
asyncQueueFullPolicy = AsyncQueueFullPolicyFactory.create();
asyncQueueFullPolicy = InstrumentationService.getInstance()
.instrumentQueueFullPolicy(
InstrumentationService.ASYNC_APPENDER, getName(), AsyncQueueFullPolicyFactory.create());

dispatcher.start();
super.start();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@
import org.apache.logging.log4j.core.impl.LogEventFactory;
import org.apache.logging.log4j.core.impl.MutableLogEvent;
import org.apache.logging.log4j.core.impl.ReusableLogEventFactory;
import org.apache.logging.log4j.core.instrumentation.InstrumentationService;
import org.apache.logging.log4j.core.jmx.RingBufferAdmin;
import org.apache.logging.log4j.core.util.Log4jThread;
import org.apache.logging.log4j.core.util.Log4jThreadFactory;
Expand Down Expand Up @@ -242,7 +243,9 @@ public Thread newThread(final Runnable r) {
return result;
}
};
asyncQueueFullPolicy = AsyncQueueFullPolicyFactory.create();
asyncQueueFullPolicy = InstrumentationService.getInstance()
.instrumentQueueFullPolicy(
InstrumentationService.ASYNC_LOGGER_CONFIG, "Default", AsyncQueueFullPolicyFactory.create());

translator = mutable ? MUTABLE_TRANSLATOR : TRANSLATOR;
factory = mutable ? MUTABLE_FACTORY : FACTORY;
Expand Down Expand Up @@ -449,6 +452,9 @@ private LogEvent ensureImmutable(final LogEvent event) {
*/
@Override
public RingBufferAdmin createRingBufferAdmin(final String contextName, final String loggerConfigName) {
return RingBufferAdmin.forAsyncLoggerConfig(disruptor.getRingBuffer(), contextName, loggerConfigName);
final RingBufferAdmin ringBufferAdmin =
RingBufferAdmin.forAsyncLoggerConfig(disruptor.getRingBuffer(), contextName, loggerConfigName);
InstrumentationService.getInstance().instrumentRingBuffer(ringBufferAdmin);
return ringBufferAdmin;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@
import org.apache.logging.log4j.Level;
import org.apache.logging.log4j.Marker;
import org.apache.logging.log4j.core.AbstractLifeCycle;
import org.apache.logging.log4j.core.instrumentation.InstrumentationService;
import org.apache.logging.log4j.core.jmx.RingBufferAdmin;
import org.apache.logging.log4j.core.util.Log4jThread;
import org.apache.logging.log4j.core.util.Log4jThreadFactory;
Expand Down Expand Up @@ -130,7 +131,9 @@ public Thread newThread(final Runnable r) {
return result;
}
};
asyncQueueFullPolicy = AsyncQueueFullPolicyFactory.create();
asyncQueueFullPolicy = InstrumentationService.getInstance()
.instrumentQueueFullPolicy(
InstrumentationService.ASYNC_LOGGER, getContextName(), AsyncQueueFullPolicyFactory.create());

disruptor = new Disruptor<>(
RingBufferLogEvent.FACTORY, ringBufferSize, threadFactory, ProducerType.MULTI, waitStrategy);
Expand Down Expand Up @@ -219,7 +222,9 @@ private static boolean hasBacklog(final Disruptor<?> theDisruptor) {
*/
public RingBufferAdmin createRingBufferAdmin(final String jmxContextName) {
final RingBuffer<RingBufferLogEvent> ring = disruptor == null ? null : disruptor.getRingBuffer();
return RingBufferAdmin.forAsyncLogger(ring, jmxContextName);
final RingBufferAdmin ringBufferAdmin = RingBufferAdmin.forAsyncLogger(ring, jmxContextName);
InstrumentationService.getInstance().instrumentRingBuffer(ringBufferAdmin);
return ringBufferAdmin;
}

EventRoute getEventRoute(final Level logLevel) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,7 @@
import org.apache.logging.log4j.core.config.plugins.util.PluginManager;
import org.apache.logging.log4j.core.config.plugins.util.PluginType;
import org.apache.logging.log4j.core.filter.AbstractFilterable;
import org.apache.logging.log4j.core.instrumentation.InstrumentationService;
import org.apache.logging.log4j.core.layout.PatternLayout;
import org.apache.logging.log4j.core.lookup.ConfigurationStrSubstitutor;
import org.apache.logging.log4j.core.lookup.Interpolator;
Expand Down Expand Up @@ -129,8 +130,8 @@ public abstract class AbstractConfiguration extends AbstractFilterable implement
private Node advertiserNode;
private Object advertisement;
private String name;
private ConcurrentMap<String, Appender> appenders = new ConcurrentHashMap<>();
private ConcurrentMap<String, LoggerConfig> loggerConfigs = new ConcurrentHashMap<>();
private final ConcurrentMap<String, Appender> appenders = new ConcurrentHashMap<>();
private final ConcurrentMap<String, LoggerConfig> loggerConfigs = new ConcurrentHashMap<>();
private List<CustomLevelConfig> customLevels = Collections.emptyList();
private final ConcurrentMap<String, String> propertyMap = new ConcurrentHashMap<>();
private final Interpolator tempLookup = new Interpolator(propertyMap);
Expand Down Expand Up @@ -693,15 +694,22 @@ protected void doConfigure() {
}
}
} else if ("Appenders".equalsIgnoreCase(child.getName())) {
appenders = child.getObject();
final InstrumentationService instrumentation = InstrumentationService.getInstance();
final Map<String, Appender> appenders = child.getObject();
appenders.forEach((name, appender) -> {
this.appenders.computeIfAbsent(name, ignored -> instrumentation.instrumentAppender(appender));
});
} else if (child.isInstanceOf(Filter.class)) {
addFilter(child.getObject(Filter.class));
} else if (child.isInstanceOf(Loggers.class)) {
final Loggers l = child.getObject();
loggerConfigs = l.getMap();
final InstrumentationService instrumentation = InstrumentationService.getInstance();
l.getMap().forEach((key, value) -> {
loggerConfigs.put(key, instrumentation.instrumentLoggerConfig(value));
});
setLoggers = true;
if (l.getRoot() != null) {
root = l.getRoot();
root = instrumentation.instrumentLoggerConfig(l.getRoot());
setRoot = true;
}
} else if (child.isInstanceOf(CustomLevels.class)) {
Expand Down Expand Up @@ -842,7 +850,8 @@ public Map<String, Appender> getAppenders() {
@Override
public void addAppender(final Appender appender) {
if (appender != null) {
appenders.putIfAbsent(appender.getName(), appender);
appenders.computeIfAbsent(appender.getName(), ignored -> InstrumentationService.getInstance()
.instrumentAppender(appender));
}
}

Expand Down Expand Up @@ -893,15 +902,19 @@ public synchronized void addLoggerAppender(
return;
}
final String loggerName = logger.getName();
appenders.putIfAbsent(appender.getName(), appender);
final Appender actualAppender =
appenders.computeIfAbsent(appender.getName(), ignored -> InstrumentationService.getInstance()
.instrumentAppender(appender));
final LoggerConfig lc = getLoggerConfig(loggerName);
if (lc.getName().equals(loggerName)) {
lc.addAppender(appender, null, null);
lc.addAppender(actualAppender, null, null);
} else {
final LoggerConfig nlc = new LoggerConfig(loggerName, lc.getLevel(), lc.isAdditive());
nlc.addAppender(appender, null, null);
nlc.setParent(lc);
loggerConfigs.putIfAbsent(loggerName, nlc);
loggerConfigs.computeIfAbsent(loggerName, name -> {
final LoggerConfig config = new LoggerConfig(name, lc.getExplicitLevel(), lc.isAdditive());
config.addAppender(actualAppender, null, null);
config.setParent(lc);
return InstrumentationService.getInstance().instrumentLoggerConfig(config);
});
setParents();
logger.getContext().updateLoggers();
}
Expand All @@ -923,10 +936,12 @@ public synchronized void addLoggerFilter(final org.apache.logging.log4j.core.Log
if (lc.getName().equals(loggerName)) {
lc.addFilter(filter);
} else {
final LoggerConfig nlc = new LoggerConfig(loggerName, lc.getLevel(), lc.isAdditive());
nlc.addFilter(filter);
nlc.setParent(lc);
loggerConfigs.putIfAbsent(loggerName, nlc);
loggerConfigs.computeIfAbsent(loggerName, name -> {
final LoggerConfig config = new LoggerConfig(name, lc.getExplicitLevel(), lc.isAdditive());
config.addFilter(filter);
config.setParent(lc);
return InstrumentationService.getInstance().instrumentLoggerConfig(config);
});
setParents();
logger.getContext().updateLoggers();
}
Expand All @@ -949,9 +964,11 @@ public synchronized void setLoggerAdditive(
if (lc.getName().equals(loggerName)) {
lc.setAdditive(additive);
} else {
final LoggerConfig nlc = new LoggerConfig(loggerName, lc.getLevel(), additive);
nlc.setParent(lc);
loggerConfigs.putIfAbsent(loggerName, nlc);
loggerConfigs.computeIfAbsent(loggerName, name -> {
final LoggerConfig config = new LoggerConfig(name, lc.getExplicitLevel(), additive);
config.setParent(lc);
return InstrumentationService.getInstance().instrumentLoggerConfig(config);
});
setParents();
logger.getContext().updateLoggers();
}
Expand Down Expand Up @@ -1052,7 +1069,8 @@ public LoggerConfig getLogger(final String loggerName) {
*/
@Override
public synchronized void addLogger(final String loggerName, final LoggerConfig loggerConfig) {
loggerConfigs.putIfAbsent(loggerName, loggerConfig);
loggerConfigs.computeIfAbsent(
loggerName, ignored -> InstrumentationService.getInstance().instrumentLoggerConfig(loggerConfig));
setParents();
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,7 @@
import org.apache.logging.log4j.core.impl.Log4jLogEvent;
import org.apache.logging.log4j.core.impl.LogEventFactory;
import org.apache.logging.log4j.core.impl.ReusableLogEventFactory;
import org.apache.logging.log4j.core.instrumentation.InstrumentationService;
import org.apache.logging.log4j.core.lookup.StrSubstitutor;
import org.apache.logging.log4j.core.util.Booleans;
import org.apache.logging.log4j.core.util.Constants;
Expand Down Expand Up @@ -237,9 +238,9 @@ public B asBuilder() {
* Default constructor.
*/
public LoggerConfig() {
this.logEventFactory = LOG_EVENT_FACTORY;
this.level = Level.ERROR;
this.name = Strings.EMPTY;
this.logEventFactory = InstrumentationService.getInstance().instrumentLogEventFactory(name, LOG_EVENT_FACTORY);
this.level = Level.ERROR;
this.properties = null;
this.propertiesRequireLookup = false;
this.config = null;
Expand All @@ -254,8 +255,8 @@ public LoggerConfig() {
* @param additive true if the Logger is additive, false otherwise.
*/
public LoggerConfig(final String name, final Level level, final boolean additive) {
this.logEventFactory = LOG_EVENT_FACTORY;
this.name = name;
this.logEventFactory = InstrumentationService.getInstance().instrumentLogEventFactory(name, LOG_EVENT_FACTORY);
this.level = level;
this.additive = additive;
this.properties = null;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@
*/
package org.apache.logging.log4j.core.config;

import org.apache.logging.log4j.core.instrumentation.InstrumentationService;
import org.apache.logging.log4j.core.util.Loader;
import org.apache.logging.log4j.status.StatusLogger;
import org.apache.logging.log4j.util.PropertiesUtil;
Expand Down Expand Up @@ -43,9 +44,14 @@ private ReliabilityStrategyFactory() {}
* configuration change
*/
public static ReliabilityStrategy getReliabilityStrategy(final LoggerConfig loggerConfig) {
return InstrumentationService.getInstance()
.instrumentReliabilityStrategy(
loggerConfig.getName(), createFromProperties(loggerConfig, PropertiesUtil.getProperties()));
}

final String strategy =
PropertiesUtil.getProperties().getStringProperty("log4j.ReliabilityStrategy", "AwaitCompletion");
private static ReliabilityStrategy createFromProperties(
final LoggerConfig loggerConfig, final PropertiesUtil properties) {
final String strategy = properties.getStringProperty("log4j.ReliabilityStrategy", "AwaitCompletion");
if ("AwaitCompletion".equals(strategy)) {
return new AwaitCompletionReliabilityStrategy(loggerConfig);
}
Expand Down
Loading
Loading