Spring/spring-boot
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

Stop MetricsEndpoint from summing up same metrics

Browse Source 
Update `MetricsEndpoint` so that only the first matching meter is used
when calculating the sum of of statistics.

Prior this this commit the endpoint would consider all Meters. This
caused incorrect statistics when multiple back-end systems were being
used since the registries contained in the `CompositeMeterRegistry`
would be  iterated, and the same effective metric would be counted more
than once.

Closes gh-14497
This commit is contained in:
pmehra
2018-09-17 16:36:40 -04:00
committed by Phillip Webb
parent 2a2908e74e
commit 950480dc1c
2 changed files with 66 additions and 10 deletions
Download Patch File Download Diff File Expand all files Collapse all files

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

@@ -16,7 +16,7 @@
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;
@@ -81,15 +81,15 @@ public class MetricsEndpoint {
public MetricResponse metric(@Selector String requiredMetricName,
@Nullable List<String> tag) {
List<Tag> tags = parseTags(tag);
List<Meter> meters = new ArrayList<>();
collectMeters(meters, this.registry, requiredMetricName, tags);
Collection<Meter> meters = findFirstMatchingMeters(this.registry,
requiredMetricName, tags);
if (meters.isEmpty()) {
return null;
}
Map<Statistic, Double> samples = getSamples(meters);
Map<String, Set<String>> availableTags = getAvailableTags(meters);
tags.forEach((t) -> availableTags.remove(t.getKey()));
Meter.Id meterId = meters.get(0).getId();
Meter.Id meterId = meters.iterator().next().getId();
return new MetricResponse(requiredMetricName, meterId.getDescription(),
meterId.getBaseUnit(), asList(samples, Sample::new),
asList(availableTags, AvailableTag::new));
@@ -112,18 +112,25 @@ public class MetricsEndpoint {
return Tag.of(parts[0], parts[1]);
}
private void collectMeters(List<Meter> meters, MeterRegistry registry, String name,
private Collection<Meter> findFirstMatchingMeters(MeterRegistry registry, String name,
Iterable<Tag> tags) {
if (registry instanceof CompositeMeterRegistry) {
((CompositeMeterRegistry) registry).getRegistries()
.forEach((member) -> collectMeters(meters, member, name, tags));
return ((CompositeMeterRegistry) registry).getRegistries().stream()
.map((r) -> findFirstMatchingMeters(r, name, tags))
.filter((match) -> !match.isEmpty()).findFirst()
.orElse(Collections.emptyList());
}
else {
meters.addAll(registry.find(name).tags(tags).meters());
Collection<Meter> metersFound = registry.find(name).tags(tags).meters();
if (!metersFound.isEmpty()) {
return metersFound;
}
}
return Collections.emptyList();
}
private Map<Statistic, Double> getSamples(List<Meter> meters) {
private Map<Statistic, Double> getSamples(Collection<Meter> meters) {
Map<Statistic, Double> samples = new LinkedHashMap<>();
meters.forEach((meter) -> mergeMeasurements(samples, meter));
return samples;
@@ -138,7 +145,7 @@ public class MetricsEndpoint {
return Statistic.MAX.equals(statistic) ? Double::max : Double::sum;
}
private Map<String, Set<String>> getAvailableTags(List<Meter> meters) {
private Map<String, Set<String>> getAvailableTags(Collection<Meter> meters) {
Map<String, Set<String>> availableTags = new HashMap<>();
meters.forEach((meter) -> mergeAvailableTags(availableTags, meter));
return availableTags;

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

@@ -95,6 +95,55 @@ public class MetricsEndpointTests {
assertThat(getCount(response)).hasValue(4.0);
}
@Test
public void findFirstMatchingMetersFromNestedRegistries() {
CompositeMeterRegistry composite = new CompositeMeterRegistry();
SimpleMeterRegistry reg1 = new SimpleMeterRegistry();
CompositeMeterRegistry reg2 = new CompositeMeterRegistry();
SimpleMeterRegistry reg3 = new SimpleMeterRegistry();
// 1st level nesting
composite.add(reg1);
// 2st level nesting
reg2.add(reg3);
composite.add(reg2);
// 2nd level registry has metrics
reg3.counter("cache", "result", "hit", "host", "1").increment(2);
reg3.counter("cache", "result", "miss", "host", "1").increment(2);
reg3.counter("cache", "result", "hit", "host", "2").increment(2);
MetricsEndpoint endpoint = new MetricsEndpoint(composite);
MetricsEndpoint.MetricResponse response = endpoint.metric("cache",
Collections.emptyList());
assertThat(response.getName()).isEqualTo("cache");
assertThat(availableTagKeys(response)).containsExactly("result", "host");
assertThat(getCount(response)).hasValue(6.0);
response = endpoint.metric("cache", Collections.singletonList("result:hit"));
assertThat(availableTagKeys(response)).containsExactly("host");
assertThat(getCount(response)).hasValue(4.0);
}
@Test
public void matchingMeterNotFoundInNestedRegistries() {
CompositeMeterRegistry composite = new CompositeMeterRegistry();
CompositeMeterRegistry reg2 = new CompositeMeterRegistry();
SimpleMeterRegistry reg3 = new SimpleMeterRegistry();
// nested registries
reg2.add(reg3);
composite.add(reg2);
MetricsEndpoint endpoint = new MetricsEndpoint(composite);
MetricsEndpoint.MetricResponse response = endpoint.metric("invalid.metric.name",
Collections.emptyList());
assertThat(response).isNull();
}
@Test
public void metricTagValuesAreDeduplicated() {
this.registry.counter("cache", "host", "1", "region", "east", "result", "hit");