From 8c7a20b07c903d4d8ef4bba828afb11051ef54b9 Mon Sep 17 00:00:00 2001 From: Anton Kislitsyn Date: Mon, 16 Oct 2017 09:14:35 +0300 Subject: [PATCH 1/2] Filter values having Zipkin compatible keys (#740) Optimization linked with necessity to exclude unrequired values from the temporary carrier of header values --- .../web/HttpServletRequestTextMap.java | 49 +++++++--- .../web/ZipkinHttpSpanExtractor.java | 64 +++++++------ .../web/ZipkinHttpSpanInjector.java | 11 ++- .../instrument/web/ZipkinHttpSpanMapper.java | 77 ++++++++++++++++ .../web/ZipkinHttpSpanMapperTest.java | 92 +++++++++++++++++++ 5 files changed, 243 insertions(+), 50 deletions(-) create mode 100644 spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/instrument/web/ZipkinHttpSpanMapper.java create mode 100644 spring-cloud-sleuth-core/src/test/java/org/springframework/cloud/sleuth/instrument/web/ZipkinHttpSpanMapperTest.java diff --git a/spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/instrument/web/HttpServletRequestTextMap.java b/spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/instrument/web/HttpServletRequestTextMap.java index 2981271c8..f3c2da620 100644 --- a/spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/instrument/web/HttpServletRequestTextMap.java +++ b/spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/instrument/web/HttpServletRequestTextMap.java @@ -16,12 +16,13 @@ package org.springframework.cloud.sleuth.instrument.web; -import javax.servlet.http.HttpServletRequest; +import java.util.AbstractMap; import java.util.Enumeration; -import java.util.HashMap; import java.util.Iterator; import java.util.Map; +import javax.servlet.http.HttpServletRequest; + import org.springframework.cloud.sleuth.SpanTextMap; import org.springframework.web.util.UrlPathHelper; @@ -34,29 +35,47 @@ import org.springframework.web.util.UrlPathHelper; class HttpServletRequestTextMap implements SpanTextMap { private final HttpServletRequest delegate; - private final Map additionalHeaders = new HashMap<>(); + private final UrlPathHelper urlPathHelper; HttpServletRequestTextMap(HttpServletRequest delegate) { this.delegate = delegate; - UrlPathHelper urlPathHelper = new UrlPathHelper(); - this.additionalHeaders.put(ZipkinHttpSpanExtractor.URI_HEADER, - urlPathHelper.getPathWithinApplication(delegate)); + this.urlPathHelper = new UrlPathHelper(); } @Override public Iterator> iterator() { - Map map = new HashMap<>(); - Enumeration headerNames = this.delegate.getHeaderNames(); - while (headerNames != null && headerNames.hasMoreElements()) { - String name = headerNames.nextElement(); - map.put(name, this.delegate.getHeader(name)); - } - map.putAll(this.additionalHeaders); - return map.entrySet().iterator(); + final Enumeration headerNames = this.delegate.getHeaderNames(); + + return new Iterator>() { + + private boolean useAdditionalHeader = true; + + @Override + public boolean hasNext() { + return useAdditionalHeader + || (headerNames != null && headerNames.hasMoreElements()); + } + + @Override + public Map.Entry next() { + if (useAdditionalHeader) { + useAdditionalHeader = false; + return new AbstractMap.SimpleImmutableEntry<>( + ZipkinHttpSpanMapper.URI_HEADER, + HttpServletRequestTextMap.this.urlPathHelper + .getPathWithinApplication( + HttpServletRequestTextMap.this.delegate)); + } + + String name = headerNames.nextElement(); + String value = HttpServletRequestTextMap.this.delegate.getHeader(name); + return new AbstractMap.SimpleEntry<>(name, value); + } + }; } @Override public void put(String key, String value) { - this.additionalHeaders.put(key, value); + throw new UnsupportedOperationException("change servlet request isn't supported"); } } diff --git a/spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/instrument/web/ZipkinHttpSpanExtractor.java b/spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/instrument/web/ZipkinHttpSpanExtractor.java index 9881182b9..19512f78d 100644 --- a/spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/instrument/web/ZipkinHttpSpanExtractor.java +++ b/spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/instrument/web/ZipkinHttpSpanExtractor.java @@ -8,7 +8,6 @@ import java.util.regex.Pattern; import org.apache.commons.logging.LogFactory; import org.springframework.cloud.sleuth.Span; import org.springframework.cloud.sleuth.SpanTextMap; -import org.springframework.cloud.sleuth.util.TextMapUtil; import org.springframework.util.StringUtils; /** @@ -21,35 +20,33 @@ public class ZipkinHttpSpanExtractor implements HttpSpanExtractor { private static final org.apache.commons.logging.Log log = LogFactory.getLog( MethodHandles.lookup().lookupClass()); - private static final String HEADER_DELIMITER = "-"; - static final String URI_HEADER = "X-Span-Uri"; + private static final String HTTP_COMPONENT = "http"; + private static final ZipkinHttpSpanMapper SPAN_CARRIER_MAPPER = new ZipkinHttpSpanMapper(); + private final Pattern skipPattern; + private final Random random; public ZipkinHttpSpanExtractor(Pattern skipPattern) { this.skipPattern = skipPattern; + this.random = new Random(); } @Override public Span joinTrace(SpanTextMap textMap) { - Map carrier = TextMapUtil.asMap(textMap); + Map carrier = SPAN_CARRIER_MAPPER.convert(textMap); + boolean debug = Span.SPAN_SAMPLED.equals(carrier.get(Span.SPAN_FLAGS)); boolean idToBeGenerated = debug && onlySpanIdIsPresent(carrier); - if (idToBeGenerated) { - // we're only generating Trace ID since if there's no Span ID will assume - // that it's equal to Trace ID - we're trying to fix a malformed request - generateIdIfMissing(carrier, Span.TRACE_ID_NAME); - } else if (traceIdIsMissing(carrier)) { + // we're only generating Trace ID since if there's no Span ID will assume + // that it's equal to Trace ID - we're trying to fix a malformed request + if (!idToBeGenerated && traceIdIsMissing(carrier)) { // can't build a Span without trace id return null; } try { - String uri = carrier.get(URI_HEADER); - boolean skip = this.skipPattern.matcher(uri).matches() - || Span.SPAN_NOT_SAMPLED.equals(carrier.get(Span.SAMPLED_NAME)); - long spanId = spanId(carrier); - return buildParentSpan(carrier, uri, skip, spanId, idToBeGenerated); + return buildParentSpan(carrier, idToBeGenerated); } catch (Exception e) { log.error("Exception occurred while trying to extract span from carrier", e); return null; @@ -68,46 +65,51 @@ public class ZipkinHttpSpanExtractor implements HttpSpanExtractor { return carrier.get(Span.SPAN_ID_NAME) != null; } - private void generateIdIfMissing(Map carrier, String key) { - if (!carrier.containsKey(key)) { - carrier.put(key, Span.idToHex(new Random().nextLong())); - } + private String generateId() { + return Span.idToHex(this.random.nextLong()); } - private long spanId(Map carrier) { - String spanId = carrier.get(Span.SPAN_ID_NAME); + private long spanId(String spanId, String traceId) { if (spanId == null) { if (log.isDebugEnabled()) { log.debug("Request is missing a span id but it has a trace id. We'll assume that this is " + "a root span with span id equal to the lower 64-bits of the trace id"); } - return Span.hexToId(carrier.get(Span.TRACE_ID_NAME)); + return Span.hexToId(traceId); } else { return Span.hexToId(spanId); } } - private Span buildParentSpan(Map carrier, String uri, boolean skip, - long spanId, boolean idToBeGenerated) { + private Span buildParentSpan(Map carrier, boolean idToBeGenerated) { String traceId = carrier.get(Span.TRACE_ID_NAME); + if (traceId == null) { + traceId = generateId(); + } Span.SpanBuilder span = Span.builder() .traceIdHigh(traceId.length() == 32 ? Span.hexToId(traceId, 0) : 0) .traceId(Span.hexToId(traceId)) - .spanId(spanId); - String processId = carrier.get(Span.PROCESS_ID_NAME); + .spanId(spanId(carrier.get(Span.SPAN_ID_NAME), traceId)); String parentName = carrier.get(Span.SPAN_NAME_NAME); if (StringUtils.hasText(parentName)) { span.name(parentName); } else { - span.name(HTTP_COMPONENT + ":/parent" + uri); + span.name(HTTP_COMPONENT + ":/parent" + + carrier.get(ZipkinHttpSpanMapper.URI_HEADER)); } + String processId = carrier.get(Span.PROCESS_ID_NAME); if (StringUtils.hasText(processId)) { span.processId(processId); } - if (carrier.containsKey(Span.PARENT_ID_NAME)) { - span.parent(Span.hexToId(carrier.get(Span.PARENT_ID_NAME))); + String parentId = carrier.get(Span.PARENT_ID_NAME); + if (parentId != null) { + span.parent(Span.hexToId(parentId)); } span.remote(true); + + boolean skip = this.skipPattern + .matcher(carrier.get(ZipkinHttpSpanMapper.URI_HEADER)).matches() + || Span.SPAN_NOT_SAMPLED.equals(carrier.get(Span.SAMPLED_NAME)); // trace, span id were retrieved from the headers and span is sampled span.shared(!(skip || idToBeGenerated)); boolean debug = Span.SPAN_SAMPLED.equals(carrier.get(Span.SPAN_FLAGS)); @@ -117,7 +119,8 @@ public class ZipkinHttpSpanExtractor implements HttpSpanExtractor { span.exportable(false); } for (Map.Entry entry : carrier.entrySet()) { - if (entry.getKey().toLowerCase().startsWith(Span.SPAN_BAGGAGE_HEADER_PREFIX + HEADER_DELIMITER)) { + if (entry.getKey().toLowerCase() + .startsWith(ZipkinHttpSpanMapper.BAGGAGE_PREFIX)) { span.baggage(unprefixedKey(entry.getKey()), entry.getValue()); } } @@ -125,7 +128,8 @@ public class ZipkinHttpSpanExtractor implements HttpSpanExtractor { } private String unprefixedKey(String key) { - return key.substring(key.indexOf(HEADER_DELIMITER) + 1).toLowerCase(); + return key.substring(key.indexOf(ZipkinHttpSpanMapper.HEADER_DELIMITER) + 1) + .toLowerCase(); } } diff --git a/spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/instrument/web/ZipkinHttpSpanInjector.java b/spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/instrument/web/ZipkinHttpSpanInjector.java index 6c085be1a..6767a1e14 100644 --- a/spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/instrument/web/ZipkinHttpSpanInjector.java +++ b/spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/instrument/web/ZipkinHttpSpanInjector.java @@ -4,7 +4,6 @@ import java.util.Map; import org.springframework.cloud.sleuth.Span; import org.springframework.cloud.sleuth.SpanTextMap; -import org.springframework.cloud.sleuth.util.TextMapUtil; import org.springframework.util.StringUtils; /** @@ -15,11 +14,11 @@ import org.springframework.util.StringUtils; */ public class ZipkinHttpSpanInjector implements HttpSpanInjector { - private static final String HEADER_DELIMITER = "-"; + private static final ZipkinHttpSpanMapper SPAN_CARRIER_MAPPER = new ZipkinHttpSpanMapper(); @Override public void inject(Span span, SpanTextMap map) { - Map carrier = TextMapUtil.asMap(map); + Map carrier = SPAN_CARRIER_MAPPER.convert(map); setHeader(map, carrier, Span.TRACE_ID_NAME, span.traceIdString()); setIdHeader(map, carrier, Span.SPAN_ID_NAME, span.getSpanId()); setHeader(map, carrier, Span.SAMPLED_NAME, span.isExportable() ? Span.SPAN_SAMPLED : Span.SPAN_NOT_SAMPLED); @@ -32,10 +31,12 @@ public class ZipkinHttpSpanInjector implements HttpSpanInjector { } private String prefixedKey(String key) { - if (key.startsWith(Span.SPAN_BAGGAGE_HEADER_PREFIX + HEADER_DELIMITER)) { + if (key.startsWith(Span.SPAN_BAGGAGE_HEADER_PREFIX + + ZipkinHttpSpanMapper.HEADER_DELIMITER)) { return key; } - return Span.SPAN_BAGGAGE_HEADER_PREFIX + HEADER_DELIMITER + key; + return Span.SPAN_BAGGAGE_HEADER_PREFIX + ZipkinHttpSpanMapper.HEADER_DELIMITER + + key; } private Long getParentId(Span span) { diff --git a/spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/instrument/web/ZipkinHttpSpanMapper.java b/spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/instrument/web/ZipkinHttpSpanMapper.java new file mode 100644 index 000000000..c2c2d9076 --- /dev/null +++ b/spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/instrument/web/ZipkinHttpSpanMapper.java @@ -0,0 +1,77 @@ +/* + * Copyright 2013-2017 the original author or authors. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.springframework.cloud.sleuth.instrument.web; + +import java.util.Collections; +import java.util.Comparator; +import java.util.Map; +import java.util.Set; +import java.util.TreeMap; +import java.util.TreeSet; + +import org.springframework.cloud.sleuth.Span; +import org.springframework.cloud.sleuth.SpanTextMap; + +/** + * Mapper util for filter Zipkin compatible carrier values only from {@link SpanTextMap} + * + * @author Anton Kislitsyn + */ +class ZipkinHttpSpanMapper { + + static final String HEADER_DELIMITER = "-"; + static final String BAGGAGE_PREFIX = Span.SPAN_BAGGAGE_HEADER_PREFIX + + HEADER_DELIMITER; + static final String URI_HEADER = "X-Span-Uri"; + + private static Comparator IGNORE_CASE_COMPARATOR = new Comparator() { + @Override + public int compare(String o1, String o2) { + return o1.toLowerCase().compareTo(o2.toLowerCase()); + } + }; + + /** + * Acceptable span fields + */ + private static final Set SPAN_FIELDS; + + static { + TreeSet fields = new TreeSet<>(IGNORE_CASE_COMPARATOR); + Collections.addAll(fields, Span.SPAN_FLAGS, Span.TRACE_ID_NAME, Span.SPAN_ID_NAME, + Span.PROCESS_ID_NAME, Span.SPAN_NAME_NAME, Span.PARENT_ID_NAME, + Span.SAMPLED_NAME, URI_HEADER); + SPAN_FIELDS = Collections.unmodifiableSet(fields); + } + + /** + * Create new Map of carrier values + */ + Map convert(SpanTextMap textMap) { + Map carrier = new TreeMap<>(IGNORE_CASE_COMPARATOR); + for (Map.Entry entry : textMap) { + if (isAcceptable(entry.getKey())) { + carrier.put(entry.getKey(), entry.getValue()); + } + } + return Collections.unmodifiableMap(carrier); + } + + private boolean isAcceptable(String key) { + return SPAN_FIELDS.contains(key) || key.startsWith(BAGGAGE_PREFIX); + } +} diff --git a/spring-cloud-sleuth-core/src/test/java/org/springframework/cloud/sleuth/instrument/web/ZipkinHttpSpanMapperTest.java b/spring-cloud-sleuth-core/src/test/java/org/springframework/cloud/sleuth/instrument/web/ZipkinHttpSpanMapperTest.java new file mode 100644 index 000000000..86598ee35 --- /dev/null +++ b/spring-cloud-sleuth-core/src/test/java/org/springframework/cloud/sleuth/instrument/web/ZipkinHttpSpanMapperTest.java @@ -0,0 +1,92 @@ +/* + * Copyright 2013-2017 the original author or authors. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.springframework.cloud.sleuth.instrument.web; + +import static org.hamcrest.Matchers.equalTo; +import static org.hamcrest.Matchers.is; +import static org.hamcrest.Matchers.nullValue; +import static org.junit.Assert.assertThat; +import static org.springframework.cloud.sleuth.instrument.web.ZipkinHttpSpanMapper.URI_HEADER; + +import java.util.Arrays; +import java.util.Collection; +import java.util.Collections; +import java.util.Iterator; +import java.util.Map; +import java.util.UUID; + +import org.junit.Test; +import org.junit.runner.RunWith; +import org.junit.runners.Parameterized; +import org.springframework.cloud.sleuth.Span; +import org.springframework.cloud.sleuth.SpanTextMap; + +/** + * @author Anton Kislitsyn + */ +@RunWith(Parameterized.class) +public class ZipkinHttpSpanMapperTest { + + private static final ZipkinHttpSpanMapper MAPPER = new ZipkinHttpSpanMapper(); + + private final String headerName; + private final String value; + private final boolean suitable; + + @Parameterized.Parameters + public static Collection parameters() { + return Arrays.asList(new Object[] { Span.TRACE_ID_NAME, "traceId", true }, + new Object[] { Span.SPAN_ID_NAME, "spanId", true }, + new Object[] { Span.SPAN_FLAGS, "flags", true }, + new Object[] { Span.PROCESS_ID_NAME, "process", true }, + new Object[] { Span.SPAN_NAME_NAME, "name", true }, + new Object[] { Span.PARENT_ID_NAME, "parent", true }, + new Object[] { Span.SAMPLED_NAME, "sampled", true }, + new Object[] { URI_HEADER, "uri", true }, + new Object[] { UUID.randomUUID().toString(), UUID.randomUUID().toString(), + false }, + new Object[] { Span.SPAN_ID_NAME, null, false }, + new Object[] { UUID.randomUUID().toString(), null, false }); + } + + public ZipkinHttpSpanMapperTest(String headerName, String value, boolean suitable) { + this.headerName = headerName; + this.value = value; + this.suitable = suitable; + } + + @Test + public void should_map_zipkin_suitable_fields() throws Exception { + Map map = MAPPER + .convert(textMap(Collections.singletonMap(headerName, value))); + assertThat(map.get(headerName), suitable ? equalTo(value) : is(nullValue())); + } + + private SpanTextMap textMap(Map textMap) { + return new SpanTextMap() { + @Override + public Iterator> iterator() { + return textMap.entrySet().iterator(); + } + + @Override + public void put(String key, String value) { + textMap.put(key, value); + } + }; + } +} \ No newline at end of file From 806f07e4ce7d02c685459ec4f4e829abfb5dca19 Mon Sep 17 00:00:00 2001 From: Marcin Grzejszczak Date: Mon, 16 Oct 2017 09:21:05 +0200 Subject: [PATCH 2/2] Updated docs with service name & bootstrap config; fixes #741 --- README.adoc | 3 +++ docs/src/main/asciidoc/intro.adoc | 3 +++ 2 files changed, 6 insertions(+) diff --git a/README.adoc b/README.adoc index 452eeb5f6..af443d190 100644 --- a/README.adoc +++ b/README.adoc @@ -350,6 +350,9 @@ tracer.addTag(baggageKey, baggageValue); === Adding to the project +IMPORTANT: To ensure that your application name is properly displayed in Zipkin + set the `spring.application.name` property in `bootstrap.yml`. + ==== Only Sleuth (log correlation) If you want to profit only from Spring Cloud Sleuth without the Zipkin integration just add diff --git a/docs/src/main/asciidoc/intro.adoc b/docs/src/main/asciidoc/intro.adoc index 7549036c9..609c63765 100644 --- a/docs/src/main/asciidoc/intro.adoc +++ b/docs/src/main/asciidoc/intro.adoc @@ -229,6 +229,9 @@ tracer.addTag(baggageKey, baggageValue); === Adding to the project +IMPORTANT: To ensure that your application name is properly displayed in Zipkin + set the `spring.application.name` property in `bootstrap.yml`. + ==== Only Sleuth (log correlation) If you want to profit only from Spring Cloud Sleuth without the Zipkin integration just add