From 30c036c7e6016e21b539242747afe245d3a7a1d7 Mon Sep 17 00:00:00 2001 From: Patrik Ivarsson Date: Mon, 20 Jan 2025 11:30:39 +0100 Subject: [PATCH] add additional verification Intended to verify that we don't continually create more filters when reloading config Signed-off-by: Patrik Ivarsson --- .../instrument/binder/logging/Log4j2Metrics.java | 3 +-- .../binder/logging/Log4j2MetricsTest.java | 16 +++++++++++----- 2 files changed, 12 insertions(+), 7 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 60617a01b6..4849e3caa7 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 @@ -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/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 d4f39df650..2f9546a997 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 @@ -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); } }