Normalize usage of IdGenerator (only in TraceManager)
Also ensures TraceFilter sends traceId and spanId in response if they are generated in the request. Fixes gh-65
This commit is contained in:
@@ -19,12 +19,9 @@ package org.springframework.cloud.sleuth.instrument.scheduling;
|
||||
import org.aspectj.lang.ProceedingJoinPoint;
|
||||
import org.aspectj.lang.annotation.Around;
|
||||
import org.aspectj.lang.annotation.Aspect;
|
||||
import org.springframework.cloud.sleuth.MilliSpan;
|
||||
import org.springframework.cloud.sleuth.Span;
|
||||
import org.springframework.cloud.sleuth.Trace;
|
||||
import org.springframework.cloud.sleuth.TraceManager;
|
||||
import org.springframework.scheduling.annotation.Scheduled;
|
||||
import org.springframework.util.IdGenerator;
|
||||
|
||||
/**
|
||||
* Aspect that creates a new Span for running threads executing methods annotated with
|
||||
@@ -42,20 +39,14 @@ import org.springframework.util.IdGenerator;
|
||||
public class TraceSchedulingAspect {
|
||||
|
||||
private final TraceManager trace;
|
||||
private final IdGenerator idGenerator;
|
||||
|
||||
public TraceSchedulingAspect(TraceManager trace, IdGenerator idGenerator) {
|
||||
public TraceSchedulingAspect(TraceManager trace) {
|
||||
this.trace = trace;
|
||||
this.idGenerator = idGenerator;
|
||||
}
|
||||
|
||||
@Around("execution (@org.springframework.scheduling.annotation.Scheduled * *.*(..))")
|
||||
public Object traceBackgroundThread(final ProceedingJoinPoint pjp) throws Throwable {
|
||||
final Span span = this.trace.isTracing() ? this.trace.getCurrentSpan()
|
||||
: MilliSpan.builder().begin(System.currentTimeMillis())
|
||||
.traceId(createId()).spanId(createId())
|
||||
.build();
|
||||
Trace scope = this.trace.startSpan(pjp.toShortString(), span);
|
||||
Trace scope = this.trace.startSpan(pjp.toShortString());
|
||||
try {
|
||||
return pjp.proceed();
|
||||
}
|
||||
@@ -64,7 +55,4 @@ public class TraceSchedulingAspect {
|
||||
}
|
||||
}
|
||||
|
||||
private String createId() {
|
||||
return this.idGenerator.generateId().toString();
|
||||
}
|
||||
}
|
||||
|
||||
@@ -30,7 +30,6 @@ import org.springframework.cloud.sleuth.autoconfig.TraceAutoConfiguration;
|
||||
import org.springframework.context.annotation.Bean;
|
||||
import org.springframework.context.annotation.Configuration;
|
||||
import org.springframework.context.annotation.EnableAspectJAutoProxy;
|
||||
import org.springframework.util.IdGenerator;
|
||||
|
||||
/**
|
||||
* Registers beans related to task scheduling.
|
||||
@@ -49,8 +48,8 @@ public class TraceSchedulingAutoConfiguration {
|
||||
|
||||
@ConditionalOnClass(ProceedingJoinPoint.class)
|
||||
@Bean
|
||||
public TraceSchedulingAspect traceSchedulingAspect(TraceManager trace, IdGenerator idGenerator) {
|
||||
return new TraceSchedulingAspect(trace, idGenerator);
|
||||
public TraceSchedulingAspect traceSchedulingAspect(TraceManager trace) {
|
||||
return new TraceSchedulingAspect(trace);
|
||||
}
|
||||
|
||||
}
|
||||
|
||||
@@ -55,13 +55,14 @@ import org.springframework.web.util.UrlPathHelper;
|
||||
* @author Dave Syer
|
||||
*/
|
||||
@Order(Ordered.HIGHEST_PRECEDENCE + 5)
|
||||
public class TraceFilter extends OncePerRequestFilter implements ApplicationEventPublisherAware {
|
||||
public class TraceFilter extends OncePerRequestFilter
|
||||
implements ApplicationEventPublisherAware {
|
||||
|
||||
protected static final String TRACE_REQUEST_ATTR = TraceFilter.class.getName()
|
||||
+ ".TRACE";
|
||||
|
||||
public static final Pattern DEFAULT_SKIP_PATTERN = Pattern
|
||||
.compile("/api-docs.*|/autoconfig|/configprops|/dump|/info|/metrics.*|/mappings|/trace|/swagger.*|.*\\.png|.*\\.css|.*\\.js|.*\\.html|/favicon.ico|/hystrix.stream");
|
||||
public static final Pattern DEFAULT_SKIP_PATTERN = Pattern.compile(
|
||||
"/api-docs.*|/autoconfig|/configprops|/dump|/info|/metrics.*|/mappings|/trace|/swagger.*|.*\\.png|.*\\.css|.*\\.js|.*\\.html|/favicon.ico|/hystrix.stream");
|
||||
|
||||
private final TraceManager traceManager;
|
||||
private final Pattern skipPattern;
|
||||
@@ -128,10 +129,10 @@ public class TraceFilter extends OncePerRequestFilter implements ApplicationEven
|
||||
publish(new ServerReceivedEvent(this, parent, trace.getSpan()));
|
||||
request.setAttribute(TRACE_REQUEST_ATTR, trace);
|
||||
// Send new span id back
|
||||
addToResponseIfNotPresent(response, Trace.TRACE_ID_NAME, trace.getSpan()
|
||||
.getTraceId());
|
||||
addToResponseIfNotPresent(response, Trace.SPAN_ID_NAME, trace.getSpan()
|
||||
.getSpanId());
|
||||
addToResponseIfNotPresent(response, Trace.TRACE_ID_NAME,
|
||||
trace.getSpan().getTraceId());
|
||||
addToResponseIfNotPresent(response, Trace.SPAN_ID_NAME,
|
||||
trace.getSpan().getSpanId());
|
||||
}
|
||||
else {
|
||||
trace = this.traceManager.startSpan(name);
|
||||
@@ -151,9 +152,11 @@ public class TraceFilter extends OncePerRequestFilter implements ApplicationEven
|
||||
return;
|
||||
}
|
||||
if (trace != null) {
|
||||
addResponseHeaders(response, trace.getSpan());
|
||||
addResponseAnnotations(response);
|
||||
if (trace.getSavedTrace()!=null) {
|
||||
publish(new ServerSentEvent(this, trace.getSavedTrace().getSpan(), trace.getSpan()));
|
||||
if (trace.getSavedTrace() != null) {
|
||||
publish(new ServerSentEvent(this, trace.getSavedTrace().getSpan(),
|
||||
trace.getSpan()));
|
||||
}
|
||||
// Double close to clean up the parent (remote span as well)
|
||||
this.traceManager.close(this.traceManager.close(trace));
|
||||
@@ -161,17 +164,24 @@ public class TraceFilter extends OncePerRequestFilter implements ApplicationEven
|
||||
}
|
||||
}
|
||||
|
||||
private void addResponseHeaders(HttpServletResponse response, Span span) {
|
||||
if (span != null) {
|
||||
response.addHeader(Trace.SPAN_ID_NAME, span.getSpanId());
|
||||
response.addHeader(Trace.TRACE_ID_NAME, span.getTraceId());
|
||||
}
|
||||
}
|
||||
|
||||
private void publish(ApplicationEvent event) {
|
||||
if (this.publisher!=null) {
|
||||
if (this.publisher != null) {
|
||||
this.publisher.publishEvent(event);
|
||||
}
|
||||
}
|
||||
|
||||
//TODO: move annotation keys to constants
|
||||
// TODO: move annotation keys to constants
|
||||
protected void addRequestAnnotations(HttpServletRequest request) {
|
||||
String uri = this.urlPathHelper.getPathWithinApplication(request);
|
||||
this.traceManager.addAnnotation("/http/request/uri", request.getRequestURL()
|
||||
.toString());
|
||||
this.traceManager.addAnnotation("/http/request/uri",
|
||||
request.getRequestURL().toString());
|
||||
this.traceManager.addAnnotation("/http/request/endpoint", uri);
|
||||
this.traceManager.addAnnotation("/http/request/method", request.getMethod());
|
||||
|
||||
|
||||
@@ -1,20 +0,0 @@
|
||||
package org.springframework.cloud.sleuth.instrument;
|
||||
|
||||
import org.springframework.boot.autoconfigure.EnableAutoConfiguration;
|
||||
import org.springframework.boot.autoconfigure.jmx.JmxAutoConfiguration;
|
||||
import org.springframework.cloud.client.loadbalancer.LoadBalancerAutoConfiguration;
|
||||
import org.springframework.context.annotation.Bean;
|
||||
import org.springframework.context.annotation.Configuration;
|
||||
import org.springframework.context.annotation.EnableAspectJAutoProxy;
|
||||
import org.springframework.context.support.PropertySourcesPlaceholderConfigurer;
|
||||
|
||||
@EnableAspectJAutoProxy(proxyTargetClass = true)
|
||||
@Configuration
|
||||
@EnableAutoConfiguration(exclude = {LoadBalancerAutoConfiguration.class, JmxAutoConfiguration.class})
|
||||
public class BaseConfigurationForITests {
|
||||
|
||||
@Bean static PropertySourcesPlaceholderConfigurer placeholderConfigurer() {
|
||||
return new PropertySourcesPlaceholderConfigurer();
|
||||
}
|
||||
|
||||
}
|
||||
@@ -6,15 +6,22 @@ import java.lang.annotation.RetentionPolicy;
|
||||
import java.lang.annotation.Target;
|
||||
|
||||
import org.springframework.boot.autoconfigure.EnableAutoConfiguration;
|
||||
import org.springframework.boot.autoconfigure.jmx.JmxAutoConfiguration;
|
||||
import org.springframework.cloud.client.loadbalancer.LoadBalancerAutoConfiguration;
|
||||
import org.springframework.cloud.netflix.archaius.ArchaiusAutoConfiguration;
|
||||
import org.springframework.cloud.sleuth.instrument.integration.TraceSpringIntegrationAutoConfiguration;
|
||||
import org.springframework.cloud.sleuth.sampler.AlwaysSampler;
|
||||
import org.springframework.context.annotation.Configuration;
|
||||
import org.springframework.context.annotation.EnableAspectJAutoProxy;
|
||||
import org.springframework.context.annotation.Import;
|
||||
|
||||
@Target(ElementType.TYPE)
|
||||
@Retention(RetentionPolicy.RUNTIME)
|
||||
@EnableAutoConfiguration(exclude = { TraceSpringIntegrationAutoConfiguration.class,
|
||||
@EnableAutoConfiguration(exclude = { LoadBalancerAutoConfiguration.class,
|
||||
JmxAutoConfiguration.class, TraceSpringIntegrationAutoConfiguration.class,
|
||||
ArchaiusAutoConfiguration.class, LoadBalancerAutoConfiguration.class })
|
||||
@EnableAspectJAutoProxy(proxyTargetClass = true)
|
||||
@Import(AlwaysSampler.class)
|
||||
@Configuration
|
||||
public @interface DefaultTestAutoConfiguration {
|
||||
}
|
||||
|
||||
@@ -1,18 +0,0 @@
|
||||
package org.springframework.cloud.sleuth.instrument.scheduling;
|
||||
|
||||
import org.springframework.cloud.sleuth.instrument.BaseConfigurationForITests;
|
||||
import org.springframework.cloud.sleuth.instrument.DefaultTestAutoConfiguration;
|
||||
import org.springframework.context.annotation.Bean;
|
||||
import org.springframework.context.annotation.Configuration;
|
||||
import org.springframework.context.annotation.Import;
|
||||
|
||||
@Configuration
|
||||
@Import(BaseConfigurationForITests.class)
|
||||
@DefaultTestAutoConfiguration
|
||||
class ScheduledTestConfiguration {
|
||||
|
||||
@Bean TestBeanWithScheduledMethod testBeanWithScheduledMethod() {
|
||||
return new TestBeanWithScheduledMethod();
|
||||
}
|
||||
|
||||
}
|
||||
@@ -1,19 +0,0 @@
|
||||
package org.springframework.cloud.sleuth.instrument.scheduling;
|
||||
|
||||
import org.springframework.cloud.sleuth.Span;
|
||||
import org.springframework.cloud.sleuth.trace.TraceContextHolder;
|
||||
import org.springframework.scheduling.annotation.Scheduled;
|
||||
|
||||
class TestBeanWithScheduledMethod {
|
||||
|
||||
Span span;
|
||||
|
||||
@Scheduled(fixedDelay = 1L)
|
||||
public void scheduledMethod() {
|
||||
this.span = TraceContextHolder.getCurrentSpan();
|
||||
}
|
||||
|
||||
public Span getSpan() {
|
||||
return this.span;
|
||||
}
|
||||
}
|
||||
@@ -8,6 +8,11 @@ import org.junit.runner.RunWith;
|
||||
import org.springframework.beans.factory.annotation.Autowired;
|
||||
import org.springframework.boot.test.SpringApplicationConfiguration;
|
||||
import org.springframework.cloud.sleuth.Span;
|
||||
import org.springframework.cloud.sleuth.instrument.DefaultTestAutoConfiguration;
|
||||
import org.springframework.cloud.sleuth.trace.TraceContextHolder;
|
||||
import org.springframework.context.annotation.Bean;
|
||||
import org.springframework.context.annotation.Configuration;
|
||||
import org.springframework.scheduling.annotation.Scheduled;
|
||||
import org.springframework.test.context.junit4.SpringJUnit4ClassRunner;
|
||||
|
||||
@RunWith(SpringJUnit4ClassRunner.class)
|
||||
@@ -23,7 +28,7 @@ public class TracingOnScheduledITest {
|
||||
|
||||
@Test
|
||||
public void should_have_a_new_span_set_each_time_a_scheduled_method_has_been_executed() {
|
||||
Span firstSpan = beanWithScheduledMethod.getSpan();
|
||||
Span firstSpan = this.beanWithScheduledMethod.getSpan();
|
||||
await().until(differentSpanHasBeenSetThan(firstSpan));
|
||||
}
|
||||
|
||||
@@ -31,7 +36,7 @@ public class TracingOnScheduledITest {
|
||||
return new Runnable() {
|
||||
@Override
|
||||
public void run() {
|
||||
Span storedSpan = beanWithScheduledMethod.getSpan();
|
||||
Span storedSpan = TracingOnScheduledITest.this.beanWithScheduledMethod.getSpan();
|
||||
then(storedSpan).isNotNull();
|
||||
then(storedSpan.getTraceId()).isNotNull();
|
||||
}
|
||||
@@ -42,9 +47,33 @@ public class TracingOnScheduledITest {
|
||||
return new Runnable() {
|
||||
@Override
|
||||
public void run() {
|
||||
then(beanWithScheduledMethod.getSpan()).isNotEqualTo(spanToCompare);
|
||||
then(TracingOnScheduledITest.this.beanWithScheduledMethod.getSpan()).isNotEqualTo(spanToCompare);
|
||||
}
|
||||
};
|
||||
}
|
||||
|
||||
}
|
||||
|
||||
@Configuration
|
||||
@DefaultTestAutoConfiguration
|
||||
class ScheduledTestConfiguration {
|
||||
|
||||
@Bean TestBeanWithScheduledMethod testBeanWithScheduledMethod() {
|
||||
return new TestBeanWithScheduledMethod();
|
||||
}
|
||||
|
||||
}
|
||||
|
||||
class TestBeanWithScheduledMethod {
|
||||
|
||||
Span span;
|
||||
|
||||
@Scheduled(fixedDelay = 1L)
|
||||
public void scheduledMethod() {
|
||||
this.span = TraceContextHolder.getCurrentSpan();
|
||||
}
|
||||
|
||||
public Span getSpan() {
|
||||
return this.span;
|
||||
}
|
||||
}
|
||||
|
||||
@@ -10,7 +10,6 @@ import org.junit.Test;
|
||||
import org.junit.runner.RunWith;
|
||||
import org.springframework.beans.factory.annotation.Autowired;
|
||||
import org.springframework.boot.test.SpringApplicationConfiguration;
|
||||
import org.springframework.cloud.sleuth.MilliSpan;
|
||||
import org.springframework.cloud.sleuth.Span;
|
||||
import org.springframework.cloud.sleuth.TraceManager;
|
||||
import org.springframework.cloud.sleuth.instrument.DefaultTestAutoConfiguration;
|
||||
@@ -21,7 +20,6 @@ import org.springframework.context.annotation.EnableAspectJAutoProxy;
|
||||
import org.springframework.scheduling.annotation.Async;
|
||||
import org.springframework.scheduling.annotation.EnableAsync;
|
||||
import org.springframework.test.context.junit4.SpringJUnit4ClassRunner;
|
||||
import org.springframework.util.IdGenerator;
|
||||
|
||||
import com.jayway.awaitility.Awaitility;
|
||||
|
||||
@@ -35,8 +33,6 @@ public class TraceAsyncITest {
|
||||
@Autowired
|
||||
AsyncDelegation asyncDelegation;
|
||||
@Autowired
|
||||
IdGenerator idGenerator;
|
||||
@Autowired
|
||||
TraceManager traceManager;
|
||||
|
||||
@Test
|
||||
@@ -49,8 +45,7 @@ public class TraceAsyncITest {
|
||||
}
|
||||
|
||||
private Span givenASpanInCurrentThread() {
|
||||
Span span = MilliSpan.builder().traceId(this.idGenerator.generateId().toString())
|
||||
.spanId(this.idGenerator.generateId().toString()).build();
|
||||
Span span = this.traceManager.startSpan("existing").getSpan();
|
||||
this.traceManager.continueSpan(span);
|
||||
return span;
|
||||
}
|
||||
|
||||
@@ -2,7 +2,8 @@ package org.springframework.cloud.sleuth.instrument.web;
|
||||
|
||||
import static org.assertj.core.api.BDDAssertions.then;
|
||||
|
||||
import org.junit.Ignore;
|
||||
import java.util.UUID;
|
||||
|
||||
import org.junit.Test;
|
||||
import org.junit.runner.RunWith;
|
||||
import org.springframework.beans.factory.annotation.Autowired;
|
||||
@@ -20,10 +21,10 @@ import org.springframework.test.web.servlet.setup.DefaultMockMvcBuilder;
|
||||
@RunWith(SpringJUnit4ClassRunner.class)
|
||||
@SpringApplicationConfiguration(TraceFilterITest.class)
|
||||
@DefaultTestAutoConfiguration
|
||||
@Ignore("Will fail cause no tracing data is set on the TraceFilter")
|
||||
public class TraceFilterITest extends MvcITest {
|
||||
|
||||
@Autowired TraceManager traceManager;
|
||||
@Autowired
|
||||
TraceManager traceManager;
|
||||
|
||||
@Test
|
||||
public void should_create_and_return_trace_in_HTTP_header() throws Exception {
|
||||
@@ -33,7 +34,8 @@ public class TraceFilterITest extends MvcITest {
|
||||
}
|
||||
|
||||
@Test
|
||||
public void when_correlationId_is_sent_should_not_create_a_new_one_but_return_the_existing_one_instead() throws Exception {
|
||||
public void when_correlationId_is_sent_should_not_create_a_new_one_but_return_the_existing_one_instead()
|
||||
throws Exception {
|
||||
String expectedTraceId = "passedCorId";
|
||||
|
||||
MvcResult mvcResult = whenSentPingWithTraceId(expectedTraceId);
|
||||
@@ -43,20 +45,27 @@ public class TraceFilterITest extends MvcITest {
|
||||
|
||||
@Override
|
||||
protected void configureMockMvcBuilder(DefaultMockMvcBuilder mockMvcBuilder) {
|
||||
mockMvcBuilder.addFilters(new TraceFilter(traceManager));
|
||||
mockMvcBuilder.addFilters(new TraceFilter(this.traceManager));
|
||||
}
|
||||
|
||||
private MvcResult whenSentPingWithoutTracingData() throws Exception {
|
||||
return mockMvc.perform(MockMvcRequestBuilders.get("/ping").accept(MediaType.TEXT_PLAIN)).andReturn();
|
||||
return this.mockMvc
|
||||
.perform(MockMvcRequestBuilders.get("/ping").accept(MediaType.TEXT_PLAIN))
|
||||
.andReturn();
|
||||
}
|
||||
|
||||
private MvcResult whenSentPingWithTraceId(String passedCorrelationId) throws Exception {
|
||||
private MvcResult whenSentPingWithTraceId(String passedCorrelationId)
|
||||
throws Exception {
|
||||
return sendPingWithTraceId(Trace.TRACE_ID_NAME, passedCorrelationId);
|
||||
}
|
||||
|
||||
private MvcResult sendPingWithTraceId(String headerName, String passedCorrelationId) throws Exception {
|
||||
return mockMvc.perform(MockMvcRequestBuilders.get("/ping").accept(MediaType.TEXT_PLAIN)
|
||||
.header(headerName, passedCorrelationId)).andReturn();
|
||||
private MvcResult sendPingWithTraceId(String headerName, String passedCorrelationId)
|
||||
throws Exception {
|
||||
return this.mockMvc
|
||||
.perform(MockMvcRequestBuilders.get("/ping").accept(MediaType.TEXT_PLAIN)
|
||||
.header(headerName, passedCorrelationId)
|
||||
.header(Trace.SPAN_ID_NAME, UUID.randomUUID().toString()))
|
||||
.andReturn();
|
||||
}
|
||||
|
||||
private String tracingHeaderFrom(MvcResult mvcResult) {
|
||||
|
||||
Reference in New Issue
Block a user