From ebb51d5533fa096d9479c27d5f3ac00e04b872fe Mon Sep 17 00:00:00 2001 From: Jon Schneider Date: Wed, 3 Jan 2018 13:45:27 -0600 Subject: [PATCH 1/2] Deduplicate tag values in metrics actuator endpoint See gh-11492 --- .../boot/actuate/metrics/MetricsEndpoint.java | 32 ++++++++----------- .../actuate/metrics/MetricsEndpointTests.java | 13 ++++++++ 2 files changed, 26 insertions(+), 19 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 ce13bcf9ae..151a80a8dd 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,10 +16,6 @@ package org.springframework.boot.actuate.metrics; -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; @@ -66,8 +62,7 @@ public class MetricsEndpoint { if (registry instanceof CompositeMeterRegistry) { ((CompositeMeterRegistry) registry).getRegistries() .forEach((member) -> collectNames(names, member)); - } - else { + } else { registry.getMeters().stream().map(this::getName).forEach(names::add); } } @@ -88,7 +83,7 @@ public class MetricsEndpoint { return null; } Map samples = getSamples(meters); - Map> availableTags = getAvailableTags(meters); + Map> availableTags = getAvailableTags(meters); tags.forEach((t) -> availableTags.remove(t.getKey())); return new MetricResponse(requiredMetricName, asList(samples, MetricResponse.Sample::new), @@ -107,8 +102,7 @@ public class MetricsEndpoint { if (registry instanceof CompositeMeterRegistry) { ((CompositeMeterRegistry) registry).getRegistries() .forEach((member) -> collectMeters(meters, member, name, tags)); - } - else { + } else { meters.addAll(registry.find(name).tags(tags).meters()); } } @@ -124,8 +118,8 @@ public class MetricsEndpoint { measurement.getValue(), Double::sum)); } - private Map> getAvailableTags(List meters) { - Map> availableTags = new HashMap<>(); + private Map> getAvailableTags(List meters) { + Map> availableTags = new HashMap<>(); meters.forEach((meter) -> mergeAvailableTags(availableTags, meter)); return availableTags; } @@ -133,15 +127,15 @@ public class MetricsEndpoint { private void mergeAvailableTags(Map> availableTags, Meter meter) { meter.getId().getTags().forEach((tag) -> { - List value = Collections.singletonList(tag.getValue()); + Set value = Collections.singleton(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); + private Set merge(Set set1, Set set2) { + Set result = new HashSet<>(set1.size() + set2.size()); + result.addAll(set1); + result.addAll(set2); return result; } @@ -204,9 +198,9 @@ public class MetricsEndpoint { private final String tag; - private final List values; + private final Set values; - AvailableTag(String tag, List values) { + AvailableTag(String tag, Set values) { this.tag = tag; this.values = values; } @@ -215,7 +209,7 @@ public class MetricsEndpoint { return this.tag; } - public List getValues() { + public Set getValues() { return this.values; } } 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 83f4326920..d2c8fe3c16 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 @@ -87,6 +87,19 @@ public class MetricsEndpointTests { assertThat(getCount(response)).hasValue(4.0); } + @Test + public void metricTagValuesAreDeduplicated() { + this.registry.counter("cache", "host", "1", "region", "east", "result", "hit"); + this.registry.counter("cache", "host", "1", "region", "east", "result", "miss"); + MetricsEndpoint.MetricResponse response = this.endpoint.metric("cache", + Collections.singletonList("host:1")); + assertThat(response.getAvailableTags() + .stream() + .filter(t -> t.getTag().equals("region")) + .flatMap(t -> t.getValues().stream())) + .containsExactly("east"); + } + @Test public void metricWithSpaceInTagValue() { this.registry.counter("counter", "key", "a space").increment(2); From 5baedf92750ddc04b635dcf348580f5d1eccbc07 Mon Sep 17 00:00:00 2001 From: Stephane Nicoll Date: Fri, 5 Jan 2018 13:41:49 +0100 Subject: [PATCH 2/2] Polish "Deduplicate tag values in metrics actuator endpoint" Closes gh-11492 --- .../boot/actuate/metrics/MetricsEndpoint.java | 15 +++++++++++---- 1 file changed, 11 insertions(+), 4 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 151a80a8dd..3188537336 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 @@ -1,5 +1,5 @@ /* - * Copyright 2012-2017 the original author or authors. + * Copyright 2012-2018 the original author or authors. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -16,6 +16,11 @@ package org.springframework.boot.actuate.metrics; +import java.util.ArrayList; +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; @@ -62,7 +67,8 @@ public class MetricsEndpoint { if (registry instanceof CompositeMeterRegistry) { ((CompositeMeterRegistry) registry).getRegistries() .forEach((member) -> collectNames(names, member)); - } else { + } + else { registry.getMeters().stream().map(this::getName).forEach(names::add); } } @@ -102,7 +108,8 @@ public class MetricsEndpoint { if (registry instanceof CompositeMeterRegistry) { ((CompositeMeterRegistry) registry).getRegistries() .forEach((member) -> collectMeters(meters, member, name, tags)); - } else { + } + else { meters.addAll(registry.find(name).tags(tags).meters()); } } @@ -124,7 +131,7 @@ public class MetricsEndpoint { return availableTags; } - private void mergeAvailableTags(Map> availableTags, + private void mergeAvailableTags(Map> availableTags, Meter meter) { meter.getId().getTags().forEach((tag) -> { Set value = Collections.singleton(tag.getValue());