Implement baggage using array of key/value pairs instead of Map. (#2016)

* Implement baggage using array of key/value pairs instead of Map.

* Cleanup
This commit is contained in:
Anuraag Agrawal 2020-11-10 01:54:22 +09:00 committed by GitHub
parent cdc725c3c3
commit 9063c385d9
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
7 changed files with 96 additions and 133 deletions

View File

@ -5,130 +5,64 @@
package io.opentelemetry.api.baggage;
import static java.util.Objects.requireNonNull;
import com.google.auto.value.AutoValue;
import io.opentelemetry.api.internal.ImmutableKeyValuePairs;
import io.opentelemetry.context.Context;
import java.util.Collections;
import java.util.HashMap;
import java.util.HashSet;
import java.util.Map;
import java.util.Objects;
import java.util.Set;
import java.util.ArrayList;
import java.util.List;
import javax.annotation.Nullable;
import javax.annotation.concurrent.Immutable;
@Immutable
// TODO: Migrate to AutoValue
// @AutoValue
class ImmutableBaggage implements Baggage {
class ImmutableBaggage extends ImmutableKeyValuePairs<String, Entry> implements Baggage {
static final Baggage EMPTY = new ImmutableBaggage.Builder().build();
static final Baggage EMPTY = new ImmutableBaggage.Builder().setNoParent().build();
// The types of the EntryKey and Entry must match for each entry.
private final Map<String, Entry> entries;
@Nullable private final Baggage parent;
@AutoValue
@Immutable
abstract static class ArrayBackedBaggage extends ImmutableBaggage {
ArrayBackedBaggage() {}
/**
* Creates a new {@link ImmutableBaggage} with the given entries.
*
* @param entries the initial entries for this {@code BaggageSdk}.
* @param parent providing a default set of entries
*/
private ImmutableBaggage(Map<String, ? extends Entry> entries, Baggage parent) {
this.entries =
Collections.unmodifiableMap(new HashMap<>(Objects.requireNonNull(entries, "entries")));
this.parent = parent;
@Override
protected abstract List<Object> data();
@Override
public Baggage.Builder toBuilder() {
return new ImmutableBaggage.Builder(new ArrayList<>(data()));
}
}
public static Baggage.Builder builder() {
return new Builder();
}
@Override
public int size() {
// TODO(anuraaga): Optimize this
BaggageCounter counter = new BaggageCounter();
forEach(counter);
return counter.count;
}
private static class BaggageCounter implements BaggageConsumer {
private int count = 0;
@Override
public void accept(String key, String value, EntryMetadata metadata) {
count++;
}
}
@Override
public void forEach(BaggageConsumer consumer) {
Set<String> consumedKeys = new HashSet<>(entries.size());
entries.forEach(
(key, entry) -> {
consumedKeys.add(key);
// Skip any null entries that may have been added by Builder.remove. We already added to
// consumed keys, so even if a parent has it it won't be consumed.
if (entry == null) {
return;
}
consumer.accept(key, entry.getValue(), entry.getEntryMetadata());
});
if (parent != null) {
parent.forEach(
(key, value, metadata) -> {
if (consumedKeys.add(key)) {
consumer.accept(key, value, metadata);
}
});
for (int i = 0; i < data().size(); i += 2) {
Entry entry = (Entry) data().get(i + 1);
consumer.accept((String) data().get(i), entry.getValue(), entry.getEntryMetadata());
}
}
@Nullable
@Override
public String getEntryValue(String entryKey) {
Entry entry = entries.get(entryKey);
if (entry != null) {
return entry.getValue();
} else {
return parent == null ? null : parent.getEntryValue(entryKey);
}
Entry entry = get(entryKey);
return entry != null ? entry.getValue() : null;
}
@Override
public Baggage.Builder toBuilder() {
Builder builder = new Builder();
builder.entries.putAll(entries);
builder.parent = parent;
Builder builder = new Builder(data());
builder.noImplicitParent = true;
return builder;
}
@Override
public boolean equals(Object o) {
if (this == o) {
return true;
}
if (!(o instanceof ImmutableBaggage)) {
return false;
}
ImmutableBaggage baggage = (ImmutableBaggage) o;
if (!entries.equals(baggage.entries)) {
return false;
}
return Objects.equals(parent, baggage.parent);
}
@Override
public int hashCode() {
int result = entries.hashCode();
result = 31 * result + (parent != null ? parent.hashCode() : 0);
return result;
}
@Override
public String toString() {
return "ImmutableBaggage{" + "entries=" + entries + ", parent=" + parent + '}';
private static Baggage sortAndFilterToBaggage(Object[] data) {
return new AutoValue_ImmutableBaggage_ArrayBackedBaggage(
sortAndFilter(data, /* filterNullValues= */ true));
}
// TODO: Migrate to AutoValue.Builder
@ -137,16 +71,19 @@ class ImmutableBaggage implements Baggage {
@Nullable private Baggage parent;
private boolean noImplicitParent;
private final Map<String, Entry> entries;
private final List<Object> data;
/** Create a new empty Baggage builder. */
Builder() {
this.entries = new HashMap<>();
this.data = new ArrayList<>();
}
Builder(List<Object> data) {
this.data = new ArrayList<>(data);
}
@Override
public Baggage.Builder setParent(Context context) {
Objects.requireNonNull(context, "context");
requireNonNull(context, "context");
parent = Baggage.fromContext(context);
return this;
}
@ -160,38 +97,48 @@ class ImmutableBaggage implements Baggage {
@Override
public Baggage.Builder put(String key, String value, EntryMetadata entryMetadata) {
entries.put(
Objects.requireNonNull(key, "key"),
Entry.create(
key,
Objects.requireNonNull(value, "value"),
Objects.requireNonNull(entryMetadata, "entryMetadata")));
requireNonNull(key, "key");
requireNonNull(value, "value");
requireNonNull(entryMetadata, "entryMetadata");
data.add(key);
data.add(Entry.create(key, value, entryMetadata));
return this;
}
@Override
public Baggage.Builder put(String key, String value) {
entries.put(
Objects.requireNonNull(key, "key"),
Entry.create(key, Objects.requireNonNull(value, "value"), EntryMetadata.EMPTY));
return this;
requireNonNull(key, "key");
requireNonNull(value, "value");
return put(key, value, EntryMetadata.EMPTY);
}
@Override
public Baggage.Builder remove(String key) {
entries.remove(Objects.requireNonNull(key, "key"));
if (parent != null && parent.getEntryValue(key) != null) {
entries.put(key, null);
}
requireNonNull(key, "key");
data.add(key);
data.add(null);
return this;
}
@Override
public ImmutableBaggage build() {
public Baggage build() {
if (parent == null && !noImplicitParent) {
parent = Baggage.current();
}
return new ImmutableBaggage(entries, parent);
List<Object> data = this.data;
if (parent != null && !parent.isEmpty()) {
List<Object> merged = new ArrayList<>(parent.size() * 2 + data.size());
parent.forEach(
(key, value, metadata) -> {
merged.add(key);
merged.add(Entry.create(key, value, metadata));
});
merged.addAll(data);
data = merged;
}
return sortAndFilterToBaggage(data.toArray());
}
}
}

View File

@ -15,6 +15,7 @@ import static io.opentelemetry.api.common.AttributeKey.stringArrayKey;
import static io.opentelemetry.api.common.AttributeKey.stringKey;
import com.google.auto.value.AutoValue;
import io.opentelemetry.api.internal.ImmutableKeyValuePairs;
import java.util.ArrayList;
import java.util.Arrays;
import java.util.List;
@ -42,7 +43,7 @@ public abstract class Attributes extends ImmutableKeyValuePairs<AttributeKey, Ob
ArrayBackedAttributes() {}
@Override
abstract List<Object> data();
protected abstract List<Object> data();
@Override
public Builder toBuilder() {
@ -171,11 +172,9 @@ public abstract class Attributes extends ImmutableKeyValuePairs<AttributeKey, Ob
if (key != null && (key.getKey() == null || "".equals(key.getKey()))) {
data[i] = null;
}
if (data[i + 1] == null) {
data[i] = null;
}
}
return new AutoValue_Attributes_ArrayBackedAttributes(sortAndFilter(data));
return new AutoValue_Attributes_ArrayBackedAttributes(
sortAndFilter(data, /* filterNullValues= */ true));
}
/** Returns a new {@link Builder} instance for creating arbitrary {@link Attributes}. */

View File

@ -6,6 +6,7 @@
package io.opentelemetry.api.common;
import com.google.auto.value.AutoValue;
import io.opentelemetry.api.internal.ImmutableKeyValuePairs;
import java.util.ArrayList;
import java.util.List;
import java.util.function.BiConsumer;
@ -25,7 +26,7 @@ public abstract class Labels extends ImmutableKeyValuePairs<String, String> {
ArrayBackedLabels() {}
@Override
abstract List<Object> data();
protected abstract List<Object> data();
@Override
public void forEach(BiConsumer<String, String> consumer) {
@ -107,7 +108,8 @@ public abstract class Labels extends ImmutableKeyValuePairs<String, String> {
}
private static Labels sortAndFilterToLabels(Object... data) {
return new AutoValue_Labels_ArrayBackedLabels(sortAndFilter(data));
return new AutoValue_Labels_ArrayBackedLabels(
sortAndFilter(data, /* filterNullValues= */ false));
}
/** Create a {@link Builder} pre-populated with the contents of this Labels instance. */

View File

@ -3,10 +3,12 @@
* SPDX-License-Identifier: Apache-2.0
*/
package io.opentelemetry.api.common;
package io.opentelemetry.api.internal;
import static io.opentelemetry.api.internal.Utils.checkArgument;
import io.opentelemetry.api.common.Attributes;
import io.opentelemetry.api.common.Labels;
import java.util.ArrayList;
import java.util.Collections;
import java.util.List;
@ -14,13 +16,13 @@ import javax.annotation.Nullable;
import javax.annotation.concurrent.Immutable;
/**
* An immutable set of key-value pairs. Keys are only {@link String} typed.
* An immutable set of key-value pairs.
*
* <p>Key-value pairs are dropped for {@code null} or empty keys.
*
* <p>Note: for subclasses of this, null keys will be removed, but if your key has another concept
* of being "empty", you'll need to remove them before calling {@link #sortAndFilter(Object[])},
* assuming you don't want the "empty" keys to be kept in your collection.
* of being "empty", you'll need to remove them before calling {@link #sortAndFilter(Object[],
* boolean)}, assuming you don't want the "empty" keys to be kept in your collection.
*
* @param <V> The type of the values contained in this.
* @see Labels
@ -28,9 +30,9 @@ import javax.annotation.concurrent.Immutable;
*/
@SuppressWarnings("rawtypes")
@Immutable
abstract class ImmutableKeyValuePairs<K, V> {
public abstract class ImmutableKeyValuePairs<K, V> {
List<Object> data() {
protected List<Object> data() {
return Collections.emptyList();
}
@ -42,6 +44,7 @@ abstract class ImmutableKeyValuePairs<K, V> {
return data().isEmpty();
}
/** Returns the value for the given {@code key}, or {@code null} if the key is not present. */
@Nullable
@SuppressWarnings("unchecked")
public V get(K key) {
@ -53,13 +56,17 @@ abstract class ImmutableKeyValuePairs<K, V> {
return null;
}
/**
* Sorts and dedupes the key/value pairs in {@code data}. If {@code filterNullValues} is {@code
* true}, {@code null} values will be removed.
*/
@SuppressWarnings("unchecked")
static List<Object> sortAndFilter(Object[] data) {
public static List<Object> sortAndFilter(Object[] data, boolean filterNullValues) {
checkArgument(
data.length % 2 == 0, "You must provide an even number of key/value pair arguments.");
quickSort(data, 0, data.length - 2);
return dedupe(data);
return dedupe(data, filterNullValues);
}
@SuppressWarnings("unchecked")
@ -95,7 +102,7 @@ abstract class ImmutableKeyValuePairs<K, V> {
return key.compareTo(pivotKey);
}
private static List<Object> dedupe(Object[] data) {
private static List<Object> dedupe(Object[] data, boolean filterNullValues) {
List<Object> result = new ArrayList<>(data.length);
Object previousKey = null;
@ -110,6 +117,9 @@ abstract class ImmutableKeyValuePairs<K, V> {
continue;
}
previousKey = key;
if (filterNullValues && value == null) {
continue;
}
// add them in reverse order, because we'll reverse the list before returning,
// to preserve insertion order.
result.add(value);

View File

@ -194,6 +194,10 @@ class ImmutableBaggageTest {
@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 baggage3 = Baggage.builder().put(K1, V2).build();
new EqualsTester()
.addEqualityGroup(
Baggage.builder().put(K1, V1, TMD).put(K2, V2, TMD).build(),
@ -201,6 +205,7 @@ class ImmutableBaggageTest {
Baggage.builder().put(K2, V2, TMD).put(K1, V1, TMD).build())
.addEqualityGroup(Baggage.builder().put(K1, V1, TMD).put(K2, V1, TMD).build())
.addEqualityGroup(Baggage.builder().put(K1, V2, TMD).put(K2, V1, TMD).build())
.addEqualityGroup(baggage2, baggage3)
.testEquals();
}
}

View File

@ -181,6 +181,6 @@ class W3CBaggagePropagatorTest {
assertThat(carrier)
.containsExactlyInAnyOrderEntriesOf(
singletonMap(
"baggage", "nometa=nometa-value,meta=meta-value;somemetadata; someother=foo"));
"baggage", "meta=meta-value;somemetadata; someother=foo,nometa=nometa-value"));
}
}

View File

@ -3,7 +3,7 @@
* SPDX-License-Identifier: Apache-2.0
*/
package io.opentelemetry.api.common;
package io.opentelemetry.api.internal;
import static org.assertj.core.api.Assertions.assertThat;
@ -33,7 +33,7 @@ class ImmutableKeyValuePairsTest {
}
@Override
List<Object> data() {
protected List<Object> data() {
return data;
}
}