Fix messaging and ribbon instrumentation for hex headers

It still feels brittle because it's up to the suthor of the
instrumentation. This patches all the places where we were still
using Long instead of Long.toHexString(..).

Fixes gh-125
This commit is contained in:
Dave Syer
2016-01-21 09:48:31 +00:00
parent 7f58eab435
commit 0bbd7cb70f
8 changed files with 67 additions and 65 deletions

View File

@@ -1,5 +1,7 @@
package org.springframework.cloud.sleuth.instrument.integration;
import java.util.Random;
import org.springframework.cloud.sleuth.Span;
import org.springframework.cloud.sleuth.Tracer;
import org.springframework.cloud.sleuth.instrument.TraceKeys;
@@ -9,8 +11,6 @@ import org.springframework.messaging.Message;
import org.springframework.messaging.MessageChannel;
import org.springframework.messaging.support.ChannelInterceptorAdapter;
import java.util.Random;
/**
* Abstraction over classes related to channel intercepting
*
@@ -47,13 +47,13 @@ abstract class AbstractTraceChannelInterceptor extends ChannelInterceptorAdapter
return null; // cannot build a span without ids
}
long spanId = hasHeader(message, Span.SPAN_ID_NAME) ?
getHeader(message, Span.SPAN_ID_NAME, Long.class) : this.random.nextLong();
long traceId = getHeader(message, Span.TRACE_ID_NAME, Long.class);
Span.fromHex(getHeader(message, Span.SPAN_ID_NAME)) : this.random.nextLong();
long traceId = Span.fromHex(getHeader(message, Span.TRACE_ID_NAME));
Span.SpanBuilder span = Span.builder().traceId(traceId).spanId(spanId);
Long parentId = getHeader(message, Span.PARENT_ID_NAME, Long.class);
if (message.getHeaders().containsKey(Span.NOT_SAMPLED_NAME)) {
span.exportable(false);
}
String parentId = getHeader(message, Span.PARENT_ID_NAME);
String processId = getHeader(message, Span.PROCESS_ID_NAME);
String spanName = getHeader(message, Span.SPAN_NAME_NAME);
if (spanName != null) {
@@ -63,7 +63,7 @@ abstract class AbstractTraceChannelInterceptor extends ChannelInterceptorAdapter
span.processId(processId);
}
if (parentId != null) {
span.parent(parentId);
span.parent(Span.fromHex(parentId));
}
span.remote(true);
return span.build();

View File

@@ -16,16 +16,16 @@
package org.springframework.cloud.sleuth.instrument.integration;
import java.util.HashMap;
import java.util.List;
import java.util.Map;
import org.springframework.cloud.sleuth.Span;
import org.springframework.cloud.sleuth.instrument.TraceKeys;
import org.springframework.integration.support.MessageBuilder;
import org.springframework.messaging.Message;
import org.springframework.util.StringUtils;
import java.util.HashMap;
import java.util.List;
import java.util.Map;
/**
* Utility for manipulating message headers related to span data.
*
@@ -45,12 +45,15 @@ public class SpanMessageHeaders {
}
Map<String, String> headers = new HashMap<>();
addHeader(headers, Span.TRACE_ID_NAME, span.getTraceId());
addHeader(headers, Span.SPAN_ID_NAME, span.getSpanId());
addHeader(headers, Span.TRACE_ID_NAME, Span.toHex(span.getTraceId()));
addHeader(headers, Span.SPAN_ID_NAME, Span.toHex(span.getSpanId()));
if (span.isExportable()) {
addAnnotations(traceKeys, message, span);
addHeader(headers, Span.PARENT_ID_NAME, getFirst(span.getParents()));
Long parentId = getFirst(span.getParents());
if (parentId != null) {
addHeader(headers, Span.PARENT_ID_NAME, Span.toHex(parentId));
}
addHeader(headers, Span.SPAN_NAME_NAME, span.getName());
addHeader(headers, Span.PROCESS_ID_NAME, span.getProcessId());
}
@@ -69,7 +72,7 @@ public class SpanMessageHeaders {
if (value == null) {
value = "null";
}
span.tag(key, value.toString()); // TODO: better way to serialize?
span.tag(key, value.toString()); // TODO: better way to serialize?
}
}
addPayloadAnnotations(traceKeys, message.getPayload(), span);

View File

@@ -16,6 +16,10 @@
package org.springframework.cloud.sleuth.instrument.integration;
import java.util.List;
import java.util.Map;
import java.util.TreeMap;
import org.springframework.cloud.sleuth.Span;
import org.springframework.cloud.sleuth.trace.SpanContextHolder;
import org.springframework.messaging.Message;
@@ -23,10 +27,6 @@ import org.springframework.messaging.simp.SimpMessageHeaderAccessor;
import org.springframework.messaging.simp.SimpMessageType;
import org.springframework.util.StringUtils;
import java.util.List;
import java.util.Map;
import java.util.TreeMap;
/**
* Builder class to create STOMP message
*
@@ -61,12 +61,12 @@ public class StompMessageBuilder {
public StompMessageBuilder setHeadersFromSpan(final Span span) {
if (span != null) {
setHeaderIfAbsent(Span.SPAN_ID_NAME, span.getSpanId());
setHeaderIfAbsent(Span.TRACE_ID_NAME, span.getTraceId());
setHeaderIfAbsent(Span.SPAN_ID_NAME, Span.toHex(span.getSpanId()));
setHeaderIfAbsent(Span.TRACE_ID_NAME, Span.toHex(span.getTraceId()));
setHeaderIfAbsent(Span.SPAN_NAME_NAME, span.getName());
Long parentId = getParentId(SpanContextHolder.getCurrentSpan());
if (parentId != null)
setHeaderIfAbsent(Span.PARENT_ID_NAME, parentId);
setHeaderIfAbsent(Span.PARENT_ID_NAME, Span.toHex(parentId));
String processId = span.getProcessId();
if (StringUtils.hasText(processId))

View File

@@ -89,8 +89,8 @@ public class TraceFeignClientAutoConfiguration {
@ConditionalOnMissingBean(SleuthHystrixConcurrencyStrategy.class)
@ConditionalOnProperty(name = "feign.hystrix.enabled", matchIfMissing = true)
public Feign.Builder feignHystrixBuilder(Tracer tracer) {
return HystrixFeign.builder()
.invocationHandlerFactory(new SleuthHystrixInvocationHandler.Factory(tracer));
return HystrixFeign.builder().invocationHandlerFactory(
new SleuthHystrixInvocationHandler.Factory(tracer));
}
@Bean
@@ -127,8 +127,11 @@ public class TraceFeignClientAutoConfiguration {
}
template.header(Span.TRACE_ID_NAME, Span.toHex(span.getTraceId()));
setHeader(template, Span.SPAN_NAME_NAME, span.getName());
setHeader(template, Span.SPAN_ID_NAME, span.getSpanId());
setHeader(template, Span.PARENT_ID_NAME, getParentId(span));
setHeader(template, Span.SPAN_ID_NAME, Span.toHex(span.getSpanId()));
Long parentId = getParentId(span);
if (parentId != null) {
setHeader(template, Span.PARENT_ID_NAME, Span.toHex(parentId));
}
setHeader(template, Span.PROCESS_ID_NAME, span.getProcessId());
publish(new ClientSentEvent(this, span));
}
@@ -142,8 +145,7 @@ public class TraceFeignClientAutoConfiguration {
}
private Long getParentId(Span span) {
return !span.getParents().isEmpty()
? span.getParents().get(0) : null;
return !span.getParents().isEmpty() ? span.getParents().get(0) : null;
}
public void setHeader(RequestTemplate request, String name, String value) {
@@ -176,13 +178,15 @@ public class TraceFeignClientAutoConfiguration {
public void setHeader(Map<String, Collection<String>> headers, String name,
String value) {
if (StringUtils.hasText(value) && !headers.containsKey(name) && this.accessor.isTracing()) {
if (StringUtils.hasText(value) && !headers.containsKey(name)
&& this.accessor.isTracing()) {
headers.put(name, singletonList(value));
}
}
public void setHeader(Map<String, Collection<String>> headers, String name,
Long value) {
if (value != null ){
if (value != null) {
setHeader(headers, name, Span.toHex(value));
}
}

View File

@@ -16,9 +16,9 @@
package org.springframework.cloud.sleuth.instrument.zuul;
import com.netflix.client.http.HttpRequest;
import com.netflix.niws.client.http.RestClient;
import lombok.SneakyThrows;
import java.io.InputStream;
import java.net.URISyntaxException;
import org.springframework.cloud.netflix.ribbon.SpringClientFactory;
import org.springframework.cloud.netflix.zuul.filters.route.RestClientRibbonCommand;
import org.springframework.cloud.netflix.zuul.filters.route.RestClientRibbonCommandFactory;
@@ -31,8 +31,10 @@ import org.springframework.context.ApplicationEventPublisher;
import org.springframework.context.ApplicationEventPublisherAware;
import org.springframework.util.MultiValueMap;
import java.io.InputStream;
import java.net.URISyntaxException;
import com.netflix.client.http.HttpRequest;
import com.netflix.niws.client.http.RestClient;
import lombok.SneakyThrows;
/**
* @author Spencer Gibb
@@ -93,11 +95,11 @@ public class TraceRestClientRibbonCommandFactory extends RestClientRibbonCommand
setHeader(requestBuilder, Span.NOT_SAMPLED_NAME, "");
return;
}
setHeader(requestBuilder, Span.TRACE_ID_NAME, span.getTraceId());
setHeader(requestBuilder, Span.SPAN_ID_NAME, span.getSpanId());
setHeader(requestBuilder, Span.TRACE_ID_NAME, Span.toHex(span.getTraceId()));
setHeader(requestBuilder, Span.SPAN_ID_NAME, Span.toHex(span.getSpanId()));
setHeader(requestBuilder, Span.SPAN_NAME_NAME, span.getName());
setHeader(requestBuilder, Span.PARENT_ID_NAME,
getParentId(span));
Span.toHex(getParentId(span)));
setHeader(requestBuilder, Span.PROCESS_ID_NAME,
span.getProcessId());
publish(new ClientSentEvent(this, span));

View File

@@ -1,5 +1,7 @@
package org.springframework.cloud.sleuth.instrument.integration;
import static org.assertj.core.api.BDDAssertions.then;
import org.junit.After;
import org.junit.Before;
import org.junit.runner.RunWith;
@@ -14,8 +16,6 @@ import org.springframework.messaging.support.ExecutorSubscribableChannel;
import org.springframework.messaging.support.GenericMessage;
import org.springframework.test.context.junit4.SpringJUnit4ClassRunner;
import static org.assertj.core.api.BDDAssertions.then;
@RunWith(SpringJUnit4ClassRunner.class)
public abstract class AbstractTraceStompIntegrationTests {
@@ -52,13 +52,13 @@ public abstract class AbstractTraceStompIntegrationTests {
}
Long thenSpanIdFromHeadersIsNotEmpty() {
Long header = getValueFromHeaders(Span.SPAN_ID_NAME, Long.class);
Long header = Span.fromHex(getValueFromHeaders(Span.SPAN_ID_NAME, String.class));
then(header).as("Span id should not be empty").isNotNull();
return header;
}
Long thenTraceIdFromHeadersIsNotEmpty() {
Long header = getValueFromHeaders(Span.TRACE_ID_NAME, Long.class);
Long header = Span.fromHex(getValueFromHeaders(Span.TRACE_ID_NAME, String.class));
then(header).as("Trace id should not be empty").isNotNull();
return header;
}

View File

@@ -16,6 +16,15 @@
package org.springframework.cloud.sleuth.instrument.integration;
import static org.assertj.core.api.BDDAssertions.then;
import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertFalse;
import static org.junit.Assert.assertNotNull;
import static org.junit.Assert.assertNull;
import java.util.ArrayList;
import java.util.List;
import org.junit.After;
import org.junit.Before;
import org.junit.Test;
@@ -43,15 +52,6 @@ import org.springframework.messaging.MessagingException;
import org.springframework.test.annotation.DirtiesContext;
import org.springframework.test.context.junit4.SpringJUnit4ClassRunner;
import java.util.ArrayList;
import java.util.List;
import static org.assertj.core.api.BDDAssertions.then;
import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertFalse;
import static org.junit.Assert.assertNotNull;
import static org.junit.Assert.assertNull;
/**
* @author Dave Syer
*/
@@ -97,8 +97,8 @@ public class TraceChannelInterceptorTests implements MessageHandler {
@Test
public void nonExportableSpanCreation() {
this.channel.send(MessageBuilder.withPayload("hi").setHeader(Span.NOT_SAMPLED_NAME, "")
.build());
this.channel.send(MessageBuilder.withPayload("hi")
.setHeader(Span.NOT_SAMPLED_NAME, "").build());
assertNotNull("message was null", this.message);
String spanId = this.message.getHeaders().get(Span.SPAN_ID_NAME, String.class);
@@ -109,9 +109,9 @@ public class TraceChannelInterceptorTests implements MessageHandler {
@Test
public void parentSpanIncluded() {
this.channel.send(MessageBuilder.withPayload("hi").setHeader(Span.TRACE_ID_NAME, 10L)
.setHeader(Span.SPAN_ID_NAME, 20L)
.build());
this.channel.send(MessageBuilder.withPayload("hi")
.setHeader(Span.TRACE_ID_NAME, Span.toHex(10L))
.setHeader(Span.SPAN_ID_NAME, Span.toHex(20L)).build());
assertNotNull("message was null", this.message);
String spanId = this.message.getHeaders().get(Span.SPAN_ID_NAME, String.class);
@@ -139,8 +139,7 @@ public class TraceChannelInterceptorTests implements MessageHandler {
@Test
public void headerCreation() {
Span span = this.tracer.startTrace("testSendMessage",
new AlwaysSampler());
Span span = this.tracer.startTrace("testSendMessage", new AlwaysSampler());
this.channel.send(MessageBuilder.withPayload("hi").build());
this.tracer.close(span);
assertNotNull("message was null", this.message);
@@ -156,8 +155,7 @@ public class TraceChannelInterceptorTests implements MessageHandler {
// TODO: Refactor to parametrized test together with sending messages via channel
@Test
public void headerCreationViaMessagingTemplate() {
Span span = this.tracer.startTrace("testSendMessage",
new AlwaysSampler());
Span span = this.tracer.startTrace("testSendMessage", new AlwaysSampler());
this.messagingTemplate.send(MessageBuilder.withPayload("hi").build());
this.tracer.close(span);
assertNotNull("message was null", this.message);
@@ -177,7 +175,7 @@ public class TraceChannelInterceptorTests implements MessageHandler {
private List<SpanReleasedEvent> events = new ArrayList<>();
@EventListener
public void handle(SpanReleasedEvent event) {
public void handle(SpanReleasedEvent event) {
this.events.add(event);
}

View File

@@ -28,11 +28,6 @@
<artifactId>spring-boot-configuration-processor</artifactId>
<optional>true</optional>
</dependency>
<dependency>
<groupId>org.springframework.boot</groupId>
<artifactId>spring-boot-devtools</artifactId>
<optional>true</optional>
</dependency>
<dependency>
<groupId>org.springframework.boot</groupId>
<artifactId>spring-boot-starter-jdbc</artifactId>