Limiting the span name size to 50 chars

without this exceptions can occur when the span name is big
with this we're truncating the name to 50 chars

fixes #530
This commit is contained in:
Marcin Grzejszczak
2017-02-24 11:53:09 +01:00
parent 7caf522ecf
commit e0c2656354
11 changed files with 153 additions and 9 deletions

View File

@@ -107,6 +107,10 @@ was a span present in this thread then it would become the parent of that span.
IMPORTANT: Always clean after you create a span! Don't forget to close a span if you want to send it to Zipkin.
IMPORTANT: If your span contains a name greater than 50 chars, then that name will
be truncated to 50 chars. Your names have to be explicit and concrete. Big names lead to
latency issues and sometimes even thrown exceptions.
=== Continuing spans [[continuing-spans]]
Sometimes you don't want to create a new span but you want to continue one. Example of such a

View File

@@ -8,6 +8,7 @@ import org.springframework.cloud.sleuth.Span;
import org.springframework.cloud.sleuth.SpanTextMap;
import org.springframework.cloud.sleuth.TraceKeys;
import org.springframework.cloud.sleuth.Tracer;
import org.springframework.cloud.sleuth.util.SpanNameUtil;
import org.springframework.integration.channel.AbstractMessageChannel;
import org.springframework.integration.context.IntegrationObjectSupport;
import org.springframework.messaging.MessageChannel;
@@ -93,7 +94,7 @@ abstract class AbstractTraceChannelInterceptor extends ChannelInterceptorAdapter
}
String getMessageChannelName(MessageChannel channel) {
return MESSAGE_COMPONENT + ":" + getChannelName(channel);
return SpanNameUtil.shorten(MESSAGE_COMPONENT + ":" + getChannelName(channel));
}
}

View File

@@ -25,6 +25,7 @@ import org.springframework.cloud.sleuth.instrument.web.HttpSpanInjector;
import org.springframework.cloud.sleuth.Span;
import org.springframework.cloud.sleuth.Tracer;
import org.springframework.cloud.sleuth.instrument.web.HttpTraceKeysInjector;
import org.springframework.cloud.sleuth.util.SpanNameUtil;
import org.springframework.http.HttpRequest;
/**
* Abstraction over classes that interact with Http requests. Allows you
@@ -54,7 +55,7 @@ abstract class AbstractTraceHttpRequestInterceptor {
*/
protected void publishStartEvent(HttpRequest request) {
URI uri = request.getURI();
String spanName = uriScheme(uri) + ":" + uri.getPath();
String spanName = getName(uri);
Span newSpan = this.tracer.createSpan(spanName);
this.spanInjector.inject(newSpan, new HttpRequestTextMap(request));
addRequestTags(request);
@@ -64,6 +65,10 @@ abstract class AbstractTraceHttpRequestInterceptor {
}
}
private String getName(URI uri) {
return SpanNameUtil.shorten(uriScheme(uri) + ":" + uri.getPath());
}
private String uriScheme(URI uri) {
return uri.getScheme() == null ? "http" : uri.getScheme();
}

View File

