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.

Sign up for GitHub

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

Rebind log4j2 metrics if configuration is changed #5810

Merged
merged 5 commits into from
Feb 6, 2025
+134 −19
Merged

Rebind log4j2 metrics if configuration is changed #5810

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
fc2913f
Rebind log4j2 metrics if configuration is changed
pativa Jan 23, 2025
64ae25e
Updates to Log4j2Metrics from code review
pativa Jan 24, 2025
356c7dc
fix cleanup of loggerConfig
pativa Jan 24, 2025
cd4b6bf
code review: add additional assert
pativa Jan 28, 2025
66368f4
Log warning and do nothing on double binding to same registry
shakuzen Feb 5, 2025
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
73 changes: 54 additions & 19 deletions ...ometer-core/src/main/java/io/micrometer/core/instrument/binder/logging/Log4j2Metrics.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,8 @@

import io.micrometer.common.lang.NonNullApi;
import io.micrometer.common.lang.NonNullFields;
import io.micrometer.common.util.internal.logging.InternalLogger;
import io.micrometer.common.util.internal.logging.InternalLoggerFactory;
import io.micrometer.core.instrument.Counter;
import io.micrometer.core.instrument.MeterRegistry;
import io.micrometer.core.instrument.Tag;
Expand All @@ -31,9 +33,12 @@
import org.apache.logging.log4j.core.filter.AbstractFilter;
import org.apache.logging.log4j.core.filter.CompositeFilter;

import java.util.*;
import java.beans.PropertyChangeListener;
import java.util.Arrays;
import java.util.List;
import java.util.concurrent.ConcurrentHashMap;
import java.util.concurrent.ConcurrentMap;
import java.util.concurrent.CopyOnWriteArrayList;

import static java.util.Collections.emptyList;

