Rebind log4j2 metrics if configuration is changed

* Use `PropertyChangeListener` to add the `MetricsFilter` to log4j2 if configuration is reloaded * When adding filters, check if the filter is already added * Keep `metricFilters` in a `HashMap` to avoid creating new filters each time configuration is reloaded Signed-off-by: Patrik Ivarsson <[email protected]>
micrometer-metrics · Jan 17, 2025 · ec8bd27 · ec8bd27
1 parent e8f41e5
commit ec8bd27
Show file tree

Hide file tree

Showing 2 changed files with 80 additions and 21 deletions.
diff --git a/...ometer-core/src/main/java/io/micrometer/core/instrument/binder/logging/Log4j2Metrics.java b/...ometer-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<MetricsFilter> metricsFilters = new ArrayList<>();
+    private final Map<String, MetricsFilter> metricsFilters = new HashMap<>();
+
+    private final List<PropertyChangeListener> changeListeners = new ArrayList<>();
 
     public Log4j2Metrics() {
         this(emptyList());
@@ -78,40 +83,51 @@ public Log4j2Metrics(Iterable<Tag> 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())
             .forEach(loggerConfig -> {
                 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/...er-core/src/test/java/io/micrometer/core/instrument/binder/logging/Log4j2MetricsTest.java b/...er-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);
+    }
+
 }