Skip to content

Commit

Permalink
Implement LogbackMetrics binder, ensure Meter deduplication in regist…
Browse files Browse the repository at this point in the history
…ries
  • Loading branch information
jkschneider committed May 16, 2017
1 parent 09daddc commit e2d873a
Show file tree
Hide file tree
Showing 20 changed files with 358 additions and 142 deletions.
10 changes: 3 additions & 7 deletions build.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -6,8 +6,6 @@ buildscript {
dependencies {
classpath 'org.junit.platform:junit-platform-gradle-plugin:1.0.0-M4'
}

configurations.classpath.resolutionStrategy.cacheDynamicVersionsFor 0, 'seconds'
}

plugins {
Expand Down Expand Up @@ -58,12 +56,14 @@ dependencies {
// cache monitoring
compile 'com.google.guava:guava:21.0', optional

// log monitoring
compile 'ch.qos.logback:logback-classic:latest.release', optional

// JUnit 5
testCompile 'org.junit.platform:junit-platform-launcher:1.0.0-M4' // see https://github.com/junit-team/junit5/issues/586 for why this is necessary
testCompile 'org.junit.jupiter:junit-jupiter-api:5.0.0-M4'
testCompile 'org.junit.jupiter:junit-jupiter-params:5.0.0-M4'
testRuntime 'org.junit.jupiter:junit-jupiter-engine:5.0.0-M4'
testRuntime 'ch.qos.logback:logback-classic:latest.release'

testCompile 'org.assertj:assertj-core:3.+'

Expand Down Expand Up @@ -100,10 +100,6 @@ jmh {
duplicateClassesStrategy = 'warn'
}

junitPlatform {
logManager 'org.apache.logging.log4j.jul.LogManager'
}

bintray.pkg {
labels = ['spring', 'metrics', 'prometheus', 'spectator']
}
Original file line number Diff line number Diff line change
Expand Up @@ -19,16 +19,14 @@
import org.springframework.metrics.instrument.binder.MeterBinder;

import javax.sql.DataSource;
import java.util.ArrayList;
import java.util.Collection;
import java.util.Map;
import java.util.Optional;
import java.util.*;
import java.util.function.ToDoubleFunction;
import java.util.stream.Stream;

import static java.util.Collections.emptyList;
import static java.util.stream.Collectors.toList;
import static java.util.stream.StreamSupport.stream;
import static org.springframework.metrics.instrument.Tags.tagList;

/**
* Creates and manages your application's set of meters. Exporters use the meter registry to iterate
Expand Down Expand Up @@ -76,7 +74,7 @@ default Counter counter(String name) {
* Measures the rate of some activity.
*/
default Counter counter(String name, String... tags) {
return counter(name, toTags(tags));
return counter(name, tagList(tags));
}

/**
Expand All @@ -102,7 +100,7 @@ default DistributionSummary distributionSummary(String name) {
* Measures the sample distribution of events.
*/
default DistributionSummary distributionSummary(String name, String... tags) {
return distributionSummary(name, toTags(tags));
return distributionSummary(name, tagList(tags));
}

/**
Expand All @@ -128,7 +126,7 @@ default Timer timer(String name) {
* Measures the time taken for short tasks.
*/
default Timer timer(String name, String... tags) {
return timer(name, toTags(tags));
return timer(name, tagList(tags));
}

/**
Expand All @@ -154,7 +152,7 @@ default LongTaskTimer longTaskTimer(String name) {
* Measures the time taken for short tasks.
*/
default LongTaskTimer longTaskTimer(String name, String... tags) {
return longTaskTimer(name, toTags(tags));
return longTaskTimer(name, tagList(tags));
}

/**
Expand Down Expand Up @@ -303,17 +301,6 @@ default <T extends Collection<?>> T collectionSize(String name, T collection) {
return mapSize(name, emptyList(), collection);
}

default Iterable<Tag> toTags(String... keyValues) {
if (keyValues.length % 2 == 1) {
throw new IllegalArgumentException("size must be even, it is a set of key=value pairs");
}
ArrayList<Tag> ts = new ArrayList<>(keyValues.length);
for (int i = 0; i < keyValues.length; i += 2) {
ts.add(new ImmutableTag(keyValues[i], keyValues[i + 1]));
}
return ts;
}

/**
* Execute an algorithm to bind one or more metrics to the registry.
*/
Expand Down
16 changes: 16 additions & 0 deletions src/main/java/org/springframework/metrics/instrument/Tags.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
package org.springframework.metrics.instrument;

import java.util.ArrayList;

public class Tags {
public static Iterable<Tag> tagList(String... keyValues) {
if (keyValues.length % 2 == 1) {
throw new IllegalArgumentException("size must be even, it is a set of key=value pairs");
}
ArrayList<Tag> ts = new ArrayList<>(keyValues.length);
for (int i = 0; i < keyValues.length; i += 2) {
ts.add(new ImmutableTag(keyValues[i], keyValues[i + 1]));
}
return ts;
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
package org.springframework.metrics.instrument.binder;

import ch.qos.logback.classic.LoggerContext;
import org.slf4j.LoggerFactory;
import org.springframework.metrics.instrument.MeterRegistry;

public class LogbackMetrics implements MeterBinder {
@Override
public void bindTo(MeterRegistry registry) {
LoggerContext context = (LoggerContext) LoggerFactory.getILoggerFactory();
context.addTurboFilter(new MetricsTurboFilter(registry));
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,34 @@
package org.springframework.metrics.instrument.binder;

import ch.qos.logback.classic.Level;
import ch.qos.logback.classic.Logger;
import ch.qos.logback.classic.turbo.TurboFilter;
import ch.qos.logback.core.spi.FilterReply;
import org.slf4j.Marker;
import org.springframework.metrics.instrument.Counter;
import org.springframework.metrics.instrument.MeterRegistry;

import java.util.HashMap;
import java.util.Map;

public class MetricsTurboFilter extends TurboFilter {
private final Map<Level, Counter> levelCounters;

public MetricsTurboFilter(MeterRegistry registry) {
levelCounters = new HashMap<Level, Counter>() {{
put(Level.ERROR, registry.counter("logback_events", "level", "error"));
put(Level.WARN, registry.counter("logback_events", "level", "warn"));
put(Level.INFO, registry.counter("logback_events", "level", "info"));
put(Level.DEBUG, registry.counter("logback_events", "level", "debug"));
put(Level.TRACE, registry.counter("logback_events", "level", "trace"));

This comment has been minimized.

Copy link
@checketts

checketts May 17, 2017

Contributor

Is it worth mimicking https://github.com/prometheus/client_java/blob/master/simpleclient_logback/src/main/java/io/prometheus/client/logback/InstrumentedAppender.java more closely? I realize you didn't use the prometheus one directly since we ant the instrumentation to apply regardless of underlying metric implementation.

Also would a switch be slightly more efficient than a map lookup and null check?

I don't typically focus on optimizations, but in the case of logging we do want it to as minimally intrusive as possible.

Lastly, it would be good to align with the existing prometheus metric name logback_appender_total (though if we aren't using an appender I guess it makes sense to change names)

This comment has been minimized.

Copy link
@jkschneider

jkschneider May 17, 2017

Author Contributor

Good suggestions, incorporated into 9f9ef7b. As you say, logback_appender_total no longer matches the underlying implementation :). Perhaps we should have constructors for these binders for you to provide a different name?

}};
}

@Override
public FilterReply decide(Marker marker, Logger logger, Level level, String format, Object[] params, Throwable t) {
Counter counter = levelCounters.get(level);
if(counter != null)
counter.increment();
return FilterReply.ACCEPT;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -34,10 +34,11 @@
import javax.sql.DataSource;
import java.util.ArrayList;
import java.util.Collection;
import java.util.HashSet;
import java.util.Set;
import java.util.stream.Stream;

public abstract class AbstractMeterRegistry implements MeterRegistry {
private Collection<Meter> meters = new ArrayList<>();
protected Clock clock;

@Autowired(required = false)
Expand All @@ -60,21 +61,11 @@ protected AbstractMeterRegistry(Clock clock) {
this.clock = clock;
}

protected <T extends Meter> T register(T meter) {
meters.add(meter);
return meter;
}

@Override
public Clock getClock() {
return clock;
}

@Override
public Collection<Meter> getMeters() {
return meters;
}

@Override
public Cache monitor(String name, Stream<Tag> tags, Cache cache) {
CacheStats stats = cache.stats();
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,53 @@
package org.springframework.metrics.instrument.internal;

import org.springframework.metrics.instrument.Tag;

import java.util.Arrays;
import java.util.Comparator;
import java.util.stream.StreamSupport;

public class MeterId {
private final String name;
private final Tag[] tags;

public MeterId(String name, Iterable<Tag> tags) {
this.name = name;
this.tags = StreamSupport.stream(tags.spliterator(), false).sorted(Comparator.comparing(Tag::getKey))
.toArray(Tag[]::new);
}

public static MeterId id(String name, Iterable<Tag> tags) {
return new MeterId(name, tags);
}

@Override
public boolean equals(Object o) {
if (this == o) return true;
if (o == null || getClass() != o.getClass()) return false;
MeterId meterId = (MeterId) o;
return (name != null ? name.equals(meterId.name) : meterId.name == null) && Arrays.equals(tags, meterId.tags);
}

@Override
public int hashCode() {
int result = name != null ? name.hashCode() : 0;
result = 31 * result + Arrays.hashCode(tags);
return result;
}

@Override
public String toString() {
return "MeterId{" +
"name='" + name + '\'' +
", tags=" + Arrays.toString(tags) +
'}';
}

public String getName() {
return name;
}

public Tag[] getTags() {
return tags;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -19,9 +19,9 @@

public class PrometheusCounter implements Counter {
private final String name;
private io.prometheus.client.Gauge.Child counter;
private io.prometheus.client.Counter.Child counter;

public PrometheusCounter(String name, io.prometheus.client.Gauge.Child counter) {
public PrometheusCounter(String name, io.prometheus.client.Counter.Child counter) {
this.name = name;
this.counter = counter;
}
Expand All @@ -33,10 +33,7 @@ public void increment() {

@Override
public void increment(double amount) {
if (amount < 0)
counter.dec(-amount);
else
counter.inc(amount);
counter.inc(amount);
}

@Override
Expand Down
Loading

0 comments on commit e2d873a

Please sign in to comment.