diff --git a/api/all/src/main/java/io/opentelemetry/api/baggage/Baggage.java b/api/all/src/main/java/io/opentelemetry/api/baggage/Baggage.java index 5fb51b7b7d..5dd2924d43 100644 --- a/api/all/src/main/java/io/opentelemetry/api/baggage/Baggage.java +++ b/api/all/src/main/java/io/opentelemetry/api/baggage/Baggage.java @@ -79,7 +79,7 @@ public interface Baggage extends ImplicitContextKeyed { } /** Iterates over all the entries in this {@link Baggage}. */ - void forEach(BiConsumer consumer); + void forEach(BiConsumer consumer); /** Returns a read-only view of this {@link Baggage} as a {@link Map}. */ Map asMap(); diff --git a/api/all/src/main/java/io/opentelemetry/api/common/Attributes.java b/api/all/src/main/java/io/opentelemetry/api/common/Attributes.java index bdf44f0ec9..db59fc34c4 100644 --- a/api/all/src/main/java/io/opentelemetry/api/common/Attributes.java +++ b/api/all/src/main/java/io/opentelemetry/api/common/Attributes.java @@ -36,7 +36,7 @@ public interface Attributes { T get(AttributeKey key); /** Iterates over all the key-value pairs of attributes contained by this instance. */ - void forEach(BiConsumer, Object> consumer); + void forEach(BiConsumer, ? super Object> consumer); /** The number of attributes contained in this. */ int size(); diff --git a/api/all/src/main/java/io/opentelemetry/api/internal/ImmutableKeyValuePairs.java b/api/all/src/main/java/io/opentelemetry/api/internal/ImmutableKeyValuePairs.java index ae50bb6d71..1a2e93c074 100644 --- a/api/all/src/main/java/io/opentelemetry/api/internal/ImmutableKeyValuePairs.java +++ b/api/all/src/main/java/io/opentelemetry/api/internal/ImmutableKeyValuePairs.java @@ -81,7 +81,7 @@ public abstract class ImmutableKeyValuePairs { /** Iterates over all the key-value pairs of labels contained by this instance. */ @SuppressWarnings("unchecked") - public final void forEach(BiConsumer consumer) { + public final void forEach(BiConsumer consumer) { if (consumer == null) { return; } diff --git a/api/metrics/src/main/java/io/opentelemetry/api/metrics/common/Labels.java b/api/metrics/src/main/java/io/opentelemetry/api/metrics/common/Labels.java index 275ce552f3..8821618ff2 100644 --- a/api/metrics/src/main/java/io/opentelemetry/api/metrics/common/Labels.java +++ b/api/metrics/src/main/java/io/opentelemetry/api/metrics/common/Labels.java @@ -102,7 +102,7 @@ public interface Labels { } /** Iterates over all the key-value pairs of labels contained by this instance. */ - void forEach(BiConsumer consumer); + void forEach(BiConsumer consumer); /** The number of key-value pairs of labels in this instance. */ int size(); diff --git a/build.gradle.kts b/build.gradle.kts index d4a8ec3bd7..32e19cf4d8 100644 --- a/build.gradle.kts +++ b/build.gradle.kts @@ -127,20 +127,22 @@ subprojects { withType(JavaCompile::class) { options.release.set(8) - options.compilerArgs.addAll(listOf( - "-Xlint:all", - // We suppress the "try" warning because it disallows managing an auto-closeable with - // try-with-resources without referencing the auto-closeable within the try block. - "-Xlint:-try", - // We suppress the "processing" warning as suggested in - // https://groups.google.com/forum/#!topic/bazel-discuss/_R3A9TJSoPM - "-Xlint:-processing", - // We suppress the "options" warning because it prevents compilation on modern JDKs - "-Xlint:-options", + if (name != "jmhCompileGeneratedClasses") { + options.compilerArgs.addAll(listOf( + "-Xlint:all", + // We suppress the "try" warning because it disallows managing an auto-closeable with + // try-with-resources without referencing the auto-closeable within the try block. + "-Xlint:-try", + // We suppress the "processing" warning as suggested in + // https://groups.google.com/forum/#!topic/bazel-discuss/_R3A9TJSoPM + "-Xlint:-processing", + // We suppress the "options" warning because it prevents compilation on modern JDKs + "-Xlint:-options", - // Fail build on any warning - "-Werror" - )) + // Fail build on any warning + "-Werror" + )) + } options.encoding = "UTF-8" @@ -383,6 +385,7 @@ subprojects { // Always include the jmhreport plugin and run it after jmh task. plugins.apply("io.morethan.jmhreport") dependencies { + add("jmh", platform(project(":dependencyManagement"))) add("jmh", "org.openjdk.jmh:jmh-core") add("jmh", "org.openjdk.jmh:jmh-generator-bytecode") } diff --git a/dependencyManagement/build.gradle.kts b/dependencyManagement/build.gradle.kts index 2141383526..5ca9c3d4d3 100644 --- a/dependencyManagement/build.gradle.kts +++ b/dependencyManagement/build.gradle.kts @@ -55,7 +55,7 @@ val DEPENDENCY_SETS = listOf( ), DependencySet( "org.openjdk.jmh", - "1.26", + "1.27", listOf("jmh-core", "jmh-generator-bytecode") ), DependencySet( diff --git a/sdk/trace/src/jmh/java/io/opentelemetry/sdk/trace/FillSpanBenchmark.java b/sdk/trace/src/jmh/java/io/opentelemetry/sdk/trace/FillSpanBenchmark.java new file mode 100644 index 0000000000..245b13afaa --- /dev/null +++ b/sdk/trace/src/jmh/java/io/opentelemetry/sdk/trace/FillSpanBenchmark.java @@ -0,0 +1,42 @@ +/* + * Copyright The OpenTelemetry Authors + * SPDX-License-Identifier: Apache-2.0 + */ + +package io.opentelemetry.sdk.trace; + +import io.opentelemetry.api.common.AttributeKey; +import io.opentelemetry.api.trace.SpanBuilder; +import java.util.concurrent.TimeUnit; +import org.openjdk.jmh.annotations.Benchmark; +import org.openjdk.jmh.annotations.Fork; +import org.openjdk.jmh.annotations.Measurement; +import org.openjdk.jmh.annotations.OutputTimeUnit; +import org.openjdk.jmh.annotations.Threads; +import org.openjdk.jmh.annotations.Warmup; + +@Threads(value = 1) +@Fork(3) +@Warmup(iterations = 10, time = 1) +@Measurement(iterations = 20, time = 1) +@OutputTimeUnit(TimeUnit.MILLISECONDS) +public class FillSpanBenchmark { + + private static final SpanBuilder spanBuilder = + SdkTracerProvider.builder().build().get("benchmark").spanBuilder("benchmark"); + + private static final AttributeKey KEY1 = AttributeKey.stringKey("key1"); + private static final AttributeKey KEY2 = AttributeKey.stringKey("key2"); + private static final AttributeKey KEY3 = AttributeKey.stringKey("key3"); + private static final AttributeKey KEY4 = AttributeKey.stringKey("key4"); + + @Benchmark + public void setFourAttributes() { + spanBuilder + .startSpan() + .setAttribute(KEY1, "value1") + .setAttribute(KEY2, "value2") + .setAttribute(KEY3, "value3") + .setAttribute(KEY4, "value4"); + } +} diff --git a/sdk/trace/src/main/java/io/opentelemetry/sdk/trace/AttributesMap.java b/sdk/trace/src/main/java/io/opentelemetry/sdk/trace/AttributesMap.java index 1e1fe9eea6..1fc4dd137b 100644 --- a/sdk/trace/src/main/java/io/opentelemetry/sdk/trace/AttributesMap.java +++ b/sdk/trace/src/main/java/io/opentelemetry/sdk/trace/AttributesMap.java @@ -9,40 +9,32 @@ import io.opentelemetry.api.common.AttributeKey; import io.opentelemetry.api.common.Attributes; import io.opentelemetry.api.common.AttributesBuilder; import java.util.Collections; -import java.util.LinkedHashMap; +import java.util.HashMap; import java.util.Map; import java.util.function.BiConsumer; -/** - * A map with a fixed capacity that drops attributes when the map gets full. - * - *

Note: this doesn't implement the Map interface, but behaves very similarly to one. - */ +/** A map with a fixed capacity that drops attributes when the map gets full. */ @SuppressWarnings({"rawtypes", "unchecked"}) -final class AttributesMap implements Attributes { - private final Map, Object> data; +final class AttributesMap extends HashMap, Object> implements Attributes { + + private static final long serialVersionUID = -5072696312123632376L; private final long capacity; private int totalAddedValues = 0; - private AttributesMap(long capacity, Map, Object> data) { - this.capacity = capacity; - this.data = data; - } - AttributesMap(long capacity) { - this(capacity, new LinkedHashMap<>()); + this.capacity = capacity; } - public void put(AttributeKey key, T value) { + void put(AttributeKey key, T value) { if (key == null || key.getKey() == null || value == null) { return; } totalAddedValues++; - if (data.size() >= capacity && !data.containsKey(key)) { + if (size() >= capacity && !containsKey(key)) { return; } - data.put(key, value); + super.put(key, value); } int getTotalAddedValues() { @@ -52,32 +44,27 @@ final class AttributesMap implements Attributes { @SuppressWarnings("unchecked") @Override public T get(AttributeKey key) { - return (T) data.get(key); - } - - @Override - public int size() { - return data.size(); - } - - @Override - public boolean isEmpty() { - return data.isEmpty(); + return (T) super.get(key); } @SuppressWarnings({"rawtypes", "unchecked"}) @Override - public void forEach(BiConsumer, Object> consumer) { - for (Map.Entry, Object> entry : data.entrySet()) { + public void forEach(BiConsumer, ? super Object> consumer) { + for (Map.Entry, Object> entry : entrySet()) { AttributeKey key = entry.getKey(); Object value = entry.getValue(); - consumer.accept(key, value); + consumer.accept((AttributeKey) key, value); } } @Override public Map, Object> asMap() { - return Collections.unmodifiableMap(data); + // Because Attributes is marked Immutable, IDEs may recognize this as redundant usage. However, + // this class is private and is actually mutable, so we need to wrap with unmodifiableMap + // anyways. We implement the immutable Attributes for this class to support the + // Attributes.builder().putAll usage - it is tricky but an implementation detail of this private + // class. + return Collections.unmodifiableMap(this); } @Override @@ -89,7 +76,7 @@ final class AttributesMap implements Attributes { public String toString() { return "AttributesMap{" + "data=" - + data + + super.toString() + ", capacity=" + capacity + ", totalAddedValues=" @@ -98,7 +85,6 @@ final class AttributesMap implements Attributes { } Attributes immutableCopy() { - Map, Object> dataCopy = new LinkedHashMap<>(data); - return new AttributesMap(capacity, Collections.unmodifiableMap(dataCopy)); + return Attributes.builder().putAll(this).build(); } } diff --git a/sdk/trace/src/test/java/io/opentelemetry/sdk/trace/AttributesMapTest.java b/sdk/trace/src/test/java/io/opentelemetry/sdk/trace/AttributesMapTest.java index 8c1a15736b..7f23afb8e6 100644 --- a/sdk/trace/src/test/java/io/opentelemetry/sdk/trace/AttributesMapTest.java +++ b/sdk/trace/src/test/java/io/opentelemetry/sdk/trace/AttributesMapTest.java @@ -9,29 +9,9 @@ import static io.opentelemetry.api.common.AttributeKey.longKey; import static org.assertj.core.api.Assertions.assertThat; import static org.assertj.core.api.Assertions.entry; -import io.opentelemetry.api.common.AttributeKey; -import io.opentelemetry.api.common.Attributes; -import java.util.Arrays; -import java.util.List; -import java.util.function.BiConsumer; import org.junit.jupiter.api.Test; class AttributesMapTest { - @Test - void attributesAreInOrder() { - AttributesMap attributesMap = new AttributesMap(5); - attributesMap.put(longKey("one"), 1L); - attributesMap.put(longKey("three"), 3L); - attributesMap.put(longKey("two"), 2L); - attributesMap.put(longKey("four"), 4L); - attributesMap.put(longKey("five"), 5L); - - List expectedKeyOrder = Arrays.asList("one", "three", "two", "four", "five"); - List expectedValueOrder = Arrays.asList(1L, 3L, 2L, 4L, 5L); - - assertOrdering(attributesMap, expectedKeyOrder, expectedValueOrder); - assertOrdering(attributesMap.immutableCopy(), expectedKeyOrder, expectedValueOrder); - } @Test void asMap() { @@ -40,22 +20,6 @@ class AttributesMapTest { attributesMap.put(longKey("two"), 2L); assertThat(attributesMap.asMap()) - .containsExactly(entry(longKey("one"), 1L), entry(longKey("two"), 2L)); - } - - private void assertOrdering( - Attributes attributes, List expectedKeyOrder, List expectedValueOrder) { - attributes.forEach( - new BiConsumer, Object>() { - private int counter = 0; - - @Override - public void accept(AttributeKey key, Object value) { - String k = key.getKey(); - Long val = (Long) value; - assertThat(val).isEqualTo(expectedValueOrder.get(counter)); - assertThat(k).isEqualTo(expectedKeyOrder.get(counter++)); - } - }); + .containsOnly(entry(longKey("one"), 1L), entry(longKey("two"), 2L)); } } diff --git a/sdk/trace/src/test/java/io/opentelemetry/sdk/trace/SdkSpanBuilderTest.java b/sdk/trace/src/test/java/io/opentelemetry/sdk/trace/SdkSpanBuilderTest.java index e7bb721451..f45c28e768 100644 --- a/sdk/trace/src/test/java/io/opentelemetry/sdk/trace/SdkSpanBuilderTest.java +++ b/sdk/trace/src/test/java/io/opentelemetry/sdk/trace/SdkSpanBuilderTest.java @@ -875,8 +875,7 @@ class SdkSpanBuilderTest { + "kind=INTERNAL, " + "startEpochNanos=[0-9]+, " + "endEpochNanos=[0-9]+, " - + "attributes=AttributesMap\\{data=\\{http.status_code=500, " - + "http.url=https://opentelemetry.io}, capacity=128, totalAddedValues=2}, " + + "attributes=AttributesMap\\{data=\\{[^}]*}, capacity=128, totalAddedValues=2}, " + "totalAttributeCount=2, " + "events=\\[], " + "totalRecordedEvents=0, " diff --git a/settings.gradle.kts b/settings.gradle.kts index 90637aeddc..0d7414e03a 100644 --- a/settings.gradle.kts +++ b/settings.gradle.kts @@ -8,7 +8,7 @@ pluginManagement { id("de.undercouch.download") version "4.1.1" id("io.codearte.nexus-staging") version "0.22.0" id("io.morethan.jmhreport") version "0.9.0" - id("me.champeau.gradle.jmh") version "0.5.2" + id("me.champeau.gradle.jmh") version "0.5.3" id("nebula.release") version "15.3.0" id("net.ltgt.errorprone") version "1.3.0" id("org.jetbrains.kotlin.jvm") version "1.4.21"