Rebind log4j2 metrics if configuration is changed

* Use `PropertyChangeListener` to rebind the `MetricsFilter` to log4j2 if configuration is reloaded * Keep track of the listener to deregister it when we close the binder Signed-off-by: Patrik Ivarsson <[email protected]>
micrometer-metrics · Jan 23, 2025 · 466e132 · 466e132
1 parent c39bda3
commit 466e132
Show file tree

Hide file tree

Showing 2 changed files with 95 additions and 19 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.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;
 
@@ -62,6 +65,7 @@ public class Log4j2Metrics implements MeterBinder, AutoCloseable {
     private final LoggerContext loggerContext;
 
     private final ConcurrentMap<MeterRegistry, MetricsFilter> metricsFilters = new ConcurrentHashMap<>();
+    private final List<PropertyChangeListener> changeListeners = new CopyOnWriteArrayList<>();
 
     public Log4j2Metrics() {
         this(emptyList());
@@ -79,10 +83,27 @@ public Log4j2Metrics(Iterable<Tag> tags, LoggerContext loggerContext) {
     @Override
     public void bindTo(MeterRegistry registry) {
         Configuration configuration = loggerContext.getConfiguration();
+        registerMetricsFilter(configuration, registry);
+        loggerContext.updateLoggers(configuration);
+
+        PropertyChangeListener changeListener = listener -> {
+            if (listener.getNewValue() instanceof Configuration && listener.getOldValue() != listener.getNewValue()) {
+                registerMetricsFilter((Configuration) listener.getNewValue(), registry);
+                if (listener.getOldValue() instanceof Configuration) {
+                    removeMetricsFilter((Configuration) listener.getOldValue());
+                }
+            }
+        };
+
+        changeListeners.add(changeListener);
+        loggerContext.addPropertyChangeListener(changeListener);
+    }
+
+    private void registerMetricsFilter(Configuration configuration, MeterRegistry registry) {
         LoggerConfig rootLoggerConfig = configuration.getRootLogger();
         rootLoggerConfig.addFilter(getOrCreateMetricsFilterAndStart(registry));
 
-        loggerContext.getConfiguration()
+        configuration
             .getLoggers()
             .values()
             .stream()
@@ -104,8 +125,6 @@ public void bindTo(MeterRegistry registry) {
                 }
                 loggerConfig.addFilter(getOrCreateMetricsFilterAndStart(registry));
             });
-
-        loggerContext.updateLoggers(configuration);
     }
 
     private MetricsFilter getOrCreateMetricsFilterAndStart(MeterRegistry registry) {
@@ -118,28 +137,33 @@ private MetricsFilter getOrCreateMetricsFilterAndStart(MeterRegistry registry) {
 
     @Override
     public void close() {
+        changeListeners.forEach(loggerContext::removePropertyChangeListener);
+
         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();
         }
     }
 
+    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(rootLoggerConfig::removeFilter);
+                }
+            });
+    }
+
     @NonNullApi
     @NonNullFields
     static class MetricsFilter extends AbstractFilter {

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
@@ -229,4 +229,56 @@ void multipleRegistriesCanBeBound() {
 
     }
 
+    @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");
+        }
+
+        // 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);
+    }
+
 }