Merge pull request #10535 from jkschneider/micrometer-rc2-2

* pr/10535:
  Polish MetricsEndpoint
  Support composite registries in MetricsEndpoint

spring-boot-project/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/metrics/MetricsEndpoint.java

@@ -16,19 +16,22 @@
 
 package org.springframework.boot.actuate.metrics;
 
-import java.util.Collection;
+import java.util.ArrayList;
 import java.util.Collections;
 import java.util.HashMap;
+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;
 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;
@@ -40,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")
@@ -52,54 +56,43 @@ public class MetricsEndpoint {
 	}
 
 	@ReadOperation
-	public Map<String, List<String>> listNames() {
+	public ListNamesResponse listNames() {
-		return Collections.singletonMap("names", this.registry.getMeters().stream()
+		Set<String> names = new LinkedHashSet<>();
-				.map(this::getMeterIdName).distinct().collect(Collectors.toList()));
+		collectNames(names, this.registry);
+		return new ListNamesResponse(names);
 	}
 
-	private String getMeterIdName(Meter meter) {
+	private void collectNames(Set<String> names, MeterRegistry registry) {
+		if (registry instanceof CompositeMeterRegistry) {
+			((CompositeMeterRegistry) registry).getRegistries()
+					.forEach((member) -> collectNames(names, member));
+		}
+		else {
+			registry.getMeters().stream().map(this::getName).forEach(names::add);
+		}
+	}
+
+	private String getName(Meter meter) {
 		return meter.getId().getName();
 	}
 
 	@ReadOperation
-	public Response metric(@Selector String requiredMetricName,
+	public MetricResponse metric(@Selector String requiredMetricName,
 			@Nullable List<String> tag) {
 		Assert.isTrue(tag == null || tag.stream().allMatch((t) -> t.contains(":")),
 				"Each tag parameter must be in the form key:value");
 		List<Tag> tags = parseTags(tag);
-		Collection<Meter> meters = this.registry.find(requiredMetricName).tags(tags)
+		List<Meter> meters = new ArrayList<>();
-				.meters();
+		collectMeters(meters, this.registry, requiredMetricName, tags);
 		if (meters.isEmpty()) {
 			return null;
 		}
-
+		Map<Statistic, Double> samples = getSamples(meters);
-		Map<Statistic, Double> samples = new HashMap<>();
+		Map<String, List<String>> availableTags = getAvailableTags(meters);
-		Map<String, List<String>> 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()));
-			}
-		}
-
 		tags.forEach((t) -> availableTags.remove(t.getKey()));
-
+		return new MetricResponse(requiredMetricName,
-		return new Response(requiredMetricName,
+				asList(samples, MetricResponse.Sample::new),
-				samples.entrySet().stream()
+				asList(availableTags, MetricResponse.AvailableTag::new));
-						.map((sample) -> new Response.Sample(sample.getKey(),
-								sample.getValue()))
-				.collect(
-						Collectors.toList()),
-				availableTags.entrySet().stream()
-						.map((tagValues) -> new Response.AvailableTag(tagValues.getKey(),
-								tagValues.getValue()))
-						.collect(Collectors.toList()));
 	}
 
 	private List<Tag> parseTags(List<String> tags) {
@@ -109,10 +102,75 @@ public class MetricsEndpoint {
 		}).collect(Collectors.toList());
 	}
 
+	private void collectMeters(List<Meter> meters, MeterRegistry registry, String name,
+			Iterable<Tag> 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<Statistic, Double> getSamples(List<Meter> meters) {
+		Map<Statistic, Double> samples = new LinkedHashMap<>();
+		meters.forEach((meter) -> mergeMeasurements(samples, meter));
+		return samples;
+	}
+
+	private void mergeMeasurements(Map<Statistic, Double> samples, Meter meter) {
+		meter.measure().forEach((measurement) -> samples.merge(measurement.getStatistic(),
+				measurement.getValue(), Double::sum));
+	}
+
+	private Map<String, List<String>> getAvailableTags(List<Meter> meters) {
+		Map<String, List<String>> availableTags = new HashMap<>();
+		meters.forEach((meter) -> mergeAvailableTags(availableTags, meter));
+		return availableTags;
+	}
+
+	private void mergeAvailableTags(Map<String, List<String>> availableTags,
+			Meter meter) {
+		meter.getId().getTags().forEach((tag) -> {
+			List<String> value = Collections.singletonList(tag.getValue());
+			availableTags.merge(tag.getKey(), value, this::merge);
+		});
+	}
+
+	private <T> List<T> merge(List<T> list1, List<T> list2) {
+		List<T> result = new ArrayList<>(list1.size() + list2.size());
+		result.addAll(list1);
+		result.addAll(list2);
+		return result;
+	}
+
+	private <K, V, T> List<T> asList(Map<K, V> map, BiFunction<K, V, T> mapper) {
+		return map.entrySet().stream()
+				.map((entry) -> mapper.apply(entry.getKey(), entry.getValue()))
+				.collect(Collectors.toCollection(ArrayList::new));
+	}
+
 	/**
-	 * Response payload.
+	 * Response payload for a metric name listing.
 	 */
-	static class Response {
+	static class ListNamesResponse {
+
+		private final Set<String> names;
+
+		ListNamesResponse(Set<String> names) {
+			this.names = names;
+		}
+
+		public Set<String> getNames() {
+			return this.names;
+		}
+	}
+
+	/**
+	 * Response payload for a metric name selector.
+	 */
+	static class MetricResponse {
 
 		private final String name;
 
@@ -120,7 +178,7 @@ public class MetricsEndpoint {
 
 		private final List<AvailableTag> availableTags;
 
-		Response(String name, List<Sample> measurements,
+		MetricResponse(String name, List<Sample> measurements,
 				List<AvailableTag> availableTags) {
 			this.name = name;
 			this.measurements = measurements;
@@ -189,6 +247,8 @@ public class MetricsEndpoint {
 				return "MeasurementSample{" + "statistic=" + this.statistic + ", value="
 						+ this.value + '}';
 			}
+
 		}
+
 	}
 }

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<String, List<String>> result = this.endpoint.listNames();
+		MetricsEndpoint.ListNamesResponse result = this.endpoint.listNames();
-		assertThat(result).containsOnlyKeys("names");
+		assertThat(result.getNames()).isEmpty();
-		assertThat(result.get("names")).isEmpty();
 	}
 
 	@Test
@@ -53,18 +51,30 @@ public class MetricsEndpointTests {
 		this.registry.counter("com.example.foo");
 		this.registry.counter("com.example.bar");
 		this.registry.counter("com.example.foo");
-		Map<String, List<String>> result = this.endpoint.listNames();
+		MetricsEndpoint.ListNamesResponse result = this.endpoint.listNames();
-		assertThat(result).containsOnlyKeys("names");
+		assertThat(result.getNames()).containsOnlyOnce("com.example.foo",
-		assertThat(result.get("names")).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 +87,43 @@ 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<Double> getCount(MetricsEndpoint.Response response) {
+	private Optional<Double> 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<String> availableTagKeys(MetricsEndpoint.Response response) {
+	private Stream<String> availableTagKeys(MetricsEndpoint.MetricResponse response) {
 		return response.getAvailableTags().stream()
-				.map(MetricsEndpoint.Response.AvailableTag::getTag);
+				.map(MetricsEndpoint.MetricResponse.AvailableTag::getTag);
 	}
 
 }