From 97574373a97b690b8b3530fded5874029c27d288 Mon Sep 17 00:00:00 2001 From: Marcin Grzejszczak Date: Thu, 16 Jul 2020 11:03:57 +0200 Subject: [PATCH] Resolves placeholders for skip pattern; fixes gh-1689 --- .../web/TraceWebAutoConfiguration.java | 35 ++++--- .../web/SkipPatternProviderConfigTest.java | 95 ++++++++++++++++++- 2 files changed, 115 insertions(+), 15 deletions(-) diff --git a/spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/instrument/web/TraceWebAutoConfiguration.java b/spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/instrument/web/TraceWebAutoConfiguration.java index 19301464a..5858ba405 100644 --- a/spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/instrument/web/TraceWebAutoConfiguration.java +++ b/spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/instrument/web/TraceWebAutoConfiguration.java @@ -42,6 +42,7 @@ import org.springframework.boot.context.properties.EnableConfigurationProperties import org.springframework.cloud.sleuth.autoconfig.TraceAutoConfiguration; import org.springframework.context.annotation.Bean; import org.springframework.context.annotation.Configuration; +import org.springframework.core.env.Environment; import org.springframework.lang.Nullable; import org.springframework.util.StringUtils; @@ -129,10 +130,12 @@ public class TraceWebAutoConfiguration { * @return optional skip pattern */ static Optional getPatternForManagementServerProperties( + Environment environment, ManagementServerProperties managementServerProperties) { String contextPath = managementServerProperties.getServlet().getContextPath(); if (StringUtils.hasText(contextPath)) { - return Optional.of(Pattern.compile(contextPath + ".*")); + return Optional.of(Pattern + .compile(environment.resolvePlaceholders(contextPath) + ".*")); } return Optional.empty(); } @@ -140,8 +143,9 @@ public class TraceWebAutoConfiguration { @Bean @ConditionalOnBean(ManagementServerProperties.class) public SingleSkipPattern skipPatternForManagementServerProperties( + Environment environment, final ManagementServerProperties managementServerProperties) { - return () -> getPatternForManagementServerProperties( + return () -> getPatternForManagementServerProperties(environment, managementServerProperties); } @@ -155,8 +159,8 @@ public class TraceWebAutoConfiguration { havingValue = "false", matchIfMissing = true) protected static class ActuatorSkipPatternProviderConfig { - static Optional getEndpointsPatterns(String contextPath, - WebEndpointProperties webEndpointProperties, + static Optional getEndpointsPatterns(Environment environment, + String contextPath, WebEndpointProperties webEndpointProperties, EndpointsSupplier endpointsSupplier) { Collection endpoints = endpointsSupplier.getEndpoints(); if (endpoints.isEmpty()) { @@ -165,7 +169,8 @@ public class TraceWebAutoConfiguration { String basePath = webEndpointProperties.getBasePath(); String pattern = patternFromEndpoints(contextPath, endpoints, basePath); if (StringUtils.hasText(pattern)) { - return Optional.of(Pattern.compile(pattern)); + return Optional + .of(Pattern.compile(environment.resolvePlaceholders(pattern))); } return Optional.empty(); } @@ -211,10 +216,10 @@ public class TraceWebAutoConfiguration { @Bean @ConditionalOnManagementPort(ManagementPortType.SAME) public SingleSkipPattern skipPatternForActuatorEndpointsSamePort( - final ServerProperties serverProperties, + Environment environment, final ServerProperties serverProperties, final WebEndpointProperties webEndpointProperties, final EndpointsSupplier endpointsSupplier) { - return () -> getEndpointsPatterns( + return () -> getEndpointsPatterns(environment, serverProperties.getServlet().getContextPath(), webEndpointProperties, endpointsSupplier); } @@ -224,10 +229,10 @@ public class TraceWebAutoConfiguration { @ConditionalOnProperty(name = "management.server.servlet.context-path", havingValue = "/", matchIfMissing = true) public SingleSkipPattern skipPatternForActuatorEndpointsDifferentPort( - final ServerProperties serverProperties, + Environment environment, final ServerProperties serverProperties, final WebEndpointProperties webEndpointProperties, final EndpointsSupplier endpointsSupplier) { - return () -> getEndpointsPatterns(null, webEndpointProperties, + return () -> getEndpointsPatterns(environment, null, webEndpointProperties, endpointsSupplier); } @@ -237,10 +242,16 @@ public class TraceWebAutoConfiguration { static class DefaultSkipPatternConfig { @Bean - SingleSkipPattern defaultSkipPatternBean( + SingleSkipPattern defaultSkipPatternBean(Environment environment, SleuthWebProperties sleuthWebProperties) { - Pattern pattern = combinePatterns(sleuthWebProperties.getSkipPattern(), - sleuthWebProperties.getAdditionalSkipPattern()); + String skipPattern = sleuthWebProperties.getSkipPattern(); + String left = StringUtils.hasText(skipPattern) + ? environment.resolvePlaceholders(skipPattern) : skipPattern; + String additionalSkipPattern = sleuthWebProperties.getAdditionalSkipPattern(); + String right = StringUtils.hasText(additionalSkipPattern) + ? environment.resolvePlaceholders(additionalSkipPattern) + : additionalSkipPattern; + Pattern pattern = combinePatterns(left, right); return () -> Optional.ofNullable(pattern); } diff --git a/spring-cloud-sleuth-core/src/test/java/org/springframework/cloud/sleuth/instrument/web/SkipPatternProviderConfigTest.java b/spring-cloud-sleuth-core/src/test/java/org/springframework/cloud/sleuth/instrument/web/SkipPatternProviderConfigTest.java index 302b49672..0e5ef3242 100644 --- a/spring-cloud-sleuth-core/src/test/java/org/springframework/cloud/sleuth/instrument/web/SkipPatternProviderConfigTest.java +++ b/spring-cloud-sleuth-core/src/test/java/org/springframework/cloud/sleuth/instrument/web/SkipPatternProviderConfigTest.java @@ -45,6 +45,8 @@ import org.springframework.cloud.sleuth.autoconfig.TraceAutoConfiguration; import org.springframework.context.ApplicationContext; import org.springframework.context.annotation.Bean; import org.springframework.context.annotation.Configuration; +import org.springframework.core.env.Environment; +import org.springframework.mock.env.MockEnvironment; import static org.assertj.core.api.BDDAssertions.then; @@ -88,11 +90,17 @@ public class SkipPatternProviderConfigTest { }); } + private Environment environment() { + MockEnvironment environment = new MockEnvironment(); + environment.setProperty("test", "value"); + return environment; + } + @Test public void should_return_empty_when_management_context_has_no_context_path() throws Exception { Optional pattern = new TraceWebAutoConfiguration.ManagementSkipPatternProviderConfig() - .skipPatternForManagementServerProperties( + .skipPatternForManagementServerProperties(environment(), new ManagementServerProperties()) .skipPattern(); @@ -113,11 +121,28 @@ public class SkipPatternProviderConfigTest { }); } + @Test + public void should_return_management_context_with_context_path_with_placeholders() + throws Exception { + contextRunner + .withConfiguration( + UserConfigurations.of(ManagementContextAutoConfiguration.class, + ServerPropertiesConfig.class)) + .withPropertyValues( + "management.server.servlet.context-path=${test:value}") + .run(context -> { + then(extractAllPatterns(context)).containsExactlyInAnyOrder( + "/actuator(/|/(health|health/.*|info|info/.*))?", "value.*", + SleuthWebProperties.DEFAULT_SKIP_PATTERN); + }); + } + @Test public void should_return_empty_when_no_endpoints() { Optional pattern = new TraceWebAutoConfiguration.ActuatorSkipPatternProviderConfig() - .skipPatternForActuatorEndpointsSamePort(new ServerProperties(), - new WebEndpointProperties(), Collections::emptyList) + .skipPatternForActuatorEndpointsSamePort(environment(), + new ServerProperties(), new WebEndpointProperties(), + Collections::emptyList) .skipPattern(); then(pattern).isEmpty(); @@ -145,6 +170,18 @@ public class SkipPatternProviderConfigTest { }); } + @Test + public void should_return_endpoints_with_context_path_with_placeholders() { + contextRunner + .withConfiguration(UserConfigurations.of(ServerPropertiesConfig.class)) + .withPropertyValues("server.servlet.context-path=${test:foo}") + .run(context -> { + then(extractAllPatterns(context)).containsExactlyInAnyOrder( + "foo/actuator(/|/(health|health/.*|info|info/.*))?", + SleuthWebProperties.DEFAULT_SKIP_PATTERN); + }); + } + @Test public void should_return_endpoints_without_context_path_and_base_path_set_to_root() { contextRunner @@ -157,6 +194,18 @@ public class SkipPatternProviderConfigTest { }); } + @Test + public void should_return_endpoints_without_context_path_and_base_path_set_to_root_with_placeholders() { + contextRunner + .withConfiguration(UserConfigurations.of(ServerPropertiesConfig.class)) + .withPropertyValues("management.endpoints.web.base-path=${test:/}") + .run(context -> { + then(extractAllPatterns(context)).containsExactlyInAnyOrder( + "/(health|health/.*|info|info/.*)", + SleuthWebProperties.DEFAULT_SKIP_PATTERN); + }); + } + @Test public void should_return_endpoints_with_context_path_and_base_path_set_to_root() { contextRunner @@ -170,6 +219,19 @@ public class SkipPatternProviderConfigTest { }); } + @Test + public void should_return_endpoints_with_context_path_and_base_path_set_to_root_with_placeholder() { + contextRunner + .withConfiguration(UserConfigurations.of(ServerPropertiesConfig.class)) + .withPropertyValues("management.endpoints.web.base-path=${test:/}", + "server.servlet.context-path=foo") + .run(context -> { + then(extractAllPatterns(context)).containsExactlyInAnyOrder( + "foo(/|/(health|health/.*|info|info/.*))?", + SleuthWebProperties.DEFAULT_SKIP_PATTERN); + }); + } + @Test public void should_return_endpoints_with_context_path_and_base_path_set_to_root_different_port() { contextRunner @@ -183,6 +245,20 @@ public class SkipPatternProviderConfigTest { }); } + @Test + public void should_return_endpoints_with_context_path_and_base_path_set_to_root_different_port_with_placeholder() { + contextRunner + .withConfiguration(UserConfigurations.of(ServerPropertiesConfig.class)) + .withPropertyValues("management.endpoints.web.base-path=/", + "management.server.port=${some-port:0}", + "server.servlet.context-path=${some-path:foo}") + .run(context -> { + then(extractAllPatterns(context)).containsExactlyInAnyOrder( + "/(health|health/.*|info|info/.*)", + SleuthWebProperties.DEFAULT_SKIP_PATTERN); + }); + } + @Test public void should_return_endpoints_with_actuator_context_path_only() { contextRunner @@ -196,6 +272,19 @@ public class SkipPatternProviderConfigTest { }); } + @Test + public void should_return_endpoints_with_actuator_context_path_only_with_placeholder() { + contextRunner + .withConfiguration(UserConfigurations.of(ServerPropertiesConfig.class)) + .withPropertyValues("management.endpoints.web.base-path=/${test:mgt}", + "server.servlet.context-path=${test2:foo}") + .run(context -> { + then(extractAllPatterns(context)).containsExactlyInAnyOrder( + "foo/mgt(/|/(health|health/.*|info|info/.*))?", + SleuthWebProperties.DEFAULT_SKIP_PATTERN); + }); + } + @Test public void should_return_endpoints_with_actuator_default_context_path_different_port() { contextRunner