diff --git a/spring-cloud-sleuth-core/pom.xml b/spring-cloud-sleuth-core/pom.xml index 64736db75..6200dd847 100644 --- a/spring-cloud-sleuth-core/pom.xml +++ b/spring-cloud-sleuth-core/pom.xml @@ -33,6 +33,11 @@ spring-boot-starter-web true + + org.springframework.boot + spring-boot-configuration-processor + true + org.springframework.boot spring-boot-starter-actuator diff --git a/spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/instrument/TraceKeys.java b/spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/instrument/TraceKeys.java index edc580f3b..5f62c6951 100644 --- a/spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/instrument/TraceKeys.java +++ b/spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/instrument/TraceKeys.java @@ -16,80 +16,103 @@ package org.springframework.cloud.sleuth.instrument; +import java.util.Collection; +import java.util.LinkedHashSet; + +import org.springframework.boot.context.properties.ConfigurationProperties; + +import lombok.Data; + /** - * Well-known {@link org.springframework.cloud.sleuth.Span#tag(String, String) span tag} keys. + * Well-known {@link org.springframework.cloud.sleuth.Span#tag(String, String) span tag} + * keys. * *

Overhead of adding Trace Data

* - * Overhead is directly related to the size of trace data exported out of process. Accordingly, it - * is better to tag what's important for latency troubleshooting, ie a whitelist, vs collecting - * everything and filtering downstream. The keys listed here are very common in tracing tools, and - * are considerate to the issue of overhead. + * Overhead is directly related to the size of trace data exported out of process. + * Accordingly, it is better to tag what's important for latency troubleshooting, i.e. a + * whitelist vs. collecting everything and filtering downstream. The keys listed here are + * very common in tracing tools, and are considerate to the issue of overhead. * - *

When evaluating new keys, consider how much additional data it implies, and if that data is - * critical to classifying, filtering or displaying traces. More data often means larger systems, - * less retention, or a lower sample rate. + *

+ * When evaluating new keys, consider how much additional data it implies, and if that + * data is critical to classifying, filtering or displaying traces. More data often means + * larger systems, less retention, or a lower sample rate. * - *

For example, in zipkin, a thrift-encoded span with an "sr" annotation is 82 bytes plus the - * size of its name and associated service. The maximum size of an HTTP cookie is 4096 bytes, - * roughly 50x that. Even if compression helps, if you aren't analyzing based on cookies, storing - * them displaces resources that could be used for more traces. Meanwhile, you have another system - * storing private data! The takeaway isn't never store cookies, as there are valid cases for this. - * The takeaway is to be conscious about what's you are storing. + *

+ * For example, in zipkin, a thrift-encoded span with an "sr" annotation is 82 bytes plus + * the size of its name and associated service. The maximum size of an HTTP cookie is 4096 + * bytes, roughly 50x that. Even if compression helps, if you aren't analyzing based on + * cookies, storing them displaces resources that could be used for more traces. + * Meanwhile, you have another system storing private data! The takeaway isn't never store + * cookies, as there are valid cases for this. The takeaway is to be conscious about + * what's you are storing. */ -public final class TraceKeys { +@ConfigurationProperties("spring.sleuth.keys") +@Data +public class TraceKeys { - /** - * The domain portion of the URL or host header. Ex. "mybucket.s3.amazonaws.com" - * - *

Used to filter by host as opposed to ip address. - */ - public static final String HTTP_HOST = "http/host"; + private Http http = new Http(); - /** - * The HTTP method, or verb, such as "GET" or "POST". - * - *

Used to filter against an http route. - */ - public static final String HTTP_METHOD = "http/method"; + @Data + public static class Http { - /** - * The absolute http path, without any query parameters. Ex. "/objects/abcd-ff" - * - *

Used to filter against an http route, portably with zipkin v1. - * - *

In zipkin v1, only equals filters are supported. Dropping query parameters makes the number - * of distinct URIs less. For example, one can query for the same resource, regardless of signing - * parameters encoded in the query line. This does not reduce cardinality to a HTTP single route. - * For example, it is common to express a route as an http URI template like - * "/resource/{resource_id}". In systems where only equals queries are available, searching for - * {@code http.uri=/resource} won't match if the actual request was "/resource/abcd-ff". - * - *

Historical note: This was commonly expressed as "http.uri" in zipkin, eventhough it was most - * often just a path. - */ - public static final String HTTP_PATH = "http/path"; + /** + * The domain portion of the URL or host header. Example: + * "mybucket.s3.amazonaws.com". Used to filter by host as opposed to ip address. + */ + private String host = "http/host"; - /** - * The entire URL, including the scheme, host and query parameters if available. Ex. - * "https://mybucket.s3.amazonaws.com/objects/abcd-ff?X-Amz-Algorithm=AWS4-HMAC-SHA256&X-Amz-Algorithm=AWS4-HMAC-SHA256..." - * - *

Combined with {@link #HTTP_METHOD}, you can understand the fully-qualified request line. - * - *

This is optional as it may include private data or be of considerable length. - */ - public static final String HTTP_URL = "http/url"; + /** + * The HTTP method, or verb, such as "GET" or "POST". Used to filter against an + * http route. + */ + private String method = "http/method"; - /** - * The HTTP response code, when not in 2xx range. Ex. "503" - * - *

Used to filter for error status. - * - *

2xx range are not logged as success codes are less interesting for latency troubleshooting. - * Omitting saves at least 20 bytes per span. - */ - public static final String HTTP_STATUS_CODE = "http/status_code"; + /** + * The absolute http path, without any query parameters. Example: + * "/objects/abcd-ff". Used to filter against an http route, portably with zipkin + * v1. In zipkin v1, only equals filters are supported. Dropping query parameters + * makes the number of distinct URIs less. For example, one can query for the same + * resource, regardless of signing parameters encoded in the query line. This does + * not reduce cardinality to a HTTP single route. For example, it is common to + * express a route as an http URI template like "/resource/{resource_id}". In + * systems where only equals queries are available, searching for + * {@code http.uri=/resource} won't match if the actual request was + * "/resource/abcd-ff". Historical note: This was commonly expressed as "http.uri" + * in zipkin, eventhough it was most often just a path. + */ + private String path = "http/path"; + + /** + * The entire URL, including the scheme, host and query parameters if available. + * Ex. + * "https://mybucket.s3.amazonaws.com/objects/abcd-ff?X-Amz-Algorithm=AWS4-HMAC-SHA256&X-Amz-Algorithm=AWS4-HMAC-SHA256..." + * Combined with {@link #HTTP_METHOD}, you can understand the fully-qualified + * request line. This is optional as it may include private data or be of + * considerable length. + */ + private String url = "http/url"; + + /** + * The HTTP response code, when not in 2xx range. Ex. "503" Used to filter for + * error status. 2xx range are not logged as success codes are less interesting + * for latency troubleshooting. Omitting saves at least 20 bytes per span. + */ + private String statusCode = "http/status_code"; + + /** + * Prefix for header names if they are added as tags. + */ + private String prefix = "http/"; + + /** + * Additional headers that should be added as tags if they exist. If the header + * value is multi-valued, the tag value will be a comma-separated, single-quoted + * list. + */ + private Collection headers = new LinkedHashSet(); + + } - private TraceKeys() { // no instances - } } diff --git a/spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/instrument/web/TraceFilter.java b/spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/instrument/web/TraceFilter.java index 181e4b4fa..f33a6930c 100644 --- a/spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/instrument/web/TraceFilter.java +++ b/spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/instrument/web/TraceFilter.java @@ -15,8 +15,25 @@ */ package org.springframework.cloud.sleuth.instrument.web; -import org.springframework.cloud.sleuth.*; +import static org.springframework.util.StringUtils.hasText; + +import java.io.IOException; +import java.util.ArrayList; +import java.util.Collections; +import java.util.Enumeration; +import java.util.Random; +import java.util.regex.Pattern; + +import javax.servlet.FilterChain; +import javax.servlet.ServletException; +import javax.servlet.http.HttpServletRequest; +import javax.servlet.http.HttpServletResponse; + +import org.springframework.cloud.sleuth.MilliSpan; import org.springframework.cloud.sleuth.MilliSpan.MilliSpanBuilder; +import org.springframework.cloud.sleuth.Span; +import org.springframework.cloud.sleuth.Trace; +import org.springframework.cloud.sleuth.Tracer; import org.springframework.cloud.sleuth.event.ServerReceivedEvent; import org.springframework.cloud.sleuth.event.ServerSentEvent; import org.springframework.cloud.sleuth.instrument.TraceKeys; @@ -31,16 +48,6 @@ import org.springframework.util.StringUtils; import org.springframework.web.filter.OncePerRequestFilter; import org.springframework.web.util.UrlPathHelper; -import javax.servlet.FilterChain; -import javax.servlet.ServletException; -import javax.servlet.http.HttpServletRequest; -import javax.servlet.http.HttpServletResponse; -import java.io.IOException; -import java.util.Random; -import java.util.regex.Pattern; - -import static org.springframework.util.StringUtils.hasText; - /** * Filter that takes the value of the {@link Trace#SPAN_ID_NAME} and * {@link Trace#TRACE_ID_NAME} header from either request or response and uses them to @@ -71,6 +78,7 @@ public class TraceFilter extends OncePerRequestFilter "/api-docs.*|/autoconfig|/configprops|/dump|/info|/metrics.*|/mappings|/trace|/swagger.*|.*\\.png|.*\\.css|.*\\.js|.*\\.html|/favicon.ico|/hystrix.stream"); private final Tracer tracer; + private final TraceKeys traceKeys; private final Pattern skipPattern; private final Random random; @@ -78,14 +86,13 @@ public class TraceFilter extends OncePerRequestFilter private ApplicationEventPublisher publisher; - public TraceFilter(Tracer tracer) { - this.tracer = tracer; - this.skipPattern = DEFAULT_SKIP_PATTERN; - this.random = new Random(); + public TraceFilter(Tracer tracer, TraceKeys traceKeys) { + this(tracer, traceKeys, DEFAULT_SKIP_PATTERN, new Random()); } - public TraceFilter(Tracer tracer, Pattern skipPattern, Random random) { + public TraceFilter(Tracer tracer, TraceKeys traceKeys, Pattern skipPattern, Random random) { this.tracer = tracer; + this.traceKeys = traceKeys; this.skipPattern = skipPattern; this.random = random; } @@ -117,7 +124,7 @@ public class TraceFilter extends OncePerRequestFilter if (hasHeader(request, response, Trace.TRACE_ID_NAME)) { long traceId = Span.IdConverter.fromHex(getHeader(request, response, Trace.TRACE_ID_NAME)); long spanId = hasHeader(request, response, Trace.SPAN_ID_NAME) ? - Span.IdConverter.fromHex(getHeader(request, response, Trace.SPAN_ID_NAME)) : random.nextLong(); + Span.IdConverter.fromHex(getHeader(request, response, Trace.SPAN_ID_NAME)) : this.random.nextLong(); MilliSpanBuilder span = MilliSpan.builder().traceId(traceId).spanId(spanId); if (skip) { @@ -203,10 +210,22 @@ public class TraceFilter extends OncePerRequestFilter /** Override to add annotations not defined in {@link TraceKeys}. */ protected void addRequestTags(HttpServletRequest request) { String uri = this.urlPathHelper.getPathWithinApplication(request); - this.tracer.addTag(TraceKeys.HTTP_URL, getFullUrl(request)); - this.tracer.addTag(TraceKeys.HTTP_HOST, request.getServerName()); - this.tracer.addTag(TraceKeys.HTTP_PATH, uri); - this.tracer.addTag(TraceKeys.HTTP_METHOD, request.getMethod()); + this.tracer.addTag(this.traceKeys.getHttp().getUrl(), getFullUrl(request)); + this.tracer.addTag(this.traceKeys.getHttp().getHost(), + request.getServerName()); + this.tracer.addTag(this.traceKeys.getHttp().getPath(), uri); + this.tracer.addTag(this.traceKeys.getHttp().getMethod(), + request.getMethod()); + for (String name : this.traceKeys.getHttp().getHeaders()) { + Enumeration values = request.getHeaders(name); + if (values.hasMoreElements()) { + String key = this.traceKeys.getHttp().getPrefix() + name.toLowerCase(); + ArrayList list = Collections.list(values); + String value = list.size() == 1 ? list.get(0) + : StringUtils.collectionToDelimitedString(list, ",", "'", "'"); + this.tracer.addTag(key, value); + } + } } /** Override to add annotations not defined in {@link TraceKeys}. */ @@ -215,11 +234,11 @@ public class TraceFilter extends OncePerRequestFilter if (httpStatus == HttpServletResponse.SC_OK && e != null) { // Filter chain threw exception but the response status may not have been set // yet, so we have to guess. - this.tracer.addTag(TraceKeys.HTTP_STATUS_CODE, + this.tracer.addTag(this.traceKeys.getHttp().getStatusCode(), String.valueOf(HttpServletResponse.SC_INTERNAL_SERVER_ERROR)); } else if ((httpStatus < 200) || (httpStatus > 299)){ - this.tracer.addTag(TraceKeys.HTTP_STATUS_CODE, + this.tracer.addTag(this.traceKeys.getHttp().getStatusCode(), String.valueOf(response.getStatus())); } } 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 47d398c84..d98051f85 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 @@ -26,9 +26,11 @@ import org.springframework.boot.autoconfigure.condition.ConditionalOnMissingBean import org.springframework.boot.autoconfigure.condition.ConditionalOnProperty; import org.springframework.boot.autoconfigure.condition.ConditionalOnWebApplication; import org.springframework.boot.context.embedded.FilterRegistrationBean; +import org.springframework.boot.context.properties.EnableConfigurationProperties; import org.springframework.cloud.sleuth.TraceAccessor; import org.springframework.cloud.sleuth.Tracer; import org.springframework.cloud.sleuth.autoconfig.TraceAutoConfiguration; +import org.springframework.cloud.sleuth.instrument.TraceKeys; import org.springframework.context.ApplicationEventPublisher; import org.springframework.context.annotation.Bean; import org.springframework.context.annotation.Configuration; @@ -47,6 +49,7 @@ import org.springframework.util.StringUtils; @ConditionalOnWebApplication @ConditionalOnBean(Tracer.class) @AutoConfigureAfter(TraceAutoConfiguration.class) +@EnableConfigurationProperties(TraceKeys.class) public class TraceWebAutoConfiguration { /** @@ -61,6 +64,9 @@ public class TraceWebAutoConfiguration { @Autowired private TraceAccessor accessor; + @Autowired + private TraceKeys traceKeys; + @Bean public TraceWebAspect traceWebAspect() { return new TraceWebAspect(this.tracer, this.accessor); @@ -71,7 +77,7 @@ public class TraceWebAutoConfiguration { public TraceFilter traceFilter(ApplicationEventPublisher publisher, Random random) { Pattern pattern = StringUtils.hasText(this.skipPattern) ? Pattern.compile(this.skipPattern) : TraceFilter.DEFAULT_SKIP_PATTERN; - TraceFilter filter = new TraceFilter(this.tracer, pattern, random); + TraceFilter filter = new TraceFilter(this.tracer, this.traceKeys, pattern, random); filter.setApplicationEventPublisher(publisher); return filter; } diff --git a/spring-cloud-sleuth-core/src/test/java/org/springframework/cloud/sleuth/AdhocTestSuite.java b/spring-cloud-sleuth-core/src/test/java/org/springframework/cloud/sleuth/AdhocTestSuite.java new file mode 100644 index 000000000..720b9fbba --- /dev/null +++ b/spring-cloud-sleuth-core/src/test/java/org/springframework/cloud/sleuth/AdhocTestSuite.java @@ -0,0 +1,37 @@ +/* + * Copyright 2012-2014 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. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.springframework.cloud.sleuth; + +import org.junit.Ignore; +import org.junit.runner.RunWith; +import org.junit.runners.Suite; +import org.junit.runners.Suite.SuiteClasses; +import org.springframework.cloud.sleuth.instrument.TraceRunnableTests; +import org.springframework.cloud.sleuth.template.TraceTemplateTests; + +/** + * A test suite for probing weird ordering problems in the tests. + * + * @author Dave Syer + */ +@RunWith(Suite.class) +@SuiteClasses({ TraceTemplateTests.class, + TraceRunnableTests.class }) +@Ignore +public class AdhocTestSuite { + +} diff --git a/spring-cloud-sleuth-core/src/test/java/org/springframework/cloud/sleuth/instrument/web/TraceFilterIntegrationTests.java b/spring-cloud-sleuth-core/src/test/java/org/springframework/cloud/sleuth/instrument/web/TraceFilterIntegrationTests.java index ebf1e454b..ec72a1a5d 100644 --- a/spring-cloud-sleuth-core/src/test/java/org/springframework/cloud/sleuth/instrument/web/TraceFilterIntegrationTests.java +++ b/spring-cloud-sleuth-core/src/test/java/org/springframework/cloud/sleuth/instrument/web/TraceFilterIntegrationTests.java @@ -1,5 +1,9 @@ package org.springframework.cloud.sleuth.instrument.web; +import static org.assertj.core.api.BDDAssertions.then; + +import java.util.Random; + import org.junit.Test; import org.junit.runner.RunWith; import org.springframework.beans.factory.annotation.Autowired; @@ -8,6 +12,7 @@ import org.springframework.cloud.sleuth.Span; import org.springframework.cloud.sleuth.Trace; import org.springframework.cloud.sleuth.Tracer; import org.springframework.cloud.sleuth.instrument.DefaultTestAutoConfiguration; +import org.springframework.cloud.sleuth.instrument.TraceKeys; import org.springframework.cloud.sleuth.instrument.web.common.AbstractMvcIntegrationTest; import org.springframework.http.MediaType; import org.springframework.test.context.junit4.SpringJUnit4ClassRunner; @@ -15,10 +20,6 @@ import org.springframework.test.web.servlet.MvcResult; import org.springframework.test.web.servlet.request.MockMvcRequestBuilders; import org.springframework.test.web.servlet.setup.DefaultMockMvcBuilder; -import java.util.Random; - -import static org.assertj.core.api.BDDAssertions.then; - @RunWith(SpringJUnit4ClassRunner.class) @SpringApplicationConfiguration(TraceFilterIntegrationTests.class) @DefaultTestAutoConfiguration @@ -26,6 +27,8 @@ public class TraceFilterIntegrationTests extends AbstractMvcIntegrationTest { @Autowired Tracer tracer; + @Autowired + TraceKeys traceKeys; @Test public void should_create_and_return_trace_in_HTTP_header() throws Exception { @@ -46,7 +49,7 @@ public class TraceFilterIntegrationTests extends AbstractMvcIntegrationTest { @Override protected void configureMockMvcBuilder(DefaultMockMvcBuilder mockMvcBuilder) { - mockMvcBuilder.addFilters(new TraceFilter(this.tracer)); + mockMvcBuilder.addFilters(new TraceFilter(this.tracer, this.traceKeys)); } private MvcResult whenSentPingWithoutTracingData() throws Exception { diff --git a/spring-cloud-sleuth-core/src/test/java/org/springframework/cloud/sleuth/instrument/web/TraceFilterMockChainIntegrationTests.java b/spring-cloud-sleuth-core/src/test/java/org/springframework/cloud/sleuth/instrument/web/TraceFilterMockChainIntegrationTests.java index aa7b34960..e87d44998 100644 --- a/spring-cloud-sleuth-core/src/test/java/org/springframework/cloud/sleuth/instrument/web/TraceFilterMockChainIntegrationTests.java +++ b/spring-cloud-sleuth-core/src/test/java/org/springframework/cloud/sleuth/instrument/web/TraceFilterMockChainIntegrationTests.java @@ -16,15 +16,18 @@ package org.springframework.cloud.sleuth.instrument.web; -import lombok.SneakyThrows; +import static org.junit.Assert.assertNull; +import static org.springframework.test.web.servlet.request.MockMvcRequestBuilders.get; + +import java.util.Random; + import org.junit.Before; import org.junit.Test; import org.springframework.cloud.sleuth.Trace; import org.springframework.cloud.sleuth.Tracer; -import org.springframework.cloud.sleuth.Tracer; +import org.springframework.cloud.sleuth.instrument.TraceKeys; import org.springframework.cloud.sleuth.sampler.AlwaysSampler; import org.springframework.cloud.sleuth.trace.DefaultTracer; -import org.springframework.cloud.sleuth.trace.DefaultTracer; import org.springframework.cloud.sleuth.trace.TraceContextHolder; import org.springframework.context.support.StaticApplicationContext; import org.springframework.http.MediaType; @@ -34,10 +37,7 @@ import org.springframework.mock.web.MockHttpServletResponse; import org.springframework.mock.web.MockServletContext; import org.springframework.test.web.servlet.request.MockHttpServletRequestBuilder; -import java.util.Random; - -import static org.junit.Assert.assertNull; -import static org.springframework.test.web.servlet.request.MockMvcRequestBuilders.get; +import lombok.SneakyThrows; /** * @author Spencer Gibb @@ -49,6 +49,7 @@ public class TraceFilterMockChainIntegrationTests { private Tracer tracer = new DefaultTracer(new AlwaysSampler(), new Random(), this.context); + private TraceKeys traceKeys = new TraceKeys(); private MockHttpServletRequest request; private MockHttpServletResponse response; @@ -72,7 +73,7 @@ public class TraceFilterMockChainIntegrationTests { @Test public void startsNewTrace() throws Exception { - TraceFilter filter = new TraceFilter(this.tracer); + TraceFilter filter = new TraceFilter(this.tracer, this.traceKeys); filter.doFilter(this.request, this.response, this.filterChain); assertNull(TraceContextHolder.getCurrentTrace()); } @@ -82,7 +83,7 @@ public class TraceFilterMockChainIntegrationTests { Random generator = new Random(); this.request = builder().header(Trace.SPAN_ID_NAME, generator.nextLong()) .header(Trace.TRACE_ID_NAME, generator.nextLong()).buildRequest(new MockServletContext()); - TraceFilter filter = new TraceFilter(this.tracer); + TraceFilter filter = new TraceFilter(this.tracer, this.traceKeys); filter.doFilter(this.request, this.response, this.filterChain); assertNull(TraceContextHolder.getCurrentSpan()); } diff --git a/spring-cloud-sleuth-core/src/test/java/org/springframework/cloud/sleuth/instrument/web/TraceFilterTests.java b/spring-cloud-sleuth-core/src/test/java/org/springframework/cloud/sleuth/instrument/web/TraceFilterTests.java index d525d566e..f4fe03c83 100644 --- a/spring-cloud-sleuth-core/src/test/java/org/springframework/cloud/sleuth/instrument/web/TraceFilterTests.java +++ b/spring-cloud-sleuth-core/src/test/java/org/springframework/cloud/sleuth/instrument/web/TraceFilterTests.java @@ -16,7 +16,16 @@ package org.springframework.cloud.sleuth.instrument.web; -import lombok.SneakyThrows; +import static org.assertj.core.api.Assertions.assertThat; +import static org.assertj.core.api.Assertions.entry; +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertFalse; +import static org.junit.Assert.assertNull; +import static org.mockito.MockitoAnnotations.initMocks; +import static org.springframework.test.web.servlet.request.MockMvcRequestBuilders.get; + +import java.util.Random; + import org.junit.Before; import org.junit.Test; import org.mockito.Mock; @@ -24,6 +33,7 @@ import org.springframework.cloud.sleuth.Sampler; import org.springframework.cloud.sleuth.Span; import org.springframework.cloud.sleuth.Trace; import org.springframework.cloud.sleuth.Tracer; +import org.springframework.cloud.sleuth.instrument.TraceKeys; import org.springframework.cloud.sleuth.sampler.AlwaysSampler; import org.springframework.cloud.sleuth.sampler.IsTracingSampler; import org.springframework.cloud.sleuth.trace.DefaultTracer; @@ -37,15 +47,7 @@ import org.springframework.mock.web.MockHttpServletResponse; import org.springframework.mock.web.MockServletContext; import org.springframework.test.web.servlet.request.MockHttpServletRequestBuilder; -import java.util.Random; - -import static org.assertj.core.api.Assertions.assertThat; -import static org.assertj.core.api.Assertions.entry; -import static org.junit.Assert.assertEquals; -import static org.junit.Assert.assertFalse; -import static org.junit.Assert.assertNull; -import static org.mockito.MockitoAnnotations.initMocks; -import static org.springframework.test.web.servlet.request.MockMvcRequestBuilders.get; +import lombok.SneakyThrows; /** * @author Spencer Gibb @@ -56,6 +58,7 @@ public class TraceFilterTests { private ApplicationEventPublisher publisher; private Tracer tracer; + private TraceKeys traceKeys = new TraceKeys(); private Span span; @@ -68,7 +71,8 @@ public class TraceFilterTests { @SneakyThrows public void init() { initMocks(this); - this.tracer = new DefaultTracer(new DelegateSampler(), new Random(), this.publisher) { + this.tracer = new DefaultTracer(new DelegateSampler(), new Random(), + this.publisher) { @Override protected Trace createTrace(Trace trace, Span span) { TraceFilterTests.this.span = span; @@ -89,7 +93,7 @@ public class TraceFilterTests { @Test public void notTraced() throws Exception { this.sampler = new IsTracingSampler(); - TraceFilter filter = new TraceFilter(this.tracer); + TraceFilter filter = new TraceFilter(this.tracer, this.traceKeys); this.request = get("/favicon.ico").accept(MediaType.ALL) .buildRequest(new MockServletContext()); @@ -102,7 +106,7 @@ public class TraceFilterTests { @Test public void startsNewTrace() throws Exception { - TraceFilter filter = new TraceFilter(this.tracer); + TraceFilter filter = new TraceFilter(this.tracer, this.traceKeys); filter.doFilter(this.request, this.response, this.filterChain); verifyHttpTags(); assertNull(TraceContextHolder.getCurrentTrace()); @@ -114,7 +118,7 @@ public class TraceFilterTests { Trace trace = this.tracer.startTrace("foo"); this.request.setAttribute(TraceFilter.TRACE_REQUEST_ATTR, trace); - TraceFilter filter = new TraceFilter(this.tracer); + TraceFilter filter = new TraceFilter(this.tracer, this.traceKeys); filter.doFilter(this.request, this.response, this.filterChain); verifyHttpTags(); @@ -125,10 +129,9 @@ public class TraceFilterTests { @Test public void continuesSpanFromHeaders() throws Exception { this.request = builder().header(Trace.SPAN_ID_NAME, 10L) - .header(Trace.TRACE_ID_NAME, 20L) - .buildRequest(new MockServletContext()); + .header(Trace.TRACE_ID_NAME, 20L).buildRequest(new MockServletContext()); - TraceFilter filter = new TraceFilter(this.tracer); + TraceFilter filter = new TraceFilter(this.tracer, this.traceKeys); filter.doFilter(this.request, this.response, this.filterChain); verifyHttpTags(); @@ -136,9 +139,40 @@ public class TraceFilterTests { assertNull(TraceContextHolder.getCurrentTrace()); } + @Test + public void addsAdditionalHeaders() throws Exception { + this.request = builder().header(Trace.SPAN_ID_NAME, 10L) + .header(Trace.TRACE_ID_NAME, 20L).buildRequest(new MockServletContext()); + + this.traceKeys.getHttp().getHeaders().add("x-foo"); + TraceFilter filter = new TraceFilter(this.tracer, this.traceKeys); + this.request.addHeader("X-Foo", "bar"); + filter.doFilter(this.request, this.response, this.filterChain); + + assertThat(this.span.tags()).contains(entry("http/x-foo", "bar")); + + assertNull(TraceContextHolder.getCurrentTrace()); + } + + @Test + public void additionalMultiValuedHeader() throws Exception { + this.request = builder().header(Trace.SPAN_ID_NAME, 10L) + .header(Trace.TRACE_ID_NAME, 20L).buildRequest(new MockServletContext()); + + this.traceKeys.getHttp().getHeaders().add("x-foo"); + TraceFilter filter = new TraceFilter(this.tracer, this.traceKeys); + this.request.addHeader("X-Foo", "bar"); + this.request.addHeader("X-Foo", "spam"); + filter.doFilter(this.request, this.response, this.filterChain); + + assertThat(this.span.tags()).contains(entry("http/x-foo", "'bar','spam'")); + + assertNull(TraceContextHolder.getCurrentTrace()); + } + @Test public void catchesException() throws Exception { - TraceFilter filter = new TraceFilter(this.tracer); + TraceFilter filter = new TraceFilter(this.tracer, this.traceKeys); this.filterChain = new MockFilterChain() { @Override public void doFilter(javax.servlet.ServletRequest request, @@ -163,23 +197,22 @@ public class TraceFilterTests { } /** - * Shows the expansion of {@link import org.springframework.cloud.sleuth.instrument.TraceKeys}. + * Shows the expansion of {@link import + * org.springframework.cloud.sleuth.instrument.TraceKeys}. */ public void verifyHttpTags(HttpStatus status) { - assertThat(this.span.tags()).contains( - entry("http/host", "localhost"), - entry("http/url", "http://localhost/?foo=bar"), - entry("http/path", "/"), - entry("http/method", "GET") - ); + assertThat(this.span.tags()).contains(entry("http/host", "localhost"), + entry("http/url", "http://localhost/?foo=bar"), entry("http/path", "/"), + entry("http/method", "GET")); - // Status is only interesting in non-success case. Omitting it saves at least 20bytes per span. + // Status is only interesting in non-success case. Omitting it saves at least + // 20bytes per span. if (status.is2xxSuccessful()) { - assertThat(this.span.tags()) - .doesNotContainKey("http/status_code"); - } else { - assertThat(this.span.tags()) - .containsEntry("http/status_code", status.toString()); + assertThat(this.span.tags()).doesNotContainKey("http/status_code"); + } + else { + assertThat(this.span.tags()).containsEntry("http/status_code", + status.toString()); } } diff --git a/spring-cloud-sleuth-core/src/test/java/org/springframework/cloud/sleuth/instrument/web/common/AbstractMvcWiremockIntegrationTest.java b/spring-cloud-sleuth-core/src/test/java/org/springframework/cloud/sleuth/instrument/web/common/AbstractMvcWiremockIntegrationTest.java index 6ea9f2b50..609528d93 100644 --- a/spring-cloud-sleuth-core/src/test/java/org/springframework/cloud/sleuth/instrument/web/common/AbstractMvcWiremockIntegrationTest.java +++ b/spring-cloud-sleuth-core/src/test/java/org/springframework/cloud/sleuth/instrument/web/common/AbstractMvcWiremockIntegrationTest.java @@ -3,7 +3,7 @@ package org.springframework.cloud.sleuth.instrument.web.common; import org.junit.Before; import org.springframework.beans.factory.annotation.Autowired; import org.springframework.cloud.sleuth.Tracer; -import org.springframework.cloud.sleuth.Tracer; +import org.springframework.cloud.sleuth.instrument.TraceKeys; import org.springframework.cloud.sleuth.instrument.web.TraceFilter; import org.springframework.test.context.ContextConfiguration; import org.springframework.test.web.servlet.setup.DefaultMockMvcBuilder; @@ -29,6 +29,7 @@ public abstract class AbstractMvcWiremockIntegrationTest extends AbstractMvcInte protected WireMock wireMock; @Autowired protected HttpMockServer httpMockServer; @Autowired protected Tracer tracer; + @Autowired protected TraceKeys traceKeys; @Override @Before @@ -52,6 +53,6 @@ public abstract class AbstractMvcWiremockIntegrationTest extends AbstractMvcInte @Override protected void configureMockMvcBuilder(DefaultMockMvcBuilder mockMvcBuilder) { - mockMvcBuilder.addFilters(new TraceFilter(this.tracer)); + mockMvcBuilder.addFilters(new TraceFilter(this.tracer, this.traceKeys)); } } diff --git a/spring-cloud-sleuth-core/src/test/java/org/springframework/cloud/sleuth/template/TraceTemplateTests.java b/spring-cloud-sleuth-core/src/test/java/org/springframework/cloud/sleuth/template/TraceTemplateTests.java index 96f0e399f..9e083b44a 100644 --- a/spring-cloud-sleuth-core/src/test/java/org/springframework/cloud/sleuth/template/TraceTemplateTests.java +++ b/spring-cloud-sleuth-core/src/test/java/org/springframework/cloud/sleuth/template/TraceTemplateTests.java @@ -1,5 +1,10 @@ package org.springframework.cloud.sleuth.template; +import static org.assertj.core.api.BDDAssertions.then; + +import java.util.Random; + +import org.junit.After; import org.junit.Test; import org.mockito.Mockito; import org.springframework.cloud.sleuth.Trace; @@ -9,19 +14,20 @@ import org.springframework.cloud.sleuth.trace.DefaultTracer; import org.springframework.cloud.sleuth.trace.TraceContextHolder; import org.springframework.context.ApplicationEventPublisher; -import java.util.Random; - -import static org.assertj.core.api.BDDAssertions.then; - public class TraceTemplateTests { Tracer tracer = new DefaultTracer(new AlwaysSampler(), new Random(), Mockito.mock(ApplicationEventPublisher.class)); + @After + public void close() { + TraceContextHolder.removeCurrentTrace(); + } + @Test public void should_pass_trace_to_the_callback_if_tracing_is_active() { - Trace initialTrace = tracer.startTrace("test"); - TraceTemplate traceTemplate = new TraceTemplate(tracer); + Trace initialTrace = this.tracer.startTrace("test"); + TraceTemplate traceTemplate = new TraceTemplate(this.tracer); Trace traceFromCallback = whenTraceCallbackReturningCurrentTraceIsExecuted(traceTemplate);