From 6948d92d134b7eab986c9faa51c5081e8b965c8a Mon Sep 17 00:00:00 2001 From: Marcin Grzejszczak Date: Thu, 25 Feb 2016 13:44:33 +0100 Subject: [PATCH] [#184] Stopped overriding tags via messaging fixes #184 --- .../AbstractTraceChannelInterceptor.java | 5 +++- .../messaging/SpanMessageHeaders.java | 20 ++++++++------ .../messaging/SpanMessageHeadersTests.java | 26 ++++++++++++++++++- .../cloud/sleuth/stream/SleuthSource.java | 2 +- 4 files changed, 42 insertions(+), 11 deletions(-) diff --git a/spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/instrument/messaging/AbstractTraceChannelInterceptor.java b/spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/instrument/messaging/AbstractTraceChannelInterceptor.java index 235e68177..7c90049e1 100644 --- a/spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/instrument/messaging/AbstractTraceChannelInterceptor.java +++ b/spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/instrument/messaging/AbstractTraceChannelInterceptor.java @@ -20,6 +20,9 @@ import org.springframework.util.ClassUtils; */ abstract class AbstractTraceChannelInterceptor extends ChannelInterceptorAdapter implements ExecutorChannelInterceptor { + /** + * A default prefix for span name for discerning messaging origin of the span + */ protected static final String MESSAGE_COMPONENT = "message"; private final Tracer tracer; @@ -57,7 +60,7 @@ abstract class AbstractTraceChannelInterceptor extends ChannelInterceptorAdapter : this.random.nextLong(); long traceId = Span.hexToId(getHeader(message, Span.TRACE_ID_NAME)); Span.SpanBuilder span = Span.builder().traceId(traceId).spanId(spanId); - if (message.getHeaders().containsKey(Span.NOT_SAMPLED_NAME)) { + if (hasHeader(message, Span.NOT_SAMPLED_NAME)) { span.exportable(false); } String parentId = getHeader(message, Span.PARENT_ID_NAME); diff --git a/spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/instrument/messaging/SpanMessageHeaders.java b/spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/instrument/messaging/SpanMessageHeaders.java index 1cb3d1bc2..847f1fc81 100644 --- a/spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/instrument/messaging/SpanMessageHeaders.java +++ b/spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/instrument/messaging/SpanMessageHeaders.java @@ -51,7 +51,8 @@ public class SpanMessageHeaders { /** * Adds default headers for a message. Check {@link Span} constants for - * more information what the default headers are. + * more information what the default headers are. If a span already has + * a tag set it will not get overridden. * * @param traceKeys - the global configuration for trace keys * @param message - message to which headers will be added @@ -60,7 +61,6 @@ public class SpanMessageHeaders { */ public static Message addSpanHeaders(TraceKeys traceKeys, Message message, Span span) { - MessageHeaderAccessor accessor = MessageHeaderAccessor .getMutableAccessor(message); if (span == null) { @@ -71,11 +71,9 @@ public class SpanMessageHeaders { } return message; } - Map headers = new HashMap<>(); addHeader(headers, Span.TRACE_ID_NAME, Span.idToHex(span.getTraceId())); addHeader(headers, Span.SPAN_ID_NAME, Span.idToHex(span.getSpanId())); - if (span.isExportable()) { addAnnotations(traceKeys, message, span); Long parentId = getFirst(span.getParents()); @@ -109,7 +107,7 @@ public class SpanMessageHeaders { if (value == null) { value = "null"; } - span.tag(key, value.toString()); // TODO: better way to serialize? + tagIfEntryMissing(span, key, value.toString()); // TODO: better way to serialize? } } addPayloadAnnotations(traceKeys, message.getPayload(), span); @@ -117,19 +115,25 @@ public class SpanMessageHeaders { static void addPayloadAnnotations(TraceKeys traceKeys, Object payload, Span span) { if (payload != null) { - span.tag(traceKeys.getMessage().getPayload().getType(), + tagIfEntryMissing(span, traceKeys.getMessage().getPayload().getType(), payload.getClass().getCanonicalName()); if (payload instanceof String) { - span.tag(traceKeys.getMessage().getPayload().getSize(), + tagIfEntryMissing(span, traceKeys.getMessage().getPayload().getSize(), String.valueOf(((String) payload).length())); } else if (payload instanceof byte[]) { - span.tag(traceKeys.getMessage().getPayload().getSize(), + tagIfEntryMissing(span, traceKeys.getMessage().getPayload().getSize(), String.valueOf(((byte[]) payload).length)); } } } + private static void tagIfEntryMissing(Span span, String key, String value) { + if (!span.tags().containsKey(key)) { + span.tag(key, value); + } + } + private static void addHeader(Map headers, String name, String value) { if (StringUtils.hasText(value)) { diff --git a/spring-cloud-sleuth-core/src/test/java/org/springframework/cloud/sleuth/instrument/messaging/SpanMessageHeadersTests.java b/spring-cloud-sleuth-core/src/test/java/org/springframework/cloud/sleuth/instrument/messaging/SpanMessageHeadersTests.java index 35aa4d2f9..f0e6a3de5 100644 --- a/spring-cloud-sleuth-core/src/test/java/org/springframework/cloud/sleuth/instrument/messaging/SpanMessageHeadersTests.java +++ b/spring-cloud-sleuth-core/src/test/java/org/springframework/cloud/sleuth/instrument/messaging/SpanMessageHeadersTests.java @@ -26,7 +26,7 @@ import org.springframework.messaging.support.MessageBuilder; import org.springframework.messaging.support.MessageHeaderAccessor; import org.springframework.messaging.support.NativeMessageHeaderAccessor; -import static org.assertj.core.api.Assertions.assertThat; +import static org.springframework.cloud.sleuth.assertions.SleuthAssertions.assertThat; /** * @author Dave Syer @@ -44,6 +44,30 @@ public class SpanMessageHeadersTests { assertThat(message.getHeaders()).containsKey(Span.SPAN_ID_NAME); } + @Test + public void shouldNotOverrideSpanTags() { + Span span = spanWithStringPayloadType(); + Message message = messageWithIntegerPayloadType(); + + message = SpanMessageHeaders.addSpanHeaders(this.traceKeys, message, span); + + assertThat(message.getHeaders()) + .containsKeys(Span.SPAN_ID_NAME, "message/payload-type"); + assertThat(span).hasATag("message/payload-type", "java.lang.String"); + } + + private Span spanWithStringPayloadType() { + Span span = Span.builder().name("http:foo").spanId(1L).traceId(2L).build(); + span.tag("message/payload-type", "java.lang.String"); + return span; + } + + private Message messageWithIntegerPayloadType() { + MessageHeaderAccessor accessor = SimpMessageHeaderAccessor.create(); + accessor.setHeader("message/payload-type", "java.lang.Integer"); + return MessageBuilder.createMessage("Hello World", accessor.getMessageHeaders()); + } + @Test public void nativeSpanHeadersAdded() { Span span = Span.builder().name("http:foo").spanId(1L).traceId(2L).build(); diff --git a/spring-cloud-sleuth-stream/src/main/java/org/springframework/cloud/sleuth/stream/SleuthSource.java b/spring-cloud-sleuth-stream/src/main/java/org/springframework/cloud/sleuth/stream/SleuthSource.java index d2f86350b..70f4d876a 100644 --- a/spring-cloud-sleuth-stream/src/main/java/org/springframework/cloud/sleuth/stream/SleuthSource.java +++ b/spring-cloud-sleuth-stream/src/main/java/org/springframework/cloud/sleuth/stream/SleuthSource.java @@ -21,7 +21,7 @@ import org.springframework.messaging.MessageChannel; /** * Defines a message channel for instrumented applications to use to send span data to a - * message broker. The channel accepts data in the form of {@link spans} to buffer + * message broker. The channel accepts data in the form of {@link Spans} to buffer * multiple actual span instances in a single message. A client app may occasionally drop * spans, and if it does it should attempt to account for and report the number dropped. *