Change DistributedContextSdk.Builder to use current dctx as implicit parent (#439)

* Change DistributedContextSdk.Builder to use current DistributedContext as implicit parent if none is provided explicitly

* Change DistributedContext.setNoParent to prevent inheriting an implicit context

* Clarify doc for DistributedContext.setNoParent

* Disallow null for DistributedContext.setParent
This commit is contained in:
Armin Ruech 2019-06-27 23:20:54 +02:00 committed by Bogdan Drutu
parent 6378db90fe
commit 36f80e1fdc
5 changed files with 38 additions and 17 deletions

View File

@ -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.
*
* <p>This <b>must</b> be used to create a {@link DistributedContext} when manual Context
* propagation is used.
*
* <p>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 <b>not</b>
* 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

View File

@ -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);

View File

@ -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<EntryKey, Entry> 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);
}
}

View File

@ -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

View File

@ -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();
}
}