Skip to content

Commit 5f6698e

Browse files
committed
Merge pull request #14497 from pulkitmehra
* pr/14497: Polish "Stop MetricsEndpoint from summing up same metrics" Stop MetricsEndpoint from summing up same metrics
2 parents 2a2908e + 30ab4f9 commit 5f6698e

File tree

2 files changed

+53
-11
lines changed

2 files changed

+53
-11
lines changed

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

Lines changed: 17 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@
1717
package org.springframework.boot.actuate.metrics;
1818

1919
import java.util.ArrayList;
20+
import java.util.Collection;
2021
import java.util.Collections;
2122
import java.util.HashMap;
2223
import java.util.HashSet;
@@ -81,15 +82,15 @@ private String getName(Meter meter) {
8182
public MetricResponse metric(@Selector String requiredMetricName,
8283
@Nullable List<String> tag) {
8384
List<Tag> tags = parseTags(tag);
84-
List<Meter> meters = new ArrayList<>();
85-
collectMeters(meters, this.registry, requiredMetricName, tags);
85+
Collection<Meter> meters = findFirstMatchingMeters(this.registry,
86+
requiredMetricName, tags);
8687
if (meters.isEmpty()) {
8788
return null;
8889
}
8990
Map<Statistic, Double> samples = getSamples(meters);
9091
Map<String, Set<String>> availableTags = getAvailableTags(meters);
9192
tags.forEach((t) -> availableTags.remove(t.getKey()));
92-
Meter.Id meterId = meters.get(0).getId();
93+
Meter.Id meterId = meters.iterator().next().getId();
9394
return new MetricResponse(requiredMetricName, meterId.getDescription(),
9495
meterId.getBaseUnit(), asList(samples, Sample::new),
9596
asList(availableTags, AvailableTag::new));
@@ -112,18 +113,23 @@ private Tag parseTag(String tag) {
112113
return Tag.of(parts[0], parts[1]);
113114
}
114115

115-
private void collectMeters(List<Meter> meters, MeterRegistry registry, String name,
116+
private Collection<Meter> findFirstMatchingMeters(MeterRegistry registry, String name,
116117
Iterable<Tag> tags) {
117118
if (registry instanceof CompositeMeterRegistry) {
118-
((CompositeMeterRegistry) registry).getRegistries()
119-
.forEach((member) -> collectMeters(meters, member, name, tags));
120-
}
121-
else {
122-
meters.addAll(registry.find(name).tags(tags).meters());
119+
return findFirstMatchingMeters((CompositeMeterRegistry) registry, name, tags);
123120
}
121+
return registry.find(name).tags(tags).meters();
122+
}
123+
124+
private Collection<Meter> findFirstMatchingMeters(CompositeMeterRegistry composite,
125+
String name, Iterable<Tag> tags) {
126+
return composite.getRegistries().stream()
127+
.map((registry) -> findFirstMatchingMeters(registry, name, tags))
128+
.filter((matching) -> !matching.isEmpty()).findFirst()
129+
.orElse(Collections.emptyList());
124130
}
125131

126-
private Map<Statistic, Double> getSamples(List<Meter> meters) {
132+
private Map<Statistic, Double> getSamples(Collection<Meter> meters) {
127133
Map<Statistic, Double> samples = new LinkedHashMap<>();
128134
meters.forEach((meter) -> mergeMeasurements(samples, meter));
129135
return samples;
@@ -138,7 +144,7 @@ private BiFunction<Double, Double, Double> mergeFunction(Statistic statistic) {
138144
return Statistic.MAX.equals(statistic) ? Double::max : Double::sum;
139145
}
140146

141-
private Map<String, Set<String>> getAvailableTags(List<Meter> meters) {
147+
private Map<String, Set<String>> getAvailableTags(Collection<Meter> meters) {
142148
Map<String, Set<String>> availableTags = new HashMap<>();
143149
meters.forEach((meter) -> mergeAvailableTags(availableTags, meter));
144150
return availableTags;

spring-boot-project/spring-boot-actuator/src/test/java/org/springframework/boot/actuate/metrics/MetricsEndpointTests.java

Lines changed: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -95,6 +95,42 @@ public void metricValuesAreTheSumOfAllTimeSeriesMatchingTags() {
9595
assertThat(getCount(response)).hasValue(4.0);
9696
}
9797

98+
@Test
99+
public void findFirstMatchingMetersFromNestedRegistries() {
100+
CompositeMeterRegistry composite = new CompositeMeterRegistry();
101+
SimpleMeterRegistry firstLevel0 = new SimpleMeterRegistry();
102+
CompositeMeterRegistry firstLevel1 = new CompositeMeterRegistry();
103+
SimpleMeterRegistry secondLevel = new SimpleMeterRegistry();
104+
composite.add(firstLevel0);
105+
composite.add(firstLevel1);
106+
firstLevel1.add(secondLevel);
107+
secondLevel.counter("cache", "result", "hit", "host", "1").increment(2);
108+
secondLevel.counter("cache", "result", "miss", "host", "1").increment(2);
109+
secondLevel.counter("cache", "result", "hit", "host", "2").increment(2);
110+
MetricsEndpoint endpoint = new MetricsEndpoint(composite);
111+
MetricsEndpoint.MetricResponse response = endpoint.metric("cache",
112+
Collections.emptyList());
113+
assertThat(response.getName()).isEqualTo("cache");
114+
assertThat(availableTagKeys(response)).containsExactly("result", "host");
115+
assertThat(getCount(response)).hasValue(6.0);
116+
response = endpoint.metric("cache", Collections.singletonList("result:hit"));
117+
assertThat(availableTagKeys(response)).containsExactly("host");
118+
assertThat(getCount(response)).hasValue(4.0);
119+
}
120+
121+
@Test
122+
public void matchingMeterNotFoundInNestedRegistries() {
123+
CompositeMeterRegistry composite = new CompositeMeterRegistry();
124+
CompositeMeterRegistry firstLevel = new CompositeMeterRegistry();
125+
SimpleMeterRegistry secondLevel = new SimpleMeterRegistry();
126+
composite.add(firstLevel);
127+
firstLevel.add(secondLevel);
128+
MetricsEndpoint endpoint = new MetricsEndpoint(composite);
129+
MetricsEndpoint.MetricResponse response = endpoint.metric("invalid.metric.name",
130+
Collections.emptyList());
131+
assertThat(response).isNull();
132+
}
133+
98134
@Test
99135
public void metricTagValuesAreDeduplicated() {
100136
this.registry.counter("cache", "host", "1", "region", "east", "result", "hit");

0 commit comments

Comments
 (0)