Handle programatically added log4j2 LoggerConfig (#5902)

The goal of this is to be able to bind to a newly added `LoggerConfig` even if the `Configuration` is the same. This means we always need to check if the filter was already added to avoid adding it twice. We need to keep track of if anything was changed to avoid an endless loop when we call `updateLoggers()` Signed-off-by: Patrik Ivarsson <patrik.ivarsson@avanza.se>
micrometer-metrics · Feb 18, 2025 · e4c9658 · e4c9658
1 parent c68cfdd
commit e4c9658
Show file tree

Hide file tree

Showing 2 changed files with 54 additions and 25 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
@@ -96,11 +96,13 @@ public void bindTo(MeterRegistry registry) {
         loggerContext.updateLoggers(configuration);
 
         PropertyChangeListener changeListener = listener -> {
-            if (listener.getNewValue() instanceof Configuration && listener.getOldValue() != listener.getNewValue()) {
+            if (listener.getNewValue() instanceof Configuration) {
                 Configuration newConfiguration = (Configuration) listener.getNewValue();
-                registerMetricsFilter(newConfiguration, registry);
-                loggerContext.updateLoggers(newConfiguration);
-                if (listener.getOldValue() instanceof Configuration) {
+                if (registerMetricsFilter(newConfiguration, registry)) {
+                    loggerContext.updateLoggers(newConfiguration);
+                }
+                if (listener.getOldValue() != listener.getNewValue()
+                        && listener.getOldValue() instanceof Configuration) {
                     removeMetricsFilter((Configuration) listener.getOldValue());
                 }
             }
@@ -110,32 +112,29 @@ public void bindTo(MeterRegistry registry) {
         loggerContext.addPropertyChangeListener(changeListener);
     }
 
-    private void registerMetricsFilter(Configuration configuration, MeterRegistry registry) {
-        LoggerConfig rootLoggerConfig = configuration.getRootLogger();
+    private boolean registerMetricsFilter(Configuration configuration, MeterRegistry registry) {
         MetricsFilter metricsFilter = getOrCreateMetricsFilterAndStart(registry);
-        rootLoggerConfig.addFilter(metricsFilter);
+        LoggerConfig rootLoggerConfig = configuration.getRootLogger();
+        boolean changed = addFilterIfAbsent(rootLoggerConfig, metricsFilter);
 
-        configuration.getLoggers()
-            .values()
-            .stream()
-            .filter(loggerConfig -> !loggerConfig.isAdditive())
-            .forEach(loggerConfig -> {
-                if (loggerConfig == rootLoggerConfig) {
-                    return;
-                }
-                Filter logFilter = loggerConfig.getFilter();
+        for (LoggerConfig loggerConfig : configuration.getLoggers().values()) {
+            if (loggerConfig != rootLoggerConfig && !loggerConfig.isAdditive()) {
+                changed |= addFilterIfAbsent(loggerConfig, metricsFilter);
+            }
+        }
 
-                if (metricsFilter.equals(logFilter)) {
-                    return;
-                }
+        return changed;
+    }
 
-                if (logFilter instanceof CompositeFilter
-                        && Arrays.asList(((CompositeFilter) logFilter).getFiltersArray()).contains(metricsFilter)) {
-                    return;
-                }
+    private boolean addFilterIfAbsent(LoggerConfig loggerConfig, MetricsFilter metricsFilter) {
+        Filter existingFilter = loggerConfig.getFilter();
+        if (existingFilter == metricsFilter || (existingFilter instanceof CompositeFilter
+                && Arrays.asList(((CompositeFilter) existingFilter).getFiltersArray()).contains(metricsFilter))) {
+            return false;
+        }
 
-                loggerConfig.addFilter(metricsFilter);
-            });
+        loggerConfig.addFilter(metricsFilter);
+        return true;
     }
 
     private MetricsFilter getOrCreateMetricsFilterAndStart(MeterRegistry registry) {

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
@@ -368,4 +368,34 @@ void bindingTwiceToSameRegistry_doesNotDoubleCount() {
         assertThat(registry.get("log4j2.events").tags("level", "error").counter().count()).isEqualTo(2);
     }
 
+    @Issue("#5901")
+    @Test
+    void programmaticallyAddedLoggerConfigShouldBeCounted() {
+        LoggerContext context = new LoggerContext("test");
+
+        Log4j2Metrics metrics = new Log4j2Metrics(emptyList(), context);
+        metrics.bindTo(registry);
+
+        Configuration configuration = context.getConfiguration();
+
+        // here we add a logger programmatically, metrics for this will not be counted
+        // until updateLoggers is called
+        LoggerConfig addedLoggerConfig = new LoggerConfig("com.test", Level.DEBUG, false);
+        configuration.addLogger("com.test", addedLoggerConfig);
+        Logger logger = context.getLogger("com.test");
+
+        assertThat(addedLoggerConfig.getFilter()).isNull();
+
+        context.updateLoggers(configuration);
+
+        assertThat(addedLoggerConfig.getFilter()).isInstanceOf(Log4j2Metrics.MetricsFilter.class);
+
+        logger.debug("test");
+        assertThat(registry.get("log4j2.events").tags("level", "debug").counter().count()).isEqualTo(1);
+
+        metrics.close();
+
+        assertThat(addedLoggerConfig.getFilter()).isNull();
+    }
+
 }