From 10aee2852bd162094e9a9c8a5eacffae85c14597 Mon Sep 17 00:00:00 2001 From: Tyler Benson Date: Mon, 2 Nov 2020 16:56:51 -0500 Subject: [PATCH] Update OpenTelemetry Bridge to latest version Remove some of the interfaces from the wrapped objects to stop encouraging casting to our interfaces as a way to integrate. Add instrumentation to sync our context with the new otel context. Due to implementation differences, there are still some edge cases where they can get out of sync. --- .../opentelemetry/opentelemetry.gradle | 29 ++- .../OpenTelemetryInstrumentation.java | 59 +++-- .../OpenTelemetryContextInstrumentation.java | 53 +++++ .../opentelemetry/OtelContextPropagators.java | 80 +++---- .../opentelemetry/OtelScope.java | 23 +- .../opentelemetry/OtelSpan.java | 220 +++++------------- .../opentelemetry/OtelSpanContext.java | 47 ---- .../opentelemetry/OtelTracer.java | 108 +++------ .../opentelemetry/OtelTracerProvider.java | 16 +- .../opentelemetry/TypeConverter.java | 108 +++++++-- .../context/ContextStorageAdvice.java | 29 +++ .../opentelemetry/context/WrappedScope.java | 20 ++ .../src/test/groovy/OpenTelemetryTest.groovy | 204 +++++++++------- .../java/datadog/trace/core/CoreTracer.java | 11 +- .../datadog/trace/core/DDSpanContext.java | 5 + .../trace/core/propagation/TagContext.java | 5 + .../instrumentation/api/AgentSpan.java | 2 + .../instrumentation/api/AgentTracer.java | 5 + 18 files changed, 513 insertions(+), 511 deletions(-) create mode 100644 dd-java-agent/instrumentation/opentelemetry/src/main/java/datadog/trace/instrumentation/opentelemetry/context/OpenTelemetryContextInstrumentation.java delete mode 100644 dd-java-agent/instrumentation/opentelemetry/src/main/java8/datadog/trace/instrumentation/opentelemetry/OtelSpanContext.java create mode 100644 dd-java-agent/instrumentation/opentelemetry/src/main/java8/datadog/trace/instrumentation/opentelemetry/context/ContextStorageAdvice.java create mode 100644 dd-java-agent/instrumentation/opentelemetry/src/main/java8/datadog/trace/instrumentation/opentelemetry/context/WrappedScope.java diff --git a/dd-java-agent/instrumentation/opentelemetry/opentelemetry.gradle b/dd-java-agent/instrumentation/opentelemetry/opentelemetry.gradle index 6cf63e69057..c1c4d2745dd 100644 --- a/dd-java-agent/instrumentation/opentelemetry/opentelemetry.gradle +++ b/dd-java-agent/instrumentation/opentelemetry/opentelemetry.gradle @@ -3,37 +3,36 @@ ext { } // Made this a variable so we can easily update to latest releases. -def otelVersion = '0.3.0' +def otelVersion = '0.11.0' muzzle { pass { module = 'opentelemetry-api' group = 'io.opentelemetry' - versions = "[$otelVersion,0.8.0)" + versions = "[$otelVersion,]" assertInverse = true - // I have no idea why gradle doesn't respect the version range above - // but I also have no time to figure it out for an instrumentation - // that is experimental skipVersions = ['0.2.2', '0.2.3', '0.9.0'] } } apply from: "$rootDir/gradle/java.gradle" -apply plugin: 'org.unbroken-dome.test-sets' - -testSets { - latestDepTest { - dirName = 'test' - } -} +// Can't have a latest test until a new version is released. +//apply plugin: 'org.unbroken-dome.test-sets' +// +//testSets { +// latestDepTest { +// dirName = 'test' +// } +//} dependencies { compileOnly group: 'io.opentelemetry', name: 'opentelemetry-api', version: otelVersion + main_java8CompileOnly group: 'io.opentelemetry', name: 'opentelemetry-api', version: otelVersion - compileOnly group: 'com.google.code.findbugs', name: 'jsr305', version: '3.0.2' - compileOnly group: 'com.google.auto.value', name: 'auto-value-annotations', version: '1.6.6' + main_java8CompileOnly group: 'com.google.code.findbugs', name: 'jsr305', version: '3.0.2' + main_java8CompileOnly group: 'com.google.auto.value', name: 'auto-value-annotations', version: '1.6.6' testCompile group: 'io.opentelemetry', name: 'opentelemetry-api', version: otelVersion - latestDepTestCompile group: 'io.opentelemetry', name: 'opentelemetry-api', version: '0.7+' + // latestDepTestCompile group: 'io.opentelemetry', name: 'opentelemetry-api', version: '0.7+' } diff --git a/dd-java-agent/instrumentation/opentelemetry/src/main/java/datadog/trace/instrumentation/opentelemetry/OpenTelemetryInstrumentation.java b/dd-java-agent/instrumentation/opentelemetry/src/main/java/datadog/trace/instrumentation/opentelemetry/OpenTelemetryInstrumentation.java index 6ef715e5d99..b48d147fe8f 100644 --- a/dd-java-agent/instrumentation/opentelemetry/src/main/java/datadog/trace/instrumentation/opentelemetry/OpenTelemetryInstrumentation.java +++ b/dd-java-agent/instrumentation/opentelemetry/src/main/java/datadog/trace/instrumentation/opentelemetry/OpenTelemetryInstrumentation.java @@ -1,13 +1,17 @@ package datadog.trace.instrumentation.opentelemetry; +import static datadog.trace.agent.tooling.bytebuddy.matcher.DDElementMatchers.implementsInterface; +import static java.util.Collections.singletonMap; import static net.bytebuddy.matcher.ElementMatchers.named; -import static net.bytebuddy.matcher.ElementMatchers.returns; +import static net.bytebuddy.matcher.ElementMatchers.takesNoArguments; import com.google.auto.service.AutoService; import datadog.trace.agent.tooling.Instrumenter; -import io.opentelemetry.context.propagation.ContextPropagators; -import io.opentelemetry.trace.TracerProvider; -import java.util.HashMap; +import datadog.trace.bootstrap.ContextStore; +import datadog.trace.bootstrap.InstrumentationContext; +import datadog.trace.bootstrap.instrumentation.api.AgentSpan; +import io.opentelemetry.api.OpenTelemetryBuilder; +import io.opentelemetry.api.trace.SpanContext; import java.util.Map; import net.bytebuddy.asm.Advice; import net.bytebuddy.description.method.MethodDescription; @@ -30,7 +34,7 @@ protected boolean defaultEnabled() { @Override public ElementMatcher typeMatcher() { - return named("io.opentelemetry.OpenTelemetry"); + return implementsInterface(named("io.opentelemetry.api.OpenTelemetryBuilder")); } @Override @@ -38,50 +42,39 @@ public String[] helperClassNames() { return new String[] { packageName + ".OtelScope", packageName + ".OtelSpan", - packageName + ".OtelSpan$1", // switch statement - packageName + ".OtelSpanContext", packageName + ".OtelTracer", - packageName + ".OtelTracer$1", // switch statement packageName + ".OtelTracerProvider", - packageName + ".OtelTracer$SpanBuilder", + packageName + ".OtelTracer$OtelSpanBuilder", packageName + ".OtelContextPropagators", packageName + ".OtelContextPropagators$1", // switch statement - packageName + ".OtelContextPropagators$OtelHttpTextFormat", + packageName + ".OtelContextPropagators$OtelTextMapPropagator", packageName + ".OtelContextPropagators$OtelSetter", packageName + ".OtelContextPropagators$OtelGetter", packageName + ".TypeConverter", + packageName + ".TypeConverter$OtelSpanContext" }; } @Override public Map, String> transformers() { - final Map, String> transformers = new HashMap<>(); - transformers.put( - named("getTracerProvider").and(returns(named("io.opentelemetry.trace.TracerProvider"))), - OpenTelemetryInstrumentation.class.getName() + "$TracerProviderAdvice"); - transformers.put( - named("getPropagators") - .and(returns(named("io.opentelemetry.context.propagation.ContextPropagators"))), - OpenTelemetryInstrumentation.class.getName() + "$ContextPropagatorsAdvice"); - return transformers; + return singletonMap( + named("build").and(takesNoArguments()), + OpenTelemetryInstrumentation.class.getName() + "$BuilderAdvice"); } - public static class TracerProviderAdvice { - @Advice.OnMethodExit(suppress = Throwable.class) - public static void returnProvider(@Advice.Return(readOnly = false) TracerProvider result) { - result = OtelTracerProvider.INSTANCE; - } + @Override + public Map contextStore() { + return singletonMap( + "io.opentelemetry.api.trace.SpanContext", AgentSpan.Context.class.getName()); } - public static class ContextPropagatorsAdvice { - @Advice.OnMethodExit(suppress = Throwable.class) - public static void returnProvider(@Advice.Return(readOnly = false) ContextPropagators result) { - result = OtelContextPropagators.INSTANCE; - } - - // Muzzle doesn't detect the advice method's argument type, so we have to help it a bit. - public static void muzzleCheck(final ContextPropagators propagators) { - propagators.getHttpTextFormat(); + public static class BuilderAdvice { + @Advice.OnMethodEnter(suppress = Throwable.class) + public static void beforeBuild(@Advice.This OpenTelemetryBuilder builder) { + ContextStore spanContextStore = + InstrumentationContext.get(SpanContext.class, AgentSpan.Context.class); + builder.setTracerProvider(new OtelTracerProvider(spanContextStore)); + builder.setPropagators(new OtelContextPropagators(spanContextStore)); } } } diff --git a/dd-java-agent/instrumentation/opentelemetry/src/main/java/datadog/trace/instrumentation/opentelemetry/context/OpenTelemetryContextInstrumentation.java b/dd-java-agent/instrumentation/opentelemetry/src/main/java/datadog/trace/instrumentation/opentelemetry/context/OpenTelemetryContextInstrumentation.java new file mode 100644 index 00000000000..f7d556ae7df --- /dev/null +++ b/dd-java-agent/instrumentation/opentelemetry/src/main/java/datadog/trace/instrumentation/opentelemetry/context/OpenTelemetryContextInstrumentation.java @@ -0,0 +1,53 @@ +package datadog.trace.instrumentation.opentelemetry.context; + +import static datadog.trace.agent.tooling.bytebuddy.matcher.DDElementMatchers.implementsInterface; +import static java.util.Collections.singletonMap; +import static net.bytebuddy.matcher.ElementMatchers.named; +import static net.bytebuddy.matcher.ElementMatchers.returns; +import static net.bytebuddy.matcher.ElementMatchers.takesArgument; + +import com.google.auto.service.AutoService; +import datadog.trace.agent.tooling.Instrumenter; +import java.util.Map; +import net.bytebuddy.description.method.MethodDescription; +import net.bytebuddy.description.type.TypeDescription; +import net.bytebuddy.matcher.ElementMatcher; + +/** + * This is experimental instrumentation and should only be enabled for evaluation/testing purposes. + */ +@AutoService(Instrumenter.class) +public class OpenTelemetryContextInstrumentation extends Instrumenter.Default { + public OpenTelemetryContextInstrumentation() { + super("opentelemetry-beta"); + } + + @Override + protected boolean defaultEnabled() { + return false; + } + + @Override + public ElementMatcher typeMatcher() { + return implementsInterface(named("io.opentelemetry.context.ContextStorage")); + } + + @Override + public String[] helperClassNames() { + return new String[] { + "datadog.trace.instrumentation.opentelemetry.OtelSpan", + "datadog.trace.instrumentation.opentelemetry.TypeConverter", + "datadog.trace.instrumentation.opentelemetry.TypeConverter$OtelSpanContext", + packageName + ".WrappedScope", + }; + } + + @Override + public Map, String> transformers() { + return singletonMap( + named("attach") + .and(takesArgument(0, named("io.opentelemetry.context.Context"))) + .and(returns(named("io.opentelemetry.context.Scope"))), + packageName + ".ContextStorageAdvice"); + } +} diff --git a/dd-java-agent/instrumentation/opentelemetry/src/main/java8/datadog/trace/instrumentation/opentelemetry/OtelContextPropagators.java b/dd-java-agent/instrumentation/opentelemetry/src/main/java8/datadog/trace/instrumentation/opentelemetry/OtelContextPropagators.java index 01c690d908b..7ff40491703 100644 --- a/dd-java-agent/instrumentation/opentelemetry/src/main/java8/datadog/trace/instrumentation/opentelemetry/OtelContextPropagators.java +++ b/dd-java-agent/instrumentation/opentelemetry/src/main/java8/datadog/trace/instrumentation/opentelemetry/OtelContextPropagators.java @@ -1,32 +1,37 @@ package datadog.trace.instrumentation.opentelemetry; +import datadog.trace.bootstrap.ContextStore; import datadog.trace.bootstrap.instrumentation.api.AgentPropagation; import datadog.trace.bootstrap.instrumentation.api.AgentSpan; import datadog.trace.bootstrap.instrumentation.api.AgentTracer; -import io.grpc.Context; +import io.opentelemetry.api.trace.Span; +import io.opentelemetry.api.trace.SpanContext; +import io.opentelemetry.context.Context; import io.opentelemetry.context.propagation.ContextPropagators; -import io.opentelemetry.context.propagation.HttpTextFormat; -import io.opentelemetry.trace.DefaultSpan; -import io.opentelemetry.trace.Span; -import io.opentelemetry.trace.TracingContextUtils; -import java.util.Arrays; +import io.opentelemetry.context.propagation.TextMapPropagator; import java.util.List; -public class OtelContextPropagators implements ContextPropagators { - public static final OtelContextPropagators INSTANCE = new OtelContextPropagators(); +public final class OtelContextPropagators implements ContextPropagators { - private OtelContextPropagators() {} + private final OtelTextMapPropagator propagator; + + public OtelContextPropagators(ContextStore spanContextStore) { + propagator = new OtelTextMapPropagator(spanContextStore); + } @Override - public HttpTextFormat getHttpTextFormat() { - return OtelHttpTextFormat.INSTANCE; + public TextMapPropagator getTextMapPropagator() { + return propagator; } - private static class OtelHttpTextFormat implements HttpTextFormat { - private static final OtelHttpTextFormat INSTANCE = new OtelHttpTextFormat(); + private static class OtelTextMapPropagator implements TextMapPropagator { private final AgentTracer.TracerAPI tracer = AgentTracer.get(); - private final TypeConverter converter = new TypeConverter(); + private final TypeConverter converter; + + private OtelTextMapPropagator(ContextStore spanContextStore) { + converter = new TypeConverter(spanContextStore); + } @Override public List fields() { @@ -35,8 +40,8 @@ public List fields() { @Override public void inject(final Context context, final C carrier, final Setter setter) { - final Span span = TracingContextUtils.getSpanWithoutDefault(context); - if (span == null || !span.getContext().isValid()) { + final Span span = Span.fromContextOrNull(context); + if (span == null || !span.getSpanContext().isValid()) { return; } tracer.inject(converter.toAgentSpan(span), carrier, new OtelSetter<>(setter)); @@ -45,15 +50,14 @@ public void inject(final Context context, final C carrier, final Setter s @Override public Context extract(final Context context, final C carrier, final Getter getter) { final AgentSpan.Context agentContext = tracer.extract(carrier, new OtelGetter<>(getter)); - return TracingContextUtils.withSpan( - DefaultSpan.create(converter.toSpanContext(agentContext)), context); + return Span.wrap(converter.toSpanContext(agentContext)).storeInContext(context); } } private static class OtelSetter implements AgentPropagation.Setter { - private final HttpTextFormat.Setter setter; + private final TextMapPropagator.Setter setter; - private OtelSetter(final HttpTextFormat.Setter setter) { + private OtelSetter(final TextMapPropagator.Setter setter) { this.setter = setter; } @@ -64,43 +68,15 @@ public void set(final C carrier, final String key, final String value) { } private static class OtelGetter implements AgentPropagation.ContextVisitor { - private static final String DD_TRACE_ID_KEY = "x-datadog-trace-id"; - private static final String DD_SPAN_ID_KEY = "x-datadog-parent-id"; - private static final String DD_SAMPLING_PRIORITY_KEY = "x-datadog-sampling-priority"; - private static final String DD_ORIGIN_KEY = "x-datadog-origin"; - - private static final String B3_TRACE_ID_KEY = "X-B3-TraceId"; - private static final String B3_SPAN_ID_KEY = "X-B3-SpanId"; - private static final String B3_SAMPLING_PRIORITY_KEY = "X-B3-Sampled"; - - private static final String HAYSTACK_TRACE_ID_KEY = "Trace-ID"; - private static final String HAYSTACK_SPAN_ID_KEY = "Span-ID"; - private static final String HAYSTACK_PARENT_ID_KEY = "Parent_ID"; - - private static final List KEYS = - Arrays.asList( - DD_TRACE_ID_KEY, - DD_SPAN_ID_KEY, - DD_SAMPLING_PRIORITY_KEY, - DD_ORIGIN_KEY, - B3_TRACE_ID_KEY, - B3_SPAN_ID_KEY, - B3_SAMPLING_PRIORITY_KEY, - HAYSTACK_TRACE_ID_KEY, - HAYSTACK_SPAN_ID_KEY, - HAYSTACK_PARENT_ID_KEY); - - private final HttpTextFormat.Getter getter; - - private OtelGetter(final HttpTextFormat.Getter getter) { + private final TextMapPropagator.Getter getter; + + private OtelGetter(final TextMapPropagator.Getter getter) { this.getter = getter; } @Override public void forEachKey(C carrier, AgentPropagation.KeyClassifier classifier) { - // TODO: Otel doesn't expose the keys, so we have to rely on hard coded keys. - // https://github.com/open-telemetry/opentelemetry-specification/issues/433 - for (String key : KEYS) { + for (String key : getter.keys(carrier)) { if (!classifier.accept(key, getter.get(carrier, key))) { return; } diff --git a/dd-java-agent/instrumentation/opentelemetry/src/main/java8/datadog/trace/instrumentation/opentelemetry/OtelScope.java b/dd-java-agent/instrumentation/opentelemetry/src/main/java8/datadog/trace/instrumentation/opentelemetry/OtelScope.java index ea778e0fe89..8b37476be61 100644 --- a/dd-java-agent/instrumentation/opentelemetry/src/main/java8/datadog/trace/instrumentation/opentelemetry/OtelScope.java +++ b/dd-java-agent/instrumentation/opentelemetry/src/main/java8/datadog/trace/instrumentation/opentelemetry/OtelScope.java @@ -1,41 +1,20 @@ package datadog.trace.instrumentation.opentelemetry; import datadog.trace.bootstrap.instrumentation.api.AgentScope; -import datadog.trace.context.TraceScope; import io.opentelemetry.context.Scope; -public class OtelScope implements Scope, TraceScope { +public final class OtelScope implements Scope { private final AgentScope delegate; OtelScope(final AgentScope delegate) { this.delegate = delegate; } - @Override - public Continuation capture() { - return delegate.capture(); - } - - @Override - public Continuation captureConcurrent() { - return delegate.captureConcurrent(); - } - @Override public void close() { delegate.close(); } - @Override - public boolean isAsyncPropagating() { - return delegate.isAsyncPropagating(); - } - - @Override - public void setAsyncPropagation(final boolean value) { - delegate.setAsyncPropagation(value); - } - public AgentScope getDelegate() { return delegate; } diff --git a/dd-java-agent/instrumentation/opentelemetry/src/main/java8/datadog/trace/instrumentation/opentelemetry/OtelSpan.java b/dd-java-agent/instrumentation/opentelemetry/src/main/java8/datadog/trace/instrumentation/opentelemetry/OtelSpan.java index fece226a9ec..df47fb33b30 100644 --- a/dd-java-agent/instrumentation/opentelemetry/src/main/java8/datadog/trace/instrumentation/opentelemetry/OtelSpan.java +++ b/dd-java-agent/instrumentation/opentelemetry/src/main/java8/datadog/trace/instrumentation/opentelemetry/OtelSpan.java @@ -1,18 +1,15 @@ package datadog.trace.instrumentation.opentelemetry; import datadog.trace.api.DDTags; -import datadog.trace.api.interceptor.MutableSpan; import datadog.trace.bootstrap.instrumentation.api.AgentSpan; -import io.opentelemetry.common.AttributeValue; -import io.opentelemetry.trace.EndSpanOptions; -import io.opentelemetry.trace.Event; -import io.opentelemetry.trace.Span; -import io.opentelemetry.trace.SpanContext; -import io.opentelemetry.trace.Status; -import java.util.Map; +import io.opentelemetry.api.common.AttributeKey; +import io.opentelemetry.api.common.Attributes; +import io.opentelemetry.api.trace.Span; +import io.opentelemetry.api.trace.SpanContext; +import io.opentelemetry.api.trace.StatusCode; import java.util.concurrent.TimeUnit; -public class OtelSpan implements Span, MutableSpan { +public final class OtelSpan implements Span { private final AgentSpan delegate; private final TypeConverter converter; @@ -22,217 +19,112 @@ public class OtelSpan implements Span, MutableSpan { } @Override - public void setAttribute(final String key, final String value) { + public Span setAttribute(final String key, final String value) { delegate.setTag(key, value); + return this; } @Override - public void setAttribute(final String key, final long value) { + public Span setAttribute(final String key, final long value) { delegate.setTag(key, value); + return this; } @Override - public void setAttribute(final String key, final double value) { + public Span setAttribute(final String key, final double value) { delegate.setTag(key, value); + return this; } @Override - public void setAttribute(final String key, final boolean value) { + public Span setAttribute(final String key, final boolean value) { delegate.setTag(key, value); + return this; } @Override - public void setAttribute(final String key, final AttributeValue value) { - switch (value.getType()) { - case LONG: - delegate.setTag(key, value.getLongValue()); - break; - case DOUBLE: - delegate.setTag(key, value.getDoubleValue()); - break; - case STRING: - delegate.setTag(key, value.getStringValue()); - break; - case BOOLEAN: - delegate.setTag(key, value.getBooleanValue()); - break; - default: - // Unsupported.... Ignoring. - } - } - - @Override - public void addEvent(final String name) {} - - @Override - public void addEvent(final String name, final long timestamp) {} - - @Override - public void addEvent(final String name, final Map attributes) {} - - @Override - public void addEvent( - final String name, final Map attributes, final long timestamp) {} - - @Override - public void addEvent(final Event event) {} - - @Override - public void addEvent(final Event event, final long timestamp) {} - - @Override - public void setStatus(final Status status) { - if (!status.isOk()) { - delegate.setError(true); - delegate.setTag(DDTags.ERROR_MSG, status.getDescription()); - } - } - - @Override - public void updateName(final String name) { - delegate.setResourceName(name); - } - - @Override - public void end() { - delegate.finish(); - } - - @Override - public void end(final EndSpanOptions endOptions) { - if (endOptions.getEndTimestamp() > 0) { - delegate.finish(TimeUnit.NANOSECONDS.toMicros(endOptions.getEndTimestamp())); - } else { - delegate.finish(); - } - } - - @Override - public SpanContext getContext() { - return converter.toSpanContext(delegate.context()); - } - - @Override - public boolean isRecording() { - return delegate.getTraceId().toLong() != 0; - } - - public AgentSpan getDelegate() { - return delegate; - } - - @Override - public long getStartTime() { - return delegate.getStartTime(); - } - - @Override - public long getDurationNano() { - return delegate.getDurationNano(); - } - - @Override - public CharSequence getOperationName() { - return delegate.getOperationName(); - } - - @Override - public MutableSpan setOperationName(final CharSequence serviceName) { - return delegate.setOperationName(serviceName); - } - - @Override - public String getServiceName() { - return delegate.getServiceName(); - } - - @Override - public MutableSpan setServiceName(final String serviceName) { - return delegate.setServiceName(serviceName); - } - - @Override - public CharSequence getResourceName() { - return delegate.getResourceName(); - } - - @Override - public MutableSpan setResourceName(final CharSequence resourceName) { - return delegate.setResourceName(resourceName); + public Span setAttribute(AttributeKey attributeKey, T t) { + delegate.setTag(attributeKey.getKey(), t); + return this; } @Override - public Integer getSamplingPriority() { - return delegate.getSamplingPriority(); + public Span addEvent(final String name) { + return this; } @Override - public MutableSpan setSamplingPriority(final int newPriority) { - return delegate.setSamplingPriority(newPriority); + public Span addEvent(String name, long timestamp, TimeUnit unit) { + return this; } @Override - public String getSpanType() { - return delegate.getSpanType(); + public Span addEvent(String s, Attributes attributes) { + return this; } @Override - public MutableSpan setSpanType(final CharSequence type) { - return delegate.setSpanType(type); + public Span addEvent(String name, Attributes attributes, long timestamp, TimeUnit unit) { + return this; } @Override - public Map getTags() { - return delegate.getTags(); - } - - @Override - public MutableSpan setTag(final String tag, final String value) { - return delegate.setTag(tag, value); + public Span setStatus(StatusCode statusCode) { + if (statusCode == StatusCode.ERROR) { + delegate.setError(true); + } + return this; } @Override - public MutableSpan setTag(final String tag, final boolean value) { - return delegate.setTag(tag, value); + public Span setStatus(StatusCode statusCode, String s) { + if (statusCode == StatusCode.ERROR) { + delegate.setError(true); + delegate.setTag(DDTags.ERROR_MSG, s); + } + return this; } @Override - public MutableSpan setTag(final String tag, final Number value) { - return delegate.setTag(tag, value); + public Span recordException(Throwable throwable) { + delegate.addThrowable(throwable); + return this; } @Override - public MutableSpan setMetric(final CharSequence metric, final int value) { - return delegate.setMetric(metric, value); + public Span recordException(Throwable throwable, Attributes attributes) { + attributes.forEach(this::setAttribute); + delegate.addThrowable(throwable); + return this; } @Override - public MutableSpan setMetric(final CharSequence metric, final long value) { - return delegate.setMetric(metric, value); + public Span updateName(final String name) { + delegate.setResourceName(name); + return this; } @Override - public MutableSpan setMetric(final CharSequence metric, final double value) { - return delegate.setMetric(metric, value); + public void end() { + delegate.finish(); } @Override - public Boolean isError() { - return delegate.isError(); + public void end(long timestamp, TimeUnit unit) { + delegate.finish(unit.toMicros(timestamp)); } @Override - public MutableSpan setError(final boolean value) { - return delegate.setError(value); + public SpanContext getSpanContext() { + return converter.toSpanContext(delegate.context()); } @Override - public MutableSpan getRootSpan() { - return delegate.getRootSpan(); + public boolean isRecording() { + return delegate.getTraceId().toLong() != 0; } - @Override - public MutableSpan getLocalRootSpan() { - return delegate.getLocalRootSpan(); + public AgentSpan getDelegate() { + return delegate; } } diff --git a/dd-java-agent/instrumentation/opentelemetry/src/main/java8/datadog/trace/instrumentation/opentelemetry/OtelSpanContext.java b/dd-java-agent/instrumentation/opentelemetry/src/main/java8/datadog/trace/instrumentation/opentelemetry/OtelSpanContext.java deleted file mode 100644 index 74a1deb2a49..00000000000 --- a/dd-java-agent/instrumentation/opentelemetry/src/main/java8/datadog/trace/instrumentation/opentelemetry/OtelSpanContext.java +++ /dev/null @@ -1,47 +0,0 @@ -package datadog.trace.instrumentation.opentelemetry; - -import datadog.trace.bootstrap.instrumentation.api.AgentSpan; -import io.opentelemetry.trace.SpanContext; -import io.opentelemetry.trace.SpanId; -import io.opentelemetry.trace.TraceFlags; -import io.opentelemetry.trace.TraceId; -import io.opentelemetry.trace.TraceState; - -public class OtelSpanContext extends SpanContext { - private static final TraceFlags FLAGS = TraceFlags.builder().setIsSampled(true).build(); - private final AgentSpan.Context delegate; - - OtelSpanContext(final AgentSpan.Context delegate) { - this.delegate = delegate; - } - - @Override - public TraceId getTraceId() { - return new TraceId(0, delegate.getTraceId().toLong()); - } - - @Override - public SpanId getSpanId() { - return new SpanId(delegate.getSpanId().toLong()); - } - - @Override - public TraceFlags getTraceFlags() { - return FLAGS; - } - - @Override - public TraceState getTraceState() { - return TraceState.getDefault(); - } - - @Override - public boolean isRemote() { - // check if delegate is a ExtractedContext - return false; - } - - AgentSpan.Context getDelegate() { - return delegate; - } -} diff --git a/dd-java-agent/instrumentation/opentelemetry/src/main/java8/datadog/trace/instrumentation/opentelemetry/OtelTracer.java b/dd-java-agent/instrumentation/opentelemetry/src/main/java8/datadog/trace/instrumentation/opentelemetry/OtelTracer.java index 7a49a672bde..fa7ffdb9210 100644 --- a/dd-java-agent/instrumentation/opentelemetry/src/main/java8/datadog/trace/instrumentation/opentelemetry/OtelTracer.java +++ b/dd-java-agent/instrumentation/opentelemetry/src/main/java8/datadog/trace/instrumentation/opentelemetry/OtelTracer.java @@ -1,20 +1,18 @@ package datadog.trace.instrumentation.opentelemetry; -import datadog.trace.bootstrap.instrumentation.api.AgentScope; -import datadog.trace.bootstrap.instrumentation.api.AgentSpan; import datadog.trace.bootstrap.instrumentation.api.AgentTracer; -import datadog.trace.bootstrap.instrumentation.api.ScopeSource; import datadog.trace.bootstrap.instrumentation.api.Tags; -import io.opentelemetry.common.AttributeValue; -import io.opentelemetry.context.Scope; -import io.opentelemetry.trace.Link; -import io.opentelemetry.trace.Span; -import io.opentelemetry.trace.SpanContext; -import io.opentelemetry.trace.Tracer; -import java.util.Map; +import io.opentelemetry.api.common.AttributeKey; +import io.opentelemetry.api.common.Attributes; +import io.opentelemetry.api.trace.Span; +import io.opentelemetry.api.trace.SpanBuilder; +import io.opentelemetry.api.trace.SpanContext; +import io.opentelemetry.api.trace.Tracer; +import io.opentelemetry.context.Context; +import java.time.Instant; import java.util.concurrent.TimeUnit; -public class OtelTracer implements Tracer { +public final class OtelTracer implements Tracer { private final String tracerName; private final AgentTracer.TracerAPI tracer; private final TypeConverter converter; @@ -27,46 +25,27 @@ public class OtelTracer implements Tracer { } @Override - public Span getCurrentSpan() { - return converter.toSpan(tracer.activeSpan()); + public SpanBuilder spanBuilder(final String spanName) { + return new OtelSpanBuilder(spanName); } - @Override - public Scope withSpan(final Span span) { - final AgentSpan agentSpan = converter.toAgentSpan(span); - final AgentScope agentScope = tracer.activateSpan(agentSpan, ScopeSource.MANUAL); - return converter.toScope(agentScope); - } - - @Override - public Span.Builder spanBuilder(final String spanName) { - return new SpanBuilder(spanName); - } - - private class SpanBuilder implements Span.Builder { + private class OtelSpanBuilder implements SpanBuilder { private final AgentTracer.SpanBuilder delegate; private boolean parentSet = false; - public SpanBuilder(final String spanName) { + public OtelSpanBuilder(final String spanName) { delegate = tracer.buildSpan(tracerName).withResourceName(spanName); } @Override - public Span.Builder setParent(final Span parent) { - parentSet = true; - delegate.asChildOf(converter.toAgentSpan(parent).context()); - return this; - } - - @Override - public Span.Builder setParent(final SpanContext remoteParent) { + public SpanBuilder setParent(Context context) { parentSet = true; - delegate.asChildOf(converter.toContext(remoteParent)); + delegate.asChildOf(converter.toAgentSpanContext(Span.fromContext(context).getSpanContext())); return this; } @Override - public Span.Builder setNoParent() { + public SpanBuilder setNoParent() { parentSet = true; delegate.asChildOf(null); delegate.ignoreActiveSpan(); @@ -74,85 +53,68 @@ public Span.Builder setNoParent() { } @Override - public Span.Builder addLink(final SpanContext spanContext) { + public SpanBuilder addLink(final SpanContext spanContext) { if (!parentSet) { - delegate.asChildOf(converter.toContext(spanContext)); + delegate.asChildOf(converter.toAgentSpanContext(spanContext)); } return this; } @Override - public Span.Builder addLink( - final SpanContext spanContext, final Map attributes) { + public SpanBuilder addLink(SpanContext spanContext, Attributes attributes) { if (!parentSet) { - delegate.asChildOf(converter.toContext(spanContext)); + delegate.asChildOf(converter.toAgentSpanContext(spanContext)); } return this; } @Override - public Span.Builder addLink(final Link link) { - if (!parentSet) { - delegate.asChildOf(converter.toContext(link.getContext())); - } + public SpanBuilder setAttribute(final String key, final String value) { + delegate.withTag(key, value); return this; } @Override - public Span.Builder setAttribute(final String key, final String value) { + public SpanBuilder setAttribute(final String key, final long value) { delegate.withTag(key, value); return this; } @Override - public Span.Builder setAttribute(final String key, final long value) { + public SpanBuilder setAttribute(final String key, final double value) { delegate.withTag(key, value); return this; } @Override - public Span.Builder setAttribute(final String key, final double value) { + public SpanBuilder setAttribute(final String key, final boolean value) { delegate.withTag(key, value); return this; } @Override - public Span.Builder setAttribute(final String key, final boolean value) { - delegate.withTag(key, value); + public SpanBuilder setAttribute(AttributeKey key, T t) { + delegate.withTag(key.getKey(), t); return this; } @Override - public Span.Builder setAttribute(final String key, final AttributeValue value) { - switch (value.getType()) { - case LONG: - delegate.withTag(key, value.getLongValue()); - break; - case DOUBLE: - delegate.withTag(key, value.getDoubleValue()); - break; - case STRING: - delegate.withTag(key, value.getStringValue()); - break; - case BOOLEAN: - delegate.withTag(key, value.getBooleanValue()); - break; - default: - // Unsupported.... Ignoring. - } + public SpanBuilder setSpanKind(final Span.Kind spanKind) { + // TODO: update delegate.operationName + delegate.withTag(Tags.SPAN_KIND, spanKind.name()); return this; } @Override - public Span.Builder setSpanKind(final Span.Kind spanKind) { - // TODO: update delegate.operationName - delegate.withTag(Tags.SPAN_KIND, spanKind.name()); + public SpanBuilder setStartTimestamp(long startTimestamp, TimeUnit unit) { + delegate.withStartTimestamp(unit.toMicros(startTimestamp)); return this; } @Override - public Span.Builder setStartTimestamp(final long startTimestamp) { - delegate.withStartTimestamp(TimeUnit.NANOSECONDS.toMicros(startTimestamp)); + public SpanBuilder setStartTimestamp(Instant startTimestamp) { + // FIXME: Loss of resolution + delegate.withStartTimestamp(TimeUnit.MILLISECONDS.toMicros(startTimestamp.toEpochMilli())); return this; } diff --git a/dd-java-agent/instrumentation/opentelemetry/src/main/java8/datadog/trace/instrumentation/opentelemetry/OtelTracerProvider.java b/dd-java-agent/instrumentation/opentelemetry/src/main/java8/datadog/trace/instrumentation/opentelemetry/OtelTracerProvider.java index c9bc473b619..5f579315c45 100644 --- a/dd-java-agent/instrumentation/opentelemetry/src/main/java8/datadog/trace/instrumentation/opentelemetry/OtelTracerProvider.java +++ b/dd-java-agent/instrumentation/opentelemetry/src/main/java8/datadog/trace/instrumentation/opentelemetry/OtelTracerProvider.java @@ -1,15 +1,19 @@ package datadog.trace.instrumentation.opentelemetry; +import datadog.trace.bootstrap.ContextStore; +import datadog.trace.bootstrap.instrumentation.api.AgentSpan; import datadog.trace.bootstrap.instrumentation.api.AgentTracer; -import io.opentelemetry.trace.Tracer; -import io.opentelemetry.trace.TracerProvider; +import io.opentelemetry.api.trace.SpanContext; +import io.opentelemetry.api.trace.Tracer; +import io.opentelemetry.api.trace.TracerProvider; -public class OtelTracerProvider implements TracerProvider { - public static final OtelTracerProvider INSTANCE = new OtelTracerProvider(); +public final class OtelTracerProvider implements TracerProvider { - private final TypeConverter converter = new TypeConverter(); + private final TypeConverter converter; - private OtelTracerProvider() {} + public OtelTracerProvider(ContextStore spanContextStore) { + converter = new TypeConverter(spanContextStore); + } @Override public Tracer get(final String instrumentationName) { diff --git a/dd-java-agent/instrumentation/opentelemetry/src/main/java8/datadog/trace/instrumentation/opentelemetry/TypeConverter.java b/dd-java-agent/instrumentation/opentelemetry/src/main/java8/datadog/trace/instrumentation/opentelemetry/TypeConverter.java index 89616b15ad2..7748cfaa566 100644 --- a/dd-java-agent/instrumentation/opentelemetry/src/main/java8/datadog/trace/instrumentation/opentelemetry/TypeConverter.java +++ b/dd-java-agent/instrumentation/opentelemetry/src/main/java8/datadog/trace/instrumentation/opentelemetry/TypeConverter.java @@ -1,48 +1,120 @@ package datadog.trace.instrumentation.opentelemetry; -import datadog.trace.bootstrap.instrumentation.api.AgentScope; +import datadog.trace.api.DDId; +import datadog.trace.bootstrap.ContextStore; import datadog.trace.bootstrap.instrumentation.api.AgentSpan; +import datadog.trace.bootstrap.instrumentation.api.AgentTrace; import datadog.trace.bootstrap.instrumentation.api.AgentTracer; -import io.opentelemetry.context.Scope; -import io.opentelemetry.trace.Span; -import io.opentelemetry.trace.SpanContext; +import io.opentelemetry.api.trace.Span; +import io.opentelemetry.api.trace.SpanContext; +import io.opentelemetry.api.trace.TraceFlags; +import io.opentelemetry.api.trace.TraceState; +import java.util.Map; // Centralized place to do conversions -public class TypeConverter { +public final class TypeConverter { + private final ContextStore spanContextStore; + + public TypeConverter(ContextStore spanContextStore) { + this.spanContextStore = spanContextStore; + } + // TODO maybe add caching to reduce new objects being created - public AgentSpan toAgentSpan(final Span span) { + // static to allow direct access from context advice. + public static AgentSpan toAgentSpan(final Span span) { + if (span == null) { + return null; + } if (span instanceof OtelSpan) { return ((OtelSpan) span).getDelegate(); } - return null == span ? null : AgentTracer.NoopAgentSpan.INSTANCE; + if (span.getSpanContext().isValid()) { + // TODO: PropagatedSpan which we don't have a good representation for. + throw new UnsupportedOperationException(); + } + + return AgentTracer.NoopAgentSpan.INSTANCE; } - public Span toSpan(final AgentSpan agentSpan) { + Span toSpan(final AgentSpan agentSpan) { if (agentSpan == null) { return null; } return new OtelSpan(agentSpan, this); } - public Scope toScope(final AgentScope scope) { - if (scope == null) { + SpanContext toSpanContext(final AgentSpan.Context context) { + if (context == null) { return null; } - return new OtelScope(scope); + SpanContext spanContext; + if (context.isRemote()) { + spanContext = + SpanContext.createFromRemoteParent( + context.getTraceId().toHexStringPadded(32), + context.getSpanId().toHexStringPadded(16), + TraceFlags.getSampled(), + TraceState.getDefault()); + } else { + spanContext = + SpanContext.create( + context.getTraceId().toHexStringPadded(32), + context.getSpanId().toHexStringPadded(16), + TraceFlags.getSampled(), + TraceState.getDefault()); + } + spanContextStore.put(spanContext, context); + return spanContext; } - public SpanContext toSpanContext(final AgentSpan.Context context) { + AgentSpan.Context toAgentSpanContext(final SpanContext spanContext) { + AgentSpan.Context context = spanContextStore.get(spanContext); if (context == null) { - return null; + if (spanContext.isValid()) { + context = new OtelSpanContext(spanContext); + } else { + context = AgentTracer.NoopContext.INSTANCE; + } + spanContextStore.put(spanContext, context); } - return new OtelSpanContext(context); + return context; } - public AgentSpan.Context toContext(final SpanContext spanContext) { - if (spanContext instanceof OtelSpanContext) { - return ((OtelSpanContext) spanContext).getDelegate(); + private static final class OtelSpanContext implements AgentSpan.Context { + private final DDId traceId; + private final DDId spanId; + private final boolean isRemote; + + public OtelSpanContext(SpanContext spanContext) { + traceId = DDId.fromHex(spanContext.getTraceIdAsHexString().substring(16)); + spanId = DDId.fromHex(spanContext.getSpanIdAsHexString()); + isRemote = spanContext.isRemote(); + } + + @Override + public DDId getTraceId() { + return traceId; + } + + @Override + public DDId getSpanId() { + return spanId; + } + + @Override + public boolean isRemote() { + return isRemote; + } + + @Override + public AgentTrace getTrace() { + return null; + } + + @Override + public Iterable> baggageItems() { + return null; } - return null == spanContext ? null : AgentTracer.NoopContext.INSTANCE; } } diff --git a/dd-java-agent/instrumentation/opentelemetry/src/main/java8/datadog/trace/instrumentation/opentelemetry/context/ContextStorageAdvice.java b/dd-java-agent/instrumentation/opentelemetry/src/main/java8/datadog/trace/instrumentation/opentelemetry/context/ContextStorageAdvice.java new file mode 100644 index 00000000000..5edad8f8f23 --- /dev/null +++ b/dd-java-agent/instrumentation/opentelemetry/src/main/java8/datadog/trace/instrumentation/opentelemetry/context/ContextStorageAdvice.java @@ -0,0 +1,29 @@ +package datadog.trace.instrumentation.opentelemetry.context; + +import static datadog.trace.bootstrap.instrumentation.api.AgentTracer.activateSpan; + +import datadog.trace.bootstrap.instrumentation.api.AgentScope; +import datadog.trace.instrumentation.opentelemetry.TypeConverter; +import io.opentelemetry.api.trace.Span; +import io.opentelemetry.api.trace.SpanBuilder; +import io.opentelemetry.context.Context; +import io.opentelemetry.context.Scope; +import net.bytebuddy.asm.Advice; + +public class ContextStorageAdvice { + @Advice.OnMethodExit(suppress = Throwable.class) + public static void updateScope( + @Advice.Argument(0) Context context, @Advice.Return(readOnly = false) Scope scope) { + // Get the span from the current context. + Span span = Span.fromContext(context); + // Activate the span so our context can be kept in sync. + AgentScope agentScope = activateSpan(TypeConverter.toAgentSpan(span)); + // Since the default returned Scope instance is an anonymous class, wrapping should be fine. + scope = new WrappedScope(scope, agentScope); + } + + // Keep in sync with OpenTelemetryInstrumentation + public static void muzzleCheck(final SpanBuilder builder) { + builder.startSpan(); + } +} diff --git a/dd-java-agent/instrumentation/opentelemetry/src/main/java8/datadog/trace/instrumentation/opentelemetry/context/WrappedScope.java b/dd-java-agent/instrumentation/opentelemetry/src/main/java8/datadog/trace/instrumentation/opentelemetry/context/WrappedScope.java new file mode 100644 index 00000000000..3577ced7cff --- /dev/null +++ b/dd-java-agent/instrumentation/opentelemetry/src/main/java8/datadog/trace/instrumentation/opentelemetry/context/WrappedScope.java @@ -0,0 +1,20 @@ +package datadog.trace.instrumentation.opentelemetry.context; + +import datadog.trace.bootstrap.instrumentation.api.AgentScope; +import io.opentelemetry.context.Scope; + +public class WrappedScope implements Scope { + private final Scope delegate; + private final AgentScope agentDelegate; + + public WrappedScope(Scope delegate, AgentScope agentDelegate) { + this.delegate = delegate; + this.agentDelegate = agentDelegate; + } + + @Override + public void close() { + agentDelegate.close(); + delegate.close(); + } +} diff --git a/dd-java-agent/instrumentation/opentelemetry/src/test/groovy/OpenTelemetryTest.groovy b/dd-java-agent/instrumentation/opentelemetry/src/test/groovy/OpenTelemetryTest.groovy index ad989a43ff1..dcef03a6a20 100644 --- a/dd-java-agent/instrumentation/opentelemetry/src/test/groovy/OpenTelemetryTest.groovy +++ b/dd-java-agent/instrumentation/opentelemetry/src/test/groovy/OpenTelemetryTest.groovy @@ -1,23 +1,30 @@ import datadog.trace.agent.test.AgentTestRunner import datadog.trace.api.DDId import datadog.trace.api.DDTags -import datadog.trace.api.interceptor.MutableSpan import datadog.trace.api.sampling.PrioritySampling -import datadog.trace.context.TraceScope import datadog.trace.core.propagation.ExtractedContext -import io.grpc.Context -import io.opentelemetry.OpenTelemetry +import datadog.trace.instrumentation.opentelemetry.OtelContextPropagators +import datadog.trace.instrumentation.opentelemetry.OtelTracer +import datadog.trace.instrumentation.opentelemetry.TypeConverter +import io.opentelemetry.api.OpenTelemetry +import io.opentelemetry.api.common.Attributes +import io.opentelemetry.api.trace.PropagatedSpan +import io.opentelemetry.api.trace.Span +import io.opentelemetry.api.trace.SpanContext +import io.opentelemetry.api.trace.TraceFlags +import io.opentelemetry.api.trace.TraceState +import io.opentelemetry.context.Context import io.opentelemetry.context.Scope -import io.opentelemetry.context.propagation.HttpTextFormat -import io.opentelemetry.trace.Span -import io.opentelemetry.trace.Status -import io.opentelemetry.trace.TracingContextUtils +import io.opentelemetry.context.propagation.TextMapPropagator +import spock.lang.Ignore import spock.lang.Subject +import static datadog.trace.bootstrap.instrumentation.api.AgentTracer.activeSpan + class OpenTelemetryTest extends AgentTestRunner { @Subject - def tracer = OpenTelemetry.tracerProvider.get("test-inst") - def httpPropagator = OpenTelemetry.getPropagators().httpTextFormat + def tracer = OpenTelemetry.get().getTracerProvider().get("test-inst") + def propagator = OpenTelemetry.getGlobalPropagators().textMapPropagator @Override void configurePreAgent() { @@ -26,6 +33,11 @@ class OpenTelemetryTest extends AgentTestRunner { injectSysConfig("dd.integration.opentelemetry-beta.enabled", "true") } + def setup() { + assert tracer instanceof OtelTracer + assert propagator instanceof OtelContextPropagators.OtelTextMapPropagator + } + def "test span tags"() { setup: def builder = tracer.spanBuilder("some name") @@ -44,9 +56,7 @@ class OpenTelemetryTest extends AgentTestRunner { } expect: - result instanceof MutableSpan - (result as MutableSpan).localRootSpan == result.delegate - tracer.currentSpan == null + Span.current() == Span.getInvalid() when: result.end() @@ -94,20 +104,16 @@ class OpenTelemetryTest extends AgentTestRunner { def "test span exception"() { setup: - def builder = tracer.spanBuilder("some name") - def result = builder.startSpan() - result.setStatus(Status.UNKNOWN) - result.setAttribute(DDTags.ERROR_MSG, (String) exception.message) - result.setAttribute(DDTags.ERROR_TYPE, (String) exception.class.name) - final StringWriter errorString = new StringWriter() - exception.printStackTrace(new PrintWriter(errorString)) - result.setAttribute(DDTags.ERROR_STACK, errorString.toString()) + def result = tracer.spanBuilder("some name").startSpan() + if (attributes) { + result.recordException(exception, attributes) + } else { + result.recordException(exception) + } expect: - result instanceof MutableSpan - (result as MutableSpan).localRootSpan == result.delegate - (result as MutableSpan).isError() == (exception != null) - tracer.currentSpan == null + result.delegate.isError() + !Span.current().spanContext.isValid() when: result.end() @@ -122,6 +128,9 @@ class OpenTelemetryTest extends AgentTestRunner { errored true tags { errorTags(exception.class) + if (attributes) { + "foo" "bar" + } defaultTags() } metrics { @@ -133,23 +142,29 @@ class OpenTelemetryTest extends AgentTestRunner { where: exception = new Exception() + attributes << [null, Attributes.builder().put("foo", "bar").build()] } def "test span links"() { setup: + TypeConverter converter = tracer.converter def builder = tracer.spanBuilder("some name") if (parentId) { - builder.setParent(tracer.converter.toSpanContext(new ExtractedContext(DDId.ONE, DDId.from(parentId), 0, null, [:], [:]))) + def spanContext = converter.toSpanContext(new ExtractedContext(DDId.ONE, DDId.from(parentId), 0, null, [:], [:])) + def parent = PropagatedSpan.create(spanContext) + assert parent.spanContext.remote + def ctx = parent.storeInContext(Context.root()) + builder.setParent(ctx) } if (linkId) { - builder.addLink(tracer.converter.toSpanContext(new ExtractedContext(DDId.ONE, DDId.from(linkId), 0, null, [:], [:]))) + def spanContext = converter.toSpanContext(new ExtractedContext(DDId.ONE, DDId.from(linkId), 0, null, [:], [:])) + builder.addLink(spanContext) } def result = builder.startSpan() expect: - result instanceof MutableSpan - (result as MutableSpan).localRootSpan == result.delegate - tracer.currentSpan == null + !result.spanContext.remote + !Span.current().spanContext.isValid() when: result.end() @@ -185,93 +200,119 @@ class OpenTelemetryTest extends AgentTestRunner { 2 | 3 | 2 } + def "test independent SpanContext"() { + setup: + def builder = tracer.spanBuilder("some name") + def spanContext = SpanContext.create("00000000000000000000000000000001", "0000000000000001", TraceFlags.default, TraceState.default) + builder.addLink(spanContext) + def result = builder.startSpan() + + expect: + spanContext.isValid() + !result.spanContext.remote + !Span.current().spanContext.isValid() + + when: + result.end() + + then: + assertTraces(1) { + trace(1) { + span { + traceDDId(DDId.ONE) + parentDDId(DDId.ONE) + operationName "test-inst" + resourceName "some name" + errored false + tags { + defaultTags(true) + } + metrics { + defaultMetrics() + } + } + } + } + } + def "test scope"() { setup: def span = tracer.spanBuilder("some name").startSpan() - def scope = tracer.withSpan(span) + def scope = span.storeInContext(Context.current()).makeCurrent() expect: - scope instanceof TraceScope - tracer.currentSpan.delegate == scope.delegate.span() + Span.current().delegate == activeSpan() + + when: + def child = tracer.spanBuilder("some name").startSpan() + + then: + child.delegate.traceId == span.delegate.traceId + child.delegate.parentId == span.delegate.spanId when: scope.close() then: - tracer.currentSpan == null + !Span.current().spanContext.isValid() cleanup: + child.end() span.end() } def "test closing scope when not on top"() { when: Span firstSpan = tracer.spanBuilder("someOperation").startSpan() - Scope firstScope = tracer.withSpan(firstSpan) + Scope firstScope = firstSpan.storeInContext(Context.current()).makeCurrent() Span secondSpan = tracer.spanBuilder("someOperation").startSpan() - Scope secondScope = tracer.withSpan(secondSpan) + Scope secondScope = secondSpan.storeInContext(Context.current()).makeCurrent() firstSpan.end() firstScope.close() + // OpenTelemetry will log a warning and continue closing the first scope resulting in an empty context. + then: - tracer.currentSpan.delegate == secondScope.delegate.span() + !Span.current().spanContext.valid + + // This results in our context being out of sync. + secondSpan.delegate == activeSpan() + 1 * STATS_D_CLIENT.incrementCounter("scope.close.error") - 1 * STATS_D_CLIENT.incrementCounter("scope.user.close.error") 0 * _ when: secondSpan.end() secondScope.close() + // Closing the scopes out of order results in the previous context being restored. + then: - tracer.currentSpan == null - 0 * _ + Span.current().delegate == firstSpan.delegate + // This results in our context being out of sync. + activeSpan() == null + 0 * _ } + @Ignore def "test continuation"() { - setup: - def span = tracer.spanBuilder("some name").startSpan() - TraceScope scope = tracer.withSpan(span) - scope.setAsyncPropagation(true) - - expect: - tracer.currentSpan.delegate == span.delegate - - when: - def continuation = scope.capture() - - then: - continuation instanceof TraceScope.Continuation - - when: - scope.close() - - then: - tracer.currentSpan == null - - when: - scope = continuation.activate() - - then: - tracer.currentSpan.delegate == span.delegate - - cleanup: - scope.close() - span.end() + // continuations are currently not supported with OTel. + throw new UnsupportedOperationException() } def "test inject extract"() { setup: def span = tracer.spanBuilder("some name").startSpan() - def context = TracingContextUtils.withSpan(span, Context.current()) - def textMap = [:] + def context = span.storeInContext(Context.current()) + Map textMap = [:] when: + Span.fromContext(context).spanContext.isValid() span.delegate.samplingPriority = contextPriority - httpPropagator.inject(context, textMap, new TextMapSetter()) + propagator.inject(context, textMap, new TextMapSetter()) then: textMap == [ @@ -281,13 +322,13 @@ class OpenTelemetryTest extends AgentTestRunner { ] when: - def extractedContext = httpPropagator.extract(context, textMap, new TextMapGetter()) - def extract = TracingContextUtils.getSpanWithoutDefault(extractedContext) + def extractedContext = propagator.extract(context, textMap, new TextMapGetter()) + def extract = Span.fromContext(extractedContext) then: - extract.context.traceId == span.context.traceId - extract.context.spanId == span.context.spanId - extract.context.delegate.samplingPriority == propagatedPriority + extract.spanContext.remote + extract.spanContext.traceIdAsHexString == span.spanContext.traceIdAsHexString + extract.spanContext.spanIdAsHexString == span.spanContext.spanIdAsHexString cleanup: span.end() @@ -301,14 +342,19 @@ class OpenTelemetryTest extends AgentTestRunner { PrioritySampling.USER_DROP | PrioritySampling.USER_DROP } - static class TextMapGetter implements HttpTextFormat.Getter> { + static class TextMapGetter implements TextMapPropagator.Getter> { + @Override + Iterable keys(Map carrier) { + return carrier.keySet() + } + @Override String get(Map carrier, String key) { return carrier.get(key) } } - static class TextMapSetter implements HttpTextFormat.Setter> { + static class TextMapSetter implements TextMapPropagator.Setter> { @Override void set(Map carrier, String key, String value) { carrier.put(key, value) diff --git a/dd-trace-core/src/main/java/datadog/trace/core/CoreTracer.java b/dd-trace-core/src/main/java/datadog/trace/core/CoreTracer.java index 0de1c74bcca..c890fa9a2ff 100644 --- a/dd-trace-core/src/main/java/datadog/trace/core/CoreTracer.java +++ b/dd-trace-core/src/main/java/datadog/trace/core/CoreTracer.java @@ -583,7 +583,7 @@ public class CoreSpanBuilder implements AgentTracer.SpanBuilder { // Builder attributes private Map tags; private long timestampMicro; - private Object parent; + private AgentSpan.Context parent; private String serviceName; private String resourceName; private boolean errorFlag; @@ -701,7 +701,7 @@ private DDSpanContext buildSpanContext() { // FIXME [API] parentContext should be an interface implemented by ExtractedContext, // TagContext, DDSpanContext, AgentSpan.Context - Object parentContext = parent; + AgentSpan.Context parentContext = parent; if (parentContext == null && !ignoreScope) { // use the Scope as parent unless overridden or ignored. final AgentSpan activeSpan = scopeManager.activeSpan(); @@ -735,6 +735,13 @@ private DDSpanContext buildSpanContext() { parentSpanId = extractedContext.getSpanId(); samplingPriority = extractedContext.getSamplingPriority(); baggage = extractedContext.getBaggage(); + } else if (parentContext != null) { + // Propagate generic external trace + DDId tid = parentContext.getTraceId(); + traceId = tid == DDId.ZERO ? IdGenerationStrategy.RANDOM.generate() : tid; + parentSpanId = parentContext.getSpanId(); + samplingPriority = PrioritySampling.UNSET; + baggage = null; } else { // Start a new trace traceId = IdGenerationStrategy.RANDOM.generate(); diff --git a/dd-trace-core/src/main/java/datadog/trace/core/DDSpanContext.java b/dd-trace-core/src/main/java/datadog/trace/core/DDSpanContext.java index 0e6e2e2cb56..07d6125adcf 100644 --- a/dd-trace-core/src/main/java/datadog/trace/core/DDSpanContext.java +++ b/dd-trace-core/src/main/java/datadog/trace/core/DDSpanContext.java @@ -162,6 +162,11 @@ public DDId getSpanId() { return spanId; } + @Override + public boolean isRemote() { + return false; + } + public String getServiceName() { return serviceName; } diff --git a/dd-trace-core/src/main/java/datadog/trace/core/propagation/TagContext.java b/dd-trace-core/src/main/java/datadog/trace/core/propagation/TagContext.java index 01d829a31fe..911f7f371cf 100644 --- a/dd-trace-core/src/main/java/datadog/trace/core/propagation/TagContext.java +++ b/dd-trace-core/src/main/java/datadog/trace/core/propagation/TagContext.java @@ -43,6 +43,11 @@ public DDId getSpanId() { return DDId.ZERO; } + @Override + public boolean isRemote() { + return true; + } + @Override public AgentTrace getTrace() { return AgentTracer.NoopAgentTrace.INSTANCE; diff --git a/internal-api/src/main/java/datadog/trace/bootstrap/instrumentation/api/AgentSpan.java b/internal-api/src/main/java/datadog/trace/bootstrap/instrumentation/api/AgentSpan.java index 279564a69a2..35aee740a4f 100644 --- a/internal-api/src/main/java/datadog/trace/bootstrap/instrumentation/api/AgentSpan.java +++ b/internal-api/src/main/java/datadog/trace/bootstrap/instrumentation/api/AgentSpan.java @@ -73,6 +73,8 @@ interface Context { DDId getSpanId(); + boolean isRemote(); + AgentTrace getTrace(); Iterable> baggageItems(); diff --git a/internal-api/src/main/java/datadog/trace/bootstrap/instrumentation/api/AgentTracer.java b/internal-api/src/main/java/datadog/trace/bootstrap/instrumentation/api/AgentTracer.java index 259b3282415..6302e08b7be 100644 --- a/internal-api/src/main/java/datadog/trace/bootstrap/instrumentation/api/AgentTracer.java +++ b/internal-api/src/main/java/datadog/trace/bootstrap/instrumentation/api/AgentTracer.java @@ -522,6 +522,11 @@ public DDId getSpanId() { return DDId.ZERO; } + @Override + public boolean isRemote() { + return false; + } + @Override public AgentTrace getTrace() { return NoopAgentTrace.INSTANCE;