From 950480dc1c500507cc0551c353a3b5e4198b3f25 Mon Sep 17 00:00:00 2001 From: pmehra Date: Mon, 17 Sep 2018 16:36:40 -0400 Subject: [PATCH 1/5] Stop MetricsEndpoint from summing up same metrics 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 --- .../boot/actuate/metrics/MetricsEndpoint.java | 27 ++++++---- .../actuate/metrics/MetricsEndpointTests.java | 49 +++++++++++++++++++ 2 files changed, 66 insertions(+), 10 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 abb0109a66..84738ac5a5 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,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 tag) { List tags = parseTags(tag); - List meters = new ArrayList<>(); - collectMeters(meters, this.registry, requiredMetricName, tags); + Collection meters = findFirstMatchingMeters(this.registry, + requiredMetricName, tags); if (meters.isEmpty()) { return null; } Map samples = getSamples(meters); Map> 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 meters, MeterRegistry registry, String name, + private Collection findFirstMatchingMeters(MeterRegistry registry, String name, Iterable 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 metersFound = registry.find(name).tags(tags).meters(); + if (!metersFound.isEmpty()) { + return metersFound; + } } + return Collections.emptyList(); } - private Map getSamples(List meters) { + private Map getSamples(Collection meters) { Map 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> getAvailableTags(List meters) { + private Map> getAvailableTags(Collection meters) { Map> availableTags = new HashMap<>(); meters.forEach((meter) -> mergeAvailableTags(availableTags, meter)); return availableTags; 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 3fe4279a38..3ad8e82c23 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 @@ -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"); From 30ab4f96914cd49c17c749dcaab53d7fc554653f Mon Sep 17 00:00:00 2001 From: Phillip Webb Date: Thu, 11 Oct 2018 14:37:44 -0700 Subject: [PATCH 2/5] Polish "Stop MetricsEndpoint from summing up same metrics" See gh-14497 --- .../boot/actuate/metrics/MetricsEndpoint.java | 23 ++++++----- .../actuate/metrics/MetricsEndpointTests.java | 39 +++++++------------ 2 files changed, 24 insertions(+), 38 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 84738ac5a5..5a9e2b25f3 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,6 +16,7 @@ package org.springframework.boot.actuate.metrics; +import java.util.ArrayList; import java.util.Collection; import java.util.Collections; import java.util.HashMap; @@ -115,19 +116,17 @@ public class MetricsEndpoint { private Collection findFirstMatchingMeters(MeterRegistry registry, String name, Iterable tags) { if (registry instanceof CompositeMeterRegistry) { - return ((CompositeMeterRegistry) registry).getRegistries().stream() - .map((r) -> findFirstMatchingMeters(r, name, tags)) - .filter((match) -> !match.isEmpty()).findFirst() - .orElse(Collections.emptyList()); + return findFirstMatchingMeters((CompositeMeterRegistry) registry, name, tags); + } + return registry.find(name).tags(tags).meters(); + } - } - else { - Collection metersFound = registry.find(name).tags(tags).meters(); - if (!metersFound.isEmpty()) { - return metersFound; - } - } - return Collections.emptyList(); + private Collection findFirstMatchingMeters(CompositeMeterRegistry composite, + String name, Iterable tags) { + return composite.getRegistries().stream() + .map((registry) -> findFirstMatchingMeters(registry, name, tags)) + .filter((matching) -> !matching.isEmpty()).findFirst() + .orElse(Collections.emptyList()); } private Map getSamples(Collection meters) { 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 3ad8e82c23..a417c40b5e 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 @@ -98,30 +98,21 @@ public class MetricsEndpointTests { @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); - + SimpleMeterRegistry firstLevel0 = new SimpleMeterRegistry(); + CompositeMeterRegistry firstLevel1 = new CompositeMeterRegistry(); + SimpleMeterRegistry secondLevel = new SimpleMeterRegistry(); + composite.add(firstLevel0); + composite.add(firstLevel1); + firstLevel1.add(secondLevel); + secondLevel.counter("cache", "result", "hit", "host", "1").increment(2); + secondLevel.counter("cache", "result", "miss", "host", "1").increment(2); + secondLevel.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); @@ -130,15 +121,11 @@ public class MetricsEndpointTests { @Test public void matchingMeterNotFoundInNestedRegistries() { CompositeMeterRegistry composite = new CompositeMeterRegistry(); - CompositeMeterRegistry reg2 = new CompositeMeterRegistry(); - SimpleMeterRegistry reg3 = new SimpleMeterRegistry(); - - // nested registries - reg2.add(reg3); - composite.add(reg2); - + CompositeMeterRegistry firstLevel = new CompositeMeterRegistry(); + SimpleMeterRegistry secondLevel = new SimpleMeterRegistry(); + composite.add(firstLevel); + firstLevel.add(secondLevel); MetricsEndpoint endpoint = new MetricsEndpoint(composite); - MetricsEndpoint.MetricResponse response = endpoint.metric("invalid.metric.name", Collections.emptyList()); assertThat(response).isNull(); From 1c3987d55ae7a39952355f35935ea9b914d9a554 Mon Sep 17 00:00:00 2001 From: Phillip Webb Date: Thu, 11 Oct 2018 15:42:56 -0700 Subject: [PATCH 3/5] Fix documentation of devtools Gradle scope Update the reference documentation to suggest that devtools uses a custom `developmentOnly` scope, rather than `compileOnly`. Closes gh-14451 --- .../src/main/asciidoc/using-spring-boot.adoc | 13 ++++++++++--- 1 file changed, 10 insertions(+), 3 deletions(-) diff --git a/spring-boot-project/spring-boot-docs/src/main/asciidoc/using-spring-boot.adoc b/spring-boot-project/spring-boot-docs/src/main/asciidoc/using-spring-boot.adoc index c54d69b64f..df9d038111 100644 --- a/spring-boot-project/spring-boot-docs/src/main/asciidoc/using-spring-boot.adoc +++ b/spring-boot-project/spring-boot-docs/src/main/asciidoc/using-spring-boot.adoc @@ -747,16 +747,23 @@ listings for Maven and Gradle: .Gradle [source,groovy,indent=0,subs="attributes"] ---- + configurations { + developmentOnly + runtimeClasspath { + extendsFrom developmentOnly + } + } dependencies { - compile("org.springframework.boot:spring-boot-devtools") + developmentOnly("org.springframework.boot:spring-boot-devtools") } ---- NOTE: Developer tools are automatically disabled when running a fully packaged application. If your application is launched from `java -jar` or if it is started from a special classloader, then it is considered a "`production application`". Flagging the -dependency as optional in Maven or using `compileOnly` in Gradle is a best practice that -prevents devtools from being transitively applied to other modules that use your project. +dependency as optional in Maven or using a custom`developmentOnly` configuration in +Gradle (as shown above) is a best practice that prevents devtools from being transitively +applied to other modules that use your project. TIP: Repackaged archives do not contain devtools by default. If you want to use a <>, you need to disable the From 0d35af1813f8fa447f8d96695f054b990bb09625 Mon Sep 17 00:00:00 2001 From: Phillip Webb Date: Thu, 11 Oct 2018 16:03:42 -0700 Subject: [PATCH 4/5] Add "Encrypting Properties" documentation Update the reference documentation with a section about encrypting properties and a link to Spring Cloud Vault. Closes gh-13618 --- .../src/main/asciidoc/spring-boot-features.adoc | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/spring-boot-project/spring-boot-docs/src/main/asciidoc/spring-boot-features.adoc b/spring-boot-project/spring-boot-docs/src/main/asciidoc/spring-boot-features.adoc index 471711b5b2..1896bda8f9 100644 --- a/spring-boot-project/spring-boot-docs/src/main/asciidoc/spring-boot-features.adoc +++ b/spring-boot-project/spring-boot-docs/src/main/asciidoc/spring-boot-features.adoc @@ -650,6 +650,21 @@ details. +[[boot-features-encrypting-properties]] +=== Encrypting Properties +Spring Boot does not provide any built in support for encrypting property values, however, +it does provide the hook points necessary to modify values contained in the Spring +`Environment`. The `EnvironmentPostProcessor` interface allows you to manipulate the +`Environment` before the application starts. See <> +for details. + +If you're looking for a secure way to store credentials and passwords, the +https://cloud.spring.io/spring-cloud-vault/[Spring Cloud Vault] project provides +support for storing externalized configuration in +https://www.vaultproject.io/[HashiCorp Vault]. + + + [[boot-features-external-config-yaml]] === Using YAML Instead of Properties http://yaml.org[YAML] is a superset of JSON and, as such, is a convenient format for From b1399db9940d9aa879f4e2c48ff96c2c04a8a387 Mon Sep 17 00:00:00 2001 From: Phillip Webb Date: Thu, 11 Oct 2018 16:17:24 -0700 Subject: [PATCH 5/5] Add a warning about `webDriver` scope to the docs Update the reference documentation with a warning about the `webDriver` scope that we create. Closes gh-13093 --- .../src/main/asciidoc/spring-boot-features.adoc | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/spring-boot-project/spring-boot-docs/src/main/asciidoc/spring-boot-features.adoc b/spring-boot-project/spring-boot-docs/src/main/asciidoc/spring-boot-features.adoc index 1896bda8f9..46fc4bebc5 100644 --- a/spring-boot-project/spring-boot-docs/src/main/asciidoc/spring-boot-features.adoc +++ b/spring-boot-project/spring-boot-docs/src/main/asciidoc/spring-boot-features.adoc @@ -6657,6 +6657,10 @@ that the driver exits after each test and that a new instance is injected. If yo not want this behavior, you can add `@Scope("singleton")` to your `WebDriver` `@Bean` definition. +WARNING: The `webDriver` scope created by Spring Boot will replace any user defined scope +of the same name. If you define your own `webDriver` scope you may find it stops working +when you use `@WebMvcTest`. + TIP: Sometimes writing Spring MVC tests is not enough; Spring Boot can help you run <>.