diff --git a/dd-java-agent/instrumentation-testing/src/main/groovy/datadog/trace/agent/test/TrackingSpanDecorator.groovy b/dd-java-agent/instrumentation-testing/src/main/groovy/datadog/trace/agent/test/TrackingSpanDecorator.groovy index 70f1a2a6731..7db4f70a272 100644 --- a/dd-java-agent/instrumentation-testing/src/main/groovy/datadog/trace/agent/test/TrackingSpanDecorator.groovy +++ b/dd-java-agent/instrumentation-testing/src/main/groovy/datadog/trace/agent/test/TrackingSpanDecorator.groovy @@ -114,6 +114,11 @@ class TrackingSpanDecorator implements AgentSpan { return delegate.setTag(key, value) } + @Override + AgentSpan setTag(TagMap.EntryReader entry) { + return delegate.setTag(entry) + } + @Override void setRequestBlockingAction(Flow.Action.RequestBlockingAction rba) { delegate.setRequestBlockingAction(rba) @@ -184,6 +189,11 @@ class TrackingSpanDecorator implements AgentSpan { return delegate.setMetric(key, value) } + @Override + AgentSpan setMetric(TagMap.EntryReader entry) { + return delegate.setMetric(entry) + } + @Override boolean isError() { return delegate.isError() diff --git a/dd-trace-core/src/main/java/datadog/trace/core/DDSpan.java b/dd-trace-core/src/main/java/datadog/trace/core/DDSpan.java index 1b321f4536a..e8a6ce87edb 100644 --- a/dd-trace-core/src/main/java/datadog/trace/core/DDSpan.java +++ b/dd-trace-core/src/main/java/datadog/trace/core/DDSpan.java @@ -392,7 +392,7 @@ private boolean isExceptionReplayEnabled() { public final DDSpan setTag(final String tag, final String value) { if (value == null || value.isEmpty()) { // Remove the tag - context.setTag(tag, null); + context.removeTag(tag); } else { context.setTag(tag, value); } @@ -405,6 +405,12 @@ public final DDSpan setTag(final String tag, final boolean value) { return this; } + @Override + public final DDSpan setTag(TagMap.EntryReader entry) { + context.setTag(entry); + return this; + } + @Override public void setRequestBlockingAction(Flow.Action.RequestBlockingAction rba) { this.requestBlockingAction = rba; @@ -474,6 +480,12 @@ public DDSpan setMetric(final CharSequence metric, final double value) { return this; } + @Override + public DDSpan setMetric(TagMap.EntryReader entry) { + context.setMetric(entry); + return this; + } + @Override public DDSpan setFlag(CharSequence name, boolean value) { context.setMetric(name, value ? 1 : 0); diff --git a/dd-trace-core/src/main/java/datadog/trace/core/DDSpanContext.java b/dd-trace-core/src/main/java/datadog/trace/core/DDSpanContext.java index 96d6292c6ae..ee7901fa2a4 100644 --- a/dd-trace-core/src/main/java/datadog/trace/core/DDSpanContext.java +++ b/dd-trace-core/src/main/java/datadog/trace/core/DDSpanContext.java @@ -751,6 +751,18 @@ public void setMetric(final CharSequence key, final Number value) { } } + public void setMetric(final TagMap.EntryReader entry) { + synchronized (unsafeTags) { + unsafeTags.set(entry); + } + } + + public void removeTag(String tag) { + synchronized (unsafeTags) { + unsafeTags.remove(tag); + } + } + /** * Sets a tag to the span. Tags are not propagated to the children. * @@ -790,6 +802,20 @@ public void setTag(final String tag, final String value) { } } + public void setTag(TagMap.EntryReader entry) { + if (entry == null) return; + + // prcheck to avoid boxing + boolean intercepted = + precheckIntercept(entry.tag()) + && tagInterceptor.interceptTag(this, entry.tag(), entry.objectValue()); + if (!intercepted) { + synchronized (unsafeTags) { + unsafeTags.set(entry); + } + } + } + /* * Uses to determine if there's an opportunity to avoid primitve boxing. * If the underlying map doesn't support efficient primitives, then boxing is used. diff --git a/dd-trace-core/src/test/java/datadog/trace/core/SpanTest.java b/dd-trace-core/src/test/java/datadog/trace/core/SpanTest.java new file mode 100644 index 00000000000..628efa3fdd4 --- /dev/null +++ b/dd-trace-core/src/test/java/datadog/trace/core/SpanTest.java @@ -0,0 +1,27 @@ +package datadog.trace.core; + +import static org.junit.jupiter.api.Assertions.assertEquals; + +import datadog.trace.api.TagMap; +import datadog.trace.bootstrap.instrumentation.api.AgentSpan; +import org.junit.jupiter.api.Test; + +public class SpanTest { + static final CoreTracer TRACER = CoreTracer.builder().build(); + + @Test + public void setTag_Entry() { + AgentSpan span = TRACER.startSpan("foo", "foo"); + span.setTag(TagMap.Entry.create("message", "hello")); + + assertEquals("hello", span.getTag("message")); + } + + @Test + public void setMetric_Entry() { + AgentSpan span = TRACER.startSpan("foo", "foo"); + span.setMetric(TagMap.Entry.create("metric", 20L)); + + assertEquals(Long.valueOf(20L), span.getTag("metric")); + } +} diff --git a/internal-api/src/main/java/datadog/trace/api/TagMap.java b/internal-api/src/main/java/datadog/trace/api/TagMap.java index dbbc19e5bba..15b2a557039 100644 --- a/internal-api/src/main/java/datadog/trace/api/TagMap.java +++ b/internal-api/src/main/java/datadog/trace/api/TagMap.java @@ -369,6 +369,37 @@ final class Entry extends EntryChange implements Map.Entry, Entr */ static final byte ANY = 0; + public static final Entry create(String tag, Object value) { + // NOTE: From the static typing, it is possible that value is a primitive box, so need to call + // Any variant + return TagMap.Entry.newAnyEntry(tag, value); + } + + public static final Entry create(String tag, CharSequence value) { + // NOTE: From the static typing, we know that value is not a primitive box + return TagMap.Entry.newObjectEntry(tag, value); + } + + public static final Entry create(String tag, boolean value) { + return TagMap.Entry.newBooleanEntry(tag, value); + } + + public static final Entry create(String tag, int value) { + return TagMap.Entry.newIntEntry(tag, value); + } + + public static final Entry create(String tag, long value) { + return TagMap.Entry.newLongEntry(tag, value); + } + + public static final Entry create(String tag, float value) { + return TagMap.Entry.newFloatEntry(tag, value); + } + + public static final Entry create(String tag, double value) { + return TagMap.Entry.newDoubleEntry(tag, value); + } + static Entry newAnyEntry(Map.Entry entry) { return newAnyEntry(entry.getKey(), entry.getValue()); } diff --git a/internal-api/src/main/java/datadog/trace/bootstrap/instrumentation/api/AgentSpan.java b/internal-api/src/main/java/datadog/trace/bootstrap/instrumentation/api/AgentSpan.java index 5deb8520d3a..bfbb2c63108 100644 --- a/internal-api/src/main/java/datadog/trace/bootstrap/instrumentation/api/AgentSpan.java +++ b/internal-api/src/main/java/datadog/trace/bootstrap/instrumentation/api/AgentSpan.java @@ -77,6 +77,8 @@ default boolean isValid() { AgentSpan setTag(String key, Object value); + AgentSpan setTag(TagMap.EntryReader entry); + AgentSpan setAllTags(Map map); @Override @@ -91,6 +93,8 @@ default boolean isValid() { @Override AgentSpan setMetric(CharSequence key, double value); + AgentSpan setMetric(TagMap.EntryReader metricEntry); + @Override AgentSpan setSpanType(final CharSequence type); diff --git a/internal-api/src/main/java/datadog/trace/bootstrap/instrumentation/api/ImmutableSpan.java b/internal-api/src/main/java/datadog/trace/bootstrap/instrumentation/api/ImmutableSpan.java index a7dcab6cebf..1f857a6e7df 100644 --- a/internal-api/src/main/java/datadog/trace/bootstrap/instrumentation/api/ImmutableSpan.java +++ b/internal-api/src/main/java/datadog/trace/bootstrap/instrumentation/api/ImmutableSpan.java @@ -1,5 +1,6 @@ package datadog.trace.bootstrap.instrumentation.api; +import datadog.trace.api.TagMap; import datadog.trace.api.gateway.Flow.Action.RequestBlockingAction; import datadog.trace.api.interceptor.MutableSpan; import java.util.Map; @@ -41,6 +42,11 @@ public AgentSpan setTag(String key, String value) { return this; } + @Override + public AgentSpan setTag(TagMap.EntryReader entry) { + return this; + } + @Override public AgentSpan setTag(String key, CharSequence value) { return this; @@ -76,6 +82,11 @@ public AgentSpan setMetric(CharSequence key, double value) { return this; } + @Override + public AgentSpan setMetric(TagMap.EntryReader entry) { + return this; + } + @Override public AgentSpan setSpanType(CharSequence type) { return this; diff --git a/internal-api/src/test/java/datadog/trace/api/TagMapEntryTest.java b/internal-api/src/test/java/datadog/trace/api/TagMapEntryTest.java index 13da8723b60..b5bb8dae2dd 100644 --- a/internal-api/src/test/java/datadog/trace/api/TagMapEntryTest.java +++ b/internal-api/src/test/java/datadog/trace/api/TagMapEntryTest.java @@ -8,7 +8,9 @@ import datadog.trace.api.TagMap.Entry; import java.util.ArrayList; import java.util.Collections; +import java.util.HashMap; import java.util.List; +import java.util.Map; import java.util.concurrent.Callable; import java.util.concurrent.ExecutionException; import java.util.concurrent.ExecutorService; @@ -48,6 +50,21 @@ public void isNumericPrimitive() { assertTrue(TagMap.Entry._isNumericPrimitive(TagMap.Entry.DOUBLE)); } + @Test + public void objectEntry_via_create() { + test( + () -> TagMap.Entry.create("foo", "bar"), + TagMap.Entry.OBJECT, + (entry) -> + multiCheck( + checkKey("foo", entry), + checkValue("bar", entry), + checkEquals("bar", entry::stringValue), + checkFalse(entry::isNumber), + checkTrue(entry::isObject), + checkType(TagMap.Entry.OBJECT, entry))); + } + @Test public void objectEntry() { test( @@ -58,8 +75,25 @@ public void objectEntry() { checkKey("foo", entry), checkValue("bar", entry), checkEquals("bar", entry::stringValue), + checkFalse(entry::isNumber), checkTrue(entry::isObject), - checkFalse(entry::isNumber))); + checkType(TagMap.Entry.OBJECT, entry))); + } + + @Test + public void anyEntry_via_create() { + Map map = new HashMap<>(); + + test( + () -> TagMap.Entry.create("foo", map), + TagMap.Entry.ANY, + (entry) -> + multiCheck( + checkKey("foo", entry), + checkValue(map, entry), + checkTrue(entry::isObject), + checkFalse(entry::isNumber), + checkType(TagMap.Entry.OBJECT, entry))); } @Test @@ -73,15 +107,14 @@ public void anyEntry_object() { checkValue("bar", entry), checkTrue(entry::isObject), checkFalse(entry::isNumber), - checkKey("foo", entry), - checkValue("bar", entry))); + checkType(TagMap.EntryReader.OBJECT, entry))); } @ParameterizedTest @ValueSource(booleans = {false, true}) - public void booleanEntry(boolean value) { + public void booleanEntry_via_create(boolean value) { test( - () -> TagMap.Entry.newBooleanEntry("foo", value), + () -> TagMap.Entry.create("foo", value), TagMap.Entry.BOOLEAN, (entry) -> multiCheck( @@ -126,6 +159,21 @@ public void anyEntry_boolean(boolean value) { checkValue(value, entry))); } + @ParameterizedTest + @ValueSource(ints = {Integer.MIN_VALUE, -256, -128, -1, 0, 1, 128, 256, Integer.MAX_VALUE}) + public void intEntry_via_create(int value) { + test( + () -> TagMap.Entry.create("foo", value), + TagMap.Entry.INT, + (entry) -> + multiCheck( + checkKey("foo", entry), + checkValue(value, entry), + checkIsNumericPrimitive(entry), + checkInstanceOf(Number.class, entry), + checkType(TagMap.Entry.INT, entry))); + } + @ParameterizedTest @ValueSource(ints = {Integer.MIN_VALUE, -256, -128, -1, 0, 1, 128, 256, Integer.MAX_VALUE}) public void intEntry(int value) { @@ -202,6 +250,35 @@ public void anyEntry_int(int value) { checkValue(value, entry))); } + @ParameterizedTest + @ValueSource( + longs = { + Long.MIN_VALUE, + Integer.MIN_VALUE, + -1_048_576L, + -256L, + -128L, + -1L, + 0L, + 1L, + 128L, + 256L, + 1_048_576L, + Integer.MAX_VALUE, + Long.MAX_VALUE + }) + public void longEntry_via_create(long value) { + test( + () -> TagMap.Entry.create("foo", value), + TagMap.Entry.LONG, + (entry) -> + multiCheck( + checkKey("foo", entry), + checkValue(value, entry), + checkTrue(entry::isNumericPrimitive), + checkType(TagMap.Entry.LONG, entry))); + } + @ParameterizedTest @ValueSource( longs = { @@ -290,6 +367,20 @@ public void anyEntry_long(long value) { checkValue(value, entry))); } + @ParameterizedTest + @ValueSource(floats = {Float.MIN_VALUE, -1F, 0F, 1F, 2.171828F, 3.1415F, Float.MAX_VALUE}) + public void floatEntry_via_create(float value) { + test( + () -> TagMap.Entry.create("foo", value), + TagMap.Entry.FLOAT, + (entry) -> + multiCheck( + checkKey("foo", entry), + checkValue(value, entry), + checkTrue(entry::isNumericPrimitive), + checkType(TagMap.Entry.FLOAT, entry))); + } + @ParameterizedTest @ValueSource(floats = {Float.MIN_VALUE, -1F, 0F, 1F, 2.171828F, 3.1415F, Float.MAX_VALUE}) public void floatEntry(float value) { @@ -332,6 +423,21 @@ public void anyEntry_float(float value) { checkType(TagMap.Entry.FLOAT, entry))); } + @ParameterizedTest + @ValueSource( + doubles = {Double.MIN_VALUE, Float.MIN_VALUE, -1D, 0D, 1D, Math.E, Math.PI, Double.MAX_VALUE}) + public void doubleEntry_via_create(double value) { + test( + () -> TagMap.Entry.create("foo", value), + TagMap.Entry.DOUBLE, + (entry) -> + multiCheck( + checkKey("foo", entry), + checkValue(value, entry), + checkIsNumericPrimitive(entry), + checkType(TagMap.Entry.DOUBLE, entry))); + } + @ParameterizedTest @ValueSource( doubles = {Double.MIN_VALUE, Float.MIN_VALUE, -1D, 0D, 1D, Math.E, Math.PI, Double.MAX_VALUE}) @@ -422,7 +528,7 @@ static final void test( static final void testSingleThreaded( Supplier entrySupplier, byte rawType, Function checkSupplier) { Entry entry = entrySupplier.get(); - assertEquals(rawType, entry.rawType); + assertEquals(rawType, entry.rawType, "rawType"); Check checks = checkSupplier.apply(entry); checks.check(); @@ -431,7 +537,7 @@ static final void testSingleThreaded( static final void testMultiThreaded( Supplier entrySupplier, byte rawType, Function checkSupplier) { Entry sharedEntry = entrySupplier.get(); - assertEquals(rawType, sharedEntry.rawType); + assertEquals(rawType, sharedEntry.rawType, "rawType"); Check checks = checkSupplier.apply(sharedEntry); @@ -716,8 +822,8 @@ static final Check checkInstanceOf(Class klass, TagMap.Entry entry) { static final Check checkType(byte entryType, TagMap.Entry entry) { // TODO: TVC checks return multiCheck( - checkTrue(() -> entry.is(entryType), "type is " + entryType), - checkEquals(entryType, entry::type)); + checkTrue(() -> entry.is(entryType), "Entry::is(type) type=" + entryType), + checkEquals(entryType, entry::type, "Entry::type check")); } static final Check multiCheck(Check... checks) {