From fdde285a69c9185dad92cd991c15fb82d78540a6 Mon Sep 17 00:00:00 2001 From: Dave Syer Date: Thu, 13 Aug 2015 08:55:07 +0100 Subject: [PATCH] Revert changes to headers Span name is properly optional in Zipkin, but it makes the logs in Sleuth a lot easier to read. Process ID is optional because it is a Sleuth feature, but it seems useful. This reverts commit b4d71727bfc52979f7784eb0eda6eedb6d09fa74. --- .../org/springframework/cloud/sleuth/Trace.java | 3 ++- .../integration/SpanMessageHeaders.java | 16 ++++++++-------- .../integration/TraceChannelInterceptor.java | 13 +++++++++++-- ...aceContextPropagationChannelInterceptor.java | 7 +++++++ .../sleuth/instrument/web/TraceFilter.java | 17 ++++++++++++----- .../client/TraceRestTemplateInterceptor.java | 4 ++++ .../TraceChannelInterceptorTests.java | 17 ++--------------- .../TraceRestTemplateInterceptorTests.java | 2 +- .../sleuth/sample/SampleZipkinApplication.java | 5 ----- 9 files changed, 47 insertions(+), 37 deletions(-) diff --git a/spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/Trace.java b/spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/Trace.java index 3a9482962..865303f13 100644 --- a/spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/Trace.java +++ b/spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/Trace.java @@ -53,8 +53,9 @@ public interface Trace { String SPAN_ID_NAME = "X-Span-Id"; String TRACE_ID_NAME = "X-Trace-Id"; + String SPAN_NAME_NAME = "X-Span-Name"; String PARENT_ID_NAME = "X-Parent-Id"; - String NOT_SAMPLED_NAME = "X-Not-Sampled"; + String PROCESS_ID_NAME = "X-Process-Id"; /** * Creates a trace scope wrapping a new span. diff --git a/spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/instrument/integration/SpanMessageHeaders.java b/spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/instrument/integration/SpanMessageHeaders.java index 3a07c331e..c34cf3613 100644 --- a/spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/instrument/integration/SpanMessageHeaders.java +++ b/spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/instrument/integration/SpanMessageHeaders.java @@ -16,9 +16,10 @@ package org.springframework.cloud.sleuth.instrument.integration; -import static org.springframework.cloud.sleuth.Trace.NOT_SAMPLED_NAME; import static org.springframework.cloud.sleuth.Trace.PARENT_ID_NAME; +import static org.springframework.cloud.sleuth.Trace.PROCESS_ID_NAME; import static org.springframework.cloud.sleuth.Trace.SPAN_ID_NAME; +import static org.springframework.cloud.sleuth.Trace.SPAN_NAME_NAME; import static org.springframework.cloud.sleuth.Trace.TRACE_ID_NAME; import java.util.HashMap; @@ -36,28 +37,27 @@ import org.springframework.messaging.Message; public class SpanMessageHeaders { public static Message addSpanHeaders(Message message, Span span) { - if (span == null) { - if (!message.getHeaders().containsKey(NOT_SAMPLED_NAME)) { - return MessageBuilder.fromMessage(message).setHeader(NOT_SAMPLED_NAME, "") - .build(); - } + if (span==null) { return message; } Map headers = new HashMap(); addHeader(headers, TRACE_ID_NAME, span.getTraceId()); addHeader(headers, SPAN_ID_NAME, span.getSpanId()); addHeader(headers, PARENT_ID_NAME, getFirst(span.getParents())); + addHeader(headers, SPAN_NAME_NAME, span.getName()); + addHeader(headers, PROCESS_ID_NAME, span.getProcessId()); return MessageBuilder.fromMessage(message).copyHeaders(headers).build(); } private static void addHeader(Map headers, String name, String value) { - if (value != null) { + if (value!=null) { headers.put(name, value); } } private static String getFirst(List parents) { - return parents == null || parents.isEmpty() ? null : parents.get(0); + return parents==null || parents.isEmpty() ? null : parents.get(0); } + } diff --git a/spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/instrument/integration/TraceChannelInterceptor.java b/spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/instrument/integration/TraceChannelInterceptor.java index 0d23ff943..735c631af 100644 --- a/spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/instrument/integration/TraceChannelInterceptor.java +++ b/spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/instrument/integration/TraceChannelInterceptor.java @@ -16,9 +16,10 @@ package org.springframework.cloud.sleuth.instrument.integration; -import static org.springframework.cloud.sleuth.Trace.NOT_SAMPLED_NAME; import static org.springframework.cloud.sleuth.Trace.PARENT_ID_NAME; +import static org.springframework.cloud.sleuth.Trace.PROCESS_ID_NAME; import static org.springframework.cloud.sleuth.Trace.SPAN_ID_NAME; +import static org.springframework.cloud.sleuth.Trace.SPAN_NAME_NAME; import static org.springframework.cloud.sleuth.Trace.TRACE_ID_NAME; import static org.springframework.util.StringUtils.hasText; @@ -46,7 +47,7 @@ public class TraceChannelInterceptor extends ChannelInterceptorAdapter { @Override public Message preSend(Message message, MessageChannel channel) { - if (TraceContextHolder.isTracing() || message.getHeaders().containsKey(NOT_SAMPLED_NAME)) { + if (TraceContextHolder.isTracing()) { return SpanMessageHeaders.addSpanHeaders(message, TraceContextHolder.getCurrentSpan()); } String spanId = getHeader(message, SPAN_ID_NAME); @@ -59,6 +60,14 @@ public class TraceChannelInterceptor extends ChannelInterceptorAdapter { MilliSpanBuilder span = MilliSpan.builder().traceId(traceId).spanId(spanId); String parentId = getHeader(message, PARENT_ID_NAME); + String processId = getHeader(message, PROCESS_ID_NAME); + String spanName = getHeader(message, SPAN_NAME_NAME); + if (spanName != null) { + span.name(spanName); + } + if (processId != null) { + span.processId(processId); + } if (parentId != null) { span.parent(parentId); } diff --git a/spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/instrument/integration/TraceContextPropagationChannelInterceptor.java b/spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/instrument/integration/TraceContextPropagationChannelInterceptor.java index a7d344883..3cc2ebf02 100644 --- a/spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/instrument/integration/TraceContextPropagationChannelInterceptor.java +++ b/spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/instrument/integration/TraceContextPropagationChannelInterceptor.java @@ -17,7 +17,9 @@ package org.springframework.cloud.sleuth.instrument.integration; import static org.springframework.cloud.sleuth.Trace.PARENT_ID_NAME; +import static org.springframework.cloud.sleuth.Trace.PROCESS_ID_NAME; import static org.springframework.cloud.sleuth.Trace.SPAN_ID_NAME; +import static org.springframework.cloud.sleuth.Trace.SPAN_NAME_NAME; import static org.springframework.cloud.sleuth.Trace.TRACE_ID_NAME; import static org.springframework.cloud.sleuth.TraceContextHolder.getCurrentSpan; import static org.springframework.cloud.sleuth.TraceContextHolder.setCurrentSpan; @@ -142,10 +144,15 @@ implements ExecutorChannelInterceptor { setHeader(headers, SPAN_ID_NAME, this.span.getSpanId()); setHeader(headers, TRACE_ID_NAME, this.span.getTraceId()); + setHeader(headers, SPAN_NAME_NAME, this.span.getName()); String parentId = getParentId(getCurrentSpan()); if (parentId != null) { setHeader(headers, PARENT_ID_NAME, parentId); } + String processId = this.span.getProcessId(); + if (processId != null) { + setHeader(headers, PROCESS_ID_NAME, processId); + } this.messageHeaders = new MessageHeaders(headers); } 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 a16eba717..d208a8094 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,9 +15,10 @@ */ package org.springframework.cloud.sleuth.instrument.web; -import static org.springframework.cloud.sleuth.Trace.NOT_SAMPLED_NAME; import static org.springframework.cloud.sleuth.Trace.PARENT_ID_NAME; +import static org.springframework.cloud.sleuth.Trace.PROCESS_ID_NAME; import static org.springframework.cloud.sleuth.Trace.SPAN_ID_NAME; +import static org.springframework.cloud.sleuth.Trace.SPAN_NAME_NAME; import static org.springframework.cloud.sleuth.Trace.TRACE_ID_NAME; import static org.springframework.util.StringUtils.hasText; @@ -81,15 +82,13 @@ public class TraceFilter extends OncePerRequestFilter { throws ServletException, IOException { String uri = this.urlPathHelper.getPathWithinApplication(request); - boolean skip = this.skipPattern.matcher(uri).matches() || getHeader(request, response, NOT_SAMPLED_NAME)!=null; + boolean skip = this.skipPattern.matcher(uri).matches(); TraceScope traceScope = (TraceScope) request.getAttribute(TRACE_REQUEST_ATTR); if (traceScope != null) { this.trace.continueSpan(traceScope.getSpan()); } - else if (skip) { - addToResponseIfNotPresent(response, NOT_SAMPLED_NAME, ""); - } else { + else if (!skip) { String spanId = getHeader(request, response, SPAN_ID_NAME); String traceId = getHeader(request, response, TRACE_ID_NAME); String name = "http" + uri; @@ -98,6 +97,14 @@ public class TraceFilter extends OncePerRequestFilter { MilliSpanBuilder span = MilliSpan.builder().traceId(traceId) .spanId(spanId); String parentId = getHeader(request, response, PARENT_ID_NAME); + String processId = getHeader(request, response, PROCESS_ID_NAME); + String parentName = getHeader(request, response, SPAN_NAME_NAME); + if (parentName != null) { + span.name(parentName); + } + if (processId != null) { + span.processId(processId); + } if (parentId != null) { span.parent(parentId); } diff --git a/spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/instrument/web/client/TraceRestTemplateInterceptor.java b/spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/instrument/web/client/TraceRestTemplateInterceptor.java index 7b149688b..3f1f2e301 100644 --- a/spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/instrument/web/client/TraceRestTemplateInterceptor.java +++ b/spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/instrument/web/client/TraceRestTemplateInterceptor.java @@ -16,7 +16,9 @@ package org.springframework.cloud.sleuth.instrument.web.client; import static org.springframework.cloud.sleuth.Trace.PARENT_ID_NAME; +import static org.springframework.cloud.sleuth.Trace.PROCESS_ID_NAME; import static org.springframework.cloud.sleuth.Trace.SPAN_ID_NAME; +import static org.springframework.cloud.sleuth.Trace.SPAN_NAME_NAME; import static org.springframework.cloud.sleuth.Trace.TRACE_ID_NAME; import static org.springframework.cloud.sleuth.TraceContextHolder.getCurrentSpan; import static org.springframework.cloud.sleuth.TraceContextHolder.isTracing; @@ -63,7 +65,9 @@ ApplicationEventPublisherAware { } setHeader(request, SPAN_ID_NAME, getCurrentSpan().getSpanId()); setHeader(request, TRACE_ID_NAME, getCurrentSpan().getTraceId()); + setHeader(request, SPAN_NAME_NAME, getCurrentSpan().getName()); setHeader(request, PARENT_ID_NAME, getParentId(getCurrentSpan())); + setHeader(request, PROCESS_ID_NAME, getCurrentSpan().getProcessId()); publish(new ClientSentEvent(this, getCurrentSpan())); return new TraceHttpResponse(this, execution.execute(request, body)); } diff --git a/spring-cloud-sleuth-core/src/test/java/org/springframework/cloud/sleuth/instrument/integration/TraceChannelInterceptorTests.java b/spring-cloud-sleuth-core/src/test/java/org/springframework/cloud/sleuth/instrument/integration/TraceChannelInterceptorTests.java index aa1bab808..6cec6b3bc 100644 --- a/spring-cloud-sleuth-core/src/test/java/org/springframework/cloud/sleuth/instrument/integration/TraceChannelInterceptorTests.java +++ b/spring-cloud-sleuth-core/src/test/java/org/springframework/cloud/sleuth/instrument/integration/TraceChannelInterceptorTests.java @@ -17,8 +17,6 @@ package org.springframework.cloud.sleuth.instrument.integration; import static org.junit.Assert.assertNotNull; -import static org.junit.Assert.assertNull; -import static org.springframework.cloud.sleuth.Trace.NOT_SAMPLED_NAME; import static org.springframework.cloud.sleuth.Trace.SPAN_ID_NAME; import static org.springframework.cloud.sleuth.Trace.TRACE_ID_NAME; @@ -50,7 +48,7 @@ import org.springframework.test.context.junit4.SpringJUnit4ClassRunner; * @author Dave Syer */ @RunWith(SpringJUnit4ClassRunner.class) -@SpringApplicationConfiguration(classes = App.class) +@SpringApplicationConfiguration(classes=App.class) @IntegrationTest @DirtiesContext public class TraceChannelInterceptorTests implements MessageHandler { @@ -80,16 +78,6 @@ public class TraceChannelInterceptorTests implements MessageHandler { this.channel.unsubscribe(this); } - @Test - public void testNoSpanCreation() { - this.channel.send(MessageBuilder.withPayload("hi").setHeader(NOT_SAMPLED_NAME, "") - .build()); - assertNotNull("message was null", this.message); - - String spanId = this.message.getHeaders().get(SPAN_ID_NAME, String.class); - assertNull("spanId was not null", spanId); - } - @Test public void testSpanCreation() { this.channel.send(MessageBuilder.withPayload("hi").build()); @@ -104,8 +92,7 @@ public class TraceChannelInterceptorTests implements MessageHandler { @Test public void testHeaderCreation() { - TraceScope traceScope = this.trace.startSpan("testSendMessage", - new AlwaysSampler(), null); + TraceScope traceScope = this.trace.startSpan("testSendMessage", new AlwaysSampler(), null); this.channel.send(MessageBuilder.withPayload("hi").build()); traceScope.close(); assertNotNull("message was null", this.message); diff --git a/spring-cloud-sleuth-core/src/test/java/org/springframework/cloud/sleuth/instrument/web/client/TraceRestTemplateInterceptorTests.java b/spring-cloud-sleuth-core/src/test/java/org/springframework/cloud/sleuth/instrument/web/client/TraceRestTemplateInterceptorTests.java index 290594456..4edf41091 100644 --- a/spring-cloud-sleuth-core/src/test/java/org/springframework/cloud/sleuth/instrument/web/client/TraceRestTemplateInterceptorTests.java +++ b/spring-cloud-sleuth-core/src/test/java/org/springframework/cloud/sleuth/instrument/web/client/TraceRestTemplateInterceptorTests.java @@ -88,7 +88,7 @@ public class TraceRestTemplateInterceptorTests { public Map home(@RequestHeader HttpHeaders headers) { Map map = new HashMap(); addHeaders(map, headers, Trace.SPAN_ID_NAME, Trace.TRACE_ID_NAME, - Trace.PARENT_ID_NAME); + Trace.PARENT_ID_NAME, Trace.SPAN_NAME_NAME, Trace.PROCESS_ID_NAME); return map; } diff --git a/spring-cloud-sleuth-samples/spring-cloud-sleuth-sample-zipkin/src/main/java/org/springframework/cloud/sleuth/sample/SampleZipkinApplication.java b/spring-cloud-sleuth-samples/spring-cloud-sleuth-sample-zipkin/src/main/java/org/springframework/cloud/sleuth/sample/SampleZipkinApplication.java index 65797f500..24d38c72b 100644 --- a/spring-cloud-sleuth-samples/spring-cloud-sleuth-sample-zipkin/src/main/java/org/springframework/cloud/sleuth/sample/SampleZipkinApplication.java +++ b/spring-cloud-sleuth-samples/spring-cloud-sleuth-sample-zipkin/src/main/java/org/springframework/cloud/sleuth/sample/SampleZipkinApplication.java @@ -43,11 +43,6 @@ public class SampleZipkinApplication { return new AlwaysSampler(); } - @Bean - public SampleController sampleController() { - return new SampleController(); - } - public static void main(String[] args) { SpringApplication.run(SampleZipkinApplication.class, args); }