From 37975836f05b85bae0110380a666fbe9ed467b0a Mon Sep 17 00:00:00 2001 From: Jon Schneider Date: Thu, 5 Oct 2017 22:43:10 -0500 Subject: [PATCH 1/2] Support composite registries in MetricsEndpoint Update `MetricsEndpoint` to deal with `CompositeMeterRegistry` instances. Closes gh-10535 --- .../boot/actuate/metrics/MetricsEndpoint.java | 76 +++++++++++++++---- .../actuate/metrics/MetricsEndpointTests.java | 58 ++++++++++---- 2 files changed, 104 insertions(+), 30 deletions(-) diff --git a/spring-boot-project/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/metrics/MetricsEndpoint.java b/spring-boot-project/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/metrics/MetricsEndpoint.java index f41c6cb50b4..53828019481 100644 --- a/spring-boot-project/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/metrics/MetricsEndpoint.java +++ b/spring-boot-project/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/metrics/MetricsEndpoint.java @@ -16,11 +16,14 @@ package org.springframework.boot.actuate.metrics; +import java.util.ArrayList; import java.util.Collection; import java.util.Collections; import java.util.HashMap; +import java.util.HashSet; import java.util.List; import java.util.Map; +import java.util.Set; import java.util.stream.Collectors; import java.util.stream.Stream; @@ -29,6 +32,7 @@ import io.micrometer.core.instrument.Meter; import io.micrometer.core.instrument.MeterRegistry; import io.micrometer.core.instrument.Statistic; import io.micrometer.core.instrument.Tag; +import io.micrometer.core.instrument.composite.CompositeMeterRegistry; import org.springframework.boot.actuate.endpoint.annotation.Endpoint; import org.springframework.boot.actuate.endpoint.annotation.ReadOperation; @@ -52,9 +56,22 @@ public class MetricsEndpoint { } @ReadOperation - public Map> listNames() { - return Collections.singletonMap("names", this.registry.getMeters().stream() - .map(this::getMeterIdName).distinct().collect(Collectors.toList())); + public ListNamesResponse listNames() { + return new ListNamesResponse(recurseListNames(this.registry)); + } + + private Set recurseListNames(MeterRegistry registry) { + Set names = new HashSet<>(); + if (registry instanceof CompositeMeterRegistry) { + for (MeterRegistry compositeMember : ((CompositeMeterRegistry) registry) + .getRegistries()) { + names.addAll(recurseListNames(compositeMember)); + } + } + else { + registry.getMeters().stream().map(this::getMeterIdName).forEach(names::add); + } + return names; } private String getMeterIdName(Meter meter) { @@ -62,13 +79,12 @@ public class MetricsEndpoint { } @ReadOperation - public Response metric(@Selector String requiredMetricName, + public MetricResponse metric(@Selector String requiredMetricName, @Nullable List tag) { Assert.isTrue(tag == null || tag.stream().allMatch((t) -> t.contains(":")), "Each tag parameter must be in the form key:value"); List tags = parseTags(tag); - Collection meters = this.registry.find(requiredMetricName).tags(tags) - .meters(); + Collection meters = recurseFindMeter(this.registry, requiredMetricName, tags); if (meters.isEmpty()) { return null; } @@ -90,18 +106,32 @@ public class MetricsEndpoint { tags.forEach((t) -> availableTags.remove(t.getKey())); - return new Response(requiredMetricName, + return new MetricResponse(requiredMetricName, samples.entrySet().stream() - .map((sample) -> new Response.Sample(sample.getKey(), + .map((sample) -> new MetricResponse.Sample(sample.getKey(), sample.getValue())) - .collect( - Collectors.toList()), + .collect(Collectors.toList()), availableTags.entrySet().stream() - .map((tagValues) -> new Response.AvailableTag(tagValues.getKey(), - tagValues.getValue())) + .map((tagValues) -> new MetricResponse.AvailableTag( + tagValues.getKey(), tagValues.getValue())) .collect(Collectors.toList())); } + private Collection recurseFindMeter(MeterRegistry registry, String name, + Iterable tags) { + Collection meters = new ArrayList<>(); + if (registry instanceof CompositeMeterRegistry) { + for (MeterRegistry compositeMember : ((CompositeMeterRegistry) registry) + .getRegistries()) { + meters.addAll(recurseFindMeter(compositeMember, name, tags)); + } + } + else { + meters.addAll(registry.find(name).tags(tags).meters()); + } + return meters; + } + private List parseTags(List tags) { return tags == null ? Collections.emptyList() : tags.stream().map((t) -> { String[] tagParts = t.split(":", 2); @@ -110,9 +140,25 @@ public class MetricsEndpoint { } /** - * Response payload. + * Response payload for a metric name listing. */ - static class Response { + static class ListNamesResponse { + + private final Set names; + + ListNamesResponse(Set names) { + this.names = names; + } + + public Set getNames() { + return this.names; + } + } + + /** + * Response payload for a metric name selector. + */ + static class MetricResponse { private final String name; @@ -120,7 +166,7 @@ public class MetricsEndpoint { private final List availableTags; - Response(String name, List measurements, + MetricResponse(String name, List measurements, List availableTags) { this.name = name; this.measurements = measurements; diff --git a/spring-boot-project/spring-boot-actuator/src/test/java/org/springframework/boot/actuate/metrics/MetricsEndpointTests.java b/spring-boot-project/spring-boot-actuator/src/test/java/org/springframework/boot/actuate/metrics/MetricsEndpointTests.java index f3e85f78331..496dbb0fef4 100644 --- a/spring-boot-project/spring-boot-actuator/src/test/java/org/springframework/boot/actuate/metrics/MetricsEndpointTests.java +++ b/spring-boot-project/spring-boot-actuator/src/test/java/org/springframework/boot/actuate/metrics/MetricsEndpointTests.java @@ -17,13 +17,12 @@ package org.springframework.boot.actuate.metrics; import java.util.Collections; -import java.util.List; -import java.util.Map; import java.util.Optional; import java.util.stream.Stream; import io.micrometer.core.instrument.MeterRegistry; import io.micrometer.core.instrument.Statistic; +import io.micrometer.core.instrument.composite.CompositeMeterRegistry; import io.micrometer.core.instrument.simple.SimpleMeterRegistry; import org.junit.Test; @@ -43,9 +42,8 @@ public class MetricsEndpointTests { @Test public void listNamesHandlesEmptyListOfMeters() { - Map> result = this.endpoint.listNames(); - assertThat(result).containsOnlyKeys("names"); - assertThat(result.get("names")).isEmpty(); + MetricsEndpoint.ListNamesResponse result = this.endpoint.listNames(); + assertThat(result.getNames()).isEmpty(); } @Test @@ -53,18 +51,32 @@ public class MetricsEndpointTests { this.registry.counter("com.example.foo"); this.registry.counter("com.example.bar"); this.registry.counter("com.example.foo"); - Map> result = this.endpoint.listNames(); - assertThat(result).containsOnlyKeys("names"); - assertThat(result.get("names")).containsOnlyOnce("com.example.foo", + MetricsEndpoint.ListNamesResponse result = this.endpoint.listNames(); + assertThat(result.getNames()).containsOnlyOnce("com.example.foo", "com.example.bar"); } + @Test + public void listNamesRecursesOverCompositeRegistries() { + CompositeMeterRegistry composite = new CompositeMeterRegistry(); + SimpleMeterRegistry reg1 = new SimpleMeterRegistry(); + SimpleMeterRegistry reg2 = new SimpleMeterRegistry(); + composite.add(reg1); + composite.add(reg2); + + reg1.counter("counter1").increment(); + reg2.counter("counter2").increment(); + + MetricsEndpoint endpoint = new MetricsEndpoint(composite); + assertThat(endpoint.listNames().getNames()).containsOnly("counter1", "counter2"); + } + @Test public void metricValuesAreTheSumOfAllTimeSeriesMatchingTags() { this.registry.counter("cache", "result", "hit", "host", "1").increment(2); this.registry.counter("cache", "result", "miss", "host", "1").increment(2); this.registry.counter("cache", "result", "hit", "host", "2").increment(2); - MetricsEndpoint.Response response = this.endpoint.metric("cache", + MetricsEndpoint.MetricResponse response = this.endpoint.metric("cache", Collections.emptyList()); assertThat(response.getName()).isEqualTo("cache"); assertThat(availableTagKeys(response)).containsExactly("result", "host"); @@ -77,29 +89,45 @@ public class MetricsEndpointTests { @Test public void metricWithSpaceInTagValue() { this.registry.counter("counter", "key", "a space").increment(2); - MetricsEndpoint.Response response = this.endpoint.metric("counter", + MetricsEndpoint.MetricResponse response = this.endpoint.metric("counter", Collections.singletonList("key:a space")); assertThat(response.getName()).isEqualTo("counter"); assertThat(availableTagKeys(response)).isEmpty(); assertThat(getCount(response)).hasValue(2.0); } + @Test + public void metricPresentInOneRegistryOfACompositeAndNotAnother() { + CompositeMeterRegistry composite = new CompositeMeterRegistry(); + SimpleMeterRegistry reg1 = new SimpleMeterRegistry(); + SimpleMeterRegistry reg2 = new SimpleMeterRegistry(); + composite.add(reg1); + composite.add(reg2); + + reg1.counter("counter1").increment(); + reg2.counter("counter2").increment(); + + MetricsEndpoint endpoint = new MetricsEndpoint(composite); + assertThat(endpoint.metric("counter1", Collections.emptyList())).isNotNull(); + assertThat(endpoint.metric("counter2", Collections.emptyList())).isNotNull(); + } + @Test public void nonExistentMetric() { - MetricsEndpoint.Response response = this.endpoint.metric("does.not.exist", + MetricsEndpoint.MetricResponse response = this.endpoint.metric("does.not.exist", Collections.emptyList()); assertThat(response).isNull(); } - private Optional getCount(MetricsEndpoint.Response response) { + private Optional getCount(MetricsEndpoint.MetricResponse response) { return response.getMeasurements().stream() .filter((ms) -> ms.getStatistic().equals(Statistic.Count)).findAny() - .map(MetricsEndpoint.Response.Sample::getValue); + .map(MetricsEndpoint.MetricResponse.Sample::getValue); } - private Stream availableTagKeys(MetricsEndpoint.Response response) { + private Stream availableTagKeys(MetricsEndpoint.MetricResponse response) { return response.getAvailableTags().stream() - .map(MetricsEndpoint.Response.AvailableTag::getTag); + .map(MetricsEndpoint.MetricResponse.AvailableTag::getTag); } } From 22a6ee03ebe9326364c3c79318942dc0694189f3 Mon Sep 17 00:00:00 2001 From: Phillip Webb Date: Tue, 10 Oct 2017 13:19:39 -0700 Subject: [PATCH 2/2] Polish MetricsEndpoint See gh-10535 --- .../boot/actuate/metrics/MetricsEndpoint.java | 124 ++++++++++-------- .../actuate/metrics/MetricsEndpointTests.java | 4 - 2 files changed, 69 insertions(+), 59 deletions(-) diff --git a/spring-boot-project/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/metrics/MetricsEndpoint.java b/spring-boot-project/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/metrics/MetricsEndpoint.java index 53828019481..ce13bcf9ae5 100644 --- a/spring-boot-project/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/metrics/MetricsEndpoint.java +++ b/spring-boot-project/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/metrics/MetricsEndpoint.java @@ -17,17 +17,16 @@ package org.springframework.boot.actuate.metrics; import java.util.ArrayList; -import java.util.Collection; import java.util.Collections; import java.util.HashMap; -import java.util.HashSet; +import java.util.LinkedHashMap; +import java.util.LinkedHashSet; import java.util.List; import java.util.Map; import java.util.Set; +import java.util.function.BiFunction; import java.util.stream.Collectors; -import java.util.stream.Stream; -import io.micrometer.core.instrument.Measurement; import io.micrometer.core.instrument.Meter; import io.micrometer.core.instrument.MeterRegistry; import io.micrometer.core.instrument.Statistic; @@ -44,6 +43,7 @@ import org.springframework.util.Assert; * An {@link Endpoint} for exposing the metrics held by a {@link MeterRegistry}. * * @author Jon Schneider + * @author Phillip Webb * @since 2.0.0 */ @Endpoint(id = "metrics") @@ -57,24 +57,22 @@ public class MetricsEndpoint { @ReadOperation public ListNamesResponse listNames() { - return new ListNamesResponse(recurseListNames(this.registry)); + Set names = new LinkedHashSet<>(); + collectNames(names, this.registry); + return new ListNamesResponse(names); } - private Set recurseListNames(MeterRegistry registry) { - Set names = new HashSet<>(); + private void collectNames(Set names, MeterRegistry registry) { if (registry instanceof CompositeMeterRegistry) { - for (MeterRegistry compositeMember : ((CompositeMeterRegistry) registry) - .getRegistries()) { - names.addAll(recurseListNames(compositeMember)); - } + ((CompositeMeterRegistry) registry).getRegistries() + .forEach((member) -> collectNames(names, member)); } else { - registry.getMeters().stream().map(this::getMeterIdName).forEach(names::add); + registry.getMeters().stream().map(this::getName).forEach(names::add); } - return names; } - private String getMeterIdName(Meter meter) { + private String getName(Meter meter) { return meter.getId().getName(); } @@ -84,52 +82,17 @@ public class MetricsEndpoint { Assert.isTrue(tag == null || tag.stream().allMatch((t) -> t.contains(":")), "Each tag parameter must be in the form key:value"); List tags = parseTags(tag); - Collection meters = recurseFindMeter(this.registry, requiredMetricName, tags); + List meters = new ArrayList<>(); + collectMeters(meters, this.registry, requiredMetricName, tags); if (meters.isEmpty()) { return null; } - - Map samples = new HashMap<>(); - Map> availableTags = new HashMap<>(); - - for (Meter meter : meters) { - for (Measurement ms : meter.measure()) { - samples.merge(ms.getStatistic(), ms.getValue(), Double::sum); - } - for (Tag availableTag : meter.getId().getTags()) { - availableTags.merge(availableTag.getKey(), - Collections.singletonList(availableTag.getValue()), - (t1, t2) -> Stream.concat(t1.stream(), t2.stream()) - .collect(Collectors.toList())); - } - } - + Map samples = getSamples(meters); + Map> availableTags = getAvailableTags(meters); tags.forEach((t) -> availableTags.remove(t.getKey())); - return new MetricResponse(requiredMetricName, - samples.entrySet().stream() - .map((sample) -> new MetricResponse.Sample(sample.getKey(), - sample.getValue())) - .collect(Collectors.toList()), - availableTags.entrySet().stream() - .map((tagValues) -> new MetricResponse.AvailableTag( - tagValues.getKey(), tagValues.getValue())) - .collect(Collectors.toList())); - } - - private Collection recurseFindMeter(MeterRegistry registry, String name, - Iterable tags) { - Collection meters = new ArrayList<>(); - if (registry instanceof CompositeMeterRegistry) { - for (MeterRegistry compositeMember : ((CompositeMeterRegistry) registry) - .getRegistries()) { - meters.addAll(recurseFindMeter(compositeMember, name, tags)); - } - } - else { - meters.addAll(registry.find(name).tags(tags).meters()); - } - return meters; + asList(samples, MetricResponse.Sample::new), + asList(availableTags, MetricResponse.AvailableTag::new)); } private List parseTags(List tags) { @@ -139,6 +102,55 @@ public class MetricsEndpoint { }).collect(Collectors.toList()); } + private void collectMeters(List meters, MeterRegistry registry, String name, + Iterable tags) { + if (registry instanceof CompositeMeterRegistry) { + ((CompositeMeterRegistry) registry).getRegistries() + .forEach((member) -> collectMeters(meters, member, name, tags)); + } + else { + meters.addAll(registry.find(name).tags(tags).meters()); + } + } + + private Map getSamples(List meters) { + Map samples = new LinkedHashMap<>(); + meters.forEach((meter) -> mergeMeasurements(samples, meter)); + return samples; + } + + private void mergeMeasurements(Map samples, Meter meter) { + meter.measure().forEach((measurement) -> samples.merge(measurement.getStatistic(), + measurement.getValue(), Double::sum)); + } + + private Map> getAvailableTags(List meters) { + Map> availableTags = new HashMap<>(); + meters.forEach((meter) -> mergeAvailableTags(availableTags, meter)); + return availableTags; + } + + private void mergeAvailableTags(Map> availableTags, + Meter meter) { + meter.getId().getTags().forEach((tag) -> { + List value = Collections.singletonList(tag.getValue()); + availableTags.merge(tag.getKey(), value, this::merge); + }); + } + + private List merge(List list1, List list2) { + List result = new ArrayList<>(list1.size() + list2.size()); + result.addAll(list1); + result.addAll(list2); + return result; + } + + private List asList(Map map, BiFunction mapper) { + return map.entrySet().stream() + .map((entry) -> mapper.apply(entry.getKey(), entry.getValue())) + .collect(Collectors.toCollection(ArrayList::new)); + } + /** * Response payload for a metric name listing. */ @@ -235,6 +247,8 @@ public class MetricsEndpoint { return "MeasurementSample{" + "statistic=" + this.statistic + ", value=" + this.value + '}'; } + } + } } diff --git a/spring-boot-project/spring-boot-actuator/src/test/java/org/springframework/boot/actuate/metrics/MetricsEndpointTests.java b/spring-boot-project/spring-boot-actuator/src/test/java/org/springframework/boot/actuate/metrics/MetricsEndpointTests.java index 496dbb0fef4..8b36d0cf119 100644 --- a/spring-boot-project/spring-boot-actuator/src/test/java/org/springframework/boot/actuate/metrics/MetricsEndpointTests.java +++ b/spring-boot-project/spring-boot-actuator/src/test/java/org/springframework/boot/actuate/metrics/MetricsEndpointTests.java @@ -63,10 +63,8 @@ public class MetricsEndpointTests { SimpleMeterRegistry reg2 = new SimpleMeterRegistry(); composite.add(reg1); composite.add(reg2); - reg1.counter("counter1").increment(); reg2.counter("counter2").increment(); - MetricsEndpoint endpoint = new MetricsEndpoint(composite); assertThat(endpoint.listNames().getNames()).containsOnly("counter1", "counter2"); } @@ -103,10 +101,8 @@ public class MetricsEndpointTests { SimpleMeterRegistry reg2 = new SimpleMeterRegistry(); composite.add(reg1); composite.add(reg2); - reg1.counter("counter1").increment(); reg2.counter("counter2").increment(); - MetricsEndpoint endpoint = new MetricsEndpoint(composite); assertThat(endpoint.metric("counter1", Collections.emptyList())).isNotNull(); assertThat(endpoint.metric("counter2", Collections.emptyList())).isNotNull();