open-telemetry
/
opentelemetry-java
mirror of https://github.com/open-telemetry/opentelemetry-java.git
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

Remove BaggageBuilder.setParent / setNoParent (#2838)

This commit is contained in:
Anuraag Agrawal 2021-02-18 02:57:56 +09:00 committed by GitHub
parent 4623334c7c
commit fa15e8b2d0
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
7 changed files with 22 additions and 208 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

33
api/all/src/jmh/java/io/opentelemetry/api/baggage/BaggageBenchmark.java
View File

@ -5,7 +5,6 @@
package io.opentelemetry.api.baggage;
import io.opentelemetry.context.Context;
import java.util.ArrayList;
import java.util.List;
import java.util.concurrent.TimeUnit;
@ -65,36 +64,4 @@ public class BaggageBenchmark {
}
return baggage;
}
@Benchmark
@BenchmarkMode({Mode.AverageTime})
@Fork(1)
@Measurement(iterations = 15, time = 1)
@OutputTimeUnit(TimeUnit.NANOSECONDS)
@Warmup(iterations = 5, time = 1)
public Baggage baggageParentBenchmark() {
Baggage baggage = Baggage.empty();
Context context = Context.root().with(baggage);
for (int i = 0; i < itemsToAdd; i++) {
baggage = Baggage.builder().put(keys.get(i), values.get(i)).setParent(context).build();
context = context.with(baggage);
}
return baggage;
}
@Benchmark
@BenchmarkMode({Mode.AverageTime})
@Fork(1)
@Measurement(iterations = 15, time = 1)
@OutputTimeUnit(TimeUnit.NANOSECONDS)
@Warmup(iterations = 5, time = 1)
public Baggage baggageParentBenchmark_noContent() {
Baggage baggage = Baggage.empty();
Context context = Context.root().with(baggage);
for (int i = 0; i < itemsToAdd; i++) {
baggage = Baggage.builder().setParent(context).build();
context = context.with(baggage);
}
return baggage;
}
}

31
api/all/src/main/java/io/opentelemetry/api/baggage/BaggageBuilder.java
View File

