From 8154b328cd463c45b21c3613b466bf308c15d388 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Christian=20Neum=C3=BCller?= Date: Tue, 1 Sep 2020 16:55:27 +0200 Subject: [PATCH 01/11] Prototype for Context as span parent. --- .../HttpTraceContextInjectBenchmark.java | 3 +- .../io/opentelemetry/trace/DefaultSpan.java | 27 ++++-- .../io/opentelemetry/trace/DefaultTracer.java | 31 +++---- .../java/io/opentelemetry/trace/Span.java | 53 ++--------- .../trace/propagation/HttpTraceContext.java | 2 +- .../trace/DefaultTracerTest.java | 23 +++-- .../opentelemetry/trace/SpanBuilderTest.java | 15 ++-- .../trace/TracingContextUtilsTest.java | 6 +- .../propagation/HttpTraceContextTest.java | 2 +- .../exporters/jaeger/AdapterTest.java | 2 +- .../exporters/otlp/SpanAdapterTest.java | 2 +- .../ZipkinSpanExporterEndToEndHttpTest.java | 14 ++- .../zipkin/ZipkinSpanExporterTest.java | 3 +- .../PropagatorContextInjectBenchmark.java | 4 +- .../trace/propagation/AwsXRayPropagator.java | 2 +- .../B3PropagatorExtractorMultipleHeaders.java | 3 +- .../B3PropagatorExtractorSingleHeader.java | 3 +- .../trace/propagation/JaegerPropagator.java | 2 +- .../trace/propagation/OtTracerPropagator.java | 2 +- .../propagation/AwsXRayPropagatorTest.java | 2 +- .../trace/propagation/B3PropagatorTest.java | 2 +- .../propagation/JaegerPropagatorTest.java | 2 +- .../propagation/OtTracerPropagatorTest.java | 2 +- .../propagation/TraceMultiPropagatorTest.java | 3 +- .../opentracingshim/Propagation.java | 4 +- .../opentracingshim/SpanBuilderShim.java | 11 ++- .../sdk/trace/RecordEventsReadableSpan.java | 36 ++++---- .../sdk/trace/SpanBuilderSdk.java | 89 +++++-------------- .../opentelemetry/sdk/trace/SpanWrapper.java | 6 ++ .../sdk/trace/data/SpanData.java | 10 +++ .../trace/RecordEventsReadableSpanTest.java | 28 +++++- .../sdk/trace/SpanBuilderSdkTest.java | 30 +++---- .../trace/testbed/actorpropagation/Actor.java | 5 +- .../trace/testbed/clientserver/Server.java | 5 +- .../HandlerTest.java | 7 +- .../RequestHandler.java | 5 +- .../testbed/multiplecallbacks/Client.java | 3 +- .../testbed/promisepropagation/Promise.java | 9 +- .../opentelemetry/sdk/trace/TestSpanData.java | 50 ++++++++--- .../sdk/trace/TestSpanDataTest.java | 1 - 40 files changed, 254 insertions(+), 255 deletions(-) diff --git a/api/src/jmh/java/io/opentelemetry/trace/propagation/HttpTraceContextInjectBenchmark.java b/api/src/jmh/java/io/opentelemetry/trace/propagation/HttpTraceContextInjectBenchmark.java index 7fd48c5bcf1..5b8d49f9e97 100644 --- a/api/src/jmh/java/io/opentelemetry/trace/propagation/HttpTraceContextInjectBenchmark.java +++ b/api/src/jmh/java/io/opentelemetry/trace/propagation/HttpTraceContextInjectBenchmark.java @@ -24,7 +24,6 @@ import io.opentelemetry.trace.TraceFlags; import io.opentelemetry.trace.TraceId; import io.opentelemetry.trace.TraceState; -import io.opentelemetry.trace.TracingContextUtils; import java.util.ArrayList; import java.util.Arrays; import java.util.HashMap; @@ -93,7 +92,7 @@ private static SpanContext createTestSpanContext(String traceId, String spanId) private static List createContexts(List spanContexts) { List contexts = new ArrayList<>(); for (SpanContext context : spanContexts) { - contexts.add(TracingContextUtils.withSpan(DefaultSpan.create(context), Context.ROOT)); + contexts.add(DefaultSpan.createInContext(context, Context.ROOT)); } return contexts; } diff --git a/api/src/main/java/io/opentelemetry/trace/DefaultSpan.java b/api/src/main/java/io/opentelemetry/trace/DefaultSpan.java index 23a3a24faa5..6dda7975418 100644 --- a/api/src/main/java/io/opentelemetry/trace/DefaultSpan.java +++ b/api/src/main/java/io/opentelemetry/trace/DefaultSpan.java @@ -16,6 +16,7 @@ package io.opentelemetry.trace; +import io.grpc.Context; import io.opentelemetry.common.AttributeValue; import io.opentelemetry.common.Attributes; import javax.annotation.concurrent.Immutable; @@ -31,8 +32,10 @@ @Immutable public final class DefaultSpan implements Span { - private static final DefaultSpan INVALID = new DefaultSpan(SpanContext.getInvalid()); + private static final DefaultSpan INVALID = + new DefaultSpan(SpanContext.getInvalid(), Context.ROOT); + // TODO: Do we need to change this to getInvalid(Context parent)? /** * Returns a {@link DefaultSpan} with an invalid {@link SpanContext}. * @@ -44,20 +47,27 @@ public static Span getInvalid() { } /** - * Creates an instance of this class with the {@link SpanContext}. + * Creates an instance of this class with a (parent) {@link Context}. * - * @param spanContext the {@code SpanContext}. + * @param spanContext the {@link SpanContext}. + * @param parent the (parent) {@link Context}. * @return a {@link DefaultSpan}. * @since 0.1.0 */ - public static Span create(SpanContext spanContext) { - return new DefaultSpan(spanContext); + public static Span create(SpanContext spanContext, Context parent) { + return new DefaultSpan(spanContext, parent); + } + + public static Context createInContext(SpanContext spanContext, Context parent) { + return TracingContextUtils.withSpan(create(spanContext, parent), parent); } private final SpanContext spanContext; + private final Context parent; - DefaultSpan(SpanContext spanContext) { + DefaultSpan(SpanContext spanContext, Context parent) { this.spanContext = spanContext; + this.parent = parent; } @Override @@ -116,6 +126,11 @@ public SpanContext getContext() { return spanContext; } + @Override + public Context getParent() { + return parent; + } + @Override public boolean isRecording() { return false; diff --git a/api/src/main/java/io/opentelemetry/trace/DefaultTracer.java b/api/src/main/java/io/opentelemetry/trace/DefaultTracer.java index 30ee48ad2c0..c50af37f4f5 100644 --- a/api/src/main/java/io/opentelemetry/trace/DefaultTracer.java +++ b/api/src/main/java/io/opentelemetry/trace/DefaultTracer.java @@ -67,43 +67,32 @@ static NoopSpanBuilder create(String spanName) { } private boolean isRootSpan; - @Nullable private SpanContext spanContext; + @Nullable private Context parent; @Override public Span startSpan() { - if (spanContext == null && !isRootSpan) { - spanContext = TracingContextUtils.getCurrentSpan().getContext(); + if (parent == null) { + parent = Context.current(); + } + if (isRootSpan) { + parent = TracingContextUtils.withSpan(DefaultSpan.getInvalid(), parent); } - return spanContext != null && !SpanContext.getInvalid().equals(spanContext) - ? new DefaultSpan(spanContext) - : DefaultSpan.getInvalid(); - } - - @Override - public NoopSpanBuilder setParent(Span parent) { - Utils.checkNotNull(parent, "parent"); - spanContext = parent.getContext(); - return this; - } - - @Override - public NoopSpanBuilder setParent(SpanContext remoteParent) { - Utils.checkNotNull(remoteParent, "remoteParent"); - spanContext = remoteParent; - return this; + return new DefaultSpan(TracingContextUtils.getSpan(parent).getContext(), parent); } @Override public NoopSpanBuilder setParent(Context context) { Utils.checkNotNull(context, "context"); - spanContext = TracingContextUtils.getSpan(context).getContext(); + isRootSpan = false; // TODO port this fix to mainline + parent = context; return this; } @Override public NoopSpanBuilder setNoParent() { isRootSpan = true; + parent = null; return this; } diff --git a/api/src/main/java/io/opentelemetry/trace/Span.java b/api/src/main/java/io/opentelemetry/trace/Span.java index ae775248119..419f79f3f16 100644 --- a/api/src/main/java/io/opentelemetry/trace/Span.java +++ b/api/src/main/java/io/opentelemetry/trace/Span.java @@ -283,6 +283,14 @@ enum Kind { */ SpanContext getContext(); + /** + * Returns the parent {@code Context} associated with this {@code Span}. + * + * @return the parent {@code Context} associated with this {@code Span}. + * @since 0.8.0 + */ + Context getParent(); + /** * Returns {@code true} if this {@code Span} records tracing events (e.g. {@link * #addEvent(String)}, {@link #setAttribute(String, long)}). @@ -383,51 +391,6 @@ enum Kind { */ interface Builder { - /** - * Sets the parent {@code Span} to use. If not set, the value of {@code Tracer.getCurrentSpan()} - * at {@link #startSpan()} time will be used as parent. - * - *

This must be used to create a {@code Span} when manual Context propagation is used - * OR when creating a root {@code Span} with a parent with an invalid {@link SpanContext}. - * - *

Observe this is the preferred method when the parent is a {@code Span} created within the - * process. Using its {@code SpanContext} as parent remains as a valid, albeit inefficient, - * operation. - * - *

If called multiple times, only the last specified value will be used. Observe that the - * state defined by a previous call to {@link #setNoParent()} will be discarded. - * - * @param parent the {@code Span} used as parent. - * @return this. - * @throws NullPointerException if {@code parent} is {@code null}. - * @see #setNoParent() - * @since 0.1.0 - */ - Builder setParent(Span parent); - - /** - * Sets the parent {@link SpanContext} to use. If not set, the value of {@code - * Tracer.getCurrentSpan()} at {@link #startSpan()} time will be used as parent. - * - *

Similar to {@link #setParent(Span parent)} but this must be used to create a {@code - * Span} when the parent is in a different process. This is only intended for use by RPC systems - * or similar. - * - *

If no {@link SpanContext} is available, users must call {@link #setNoParent()} in order to - * create a root {@code Span} for a new trace. - * - *

If called multiple times, only the last specified value will be used. Observe that the - * state defined by a previous call to {@link #setNoParent()} will be discarded. - * - * @param remoteParent the {@link SpanContext} used as parent. - * @return this. - * @throws NullPointerException if {@code remoteParent} is {@code null}. - * @see #setParent(Span parent) - * @see #setNoParent() - * @since 0.1.0 - */ - Builder setParent(SpanContext remoteParent); - /** * Sets the parent to use from the specified {@code Context}. If not set, the value of {@code * Tracer.getCurrentSpan()} at {@link #startSpan()} time will be used as parent. diff --git a/api/src/main/java/io/opentelemetry/trace/propagation/HttpTraceContext.java b/api/src/main/java/io/opentelemetry/trace/propagation/HttpTraceContext.java index f7d974493f7..34d761cdaf7 100644 --- a/api/src/main/java/io/opentelemetry/trace/propagation/HttpTraceContext.java +++ b/api/src/main/java/io/opentelemetry/trace/propagation/HttpTraceContext.java @@ -130,7 +130,7 @@ private static void injectImpl(SpanContext spanContext, C carrier, Setter return context; } - return TracingContextUtils.withSpan(DefaultSpan.create(spanContext), context); + return DefaultSpan.createInContext(spanContext, context); } private static SpanContext extractImpl(C carrier, Getter getter) { diff --git a/api/src/test/java/io/opentelemetry/trace/DefaultTracerTest.java b/api/src/test/java/io/opentelemetry/trace/DefaultTracerTest.java index ea262782264..a17b0488d2a 100644 --- a/api/src/test/java/io/opentelemetry/trace/DefaultTracerTest.java +++ b/api/src/test/java/io/opentelemetry/trace/DefaultTracerTest.java @@ -81,15 +81,23 @@ void testInProcessContext() { @Test void testSpanContextPropagationExplicitParent() { - Span span = defaultTracer.spanBuilder(SPAN_NAME).setParent(spanContext).startSpan(); + Span span = + defaultTracer + .spanBuilder(SPAN_NAME) + .setParent(DefaultSpan.createInContext(spanContext, Context.ROOT)) + .startSpan(); assertThat(span.getContext()).isSameAs(spanContext); } @Test void testSpanContextPropagation() { - DefaultSpan parent = new DefaultSpan(spanContext); + DefaultSpan parent = new DefaultSpan(spanContext, Context.ROOT); - Span span = defaultTracer.spanBuilder(SPAN_NAME).setParent(parent).startSpan(); + Span span = + defaultTracer + .spanBuilder(SPAN_NAME) + .setParent(TracingContextUtils.withSpan(parent, Context.ROOT)) + .startSpan(); assertThat(span.getContext()).isSameAs(spanContext); } @@ -108,7 +116,9 @@ void testSpanContextPropagation_nullContext() { @Test void testSpanContextPropagation_fromContext() { - Context context = TracingContextUtils.withSpan(new DefaultSpan(spanContext), Context.current()); + Context context = + TracingContextUtils.withSpan( + new DefaultSpan(spanContext, Context.current()), Context.current()); Span span = defaultTracer.spanBuilder(SPAN_NAME).setParent(context).startSpan(); assertThat(span.getContext()).isSameAs(spanContext); @@ -116,7 +126,7 @@ void testSpanContextPropagation_fromContext() { @Test void testSpanContextPropagationCurrentSpan() { - DefaultSpan parent = new DefaultSpan(spanContext); + DefaultSpan parent = new DefaultSpan(spanContext, Context.ROOT); try (Scope scope = defaultTracer.withSpan(parent)) { Span span = defaultTracer.spanBuilder(SPAN_NAME).startSpan(); assertThat(span.getContext()).isSameAs(spanContext); @@ -125,8 +135,7 @@ void testSpanContextPropagationCurrentSpan() { @Test void testSpanContextPropagationCurrentSpanContext() { - Context context = - TracingContextUtils.withSpan(DefaultSpan.create(spanContext), Context.current()); + Context context = DefaultSpan.createInContext(spanContext, Context.current()); try (Scope scope = ContextUtils.withScopedContext(context)) { Span span = defaultTracer.spanBuilder(SPAN_NAME).startSpan(); assertThat(span.getContext()).isSameAs(spanContext); diff --git a/api/src/test/java/io/opentelemetry/trace/SpanBuilderTest.java b/api/src/test/java/io/opentelemetry/trace/SpanBuilderTest.java index 4efa732bd6e..3a819ab7b22 100644 --- a/api/src/test/java/io/opentelemetry/trace/SpanBuilderTest.java +++ b/api/src/test/java/io/opentelemetry/trace/SpanBuilderTest.java @@ -19,6 +19,7 @@ import static org.assertj.core.api.Assertions.assertThat; import static org.junit.jupiter.api.Assertions.assertThrows; +import io.grpc.Context; import io.opentelemetry.common.AttributeValue; import io.opentelemetry.common.Attributes; import io.opentelemetry.trace.Span.Kind; @@ -32,8 +33,8 @@ class SpanBuilderTest { void doNotCrash_NoopImplementation() { Span.Builder spanBuilder = tracer.spanBuilder("MySpanName"); spanBuilder.setSpanKind(Kind.SERVER); - spanBuilder.setParent(DefaultSpan.getInvalid()); - spanBuilder.setParent(DefaultSpan.getInvalid().getContext()); + spanBuilder.setParent(DefaultSpan.createInContext(null, Context.ROOT)); + spanBuilder.setParent(Context.ROOT); spanBuilder.setNoParent(); spanBuilder.addLink(DefaultSpan.getInvalid().getContext()); spanBuilder.addLink(DefaultSpan.getInvalid().getContext(), Attributes.empty()); @@ -61,15 +62,9 @@ public Attributes getAttributes() { } @Test - void setParent_NullSpan() { + void setParent_EmptyContext() { Span.Builder spanBuilder = tracer.spanBuilder("MySpanName"); - assertThrows(NullPointerException.class, () -> spanBuilder.setParent((Span) null)); - } - - @Test - void setParent_NullSpanContext() { - Span.Builder spanBuilder = tracer.spanBuilder("MySpanName"); - assertThrows(NullPointerException.class, () -> spanBuilder.setParent((SpanContext) null)); + assertThrows(NullPointerException.class, () -> spanBuilder.setParent(null)); } @Test diff --git a/api/src/test/java/io/opentelemetry/trace/TracingContextUtilsTest.java b/api/src/test/java/io/opentelemetry/trace/TracingContextUtilsTest.java index 2fe45e9fd32..2ac40beb2ef 100644 --- a/api/src/test/java/io/opentelemetry/trace/TracingContextUtilsTest.java +++ b/api/src/test/java/io/opentelemetry/trace/TracingContextUtilsTest.java @@ -32,7 +32,7 @@ void testGetCurrentSpan_Default() { @Test void testGetCurrentSpan_SetSpan() { - Span span = DefaultSpan.create(SpanContext.getInvalid()); + Span span = DefaultSpan.create(SpanContext.getInvalid(), Context.current()); Context orig = TracingContextUtils.withSpan(span, Context.current()).attach(); try { assertThat(TracingContextUtils.getCurrentSpan()).isSameAs(span); @@ -49,7 +49,7 @@ void testGetSpan_DefaultContext() { @Test void testGetSpan_ExplicitContext() { - Span span = DefaultSpan.create(SpanContext.getInvalid()); + Span span = DefaultSpan.create(SpanContext.getInvalid(), Context.current()); Context context = TracingContextUtils.withSpan(span, Context.current()); assertThat(TracingContextUtils.getSpan(context)).isSameAs(span); } @@ -62,7 +62,7 @@ void testGetSpanWithoutDefault_DefaultContext() { @Test void testGetSpanWithoutDefault_ExplicitContext() { - Span span = DefaultSpan.create(SpanContext.getInvalid()); + Span span = DefaultSpan.create(SpanContext.getInvalid(), Context.current()); Context context = TracingContextUtils.withSpan(span, Context.current()); assertThat(TracingContextUtils.getSpanWithoutDefault(context)).isSameAs(span); } diff --git a/api/src/test/java/io/opentelemetry/trace/propagation/HttpTraceContextTest.java b/api/src/test/java/io/opentelemetry/trace/propagation/HttpTraceContextTest.java index ccecdae2ed1..56c97cbaed5 100644 --- a/api/src/test/java/io/opentelemetry/trace/propagation/HttpTraceContextTest.java +++ b/api/src/test/java/io/opentelemetry/trace/propagation/HttpTraceContextTest.java @@ -67,7 +67,7 @@ private static SpanContext getSpanContext(Context context) { } private static Context withSpanContext(SpanContext spanContext, Context context) { - return TracingContextUtils.withSpan(DefaultSpan.create(spanContext), context); + return DefaultSpan.createInContext(spanContext, context); } @Test diff --git a/exporters/jaeger/src/test/java/io/opentelemetry/exporters/jaeger/AdapterTest.java b/exporters/jaeger/src/test/java/io/opentelemetry/exporters/jaeger/AdapterTest.java index e1792e35413..3b792a7311f 100644 --- a/exporters/jaeger/src/test/java/io/opentelemetry/exporters/jaeger/AdapterTest.java +++ b/exporters/jaeger/src/test/java/io/opentelemetry/exporters/jaeger/AdapterTest.java @@ -305,7 +305,7 @@ private static SpanData getSpanData(long startMs, long endMs) { .setHasEnded(true) .setTraceId(TraceId.fromLowerBase16(TRACE_ID, 0)) .setSpanId(SpanId.fromLowerBase16(SPAN_ID, 0)) - .setParentSpanId(SpanId.fromLowerBase16(PARENT_SPAN_ID, 0)) + .setParent(SpanId.fromLowerBase16(PARENT_SPAN_ID, 0), false) .setName("GET /api/endpoint") .setStartEpochNanos(TimeUnit.MILLISECONDS.toNanos(startMs)) .setEndEpochNanos(TimeUnit.MILLISECONDS.toNanos(endMs)) diff --git a/exporters/otlp/src/test/java/io/opentelemetry/exporters/otlp/SpanAdapterTest.java b/exporters/otlp/src/test/java/io/opentelemetry/exporters/otlp/SpanAdapterTest.java index 11c77990c75..ba82d819f76 100644 --- a/exporters/otlp/src/test/java/io/opentelemetry/exporters/otlp/SpanAdapterTest.java +++ b/exporters/otlp/src/test/java/io/opentelemetry/exporters/otlp/SpanAdapterTest.java @@ -59,7 +59,7 @@ void toProtoSpan() { .setHasEnded(true) .setTraceId(TRACE_ID) .setSpanId(SPAN_ID) - .setParentSpanId(SpanId.getInvalid()) + .setParent(SpanId.getInvalid(), false) .setName("GET /api/endpoint") .setKind(Kind.SERVER) .setStartEpochNanos(12345) diff --git a/exporters/zipkin/src/test/java/io/opentelemetry/exporters/zipkin/ZipkinSpanExporterEndToEndHttpTest.java b/exporters/zipkin/src/test/java/io/opentelemetry/exporters/zipkin/ZipkinSpanExporterEndToEndHttpTest.java index f0a2f3f704d..af06ff7e05e 100644 --- a/exporters/zipkin/src/test/java/io/opentelemetry/exporters/zipkin/ZipkinSpanExporterEndToEndHttpTest.java +++ b/exporters/zipkin/src/test/java/io/opentelemetry/exporters/zipkin/ZipkinSpanExporterEndToEndHttpTest.java @@ -19,17 +19,21 @@ import static org.assertj.core.api.Assertions.assertThat; import com.google.common.collect.ImmutableList; +import io.grpc.Context; import io.opentelemetry.common.Attributes; import io.opentelemetry.sdk.common.CompletableResultCode; import io.opentelemetry.sdk.trace.TestSpanData; import io.opentelemetry.sdk.trace.data.EventImpl; import io.opentelemetry.sdk.trace.data.SpanData; import io.opentelemetry.sdk.trace.data.SpanData.Event; +import io.opentelemetry.trace.DefaultSpan; import io.opentelemetry.trace.Span.Kind; +import io.opentelemetry.trace.SpanContext; import io.opentelemetry.trace.SpanId; import io.opentelemetry.trace.Status; import io.opentelemetry.trace.TraceFlags; import io.opentelemetry.trace.TraceId; +import io.opentelemetry.trace.TraceState; import java.util.Collections; import java.util.List; import java.util.concurrent.TimeUnit; @@ -155,11 +159,17 @@ private static TestSpanData.Builder buildStandardSpan() { return TestSpanData.newBuilder() .setTraceId(TraceId.fromLowerBase16(TRACE_ID, 0)) .setSpanId(SpanId.fromLowerBase16(SPAN_ID, 0)) - .setParentSpanId(SpanId.fromLowerBase16(PARENT_SPAN_ID, 0)) + .setParent( + DefaultSpan.createInContext( + SpanContext.createFromRemoteParent( + TraceId.fromLowerBase16(TRACE_ID, 0), + SpanId.fromLowerBase16(PARENT_SPAN_ID, 0), + TraceFlags.getDefault(), + TraceState.getDefault()), + Context.current())) .setTraceFlags(TraceFlags.builder().setIsSampled(true).build()) .setStatus(Status.OK) .setKind(Kind.SERVER) - .setHasRemoteParent(true) .setName(SPAN_NAME) .setStartEpochNanos(START_EPOCH_NANOS) .setAttributes(attributes) diff --git a/exporters/zipkin/src/test/java/io/opentelemetry/exporters/zipkin/ZipkinSpanExporterTest.java b/exporters/zipkin/src/test/java/io/opentelemetry/exporters/zipkin/ZipkinSpanExporterTest.java index cef65deeaa5..ccf5fbc3a4a 100644 --- a/exporters/zipkin/src/test/java/io/opentelemetry/exporters/zipkin/ZipkinSpanExporterTest.java +++ b/exporters/zipkin/src/test/java/io/opentelemetry/exporters/zipkin/ZipkinSpanExporterTest.java @@ -313,11 +313,10 @@ private static TestSpanData.Builder buildStandardSpan() { return TestSpanData.newBuilder() .setTraceId(TraceId.fromLowerBase16(TRACE_ID, 0)) .setSpanId(SpanId.fromLowerBase16(SPAN_ID, 0)) - .setParentSpanId(SpanId.fromLowerBase16(PARENT_SPAN_ID, 0)) + .setParent(SpanId.fromLowerBase16(PARENT_SPAN_ID, 0), true) .setTraceFlags(TraceFlags.builder().setIsSampled(true).build()) .setStatus(Status.OK) .setKind(Kind.SERVER) - .setHasRemoteParent(true) .setName("Recv.helloworld.Greeter.SayHello") .setStartEpochNanos(1505855794_194009601L) .setAttributes(attributes) diff --git a/extensions/trace_propagators/src/jmh/java/io/opentelemetry/extensions/trace/propagation/PropagatorContextInjectBenchmark.java b/extensions/trace_propagators/src/jmh/java/io/opentelemetry/extensions/trace/propagation/PropagatorContextInjectBenchmark.java index 64e1a42fa6a..6cec88a3c62 100644 --- a/extensions/trace_propagators/src/jmh/java/io/opentelemetry/extensions/trace/propagation/PropagatorContextInjectBenchmark.java +++ b/extensions/trace_propagators/src/jmh/java/io/opentelemetry/extensions/trace/propagation/PropagatorContextInjectBenchmark.java @@ -24,7 +24,6 @@ import io.opentelemetry.trace.TraceFlags; import io.opentelemetry.trace.TraceId; import io.opentelemetry.trace.TraceState; -import io.opentelemetry.trace.TracingContextUtils; import java.util.Arrays; import java.util.HashMap; import java.util.List; @@ -75,8 +74,7 @@ public abstract static class AbstractContextInjectBenchmark { @BenchmarkMode(Mode.AverageTime) @Fork(1) public Map measureInject() { - Context context = - TracingContextUtils.withSpan(DefaultSpan.create(contextToTest), Context.current()); + Context context = DefaultSpan.createInContext(contextToTest, Context.current()); doInject(context, carrier); return carrier; } diff --git a/extensions/trace_propagators/src/main/java/io/opentelemetry/extensions/trace/propagation/AwsXRayPropagator.java b/extensions/trace_propagators/src/main/java/io/opentelemetry/extensions/trace/propagation/AwsXRayPropagator.java index 734b0039a63..5053f050044 100644 --- a/extensions/trace_propagators/src/main/java/io/opentelemetry/extensions/trace/propagation/AwsXRayPropagator.java +++ b/extensions/trace_propagators/src/main/java/io/opentelemetry/extensions/trace/propagation/AwsXRayPropagator.java @@ -129,7 +129,7 @@ public Context extract(Context context, C carrier, Getter getter) { return context; } - return TracingContextUtils.withSpan(DefaultSpan.create(spanContext), context); + return DefaultSpan.createInContext(spanContext, context); } private static SpanContext getSpanContextFromHeader(C carrier, Getter getter) { diff --git a/extensions/trace_propagators/src/main/java/io/opentelemetry/extensions/trace/propagation/B3PropagatorExtractorMultipleHeaders.java b/extensions/trace_propagators/src/main/java/io/opentelemetry/extensions/trace/propagation/B3PropagatorExtractorMultipleHeaders.java index 0755e556cc7..cdde0697ec2 100644 --- a/extensions/trace_propagators/src/main/java/io/opentelemetry/extensions/trace/propagation/B3PropagatorExtractorMultipleHeaders.java +++ b/extensions/trace_propagators/src/main/java/io/opentelemetry/extensions/trace/propagation/B3PropagatorExtractorMultipleHeaders.java @@ -24,7 +24,6 @@ import io.opentelemetry.context.propagation.TextMapPropagator; import io.opentelemetry.trace.DefaultSpan; import io.opentelemetry.trace.SpanContext; -import io.opentelemetry.trace.TracingContextUtils; import java.util.Objects; import java.util.logging.Logger; import javax.annotation.concurrent.Immutable; @@ -43,7 +42,7 @@ public Context extract(Context context, C carrier, TextMapPropagator.Getter< return context; } - return TracingContextUtils.withSpan(DefaultSpan.create(spanContext), context); + return DefaultSpan.createInContext(spanContext, context); } private static SpanContext getSpanContextFromMultipleHeaders( diff --git a/extensions/trace_propagators/src/main/java/io/opentelemetry/extensions/trace/propagation/B3PropagatorExtractorSingleHeader.java b/extensions/trace_propagators/src/main/java/io/opentelemetry/extensions/trace/propagation/B3PropagatorExtractorSingleHeader.java index d3f073e46aa..2a34f35f3ef 100644 --- a/extensions/trace_propagators/src/main/java/io/opentelemetry/extensions/trace/propagation/B3PropagatorExtractorSingleHeader.java +++ b/extensions/trace_propagators/src/main/java/io/opentelemetry/extensions/trace/propagation/B3PropagatorExtractorSingleHeader.java @@ -23,7 +23,6 @@ import io.opentelemetry.context.propagation.TextMapPropagator; import io.opentelemetry.trace.DefaultSpan; import io.opentelemetry.trace.SpanContext; -import io.opentelemetry.trace.TracingContextUtils; import java.util.Objects; import java.util.logging.Logger; import javax.annotation.concurrent.Immutable; @@ -42,7 +41,7 @@ public Context extract(Context context, C carrier, TextMapPropagator.Getter< return context; } - return TracingContextUtils.withSpan(DefaultSpan.create(spanContext), context); + return DefaultSpan.createInContext(spanContext, context); } @SuppressWarnings("StringSplitter") diff --git a/extensions/trace_propagators/src/main/java/io/opentelemetry/extensions/trace/propagation/JaegerPropagator.java b/extensions/trace_propagators/src/main/java/io/opentelemetry/extensions/trace/propagation/JaegerPropagator.java index c2965e1c360..d464433b428 100644 --- a/extensions/trace_propagators/src/main/java/io/opentelemetry/extensions/trace/propagation/JaegerPropagator.java +++ b/extensions/trace_propagators/src/main/java/io/opentelemetry/extensions/trace/propagation/JaegerPropagator.java @@ -114,7 +114,7 @@ public Context extract(Context context, C carrier, Getter getter) { return context; } - return TracingContextUtils.withSpan(DefaultSpan.create(spanContext), context); + return DefaultSpan.createInContext(spanContext, context); } @SuppressWarnings("StringSplitter") diff --git a/extensions/trace_propagators/src/main/java/io/opentelemetry/extensions/trace/propagation/OtTracerPropagator.java b/extensions/trace_propagators/src/main/java/io/opentelemetry/extensions/trace/propagation/OtTracerPropagator.java index 2231710ba6b..b9769f519eb 100644 --- a/extensions/trace_propagators/src/main/java/io/opentelemetry/extensions/trace/propagation/OtTracerPropagator.java +++ b/extensions/trace_propagators/src/main/java/io/opentelemetry/extensions/trace/propagation/OtTracerPropagator.java @@ -88,7 +88,7 @@ public Context extract(Context context, C carrier, Getter getter) { if (!spanContext.isValid()) { return context; } - return TracingContextUtils.withSpan(DefaultSpan.create(spanContext), context); + return DefaultSpan.createInContext(spanContext, context); } static SpanContext buildSpanContext(String traceId, String spanId, String sampled) { diff --git a/extensions/trace_propagators/src/test/java/io/opentelemetry/extensions/trace/propagation/AwsXRayPropagatorTest.java b/extensions/trace_propagators/src/test/java/io/opentelemetry/extensions/trace/propagation/AwsXRayPropagatorTest.java index a81369f20fa..4804eb0d8fd 100644 --- a/extensions/trace_propagators/src/test/java/io/opentelemetry/extensions/trace/propagation/AwsXRayPropagatorTest.java +++ b/extensions/trace_propagators/src/test/java/io/opentelemetry/extensions/trace/propagation/AwsXRayPropagatorTest.java @@ -258,7 +258,7 @@ void extract_InvalidFlags_NonNumeric() { } private static Context withSpanContext(SpanContext spanContext, Context context) { - return TracingContextUtils.withSpan(DefaultSpan.create(spanContext), context); + return DefaultSpan.createInContext(spanContext, context); } private static SpanContext getSpanContext(Context context) { diff --git a/extensions/trace_propagators/src/test/java/io/opentelemetry/extensions/trace/propagation/B3PropagatorTest.java b/extensions/trace_propagators/src/test/java/io/opentelemetry/extensions/trace/propagation/B3PropagatorTest.java index 94e9286d7b1..d75c3bc7684 100644 --- a/extensions/trace_propagators/src/test/java/io/opentelemetry/extensions/trace/propagation/B3PropagatorTest.java +++ b/extensions/trace_propagators/src/test/java/io/opentelemetry/extensions/trace/propagation/B3PropagatorTest.java @@ -68,7 +68,7 @@ private static SpanContext getSpanContext(Context context) { } private static Context withSpanContext(SpanContext spanContext, Context context) { - return TracingContextUtils.withSpan(DefaultSpan.create(spanContext), context); + return DefaultSpan.createInContext(spanContext, context); } @Test diff --git a/extensions/trace_propagators/src/test/java/io/opentelemetry/extensions/trace/propagation/JaegerPropagatorTest.java b/extensions/trace_propagators/src/test/java/io/opentelemetry/extensions/trace/propagation/JaegerPropagatorTest.java index 096bac486dc..d255bd5e396 100644 --- a/extensions/trace_propagators/src/test/java/io/opentelemetry/extensions/trace/propagation/JaegerPropagatorTest.java +++ b/extensions/trace_propagators/src/test/java/io/opentelemetry/extensions/trace/propagation/JaegerPropagatorTest.java @@ -76,7 +76,7 @@ private static SpanContext getSpanContext(Context context) { } private static Context withSpanContext(SpanContext spanContext, Context context) { - return TracingContextUtils.withSpan(DefaultSpan.create(spanContext), context); + return DefaultSpan.createInContext(spanContext, context); } @Test diff --git a/extensions/trace_propagators/src/test/java/io/opentelemetry/extensions/trace/propagation/OtTracerPropagatorTest.java b/extensions/trace_propagators/src/test/java/io/opentelemetry/extensions/trace/propagation/OtTracerPropagatorTest.java index 6344a51c928..f40430fb6b9 100644 --- a/extensions/trace_propagators/src/test/java/io/opentelemetry/extensions/trace/propagation/OtTracerPropagatorTest.java +++ b/extensions/trace_propagators/src/test/java/io/opentelemetry/extensions/trace/propagation/OtTracerPropagatorTest.java @@ -56,7 +56,7 @@ private static SpanContext getSpanContext(Context context) { } private static Context withSpanContext(SpanContext spanContext, Context context) { - return TracingContextUtils.withSpan(DefaultSpan.create(spanContext), context); + return DefaultSpan.createInContext(spanContext, context); } @Test diff --git a/extensions/trace_propagators/src/test/java/io/opentelemetry/extensions/trace/propagation/TraceMultiPropagatorTest.java b/extensions/trace_propagators/src/test/java/io/opentelemetry/extensions/trace/propagation/TraceMultiPropagatorTest.java index 71d8d0c9fd5..68e466beef2 100644 --- a/extensions/trace_propagators/src/test/java/io/opentelemetry/extensions/trace/propagation/TraceMultiPropagatorTest.java +++ b/extensions/trace_propagators/src/test/java/io/opentelemetry/extensions/trace/propagation/TraceMultiPropagatorTest.java @@ -53,7 +53,8 @@ class TraceMultiPropagatorTest { new TraceId(1245, 67890), new SpanId(12345), TraceFlags.getDefault(), - TraceState.getDefault())); + TraceState.getDefault()), + Context.ROOT); @BeforeEach void init() { diff --git a/opentracing_shim/src/main/java/io/opentelemetry/opentracingshim/Propagation.java b/opentracing_shim/src/main/java/io/opentelemetry/opentracingshim/Propagation.java index 8448a9a3329..2f404832baf 100644 --- a/opentracing_shim/src/main/java/io/opentelemetry/opentracingshim/Propagation.java +++ b/opentracing_shim/src/main/java/io/opentelemetry/opentracingshim/Propagation.java @@ -34,9 +34,7 @@ final class Propagation extends BaseShimObject { } public void injectTextMap(SpanContextShim contextShim, TextMapInject carrier) { - Context context = - TracingContextUtils.withSpan( - DefaultSpan.create(contextShim.getSpanContext()), Context.current()); + Context context = DefaultSpan.createInContext(contextShim.getSpanContext(), Context.current()); context = CorrelationsContextUtils.withCorrelationContext( contextShim.getCorrelationContext(), context); diff --git a/opentracing_shim/src/main/java/io/opentelemetry/opentracingshim/SpanBuilderShim.java b/opentracing_shim/src/main/java/io/opentelemetry/opentracingshim/SpanBuilderShim.java index 13344c7ced4..ca50be8cb3c 100644 --- a/opentracing_shim/src/main/java/io/opentelemetry/opentracingshim/SpanBuilderShim.java +++ b/opentracing_shim/src/main/java/io/opentelemetry/opentracingshim/SpanBuilderShim.java @@ -16,10 +16,13 @@ package io.opentelemetry.opentracingshim; +import io.grpc.Context; import io.opentelemetry.common.AttributeValue; import io.opentelemetry.correlationcontext.CorrelationContext; +import io.opentelemetry.trace.DefaultSpan; import io.opentelemetry.trace.Span.Kind; import io.opentelemetry.trace.Status; +import io.opentelemetry.trace.TracingContextUtils; import io.opentracing.Span; import io.opentracing.SpanContext; import io.opentracing.Tracer.SpanBuilder; @@ -183,11 +186,15 @@ public Span start() { if (ignoreActiveSpan && parentSpan == null && parentSpanContext == null) { builder.setNoParent(); } else if (parentSpan != null) { - builder.setParent(parentSpan.getSpan()); + // TODO: We ignore current() here + builder.setParent( + TracingContextUtils.withSpan(parentSpan.getSpan(), parentSpan.getSpan().getParent())); SpanContextShim contextShim = spanContextTable().get(parentSpan); distContext = contextShim == null ? null : contextShim.getCorrelationContext(); } else if (parentSpanContext != null) { - builder.setParent(parentSpanContext.getSpanContext()); + // TODO: This might be wonky + builder.setParent( + DefaultSpan.createInContext(parentSpanContext.getSpanContext(), Context.current())); distContext = parentSpanContext.getCorrelationContext(); } diff --git a/sdk/tracing/src/main/java/io/opentelemetry/sdk/trace/RecordEventsReadableSpan.java b/sdk/tracing/src/main/java/io/opentelemetry/sdk/trace/RecordEventsReadableSpan.java index 0a3ad5dd52c..75a66a1bf6d 100644 --- a/sdk/tracing/src/main/java/io/opentelemetry/sdk/trace/RecordEventsReadableSpan.java +++ b/sdk/tracing/src/main/java/io/opentelemetry/sdk/trace/RecordEventsReadableSpan.java @@ -17,6 +17,7 @@ package io.opentelemetry.sdk.trace; import com.google.common.collect.EvictingQueue; +import io.grpc.Context; import io.opentelemetry.common.AttributeValue; import io.opentelemetry.common.Attributes; import io.opentelemetry.common.ReadableAttributes; @@ -36,6 +37,7 @@ import io.opentelemetry.trace.SpanId; import io.opentelemetry.trace.Status; import io.opentelemetry.trace.Tracer; +import io.opentelemetry.trace.TracingContextUtils; import io.opentelemetry.trace.attributes.SemanticAttributes; import java.io.PrintWriter; import java.io.StringWriter; @@ -59,10 +61,8 @@ final class RecordEventsReadableSpan implements ReadWriteSpan { private final TraceConfig traceConfig; // Contains the identifiers associated with this Span. private final SpanContext context; - // The parent SpanId of this span. Invalid if this is a root span. - private final SpanId parentSpanId; - // True if the parent is on a different process. - private final boolean hasRemoteParent; + // Parent context. + private final Context parent; // Handler called when the span starts and ends. private final SpanProcessor spanProcessor; // The displayed name of the span. @@ -112,8 +112,7 @@ private RecordEventsReadableSpan( String name, InstrumentationLibraryInfo instrumentationLibraryInfo, Kind kind, - SpanId parentSpanId, - boolean hasRemoteParent, + Context parent, TraceConfig traceConfig, SpanProcessor spanProcessor, Clock clock, @@ -124,8 +123,7 @@ private RecordEventsReadableSpan( long startEpochNanos) { this.context = context; this.instrumentationLibraryInfo = instrumentationLibraryInfo; - this.parentSpanId = parentSpanId; - this.hasRemoteParent = hasRemoteParent; + this.parent = parent; this.links = links; this.totalRecordedLinks = totalRecordedLinks; this.name = name; @@ -146,10 +144,7 @@ private RecordEventsReadableSpan( * @param context supplies the trace_id and span_id for the newly started span. * @param name the displayed name for the new span. * @param kind the span kind. - * @param parentSpanId the span_id of the parent span, or {@code Span.INVALID} if the new span is - * a root span. - * @param hasRemoteParent {@code true} if the parentContext is remote. {@code false} if this is a - * root span. + * @param parent parent Context. * @param traceConfig trace parameters like sampler and probability. * @param spanProcessor handler called when the span starts and ends. * @param clock the clock used to get the time. @@ -163,8 +158,7 @@ static RecordEventsReadableSpan startSpan( String name, InstrumentationLibraryInfo instrumentationLibraryInfo, Kind kind, - SpanId parentSpanId, - boolean hasRemoteParent, + Context parent, TraceConfig traceConfig, SpanProcessor spanProcessor, Clock clock, @@ -179,8 +173,7 @@ static RecordEventsReadableSpan startSpan( name, instrumentationLibraryInfo, kind, - parentSpanId, - hasRemoteParent, + parent, traceConfig, spanProcessor, clock, @@ -225,6 +218,11 @@ public SpanContext getSpanContext() { return getContext(); } + @Override + public Context getParent() { + return parent; + } + /** * Returns the name of the {@code Span}. * @@ -508,7 +506,7 @@ private Status getStatusWithDefault() { } SpanId getParentSpanId() { - return parentSpanId; + return TracingContextUtils.getSpan(parent).getContext().getSpanId(); } Resource getResource() { @@ -524,7 +522,7 @@ long getStartEpochNanos() { } boolean hasRemoteParent() { - return hasRemoteParent; + return TracingContextUtils.getSpan(parent).getContext().isRemote(); } int getTotalRecordedLinks() { @@ -613,7 +611,7 @@ public String toString() { sb.append(", spanId="); sb.append(context.getSpanId()); sb.append(", parentSpanId="); - sb.append(parentSpanId); + sb.append(getParentSpanId()); sb.append(", name="); sb.append(name); sb.append(", kind="); diff --git a/sdk/tracing/src/main/java/io/opentelemetry/sdk/trace/SpanBuilderSdk.java b/sdk/tracing/src/main/java/io/opentelemetry/sdk/trace/SpanBuilderSdk.java index d58bb5dad9a..9f053de5fa8 100644 --- a/sdk/tracing/src/main/java/io/opentelemetry/sdk/trace/SpanBuilderSdk.java +++ b/sdk/tracing/src/main/java/io/opentelemetry/sdk/trace/SpanBuilderSdk.java @@ -60,14 +60,13 @@ final class SpanBuilderSdk implements Span.Builder { private final IdsGenerator idsGenerator; private final Clock clock; - @Nullable private Span parent; - @Nullable private SpanContext remoteParent; + @Nullable private Context parent; private Kind spanKind = Kind.INTERNAL; @Nullable private AttributesMap attributes; @Nullable private List links; private int totalNumberOfLinksAdded = 0; - private ParentType parentType = ParentType.CURRENT_CONTEXT; private long startEpochNanos = 0; + private boolean isRootSpan; SpanBuilderSdk( String spanName, @@ -86,34 +85,18 @@ final class SpanBuilderSdk implements Span.Builder { this.clock = clock; } - @Override - public Span.Builder setParent(Span parent) { - this.parent = Objects.requireNonNull(parent, "parent"); - this.remoteParent = null; - this.parentType = ParentType.EXPLICIT_PARENT; - return this; - } - - @Override - public Span.Builder setParent(SpanContext remoteParent) { - this.remoteParent = Objects.requireNonNull(remoteParent, "remoteParent"); - this.parent = null; - this.parentType = ParentType.EXPLICIT_REMOTE_PARENT; - return this; - } - @Override public Span.Builder setParent(Context context) { Objects.requireNonNull(context, "context"); - setParent(TracingContextUtils.getSpan(context)); + this.isRootSpan = false; + this.parent = context; return this; } @Override public Span.Builder setNoParent() { - this.parentType = ParentType.NO_PARENT; + this.isRootSpan = true; this.parent = null; - this.remoteParent = null; return this; } @@ -209,17 +192,23 @@ public Span.Builder setStartTimestamp(long startTimestamp) { @Override public Span startSpan() { - SpanContext parentContext = parent(parentType, parent, remoteParent); + final Context originalParent = parent == null ? Context.current() : parent; + final Context parentContext = + isRootSpan + ? DefaultSpan.createInContext(SpanContext.getInvalid(), originalParent) + : originalParent; + final Span parentSpan = TracingContextUtils.getSpan(parentContext); + final SpanContext parentSpanContext = parentSpan.getContext(); TraceId traceId; SpanId spanId = idsGenerator.generateSpanId(); TraceState traceState = TraceState.getDefault(); - if (!parentContext.isValid()) { + if (!parentSpanContext.isValid()) { // New root span. traceId = idsGenerator.generateTraceId(); } else { // New child span. - traceId = parentContext.getTraceId(); - traceState = parentContext.getTraceState(); + traceId = parentSpanContext.getTraceId(); + traceState = parentSpanContext.getTraceState(); } List immutableLinks = links == null @@ -233,7 +222,12 @@ public Span startSpan() { traceConfig .getSampler() .shouldSample( - parentContext, traceId, spanName, spanKind, immutableAttributes, immutableLinks); + parentSpanContext, + traceId, + spanName, + spanKind, + immutableAttributes, + immutableLinks); Sampler.Decision samplingDecision = samplingResult.getDecision(); SpanContext spanContext = @@ -246,7 +240,7 @@ public Span startSpan() { traceState); if (!Samplers.isRecording(samplingDecision)) { - return DefaultSpan.create(spanContext); + return DefaultSpan.create(spanContext, parentContext); } ReadableAttributes samplingAttributes = samplingResult.getAttributes(); if (!samplingAttributes.isEmpty()) { @@ -272,11 +266,10 @@ public void consume(String key, AttributeValue value) { spanName, instrumentationLibraryInfo, spanKind, - parentContext.getSpanId(), - parentContext.isRemote(), + parentContext, traceConfig, spanProcessor, - getClock(parentSpan(parentType, parent), clock), + getClock(parentSpan, clock), resource, recordedAttributes, immutableLinks, @@ -292,38 +285,4 @@ private static Clock getClock(Span parent, Clock clock) { return MonotonicClock.create(clock); } } - - private static SpanContext parent( - ParentType parentType, Span explicitParent, SpanContext remoteParent) { - switch (parentType) { - case NO_PARENT: - return SpanContext.getInvalid(); - case CURRENT_CONTEXT: - return TracingContextUtils.getCurrentSpan().getContext(); - case EXPLICIT_PARENT: - return explicitParent.getContext(); - case EXPLICIT_REMOTE_PARENT: - return remoteParent; - } - throw new IllegalStateException("Unknown parent type"); - } - - @Nullable - private static Span parentSpan(ParentType parentType, Span explicitParent) { - switch (parentType) { - case CURRENT_CONTEXT: - return TracingContextUtils.getSpanWithoutDefault(Context.current()); - case EXPLICIT_PARENT: - return explicitParent; - default: - return null; - } - } - - private enum ParentType { - CURRENT_CONTEXT, - EXPLICIT_PARENT, - EXPLICIT_REMOTE_PARENT, - NO_PARENT, - } } diff --git a/sdk/tracing/src/main/java/io/opentelemetry/sdk/trace/SpanWrapper.java b/sdk/tracing/src/main/java/io/opentelemetry/sdk/trace/SpanWrapper.java index 37cb816c233..1f23639b4b2 100644 --- a/sdk/tracing/src/main/java/io/opentelemetry/sdk/trace/SpanWrapper.java +++ b/sdk/tracing/src/main/java/io/opentelemetry/sdk/trace/SpanWrapper.java @@ -17,6 +17,7 @@ package io.opentelemetry.sdk.trace; import com.google.auto.value.AutoValue; +import io.grpc.Context; import io.opentelemetry.common.ReadableAttributes; import io.opentelemetry.sdk.common.InstrumentationLibraryInfo; import io.opentelemetry.sdk.resources.Resource; @@ -116,6 +117,11 @@ public SpanId getParentSpanId() { return delegate().getParentSpanId(); } + @Override + public Context getParent() { + return delegate().getParent(); + } + @Override public Resource getResource() { return delegate().getResource(); diff --git a/sdk/tracing/src/main/java/io/opentelemetry/sdk/trace/data/SpanData.java b/sdk/tracing/src/main/java/io/opentelemetry/sdk/trace/data/SpanData.java index e8982a20495..d110d61b755 100644 --- a/sdk/tracing/src/main/java/io/opentelemetry/sdk/trace/data/SpanData.java +++ b/sdk/tracing/src/main/java/io/opentelemetry/sdk/trace/data/SpanData.java @@ -17,6 +17,7 @@ package io.opentelemetry.sdk.trace.data; import com.google.auto.value.AutoValue; +import io.grpc.Context; import io.opentelemetry.common.Attributes; import io.opentelemetry.common.ReadableAttributes; import io.opentelemetry.sdk.common.InstrumentationLibraryInfo; @@ -77,6 +78,15 @@ public interface SpanData { */ SpanId getParentSpanId(); + /** + * Returns the parent {@code Context}. If the {@code Span} is a root {@code Span}, the Context + * will contain no Span or an invalid span. + * + * @return the parent {@code SpanId} or an invalid SpanId if this is a root {@code Span}. + * @since 0.8.0 + */ + Context getParent(); + /** * Returns the resource of this {@code Span}. * diff --git a/sdk/tracing/src/test/java/io/opentelemetry/sdk/trace/RecordEventsReadableSpanTest.java b/sdk/tracing/src/test/java/io/opentelemetry/sdk/trace/RecordEventsReadableSpanTest.java index 724c9e61c73..e9fa00f0de4 100644 --- a/sdk/tracing/src/test/java/io/opentelemetry/sdk/trace/RecordEventsReadableSpanTest.java +++ b/sdk/tracing/src/test/java/io/opentelemetry/sdk/trace/RecordEventsReadableSpanTest.java @@ -20,6 +20,7 @@ import static org.assertj.core.api.Assertions.assertThat; import static org.junit.jupiter.api.Assertions.assertThrows; +import io.grpc.Context; import io.opentelemetry.common.AttributeValue; import io.opentelemetry.common.Attributes; import io.opentelemetry.common.ReadableAttributes; @@ -31,6 +32,7 @@ import io.opentelemetry.sdk.trace.data.SpanData; import io.opentelemetry.sdk.trace.data.SpanData.Event; import io.opentelemetry.sdk.trace.data.SpanData.Link; +import io.opentelemetry.trace.DefaultSpan; import io.opentelemetry.trace.Span.Kind; import io.opentelemetry.trace.SpanContext; import io.opentelemetry.trace.SpanId; @@ -824,8 +826,15 @@ private RecordEventsReadableSpan createTestSpan( SPAN_NAME, instrumentationLibraryInfo, kind, - parentSpanId, - /* hasRemoteParent= */ true, + parentSpanId == null + ? Context.ROOT + : DefaultSpan.createInContext( + SpanContext.createFromRemoteParent( + spanContext.getTraceId(), + parentSpanId, + spanContext.getTraceFlags(), + spanContext.getTraceState()), + Context.current()), config, spanProcessor, testClock, @@ -904,14 +913,25 @@ void testAsSpanData() { SpanContext.create(traceId, spanId, TraceFlags.getDefault(), TraceState.getDefault()); Link link1 = Link.create(context, TestUtils.generateRandomAttributes()); + final SpanContext parentSpanContext = + EXPECTED_HAS_REMOTE_PARENT + ? SpanContext.createFromRemoteParent( + context.getTraceId(), + parentSpanId, + context.getTraceFlags(), + context.getTraceState()) + : SpanContext.create( + context.getTraceId(), + parentSpanId, + context.getTraceFlags(), + context.getTraceState()); RecordEventsReadableSpan readableSpan = RecordEventsReadableSpan.startSpan( context, name, instrumentationLibraryInfo, kind, - parentSpanId, - /* hasRemoteParent= */ EXPECTED_HAS_REMOTE_PARENT, + DefaultSpan.createInContext(parentSpanContext, Context.current()), traceConfig, spanProcessor, clock, diff --git a/sdk/tracing/src/test/java/io/opentelemetry/sdk/trace/SpanBuilderSdkTest.java b/sdk/tracing/src/test/java/io/opentelemetry/sdk/trace/SpanBuilderSdkTest.java index dc3e5029ab9..95225a36e03 100644 --- a/sdk/tracing/src/test/java/io/opentelemetry/sdk/trace/SpanBuilderSdkTest.java +++ b/sdk/tracing/src/test/java/io/opentelemetry/sdk/trace/SpanBuilderSdkTest.java @@ -64,14 +64,7 @@ void setSpanKind_null() { @Test void setParent_null() { assertThrows( - NullPointerException.class, () -> tracerSdk.spanBuilder(SPAN_NAME).setParent((Span) null)); - } - - @Test - void setRemoteParent_null() { - assertThrows( - NullPointerException.class, - () -> tracerSdk.spanBuilder(SPAN_NAME).setParent((SpanContext) null)); + NullPointerException.class, () -> tracerSdk.spanBuilder(SPAN_NAME).setParent(null)); } @Test @@ -617,7 +610,7 @@ void noParent() { tracerSdk .spanBuilder(SPAN_NAME) .setNoParent() - .setParent(parent) + .setParent(Context.current()) .setNoParent() .startSpan(); try { @@ -639,7 +632,11 @@ void noParent_override() { try { RecordEventsReadableSpan span = (RecordEventsReadableSpan) - tracerSdk.spanBuilder(SPAN_NAME).setNoParent().setParent(parent).startSpan(); + tracerSdk + .spanBuilder(SPAN_NAME) + .setNoParent() + .setParent(TracingContextUtils.withSpan(parent, Context.current())) + .startSpan(); try { assertThat(span.getContext().getTraceId()).isEqualTo(parent.getContext().getTraceId()); assertThat(span.toSpanData().getParentSpanId()).isEqualTo(parent.getContext().getSpanId()); @@ -649,7 +646,7 @@ void noParent_override() { tracerSdk .spanBuilder(SPAN_NAME) .setNoParent() - .setParent(parent.getContext()) + .setParent(TracingContextUtils.withSpan(parent, Context.current())) .startSpan(); try { assertThat(span2.getContext().getTraceId()).isEqualTo(parent.getContext().getTraceId()); @@ -674,7 +671,7 @@ void overrideNoParent_remoteParent() { tracerSdk .spanBuilder(SPAN_NAME) .setNoParent() - .setParent(parent.getContext()) + .setParent(TracingContextUtils.withSpan(parent, Context.current())) .startSpan(); try { assertThat(span.getContext().getTraceId()).isEqualTo(parent.getContext().getTraceId()); @@ -753,7 +750,10 @@ void parent_invalidContext() { RecordEventsReadableSpan span = (RecordEventsReadableSpan) - tracerSdk.spanBuilder(SPAN_NAME).setParent(parent.getContext()).startSpan(); + tracerSdk + .spanBuilder(SPAN_NAME) + .setParent(TracingContextUtils.withSpan(parent, Context.current())) + .startSpan(); try { assertThat(span.getContext().getTraceId()).isNotEqualTo(parent.getContext().getTraceId()); assertFalse(span.toSpanData().getParentSpanId().isValid()); @@ -773,9 +773,9 @@ void startTimestamp_null() { @Test void parent_clockIsSame() { Span parent = tracerSdk.spanBuilder(SPAN_NAME).startSpan(); - try { + try (Scope scope = tracerSdk.withSpan(parent)) { RecordEventsReadableSpan span = - (RecordEventsReadableSpan) tracerSdk.spanBuilder(SPAN_NAME).setParent(parent).startSpan(); + (RecordEventsReadableSpan) tracerSdk.spanBuilder(SPAN_NAME).startSpan(); assertThat(span.getClock()).isSameAs(((RecordEventsReadableSpan) parent).getClock()); } finally { diff --git a/sdk_extensions/testbed/src/test/java/io/opentelemetry/sdk/extensions/trace/testbed/actorpropagation/Actor.java b/sdk_extensions/testbed/src/test/java/io/opentelemetry/sdk/extensions/trace/testbed/actorpropagation/Actor.java index a0dd0f37376..c08205039d7 100644 --- a/sdk_extensions/testbed/src/test/java/io/opentelemetry/sdk/extensions/trace/testbed/actorpropagation/Actor.java +++ b/sdk_extensions/testbed/src/test/java/io/opentelemetry/sdk/extensions/trace/testbed/actorpropagation/Actor.java @@ -16,6 +16,7 @@ package io.opentelemetry.sdk.extensions.trace.testbed.actorpropagation; +import io.grpc.Context; import io.opentelemetry.context.Scope; import io.opentelemetry.trace.Span; import io.opentelemetry.trace.Span.Kind; @@ -44,7 +45,7 @@ public void close() { } Future tell(final String message) { - final Span parent = tracer.getCurrentSpan(); + final Context parent = Context.current(); phaser.register(); return executor.submit( () -> { @@ -68,7 +69,7 @@ Future tell(final String message) { } Future ask(final String message) { - final Span parent = tracer.getCurrentSpan(); + final Context parent = Context.current(); phaser.register(); return executor.submit( () -> { diff --git a/sdk_extensions/testbed/src/test/java/io/opentelemetry/sdk/extensions/trace/testbed/clientserver/Server.java b/sdk_extensions/testbed/src/test/java/io/opentelemetry/sdk/extensions/trace/testbed/clientserver/Server.java index 0ed2c20fa95..953ee7b257d 100644 --- a/sdk_extensions/testbed/src/test/java/io/opentelemetry/sdk/extensions/trace/testbed/clientserver/Server.java +++ b/sdk_extensions/testbed/src/test/java/io/opentelemetry/sdk/extensions/trace/testbed/clientserver/Server.java @@ -22,9 +22,7 @@ import io.opentelemetry.context.propagation.TextMapPropagator.Getter; import io.opentelemetry.trace.Span; import io.opentelemetry.trace.Span.Kind; -import io.opentelemetry.trace.SpanContext; import io.opentelemetry.trace.Tracer; -import io.opentelemetry.trace.TracingContextUtils; import java.util.concurrent.ArrayBlockingQueue; import javax.annotation.Nullable; @@ -52,9 +50,8 @@ public String get(Message carrier, String key) { return carrier.get(key); } }); - SpanContext spanContext = TracingContextUtils.getSpan(context).getContext(); Span span = - tracer.spanBuilder("receive").setSpanKind(Kind.SERVER).setParent(spanContext).startSpan(); + tracer.spanBuilder("receive").setSpanKind(Kind.SERVER).setParent(context).startSpan(); span.setAttribute("component", "example-server"); try (Scope ignored = tracer.withSpan(span)) { diff --git a/sdk_extensions/testbed/src/test/java/io/opentelemetry/sdk/extensions/trace/testbed/concurrentcommonrequesthandler/HandlerTest.java b/sdk_extensions/testbed/src/test/java/io/opentelemetry/sdk/extensions/trace/testbed/concurrentcommonrequesthandler/HandlerTest.java index dcf06715888..81edb9bf4fa 100644 --- a/sdk_extensions/testbed/src/test/java/io/opentelemetry/sdk/extensions/trace/testbed/concurrentcommonrequesthandler/HandlerTest.java +++ b/sdk_extensions/testbed/src/test/java/io/opentelemetry/sdk/extensions/trace/testbed/concurrentcommonrequesthandler/HandlerTest.java @@ -18,6 +18,7 @@ import static org.assertj.core.api.Assertions.assertThat; +import io.grpc.Context; import io.opentelemetry.context.Scope; import io.opentelemetry.exporters.inmemory.InMemoryTracing; import io.opentelemetry.sdk.extensions.trace.testbed.TestUtils; @@ -27,6 +28,7 @@ import io.opentelemetry.trace.Span; import io.opentelemetry.trace.SpanId; import io.opentelemetry.trace.Tracer; +import io.opentelemetry.trace.TracingContextUtils; import java.util.List; import java.util.concurrent.Future; import java.util.concurrent.TimeUnit; @@ -108,7 +110,10 @@ void bad_solution_to_set_parent() throws Exception { Client client; Span parentSpan = tracer.spanBuilder("parent").startSpan(); try (Scope ignored = tracer.withSpan(parentSpan)) { - client = new Client(new RequestHandler(tracer, parentSpan.getContext())); + client = + new Client( + new RequestHandler( + tracer, TracingContextUtils.withSpan(parentSpan, Context.current()))); String response = client.send("correct_parent").get(15, TimeUnit.SECONDS); assertThat(response).isEqualTo("correct_parent:response"); } finally { diff --git a/sdk_extensions/testbed/src/test/java/io/opentelemetry/sdk/extensions/trace/testbed/concurrentcommonrequesthandler/RequestHandler.java b/sdk_extensions/testbed/src/test/java/io/opentelemetry/sdk/extensions/trace/testbed/concurrentcommonrequesthandler/RequestHandler.java index 27055f15d41..cfda79ed1ad 100644 --- a/sdk_extensions/testbed/src/test/java/io/opentelemetry/sdk/extensions/trace/testbed/concurrentcommonrequesthandler/RequestHandler.java +++ b/sdk_extensions/testbed/src/test/java/io/opentelemetry/sdk/extensions/trace/testbed/concurrentcommonrequesthandler/RequestHandler.java @@ -18,7 +18,6 @@ import io.opentelemetry.trace.Span; import io.opentelemetry.trace.Span.Kind; -import io.opentelemetry.trace.SpanContext; import io.opentelemetry.trace.Tracer; /** @@ -30,13 +29,13 @@ final class RequestHandler { private final Tracer tracer; - private final SpanContext parentContext; + private final io.grpc.Context parentContext; public RequestHandler(Tracer tracer) { this(tracer, null); } - public RequestHandler(Tracer tracer, SpanContext parentContext) { + public RequestHandler(Tracer tracer, io.grpc.Context parentContext) { this.tracer = tracer; this.parentContext = parentContext; } diff --git a/sdk_extensions/testbed/src/test/java/io/opentelemetry/sdk/extensions/trace/testbed/multiplecallbacks/Client.java b/sdk_extensions/testbed/src/test/java/io/opentelemetry/sdk/extensions/trace/testbed/multiplecallbacks/Client.java index 71dd4328469..3ab1217921b 100644 --- a/sdk_extensions/testbed/src/test/java/io/opentelemetry/sdk/extensions/trace/testbed/multiplecallbacks/Client.java +++ b/sdk_extensions/testbed/src/test/java/io/opentelemetry/sdk/extensions/trace/testbed/multiplecallbacks/Client.java @@ -16,6 +16,7 @@ package io.opentelemetry.sdk.extensions.trace.testbed.multiplecallbacks; +import io.grpc.Context; import io.opentelemetry.context.Scope; import io.opentelemetry.trace.Span; import io.opentelemetry.trace.Tracer; @@ -35,7 +36,7 @@ public Client(Tracer tracer, CountDownLatch parentDoneLatch) { } public Future send(final Object message) { - final Span parent = tracer.getCurrentSpan(); + final Context parent = Context.current(); return executor.submit( () -> { diff --git a/sdk_extensions/testbed/src/test/java/io/opentelemetry/sdk/extensions/trace/testbed/promisepropagation/Promise.java b/sdk_extensions/testbed/src/test/java/io/opentelemetry/sdk/extensions/trace/testbed/promisepropagation/Promise.java index cccdfdb36d1..81c54f3b52d 100644 --- a/sdk_extensions/testbed/src/test/java/io/opentelemetry/sdk/extensions/trace/testbed/promisepropagation/Promise.java +++ b/sdk_extensions/testbed/src/test/java/io/opentelemetry/sdk/extensions/trace/testbed/promisepropagation/Promise.java @@ -16,6 +16,7 @@ package io.opentelemetry.sdk.extensions.trace.testbed.promisepropagation; +import io.grpc.Context; import io.opentelemetry.context.Scope; import io.opentelemetry.trace.Span; import io.opentelemetry.trace.Tracer; @@ -25,7 +26,7 @@ final class Promise { private final PromiseContext context; private final Tracer tracer; - private final Span parentSpan; + private final Context parent; private final Collection> successCallbacks = new ArrayList<>(); private final Collection errorCallbacks = new ArrayList<>(); @@ -35,7 +36,7 @@ final class Promise { // Passed along here for testing. Normally should be referenced via GlobalTracer.get(). this.tracer = tracer; - parentSpan = tracer.getCurrentSpan(); + parent = Context.current(); } void onSuccess(SuccessCallback successCallback) { @@ -51,7 +52,7 @@ void success(final T result) { for (final SuccessCallback callback : successCallbacks) { context.submit( () -> { - Span childSpan = tracer.spanBuilder("success").setParent(parentSpan).startSpan(); + Span childSpan = tracer.spanBuilder("success").setParent(parent).startSpan(); childSpan.setAttribute("component", "success"); try (Scope ignored = tracer.withSpan(childSpan)) { callback.accept(result); @@ -68,7 +69,7 @@ void error(final Throwable error) { for (final ErrorCallback callback : errorCallbacks) { context.submit( () -> { - Span childSpan = tracer.spanBuilder("error").setParent(parentSpan).startSpan(); + Span childSpan = tracer.spanBuilder("error").setParent(parent).startSpan(); childSpan.setAttribute("component", "error"); try (Scope ignored = tracer.withSpan(childSpan)) { callback.accept(error); diff --git a/testing_internal/src/main/java/io/opentelemetry/sdk/trace/TestSpanData.java b/testing_internal/src/main/java/io/opentelemetry/sdk/trace/TestSpanData.java index 5a60d5f8794..e666ec8d9f6 100644 --- a/testing_internal/src/main/java/io/opentelemetry/sdk/trace/TestSpanData.java +++ b/testing_internal/src/main/java/io/opentelemetry/sdk/trace/TestSpanData.java @@ -17,18 +17,22 @@ package io.opentelemetry.sdk.trace; import com.google.auto.value.AutoValue; +import io.grpc.Context; import io.opentelemetry.common.AttributeValue; import io.opentelemetry.common.Attributes; import io.opentelemetry.common.ReadableAttributes; import io.opentelemetry.sdk.common.InstrumentationLibraryInfo; import io.opentelemetry.sdk.resources.Resource; import io.opentelemetry.sdk.trace.data.SpanData; +import io.opentelemetry.trace.DefaultSpan; import io.opentelemetry.trace.Span.Kind; +import io.opentelemetry.trace.SpanContext; import io.opentelemetry.trace.SpanId; import io.opentelemetry.trace.Status; import io.opentelemetry.trace.TraceFlags; import io.opentelemetry.trace.TraceId; import io.opentelemetry.trace.TraceState; +import io.opentelemetry.trace.TracingContextUtils; import java.util.ArrayList; import java.util.Collections; import java.util.List; @@ -51,7 +55,7 @@ public abstract class TestSpanData implements SpanData { */ public static Builder newBuilder() { return new AutoValue_TestSpanData.Builder() - .setParentSpanId(SpanId.getInvalid()) + .setParent(Context.ROOT) .setInstrumentationLibraryInfo(InstrumentationLibraryInfo.getEmpty()) .setLinks(Collections.emptyList()) .setTotalRecordedLinks(0) @@ -61,10 +65,19 @@ public static Builder newBuilder() { .setResource(Resource.getEmpty()) .setTraceState(TraceState.getDefault()) .setTraceFlags(TraceFlags.getDefault()) - .setHasRemoteParent(false) .setTotalAttributeCount(0); } + @Override + public boolean getHasRemoteParent() { + return TracingContextUtils.getSpan(getParent()).getContext().isRemote(); + } + + @Override + public SpanId getParentSpanId() { + return TracingContextUtils.getSpan(getParent()).getContext().getSpanId(); + } + /** * A {@code Builder} class for {@link TestSpanData}. * @@ -79,6 +92,8 @@ public abstract static class Builder { abstract List getLinks(); + abstract TraceId getTraceId(); + /** * Create a new SpanData instance from the data in this. * @@ -125,13 +140,29 @@ public TestSpanData build() { public abstract Builder setTraceState(TraceState traceState); /** - * The parent span id associated for this span, which may be null. + * The parent Context associated for this span, which may be empty. * - * @param parentSpanId the SpanId of the parent + * @param parent the parent Context of the parent * @return this. * @since 0.1.0 */ - public abstract Builder setParentSpanId(SpanId parentSpanId); + public abstract Builder setParent(Context parent); + + /** + * Utility function to set a parent context based on the current one. + * + * @see #setParent(Context) + */ + public Builder setParent(SpanId parentSpanId, boolean hasRemoteParent) { + return setParent( + DefaultSpan.createInContext( + hasRemoteParent + ? SpanContext.createFromRemoteParent( + getTraceId(), parentSpanId, TraceFlags.getDefault(), TraceState.getDefault()) + : SpanContext.create( + getTraceId(), parentSpanId, TraceFlags.getDefault(), TraceState.getDefault()), + Context.current())); + } /** * Set the {@link Resource} associated with this span. Must not be null. @@ -229,15 +260,6 @@ public abstract Builder setInstrumentationLibraryInfo( */ public abstract Builder setLinks(List links); - /** - * Sets to true if the span has a parent on a different process. - * - * @param hasRemoteParent A boolean indicating if the span has a remote parent. - * @return this - * @since 0.3.0 - */ - public abstract Builder setHasRemoteParent(boolean hasRemoteParent); - /** * Sets to true if the span has been ended. * diff --git a/testing_internal/src/test/java/io/opentelemetry/sdk/trace/TestSpanDataTest.java b/testing_internal/src/test/java/io/opentelemetry/sdk/trace/TestSpanDataTest.java index fa20ae472cf..555b1ed9566 100644 --- a/testing_internal/src/test/java/io/opentelemetry/sdk/trace/TestSpanDataTest.java +++ b/testing_internal/src/test/java/io/opentelemetry/sdk/trace/TestSpanDataTest.java @@ -125,7 +125,6 @@ private static TestSpanData.Builder createBasicSpanBuilder() { .setEndEpochNanos(END_EPOCH_NANOS) .setKind(Kind.SERVER) .setStatus(Status.OK) - .setHasRemoteParent(false) .setTotalRecordedEvents(0) .setTotalRecordedLinks(0); } From a7fccf470dd250441820c43ae00bca840a1fdd48 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Christian=20Neum=C3=BCller?= Date: Tue, 1 Sep 2020 18:52:36 +0200 Subject: [PATCH 02/11] Simplify ZipkinSpanExporterEndToEndHttpTest. --- .../zipkin/ZipkinSpanExporterEndToEndHttpTest.java | 13 +------------ 1 file changed, 1 insertion(+), 12 deletions(-) diff --git a/exporters/zipkin/src/test/java/io/opentelemetry/exporters/zipkin/ZipkinSpanExporterEndToEndHttpTest.java b/exporters/zipkin/src/test/java/io/opentelemetry/exporters/zipkin/ZipkinSpanExporterEndToEndHttpTest.java index af06ff7e05e..de13d04ad2f 100644 --- a/exporters/zipkin/src/test/java/io/opentelemetry/exporters/zipkin/ZipkinSpanExporterEndToEndHttpTest.java +++ b/exporters/zipkin/src/test/java/io/opentelemetry/exporters/zipkin/ZipkinSpanExporterEndToEndHttpTest.java @@ -19,21 +19,17 @@ import static org.assertj.core.api.Assertions.assertThat; import com.google.common.collect.ImmutableList; -import io.grpc.Context; import io.opentelemetry.common.Attributes; import io.opentelemetry.sdk.common.CompletableResultCode; import io.opentelemetry.sdk.trace.TestSpanData; import io.opentelemetry.sdk.trace.data.EventImpl; import io.opentelemetry.sdk.trace.data.SpanData; import io.opentelemetry.sdk.trace.data.SpanData.Event; -import io.opentelemetry.trace.DefaultSpan; import io.opentelemetry.trace.Span.Kind; -import io.opentelemetry.trace.SpanContext; import io.opentelemetry.trace.SpanId; import io.opentelemetry.trace.Status; import io.opentelemetry.trace.TraceFlags; import io.opentelemetry.trace.TraceId; -import io.opentelemetry.trace.TraceState; import java.util.Collections; import java.util.List; import java.util.concurrent.TimeUnit; @@ -159,14 +155,7 @@ private static TestSpanData.Builder buildStandardSpan() { return TestSpanData.newBuilder() .setTraceId(TraceId.fromLowerBase16(TRACE_ID, 0)) .setSpanId(SpanId.fromLowerBase16(SPAN_ID, 0)) - .setParent( - DefaultSpan.createInContext( - SpanContext.createFromRemoteParent( - TraceId.fromLowerBase16(TRACE_ID, 0), - SpanId.fromLowerBase16(PARENT_SPAN_ID, 0), - TraceFlags.getDefault(), - TraceState.getDefault()), - Context.current())) + .setParent(SpanId.fromLowerBase16(PARENT_SPAN_ID, 0), false) .setTraceFlags(TraceFlags.builder().setIsSampled(true).build()) .setStatus(Status.OK) .setKind(Kind.SERVER) From e102deff195ea8f8088f59e28de9edbdadbfa079 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Christian=20Neum=C3=BCller?= Date: Tue, 1 Sep 2020 18:55:52 +0200 Subject: [PATCH 03/11] Minimize PR diff. --- .../HttpTraceContextInjectBenchmark.java | 3 ++- .../io/opentelemetry/trace/DefaultSpan.java | 27 +++++-------------- .../io/opentelemetry/trace/DefaultTracer.java | 2 +- .../java/io/opentelemetry/trace/Span.java | 8 ------ .../trace/propagation/HttpTraceContext.java | 2 +- .../trace/DefaultTracerTest.java | 13 +++++---- .../opentelemetry/trace/SpanBuilderTest.java | 4 +-- .../trace/TracingContextUtilsTest.java | 6 ++--- .../propagation/HttpTraceContextTest.java | 2 +- .../PropagatorContextInjectBenchmark.java | 4 ++- .../trace/propagation/AwsXRayPropagator.java | 2 +- .../B3PropagatorExtractorMultipleHeaders.java | 3 ++- .../B3PropagatorExtractorSingleHeader.java | 3 ++- .../trace/propagation/JaegerPropagator.java | 2 +- .../trace/propagation/OtTracerPropagator.java | 2 +- .../propagation/AwsXRayPropagatorTest.java | 2 +- .../trace/propagation/B3PropagatorTest.java | 2 +- .../propagation/JaegerPropagatorTest.java | 2 +- .../propagation/OtTracerPropagatorTest.java | 2 +- .../propagation/TraceMultiPropagatorTest.java | 3 +-- .../opentracingshim/Propagation.java | 4 ++- .../opentracingshim/SpanBuilderShim.java | 8 +++--- .../sdk/trace/RecordEventsReadableSpan.java | 9 +++---- .../sdk/trace/SpanBuilderSdk.java | 4 +-- .../trace/RecordEventsReadableSpanTest.java | 16 ++++++----- .../opentelemetry/sdk/trace/TestSpanData.java | 19 ++++++++----- 26 files changed, 72 insertions(+), 82 deletions(-) diff --git a/api/src/jmh/java/io/opentelemetry/trace/propagation/HttpTraceContextInjectBenchmark.java b/api/src/jmh/java/io/opentelemetry/trace/propagation/HttpTraceContextInjectBenchmark.java index 5b8d49f9e97..7fd48c5bcf1 100644 --- a/api/src/jmh/java/io/opentelemetry/trace/propagation/HttpTraceContextInjectBenchmark.java +++ b/api/src/jmh/java/io/opentelemetry/trace/propagation/HttpTraceContextInjectBenchmark.java @@ -24,6 +24,7 @@ import io.opentelemetry.trace.TraceFlags; import io.opentelemetry.trace.TraceId; import io.opentelemetry.trace.TraceState; +import io.opentelemetry.trace.TracingContextUtils; import java.util.ArrayList; import java.util.Arrays; import java.util.HashMap; @@ -92,7 +93,7 @@ private static SpanContext createTestSpanContext(String traceId, String spanId) private static List createContexts(List spanContexts) { List contexts = new ArrayList<>(); for (SpanContext context : spanContexts) { - contexts.add(DefaultSpan.createInContext(context, Context.ROOT)); + contexts.add(TracingContextUtils.withSpan(DefaultSpan.create(context), Context.ROOT)); } return contexts; } diff --git a/api/src/main/java/io/opentelemetry/trace/DefaultSpan.java b/api/src/main/java/io/opentelemetry/trace/DefaultSpan.java index 6dda7975418..23a3a24faa5 100644 --- a/api/src/main/java/io/opentelemetry/trace/DefaultSpan.java +++ b/api/src/main/java/io/opentelemetry/trace/DefaultSpan.java @@ -16,7 +16,6 @@ package io.opentelemetry.trace; -import io.grpc.Context; import io.opentelemetry.common.AttributeValue; import io.opentelemetry.common.Attributes; import javax.annotation.concurrent.Immutable; @@ -32,10 +31,8 @@ @Immutable public final class DefaultSpan implements Span { - private static final DefaultSpan INVALID = - new DefaultSpan(SpanContext.getInvalid(), Context.ROOT); + private static final DefaultSpan INVALID = new DefaultSpan(SpanContext.getInvalid()); - // TODO: Do we need to change this to getInvalid(Context parent)? /** * Returns a {@link DefaultSpan} with an invalid {@link SpanContext}. * @@ -47,27 +44,20 @@ public static Span getInvalid() { } /** - * Creates an instance of this class with a (parent) {@link Context}. + * Creates an instance of this class with the {@link SpanContext}. * - * @param spanContext the {@link SpanContext}. - * @param parent the (parent) {@link Context}. + * @param spanContext the {@code SpanContext}. * @return a {@link DefaultSpan}. * @since 0.1.0 */ - public static Span create(SpanContext spanContext, Context parent) { - return new DefaultSpan(spanContext, parent); - } - - public static Context createInContext(SpanContext spanContext, Context parent) { - return TracingContextUtils.withSpan(create(spanContext, parent), parent); + public static Span create(SpanContext spanContext) { + return new DefaultSpan(spanContext); } private final SpanContext spanContext; - private final Context parent; - DefaultSpan(SpanContext spanContext, Context parent) { + DefaultSpan(SpanContext spanContext) { this.spanContext = spanContext; - this.parent = parent; } @Override @@ -126,11 +116,6 @@ public SpanContext getContext() { return spanContext; } - @Override - public Context getParent() { - return parent; - } - @Override public boolean isRecording() { return false; diff --git a/api/src/main/java/io/opentelemetry/trace/DefaultTracer.java b/api/src/main/java/io/opentelemetry/trace/DefaultTracer.java index c50af37f4f5..91661552529 100644 --- a/api/src/main/java/io/opentelemetry/trace/DefaultTracer.java +++ b/api/src/main/java/io/opentelemetry/trace/DefaultTracer.java @@ -78,7 +78,7 @@ public Span startSpan() { parent = TracingContextUtils.withSpan(DefaultSpan.getInvalid(), parent); } - return new DefaultSpan(TracingContextUtils.getSpan(parent).getContext(), parent); + return new DefaultSpan(TracingContextUtils.getSpan(parent).getContext()); } @Override diff --git a/api/src/main/java/io/opentelemetry/trace/Span.java b/api/src/main/java/io/opentelemetry/trace/Span.java index 419f79f3f16..43d79b4827a 100644 --- a/api/src/main/java/io/opentelemetry/trace/Span.java +++ b/api/src/main/java/io/opentelemetry/trace/Span.java @@ -283,14 +283,6 @@ enum Kind { */ SpanContext getContext(); - /** - * Returns the parent {@code Context} associated with this {@code Span}. - * - * @return the parent {@code Context} associated with this {@code Span}. - * @since 0.8.0 - */ - Context getParent(); - /** * Returns {@code true} if this {@code Span} records tracing events (e.g. {@link * #addEvent(String)}, {@link #setAttribute(String, long)}). diff --git a/api/src/main/java/io/opentelemetry/trace/propagation/HttpTraceContext.java b/api/src/main/java/io/opentelemetry/trace/propagation/HttpTraceContext.java index 34d761cdaf7..f7d974493f7 100644 --- a/api/src/main/java/io/opentelemetry/trace/propagation/HttpTraceContext.java +++ b/api/src/main/java/io/opentelemetry/trace/propagation/HttpTraceContext.java @@ -130,7 +130,7 @@ private static void injectImpl(SpanContext spanContext, C carrier, Setter return context; } - return DefaultSpan.createInContext(spanContext, context); + return TracingContextUtils.withSpan(DefaultSpan.create(spanContext), context); } private static SpanContext extractImpl(C carrier, Getter getter) { diff --git a/api/src/test/java/io/opentelemetry/trace/DefaultTracerTest.java b/api/src/test/java/io/opentelemetry/trace/DefaultTracerTest.java index a17b0488d2a..db575d8f349 100644 --- a/api/src/test/java/io/opentelemetry/trace/DefaultTracerTest.java +++ b/api/src/test/java/io/opentelemetry/trace/DefaultTracerTest.java @@ -84,14 +84,14 @@ void testSpanContextPropagationExplicitParent() { Span span = defaultTracer .spanBuilder(SPAN_NAME) - .setParent(DefaultSpan.createInContext(spanContext, Context.ROOT)) + .setParent(TracingContextUtils.withSpan(DefaultSpan.create(spanContext), Context.ROOT)) .startSpan(); assertThat(span.getContext()).isSameAs(spanContext); } @Test void testSpanContextPropagation() { - DefaultSpan parent = new DefaultSpan(spanContext, Context.ROOT); + DefaultSpan parent = new DefaultSpan(spanContext); Span span = defaultTracer @@ -116,9 +116,7 @@ void testSpanContextPropagation_nullContext() { @Test void testSpanContextPropagation_fromContext() { - Context context = - TracingContextUtils.withSpan( - new DefaultSpan(spanContext, Context.current()), Context.current()); + Context context = TracingContextUtils.withSpan(new DefaultSpan(spanContext), Context.current()); Span span = defaultTracer.spanBuilder(SPAN_NAME).setParent(context).startSpan(); assertThat(span.getContext()).isSameAs(spanContext); @@ -126,7 +124,7 @@ void testSpanContextPropagation_fromContext() { @Test void testSpanContextPropagationCurrentSpan() { - DefaultSpan parent = new DefaultSpan(spanContext, Context.ROOT); + DefaultSpan parent = new DefaultSpan(spanContext); try (Scope scope = defaultTracer.withSpan(parent)) { Span span = defaultTracer.spanBuilder(SPAN_NAME).startSpan(); assertThat(span.getContext()).isSameAs(spanContext); @@ -135,7 +133,8 @@ void testSpanContextPropagationCurrentSpan() { @Test void testSpanContextPropagationCurrentSpanContext() { - Context context = DefaultSpan.createInContext(spanContext, Context.current()); + Context context = + TracingContextUtils.withSpan(DefaultSpan.create(spanContext), Context.current()); try (Scope scope = ContextUtils.withScopedContext(context)) { Span span = defaultTracer.spanBuilder(SPAN_NAME).startSpan(); assertThat(span.getContext()).isSameAs(spanContext); diff --git a/api/src/test/java/io/opentelemetry/trace/SpanBuilderTest.java b/api/src/test/java/io/opentelemetry/trace/SpanBuilderTest.java index 3a819ab7b22..4ec0643cd95 100644 --- a/api/src/test/java/io/opentelemetry/trace/SpanBuilderTest.java +++ b/api/src/test/java/io/opentelemetry/trace/SpanBuilderTest.java @@ -33,7 +33,7 @@ class SpanBuilderTest { void doNotCrash_NoopImplementation() { Span.Builder spanBuilder = tracer.spanBuilder("MySpanName"); spanBuilder.setSpanKind(Kind.SERVER); - spanBuilder.setParent(DefaultSpan.createInContext(null, Context.ROOT)); + spanBuilder.setParent(TracingContextUtils.withSpan(DefaultSpan.create(null), Context.ROOT)); spanBuilder.setParent(Context.ROOT); spanBuilder.setNoParent(); spanBuilder.addLink(DefaultSpan.getInvalid().getContext()); @@ -62,7 +62,7 @@ public Attributes getAttributes() { } @Test - void setParent_EmptyContext() { + void setParent_NullContext() { Span.Builder spanBuilder = tracer.spanBuilder("MySpanName"); assertThrows(NullPointerException.class, () -> spanBuilder.setParent(null)); } diff --git a/api/src/test/java/io/opentelemetry/trace/TracingContextUtilsTest.java b/api/src/test/java/io/opentelemetry/trace/TracingContextUtilsTest.java index 2ac40beb2ef..2fe45e9fd32 100644 --- a/api/src/test/java/io/opentelemetry/trace/TracingContextUtilsTest.java +++ b/api/src/test/java/io/opentelemetry/trace/TracingContextUtilsTest.java @@ -32,7 +32,7 @@ void testGetCurrentSpan_Default() { @Test void testGetCurrentSpan_SetSpan() { - Span span = DefaultSpan.create(SpanContext.getInvalid(), Context.current()); + Span span = DefaultSpan.create(SpanContext.getInvalid()); Context orig = TracingContextUtils.withSpan(span, Context.current()).attach(); try { assertThat(TracingContextUtils.getCurrentSpan()).isSameAs(span); @@ -49,7 +49,7 @@ void testGetSpan_DefaultContext() { @Test void testGetSpan_ExplicitContext() { - Span span = DefaultSpan.create(SpanContext.getInvalid(), Context.current()); + Span span = DefaultSpan.create(SpanContext.getInvalid()); Context context = TracingContextUtils.withSpan(span, Context.current()); assertThat(TracingContextUtils.getSpan(context)).isSameAs(span); } @@ -62,7 +62,7 @@ void testGetSpanWithoutDefault_DefaultContext() { @Test void testGetSpanWithoutDefault_ExplicitContext() { - Span span = DefaultSpan.create(SpanContext.getInvalid(), Context.current()); + Span span = DefaultSpan.create(SpanContext.getInvalid()); Context context = TracingContextUtils.withSpan(span, Context.current()); assertThat(TracingContextUtils.getSpanWithoutDefault(context)).isSameAs(span); } diff --git a/api/src/test/java/io/opentelemetry/trace/propagation/HttpTraceContextTest.java b/api/src/test/java/io/opentelemetry/trace/propagation/HttpTraceContextTest.java index 56c97cbaed5..ccecdae2ed1 100644 --- a/api/src/test/java/io/opentelemetry/trace/propagation/HttpTraceContextTest.java +++ b/api/src/test/java/io/opentelemetry/trace/propagation/HttpTraceContextTest.java @@ -67,7 +67,7 @@ private static SpanContext getSpanContext(Context context) { } private static Context withSpanContext(SpanContext spanContext, Context context) { - return DefaultSpan.createInContext(spanContext, context); + return TracingContextUtils.withSpan(DefaultSpan.create(spanContext), context); } @Test diff --git a/extensions/trace_propagators/src/jmh/java/io/opentelemetry/extensions/trace/propagation/PropagatorContextInjectBenchmark.java b/extensions/trace_propagators/src/jmh/java/io/opentelemetry/extensions/trace/propagation/PropagatorContextInjectBenchmark.java index 6cec88a3c62..64e1a42fa6a 100644 --- a/extensions/trace_propagators/src/jmh/java/io/opentelemetry/extensions/trace/propagation/PropagatorContextInjectBenchmark.java +++ b/extensions/trace_propagators/src/jmh/java/io/opentelemetry/extensions/trace/propagation/PropagatorContextInjectBenchmark.java @@ -24,6 +24,7 @@ import io.opentelemetry.trace.TraceFlags; import io.opentelemetry.trace.TraceId; import io.opentelemetry.trace.TraceState; +import io.opentelemetry.trace.TracingContextUtils; import java.util.Arrays; import java.util.HashMap; import java.util.List; @@ -74,7 +75,8 @@ public abstract static class AbstractContextInjectBenchmark { @BenchmarkMode(Mode.AverageTime) @Fork(1) public Map measureInject() { - Context context = DefaultSpan.createInContext(contextToTest, Context.current()); + Context context = + TracingContextUtils.withSpan(DefaultSpan.create(contextToTest), Context.current()); doInject(context, carrier); return carrier; } diff --git a/extensions/trace_propagators/src/main/java/io/opentelemetry/extensions/trace/propagation/AwsXRayPropagator.java b/extensions/trace_propagators/src/main/java/io/opentelemetry/extensions/trace/propagation/AwsXRayPropagator.java index 5053f050044..734b0039a63 100644 --- a/extensions/trace_propagators/src/main/java/io/opentelemetry/extensions/trace/propagation/AwsXRayPropagator.java +++ b/extensions/trace_propagators/src/main/java/io/opentelemetry/extensions/trace/propagation/AwsXRayPropagator.java @@ -129,7 +129,7 @@ public Context extract(Context context, C carrier, Getter getter) { return context; } - return DefaultSpan.createInContext(spanContext, context); + return TracingContextUtils.withSpan(DefaultSpan.create(spanContext), context); } private static SpanContext getSpanContextFromHeader(C carrier, Getter getter) { diff --git a/extensions/trace_propagators/src/main/java/io/opentelemetry/extensions/trace/propagation/B3PropagatorExtractorMultipleHeaders.java b/extensions/trace_propagators/src/main/java/io/opentelemetry/extensions/trace/propagation/B3PropagatorExtractorMultipleHeaders.java index cdde0697ec2..0755e556cc7 100644 --- a/extensions/trace_propagators/src/main/java/io/opentelemetry/extensions/trace/propagation/B3PropagatorExtractorMultipleHeaders.java +++ b/extensions/trace_propagators/src/main/java/io/opentelemetry/extensions/trace/propagation/B3PropagatorExtractorMultipleHeaders.java @@ -24,6 +24,7 @@ import io.opentelemetry.context.propagation.TextMapPropagator; import io.opentelemetry.trace.DefaultSpan; import io.opentelemetry.trace.SpanContext; +import io.opentelemetry.trace.TracingContextUtils; import java.util.Objects; import java.util.logging.Logger; import javax.annotation.concurrent.Immutable; @@ -42,7 +43,7 @@ public Context extract(Context context, C carrier, TextMapPropagator.Getter< return context; } - return DefaultSpan.createInContext(spanContext, context); + return TracingContextUtils.withSpan(DefaultSpan.create(spanContext), context); } private static SpanContext getSpanContextFromMultipleHeaders( diff --git a/extensions/trace_propagators/src/main/java/io/opentelemetry/extensions/trace/propagation/B3PropagatorExtractorSingleHeader.java b/extensions/trace_propagators/src/main/java/io/opentelemetry/extensions/trace/propagation/B3PropagatorExtractorSingleHeader.java index 2a34f35f3ef..d3f073e46aa 100644 --- a/extensions/trace_propagators/src/main/java/io/opentelemetry/extensions/trace/propagation/B3PropagatorExtractorSingleHeader.java +++ b/extensions/trace_propagators/src/main/java/io/opentelemetry/extensions/trace/propagation/B3PropagatorExtractorSingleHeader.java @@ -23,6 +23,7 @@ import io.opentelemetry.context.propagation.TextMapPropagator; import io.opentelemetry.trace.DefaultSpan; import io.opentelemetry.trace.SpanContext; +import io.opentelemetry.trace.TracingContextUtils; import java.util.Objects; import java.util.logging.Logger; import javax.annotation.concurrent.Immutable; @@ -41,7 +42,7 @@ public Context extract(Context context, C carrier, TextMapPropagator.Getter< return context; } - return DefaultSpan.createInContext(spanContext, context); + return TracingContextUtils.withSpan(DefaultSpan.create(spanContext), context); } @SuppressWarnings("StringSplitter") diff --git a/extensions/trace_propagators/src/main/java/io/opentelemetry/extensions/trace/propagation/JaegerPropagator.java b/extensions/trace_propagators/src/main/java/io/opentelemetry/extensions/trace/propagation/JaegerPropagator.java index d464433b428..c2965e1c360 100644 --- a/extensions/trace_propagators/src/main/java/io/opentelemetry/extensions/trace/propagation/JaegerPropagator.java +++ b/extensions/trace_propagators/src/main/java/io/opentelemetry/extensions/trace/propagation/JaegerPropagator.java @@ -114,7 +114,7 @@ public Context extract(Context context, C carrier, Getter getter) { return context; } - return DefaultSpan.createInContext(spanContext, context); + return TracingContextUtils.withSpan(DefaultSpan.create(spanContext), context); } @SuppressWarnings("StringSplitter") diff --git a/extensions/trace_propagators/src/main/java/io/opentelemetry/extensions/trace/propagation/OtTracerPropagator.java b/extensions/trace_propagators/src/main/java/io/opentelemetry/extensions/trace/propagation/OtTracerPropagator.java index b9769f519eb..2231710ba6b 100644 --- a/extensions/trace_propagators/src/main/java/io/opentelemetry/extensions/trace/propagation/OtTracerPropagator.java +++ b/extensions/trace_propagators/src/main/java/io/opentelemetry/extensions/trace/propagation/OtTracerPropagator.java @@ -88,7 +88,7 @@ public Context extract(Context context, C carrier, Getter getter) { if (!spanContext.isValid()) { return context; } - return DefaultSpan.createInContext(spanContext, context); + return TracingContextUtils.withSpan(DefaultSpan.create(spanContext), context); } static SpanContext buildSpanContext(String traceId, String spanId, String sampled) { diff --git a/extensions/trace_propagators/src/test/java/io/opentelemetry/extensions/trace/propagation/AwsXRayPropagatorTest.java b/extensions/trace_propagators/src/test/java/io/opentelemetry/extensions/trace/propagation/AwsXRayPropagatorTest.java index 4804eb0d8fd..a81369f20fa 100644 --- a/extensions/trace_propagators/src/test/java/io/opentelemetry/extensions/trace/propagation/AwsXRayPropagatorTest.java +++ b/extensions/trace_propagators/src/test/java/io/opentelemetry/extensions/trace/propagation/AwsXRayPropagatorTest.java @@ -258,7 +258,7 @@ void extract_InvalidFlags_NonNumeric() { } private static Context withSpanContext(SpanContext spanContext, Context context) { - return DefaultSpan.createInContext(spanContext, context); + return TracingContextUtils.withSpan(DefaultSpan.create(spanContext), context); } private static SpanContext getSpanContext(Context context) { diff --git a/extensions/trace_propagators/src/test/java/io/opentelemetry/extensions/trace/propagation/B3PropagatorTest.java b/extensions/trace_propagators/src/test/java/io/opentelemetry/extensions/trace/propagation/B3PropagatorTest.java index d75c3bc7684..94e9286d7b1 100644 --- a/extensions/trace_propagators/src/test/java/io/opentelemetry/extensions/trace/propagation/B3PropagatorTest.java +++ b/extensions/trace_propagators/src/test/java/io/opentelemetry/extensions/trace/propagation/B3PropagatorTest.java @@ -68,7 +68,7 @@ private static SpanContext getSpanContext(Context context) { } private static Context withSpanContext(SpanContext spanContext, Context context) { - return DefaultSpan.createInContext(spanContext, context); + return TracingContextUtils.withSpan(DefaultSpan.create(spanContext), context); } @Test diff --git a/extensions/trace_propagators/src/test/java/io/opentelemetry/extensions/trace/propagation/JaegerPropagatorTest.java b/extensions/trace_propagators/src/test/java/io/opentelemetry/extensions/trace/propagation/JaegerPropagatorTest.java index d255bd5e396..096bac486dc 100644 --- a/extensions/trace_propagators/src/test/java/io/opentelemetry/extensions/trace/propagation/JaegerPropagatorTest.java +++ b/extensions/trace_propagators/src/test/java/io/opentelemetry/extensions/trace/propagation/JaegerPropagatorTest.java @@ -76,7 +76,7 @@ private static SpanContext getSpanContext(Context context) { } private static Context withSpanContext(SpanContext spanContext, Context context) { - return DefaultSpan.createInContext(spanContext, context); + return TracingContextUtils.withSpan(DefaultSpan.create(spanContext), context); } @Test diff --git a/extensions/trace_propagators/src/test/java/io/opentelemetry/extensions/trace/propagation/OtTracerPropagatorTest.java b/extensions/trace_propagators/src/test/java/io/opentelemetry/extensions/trace/propagation/OtTracerPropagatorTest.java index f40430fb6b9..6344a51c928 100644 --- a/extensions/trace_propagators/src/test/java/io/opentelemetry/extensions/trace/propagation/OtTracerPropagatorTest.java +++ b/extensions/trace_propagators/src/test/java/io/opentelemetry/extensions/trace/propagation/OtTracerPropagatorTest.java @@ -56,7 +56,7 @@ private static SpanContext getSpanContext(Context context) { } private static Context withSpanContext(SpanContext spanContext, Context context) { - return DefaultSpan.createInContext(spanContext, context); + return TracingContextUtils.withSpan(DefaultSpan.create(spanContext), context); } @Test diff --git a/extensions/trace_propagators/src/test/java/io/opentelemetry/extensions/trace/propagation/TraceMultiPropagatorTest.java b/extensions/trace_propagators/src/test/java/io/opentelemetry/extensions/trace/propagation/TraceMultiPropagatorTest.java index 68e466beef2..71d8d0c9fd5 100644 --- a/extensions/trace_propagators/src/test/java/io/opentelemetry/extensions/trace/propagation/TraceMultiPropagatorTest.java +++ b/extensions/trace_propagators/src/test/java/io/opentelemetry/extensions/trace/propagation/TraceMultiPropagatorTest.java @@ -53,8 +53,7 @@ class TraceMultiPropagatorTest { new TraceId(1245, 67890), new SpanId(12345), TraceFlags.getDefault(), - TraceState.getDefault()), - Context.ROOT); + TraceState.getDefault())); @BeforeEach void init() { diff --git a/opentracing_shim/src/main/java/io/opentelemetry/opentracingshim/Propagation.java b/opentracing_shim/src/main/java/io/opentelemetry/opentracingshim/Propagation.java index 2f404832baf..8448a9a3329 100644 --- a/opentracing_shim/src/main/java/io/opentelemetry/opentracingshim/Propagation.java +++ b/opentracing_shim/src/main/java/io/opentelemetry/opentracingshim/Propagation.java @@ -34,7 +34,9 @@ final class Propagation extends BaseShimObject { } public void injectTextMap(SpanContextShim contextShim, TextMapInject carrier) { - Context context = DefaultSpan.createInContext(contextShim.getSpanContext(), Context.current()); + Context context = + TracingContextUtils.withSpan( + DefaultSpan.create(contextShim.getSpanContext()), Context.current()); context = CorrelationsContextUtils.withCorrelationContext( contextShim.getCorrelationContext(), context); diff --git a/opentracing_shim/src/main/java/io/opentelemetry/opentracingshim/SpanBuilderShim.java b/opentracing_shim/src/main/java/io/opentelemetry/opentracingshim/SpanBuilderShim.java index ca50be8cb3c..555e583d6fb 100644 --- a/opentracing_shim/src/main/java/io/opentelemetry/opentracingshim/SpanBuilderShim.java +++ b/opentracing_shim/src/main/java/io/opentelemetry/opentracingshim/SpanBuilderShim.java @@ -186,15 +186,15 @@ public Span start() { if (ignoreActiveSpan && parentSpan == null && parentSpanContext == null) { builder.setNoParent(); } else if (parentSpan != null) { - // TODO: We ignore current() here - builder.setParent( - TracingContextUtils.withSpan(parentSpan.getSpan(), parentSpan.getSpan().getParent())); + // Note: We ignore the (potentially stored) parentSpan's (grand)parent context here. + builder.setParent(TracingContextUtils.withSpan(parentSpan.getSpan(), Context.current())); SpanContextShim contextShim = spanContextTable().get(parentSpan); distContext = contextShim == null ? null : contextShim.getCorrelationContext(); } else if (parentSpanContext != null) { // TODO: This might be wonky builder.setParent( - DefaultSpan.createInContext(parentSpanContext.getSpanContext(), Context.current())); + TracingContextUtils.withSpan( + DefaultSpan.create(parentSpanContext.getSpanContext()), Context.current())); distContext = parentSpanContext.getCorrelationContext(); } diff --git a/sdk/tracing/src/main/java/io/opentelemetry/sdk/trace/RecordEventsReadableSpan.java b/sdk/tracing/src/main/java/io/opentelemetry/sdk/trace/RecordEventsReadableSpan.java index 75a66a1bf6d..c4558ac7384 100644 --- a/sdk/tracing/src/main/java/io/opentelemetry/sdk/trace/RecordEventsReadableSpan.java +++ b/sdk/tracing/src/main/java/io/opentelemetry/sdk/trace/RecordEventsReadableSpan.java @@ -218,11 +218,6 @@ public SpanContext getSpanContext() { return getContext(); } - @Override - public Context getParent() { - return parent; - } - /** * Returns the name of the {@code Span}. * @@ -529,6 +524,10 @@ int getTotalRecordedLinks() { return totalRecordedLinks; } + Context getParent() { + return parent; + } + @GuardedBy("lock") private List getImmutableLinks() { if (links.isEmpty()) { diff --git a/sdk/tracing/src/main/java/io/opentelemetry/sdk/trace/SpanBuilderSdk.java b/sdk/tracing/src/main/java/io/opentelemetry/sdk/trace/SpanBuilderSdk.java index 9f053de5fa8..fec6f70f365 100644 --- a/sdk/tracing/src/main/java/io/opentelemetry/sdk/trace/SpanBuilderSdk.java +++ b/sdk/tracing/src/main/java/io/opentelemetry/sdk/trace/SpanBuilderSdk.java @@ -195,7 +195,7 @@ public Span startSpan() { final Context originalParent = parent == null ? Context.current() : parent; final Context parentContext = isRootSpan - ? DefaultSpan.createInContext(SpanContext.getInvalid(), originalParent) + ? TracingContextUtils.withSpan(DefaultSpan.getInvalid(), originalParent) : originalParent; final Span parentSpan = TracingContextUtils.getSpan(parentContext); final SpanContext parentSpanContext = parentSpan.getContext(); @@ -240,7 +240,7 @@ public Span startSpan() { traceState); if (!Samplers.isRecording(samplingDecision)) { - return DefaultSpan.create(spanContext, parentContext); + return DefaultSpan.create(spanContext); } ReadableAttributes samplingAttributes = samplingResult.getAttributes(); if (!samplingAttributes.isEmpty()) { diff --git a/sdk/tracing/src/test/java/io/opentelemetry/sdk/trace/RecordEventsReadableSpanTest.java b/sdk/tracing/src/test/java/io/opentelemetry/sdk/trace/RecordEventsReadableSpanTest.java index e9fa00f0de4..3314cd94e50 100644 --- a/sdk/tracing/src/test/java/io/opentelemetry/sdk/trace/RecordEventsReadableSpanTest.java +++ b/sdk/tracing/src/test/java/io/opentelemetry/sdk/trace/RecordEventsReadableSpanTest.java @@ -40,6 +40,7 @@ import io.opentelemetry.trace.TraceFlags; import io.opentelemetry.trace.TraceId; import io.opentelemetry.trace.TraceState; +import io.opentelemetry.trace.TracingContextUtils; import java.io.PrintWriter; import java.io.StringWriter; import java.util.Arrays; @@ -828,12 +829,13 @@ private RecordEventsReadableSpan createTestSpan( kind, parentSpanId == null ? Context.ROOT - : DefaultSpan.createInContext( - SpanContext.createFromRemoteParent( - spanContext.getTraceId(), - parentSpanId, - spanContext.getTraceFlags(), - spanContext.getTraceState()), + : TracingContextUtils.withSpan( + DefaultSpan.create( + SpanContext.createFromRemoteParent( + spanContext.getTraceId(), + parentSpanId, + spanContext.getTraceFlags(), + spanContext.getTraceState())), Context.current()), config, spanProcessor, @@ -931,7 +933,7 @@ void testAsSpanData() { name, instrumentationLibraryInfo, kind, - DefaultSpan.createInContext(parentSpanContext, Context.current()), + TracingContextUtils.withSpan(DefaultSpan.create(parentSpanContext), Context.current()), traceConfig, spanProcessor, clock, diff --git a/testing_internal/src/main/java/io/opentelemetry/sdk/trace/TestSpanData.java b/testing_internal/src/main/java/io/opentelemetry/sdk/trace/TestSpanData.java index e666ec8d9f6..6db02640ed3 100644 --- a/testing_internal/src/main/java/io/opentelemetry/sdk/trace/TestSpanData.java +++ b/testing_internal/src/main/java/io/opentelemetry/sdk/trace/TestSpanData.java @@ -155,12 +155,19 @@ public TestSpanData build() { */ public Builder setParent(SpanId parentSpanId, boolean hasRemoteParent) { return setParent( - DefaultSpan.createInContext( - hasRemoteParent - ? SpanContext.createFromRemoteParent( - getTraceId(), parentSpanId, TraceFlags.getDefault(), TraceState.getDefault()) - : SpanContext.create( - getTraceId(), parentSpanId, TraceFlags.getDefault(), TraceState.getDefault()), + TracingContextUtils.withSpan( + DefaultSpan.create( + hasRemoteParent + ? SpanContext.createFromRemoteParent( + getTraceId(), + parentSpanId, + TraceFlags.getDefault(), + TraceState.getDefault()) + : SpanContext.create( + getTraceId(), + parentSpanId, + TraceFlags.getDefault(), + TraceState.getDefault())), Context.current())); } From 5500b81d50b13fe508c6c3ac5489410aa21004eb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Christian=20Neum=C3=BCller?= Date: Tue, 1 Sep 2020 19:26:57 +0200 Subject: [PATCH 04/11] Make Codecov happy. --- .../test/java/io/opentelemetry/sdk/trace/SpanBuilderSdkTest.java | 1 + 1 file changed, 1 insertion(+) diff --git a/sdk/tracing/src/test/java/io/opentelemetry/sdk/trace/SpanBuilderSdkTest.java b/sdk/tracing/src/test/java/io/opentelemetry/sdk/trace/SpanBuilderSdkTest.java index 95225a36e03..9033dfa8486 100644 --- a/sdk/tracing/src/test/java/io/opentelemetry/sdk/trace/SpanBuilderSdkTest.java +++ b/sdk/tracing/src/test/java/io/opentelemetry/sdk/trace/SpanBuilderSdkTest.java @@ -736,6 +736,7 @@ void parentCurrentSpan() { try { assertThat(span.getContext().getTraceId()).isEqualTo(parent.getContext().getTraceId()); assertThat(span.toSpanData().getParentSpanId()).isEqualTo(parent.getContext().getSpanId()); + assertThat(span.toSpanData().getParent()).isSameAs(Context.current()); } finally { span.end(); } From 0eb98299bacafc3a226a944e99be0245b3968d92 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Christian=20Neum=C3=BCller?= Date: Wed, 2 Sep 2020 09:57:34 +0200 Subject: [PATCH 05/11] Fix overcomplicated DefaultTracer. --- .../main/java/io/opentelemetry/trace/DefaultTracer.java | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/api/src/main/java/io/opentelemetry/trace/DefaultTracer.java b/api/src/main/java/io/opentelemetry/trace/DefaultTracer.java index 91661552529..dd7683cd3e1 100644 --- a/api/src/main/java/io/opentelemetry/trace/DefaultTracer.java +++ b/api/src/main/java/io/opentelemetry/trace/DefaultTracer.java @@ -71,13 +71,12 @@ static NoopSpanBuilder create(String spanName) { @Override public Span startSpan() { + if (isRootSpan) { + return DefaultSpan.getInvalid(); + } if (parent == null) { parent = Context.current(); } - if (isRootSpan) { - parent = TracingContextUtils.withSpan(DefaultSpan.getInvalid(), parent); - } - return new DefaultSpan(TracingContextUtils.getSpan(parent).getContext()); } From 66b6130c30fe65210a97919adffadb39223567aa Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Christian=20Neum=C3=BCller?= Date: Fri, 4 Sep 2020 18:22:07 +0200 Subject: [PATCH 06/11] Fix build after merge. --- .../java/io/opentelemetry/exporters/jaeger/AdapterTest.java | 4 ++-- .../sdk/trace/RecordEventsReadableSpanTest.java | 6 +++--- .../main/java/io/opentelemetry/sdk/trace/TestSpanData.java | 5 ++--- 3 files changed, 7 insertions(+), 8 deletions(-) diff --git a/exporters/jaeger/src/test/java/io/opentelemetry/exporters/jaeger/AdapterTest.java b/exporters/jaeger/src/test/java/io/opentelemetry/exporters/jaeger/AdapterTest.java index 891bc1d4a26..9bba43e6f25 100644 --- a/exporters/jaeger/src/test/java/io/opentelemetry/exporters/jaeger/AdapterTest.java +++ b/exporters/jaeger/src/test/java/io/opentelemetry/exporters/jaeger/AdapterTest.java @@ -299,8 +299,8 @@ private static SpanData getSpanData(long startMs, long endMs) { return TestSpanData.newBuilder() .setHasEnded(true) - .setTraceId(TraceId.fromLowerBase16(TRACE_ID, 0)) - .setSpanId(SpanId.fromLowerBase16(SPAN_ID, 0)) + .setTraceId(TRACE_ID) + .setSpanId(SPAN_ID) .setParent(PARENT_SPAN_ID, false) .setName("GET /api/endpoint") .setStartEpochNanos(TimeUnit.MILLISECONDS.toNanos(startMs)) diff --git a/sdk/tracing/src/test/java/io/opentelemetry/sdk/trace/RecordEventsReadableSpanTest.java b/sdk/tracing/src/test/java/io/opentelemetry/sdk/trace/RecordEventsReadableSpanTest.java index 6bea622c6ed..6d4a27e1e8e 100644 --- a/sdk/tracing/src/test/java/io/opentelemetry/sdk/trace/RecordEventsReadableSpanTest.java +++ b/sdk/tracing/src/test/java/io/opentelemetry/sdk/trace/RecordEventsReadableSpanTest.java @@ -831,7 +831,7 @@ private RecordEventsReadableSpan createTestSpan( : TracingContextUtils.withSpan( DefaultSpan.create( SpanContext.createFromRemoteParent( - spanContext.getTraceId(), + spanContext.getTraceIdAsHexString(), parentSpanId, spanContext.getTraceFlags(), spanContext.getTraceState())), @@ -917,12 +917,12 @@ void testAsSpanData() { final SpanContext parentSpanContext = EXPECTED_HAS_REMOTE_PARENT ? SpanContext.createFromRemoteParent( - context.getTraceId(), + context.getTraceIdAsHexString(), parentSpanId, context.getTraceFlags(), context.getTraceState()) : SpanContext.create( - context.getTraceId(), + context.getTraceIdAsHexString(), parentSpanId, context.getTraceFlags(), context.getTraceState()); diff --git a/testing_internal/src/main/java/io/opentelemetry/sdk/trace/TestSpanData.java b/testing_internal/src/main/java/io/opentelemetry/sdk/trace/TestSpanData.java index 348574c1bfe..b2b1bb98f69 100644 --- a/testing_internal/src/main/java/io/opentelemetry/sdk/trace/TestSpanData.java +++ b/testing_internal/src/main/java/io/opentelemetry/sdk/trace/TestSpanData.java @@ -27,7 +27,6 @@ import io.opentelemetry.trace.DefaultSpan; import io.opentelemetry.trace.Span.Kind; import io.opentelemetry.trace.SpanContext; -import io.opentelemetry.trace.SpanId; import io.opentelemetry.trace.Status; import io.opentelemetry.trace.TraceFlags; import io.opentelemetry.trace.TraceState; @@ -73,8 +72,8 @@ public boolean getHasRemoteParent() { } @Override - public SpanId getParentSpanId() { - return TracingContextUtils.getSpan(getParent()).getContext().getSpanId(); + public String getParentSpanId() { + return TracingContextUtils.getSpan(getParent()).getContext().getSpanIdAsHexString(); } /** From 30fbaf74c9fa6e1f20346b91a762fe6d3932214a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Christian=20Neum=C3=BCller?= Date: Tue, 15 Sep 2020 16:24:02 +0200 Subject: [PATCH 07/11] Fix build after merge. --- .../src/main/java/io/opentelemetry/sdk/trace/TestSpanData.java | 1 + 1 file changed, 1 insertion(+) diff --git a/testing_internal/src/main/java/io/opentelemetry/sdk/trace/TestSpanData.java b/testing_internal/src/main/java/io/opentelemetry/sdk/trace/TestSpanData.java index bf7fedeac9a..0c27bc430e9 100644 --- a/testing_internal/src/main/java/io/opentelemetry/sdk/trace/TestSpanData.java +++ b/testing_internal/src/main/java/io/opentelemetry/sdk/trace/TestSpanData.java @@ -28,6 +28,7 @@ import io.opentelemetry.trace.Span.Kind; import io.opentelemetry.trace.SpanContext; import io.opentelemetry.trace.Status; +import io.opentelemetry.trace.TraceFlags; import io.opentelemetry.trace.TraceState; import io.opentelemetry.trace.TracingContextUtils; import java.util.ArrayList; From 2253f8dbade0c7c6591b1f07a2da4ed2bba31349 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Christian=20Neum=C3=BCller?= Date: Wed, 16 Sep 2020 11:26:20 +0200 Subject: [PATCH 08/11] Revert storage of full Context. --- .../exporters/jaeger/AdapterTest.java | 2 +- .../exporters/otlp/SpanAdapterTest.java | 2 +- .../ZipkinSpanExporterEndToEndHttpTest.java | 3 +- .../zipkin/ZipkinSpanExporterTest.java | 3 +- .../sdk/trace/RecordEventsReadableSpan.java | 35 ++++++----- .../sdk/trace/SpanBuilderSdk.java | 8 +-- .../opentelemetry/sdk/trace/SpanWrapper.java | 6 -- .../sdk/trace/data/SpanData.java | 10 ---- .../trace/RecordEventsReadableSpanTest.java | 30 ++-------- .../sdk/trace/SpanBuilderSdkTest.java | 1 - .../opentelemetry/sdk/trace/TestSpanData.java | 59 +++++-------------- .../sdk/trace/TestSpanDataTest.java | 1 + 12 files changed, 48 insertions(+), 112 deletions(-) diff --git a/exporters/jaeger/src/test/java/io/opentelemetry/exporters/jaeger/AdapterTest.java b/exporters/jaeger/src/test/java/io/opentelemetry/exporters/jaeger/AdapterTest.java index 8af3427f007..809c0e59f7e 100644 --- a/exporters/jaeger/src/test/java/io/opentelemetry/exporters/jaeger/AdapterTest.java +++ b/exporters/jaeger/src/test/java/io/opentelemetry/exporters/jaeger/AdapterTest.java @@ -301,7 +301,7 @@ private static SpanData getSpanData(long startMs, long endMs) { .setHasEnded(true) .setTraceId(TRACE_ID) .setSpanId(SPAN_ID) - .setParent(PARENT_SPAN_ID, false) + .setParentSpanId(PARENT_SPAN_ID) .setName("GET /api/endpoint") .setStartEpochNanos(TimeUnit.MILLISECONDS.toNanos(startMs)) .setEndEpochNanos(TimeUnit.MILLISECONDS.toNanos(endMs)) diff --git a/exporters/otlp/src/test/java/io/opentelemetry/exporters/otlp/SpanAdapterTest.java b/exporters/otlp/src/test/java/io/opentelemetry/exporters/otlp/SpanAdapterTest.java index a139b4ebf9e..06f0bd68387 100644 --- a/exporters/otlp/src/test/java/io/opentelemetry/exporters/otlp/SpanAdapterTest.java +++ b/exporters/otlp/src/test/java/io/opentelemetry/exporters/otlp/SpanAdapterTest.java @@ -79,7 +79,7 @@ void toProtoSpan() { .setHasEnded(true) .setTraceId(TRACE_ID) .setSpanId(SPAN_ID) - .setParent(SpanId.getInvalid(), false) + .setParentSpanId(SpanId.getInvalid()) .setName("GET /api/endpoint") .setKind(Kind.SERVER) .setStartEpochNanos(12345) diff --git a/exporters/zipkin/src/test/java/io/opentelemetry/exporters/zipkin/ZipkinSpanExporterEndToEndHttpTest.java b/exporters/zipkin/src/test/java/io/opentelemetry/exporters/zipkin/ZipkinSpanExporterEndToEndHttpTest.java index c4002ed3c8a..6974336e876 100644 --- a/exporters/zipkin/src/test/java/io/opentelemetry/exporters/zipkin/ZipkinSpanExporterEndToEndHttpTest.java +++ b/exporters/zipkin/src/test/java/io/opentelemetry/exporters/zipkin/ZipkinSpanExporterEndToEndHttpTest.java @@ -152,10 +152,11 @@ private static TestSpanData.Builder buildStandardSpan() { return TestSpanData.newBuilder() .setTraceId(TRACE_ID) .setSpanId(SPAN_ID) - .setParent(PARENT_SPAN_ID, false) + .setParentSpanId(PARENT_SPAN_ID) .setSampled(true) .setStatus(Status.OK) .setKind(Kind.SERVER) + .setHasRemoteParent(true) .setName(SPAN_NAME) .setStartEpochNanos(START_EPOCH_NANOS) .setAttributes(attributes) diff --git a/exporters/zipkin/src/test/java/io/opentelemetry/exporters/zipkin/ZipkinSpanExporterTest.java b/exporters/zipkin/src/test/java/io/opentelemetry/exporters/zipkin/ZipkinSpanExporterTest.java index d1783ba6d26..dab9ab9601a 100644 --- a/exporters/zipkin/src/test/java/io/opentelemetry/exporters/zipkin/ZipkinSpanExporterTest.java +++ b/exporters/zipkin/src/test/java/io/opentelemetry/exporters/zipkin/ZipkinSpanExporterTest.java @@ -310,10 +310,11 @@ private static TestSpanData.Builder buildStandardSpan() { return TestSpanData.newBuilder() .setTraceId(TRACE_ID) .setSpanId(SPAN_ID) - .setParent(PARENT_SPAN_ID, true) + .setParentSpanId(PARENT_SPAN_ID) .setSampled(true) .setStatus(Status.OK) .setKind(Kind.SERVER) + .setHasRemoteParent(true) .setName("Recv.helloworld.Greeter.SayHello") .setStartEpochNanos(1505855794_194009601L) .setAttributes(attributes) diff --git a/sdk/tracing/src/main/java/io/opentelemetry/sdk/trace/RecordEventsReadableSpan.java b/sdk/tracing/src/main/java/io/opentelemetry/sdk/trace/RecordEventsReadableSpan.java index deda5edafab..3ff00020730 100644 --- a/sdk/tracing/src/main/java/io/opentelemetry/sdk/trace/RecordEventsReadableSpan.java +++ b/sdk/tracing/src/main/java/io/opentelemetry/sdk/trace/RecordEventsReadableSpan.java @@ -17,7 +17,6 @@ package io.opentelemetry.sdk.trace; import com.google.common.collect.EvictingQueue; -import io.grpc.Context; import io.opentelemetry.common.AttributeConsumer; import io.opentelemetry.common.AttributeValue; import io.opentelemetry.common.Attributes; @@ -36,7 +35,6 @@ import io.opentelemetry.trace.SpanContext; import io.opentelemetry.trace.Status; import io.opentelemetry.trace.Tracer; -import io.opentelemetry.trace.TracingContextUtils; import io.opentelemetry.trace.attributes.SemanticAttributes; import java.io.PrintWriter; import java.io.StringWriter; @@ -59,8 +57,10 @@ final class RecordEventsReadableSpan implements ReadWriteSpan { private final TraceConfig traceConfig; // Contains the identifiers associated with this Span. private final SpanContext context; - // Parent context. - private final Context parent; + // The parent SpanId of this span. Invalid if this is a root span. + private final String parentSpanId; + // True if the parent is on a different process. + private final boolean hasRemoteParent; // Handler called when the span starts and ends. private final SpanProcessor spanProcessor; // The displayed name of the span. @@ -110,7 +110,8 @@ private RecordEventsReadableSpan( String name, InstrumentationLibraryInfo instrumentationLibraryInfo, Kind kind, - Context parent, + String parentSpanId, + boolean hasRemoteParent, TraceConfig traceConfig, SpanProcessor spanProcessor, Clock clock, @@ -121,7 +122,8 @@ private RecordEventsReadableSpan( long startEpochNanos) { this.context = context; this.instrumentationLibraryInfo = instrumentationLibraryInfo; - this.parent = parent; + this.parentSpanId = parentSpanId; + this.hasRemoteParent = hasRemoteParent; this.links = links; this.totalRecordedLinks = totalRecordedLinks; this.name = name; @@ -142,7 +144,10 @@ private RecordEventsReadableSpan( * @param context supplies the trace_id and span_id for the newly started span. * @param name the displayed name for the new span. * @param kind the span kind. - * @param parent parent Context. + * @param parentSpanId the span_id of the parent span, or {@code Span.INVALID} if the new span is + * a root span. + * @param hasRemoteParent {@code true} if the parentContext is remote. {@code false} if this is a + * root span. * @param traceConfig trace parameters like sampler and probability. * @param spanProcessor handler called when the span starts and ends. * @param clock the clock used to get the time. @@ -156,7 +161,8 @@ static RecordEventsReadableSpan startSpan( String name, InstrumentationLibraryInfo instrumentationLibraryInfo, Kind kind, - Context parent, + @Nullable String parentSpanId, + boolean hasRemoteParent, TraceConfig traceConfig, SpanProcessor spanProcessor, Clock clock, @@ -171,7 +177,8 @@ static RecordEventsReadableSpan startSpan( name, instrumentationLibraryInfo, kind, - parent, + parentSpanId, + hasRemoteParent, traceConfig, spanProcessor, clock, @@ -499,7 +506,7 @@ private Status getStatusWithDefault() { } String getParentSpanId() { - return TracingContextUtils.getSpan(parent).getContext().getSpanIdAsHexString(); + return parentSpanId; } Resource getResource() { @@ -515,17 +522,13 @@ long getStartEpochNanos() { } boolean hasRemoteParent() { - return TracingContextUtils.getSpan(parent).getContext().isRemote(); + return hasRemoteParent; } int getTotalRecordedLinks() { return totalRecordedLinks; } - Context getParent() { - return parent; - } - @GuardedBy("lock") private List getImmutableLinks() { if (links.isEmpty()) { @@ -604,7 +607,7 @@ public String toString() { sb.append(", spanId="); sb.append(context.getSpanIdAsHexString()); sb.append(", parentSpanId="); - sb.append(getParentSpanId()); + sb.append(parentSpanId); sb.append(", name="); sb.append(name); sb.append(", kind="); diff --git a/sdk/tracing/src/main/java/io/opentelemetry/sdk/trace/SpanBuilderSdk.java b/sdk/tracing/src/main/java/io/opentelemetry/sdk/trace/SpanBuilderSdk.java index 8a67d8ef51f..b3b08a4cc0f 100644 --- a/sdk/tracing/src/main/java/io/opentelemetry/sdk/trace/SpanBuilderSdk.java +++ b/sdk/tracing/src/main/java/io/opentelemetry/sdk/trace/SpanBuilderSdk.java @@ -185,11 +185,8 @@ public Span.Builder setStartTimestamp(long startTimestamp) { @Override public Span startSpan() { - final Context originalParent = parent == null ? Context.current() : parent; final Context parentContext = - isRootSpan - ? TracingContextUtils.withSpan(DefaultSpan.getInvalid(), originalParent) - : originalParent; + isRootSpan ? Context.ROOT : parent == null ? Context.current() : parent; final Span parentSpan = TracingContextUtils.getSpan(parentContext); final SpanContext parentSpanContext = parentSpan.getContext(); String traceId; @@ -253,7 +250,8 @@ public void consume(String key, AttributeValue value) { spanName, instrumentationLibraryInfo, spanKind, - parentContext, + parentSpanContext.getSpanIdAsHexString(), + parentSpanContext.isRemote(), traceConfig, spanProcessor, getClock(parentSpan, clock), diff --git a/sdk/tracing/src/main/java/io/opentelemetry/sdk/trace/SpanWrapper.java b/sdk/tracing/src/main/java/io/opentelemetry/sdk/trace/SpanWrapper.java index 39fbefd96b8..9a7740dcc4d 100644 --- a/sdk/tracing/src/main/java/io/opentelemetry/sdk/trace/SpanWrapper.java +++ b/sdk/tracing/src/main/java/io/opentelemetry/sdk/trace/SpanWrapper.java @@ -17,7 +17,6 @@ package io.opentelemetry.sdk.trace; import com.google.auto.value.AutoValue; -import io.grpc.Context; import io.opentelemetry.common.ReadableAttributes; import io.opentelemetry.sdk.common.InstrumentationLibraryInfo; import io.opentelemetry.sdk.resources.Resource; @@ -114,11 +113,6 @@ public String getParentSpanId() { return delegate().getParentSpanId(); } - @Override - public Context getParent() { - return delegate().getParent(); - } - @Override public Resource getResource() { return delegate().getResource(); diff --git a/sdk/tracing/src/main/java/io/opentelemetry/sdk/trace/data/SpanData.java b/sdk/tracing/src/main/java/io/opentelemetry/sdk/trace/data/SpanData.java index e9c2cd507d3..7365be15418 100644 --- a/sdk/tracing/src/main/java/io/opentelemetry/sdk/trace/data/SpanData.java +++ b/sdk/tracing/src/main/java/io/opentelemetry/sdk/trace/data/SpanData.java @@ -17,7 +17,6 @@ package io.opentelemetry.sdk.trace.data; import com.google.auto.value.AutoValue; -import io.grpc.Context; import io.opentelemetry.common.Attributes; import io.opentelemetry.common.ReadableAttributes; import io.opentelemetry.sdk.common.InstrumentationLibraryInfo; @@ -71,15 +70,6 @@ public interface SpanData { */ String getParentSpanId(); - /** - * Returns the parent {@code Context}. If the {@code Span} is a root {@code Span}, the Context - * will contain no Span or an invalid span. - * - * @return the parent {@code SpanId} or an invalid SpanId if this is a root {@code Span}. - * @since 0.8.0 - */ - Context getParent(); - /** * Returns the resource of this {@code Span}. * diff --git a/sdk/tracing/src/test/java/io/opentelemetry/sdk/trace/RecordEventsReadableSpanTest.java b/sdk/tracing/src/test/java/io/opentelemetry/sdk/trace/RecordEventsReadableSpanTest.java index 0578a4c87b2..94742f3ab73 100644 --- a/sdk/tracing/src/test/java/io/opentelemetry/sdk/trace/RecordEventsReadableSpanTest.java +++ b/sdk/tracing/src/test/java/io/opentelemetry/sdk/trace/RecordEventsReadableSpanTest.java @@ -20,7 +20,6 @@ import static org.assertj.core.api.Assertions.assertThat; import static org.junit.jupiter.api.Assertions.assertThrows; -import io.grpc.Context; import io.opentelemetry.common.AttributeValue; import io.opentelemetry.common.Attributes; import io.opentelemetry.common.ReadableAttributes; @@ -32,14 +31,12 @@ import io.opentelemetry.sdk.trace.data.SpanData; import io.opentelemetry.sdk.trace.data.SpanData.Event; import io.opentelemetry.sdk.trace.data.SpanData.Link; -import io.opentelemetry.trace.DefaultSpan; import io.opentelemetry.trace.Span.Kind; import io.opentelemetry.trace.SpanContext; import io.opentelemetry.trace.SpanId; import io.opentelemetry.trace.Status; import io.opentelemetry.trace.TraceFlags; import io.opentelemetry.trace.TraceState; -import io.opentelemetry.trace.TracingContextUtils; import java.io.PrintWriter; import java.io.StringWriter; import java.util.Arrays; @@ -826,16 +823,8 @@ private RecordEventsReadableSpan createTestSpan( SPAN_NAME, instrumentationLibraryInfo, kind, - parentSpanId == null - ? Context.ROOT - : TracingContextUtils.withSpan( - DefaultSpan.create( - SpanContext.createFromRemoteParent( - spanContext.getTraceIdAsHexString(), - parentSpanId, - spanContext.getTraceFlags(), - spanContext.getTraceState())), - Context.current()), + parentSpanId, + /* hasRemoteParent= */ true, config, spanProcessor, testClock, @@ -914,25 +903,14 @@ void testAsSpanData() { SpanContext.create(traceId, spanId, TraceFlags.getDefault(), TraceState.getDefault()); Link link1 = Link.create(context, TestUtils.generateRandomAttributes()); - final SpanContext parentSpanContext = - EXPECTED_HAS_REMOTE_PARENT - ? SpanContext.createFromRemoteParent( - context.getTraceIdAsHexString(), - parentSpanId, - context.getTraceFlags(), - context.getTraceState()) - : SpanContext.create( - context.getTraceIdAsHexString(), - parentSpanId, - context.getTraceFlags(), - context.getTraceState()); RecordEventsReadableSpan readableSpan = RecordEventsReadableSpan.startSpan( context, name, instrumentationLibraryInfo, kind, - TracingContextUtils.withSpan(DefaultSpan.create(parentSpanContext), Context.current()), + parentSpanId, + /* hasRemoteParent= */ EXPECTED_HAS_REMOTE_PARENT, traceConfig, spanProcessor, clock, diff --git a/sdk/tracing/src/test/java/io/opentelemetry/sdk/trace/SpanBuilderSdkTest.java b/sdk/tracing/src/test/java/io/opentelemetry/sdk/trace/SpanBuilderSdkTest.java index cb6c2d73c15..82485489bf3 100644 --- a/sdk/tracing/src/test/java/io/opentelemetry/sdk/trace/SpanBuilderSdkTest.java +++ b/sdk/tracing/src/test/java/io/opentelemetry/sdk/trace/SpanBuilderSdkTest.java @@ -747,7 +747,6 @@ void parentCurrentSpan() { .isEqualTo(parent.getContext().getTraceIdAsHexString()); assertThat(span.toSpanData().getParentSpanId()) .isEqualTo(parent.getContext().getSpanIdAsHexString()); - assertThat(span.toSpanData().getParent()).isSameAs(Context.current()); } finally { span.end(); } diff --git a/testing_internal/src/main/java/io/opentelemetry/sdk/trace/TestSpanData.java b/testing_internal/src/main/java/io/opentelemetry/sdk/trace/TestSpanData.java index 0c27bc430e9..6329a29aae4 100644 --- a/testing_internal/src/main/java/io/opentelemetry/sdk/trace/TestSpanData.java +++ b/testing_internal/src/main/java/io/opentelemetry/sdk/trace/TestSpanData.java @@ -17,20 +17,16 @@ package io.opentelemetry.sdk.trace; import com.google.auto.value.AutoValue; -import io.grpc.Context; import io.opentelemetry.common.AttributeValue; import io.opentelemetry.common.Attributes; import io.opentelemetry.common.ReadableAttributes; import io.opentelemetry.sdk.common.InstrumentationLibraryInfo; import io.opentelemetry.sdk.resources.Resource; import io.opentelemetry.sdk.trace.data.SpanData; -import io.opentelemetry.trace.DefaultSpan; import io.opentelemetry.trace.Span.Kind; -import io.opentelemetry.trace.SpanContext; +import io.opentelemetry.trace.SpanId; import io.opentelemetry.trace.Status; -import io.opentelemetry.trace.TraceFlags; import io.opentelemetry.trace.TraceState; -import io.opentelemetry.trace.TracingContextUtils; import java.util.ArrayList; import java.util.Collections; import java.util.List; @@ -53,7 +49,7 @@ public abstract class TestSpanData implements SpanData { */ public static Builder newBuilder() { return new AutoValue_TestSpanData.Builder() - .setParent(Context.ROOT) + .setParentSpanId(SpanId.getInvalid()) .setInstrumentationLibraryInfo(InstrumentationLibraryInfo.getEmpty()) .setLinks(Collections.emptyList()) .setTotalRecordedLinks(0) @@ -63,19 +59,10 @@ public static Builder newBuilder() { .setResource(Resource.getEmpty()) .setTraceState(TraceState.getDefault()) .setSampled(false) + .setHasRemoteParent(false) .setTotalAttributeCount(0); } - @Override - public boolean getHasRemoteParent() { - return TracingContextUtils.getSpan(getParent()).getContext().isRemote(); - } - - @Override - public String getParentSpanId() { - return TracingContextUtils.getSpan(getParent()).getContext().getSpanIdAsHexString(); - } - /** * A {@code Builder} class for {@link TestSpanData}. * @@ -90,8 +77,6 @@ public abstract static class Builder { abstract List getLinks(); - abstract String getTraceId(); - /** * Create a new SpanData instance from the data in this. * @@ -132,36 +117,13 @@ public TestSpanData build() { public abstract Builder setTraceState(TraceState traceState); /** - * The parent Context associated for this span, which may be empty. + * The parent span id associated for this span, which may be null. * - * @param parent the parent Context of the parent + * @param parentSpanId the SpanId of the parent * @return this. * @since 0.1.0 */ - public abstract Builder setParent(Context parent); - - /** - * Utility function to set a parent context based on the current one. - * - * @see #setParent(Context) - */ - public Builder setParent(String parentSpanId, boolean hasRemoteParent) { - return setParent( - TracingContextUtils.withSpan( - DefaultSpan.create( - hasRemoteParent - ? SpanContext.createFromRemoteParent( - getTraceId(), - parentSpanId, - TraceFlags.getDefault(), - TraceState.getDefault()) - : SpanContext.create( - getTraceId(), - parentSpanId, - TraceFlags.getDefault(), - TraceState.getDefault())), - Context.current())); - } + public abstract Builder setParentSpanId(String parentSpanId); /** * Set the {@link Resource} associated with this span. Must not be null. @@ -259,6 +221,15 @@ public abstract Builder setInstrumentationLibraryInfo( */ public abstract Builder setLinks(List links); + /** + * Sets to true if the span has a parent on a different process. + * + * @param hasRemoteParent A boolean indicating if the span has a remote parent. + * @return this + * @since 0.3.0 + */ + public abstract Builder setHasRemoteParent(boolean hasRemoteParent); + /** * Sets to true if the span has been ended. * diff --git a/testing_internal/src/test/java/io/opentelemetry/sdk/trace/TestSpanDataTest.java b/testing_internal/src/test/java/io/opentelemetry/sdk/trace/TestSpanDataTest.java index 03875ae2e1f..30ef616b9ce 100644 --- a/testing_internal/src/test/java/io/opentelemetry/sdk/trace/TestSpanDataTest.java +++ b/testing_internal/src/test/java/io/opentelemetry/sdk/trace/TestSpanDataTest.java @@ -125,6 +125,7 @@ private static TestSpanData.Builder createBasicSpanBuilder() { .setEndEpochNanos(END_EPOCH_NANOS) .setKind(Kind.SERVER) .setStatus(Status.OK) + .setHasRemoteParent(false) .setTotalRecordedEvents(0) .setTotalRecordedLinks(0); } From e8dc898e21cbc1c7428ee0383824e19fba3be9ba Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Christian=20Neum=C3=BCller?= Date: Wed, 23 Sep 2020 16:29:38 +0200 Subject: [PATCH 09/11] Lint. --- .../java/io/opentelemetry/opentracingshim/SpanBuilderShim.java | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/opentracing_shim/src/main/java/io/opentelemetry/opentracingshim/SpanBuilderShim.java b/opentracing_shim/src/main/java/io/opentelemetry/opentracingshim/SpanBuilderShim.java index 6d316072d50..ccaa75439af 100644 --- a/opentracing_shim/src/main/java/io/opentelemetry/opentracingshim/SpanBuilderShim.java +++ b/opentracing_shim/src/main/java/io/opentelemetry/opentracingshim/SpanBuilderShim.java @@ -16,13 +16,12 @@ package io.opentelemetry.opentracingshim; -import io.grpc.Context; - import static io.opentelemetry.common.AttributesKeys.booleanKey; import static io.opentelemetry.common.AttributesKeys.doubleKey; import static io.opentelemetry.common.AttributesKeys.longKey; import static io.opentelemetry.common.AttributesKeys.stringKey; +import io.grpc.Context; import io.opentelemetry.common.AttributeKey; import io.opentelemetry.correlationcontext.CorrelationContext; import io.opentelemetry.trace.DefaultSpan; From 9f618e0c3e526d39a86bacba672fdac48a65ee67 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Christian=20Neum=C3=BCller?= Date: Thu, 24 Sep 2020 17:46:53 +0200 Subject: [PATCH 10/11] Clean up OpenTracing SpanBuilderShim. --- .../io/opentelemetry/opentracingshim/SpanBuilderShim.java | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/opentracing_shim/src/main/java/io/opentelemetry/opentracingshim/SpanBuilderShim.java b/opentracing_shim/src/main/java/io/opentelemetry/opentracingshim/SpanBuilderShim.java index ccaa75439af..2a4d4a91fb3 100644 --- a/opentracing_shim/src/main/java/io/opentelemetry/opentracingshim/SpanBuilderShim.java +++ b/opentracing_shim/src/main/java/io/opentelemetry/opentracingshim/SpanBuilderShim.java @@ -195,15 +195,13 @@ public Span start() { if (ignoreActiveSpan && parentSpan == null && parentSpanContext == null) { builder.setNoParent(); } else if (parentSpan != null) { - // Note: We ignore the (potentially stored) parentSpan's (grand)parent context here. - builder.setParent(TracingContextUtils.withSpan(parentSpan.getSpan(), Context.current())); + builder.setParent(TracingContextUtils.withSpan(parentSpan.getSpan(), Context.ROOT)); SpanContextShim contextShim = spanContextTable().get(parentSpan); distContext = contextShim == null ? null : contextShim.getCorrelationContext(); } else if (parentSpanContext != null) { - // TODO: This might be wonky builder.setParent( TracingContextUtils.withSpan( - DefaultSpan.create(parentSpanContext.getSpanContext()), Context.current())); + DefaultSpan.create(parentSpanContext.getSpanContext()), Context.ROOT)); distContext = parentSpanContext.getCorrelationContext(); } From 6eea962875d0971421d557d067d17018a34832c4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Christian=20Neum=C3=BCller?= Date: Thu, 24 Sep 2020 17:55:41 +0200 Subject: [PATCH 11/11] Rename Context -> RequestHandlerContext in testbed. --- .../concurrentcommonrequesthandler/Client.java | 6 +++--- .../RequestHandler.java | 13 +++++++------ .../{Context.java => RequestHandlerContext.java} | 2 +- 3 files changed, 11 insertions(+), 10 deletions(-) rename sdk_extensions/testbed/src/test/java/io/opentelemetry/sdk/extensions/trace/testbed/concurrentcommonrequesthandler/{Context.java => RequestHandlerContext.java} (91%) diff --git a/sdk_extensions/testbed/src/test/java/io/opentelemetry/sdk/extensions/trace/testbed/concurrentcommonrequesthandler/Client.java b/sdk_extensions/testbed/src/test/java/io/opentelemetry/sdk/extensions/trace/testbed/concurrentcommonrequesthandler/Client.java index ffc2ba1046d..137144c68ba 100644 --- a/sdk_extensions/testbed/src/test/java/io/opentelemetry/sdk/extensions/trace/testbed/concurrentcommonrequesthandler/Client.java +++ b/sdk_extensions/testbed/src/test/java/io/opentelemetry/sdk/extensions/trace/testbed/concurrentcommonrequesthandler/Client.java @@ -31,7 +31,7 @@ public Client(RequestHandler requestHandler) { } public Future send(final Object message) { - final Context context = new Context(); + final RequestHandlerContext requestHandlerContext = new RequestHandlerContext(); return executor.submit( () -> { TestUtils.sleep(); @@ -39,7 +39,7 @@ public Future send(final Object message) { .submit( () -> { TestUtils.sleep(); - requestHandler.beforeRequest(message, context); + requestHandler.beforeRequest(message, requestHandlerContext); }) .get(); @@ -47,7 +47,7 @@ public Future send(final Object message) { .submit( () -> { TestUtils.sleep(); - requestHandler.afterResponse(message, context); + requestHandler.afterResponse(message, requestHandlerContext); }) .get(); diff --git a/sdk_extensions/testbed/src/test/java/io/opentelemetry/sdk/extensions/trace/testbed/concurrentcommonrequesthandler/RequestHandler.java b/sdk_extensions/testbed/src/test/java/io/opentelemetry/sdk/extensions/trace/testbed/concurrentcommonrequesthandler/RequestHandler.java index cfda79ed1ad..aec60bf15a7 100644 --- a/sdk_extensions/testbed/src/test/java/io/opentelemetry/sdk/extensions/trace/testbed/concurrentcommonrequesthandler/RequestHandler.java +++ b/sdk_extensions/testbed/src/test/java/io/opentelemetry/sdk/extensions/trace/testbed/concurrentcommonrequesthandler/RequestHandler.java @@ -16,6 +16,7 @@ package io.opentelemetry.sdk.extensions.trace.testbed.concurrentcommonrequesthandler; +import io.grpc.Context; import io.opentelemetry.trace.Span; import io.opentelemetry.trace.Span.Kind; import io.opentelemetry.trace.Tracer; @@ -29,18 +30,18 @@ final class RequestHandler { private final Tracer tracer; - private final io.grpc.Context parentContext; + private final Context parentContext; public RequestHandler(Tracer tracer) { this(tracer, null); } - public RequestHandler(Tracer tracer, io.grpc.Context parentContext) { + public RequestHandler(Tracer tracer, Context parentContext) { this.tracer = tracer; this.parentContext = parentContext; } - public void beforeRequest(Object request, Context context) { + public void beforeRequest(Object request, RequestHandlerContext requestHandlerContext) { // we cannot use active span because we don't know in which thread it is executed // and we cannot therefore activate span. thread can come from common thread pool. Span.Builder spanBuilder = @@ -50,11 +51,11 @@ public void beforeRequest(Object request, Context context) { spanBuilder.setParent(parentContext); } - context.put("span", spanBuilder.startSpan()); + requestHandlerContext.put("span", spanBuilder.startSpan()); } - public void afterResponse(Object response, Context context) { - Object spanObject = context.get("span"); + public void afterResponse(Object response, RequestHandlerContext requestHandlerContext) { + Object spanObject = requestHandlerContext.get("span"); if (spanObject instanceof Span) { Span span = (Span) spanObject; span.end(); diff --git a/sdk_extensions/testbed/src/test/java/io/opentelemetry/sdk/extensions/trace/testbed/concurrentcommonrequesthandler/Context.java b/sdk_extensions/testbed/src/test/java/io/opentelemetry/sdk/extensions/trace/testbed/concurrentcommonrequesthandler/RequestHandlerContext.java similarity index 91% rename from sdk_extensions/testbed/src/test/java/io/opentelemetry/sdk/extensions/trace/testbed/concurrentcommonrequesthandler/Context.java rename to sdk_extensions/testbed/src/test/java/io/opentelemetry/sdk/extensions/trace/testbed/concurrentcommonrequesthandler/RequestHandlerContext.java index 4ec2157f7de..b401a2bdb01 100644 --- a/sdk_extensions/testbed/src/test/java/io/opentelemetry/sdk/extensions/trace/testbed/concurrentcommonrequesthandler/Context.java +++ b/sdk_extensions/testbed/src/test/java/io/opentelemetry/sdk/extensions/trace/testbed/concurrentcommonrequesthandler/RequestHandlerContext.java @@ -18,4 +18,4 @@ import java.util.HashMap; -final class Context extends HashMap {} +final class RequestHandlerContext extends HashMap {}