From a603eee8418ff0c6c1eae27fb80795a4182edc71 Mon Sep 17 00:00:00 2001 From: Tyler Benson Date: Mon, 22 Oct 2018 12:23:51 +1000 Subject: [PATCH] Fix issue with saving tags when not propagating trace. Previously, when setting headers as tags, the tags would only be saved if the request was propagated from another trace. --- .../java/datadog/opentracing/DDTracer.java | 44 ++++++++++++------- .../opentracing/propagation/Codec.java | 3 +- .../propagation/ExtractedContext.java | 10 +---- .../opentracing/propagation/HTTPCodec.java | 18 +++++--- .../opentracing/propagation/TagContext.java | 22 ++++++++++ .../propagation/HTTPCodecTest.groovy | 18 ++++++++ 6 files changed, 84 insertions(+), 31 deletions(-) create mode 100644 dd-trace-ot/src/main/java/datadog/opentracing/propagation/TagContext.java diff --git a/dd-trace-ot/src/main/java/datadog/opentracing/DDTracer.java b/dd-trace-ot/src/main/java/datadog/opentracing/DDTracer.java index dd30422d81..28eb1ec9c3 100644 --- a/dd-trace-ot/src/main/java/datadog/opentracing/DDTracer.java +++ b/dd-trace-ot/src/main/java/datadog/opentracing/DDTracer.java @@ -5,6 +5,7 @@ import datadog.opentracing.decorators.DDDecoratorsFactory; import datadog.opentracing.propagation.Codec; import datadog.opentracing.propagation.ExtractedContext; import datadog.opentracing.propagation.HTTPCodec; +import datadog.opentracing.propagation.TagContext; import datadog.opentracing.scopemanager.ContextualScopeManager; import datadog.opentracing.scopemanager.ScopeContext; import datadog.trace.api.Config; @@ -545,25 +546,36 @@ public class DDTracer implements io.opentracing.Tracer, Closeable, datadog.trace spanType = ddsc.getSpanType(); } - // Propagate external trace - } else if (parentContext instanceof ExtractedContext) { - final ExtractedContext ddsc = (ExtractedContext) parentContext; - traceId = ddsc.getTraceId(); - parentSpanId = ddsc.getSpanId(); - baggage = ddsc.getBaggage(); - tags.putAll(ddsc.getTags()); - tags.put(Config.RUNTIME_ID_TAG, runtimeId); - parentTrace = new PendingTrace(DDTracer.this, traceId, serviceNameMappings); - samplingPriority = ddsc.getSamplingPriority(); - - // Start a new trace } else { - traceId = generateNewId(); - parentSpanId = "0"; - baggage = null; + if (parentContext instanceof ExtractedContext) { + // Propagate external trace + final ExtractedContext extractedContext = (ExtractedContext) parentContext; + traceId = extractedContext.getTraceId(); + parentSpanId = extractedContext.getSpanId(); + samplingPriority = extractedContext.getSamplingPriority(); + baggage = extractedContext.getBaggage(); + } else { + // Start a new trace + traceId = generateNewId(); + parentSpanId = "0"; + samplingPriority = PrioritySampling.UNSET; + baggage = null; + } + + // Get header tags whether propagating or not. + if (parentContext instanceof TagContext) { + final TagContext tagContext = (TagContext) parentContext; + + if (tags.isEmpty() && !tagContext.getTags().isEmpty()) { + tags = new HashMap<>(); + } + if (!tagContext.getTags().isEmpty()) { + tags.putAll(tagContext.getTags()); + } + } tags.put(Config.RUNTIME_ID_TAG, runtimeId); + parentTrace = new PendingTrace(DDTracer.this, traceId, serviceNameMappings); - samplingPriority = PrioritySampling.UNSET; } if (serviceName == null) { diff --git a/dd-trace-ot/src/main/java/datadog/opentracing/propagation/Codec.java b/dd-trace-ot/src/main/java/datadog/opentracing/propagation/Codec.java index f6f097d322..c6b513e5b1 100644 --- a/dd-trace-ot/src/main/java/datadog/opentracing/propagation/Codec.java +++ b/dd-trace-ot/src/main/java/datadog/opentracing/propagation/Codec.java @@ -22,6 +22,7 @@ package datadog.opentracing.propagation; import datadog.opentracing.DDSpanContext; +import io.opentracing.SpanContext; /** A codec is a simple object that can encode and decode a span context through a carrier */ public interface Codec { @@ -41,5 +42,5 @@ public interface Codec { * @param carrier * @return the span context */ - ExtractedContext extract(T carrier); + SpanContext extract(T carrier); } diff --git a/dd-trace-ot/src/main/java/datadog/opentracing/propagation/ExtractedContext.java b/dd-trace-ot/src/main/java/datadog/opentracing/propagation/ExtractedContext.java index e89d2f659f..f84804879b 100644 --- a/dd-trace-ot/src/main/java/datadog/opentracing/propagation/ExtractedContext.java +++ b/dd-trace-ot/src/main/java/datadog/opentracing/propagation/ExtractedContext.java @@ -1,15 +1,13 @@ package datadog.opentracing.propagation; -import io.opentracing.SpanContext; import java.util.Map; import java.util.concurrent.atomic.AtomicBoolean; -public class ExtractedContext implements SpanContext { +public class ExtractedContext extends TagContext { private final String traceId; private final String spanId; private final int samplingPriority; private final Map baggage; - private final Map tags; private final AtomicBoolean samplingPriorityLocked = new AtomicBoolean(false); public ExtractedContext( @@ -18,11 +16,11 @@ public class ExtractedContext implements SpanContext { final int samplingPriority, final Map baggage, final Map tags) { + super(tags); this.traceId = traceId; this.spanId = spanId; this.samplingPriority = samplingPriority; this.baggage = baggage; - this.tags = tags; } @Override @@ -50,10 +48,6 @@ public class ExtractedContext implements SpanContext { return baggage; } - public Map getTags() { - return tags; - } - public boolean getSamplingPriorityLocked() { return samplingPriorityLocked.get(); } diff --git a/dd-trace-ot/src/main/java/datadog/opentracing/propagation/HTTPCodec.java b/dd-trace-ot/src/main/java/datadog/opentracing/propagation/HTTPCodec.java index 6df5cbcd27..3d5158a85e 100644 --- a/dd-trace-ot/src/main/java/datadog/opentracing/propagation/HTTPCodec.java +++ b/dd-trace-ot/src/main/java/datadog/opentracing/propagation/HTTPCodec.java @@ -2,6 +2,7 @@ package datadog.opentracing.propagation; import datadog.opentracing.DDSpanContext; import datadog.trace.api.sampling.PrioritySampling; +import io.opentracing.SpanContext; import io.opentracing.propagation.TextMap; import java.io.UnsupportedEncodingException; import java.math.BigInteger; @@ -49,7 +50,7 @@ public class HTTPCodec implements Codec { } @Override - public ExtractedContext extract(final TextMap carrier) { + public SpanContext extract(final TextMap carrier) { Map baggage = Collections.emptyMap(); Map tags = Collections.emptyMap(); @@ -81,12 +82,17 @@ public class HTTPCodec implements Codec { tags.put(taggedHeaders.get(key), decode(val)); } } - ExtractedContext context = null; - if (!"0".equals(traceId)) { - context = new ExtractedContext(traceId, spanId, samplingPriority, baggage, tags); - context.lockSamplingPriority(); - log.debug("{} - Parent context extracted", context.getTraceId()); + SpanContext context = null; + if (!"0".equals(traceId)) { + final ExtractedContext ctx = + new ExtractedContext(traceId, spanId, samplingPriority, baggage, tags); + ctx.lockSamplingPriority(); + + log.debug("{} - Parent context extracted", ctx.getTraceId()); + context = ctx; + } else if (!tags.isEmpty()) { + context = new TagContext(tags); } return context; diff --git a/dd-trace-ot/src/main/java/datadog/opentracing/propagation/TagContext.java b/dd-trace-ot/src/main/java/datadog/opentracing/propagation/TagContext.java new file mode 100644 index 0000000000..6198a292a8 --- /dev/null +++ b/dd-trace-ot/src/main/java/datadog/opentracing/propagation/TagContext.java @@ -0,0 +1,22 @@ +package datadog.opentracing.propagation; + +import io.opentracing.SpanContext; +import java.util.Collections; +import java.util.Map; + +public class TagContext implements SpanContext { + private final Map tags; + + public TagContext(final Map tags) { + this.tags = tags; + } + + public Map getTags() { + return tags; + } + + @Override + public Iterable> baggageItems() { + return Collections.emptyList(); + } +} diff --git a/dd-trace-ot/src/test/groovy/datadog/opentracing/propagation/HTTPCodecTest.groovy b/dd-trace-ot/src/test/groovy/datadog/opentracing/propagation/HTTPCodecTest.groovy index 8fdc478b2c..d61008bee5 100644 --- a/dd-trace-ot/src/test/groovy/datadog/opentracing/propagation/HTTPCodecTest.groovy +++ b/dd-trace-ot/src/test/groovy/datadog/opentracing/propagation/HTTPCodecTest.groovy @@ -186,6 +186,24 @@ class HTTPCodecTest extends Specification { PrioritySampling.SAMPLER_KEEP | _ } + def "extract header tags with no propagation"() { + setup: + final Map actual = [ + SOME_HEADER: "my-interesting-info", + ] + + TagContext context = codec.extract(new TextMapExtractAdapter(actual)) + + expect: + !(context instanceof ExtractedContext) + context.getTags() == ["some-tag": "my-interesting-info"] + } + + def "extract empty headers returns null"() { + expect: + codec.extract(new TextMapExtractAdapter(["ignored-header": "ignored-value"])) == null + } + def "extract http headers with larger than Java long IDs"() { setup: String largeTraceId = "9523372036854775807"