diff --git a/micrometer-core/src/main/java/io/micrometer/core/instrument/binder/logging/Log4j2Metrics.java b/micrometer-core/src/main/java/io/micrometer/core/instrument/binder/logging/Log4j2Metrics.java index 166b424db9..11de4d6983 100644 --- a/micrometer-core/src/main/java/io/micrometer/core/instrument/binder/logging/Log4j2Metrics.java +++ b/micrometer-core/src/main/java/io/micrometer/core/instrument/binder/logging/Log4j2Metrics.java @@ -31,9 +31,12 @@ import org.apache.logging.log4j.core.filter.AbstractFilter; import org.apache.logging.log4j.core.filter.CompositeFilter; +import java.beans.PropertyChangeListener; import java.util.ArrayList; import java.util.Arrays; +import java.util.HashMap; import java.util.List; +import java.util.Map; import static java.util.Collections.emptyList; @@ -61,7 +64,9 @@ public class Log4j2Metrics implements MeterBinder, AutoCloseable { private final LoggerContext loggerContext; - private final List metricsFilters = new ArrayList<>(); + private final Map metricsFilters = new HashMap<>(); + + private final List changeListeners = new ArrayList<>(); public Log4j2Metrics() { this(emptyList()); @@ -78,12 +83,29 @@ public Log4j2Metrics(Iterable tags, LoggerContext loggerContext) { @Override public void bindTo(MeterRegistry registry) { - Configuration configuration = loggerContext.getConfiguration(); + PropertyChangeListener changeListener = listener -> { + if (listener.getNewValue() instanceof Configuration) { + Configuration configuration = (Configuration) listener.getNewValue(); + addMetricsFilterToConfiguration(configuration, registry); + } + }; + + loggerContext.addPropertyChangeListener(changeListener); + changeListeners.add(changeListener); + + // This will trigger the changeListener + loggerContext.updateLoggers(); + } + + private void addMetricsFilterToConfiguration(Configuration configuration, MeterRegistry registry) { LoggerConfig rootLoggerConfig = configuration.getRootLogger(); - rootLoggerConfig.addFilter(createMetricsFilterAndStart(registry)); - loggerContext.getConfiguration() - .getLoggers() + if (!isMetricsFilter(rootLoggerConfig.getFilter())) { + rootLoggerConfig.addFilter(metricsFilters.computeIfAbsent(rootLoggerConfig.getName(), + name -> createMetricsFilterAndStart(registry))); + } + + configuration.getLoggers() .values() .stream() .filter(loggerConfig -> !loggerConfig.isAdditive()) @@ -91,27 +113,21 @@ public void bindTo(MeterRegistry registry) { if (loggerConfig == rootLoggerConfig) { return; } - Filter logFilter = loggerConfig.getFilter(); - - if (logFilter instanceof CompositeFilter - && Arrays.stream(((CompositeFilter) logFilter).getFiltersArray()) - .anyMatch(innerFilter -> innerFilter instanceof MetricsFilter)) { - return; + if (!isMetricsFilter(loggerConfig.getFilter())) { + loggerConfig.addFilter(metricsFilters.computeIfAbsent(loggerConfig.getName(), + name -> createMetricsFilterAndStart(registry))); } - - if (logFilter instanceof MetricsFilter) { - return; - } - loggerConfig.addFilter(createMetricsFilterAndStart(registry)); }); + } - loggerContext.updateLoggers(configuration); + private boolean isMetricsFilter(Filter logFilter) { + return logFilter instanceof MetricsFilter || (logFilter instanceof CompositeFilter + && Arrays.stream(((CompositeFilter) logFilter).getFiltersArray()).anyMatch(this::isMetricsFilter)); } private MetricsFilter createMetricsFilterAndStart(MeterRegistry registry) { MetricsFilter metricsFilter = new MetricsFilter(registry, tags); metricsFilter.start(); - metricsFilters.add(metricsFilter); return metricsFilter; } @@ -120,7 +136,7 @@ public void close() { if (!metricsFilters.isEmpty()) { Configuration configuration = loggerContext.getConfiguration(); LoggerConfig rootLoggerConfig = configuration.getRootLogger(); - metricsFilters.forEach(rootLoggerConfig::removeFilter); + rootLoggerConfig.removeFilter(metricsFilters.get(rootLoggerConfig.getName())); loggerContext.getConfiguration() .getLoggers() @@ -129,12 +145,13 @@ public void close() { .filter(loggerConfig -> !loggerConfig.isAdditive()) .forEach(loggerConfig -> { if (loggerConfig != rootLoggerConfig) { - metricsFilters.forEach(loggerConfig::removeFilter); + loggerConfig.removeFilter(metricsFilters.get(loggerConfig.getName())); } }); + changeListeners.forEach(loggerContext::removePropertyChangeListener); loggerContext.updateLoggers(configuration); - metricsFilters.forEach(MetricsFilter::stop); + metricsFilters.values().forEach(MetricsFilter::stop); } } diff --git a/micrometer-core/src/test/java/io/micrometer/core/instrument/binder/logging/Log4j2MetricsTest.java b/micrometer-core/src/test/java/io/micrometer/core/instrument/binder/logging/Log4j2MetricsTest.java index 1916544f1a..77b774d723 100644 --- a/micrometer-core/src/test/java/io/micrometer/core/instrument/binder/logging/Log4j2MetricsTest.java +++ b/micrometer-core/src/test/java/io/micrometer/core/instrument/binder/logging/Log4j2MetricsTest.java @@ -184,4 +184,46 @@ void asyncLogShouldNotBeDuplicated() throws IOException { .until(() -> registry.get("log4j2.events").tags("level", "info").counter().count() == 3); } + @Issue("#5756") + @Test + void rebindsMetricsWhenConfigurationIsReloaded() { + MeterRegistry registry = new SimpleMeterRegistry(); + LoggerContext context = (LoggerContext) LogManager.getContext(false); + Logger logger = context.getLogger(getClass()); + + try (Log4j2Metrics metrics = new Log4j2Metrics(emptyList(), context)) { + metrics.bindTo(registry); + + logger.error("first"); + + // This will reload the configuration to default + context.reconfigure(); + + // 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); + } + } + + @Test + void shouldNotRebindMetricsIfBinderIsClosed() { + MeterRegistry registry = new SimpleMeterRegistry(); + LoggerContext context = (LoggerContext) LogManager.getContext(false); + Logger logger = context.getLogger(getClass()); + + try (Log4j2Metrics metrics = new Log4j2Metrics(emptyList(), context)) { + metrics.bindTo(registry); + logger.error("first"); + } + + // 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); + } + }