@@ -30,6 +30,7 @@ import org.springframework.cloud.sleuth.Span;
import org.springframework.cloud.sleuth.Tracer;
import org.springframework.cloud.sleuth.instrument.web.HttpTraceKeysInjector;
import org.springframework.cloud.sleuth.util.ExceptionUtils;
import org.springframework.cloud.sleuth.util.SpanNameUtil;
import feign.Client;
import feign.Request;
@@ -101,7 +102,7 @@ class TraceFeignClient implements Client {
private String getSpanName(Request request) {
URI uri = URI.create(request.url());
return uriScheme(uri) + ":" + uri.getPath();
return SpanNameUtil.shorten(uriScheme(uri) + ":" + uri.getPath());
}
private String uriScheme(URI uri) {

View File

@@ -29,6 +29,7 @@ import org.springframework.cloud.sleuth.instrument.async.SpanContinuingTraceCall
import org.springframework.cloud.sleuth.instrument.async.SpanContinuingTraceRunnable;
import org.springframework.cloud.sleuth.log.SpanLogger;
import org.springframework.cloud.sleuth.util.ExceptionUtils;
import org.springframework.cloud.sleuth.util.SpanNameUtil;
/**
* Default implementation of {@link Tracer}
@@ -38,6 +39,8 @@ import org.springframework.cloud.sleuth.util.ExceptionUtils;
*/
public class DefaultTracer implements Tracer {
private static final int MAX_CHARS_IN_SPAN_NAME = 50;
private final Sampler defaultSampler;
private final Random random;
@@ -96,13 +99,14 @@ public class DefaultTracer implements Tracer {
@Override
public Span createSpan(String name, Sampler sampler) {
String shortenedName = SpanNameUtil.shorten(name);
Span span;
if (isTracing()) {
span = createChild(getCurrentSpan(), name);
span = createChild(getCurrentSpan(), shortenedName);
}
else {
long id = createId();
span = Span.builder().name(name)
span = Span.builder().name(shortenedName)
.traceIdHigh(this.traceId128 ? createId() : 0L)
.traceId(id)
.spanId(id).build();
@@ -115,6 +119,11 @@ public class DefaultTracer implements Tracer {
return continueSpan(span);
}
private String shortenNameIfNecessary(String name) {
int maxLength = name.length() > MAX_CHARS_IN_SPAN_NAME ? MAX_CHARS_IN_SPAN_NAME : name.length();
return name.substring(0, maxLength);
}
@Override
public Span detach(Span span) {
if (span == null) {
@@ -166,9 +175,10 @@ public class DefaultTracer implements Tracer {
}
Span createChild(Span parent, String name) {
String shortenedName = SpanNameUtil.shorten(name);
long id = createId();
if (parent == null) {
Span span = Span.builder().name(name)
Span span = Span.builder().name(shortenedName)
.traceIdHigh(this.traceId128 ? createId() : 0L)
.traceId(id)
.spanId(id).build();
@@ -180,7 +190,7 @@ public class DefaultTracer implements Tracer {
if (!isTracing()) {
SpanContextHolder.push(parent, true);
}
Span span = Span.builder().name(name)
Span span = Span.builder().name(shortenedName)
.traceIdHigh(parent.getTraceIdHigh())
.traceId(parent.getTraceId()).parent(parent.getSpanId()).spanId(id)
.processId(parent.getProcessId()).savedSpan(parent)

View File

@@ -16,6 +16,8 @@
package org.springframework.cloud.sleuth.util;
import org.springframework.util.StringUtils;
/**
* Utility class that provides the name in hyphen based notation
*
@@ -24,6 +26,16 @@ package org.springframework.cloud.sleuth.util;
*/
public final class SpanNameUtil {
static final int MAX_NAME_LENGTH = 50;
public static String shorten(String name) {
if (StringUtils.isEmpty(name)) {
return name;
}
int maxLength = name.length() > MAX_NAME_LENGTH ? MAX_NAME_LENGTH : name.length();
return name.substring(0, maxLength);
}
public static String toLowerHyphen(String name) {
StringBuilder result = new StringBuilder();
for (int i = 0; i < name.length(); i++) {
@@ -35,6 +47,6 @@ public final class SpanNameUtil {
result.append(c);
}
}
return result.toString();
return SpanNameUtil.shorten(result.toString());
}
}

View File

@@ -277,6 +277,28 @@ public class TraceChannelInterceptorTests implements MessageHandler {
then(TestSpanContextHolder.getCurrentSpan()).isNull();
}
@Test
public void shouldShortenTheNameWhenItsTooLarge() {
this.tracedChannel.send(MessageBuilder.withPayload("hi")
.setHeader(TraceMessageHeaders.SPAN_NAME_NAME, bigName())
.setHeader(TraceMessageHeaders.TRACE_ID_NAME, Span.idToHex(10L))
.setHeader(TraceMessageHeaders.SPAN_ID_NAME, Span.idToHex(20L)).build());
then(this.message).isNotNull();
then(this.accumulator.getSpans()).isNotEmpty();
this.accumulator.getSpans().forEach(span1 -> then(span1.getName().length()).isLessThanOrEqualTo(50));
then(TestSpanContextHolder.getCurrentSpan()).isNull();
}
private String bigName() {
StringBuilder sb = new StringBuilder();
for (int i = 0; i < 60; i++) {
sb.append("a");
}
return sb.toString();
}
@Test
public void serializeMutableHeaders() throws Exception {
Map<String, Object> headers = new HashMap<>();

View File

@@ -36,6 +36,7 @@ import org.springframework.cloud.sleuth.sampler.AlwaysSampler;
import org.springframework.cloud.sleuth.trace.DefaultTracer;
import org.springframework.cloud.sleuth.trace.TestSpanContextHolder;
import org.springframework.cloud.sleuth.util.ArrayListSpanAccumulator;
import org.springframework.cloud.sleuth.util.ExceptionUtils;
import org.springframework.http.HttpHeaders;
import org.springframework.http.client.ClientHttpRequestInterceptor;
import org.springframework.test.web.client.MockMvcClientHttpRequestFactory;
@@ -153,6 +154,28 @@ public class TraceRestTemplateInterceptorTests {
then(this.testController.span).hasNameEqualTo("http:/");
}
@Test
public void willShortenTheNameOfTheSpan() {
this.tracer.continueSpan(Span.builder().traceId(1L).spanId(2L).exportable(false).build());
try {
this.template.getForEntity("/" + bigName(), Map.class).getBody();
} catch (Exception e) {
}
then(this.spanAccumulator.getSpans().get(0).getName()).hasSize(50);
then(ExceptionUtils.getLastException()).isNull();
}
private String bigName() {
StringBuilder sb = new StringBuilder();
for (int i = 0; i < 60; i++) {
sb.append("a");
}
return sb.toString();
}
@RestController
public class TestController {

View File

@@ -105,4 +105,21 @@ public class TraceFeignClientTests {
.hasATag(Span.SPAN_ERROR_TAG_NAME, "exception has occurred");
}
@Test
public void should_shorten_the_span_name() throws IOException {
this.traceFeignClient.execute(
Request.create("GET", "http://foo/" + bigName(), new HashMap<>(), "".getBytes(),
Charset.defaultCharset()), new Request.Options());
then(this.spanAccumulator.getSpans().get(0).getName()).hasSize(50);
}
private String bigName() {
StringBuilder sb = new StringBuilder();
for (int i = 0; i < 60; i++) {
sb.append("a");
}
return sb.toString();
}
}

View File

@@ -68,7 +68,6 @@ public class DefaultTracerTests {
@Test
public void tracingWorks() {
DefaultTracer tracer = new DefaultTracer(NeverSampler.INSTANCE, new Random(),
new DefaultSpanNamer(), this.spanLogger, this.spanReporter, new TraceKeys());
@@ -219,6 +218,35 @@ public class DefaultTracerTests {
then(parent).isEqualTo(continuedSpan);
}
@Test
public void shouldCreateNewSpanWithShortenedName() {
DefaultTracer tracer = new DefaultTracer(new AlwaysSampler(), new Random(),
this.spanNamer, this.spanLogger, this.spanReporter, new TraceKeys());
Span span = tracer.createSpan(bigName());
then(span.getName().length()).isEqualTo(50);
tracer.close(span);
}
@Test
public void shouldCreateChildOfSpanWithShortenedName() {
DefaultTracer tracer = new DefaultTracer(new AlwaysSampler(), new Random(),
this.spanNamer, this.spanLogger, this.spanReporter, new TraceKeys());
Span span = Span.builder().name(bigName()).traceId(1L).spanId(1L).build();
Span child = tracer.createChild(span, bigName());
then(child.getName().length()).isEqualTo(50);
}
private String bigName() {
StringBuilder sb = new StringBuilder();
for (int i = 0; i < 60; i++) {
sb.append("a");
}
return sb.toString();
}
private Span assertSpan(List<Span> spans, Long parentId, String name) {
List<Span> found = findSpans(spans, parentId);
assertThat(found).as("More than one span with parentId %s", parentId).hasSize(1);

View File

@@ -32,4 +32,25 @@ public class SpanNameUtilTests {
SleuthAssertions.then(SpanNameUtil.toLowerHyphen("MySuperClassName"))
.isEqualTo("my-super-class-name");
}
@Test
public void should_not_shorten_a_name_that_is_below_max_threshold() throws Exception {
SleuthAssertions.then(SpanNameUtil.shorten("someName"))
.isEqualTo("someName");
}
@Test
public void should_not_shorten_a_name_that_is_null() throws Exception {
SleuthAssertions.then(SpanNameUtil.shorten(null)).isNull();
}
@Test
public void should_shorten_a_name_that_is_above_max_threshold() throws Exception {
StringBuilder sb = new StringBuilder();
for (int i = 0; i < 60; i++) {
sb.append("a");
}
SleuthAssertions.then(SpanNameUtil.shorten(sb.toString()).length())
.isEqualTo(SpanNameUtil.MAX_NAME_LENGTH);
}
}