From b5c11a9892d7ac615a763f9689d269df08b48876 Mon Sep 17 00:00:00 2001
From: Tommy Ludwig <8924140+shakuzen@users.noreply.github.com>
Date: Wed, 5 Feb 2025 13:15:25 +0900
Subject: [PATCH] Compare MetricsFilter equality rather than instanceof

Adding the MetricsFilter was being skipped for non-root loggers because the `instanceof` check would return true. However, it is an instance for a different registry. This checks equality of the MetricsFilter and adds a test to verify.

Closes gh-5893
---
 .../binder/logging/Log4j2Metrics.java         | 13 ++++---
 .../binder/logging/Log4j2MetricsTest.java     | 37 +++++++++++++++++++
 2 files changed, 44 insertions(+), 6 deletions(-)

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 98e9b772f5..e2292e9bb9 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
@@ -80,7 +80,8 @@ public Log4j2Metrics(Iterable<Tag> tags, LoggerContext loggerContext) {
     public void bindTo(MeterRegistry registry) {
         Configuration configuration = loggerContext.getConfiguration();
         LoggerConfig rootLoggerConfig = configuration.getRootLogger();
-        rootLoggerConfig.addFilter(getOrCreateMetricsFilterAndStart(registry));
+        MetricsFilter metricsFilter = getOrCreateMetricsFilterAndStart(registry);
+        rootLoggerConfig.addFilter(metricsFilter);
 
         loggerContext.getConfiguration()
             .getLoggers()
@@ -93,16 +94,16 @@ public void bindTo(MeterRegistry registry) {
                 }
                 Filter logFilter = loggerConfig.getFilter();
 
-                if (logFilter instanceof CompositeFilter
-                        && Arrays.stream(((CompositeFilter) logFilter).getFiltersArray())
-                            .anyMatch(innerFilter -> innerFilter instanceof MetricsFilter)) {
+                if (metricsFilter.equals(logFilter)) {
                     return;
                 }
 
-                if (logFilter instanceof MetricsFilter) {
+                if (logFilter instanceof CompositeFilter
+                        && Arrays.asList(((CompositeFilter) logFilter).getFiltersArray()).contains(metricsFilter)) {
                     return;
                 }
-                loggerConfig.addFilter(getOrCreateMetricsFilterAndStart(registry));
+
+                loggerConfig.addFilter(metricsFilter);
             });
 
         loggerContext.updateLoggers(configuration);
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 29d4746b46..d75c25a73d 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
@@ -251,4 +251,41 @@ void multipleRegistriesCanBeBound() {
 
     }
 
+    @Test
+    void multipleRegistriesCanBeBoundWithNonRootLoggerContext() {
+        LoggerContext loggerContext = new LoggerContext("test");
+
+        LoggerConfig loggerConfig = new LoggerConfig("com.test", Level.INFO, false);
+        Configuration configuration = loggerContext.getConfiguration();
+        configuration.getRootLogger().setLevel(Level.INFO);
+        configuration.addLogger("com.test", loggerConfig);
+        loggerContext.updateLoggers(configuration);
+
+        MeterRegistry registry2 = new SimpleMeterRegistry();
+
+        Log4j2Metrics log4j2Metrics = new Log4j2Metrics(emptyList(), loggerContext);
+        log4j2Metrics.bindTo(registry);
+
+        // verify root logger
+        Logger logger = loggerContext.getLogger(Log4j2MetricsTest.class);
+        logger.info("Hello, world!");
+        assertThat(registry.get("log4j2.events").tags("level", "info").counter().count()).isEqualTo(1);
+
+        // verify other logger
+        Logger logger2 = loggerContext.getLogger("com.test");
+        logger2.info("Using other logger than root logger");
+        assertThat(registry.get("log4j2.events").tags("level", "info").counter().count()).isEqualTo(2);
+
+        log4j2Metrics.bindTo(registry2);
+
+        logger.info("Hello, world!");
+        assertThat(registry.get("log4j2.events").tags("level", "info").counter().count()).isEqualTo(3);
+        assertThat(registry2.get("log4j2.events").tags("level", "info").counter().count()).isEqualTo(1);
+
+        logger2.info("Using other logger than root logger");
+        assertThat(registry.get("log4j2.events").tags("level", "info").counter().count()).isEqualTo(4);
+        // this final check does not pass as the log event is not properly counted
+        assertThat(registry2.get("log4j2.events").tags("level", "info").counter().count()).isEqualTo(2);
+    }
+
 }