Expand All @@ -57,12 +62,16 @@ public class Log4j2Metrics implements MeterBinder, AutoCloseable {

private static final String METER_DESCRIPTION = "Number of log events";

private static final InternalLogger logger = InternalLoggerFactory.getInstance(Log4j2Metrics.class);

private final Iterable<Tag> tags;

private final LoggerContext loggerContext;

private final ConcurrentMap<MeterRegistry, MetricsFilter> metricsFilters = new ConcurrentHashMap<>();

private final List<PropertyChangeListener> changeListeners = new CopyOnWriteArrayList<>();

public Log4j2Metrics() {
this(emptyList());
}
Expand All @@ -78,13 +87,35 @@ public Log4j2Metrics(Iterable<Tag> tags, LoggerContext loggerContext) {

@Override
public void bindTo(MeterRegistry registry) {
if (metricsFilters.containsKey(registry)) {
logger.warn("This Log4j2Metrics instance has already been bound to the registry {}", registry);
return;
}
Configuration configuration = loggerContext.getConfiguration();
registerMetricsFilter(configuration, registry);
loggerContext.updateLoggers(configuration);

PropertyChangeListener changeListener = listener -> {
if (listener.getNewValue() instanceof Configuration && listener.getOldValue() != listener.getNewValue()) {
Configuration newConfiguration = (Configuration) listener.getNewValue();
registerMetricsFilter(newConfiguration, registry);
loggerContext.updateLoggers(newConfiguration);
if (listener.getOldValue() instanceof Configuration) {
removeMetricsFilter((Configuration) listener.getOldValue());
}
}
};

changeListeners.add(changeListener);
pativa marked this conversation as resolved.
Show resolved Hide resolved
loggerContext.addPropertyChangeListener(changeListener);
}

private void registerMetricsFilter(Configuration configuration, MeterRegistry registry) {
LoggerConfig rootLoggerConfig = configuration.getRootLogger();
MetricsFilter metricsFilter = getOrCreateMetricsFilterAndStart(registry);
rootLoggerConfig.addFilter(metricsFilter);

loggerContext.getConfiguration()
.getLoggers()
configuration.getLoggers()
.values()
.stream()
.filter(loggerConfig -> !loggerConfig.isAdditive())
Expand All @@ -105,8 +136,6 @@ public void bindTo(MeterRegistry registry) {

loggerConfig.addFilter(metricsFilter);
});

loggerContext.updateLoggers(configuration);
}

private MetricsFilter getOrCreateMetricsFilterAndStart(MeterRegistry registry) {
Expand All @@ -119,28 +148,34 @@ private MetricsFilter getOrCreateMetricsFilterAndStart(MeterRegistry registry) {

@Override
public void close() {
changeListeners.forEach(loggerContext::removePropertyChangeListener);
changeListeners.clear();

if (!metricsFilters.isEmpty()) {
Configuration configuration = loggerContext.getConfiguration();
LoggerConfig rootLoggerConfig = configuration.getRootLogger();
metricsFilters.values().forEach(rootLoggerConfig::removeFilter);

loggerContext.getConfiguration()
.getLoggers()
.values()
.stream()
.filter(loggerConfig -> !loggerConfig.isAdditive())
.forEach(loggerConfig -> {
if (loggerConfig != rootLoggerConfig) {
metricsFilters.values().forEach(loggerConfig::removeFilter);
}
});

removeMetricsFilter(configuration);
loggerContext.updateLoggers(configuration);

metricsFilters.values().forEach(MetricsFilter::stop);
metricsFilters.clear();
pativa marked this conversation as resolved.
Show resolved Hide resolved
}
}

private void removeMetricsFilter(Configuration configuration) {
LoggerConfig rootLoggerConfig = configuration.getRootLogger();
metricsFilters.values().forEach(rootLoggerConfig::removeFilter);

configuration.getLoggers()
.values()
.stream()
.filter(loggerConfig -> !loggerConfig.isAdditive())
.forEach(loggerConfig -> {
if (loggerConfig != rootLoggerConfig) {
metricsFilters.values().forEach(loggerConfig::removeFilter);
}
});
}

@NonNullApi
@NonNullFields
static class MetricsFilter extends AbstractFilter {
Expand Down
80 changes: 80 additions & 0 deletions ...er-core/src/test/java/io/micrometer/core/instrument/binder/logging/Log4j2MetricsTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -288,4 +288,84 @@ void multipleRegistriesCanBeBoundWithNonRootLoggerContext() {
assertThat(registry2.get("log4j2.events").tags("level", "info").counter().count()).isEqualTo(2);
}

@Issue("#5756")
@Test
void rebindsMetricsWhenConfigurationIsReloaded() {
LoggerContext context = new LoggerContext("test");
Logger logger = context.getLogger("com.test");
Configuration oldConfiguration = context.getConfiguration();

try (Log4j2Metrics metrics = new Log4j2Metrics(emptyList(), context)) {
metrics.bindTo(registry);

logger.error("first");
assertThat(registry.get("log4j2.events").tags("level", "error").counter().count()).isEqualTo(1);

// Should have added filter to configuration
Filter oldFilter = oldConfiguration.getRootLogger().getFilter();
assertThat(oldFilter).isInstanceOf(Log4j2Metrics.MetricsFilter.class);

// This will reload the configuration to default
context.reconfigure();

Configuration newConfiguration = context.getConfiguration();

// For this event to be counted, the metrics must be rebound
logger.error("second");
assertThat(registry.get("log4j2.events").tags("level", "error").counter().count()).isEqualTo(2);

// Should have removed filter from old configuration, adding it to the new
assertThat(oldConfiguration.getRootLogger().getFilter()).isNull();
Filter newFilter = newConfiguration.getRootLogger().getFilter();
assertThat(newFilter).isInstanceOf(Log4j2Metrics.MetricsFilter.class);
}
}

@Test
void shouldNotRebindMetricsIfBinderIsClosed() {
LoggerContext context = new LoggerContext("test");
Logger logger = context.getLogger("com.test");

try (Log4j2Metrics metrics = new Log4j2Metrics(emptyList(), context)) {
metrics.bindTo(registry);
logger.error("first");
assertThat(registry.get("log4j2.events").tags("level", "error").counter().count()).isEqualTo(1);
}

// This will reload the configuration to default
context.reconfigure();

// This event should not be counted as the metrics binder is already closed
logger.error("second");

assertThat(registry.get("log4j2.events").tags("level", "error").counter().count()).isEqualTo(1);
pativa marked this conversation as resolved.
Show resolved Hide resolved
}

@Test
void bindingTwiceToSameRegistry_doesNotDoubleCount() {
LoggerContext context = new LoggerContext("test");
Logger logger = context.getLogger("com.test");

try (Log4j2Metrics metrics = new Log4j2Metrics(emptyList(), context)) {
// binding twice
metrics.bindTo(registry);
metrics.bindTo(registry);

logger.error("first");

assertThat(registry.get("log4j2.events").tags("level", "error").counter().count()).isEqualTo(1);

context.reconfigure();

logger.error("second");
assertThat(registry.get("log4j2.events").tags("level", "error").counter().count()).isEqualTo(2);
}

// no additional events should be counted now
context.reconfigure();

logger.error("third");
assertThat(registry.get("log4j2.events").tags("level", "error").counter().count()).isEqualTo(2);
}

}