Restrict maximum URI tags to prevent memory issues

Add MeterFilter to restrict the maximum number of web client URI tags
created. Prior to this commit, if a user was manually building URIs for
use with a RestTemplate (rather than using uriVariables) the JVM could
run out of memory.

Fixes gh-11338

Co-authored-by: Phillip Webb <pwebb@pivotal.io>
This commit is contained in:
Jon Schneider
2018-01-25 20:21:02 -06:00
committed by Phillip Webb
parent 8f23ee4e58
commit 2612b43179
3 changed files with 97 additions and 0 deletions

View File

@@ -75,6 +75,13 @@ public class MetricsProperties {
*/
private String requestsMetricName = "http.client.requests";
/**
* Maximum number of unique URI tag values allowed. After the max number of
* tag values is reached, metrics with additional tag values are denied by
* filter.
*/
private int maxUriTags = 100;
public boolean isRecordRequestPercentiles() {
return this.recordRequestPercentiles;
}
@@ -91,6 +98,14 @@ public class MetricsProperties {
this.requestsMetricName = requestsMetricName;
}
public int getMaxUriTags() {
return this.maxUriTags;
}
public void setMaxUriTags(int maxUriTags) {
this.maxUriTags = maxUriTags;
}
}
public static class Server {

View File

@@ -16,7 +16,14 @@
package org.springframework.boot.actuate.autoconfigure.metrics.web.client;
import java.util.concurrent.atomic.AtomicBoolean;
import io.micrometer.core.instrument.Meter.Id;
import io.micrometer.core.instrument.MeterRegistry;
import io.micrometer.core.instrument.config.MeterFilter;
import io.micrometer.core.instrument.config.MeterFilterReply;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
import org.springframework.boot.actuate.autoconfigure.metrics.MetricsProperties;
import org.springframework.boot.actuate.metrics.web.client.DefaultRestTemplateExchangeTagsProvider;
@@ -26,6 +33,7 @@ import org.springframework.boot.autoconfigure.condition.ConditionalOnClass;
import org.springframework.boot.autoconfigure.condition.ConditionalOnMissingBean;
import org.springframework.context.annotation.Bean;
import org.springframework.context.annotation.Configuration;
import org.springframework.core.annotation.Order;
import org.springframework.web.client.RestTemplate;
/**
@@ -55,4 +63,47 @@ public class RestTemplateMetricsConfiguration {
properties.getWeb().getClient().isRecordRequestPercentiles());
}
@Bean
@Order(0)
public MeterFilter metricsWebClientUriTagFilter(MetricsProperties properties) {
String metricName = properties.getWeb().getClient().getRequestsMetricName();
MeterFilter denyFilter = new MaximumUriTagsReachedMeterFilter(metricName);
return MeterFilter.maximumAllowableTags(metricName, "uri",
properties.getWeb().getClient().getMaxUriTags(), denyFilter);
}
/**
* {@link MeterFilter} to deny further client requests and log a warning.
*/
private static class MaximumUriTagsReachedMeterFilter implements MeterFilter {
private final Logger logger = LoggerFactory
.getLogger(RestTemplateMetricsConfiguration.class);
private final String metricName;
private final AtomicBoolean alreadyWarned = new AtomicBoolean(false);
MaximumUriTagsReachedMeterFilter(String metricName) {
this.metricName = metricName;
}
@Override
public MeterFilterReply accept(Id id) {
if (this.alreadyWarned.compareAndSet(false, true)) {
logWarning();
}
return MeterFilterReply.DENY;
}
private void logWarning() {
if (this.logger.isWarnEnabled()) {
this.logger.warn(
"Reached the maximum number of URI tags for '" + this.metricName
+ "'. Are you using uriVariables on RestTemplate calls?");
}
}
}
}

View File

@@ -17,13 +17,16 @@
package org.springframework.boot.actuate.autoconfigure.metrics.web.client;
import io.micrometer.core.instrument.MeterRegistry;
import org.junit.Rule;
import org.junit.Test;
import org.springframework.boot.actuate.autoconfigure.metrics.MetricsProperties;
import org.springframework.boot.actuate.autoconfigure.metrics.MetricsRun;
import org.springframework.boot.actuate.metrics.web.client.MetricsRestTemplateCustomizer;
import org.springframework.boot.autoconfigure.AutoConfigurations;
import org.springframework.boot.autoconfigure.web.client.RestTemplateAutoConfiguration;
import org.springframework.boot.test.context.runner.ApplicationContextRunner;
import org.springframework.boot.test.rule.OutputCapture;
import org.springframework.boot.web.client.RestTemplateBuilder;
import org.springframework.http.HttpStatus;
import org.springframework.test.web.client.MockRestServiceServer;
@@ -37,6 +40,7 @@ import static org.springframework.test.web.client.response.MockRestResponseCreat
* Tests for {@link RestTemplateMetricsConfiguration}.
*
* @author Stephane Nicoll
* @author Jon Schneider
*/
public class RestTemplateMetricsConfigurationTests {
@@ -44,6 +48,9 @@ public class RestTemplateMetricsConfigurationTests {
.with(MetricsRun.simple()).withConfiguration(
AutoConfigurations.of(RestTemplateAutoConfiguration.class));
@Rule
public OutputCapture out = new OutputCapture();
@Test
public void restTemplateCreatedWithBuilderIsInstrumented() {
this.contextRunner.run((context) -> {
@@ -66,6 +73,30 @@ public class RestTemplateMetricsConfigurationTests {
});
}
@Test
public void afterMaxUrisReachedFurtherUrisAreDenied() {
this.contextRunner.run((context) -> {
MetricsProperties properties = context.getBean(MetricsProperties.class);
int maxUriTags = properties.getWeb().getClient().getMaxUriTags();
MeterRegistry registry = context.getBean(MeterRegistry.class);
RestTemplate restTemplate = context.getBean(RestTemplateBuilder.class)
.build();
MockRestServiceServer server = MockRestServiceServer
.createServer(restTemplate);
for (int i = 0; i < maxUriTags + 10; i++) {
server.expect(requestTo("/test/" + i))
.andRespond(withStatus(HttpStatus.OK));
}
for (int i = 0; i < maxUriTags + 10; i++) {
restTemplate.getForObject("/test/" + i, String.class);
}
assertThat(registry.get("http.client.requests").meters()).hasSize(maxUriTags);
assertThat(this.out.toString())
.contains("Reached the maximum number of URI tags "
+ "for 'http.client.requests'");
});
}
private void validateRestTemplate(RestTemplate restTemplate, MeterRegistry registry) {
MockRestServiceServer server = MockRestServiceServer.createServer(restTemplate);
server.expect(requestTo("/test")).andRespond(withStatus(HttpStatus.OK));