SpringCloud/spring-cloud-sleuth
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

[#161] Fixed the way collections are passed between spans

Browse Source 
When a span is continued a new one is created from the previous one. We've been copying span collections thus the changes in the continued instance were not reflected in the parent.

Fixes #161
This commit is contained in:
Marcin Grzejszczak
2016-02-18 13:56:41 +01:00
parent 2ce9b085dd
commit 785dab0908
9 changed files with 48 additions and 12 deletions
Download Patch File Download Diff File Expand all files Collapse all files

16
spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/Span.java
View File

@@ -63,11 +63,17 @@ public class Span {
private final long spanId;
private boolean remote = false;
private boolean exportable = true;
private final Map<String, String> tags = new LinkedHashMap<>();
private final Map<String, String> tags;
private final String processId;
private final List<Log> logs = new ArrayList<>();
private final List<Log> logs;
private final Span savedSpan;
/**
* Creates a new span that still tracks tags and logs of the
* current span. This is crucial when continuing spans
* since the changes in those collections done in the continued span
* need to be reflected until the span gets closed.
*/
public Span(Span current, Span savedSpan) {
this.begin = current.getBegin();
this.end = current.getEnd();
@@ -78,8 +84,8 @@ public class Span {
this.remote = current.isRemote();
this.exportable = current.isExportable();
this.processId = current.getProcessId();
this.tags.putAll(current.tags());
this.logs.addAll(current.logs());
this.tags = current.tags;
this.logs = current.logs;
this.savedSpan = savedSpan;
}
@@ -102,6 +108,8 @@ public class Span {
this.exportable = exportable;
this.processId = processId;
this.savedSpan = savedSpan;
this.tags = new LinkedHashMap<>();
this.logs = new ArrayList<>();
}
public static SpanBuilder builder() {

2
spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/instrument/messaging/SpanMessageHeaders.java
View File

@@ -73,7 +73,7 @@ public class SpanMessageHeaders {
if (parentId != null) {
addHeader(headers, Span.PARENT_ID_NAME, Span.toHex(parentId));
}
addHeader(headers, Span.SPAN_NAME_NAME, span.getName().toString());
addHeader(headers, Span.SPAN_NAME_NAME, span.getName());
addHeader(headers, Span.PROCESS_ID_NAME, span.getProcessId());
}
else {

2
spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/instrument/web/client/TraceFeignClientAutoConfiguration.java
View File

@@ -127,7 +127,7 @@ public class TraceFeignClientAutoConfiguration {
return;
}
template.header(Span.TRACE_ID_NAME, Span.toHex(span.getTraceId()));
setHeader(template, Span.SPAN_NAME_NAME, span.getName().toString());
setHeader(template, Span.SPAN_NAME_NAME, span.getName());
setHeader(template, Span.SPAN_ID_NAME, Span.toHex(span.getSpanId()));
if (!span.isExportable()) {
setHeader(template, Span.NOT_SAMPLED_NAME, "true");

2
spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/instrument/zuul/TracePreZuulFilter.java
View File

@@ -69,7 +69,7 @@ public class TracePreZuulFilter extends ZuulFilter
try {
setHeader(response, Span.SPAN_ID_NAME, span.getSpanId());
setHeader(response, Span.TRACE_ID_NAME, span.getTraceId());
setHeader(response, Span.SPAN_NAME_NAME, span.getName().toString());
setHeader(response, Span.SPAN_NAME_NAME, span.getName());
if (!span.isExportable()) {
setHeader(response, Span.NOT_SAMPLED_NAME, "true");
}

2
spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/instrument/zuul/TraceRestClientRibbonCommandFactory.java
View File

@@ -103,7 +103,7 @@ public class TraceRestClientRibbonCommandFactory extends RestClientRibbonCommand
}
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().toString());
setHeader(requestBuilder, Span.SPAN_NAME_NAME, span.getName());
setHeader(requestBuilder, Span.PARENT_ID_NAME,
Span.toHex(getParentId(span)));
setHeader(requestBuilder, Span.PROCESS_ID_NAME,

4
spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/trace/DefaultTracer.java
View File

@@ -164,12 +164,12 @@ public class DefaultTracer implements Tracer {
} else {
return null;
}
Span newSpan = createSpan(span, SpanContextHolder.getCurrentSpan());
Span newSpan = createContinuedSpan(span, SpanContextHolder.getCurrentSpan());
SpanContextHolder.setCurrentSpan(newSpan);
return newSpan;
}
protected Span createSpan(Span span, Span saved) {
private Span createContinuedSpan(Span span, Span saved) {
if (saved == null && span.getSavedSpan() != null) {
saved = span.getSavedSpan();
}

12
spring-cloud-sleuth-core/src/test/java/org/springframework/cloud/sleuth/assertions/SpanAssert.java
View File

@@ -103,4 +103,16 @@ public class SpanAssert extends AbstractAssert<SpanAssert, Span> {
}
return this;
}
public SpanAssert hasLoggedAnEvent(String event) {
isNotNull();
if (!this.actual.logs().stream().map(org.springframework.cloud.sleuth.Log::getEvent)
.filter(s -> s.equals(event)).findAny().isPresent()) {
String message = String.format("Expected span to have the event with event value <%s>. "
+ "Found logs are <%s>", event, this.actual.logs());
log.error(message);
failWithMessage(message);
}
return this;
}
}

4
spring-cloud-sleuth-core/src/test/java/org/springframework/cloud/sleuth/instrument/web/TraceFilterTests.java
View File

@@ -71,8 +71,8 @@ public class TraceFilterTests {
this.tracer = new DefaultTracer(new DelegateSampler(), new Random(),
this.publisher, new DefaultSpanNamer()) {
@Override
protected Span createSpan(Span span, Span saved) {
TraceFilterTests.this.span = super.createSpan(span, saved);
public Span continueSpan(Span span) {
TraceFilterTests.this.span = super.continueSpan(span);
return TraceFilterTests.this.span;
}
};

16
spring-cloud-sleuth-core/src/test/java/org/springframework/cloud/sleuth/trace/DefaultTracerTests.java
View File

@@ -43,6 +43,7 @@ import static org.mockito.Mockito.atLeast;
import static org.mockito.Mockito.mock;
import static org.mockito.Mockito.times;
import static org.mockito.Mockito.verify;
import static org.springframework.cloud.sleuth.assertions.SleuthAssertions.then;
/**
* @author Spencer Gibb
@@ -168,6 +169,21 @@ public class DefaultTracerTests {
assertThat(tracer.getCurrentSpan(), is(equalTo(grandParent)));
}
@Test
public void shouldUpdateLogsInSpanWhenItGetsContinued() {
DefaultTracer tracer = new DefaultTracer(new AlwaysSampler(), new Random(),
this.publisher, this.spanNamer);
Span span = Span.builder().name(IMPORTANT_WORK_1).traceId(1L).spanId(1L)
.build();
Span continuedSpan = tracer.continueSpan(span);
tracer.addTag("key", "value");
continuedSpan.logEvent("event");
then(span).hasATag("key", "value").hasLoggedAnEvent("event");
then(continuedSpan).hasATag("key", "value").hasLoggedAnEvent("event");
}
private Span assertSpan(List<Span> spans, Long parentId, String name) {
List<Span> found = findSpans(spans, parentId);
assertThat("more than one span with parentId " + parentId, found.size(), is(1));