diff --git a/api/src/main/java/io/opentelemetry/distributedcontext/DistributedContext.java b/api/src/main/java/io/opentelemetry/distributedcontext/DistributedContext.java index 0fa4ffa9f8..f07ac1e950 100644 --- a/api/src/main/java/io/opentelemetry/distributedcontext/DistributedContext.java +++ b/api/src/main/java/io/opentelemetry/distributedcontext/DistributedContext.java @@ -57,16 +57,16 @@ public interface DistributedContext { */ interface Builder { /** - * Sets the parent {@link DistributedContext} to use. If not set, the value of {@link - * DistributedContextManager#getCurrentContext()} at {@link #build()} time will be used as - * parent. + * Sets the parent {@link DistributedContext} to use. If no parent is provided, the value of + * {@link DistributedContextManager#getCurrentContext()} at {@link #build()} time will be used + * as parent, unless {@link #setNoParent()} was called. * *

This must be used to create a {@link DistributedContext} when manual Context * propagation is used. * *

If called multiple times, only the last specified value will be used. * - * @param parent the {@link DistributedContext} used as parent. + * @param parent the {@link DistributedContext} used as parent, not null. * @return this. * @throws NullPointerException if {@code parent} is {@code null}. * @see #setNoParent() @@ -75,9 +75,10 @@ public interface DistributedContext { Builder setParent(DistributedContext parent); /** - * Sets the option to become a {@link DistributedContext} with no parent. If not set, the value - * of {@link DistributedContextManager#getCurrentContext()} at {@link #build()} time will be - * used as parent. + * Sets the option to become a root {@link DistributedContext} with no parent. If not + * called, the value provided using {@link #setParent(DistributedContext)} or otherwise {@link + * DistributedContextManager#getCurrentContext()} at {@link #build()} time will be used as + * parent. * * @return this. * @since 0.1.0 diff --git a/api/src/test/java/io/opentelemetry/distributedcontext/DefaultDistributedContextManagerTest.java b/api/src/test/java/io/opentelemetry/distributedcontext/DefaultDistributedContextManagerTest.java index 5260ffdd27..93fbf19025 100644 --- a/api/src/test/java/io/opentelemetry/distributedcontext/DefaultDistributedContextManagerTest.java +++ b/api/src/test/java/io/opentelemetry/distributedcontext/DefaultDistributedContextManagerTest.java @@ -135,7 +135,7 @@ public final class DefaultDistributedContextManagerTest { } @Test - public void noopContextBuilder_SetParent_DisallowsNullKey() { + public void noopContextBuilder_SetParent_DisallowsNullParent() { DistributedContext.Builder noopBuilder = defaultDistributedContextManager.contextBuilder(); thrown.expect(NullPointerException.class); noopBuilder.setParent(null); diff --git a/sdk/src/main/java/io/opentelemetry/sdk/distributedcontext/DistributedContextSdk.java b/sdk/src/main/java/io/opentelemetry/sdk/distributedcontext/DistributedContextSdk.java index be845601bc..bd0d61fbd8 100644 --- a/sdk/src/main/java/io/opentelemetry/sdk/distributedcontext/DistributedContextSdk.java +++ b/sdk/src/main/java/io/opentelemetry/sdk/distributedcontext/DistributedContextSdk.java @@ -18,11 +18,13 @@ package io.opentelemetry.sdk.distributedcontext; import static io.opentelemetry.internal.Utils.checkNotNull; +import io.opentelemetry.OpenTelemetry; import io.opentelemetry.distributedcontext.DistributedContext; import io.opentelemetry.distributedcontext.Entry; import io.opentelemetry.distributedcontext.EntryKey; import io.opentelemetry.distributedcontext.EntryMetadata; import io.opentelemetry.distributedcontext.EntryValue; +import io.opentelemetry.internal.Utils; import java.util.Collection; import java.util.Collections; import java.util.HashMap; @@ -111,6 +113,7 @@ class DistributedContextSdk implements DistributedContext { // @AutoValue.Builder static class Builder implements DistributedContext.Builder { @Nullable private DistributedContext parent; + private boolean noImplicitParent; private final Map entries; /** Create a new empty DistributedContext builder. */ @@ -120,13 +123,14 @@ class DistributedContextSdk implements DistributedContext { @Override public DistributedContext.Builder setParent(DistributedContext parent) { - this.parent = parent; + this.parent = Utils.checkNotNull(parent, "parent"); return this; } @Override public DistributedContext.Builder setNoParent() { - parent = null; + this.parent = null; + noImplicitParent = true; return this; } @@ -151,7 +155,9 @@ class DistributedContextSdk implements DistributedContext { @Override public DistributedContextSdk build() { - // TODO if (parent == null) parent = DistributedContextManager.getCurrentContext(); + if (parent == null && !noImplicitParent) { + parent = OpenTelemetry.getDistributedContextManager().getCurrentContext(); + } return new DistributedContextSdk(entries, parent); } } diff --git a/sdk/src/test/java/io/opentelemetry/sdk/distributedcontext/DistributedContextSdkTest.java b/sdk/src/test/java/io/opentelemetry/sdk/distributedcontext/DistributedContextSdkTest.java index 586e63eafa..f015e74579 100644 --- a/sdk/src/test/java/io/opentelemetry/sdk/distributedcontext/DistributedContextSdkTest.java +++ b/sdk/src/test/java/io/opentelemetry/sdk/distributedcontext/DistributedContextSdkTest.java @@ -128,9 +128,8 @@ public class DistributedContextSdkTest { @Test public void setParent_nullValue() { DistributedContextSdk parent = listToDistributedContext(T1); - DistributedContext distContext = - contextManager.contextBuilder().setParent(parent).setParent(null).build(); - assertThat(distContext.getEntries()).isEmpty(); + thrown.expect(NullPointerException.class); + contextManager.contextBuilder().setParent(parent).setParent(null).build(); } @Test diff --git a/sdk/src/test/java/io/opentelemetry/sdk/distributedcontext/ScopedDistributedContextTest.java b/sdk/src/test/java/io/opentelemetry/sdk/distributedcontext/ScopedDistributedContextTest.java index e59b141318..f034ce071c 100644 --- a/sdk/src/test/java/io/opentelemetry/sdk/distributedcontext/ScopedDistributedContextTest.java +++ b/sdk/src/test/java/io/opentelemetry/sdk/distributedcontext/ScopedDistributedContextTest.java @@ -78,7 +78,6 @@ public class ScopedDistributedContextTest { DistributedContext newEntries = contextManager .contextBuilder() - .setParent(contextManager.getCurrentContext()) .put(KEY_2, VALUE_2, METADATA_UNLIMITED_PROPAGATION) .build(); assertThat(newEntries.getEntries()) @@ -110,7 +109,6 @@ public class ScopedDistributedContextTest { DistributedContext innerDistContext = contextManager .contextBuilder() - .setParent(contextManager.getCurrentContext()) .put(KEY_2, VALUE_2, METADATA_UNLIMITED_PROPAGATION) .build(); try (Scope scope2 = contextManager.withContext(innerDistContext)) { @@ -136,7 +134,6 @@ public class ScopedDistributedContextTest { DistributedContext innerDistContext = contextManager .contextBuilder() - .setParent(contextManager.getCurrentContext()) .put(KEY_3, VALUE_3, METADATA_NO_PROPAGATION) .put(KEY_2, VALUE_4, METADATA_NO_PROPAGATION) .build(); @@ -151,4 +148,22 @@ public class ScopedDistributedContextTest { assertThat(contextManager.getCurrentContext()).isSameInstanceAs(scopedDistContext); } } + + @Test + public void setNoParent_doesNotInheritContext() { + assertThat(contextManager.getCurrentContext().getEntries()).isEmpty(); + DistributedContext scopedDistContext = + contextManager.contextBuilder().put(KEY_1, VALUE_1, METADATA_UNLIMITED_PROPAGATION).build(); + try (Scope scope = contextManager.withContext(scopedDistContext)) { + DistributedContext innerDistContext = + contextManager + .contextBuilder() + .setNoParent() + .put(KEY_2, VALUE_2, METADATA_UNLIMITED_PROPAGATION) + .build(); + assertThat(innerDistContext.getEntries()) + .containsExactly(Entry.create(KEY_2, VALUE_2, METADATA_UNLIMITED_PROPAGATION)); + } + assertThat(contextManager.getCurrentContext().getEntries()).isEmpty(); + } }