add additional verification

Intended to verify that we don't continually create more filters when reloading config Signed-off-by: Patrik Ivarsson <[email protected]>
micrometer-metrics · Jan 20, 2025 · 30c036c · 30c036c
1 parent 41d0e7e
commit 30c036c
Show file tree

Hide file tree

Showing 2 changed files with 12 additions and 7 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
@@ -89,8 +89,7 @@ public void bindTo(MeterRegistry registry) {
 
         PropertyChangeListener changeListener = listener -> {
             if (listener.getNewValue() instanceof Configuration && listener.getOldValue() != listener.getNewValue()) {
-                Configuration newConfiguration = (Configuration) listener.getNewValue();
-                addMetricsFilterToConfiguration(newConfiguration, registry);
+                addMetricsFilterToConfiguration((Configuration) listener.getNewValue(), registry);
                 if (listener.getOldValue() instanceof Configuration) {
                     removeMetricsFilter((Configuration) listener.getOldValue());
                 }

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
@@ -23,6 +23,7 @@
 import org.apache.logging.log4j.Level;
 import org.apache.logging.log4j.LogManager;
 import org.apache.logging.log4j.Logger;
+import org.apache.logging.log4j.core.Filter;
 import org.apache.logging.log4j.core.LoggerContext;
 import org.apache.logging.log4j.core.config.Configuration;
 import org.apache.logging.log4j.core.config.ConfigurationSource;
@@ -195,9 +196,11 @@ void rebindsMetricsWhenConfigurationIsReloaded() {
             metrics.bindTo(registry);
 
             logger.error("first");
+            assertThat(registry.get("log4j2.events").tags("level", "error").counter().count()).isEqualTo(1);
 
             // Should have added filter to configuration
-            assertThat(oldConfiguration.getRootLogger().getFilter()).isInstanceOf(Log4j2Metrics.MetricsFilter.class);
+            Filter oldFilter = oldConfiguration.getRootLogger().getFilter();
+            assertThat(oldFilter).isInstanceOf(Log4j2Metrics.MetricsFilter.class);
 
             // This will reload the configuration to default
             context.reconfigure();
@@ -206,13 +209,16 @@ void rebindsMetricsWhenConfigurationIsReloaded() {
 
             // 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 new
-            // configuration
+            // Should have removed filter from old configuration, adding it to the new
             assertThat(oldConfiguration.getRootLogger().getFilter()).isNull();
-            assertThat(newConfiguration.getRootLogger().getFilter()).isInstanceOf(Log4j2Metrics.MetricsFilter.class);
+            Filter newFilter = newConfiguration.getRootLogger().getFilter();
+            assertThat(newFilter).isInstanceOf(Log4j2Metrics.MetricsFilter.class);
 
-            assertThat(registry.get("log4j2.events").tags("level", "error").counter().count()).isEqualTo(2);
+            // The filters should be the same instance, to verify that we don't
+            // continually create more filters when reloading
+            assertThat(oldFilter).isSameAs(newFilter);
         }
     }