From 7e05f995fe400edfe521a8c3a862068f1d7ee3b8 Mon Sep 17 00:00:00 2001 From: Marcin Grzejszczak Date: Fri, 4 May 2018 21:30:58 -0400 Subject: [PATCH] Added servlet context path in skip patterns fixes gh-971 --- .../web/TraceWebAutoConfiguration.java | 46 ++++++++---- .../web/SkipPatternProviderConfigTest.java | 70 ++++++++++++++++--- 2 files changed, 93 insertions(+), 23 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 230c1c350..ea26bb9dc 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 @@ -25,6 +25,7 @@ import org.springframework.boot.autoconfigure.condition.ConditionalOnClass; import org.springframework.boot.autoconfigure.condition.ConditionalOnMissingBean; import org.springframework.boot.autoconfigure.condition.ConditionalOnMissingClass; import org.springframework.boot.autoconfigure.condition.ConditionalOnProperty; +import org.springframework.boot.autoconfigure.web.ServerProperties; import org.springframework.boot.context.properties.EnableConfigurationProperties; import org.springframework.cloud.sleuth.autoconfig.TraceAutoConfiguration; import org.springframework.context.annotation.Bean; @@ -56,13 +57,15 @@ public class TraceWebAutoConfiguration { @ConditionalOnBean(ManagementServerProperties.class) public SkipPatternProvider skipPatternForManagementServerProperties( final ManagementServerProperties managementServerProperties, - final SleuthWebProperties sleuthWebProperties) { + final SleuthWebProperties sleuthWebProperties, + ServerProperties serverProperties) { return new SkipPatternProvider() { @Override public Pattern skipPattern() { return getPatternForManagementServerProperties( managementServerProperties, - sleuthWebProperties); + sleuthWebProperties, + serverProperties); } }; } @@ -73,24 +76,31 @@ public class TraceWebAutoConfiguration { */ static Pattern getPatternForManagementServerProperties( ManagementServerProperties managementServerProperties, - SleuthWebProperties sleuthWebProperties) { + SleuthWebProperties sleuthWebProperties, ServerProperties serverProperties) { String skipPattern = sleuthWebProperties.getSkipPattern(); String additionalSkipPattern = sleuthWebProperties.getAdditionalSkipPattern(); String contextPath = managementServerProperties.getServlet().getContextPath(); - + String servletContextPath = serverProperties.getServlet().getContextPath(); if (StringUtils.hasText(skipPattern) && StringUtils.hasText(contextPath)) { - return Pattern.compile(combinedPattern(skipPattern + "|" + contextPath + ".*", additionalSkipPattern)); + String contextPathPattern = skipPattern + "|" + contextPath + ".*"; + contextPathPattern = StringUtils.hasText(servletContextPath) ? + servletContextPath + ".*|" + contextPathPattern : contextPathPattern; + return Pattern.compile(combinedPattern(contextPathPattern, additionalSkipPattern)); } else if (StringUtils.hasText(contextPath)) { - return Pattern.compile(combinedPattern(contextPath + ".*", additionalSkipPattern)); + String contextPathPattern = contextPath + ".*"; + contextPathPattern = StringUtils.hasText(servletContextPath) ? + servletContextPath + ".*|" + contextPathPattern : contextPathPattern; + return Pattern.compile(combinedPattern(contextPathPattern, additionalSkipPattern)); } - return defaultSkipPattern(skipPattern, additionalSkipPattern); + return defaultSkipPattern(serverProperties, skipPattern, additionalSkipPattern); } @Bean @ConditionalOnMissingBean(ManagementServerProperties.class) - public SkipPatternProvider defaultSkipPatternBeanIfManagementServerPropsArePresent(SleuthWebProperties sleuthWebProperties) { - return defaultSkipPatternProvider(sleuthWebProperties.getSkipPattern(), + public SkipPatternProvider defaultSkipPatternBeanIfManagementServerPropsArePresent(SleuthWebProperties sleuthWebProperties, + ServerProperties serverProperties) { + return defaultSkipPatternProvider(serverProperties, sleuthWebProperties.getSkipPattern(), sleuthWebProperties.getAdditionalSkipPattern()); } } @@ -99,18 +109,24 @@ public class TraceWebAutoConfiguration { @ConditionalOnMissingClass("org.springframework.boot.actuate.autoconfigure.ManagementServerProperties") @ConditionalOnMissingBean( SkipPatternProvider.class) - public SkipPatternProvider defaultSkipPatternBean(SleuthWebProperties sleuthWebProperties) { - return defaultSkipPatternProvider(sleuthWebProperties.getSkipPattern(), + public SkipPatternProvider defaultSkipPatternBean(SleuthWebProperties sleuthWebProperties, + ServerProperties serverProperties) { + return defaultSkipPatternProvider(serverProperties, sleuthWebProperties.getSkipPattern(), sleuthWebProperties.getAdditionalSkipPattern()); } - private static SkipPatternProvider defaultSkipPatternProvider( + private static SkipPatternProvider defaultSkipPatternProvider(ServerProperties serverProperties, final String skipPattern, final String additionalSkipPattern) { - return () -> defaultSkipPattern(skipPattern, additionalSkipPattern); + return () -> defaultSkipPattern(serverProperties, skipPattern, additionalSkipPattern); } - private static Pattern defaultSkipPattern(String skipPattern, String additionalSkipPattern) { - return Pattern.compile(combinedPattern(skipPattern, additionalSkipPattern)); + private static Pattern defaultSkipPattern(ServerProperties serverProperties, + String skipPattern, String additionalSkipPattern) { + String combinedPattern = combinedPattern(skipPattern, additionalSkipPattern); + if (StringUtils.hasText(serverProperties.getServlet().getContextPath())) { + combinedPattern = serverProperties.getServlet().getContextPath() + ".*" + "|" + combinedPattern; + } + return Pattern.compile(combinedPattern); } private static String combinedPattern(String skipPattern, String additionalSkipPattern) { 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 7df40583b..ddc1386f7 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 @@ -20,6 +20,7 @@ import java.util.regex.Pattern; import org.junit.Test; import org.springframework.boot.actuate.autoconfigure.web.server.ManagementServerProperties; +import org.springframework.boot.autoconfigure.web.ServerProperties; import static org.assertj.core.api.BDDAssertions.then; @@ -33,39 +34,79 @@ public class SkipPatternProviderConfigTest { SleuthWebProperties sleuthWebProperties = new SleuthWebProperties(); sleuthWebProperties.setSkipPattern("foo.*|bar.*"); Pattern pattern = TraceWebAutoConfiguration.SkipPatternProviderConfig.getPatternForManagementServerProperties( - managementServerPropertiesWithContextPath(), sleuthWebProperties); + managementServerPropertiesWithContextPath(), sleuthWebProperties, new ServerProperties()); then(pattern.pattern()).isEqualTo("foo.*|bar.*|/management/context.*"); } + @Test + public void should_combine_skip_pattern_management_context_and_servlet_context() throws Exception { + SleuthWebProperties sleuthWebProperties = new SleuthWebProperties(); + sleuthWebProperties.setSkipPattern("foo.*|bar.*"); + ServerProperties serverProperties = new ServerProperties(); + serverProperties.getServlet().setContextPath("baz"); + Pattern pattern = TraceWebAutoConfiguration.SkipPatternProviderConfig.getPatternForManagementServerProperties( + managementServerPropertiesWithContextPath(), sleuthWebProperties, serverProperties); + + then(pattern.pattern()).isEqualTo("baz.*|foo.*|bar.*|/management/context.*"); + } + @Test public void should_combine_skip_pattern_management_context_and_additional_pattern_when_all_are_not_empty() throws Exception { SleuthWebProperties sleuthWebProperties = new SleuthWebProperties(); sleuthWebProperties.setSkipPattern("foo.*|bar.*"); sleuthWebProperties.setAdditionalSkipPattern("baz.*|faz.*"); Pattern pattern = TraceWebAutoConfiguration.SkipPatternProviderConfig.getPatternForManagementServerProperties( - managementServerPropertiesWithContextPath(), sleuthWebProperties); + managementServerPropertiesWithContextPath(), sleuthWebProperties, new ServerProperties()); then(pattern.pattern()).isEqualTo("foo.*|bar.*|/management/context.*|baz.*|faz.*"); } + @Test + public void should_combine_skip_pattern_management_context_servlet_context_and_additional_pattern_when_all_are_not_empty() throws Exception { + SleuthWebProperties sleuthWebProperties = new SleuthWebProperties(); + sleuthWebProperties.setSkipPattern("foo.*|bar.*"); + sleuthWebProperties.setAdditionalSkipPattern("baz.*|faz.*"); + ServerProperties serverProperties = new ServerProperties(); + serverProperties.getServlet().setContextPath("bazzz"); + Pattern pattern = TraceWebAutoConfiguration.SkipPatternProviderConfig.getPatternForManagementServerProperties( + managementServerPropertiesWithContextPath(), sleuthWebProperties, serverProperties); + + then(pattern.pattern()).isEqualTo("bazzz.*|foo.*|bar.*|/management/context.*|baz.*|faz.*"); + } + @Test public void should_pick_skip_pattern_when_its_not_empty_and_management_context_is_empty() throws Exception { SleuthWebProperties sleuthWebProperties = new SleuthWebProperties(); sleuthWebProperties.setSkipPattern("foo.*|bar.*"); - Pattern pattern = TraceWebAutoConfiguration.SkipPatternProviderConfig.getPatternForManagementServerProperties(new ManagementServerProperties(), sleuthWebProperties); + Pattern pattern = TraceWebAutoConfiguration.SkipPatternProviderConfig.getPatternForManagementServerProperties(new ManagementServerProperties(), + sleuthWebProperties, new ServerProperties()); then(pattern.pattern()).isEqualTo("foo.*|bar.*"); } + @Test + public void should_pick_skip_pattern_with_servlet_context_path_when_its_not_empty_and_management_context_is_empty() throws Exception { + SleuthWebProperties sleuthWebProperties = new SleuthWebProperties(); + sleuthWebProperties.setSkipPattern("foo.*|bar.*"); + ServerProperties serverProperties = new ServerProperties(); + serverProperties.getServlet().setContextPath("bla"); + + Pattern pattern = TraceWebAutoConfiguration.SkipPatternProviderConfig.getPatternForManagementServerProperties(new ManagementServerProperties(), + sleuthWebProperties, serverProperties); + + then(pattern.pattern()).isEqualTo("bla.*|foo.*|bar.*"); + } + @Test public void should_pick_skip_pattern_and_additional_pattern_when_its_not_empty_and_management_context_is_empty() throws Exception { SleuthWebProperties sleuthWebProperties = new SleuthWebProperties(); sleuthWebProperties.setSkipPattern("foo.*|bar.*"); sleuthWebProperties.setAdditionalSkipPattern("baz.*|faz.*"); - Pattern pattern = TraceWebAutoConfiguration.SkipPatternProviderConfig.getPatternForManagementServerProperties(new ManagementServerProperties(), sleuthWebProperties); + Pattern pattern = TraceWebAutoConfiguration.SkipPatternProviderConfig.getPatternForManagementServerProperties(new ManagementServerProperties(), + sleuthWebProperties, new ServerProperties()); then(pattern.pattern()).isEqualTo("foo.*|bar.*|baz.*|faz.*"); } @@ -76,11 +117,24 @@ public class SkipPatternProviderConfigTest { sleuthWebProperties.setSkipPattern(""); Pattern pattern = TraceWebAutoConfiguration.SkipPatternProviderConfig.getPatternForManagementServerProperties( - managementServerPropertiesWithContextPath(), sleuthWebProperties); + managementServerPropertiesWithContextPath(), sleuthWebProperties, new ServerProperties()); then(pattern.pattern()).isEqualTo("/management/context.*"); } + @Test + public void should_pick_management_context_and_servlet_context_when_skip_patterns_is_empty_and_context_path_is_not() throws Exception { + SleuthWebProperties sleuthWebProperties = new SleuthWebProperties(); + sleuthWebProperties.setSkipPattern(""); + ServerProperties serverProperties = new ServerProperties(); + serverProperties.getServlet().setContextPath("baz"); + + Pattern pattern = TraceWebAutoConfiguration.SkipPatternProviderConfig.getPatternForManagementServerProperties( + managementServerPropertiesWithContextPath(), sleuthWebProperties, serverProperties); + + then(pattern.pattern()).isEqualTo("baz.*|/management/context.*"); + } + @Test public void should_pick_management_context_and_additional_pattern_when_skip_patterns_is_empty_and_context_path_is_not() throws Exception { SleuthWebProperties sleuthWebProperties = new SleuthWebProperties(); @@ -88,7 +142,7 @@ public class SkipPatternProviderConfigTest { sleuthWebProperties.setAdditionalSkipPattern("baz.*|faz.*"); Pattern pattern = TraceWebAutoConfiguration.SkipPatternProviderConfig.getPatternForManagementServerProperties( - managementServerPropertiesWithContextPath(), sleuthWebProperties); + managementServerPropertiesWithContextPath(), sleuthWebProperties, new ServerProperties()); then(pattern.pattern()).isEqualTo("/management/context.*|baz.*|faz.*"); } @@ -101,7 +155,7 @@ public class SkipPatternProviderConfigTest { managementServerProperties.getServlet().setContextPath(""); Pattern pattern = TraceWebAutoConfiguration.SkipPatternProviderConfig.getPatternForManagementServerProperties( - managementServerProperties, sleuthWebProperties); + managementServerProperties, sleuthWebProperties, new ServerProperties()); then(pattern.pattern()).isEqualTo(SleuthWebProperties.DEFAULT_SKIP_PATTERN); } @@ -115,7 +169,7 @@ public class SkipPatternProviderConfigTest { managementServerProperties.getServlet().setContextPath(""); Pattern pattern = TraceWebAutoConfiguration.SkipPatternProviderConfig.getPatternForManagementServerProperties( - managementServerProperties, sleuthWebProperties); + managementServerProperties, sleuthWebProperties, new ServerProperties()); then(pattern.pattern()).isEqualTo(SleuthWebProperties.DEFAULT_SKIP_PATTERN + "|baz.*|faz.*"); }