diff --git a/docs/src/main/asciidoc/spring-cloud-sleuth.adoc b/docs/src/main/asciidoc/spring-cloud-sleuth.adoc index a8107ec6e..dd72e2c68 100644 --- a/docs/src/main/asciidoc/spring-cloud-sleuth.adoc +++ b/docs/src/main/asciidoc/spring-cloud-sleuth.adoc @@ -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 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 95bed7071..520211a76 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 @@ -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)); } } diff --git a/spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/instrument/web/client/AbstractTraceHttpRequestInterceptor.java b/spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/instrument/web/client/AbstractTraceHttpRequestInterceptor.java index d2564b9d7..75cd516f7 100644 --- a/spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/instrument/web/client/AbstractTraceHttpRequestInterceptor.java +++ b/spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/instrument/web/client/AbstractTraceHttpRequestInterceptor.java @@ -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(); } diff --git a/spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/instrument/web/client/feign/TraceFeignClient.java b/spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/instrument/web/client/feign/TraceFeignClient.java index c3d4c8a6f..9e6d5c93c 100644 --- a/spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/instrument/web/client/feign/TraceFeignClient.java +++ b/spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/instrument/web/client/feign/TraceFeignClient.java @@ -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) { diff --git a/spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/trace/DefaultTracer.java b/spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/trace/DefaultTracer.java index e57bf719e..94d0abe9a 100644 --- a/spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/trace/DefaultTracer.java +++ b/spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/trace/DefaultTracer.java @@ -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) diff --git a/spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/util/SpanNameUtil.java b/spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/util/SpanNameUtil.java index 78ef5ec7d..1aef47d1c 100644 --- a/spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/util/SpanNameUtil.java +++ b/spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/util/SpanNameUtil.java @@ -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()); } } diff --git a/spring-cloud-sleuth-core/src/test/java/org/springframework/cloud/sleuth/instrument/messaging/TraceChannelInterceptorTests.java b/spring-cloud-sleuth-core/src/test/java/org/springframework/cloud/sleuth/instrument/messaging/TraceChannelInterceptorTests.java index 9fc8f1c8e..5d96da57b 100644 --- a/spring-cloud-sleuth-core/src/test/java/org/springframework/cloud/sleuth/instrument/messaging/TraceChannelInterceptorTests.java +++ b/spring-cloud-sleuth-core/src/test/java/org/springframework/cloud/sleuth/instrument/messaging/TraceChannelInterceptorTests.java @@ -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 headers = new HashMap<>(); 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 8e5604888..2403b9eb6 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 @@ -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 { diff --git a/spring-cloud-sleuth-core/src/test/java/org/springframework/cloud/sleuth/instrument/web/client/feign/TraceFeignClientTests.java b/spring-cloud-sleuth-core/src/test/java/org/springframework/cloud/sleuth/instrument/web/client/feign/TraceFeignClientTests.java index fc8a1313f..dda574886 100644 --- a/spring-cloud-sleuth-core/src/test/java/org/springframework/cloud/sleuth/instrument/web/client/feign/TraceFeignClientTests.java +++ b/spring-cloud-sleuth-core/src/test/java/org/springframework/cloud/sleuth/instrument/web/client/feign/TraceFeignClientTests.java @@ -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(); + } + } \ No newline at end of file diff --git a/spring-cloud-sleuth-core/src/test/java/org/springframework/cloud/sleuth/trace/DefaultTracerTests.java b/spring-cloud-sleuth-core/src/test/java/org/springframework/cloud/sleuth/trace/DefaultTracerTests.java index 8f8cd674f..6c55909a9 100644 --- a/spring-cloud-sleuth-core/src/test/java/org/springframework/cloud/sleuth/trace/DefaultTracerTests.java +++ b/spring-cloud-sleuth-core/src/test/java/org/springframework/cloud/sleuth/trace/DefaultTracerTests.java @@ -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 spans, Long parentId, String name) { List found = findSpans(spans, parentId); assertThat(found).as("More than one span with parentId %s", parentId).hasSize(1); diff --git a/spring-cloud-sleuth-core/src/test/java/org/springframework/cloud/sleuth/util/SpanNameUtilTests.java b/spring-cloud-sleuth-core/src/test/java/org/springframework/cloud/sleuth/util/SpanNameUtilTests.java index 517576f2b..d531691ef 100644 --- a/spring-cloud-sleuth-core/src/test/java/org/springframework/cloud/sleuth/util/SpanNameUtilTests.java +++ b/spring-cloud-sleuth-core/src/test/java/org/springframework/cloud/sleuth/util/SpanNameUtilTests.java @@ -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); + } } \ No newline at end of file