Skip to content

Commit

Permalink
Fix multiple filter calls micrometer-metrics#2176
Browse files Browse the repository at this point in the history
  • Loading branch information
Dennys Fredericci committed Aug 11, 2020
1 parent a5a9608 commit 1c7141d
Show file tree
Hide file tree
Showing 2 changed files with 39 additions and 35 deletions.
55 changes: 20 additions & 35 deletions ...ometer-core/src/main/java/io/micrometer/core/instrument/binder/logging/Log4j2Metrics.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -22,19 +22,18 @@
import io.micrometer.core.instrument.binder.MeterBinder;
import io.micrometer.core.lang.NonNullApi;
import io.micrometer.core.lang.NonNullFields;
import io.micrometer.core.lang.Nullable;
import org.apache.logging.log4j.LogManager;
import org.apache.logging.log4j.core.Filter;
import org.apache.logging.log4j.core.LogEvent;
import org.apache.logging.log4j.core.LoggerContext;
import org.apache.logging.log4j.core.async.AsyncLoggerConfig;
import org.apache.logging.log4j.core.config.Configuration;
import org.apache.logging.log4j.core.config.Configurator;
import org.apache.logging.log4j.core.config.LoggerConfig;
import org.apache.logging.log4j.core.filter.AbstractFilter;
import org.apache.logging.log4j.core.filter.CompositeFilter;

import java.util.ArrayList;
import java.util.Arrays;
import java.util.List;

import static java.util.Collections.emptyList;

Expand All @@ -54,7 +53,8 @@ public class Log4j2Metrics implements MeterBinder, AutoCloseable {
private final Iterable<Tag> tags;
private final LoggerContext loggerContext;

private List<MetricsFilter> metricsFilters = new ArrayList<>();
@Nullable
private MetricsFilter metricsFilter;

public Log4j2Metrics() {
this(emptyList());
Expand All @@ -71,10 +71,12 @@ public Log4j2Metrics(Iterable<Tag> tags, LoggerContext loggerContext) {

@Override
public void bindTo(MeterRegistry registry) {
metricsFilter = new MetricsFilter(registry, tags);
metricsFilter.start();

Configuration configuration = loggerContext.getConfiguration();
LoggerConfig rootLoggerConfig = configuration.getRootLogger();
rootLoggerConfig.addFilter(createMetricsFilterAndStart(registry, rootLoggerConfig));
rootLoggerConfig.addFilter(metricsFilter);

loggerContext.getConfiguration().getLoggers().values().stream()
.filter(loggerConfig -> !loggerConfig.isAdditive())
Expand All @@ -92,36 +94,30 @@ public void bindTo(MeterRegistry registry) {
if (logFilter instanceof MetricsFilter) {
return;
}
loggerConfig.addFilter(createMetricsFilterAndStart(registry, loggerConfig));
loggerConfig.addFilter(metricsFilter);
});

loggerContext.updateLoggers(configuration);
}

private MetricsFilter createMetricsFilterAndStart(MeterRegistry registry, LoggerConfig loggerConfig) {
MetricsFilter metricsFilter = new MetricsFilter(registry, tags, loggerConfig instanceof AsyncLoggerConfig);
metricsFilter.start();
metricsFilters.add(metricsFilter);
return metricsFilter;
Configurator.reconfigure(configuration);
}

@Override
public void close() {
if (!metricsFilters.isEmpty()) {
if (metricsFilter != null) {
Configuration configuration = loggerContext.getConfiguration();
LoggerConfig rootLoggerConfig = configuration.getRootLogger();
metricsFilters.forEach(rootLoggerConfig::removeFilter);
rootLoggerConfig.removeFilter(metricsFilter);

loggerContext.getConfiguration().getLoggers().values().stream()
.filter(loggerConfig -> !loggerConfig.isAdditive())
.forEach(loggerConfig -> {
if (loggerConfig != rootLoggerConfig) {
metricsFilters.forEach(loggerConfig::removeFilter);
}
});
.filter(loggerConfig -> !loggerConfig.isAdditive())
.forEach(loggerConfig -> {
if (loggerConfig != rootLoggerConfig) {
loggerConfig.removeFilter(metricsFilter);
}
});

loggerContext.updateLoggers(configuration);
metricsFilters.forEach(MetricsFilter::stop);
metricsFilter.stop();
}
}

Expand All @@ -135,10 +131,8 @@ class MetricsFilter extends AbstractFilter {
private final Counter infoCounter;
private final Counter debugCounter;
private final Counter traceCounter;
private final boolean isAsyncLogger;

MetricsFilter(MeterRegistry registry, Iterable<Tag> tags, boolean isAsyncLogger) {
this.isAsyncLogger = isAsyncLogger;
MetricsFilter(MeterRegistry registry, Iterable<Tag> tags) {
fatalCounter = Counter.builder(METER_NAME)
.tags(tags)
.tags("level", "fatal")
Expand Down Expand Up @@ -184,19 +178,10 @@ class MetricsFilter extends AbstractFilter {

@Override
public Result filter(LogEvent event) {

if (!isAsyncLogger || isAsyncLoggerAndEndOfBatch(event)) {
incrementCounter(event);
}

incrementCounter(event);
return Result.NEUTRAL;
}


private boolean isAsyncLoggerAndEndOfBatch(LogEvent event) {
return isAsyncLogger && event.isEndOfBatch();
}

private void incrementCounter(LogEvent event) {
switch (event.getLevel().getStandardLevel()) {
case FATAL:
Expand Down
19 changes: 19 additions & 0 deletions ...er-core/src/test/java/io/micrometer/core/instrument/binder/logging/Log4j2MetricsTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -174,4 +174,23 @@ void asyncLogShouldNotBeDuplicated() throws IOException {
assertThat(registry.get("log4j2.events").tags("level", "info").counter().count()).isEqualTo(1);
}

@Issue("#2176")
@Test
void asyncLogShouldNotBeDuplicatedMultipleLogInfoCalls() throws IOException {
ConfigurationSource source = new ConfigurationSource(getClass().getResourceAsStream("/binder/logging/log4j2-async-logger.xml"));
Configurator.initialize(null, source);

Logger logger = LogManager.getLogger(Log4j2MetricsTest.class);

new Log4j2Metrics().bindTo(registry);

assertThat(registry.get("log4j2.events").tags("level", "info").counter().count()).isEqualTo(0);

for (int i = 0; i < 10; i++) {
logger.info("Hello, world! " + i);
}

assertThat(registry.get("log4j2.events").tags("level", "info").counter().count()).isEqualTo(10);
}

}

0 comments on commit 1c7141d

Please sign in to comment.