@ -5,8 +5,6 @@
package io.opentelemetry.api.baggage;
import io.opentelemetry.context.Context;
/**
* A builder of {@link Baggage}.
*
@ -14,35 +12,6 @@ import io.opentelemetry.context.Context;
*/
public interface BaggageBuilder {
/**
* Sets the parent {@link Baggage} to use from the specified {@code Context}. If no parent {@link
* Baggage} is provided, the value of {@link Baggage#current()} at {@link #build()} time will be
* used as parent, unless {@link #setNoParent()} was called.
*
* <p>If no parent {@link Baggage} is available in the specified {@code Context}, the resulting
* {@link Baggage} will become a root instance, as if {@link #setNoParent()} had been called.
*
* <p>This <b>must</b> be used to create a {@link Baggage} when manual Context propagation is
* used.
*
* <p>If called multiple times, only the last specified value will be used.
*
* @param context the {@code Context}.
* @return this.
* @throws NullPointerException if {@code context} is {@code null}.
* @see #setNoParent()
*/
BaggageBuilder setParent(Context context);
/**
* Sets the option to become a root {@link Baggage} with no parent. If <b>not</b> called, the
* value provided using {@link #setParent(Context)} or otherwise {@link Baggage#current()} at
* {@link #build()} time will be used as parent.
*
* @return this.
*/
BaggageBuilder setNoParent();
/**
* Adds the key/value pair and metadata regardless of whether the key is present.
*

42
api/all/src/main/java/io/opentelemetry/api/baggage/ImmutableBaggage.java
View File

@ -5,11 +5,8 @@
package io.opentelemetry.api.baggage;
import static java.util.Objects.requireNonNull;
import io.opentelemetry.api.internal.ImmutableKeyValuePairs;
import io.opentelemetry.api.internal.StringUtils;
import io.opentelemetry.context.Context;
import java.util.ArrayList;
import java.util.List;
import javax.annotation.Nullable;
@ -42,9 +39,7 @@ final class ImmutableBaggage extends ImmutableKeyValuePairs<String, BaggageEntry
@Override
public BaggageBuilder toBuilder() {
Builder builder = new Builder(new ArrayList<>(data()));
builder.noImplicitParent = true;
return builder;
return new Builder(new ArrayList<>(data()));
}
private static Baggage sortAndFilterToBaggage(Object[] data) {
@ -55,8 +50,6 @@ final class ImmutableBaggage extends ImmutableKeyValuePairs<String, BaggageEntry
// @AutoValue.Builder
static class Builder implements BaggageBuilder {
@Nullable private Baggage parent;
private boolean noImplicitParent;
private final List<Object> data;
Builder() {
@ -67,20 +60,6 @@ final class ImmutableBaggage extends ImmutableKeyValuePairs<String, BaggageEntry
this.data = data;
}
@Override
public BaggageBuilder setParent(Context context) {
requireNonNull(context, "context");
parent = Baggage.fromContext(context);
return this;
}
@Override
public BaggageBuilder setNoParent() {
this.parent = null;
noImplicitParent = true;
return this;
}
@Override
public BaggageBuilder put(String key, String value, BaggageEntryMetadata entryMetadata) {
if (!isKeyValid(key) || !isValueValid(value) || entryMetadata == null) {
@ -104,25 +83,6 @@ final class ImmutableBaggage extends ImmutableKeyValuePairs<String, BaggageEntry
@Override
public Baggage build() {
if (parent == null && !noImplicitParent) {
parent = Baggage.current();
}
List<Object> data = this.data;
if (parent != null && !parent.isEmpty()) {
List<Object> merged = new ArrayList<>(parent.size() * 2 + data.size());
if (parent instanceof ImmutableBaggage) {
merged.addAll(((ImmutableBaggage) parent).data());
} else {
parent.forEach(
(key, entry) -> {
merged.add(key);
merged.add(entry);
});
}
merged.addAll(data);
data = merged;
}
return sortAndFilterToBaggage(data.toArray());
}
}

5
api/all/src/test/java/io/opentelemetry/api/baggage/BaggageTest.java
View File

@ -22,12 +22,9 @@ class BaggageTest {
@Test
void current() {
try (Scope scope =
Context.root()
.with(Baggage.builder().put("foo", "bar").setNoParent().build())
.makeCurrent()) {
Context.root().with(Baggage.builder().put("foo", "bar").build()).makeCurrent()) {
Baggage result = Baggage.current();
assertThat(result.getEntryValue("foo")).isEqualTo("bar");
assertThat(result).isEqualTo(Baggage.builder().setNoParent().put("foo", "bar").build());
}
}
}

90
api/all/src/test/java/io/opentelemetry/api/baggage/ImmutableBaggageTest.java
View File

@ -6,12 +6,9 @@
package io.opentelemetry.api.baggage;
import static org.assertj.core.api.Assertions.assertThat;
import static org.assertj.core.api.Assertions.assertThatThrownBy;
import static org.assertj.core.api.Assertions.entry;
import com.google.common.testing.EqualsTester;
import io.opentelemetry.context.Context;
import io.opentelemetry.context.Scope;
import org.junit.jupiter.api.Test;
/**
@ -61,8 +58,7 @@ class ImmutableBaggageTest {
@Test
void getEntries_chain() {
Context parentContext = Context.root().with(TWO_ENTRIES);
Baggage baggage = Baggage.builder().setParent(parentContext).put(K1, V2, TMD).build();
Baggage baggage = TWO_ENTRIES.toBuilder().put(K1, V2, TMD).build();
assertThat(baggage.asMap())
.containsOnly(
entry(K1, ImmutableEntry.create(V2, TMD)), entry(K2, ImmutableEntry.create(V2, TMD)));
@ -70,23 +66,20 @@ class ImmutableBaggageTest {
@Test
void put_newKey() {
Context parentContext = Context.root().with(ONE_ENTRY);
assertThat(Baggage.builder().setParent(parentContext).put(K2, V2, TMD).build().asMap())
assertThat(ONE_ENTRY.toBuilder().put(K2, V2, TMD).build().asMap())
.containsOnly(
entry(K1, ImmutableEntry.create(V1, TMD)), entry(K2, ImmutableEntry.create(V2, TMD)));
}
@Test
void put_existingKey() {
Context parentContext = Context.root().with(ONE_ENTRY);
assertThat(Baggage.builder().setParent(parentContext).put(K1, V2, TMD).build().asMap())
assertThat(ONE_ENTRY.toBuilder().put(K1, V2, TMD).build().asMap())
.containsOnly(entry(K1, ImmutableEntry.create(V2, TMD)));
}
@Test
void put_nullKey() {
Context parentContext = Context.root().with(ONE_ENTRY);
BaggageBuilder builder = Baggage.builder().setParent(parentContext);
BaggageBuilder builder = ONE_ENTRY.toBuilder();
Baggage built = builder.build();
builder.put(null, V2, TMD);
assertThat(builder.build()).isEqualTo(built);
@ -94,8 +87,7 @@ class ImmutableBaggageTest {
@Test
void put_nullValue() {
Context parentContext = Context.root().with(ONE_ENTRY);
BaggageBuilder builder = Baggage.builder().setParent(parentContext);
BaggageBuilder builder = ONE_ENTRY.toBuilder();
Baggage built = builder.build();
builder.put(K2, null, TMD);
assertThat(builder.build()).isEqualTo(built);
@ -103,8 +95,7 @@ class ImmutableBaggageTest {
@Test
void put_nullMetadata() {
Context parentContext = Context.root().with(ONE_ENTRY);
BaggageBuilder builder = Baggage.builder().setParent(parentContext);
BaggageBuilder builder = ONE_ENTRY.toBuilder();
Baggage built = builder.build();
builder.put(K2, V2, null);
assertThat(builder.build()).isEqualTo(built);
@ -112,8 +103,7 @@ class ImmutableBaggageTest {
@Test
void put_keyUnprintableChars() {
Context parentContext = Context.root().with(ONE_ENTRY);
BaggageBuilder builder = Baggage.builder().setParent(parentContext);
BaggageBuilder builder = ONE_ENTRY.toBuilder();
Baggage built = builder.build();
builder.put("\2ab\3cd", "value");
assertThat(builder.build()).isEqualTo(built);
@ -121,8 +111,7 @@ class ImmutableBaggageTest {
@Test
void put_keyEmpty() {
Context parentContext = Context.root().with(ONE_ENTRY);
BaggageBuilder builder = Baggage.builder().setParent(parentContext);
BaggageBuilder builder = ONE_ENTRY.toBuilder();
Baggage built = builder.build();
builder.put("", "value");
assertThat(builder.build()).isEqualTo(built);
@ -130,44 +119,12 @@ class ImmutableBaggageTest {
@Test
void put_valueUnprintableChars() {
Context parentContext = Context.root().with(ONE_ENTRY);
BaggageBuilder builder = Baggage.builder().setParent(parentContext);
BaggageBuilder builder = ONE_ENTRY.toBuilder();
Baggage built = builder.build();
builder.put(K2, "\2ab\3cd");
assertThat(builder.build()).isEqualTo(built);
}
@Test
void setParent_nullContext() {
assertThatThrownBy(
() -> Baggage.builder().setParent(null).put("cat", "meow").build().getEntryValue("cat"))
.isInstanceOf(NullPointerException.class);
}
@Test
void setParent_fromContext() {
Baggage baggage = Baggage.builder().put(K2, V2, TMD).build();
Context context = Context.root().with(baggage);
baggage = Baggage.builder().setParent(context).build();
assertThat(baggage.asMap()).containsOnly(entry(K2, ImmutableEntry.create(V2, TMD)));
}
@Test
void setParent_fromEmptyContext() {
Context emptyContext = Context.root();
try (Scope ignored = ONE_ENTRY.makeCurrent()) {
Baggage baggage = Baggage.builder().setParent(emptyContext).build();
assertThat(baggage.isEmpty()).isTrue();
}
}
@Test
void setParent_setNoParent() {
Context parentContext = Context.root().with(ONE_ENTRY);
Baggage baggage = Baggage.builder().setParent(parentContext).setNoParent().build();
assertThat(baggage.isEmpty()).isTrue();
}
@Test
void remove_existingKey() {
BaggageBuilder builder = Baggage.builder();
@ -190,8 +147,7 @@ class ImmutableBaggageTest {
@Test
void remove_keyFromParent() {
Context parentContext = Context.root().with(TWO_ENTRIES);
assertThat(Baggage.builder().setParent(parentContext).remove(K1).build().asMap())
assertThat(TWO_ENTRIES.toBuilder().remove(K1).build().asMap())
.containsOnly(entry(K2, ImmutableEntry.create(V2, TMD)));
}
@ -210,42 +166,28 @@ class ImmutableBaggageTest {
Baggage originalBaggage = Baggage.builder().put("key", "value").build();
assertThat(originalBaggage.toBuilder().build()).isEqualTo(originalBaggage);
Baggage parentedBaggage =
Baggage.builder().setParent(Context.root().with(originalBaggage)).build();
assertThat(parentedBaggage.toBuilder().build()).isEqualTo(parentedBaggage);
}
@Test
void toBuilder_allowChanges() {
Baggage singleItemNoParent = Baggage.builder().put("key1", "value1").setNoParent().build();
Baggage singleItemWithParent =
Baggage.builder()
.setParent(Context.root().with(Baggage.empty()))
.put("key1", "value1")
.build();
Baggage singleItemNoParent = Baggage.builder().put("key1", "value1").build();
Baggage singleItemWithParent = Baggage.builder().put("key1", "value1").build();
assertThat(Baggage.empty().toBuilder().put("key1", "value1").build())
.isEqualTo(singleItemNoParent);
assertThat(singleItemNoParent.toBuilder().put("key2", "value2").build())
.isEqualTo(
Baggage.builder().put("key1", "value1").put("key2", "value2").setNoParent().build());
.isEqualTo(Baggage.builder().put("key1", "value1").put("key2", "value2").build());
assertThat(singleItemNoParent.toBuilder().put("key1", "value2").build())
.isEqualTo(Baggage.builder().put("key1", "value2").setNoParent().build());
.isEqualTo(Baggage.builder().put("key1", "value2").build());
assertThat(singleItemWithParent.toBuilder().put("key1", "value2").build())
.isEqualTo(
Baggage.builder()
.put("key1", "value2")
.setParent(Context.root().with(Baggage.empty()))
.build());
.isEqualTo(Baggage.builder().put("key1", "value2").build());
}
@Test
void testEquals() {
Baggage baggage1 = Baggage.builder().put(K1, V1).build();
Baggage baggage2 =
Baggage.builder().setParent(Context.current().with(baggage1)).put(K1, V2).build();
Baggage baggage2 = baggage1.toBuilder().put(K1, V2).build();
Baggage baggage3 = Baggage.builder().put(K1, V2).build();
new EqualsTester()
.addEqualityGroup(

24
api/all/src/test/java/io/opentelemetry/api/baggage/ScopedBaggageTest.java
View File

@ -51,7 +51,7 @@ class ScopedBaggageTest {
Baggage.builder().put(KEY_1, VALUE_1, METADATA_UNLIMITED_PROPAGATION).build();
try (Scope scope = scopedBaggage.makeCurrent()) {
Baggage newEntries =
Baggage.builder().put(KEY_2, VALUE_2, METADATA_UNLIMITED_PROPAGATION).build();
Baggage.current().toBuilder().put(KEY_2, VALUE_2, METADATA_UNLIMITED_PROPAGATION).build();
assertThat(newEntries.asMap())
.containsOnly(
entry(KEY_1, ImmutableEntry.create(VALUE_1, METADATA_UNLIMITED_PROPAGATION)),
@ -80,7 +80,7 @@ class ScopedBaggageTest {
Baggage.builder().put(KEY_1, VALUE_1, METADATA_UNLIMITED_PROPAGATION).build();
try (Scope scope1 = scopedBaggage.makeCurrent()) {
Baggage innerBaggage =
Baggage.builder().put(KEY_2, VALUE_2, METADATA_UNLIMITED_PROPAGATION).build();
Baggage.current().toBuilder().put(KEY_2, VALUE_2, METADATA_UNLIMITED_PROPAGATION).build();
try (Scope scope2 = innerBaggage.makeCurrent()) {
assertThat(Baggage.current().asMap())
.containsOnly(
@ -101,7 +101,7 @@ class ScopedBaggageTest {
.build();
try (Scope scope1 = scopedBaggage.makeCurrent()) {
Baggage innerBaggage =
Baggage.builder()
Baggage.current().toBuilder()
.put(KEY_3, VALUE_3, METADATA_NO_PROPAGATION)
.put(KEY_2, VALUE_4, METADATA_NO_PROPAGATION)
.build();
@ -116,22 +116,4 @@ class ScopedBaggageTest {
assertThat(Baggage.current()).isSameAs(scopedBaggage);
}
}
@Test
void setNoParent_doesNotInheritContext() {
assertThat(Baggage.current().isEmpty()).isTrue();
Baggage scopedBaggage =
Baggage.builder().put(KEY_1, VALUE_1, METADATA_UNLIMITED_PROPAGATION).build();
try (Scope scope = scopedBaggage.makeCurrent()) {
Baggage innerBaggage =
Baggage.builder()
.setNoParent()
.put(KEY_2, VALUE_2, METADATA_UNLIMITED_PROPAGATION)
.build();
assertThat(innerBaggage.asMap())
.containsOnly(
entry(KEY_2, ImmutableEntry.create(VALUE_2, METADATA_UNLIMITED_PROPAGATION)));
}
assertThat(Baggage.current().isEmpty()).isTrue();
}
}

5
opentracing-shim/src/main/java/io/opentelemetry/opentracingshim/SpanContextShim.java
View File

@ -8,7 +8,6 @@ package io.opentelemetry.opentracingshim;
import io.opentelemetry.api.baggage.Baggage;
import io.opentelemetry.api.baggage.BaggageBuilder;
import io.opentelemetry.api.baggage.BaggageEntryMetadata;
import io.opentelemetry.context.Context;
import io.opentracing.SpanContext;
import java.util.AbstractMap;
import java.util.ArrayList;
@ -42,9 +41,7 @@ final class SpanContextShim extends BaseShimObject implements SpanContext {
}
SpanContextShim newWithKeyValue(String key, String value) {
Context parentContext = Context.current().with(baggage);
BaggageBuilder builder = Baggage.builder().setParent(parentContext);
BaggageBuilder builder = baggage.toBuilder();
builder.put(key, value, BaggageEntryMetadata.empty());
return new SpanContextShim(telemetryInfo(), context, builder.build());