Skip to content

Commit b5c11a9

Browse files
committed
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
1 parent bb2f27a commit b5c11a9

File tree

2 files changed

+44
-6
lines changed

2 files changed

+44
-6
lines changed

micrometer-core/src/main/java/io/micrometer/core/instrument/binder/logging/Log4j2Metrics.java

Lines changed: 7 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -80,7 +80,8 @@ public Log4j2Metrics(Iterable<Tag> tags, LoggerContext loggerContext) {
8080
public void bindTo(MeterRegistry registry) {
8181
Configuration configuration = loggerContext.getConfiguration();
8282
LoggerConfig rootLoggerConfig = configuration.getRootLogger();
83-
rootLoggerConfig.addFilter(getOrCreateMetricsFilterAndStart(registry));
83+
MetricsFilter metricsFilter = getOrCreateMetricsFilterAndStart(registry);
84+
rootLoggerConfig.addFilter(metricsFilter);
8485

8586
loggerContext.getConfiguration()
8687
.getLoggers()
@@ -93,16 +94,16 @@ public void bindTo(MeterRegistry registry) {
9394
}
9495
Filter logFilter = loggerConfig.getFilter();
9596

96-
if (logFilter instanceof CompositeFilter
97-
&& Arrays.stream(((CompositeFilter) logFilter).getFiltersArray())
98-
.anyMatch(innerFilter -> innerFilter instanceof MetricsFilter)) {
97+
if (metricsFilter.equals(logFilter)) {
9998
return;
10099
}
101100

102-
if (logFilter instanceof MetricsFilter) {
101+
if (logFilter instanceof CompositeFilter
102+
&& Arrays.asList(((CompositeFilter) logFilter).getFiltersArray()).contains(metricsFilter)) {
103103
return;
104104
}
105-
loggerConfig.addFilter(getOrCreateMetricsFilterAndStart(registry));
105+
106+
loggerConfig.addFilter(metricsFilter);
106107
});
107108

108109
loggerContext.updateLoggers(configuration);

micrometer-core/src/test/java/io/micrometer/core/instrument/binder/logging/Log4j2MetricsTest.java

Lines changed: 37 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -251,4 +251,41 @@ void multipleRegistriesCanBeBound() {
251251

252252
}
253253

254+
@Test
255+
void multipleRegistriesCanBeBoundWithNonRootLoggerContext() {
256+
LoggerContext loggerContext = new LoggerContext("test");
257+
258+
LoggerConfig loggerConfig = new LoggerConfig("com.test", Level.INFO, false);
259+
Configuration configuration = loggerContext.getConfiguration();
260+
configuration.getRootLogger().setLevel(Level.INFO);
261+
configuration.addLogger("com.test", loggerConfig);
262+
loggerContext.updateLoggers(configuration);
263+
264+
MeterRegistry registry2 = new SimpleMeterRegistry();
265+
266+
Log4j2Metrics log4j2Metrics = new Log4j2Metrics(emptyList(), loggerContext);
267+
log4j2Metrics.bindTo(registry);
268+
269+
// verify root logger
270+
Logger logger = loggerContext.getLogger(Log4j2MetricsTest.class);
271+
logger.info("Hello, world!");
272+
assertThat(registry.get("log4j2.events").tags("level", "info").counter().count()).isEqualTo(1);
273+
274+
// verify other logger
275+
Logger logger2 = loggerContext.getLogger("com.test");
276+
logger2.info("Using other logger than root logger");
277+
assertThat(registry.get("log4j2.events").tags("level", "info").counter().count()).isEqualTo(2);
278+
279+
log4j2Metrics.bindTo(registry2);
280+
281+
logger.info("Hello, world!");
282+
assertThat(registry.get("log4j2.events").tags("level", "info").counter().count()).isEqualTo(3);
283+
assertThat(registry2.get("log4j2.events").tags("level", "info").counter().count()).isEqualTo(1);
284+
285+
logger2.info("Using other logger than root logger");
286+
assertThat(registry.get("log4j2.events").tags("level", "info").counter().count()).isEqualTo(4);
287+
// this final check does not pass as the log event is not properly counted
288+
assertThat(registry2.get("log4j2.events").tags("level", "info").counter().count()).isEqualTo(2);
289+
}
290+
254291
}

0 commit comments

Comments
 (0)