diff --git a/dd-trace-core/src/jmh/java/datadog/trace/core/SetTagsFromBenchmark.java b/dd-trace-core/src/jmh/java/datadog/trace/core/SetTagsFromBenchmark.java new file mode 100644 index 00000000000..f7fc315872e --- /dev/null +++ b/dd-trace-core/src/jmh/java/datadog/trace/core/SetTagsFromBenchmark.java @@ -0,0 +1,147 @@ +package datadog.trace.core; + +import static java.util.concurrent.TimeUnit.NANOSECONDS; + +import datadog.trace.api.KnownTags; +import datadog.trace.bootstrap.instrumentation.api.SpanData; +import datadog.trace.bootstrap.instrumentation.api.TagContributor; +import datadog.trace.bootstrap.instrumentation.api.TagExtractor; +import datadog.trace.bootstrap.instrumentation.api.Tags; +import org.openjdk.jmh.annotations.Benchmark; +import org.openjdk.jmh.annotations.BenchmarkMode; +import org.openjdk.jmh.annotations.Fork; +import org.openjdk.jmh.annotations.Measurement; +import org.openjdk.jmh.annotations.Mode; +import org.openjdk.jmh.annotations.OutputTimeUnit; +import org.openjdk.jmh.annotations.Scope; +import org.openjdk.jmh.annotations.Setup; +import org.openjdk.jmh.annotations.State; +import org.openjdk.jmh.annotations.TearDown; +import org.openjdk.jmh.annotations.Warmup; +import org.openjdk.jmh.infra.Blackhole; + +/** + * Measures {@code DDSpan.setTagsFrom}, which takes the span's tag lock once for a whole batch, + * against the same tags set one {@code setTag} call (and one lock) at a time. + * + *

The batch is a jdbc span's nine tags: eight known tags by id, among them an int ({@code + * peer.port}) and an intercepted tag ({@code span.kind}), and one custom tag. They come from a + * {@code DbInfo}-style POJO, the case {@link TagContributor} is for, or through a {@link + * TagExtractor}. + * + *

+ * + *

Run with {@code -prof gc} to check that the batch's short-lived {@code SpanData} is + * scalar-replaced: the batched arms should allocate no more than the one-by-one arms. + */ +@State(Scope.Thread) +@Warmup(iterations = 3, time = 1) +@Measurement(iterations = 5, time = 1) +@BenchmarkMode(Mode.AverageTime) +@OutputTimeUnit(NANOSECONDS) +@Fork(value = 3, jvmArgsAppend = "-DTEST_LOG_LEVEL=warn") +public class SetTagsFromBenchmark { + /** A jdbc span's connection facts, contributed as tags: the shape TagContributor is for. */ + static final class DbInfo implements TagContributor { + final String type = "postgresql"; + final String pool = "HikariPool-1"; + final String user = "app"; + final String instance = "petclinic"; + final String host = "db.internal"; + final int port = 54321; // outside Integer's box cache, like a real port + final String operation = "SELECT"; + final String statement = "SELECT * FROM owners WHERE id = ?"; + + @Override + public void addTo(SpanData span) { + span.setTag(KnownTags.SPAN_KIND_ID, Tags.SPAN_KIND_CLIENT); + span.setTag(KnownTags.COMPONENT_ID, "java-jdbc-prepared_statement"); + span.setTag(KnownTags.DB_TYPE_ID, type); + span.setTag(KnownTags.DB_POOL_NAME_ID, pool); + span.setTag(KnownTags.DB_USER_ID, user); + span.setTag(KnownTags.DB_INSTANCE_ID, instance); + span.setTag(KnownTags.PEER_HOSTNAME_ID, host); + span.setTag(KnownTags.PEER_PORT_ID, port); + span.setTag(KnownTags.DB_OPERATION_ID, operation); + span.setTag("db.statement.custom", statement); + } + } + + /** The same writes from a foreign source, through an extractor held in a constant. */ + static final class DbInfoExtractor implements TagExtractor { + static final DbInfoExtractor INSTANCE = new DbInfoExtractor(); + + @Override + public void extract(DbInfo info, SpanData span) { + info.addTo(span); + } + } + + static final DbInfo DB_INFO = new DbInfo(); + + private CoreTracer tracer; + private DDSpan span; + + @Setup + public void setup(Blackhole blackhole) { + tracer = + CoreTracer.builder().writer(new TraceShapeBenchmark.SizeRecordingWriter(blackhole)).build(); + span = (DDSpan) tracer.startSpan("java-jdbc", "database.query"); + } + + @TearDown + public void tearDown() { + span.finish(); + tracer.close(); + } + + /** The batch one setTag at a time: DDSpan is a SpanData too, so the same addTo applies. */ + private static void oneByOne(DDSpan span) { + DB_INFO.addTo(span); + } + + @Benchmark + public Object reused_oneByOne() { + oneByOne(span); + return span; + } + + @Benchmark + public Object reused_contributor() { + return span.setTagsFrom(DB_INFO); + } + + @Benchmark + public Object reused_extractor() { + return span.setTagsFrom(DB_INFO, DbInfoExtractor.INSTANCE); + } + + @Benchmark + public Object fresh_oneByOne() { + DDSpan fresh = (DDSpan) tracer.startSpan("java-jdbc", "database.query"); + oneByOne(fresh); + fresh.finish(); + return fresh; + } + + @Benchmark + public Object fresh_contributor() { + DDSpan fresh = (DDSpan) tracer.startSpan("java-jdbc", "database.query"); + fresh.setTagsFrom(DB_INFO); + fresh.finish(); + return fresh; + } + + @Benchmark + public Object fresh_extractor() { + DDSpan fresh = (DDSpan) tracer.startSpan("java-jdbc", "database.query"); + fresh.setTagsFrom(DB_INFO, DbInfoExtractor.INSTANCE); + fresh.finish(); + return fresh; + } +} 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 e15a7ed49cb..df38b71aed7 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 @@ -22,6 +22,7 @@ import datadog.trace.api.TagMap; import datadog.trace.api.TraceConfig; import datadog.trace.api.debugger.DebuggerConfigBridge; +import datadog.trace.api.function.StrategyConsumer; import datadog.trace.api.gateway.Flow; import datadog.trace.api.gateway.RequestContext; import datadog.trace.api.internal.VisibleForTesting; @@ -37,6 +38,8 @@ import datadog.trace.bootstrap.instrumentation.api.ResourceNamePriorities; import datadog.trace.bootstrap.instrumentation.api.SpanPrototype; import datadog.trace.bootstrap.instrumentation.api.SpanWrapper; +import datadog.trace.bootstrap.instrumentation.api.TagContributor; +import datadog.trace.bootstrap.instrumentation.api.TagExtractor; import datadog.trace.core.util.StackTraces; import edu.umd.cs.findbugs.annotations.SuppressFBWarnings; import java.util.ArrayList; @@ -589,6 +592,29 @@ public DDSpan setTag(final long tagId, final Object value) { return this; } + /** + * Applies {@code extractor} holding the span's tag lock once for the whole call, instead of once + * per {@code setTag}; the extractor sees the span's usual tag semantics. Kept small so it inlines + * and each call site sees its exact extractor type. + */ + @Override + @StrategyConsumer + public DDSpan setTagsFrom(final T source, final TagExtractor extractor) { + if (source != null) { + context.setTagsFrom(source, extractor); + } + return this; + } + + /** Like {@link #setTagsFrom(Object, TagExtractor)}, for a {@link TagContributor}. */ + @Override + public DDSpan setTagsFrom(final TagContributor contributor) { + if (contributor != null) { + context.setTagsFrom(contributor); + } + return this; + } + @Override public AgentSpan setAllTags(Map map) { context.setAllTags(map); 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 79aa8468b1b..d7c39873de9 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 @@ -27,6 +27,7 @@ import datadog.trace.api.cache.DDCaches; import datadog.trace.api.config.TracerConfig; import datadog.trace.api.datastreams.PathwayContext; +import datadog.trace.api.function.NoEscape; import datadog.trace.api.gateway.BlockResponseFunction; import datadog.trace.api.gateway.RequestContext; import datadog.trace.api.gateway.RequestContextSlot; @@ -38,10 +39,14 @@ import datadog.trace.bootstrap.instrumentation.api.AppendableSpanLinks; import datadog.trace.bootstrap.instrumentation.api.Baggage; import datadog.trace.bootstrap.instrumentation.api.ClientIpAddressData; +import datadog.trace.bootstrap.instrumentation.api.ErrorPriorities; import datadog.trace.bootstrap.instrumentation.api.ProfilerContext; import datadog.trace.bootstrap.instrumentation.api.ProfilingContextIntegration; import datadog.trace.bootstrap.instrumentation.api.ResourceNamePriorities; +import datadog.trace.bootstrap.instrumentation.api.SpanData; import datadog.trace.bootstrap.instrumentation.api.SpanPrototype; +import datadog.trace.bootstrap.instrumentation.api.TagContributor; +import datadog.trace.bootstrap.instrumentation.api.TagExtractor; import datadog.trace.bootstrap.instrumentation.api.Tags; import datadog.trace.bootstrap.instrumentation.api.UTF8BytesString; import datadog.trace.core.propagation.PropagationTags; @@ -74,7 +79,12 @@ * the associated Span instance */ public class DDSpanContext - implements AgentSpanContext, RequestContext, TraceSegment, ProfilerContext, SpanStorage { + implements AgentSpanContext, + RequestContext, + TraceSegment, + ProfilerContext, + SpanStorage, + SpanData { private static final Logger log = LoggerFactory.getLogger(DDSpanContext.class); public static final String PRIORITY_SAMPLING_KEY = "_sampling_priority_v1"; @@ -672,10 +682,12 @@ public boolean isMeasured() { return measured; } - public void setMeasured(boolean measured) { + @Override + public DDSpanContext setMeasured(boolean measured) { if (measured != this.measured) { this.measured = measured; } + return this; } public boolean isTopLevel() { @@ -1429,9 +1441,10 @@ private boolean isPresent(TagMap.EntryReader entry) { * @param tag The tag name. * @param value The nullable tag value. */ - public void setTag(final String tag, final Object value) { + @Override + public DDSpanContext setTag(final String tag, final Object value) { if (null == tag) { - return; + return this; } final long tagId = KnownTagCodec.keyOf(tag); if (tagId != 0) { @@ -1443,11 +1456,13 @@ public void setTag(final String tag, final Object value) { customTagsForWrite().set(tag, value); // already known to be custom } } + return this; } - public void setTag(final String tag, final String value) { + @Override + public DDSpanContext setTag(final String tag, final String value) { if (null == tag) { - return; + return this; } final long tagId = KnownTagCodec.keyOf(tag); if (tagId != 0) { @@ -1459,6 +1474,7 @@ public void setTag(final String tag, final String value) { customTagsForWrite().set(tag, value); // already known to be custom } } + return this; } /* @@ -1466,39 +1482,44 @@ public void setTag(final String tag, final String value) { * KnownTagCodec.INTERCEPTED test folds, leaving only the split-by-tags table load; an id set never * takes the custom-tag path. An id that names no known tag is ignored, like a null tag. */ - public void setTag(final long tagId, final Object value) { + @Override + public DDSpanContext setTag(final long tagId, final Object value) { if (!KnownTagCodec.isKnown(tagId)) { - return; + return this; } if (null == value) { removeTag(tagId); - return; + return this; } if (!tagInterceptor.needsIntercept(tagId) || !tagInterceptor.interceptTag(this, tagId, value)) { synchronized (knownTags) { knownTags.set(tagId, value); } } + return this; } - public void setTag(final long tagId, final CharSequence value) { + @Override + public DDSpanContext setTag(final long tagId, final CharSequence value) { if (!KnownTagCodec.isKnown(tagId)) { - return; + return this; } if (null == value) { removeTag(tagId); - return; + return this; } if (!tagInterceptor.needsIntercept(tagId) || !tagInterceptor.interceptTag(this, tagId, value)) { synchronized (knownTags) { knownTags.set(tagId, value); } } + return this; } - public void setTag(final long tagId, final boolean value) { + @Override + public DDSpanContext setTag(final long tagId, final boolean value) { if (!KnownTagCodec.isKnown(tagId)) { - return; + return this; } if (tagInterceptor.needsIntercept(tagId)) { this.setBox(tagId, value); @@ -1507,11 +1528,13 @@ public void setTag(final long tagId, final boolean value) { knownTags.set(tagId, value); } } + return this; } - public void setTag(final long tagId, final int value) { + @Override + public DDSpanContext setTag(final long tagId, final int value) { if (!KnownTagCodec.isKnown(tagId)) { - return; + return this; } if (tagInterceptor.needsIntercept(tagId)) { this.setBox(tagId, value); @@ -1520,11 +1543,13 @@ public void setTag(final long tagId, final int value) { knownTags.set(tagId, value); } } + return this; } - public void setTag(final long tagId, final long value) { + @Override + public DDSpanContext setTag(final long tagId, final long value) { if (!KnownTagCodec.isKnown(tagId)) { - return; + return this; } if (tagInterceptor.needsIntercept(tagId)) { this.setBox(tagId, value); @@ -1533,11 +1558,13 @@ public void setTag(final long tagId, final long value) { knownTags.set(tagId, value); } } + return this; } - public void setTag(final long tagId, final float value) { + @Override + public DDSpanContext setTag(final long tagId, final float value) { if (!KnownTagCodec.isKnown(tagId)) { - return; + return this; } if (tagInterceptor.needsIntercept(tagId)) { this.setBox(tagId, value); @@ -1546,11 +1573,13 @@ public void setTag(final long tagId, final float value) { knownTags.set(tagId, value); } } + return this; } - public void setTag(final long tagId, final double value) { + @Override + public DDSpanContext setTag(final long tagId, final double value) { if (!KnownTagCodec.isKnown(tagId)) { - return; + return this; } if (tagInterceptor.needsIntercept(tagId)) { this.setBox(tagId, value); @@ -1559,11 +1588,13 @@ public void setTag(final long tagId, final double value) { knownTags.set(tagId, value); } } + return this; } - public void setTag(TagMap.EntryReader entry) { + @Override + public DDSpanContext setTag(TagMap.EntryReader entry) { if (entry == null) { - return; + return this; } // the interceptor prechecks the entry, to avoid boxing @@ -1572,6 +1603,7 @@ public void setTag(TagMap.EntryReader entry) { routePut(entry); } } + return this; } /* @@ -1594,9 +1626,10 @@ private void setBox(long tagId, Object box) { } } - public void setTag(final String tag, final boolean value) { + @Override + public DDSpanContext setTag(final String tag, final boolean value) { if (null == tag) { - return; + return this; } final long tagId = KnownTagCodec.keyOf(tag); if (tagId != 0) { @@ -1607,11 +1640,13 @@ public void setTag(final String tag, final boolean value) { customTagsForWrite().set(tag, value); // already known to be custom } } + return this; } - public void setTag(final String tag, final int value) { + @Override + public DDSpanContext setTag(final String tag, final int value) { if (null == tag) { - return; + return this; } final long tagId = KnownTagCodec.keyOf(tag); if (tagId != 0) { @@ -1622,11 +1657,13 @@ public void setTag(final String tag, final int value) { customTagsForWrite().set(tag, value); // already known to be custom } } + return this; } - public void setTag(final String tag, final long value) { + @Override + public DDSpanContext setTag(final String tag, final long value) { if (null == tag) { - return; + return this; } final long tagId = KnownTagCodec.keyOf(tag); if (tagId != 0) { @@ -1637,11 +1674,13 @@ public void setTag(final String tag, final long value) { customTagsForWrite().set(tag, value); // already known to be custom } } + return this; } - public void setTag(final String tag, final float value) { + @Override + public DDSpanContext setTag(final String tag, final float value) { if (null == tag) { - return; + return this; } final long tagId = KnownTagCodec.keyOf(tag); if (tagId != 0) { @@ -1652,11 +1691,13 @@ public void setTag(final String tag, final float value) { customTagsForWrite().set(tag, value); // already known to be custom } } + return this; } - public void setTag(final String tag, final double value) { + @Override + public DDSpanContext setTag(final String tag, final double value) { if (null == tag) { - return; + return this; } final long tagId = KnownTagCodec.keyOf(tag); if (tagId != 0) { @@ -1667,6 +1708,404 @@ public void setTag(final String tag, final double value) { customTagsForWrite().set(tag, value); // already known to be custom } } + return this; + } + + /* + * The *HoldingLock variants: the same tag semantics as setTag / removeTag / getTag (a null value + * removes, the tag interceptor runs), for a caller that already holds the knownTags lock, so a + * batch takes the lock once. Only HeldSpanData calls them, during setTagsFrom. + */ + + void setTagHoldingLock(final String tag, final Object value) { + if (null == tag) { + return; + } + final long tagId = KnownTagCodec.keyOf(tag); + if (tagId != 0) { + setTagHoldingLock(tagId, value); + } else if (null == value) { + removeTagHoldingLock(tag); + } else if (!tagInterceptor.interceptCustomTag(this, tag, value)) { + customTagsForWrite().set(tag, value); // already known to be custom + } + } + + void setTagHoldingLock(final String tag, final String value) { + if (null == tag) { + return; + } + final long tagId = KnownTagCodec.keyOf(tag); + if (tagId != 0) { + setTagHoldingLock(tagId, value); + } else if (null == value) { + removeTagHoldingLock(tag); + } else if (!tagInterceptor.interceptCustomTag(this, tag, value)) { + customTagsForWrite().set(tag, value); // already known to be custom + } + } + + void setTagHoldingLock(final long tagId, final Object value) { + if (!KnownTagCodec.isKnown(tagId)) { + return; + } + if (null == value) { + removeTagHoldingLock(tagId); + return; + } + if (!tagInterceptor.needsIntercept(tagId) || !tagInterceptor.interceptTag(this, tagId, value)) { + knownTags.set(tagId, value); + } + } + + void setTagHoldingLock(final long tagId, final CharSequence value) { + if (!KnownTagCodec.isKnown(tagId)) { + return; + } + if (null == value) { + removeTagHoldingLock(tagId); + return; + } + if (!tagInterceptor.needsIntercept(tagId) || !tagInterceptor.interceptTag(this, tagId, value)) { + knownTags.set(tagId, value); + } + } + + void setTagHoldingLock(final long tagId, final boolean value) { + if (!KnownTagCodec.isKnown(tagId)) { + return; + } + if (tagInterceptor.needsIntercept(tagId)) { + this.setBoxHoldingLock(tagId, value); + } else { + knownTags.set(tagId, value); + } + } + + void setTagHoldingLock(final long tagId, final int value) { + if (!KnownTagCodec.isKnown(tagId)) { + return; + } + if (tagInterceptor.needsIntercept(tagId)) { + this.setBoxHoldingLock(tagId, value); + } else { + knownTags.set(tagId, value); + } + } + + void setTagHoldingLock(final long tagId, final long value) { + if (!KnownTagCodec.isKnown(tagId)) { + return; + } + if (tagInterceptor.needsIntercept(tagId)) { + this.setBoxHoldingLock(tagId, value); + } else { + knownTags.set(tagId, value); + } + } + + void setTagHoldingLock(final long tagId, final float value) { + if (!KnownTagCodec.isKnown(tagId)) { + return; + } + if (tagInterceptor.needsIntercept(tagId)) { + this.setBoxHoldingLock(tagId, value); + } else { + knownTags.set(tagId, value); + } + } + + void setTagHoldingLock(final long tagId, final double value) { + if (!KnownTagCodec.isKnown(tagId)) { + return; + } + if (tagInterceptor.needsIntercept(tagId)) { + this.setBoxHoldingLock(tagId, value); + } else { + knownTags.set(tagId, value); + } + } + + void setTagHoldingLock(TagMap.EntryReader entry) { + if (entry == null) { + return; + } + + // the interceptor prechecks the entry, to avoid boxing + if (!tagInterceptor.interceptTag(this, entry)) { + routePut(entry); + } + } + + void setTagHoldingLock(final String tag, final boolean value) { + if (null == tag) { + return; + } + final long tagId = KnownTagCodec.keyOf(tag); + if (tagId != 0) { + setTagHoldingLock(tagId, value); + } else if (!tagInterceptor.needsInterceptCustomTag(tag) + || !tagInterceptor.interceptCustomTag(this, tag, value)) { + customTagsForWrite().set(tag, value); // already known to be custom + } + } + + void setTagHoldingLock(final String tag, final int value) { + if (null == tag) { + return; + } + final long tagId = KnownTagCodec.keyOf(tag); + if (tagId != 0) { + setTagHoldingLock(tagId, value); + } else if (!tagInterceptor.needsInterceptCustomTag(tag) + || !tagInterceptor.interceptCustomTag(this, tag, value)) { + customTagsForWrite().set(tag, value); // already known to be custom + } + } + + void setTagHoldingLock(final String tag, final long value) { + if (null == tag) { + return; + } + final long tagId = KnownTagCodec.keyOf(tag); + if (tagId != 0) { + setTagHoldingLock(tagId, value); + } else if (!tagInterceptor.needsInterceptCustomTag(tag) + || !tagInterceptor.interceptCustomTag(this, tag, value)) { + customTagsForWrite().set(tag, value); // already known to be custom + } + } + + void setTagHoldingLock(final String tag, final float value) { + if (null == tag) { + return; + } + final long tagId = KnownTagCodec.keyOf(tag); + if (tagId != 0) { + setTagHoldingLock(tagId, value); + } else if (!tagInterceptor.needsInterceptCustomTag(tag) + || !tagInterceptor.interceptCustomTag(this, tag, value)) { + customTagsForWrite().set(tag, value); // already known to be custom + } + } + + void setTagHoldingLock(final String tag, final double value) { + if (null == tag) { + return; + } + final long tagId = KnownTagCodec.keyOf(tag); + if (tagId != 0) { + setTagHoldingLock(tagId, value); + } else if (!tagInterceptor.needsInterceptCustomTag(tag) + || !tagInterceptor.interceptCustomTag(this, tag, value)) { + customTagsForWrite().set(tag, value); // already known to be custom + } + } + + void removeTagHoldingLock(String tag) { + if (tagEquals(tag, Tags.SPAN_KIND)) { + // Clear the cached ordinal; the tag store still needs to be updated below. + spanKindOrdinal = SPAN_KIND_UNSET; + } + routeRemoval(tag); + } + + void removeTagHoldingLock(long tagId) { + if (!KnownTagCodec.isKnown(tagId)) { + return; + } + if (tagId == KnownTags.SPAN_KIND_ID) { + spanKindOrdinal = SPAN_KIND_UNSET; + } + knownTags.remove(tagId); + } + + private void setBoxHoldingLock(long tagId, Object box) { + if (!tagInterceptor.interceptTag(this, tagId, box)) { + knownTags.set(tagId, box); + } + } + + Object getTagHoldingLock(final String key) { + switch (key) { + case DDTags.THREAD_ID: + return threadId; + case DDTags.THREAD_NAME: + // maintain previously observable type of the thread name :| + return threadName.toString(); + case Tags.HTTP_STATUS: + case KnownTags.HTTP_STATUS_CODE_OTEL_NAME: + return 0 == httpStatusCode ? null : (int) httpStatusCode; + case Tags.SPAN_KIND: + return getSpanKindString(); + default: + Object value = unsafeGetTag(key); + // maintain previously observable type of http url :| + return value == null ? null : Tags.HTTP_URL.equals(key) ? value.toString() : value; + } + } + + @Override + public DDSpanContext setResourceName(final CharSequence resourceName) { + setResourceName(resourceName, ResourceNamePriorities.DEFAULT); + return this; + } + + @Override + public DDSpanContext setError(final boolean error) { + setErrorFlag(error, ErrorPriorities.DEFAULT); + return this; + } + + /** + * Applies {@code extractor} to {@code source} holding the tag lock once for the whole call: the + * extractor's writes go through a {@link HeldSpanData}, which keeps the span's tag semantics but + * doesn't lock again. Kept small so it inlines and each call site sees its exact extractor type. + */ + void setTagsFrom(final T source, final TagExtractor extractor) { + synchronized (knownTags) { + extractor.extract(source, new HeldSpanData(this)); + } + } + + /** {@link #setTagsFrom(Object, TagExtractor)} for a {@link TagContributor}. */ + void setTagsFrom(final TagContributor contributor) { + synchronized (knownTags) { + contributor.addTo(new HeldSpanData(this)); + } + } + + /** + * The span as a {@link SpanData} for the length of one {@code setTagsFrom} call, which holds the + * tag lock throughout: tag writes and reads skip relocking. It must not outlive the call; with + * the contributor's call site monomorphic, it is scalar-replaced. + */ + @NoEscape + static final class HeldSpanData implements SpanData { + private final DDSpanContext span; + + HeldSpanData(DDSpanContext span) { + this.span = span; + } + + /** The span, checking (with assertions on) that this is still inside its setTagsFrom call. */ + private DDSpanContext span() { + assert Thread.holdsLock(span.knownTags) : "HeldSpanData used outside its setTagsFrom call"; + return span; + } + + @Override + public SpanData setTag(String key, Object value) { + span().setTagHoldingLock(key, value); + return this; + } + + @Override + public SpanData setTag(String key, String value) { + span().setTagHoldingLock(key, value); + return this; + } + + @Override + public SpanData setTag(String key, boolean value) { + span().setTagHoldingLock(key, value); + return this; + } + + @Override + public SpanData setTag(String key, int value) { + span().setTagHoldingLock(key, value); + return this; + } + + @Override + public SpanData setTag(String key, long value) { + span().setTagHoldingLock(key, value); + return this; + } + + @Override + public SpanData setTag(String key, float value) { + span().setTagHoldingLock(key, value); + return this; + } + + @Override + public SpanData setTag(String key, double value) { + span().setTagHoldingLock(key, value); + return this; + } + + @Override + public SpanData setTag(long tagId, Object value) { + span().setTagHoldingLock(tagId, value); + return this; + } + + @Override + public SpanData setTag(long tagId, CharSequence value) { + span().setTagHoldingLock(tagId, value); + return this; + } + + @Override + public SpanData setTag(long tagId, boolean value) { + span().setTagHoldingLock(tagId, value); + return this; + } + + @Override + public SpanData setTag(long tagId, int value) { + span().setTagHoldingLock(tagId, value); + return this; + } + + @Override + public SpanData setTag(long tagId, long value) { + span().setTagHoldingLock(tagId, value); + return this; + } + + @Override + public SpanData setTag(long tagId, float value) { + span().setTagHoldingLock(tagId, value); + return this; + } + + @Override + public SpanData setTag(long tagId, double value) { + span().setTagHoldingLock(tagId, value); + return this; + } + + @Override + public SpanData setTag(TagMap.EntryReader entry) { + span().setTagHoldingLock(entry); + return this; + } + + @Override + public Object getTag(String key) { + return span().getTagHoldingLock(key); + } + + @Override + public SpanData setResourceName(CharSequence resourceName) { + span().setResourceName(resourceName); + return this; + } + + @Override + public SpanData setError(boolean error) { + span().setError(error); + return this; + } + + @Override + public SpanData setMeasured(boolean measured) { + span().setMeasured(measured); + return this; + } } void setAllTags(final TagMap map) { @@ -1752,7 +2191,8 @@ void unsafeSetTag(final String tag, final double value) { routeSet(tag, value); } - Object getTag(final String key) { + @Override + public Object getTag(final String key) { switch (key) { case DDTags.THREAD_ID: return threadId; diff --git a/dd-trace-core/src/test/java/datadog/trace/core/DDSpanSetTagsFromTest.java b/dd-trace-core/src/test/java/datadog/trace/core/DDSpanSetTagsFromTest.java new file mode 100644 index 00000000000..a6c1a573bd4 --- /dev/null +++ b/dd-trace-core/src/test/java/datadog/trace/core/DDSpanSetTagsFromTest.java @@ -0,0 +1,147 @@ +package datadog.trace.core; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertFalse; +import static org.junit.jupiter.api.Assertions.assertNull; +import static org.junit.jupiter.api.Assertions.assertSame; +import static org.junit.jupiter.api.Assertions.assertThrows; +import static org.junit.jupiter.api.Assertions.assertTrue; + +import datadog.trace.api.DDTags; +import datadog.trace.api.KnownTags; +import datadog.trace.bootstrap.instrumentation.api.SpanData; +import datadog.trace.bootstrap.instrumentation.api.TagContributor; +import datadog.trace.bootstrap.instrumentation.api.TagExtractor; +import datadog.trace.bootstrap.instrumentation.api.Tags; +import datadog.trace.common.writer.ListWriter; +import java.util.TreeMap; +import java.util.concurrent.atomic.AtomicReference; +import org.junit.jupiter.api.AfterEach; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; + +/** + * {@code DDSpan.setTagsFrom}: the contributor or extractor runs holding the span's tag lock once, + * through a short-lived {@link SpanData} that keeps the span's tag semantics. + */ +class DDSpanSetTagsFromTest extends DDCoreJavaSpecification { + private CoreTracer tracer; + + @BeforeEach + void setup() { + tracer = tracerBuilder().writer(new ListWriter()).build(); + } + + @AfterEach + void tearDown() { + tracer.close(); + } + + private DDSpan start() { + return (DDSpan) tracer.startSpan("test-instr", "tags-from.op"); + } + + /** + * A non-String value, shared so both spans hold the same object (StringBuilder has no equals). + */ + private static final Object OBJECT_VALUE = new StringBuilder("built"); + + /** The same writes either way: known tags by id and by name, custom tags, every value type. */ + private static void writeTags(SpanData span) { + span.setTag(KnownTags.HTTP_ROUTE_ID, "/users/{id}"); + span.setTag(KnownTags.HTTP_STATUS_CODE_ID, 503); + span.setTag(Tags.COMPONENT, "jdbc"); + span.setTag("custom.string", "value"); + span.setTag("custom.object", OBJECT_VALUE); + span.setTag("custom.bool", true); + span.setTag("custom.int", 7); + span.setTag("custom.long", 8L); + span.setTag("custom.float", 1.5f); + span.setTag("custom.double", 2.5d); + span.setTag("custom.removed", "gone"); + span.setTag("custom.removed", (Object) null); + } + + @Test + void aContributorSetsWhatTheSameSetTagCallsWouldSet() { + DDSpan oneByOne = start(); + DDSpan batched = start(); + try { + writeTags(oneByOne); + TagContributor contributor = DDSpanSetTagsFromTest::writeTags; + assertSame(batched, batched.setTagsFrom(contributor)); + + // compared as plain maps: TagMap.equals isn't Map content equality + assertEquals(new TreeMap<>(oneByOne.getTags()), new TreeMap<>(batched.getTags())); + assertNull(batched.getTag("custom.removed")); + } finally { + oneByOne.finish(); + batched.finish(); + } + } + + @Test + void anExtractorSeesTheSpanTagSemanticsIncludingInterception() { + DDSpan span = start(); + try { + TagExtractor extractor = + (resource, target) -> { + target.setTag(DDTags.RESOURCE_NAME, resource); // intercepted: becomes the resource + target.setTag(Tags.SPAN_KIND, Tags.SPAN_KIND_CLIENT); // intercepted: the cached kind + target.setTag(KnownTags.HTTP_ROUTE_ID, "/orders"); + // reads inside the batch see the batch's own writes + target.setTag("custom.route.seen", String.valueOf(target.getTag(Tags.HTTP_ROUTE))); + }; + span.setTagsFrom("GET /orders", extractor); + + assertEquals("GET /orders", span.getResourceName().toString()); + assertEquals(Tags.SPAN_KIND_CLIENT, span.getSpanKindString()); + assertEquals("/orders", span.getTag("custom.route.seen")); + } finally { + span.finish(); + } + } + + @Test + void theSpanLevelSettersWorkInsideTheBatch() { + DDSpan span = start(); + try { + span.setTagsFrom( + (TagContributor) + target -> target.setResourceName("batched").setError(true).setMeasured(true)); + + assertEquals("batched", span.getResourceName().toString()); + assertTrue(span.isError()); + assertTrue(span.isMeasured()); + } finally { + span.finish(); + } + } + + @Test + void aNullSourceOrContributorIsSkipped() { + DDSpan span = start(); + try { + TagExtractor extractor = (source, target) -> target.setTag("custom.ran", true); + assertSame(span, span.setTagsFrom(null, extractor)); + assertSame(span, span.setTagsFrom((TagContributor) null)); + assertFalse(span.getTags().containsKey("custom.ran")); + } finally { + span.finish(); + } + } + + @Test + void theBatchsSpanDataMustNotOutliveTheCall() { + DDSpan span = start(); + try { + AtomicReference kept = new AtomicReference<>(); + span.setTagsFrom((TagContributor) kept::set); + + // assertions are on in tests, so a write outside the lock is caught + assertThrows(AssertionError.class, () -> kept.get().setTag("custom.late", "late")); + } finally { + span.finish(); + } + } +} 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 5e3918f123c..388d311c7b2 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 @@ -27,7 +27,8 @@ public interface AgentSpan ImplicitContextKeyed, Context, IGSpanInfo, - WithAgentSpan { + WithAgentSpan, + SpanData { /** * Extracts the span from the current {@link Context}. diff --git a/internal-api/src/main/java/datadog/trace/bootstrap/instrumentation/api/SpanData.java b/internal-api/src/main/java/datadog/trace/bootstrap/instrumentation/api/SpanData.java new file mode 100644 index 00000000000..df5d10c93a4 --- /dev/null +++ b/internal-api/src/main/java/datadog/trace/bootstrap/instrumentation/api/SpanData.java @@ -0,0 +1,59 @@ +package datadog.trace.bootstrap.instrumentation.api; + +import datadog.trace.api.TagMap; + +/** + * What a {@link TagContributor} or {@link TagExtractor} may read and set on a span: its tags, with + * the span's full tag semantics (a null value removes the tag, and the tag interceptor runs), and + * the few span-level fields that aren't yet tags. + * + *

The intercepted counterpart of {@link datadog.trace.api.SpanStorage}, which loads and stores + * raw. It uses the span's own vocabulary, {@code getTag}/{@code setTag}, so contributor code reads + * like span code. Unlike OpenTelemetry's {@code SpanData}, it is not a snapshot: writes go to the + * live span. + * + *

{@link AgentSpan#setTagsFrom} may hand its contributor a short-lived {@code SpanData} that + * holds the span's tag lock for the whole call, so a run of {@code setTag} calls takes the lock + * once. Such an instance is valid only during that call: don't keep it. + */ +public interface SpanData { + SpanData setTag(String key, Object value); + + SpanData setTag(String key, String value); + + SpanData setTag(String key, boolean value); + + SpanData setTag(String key, int value); + + SpanData setTag(String key, long value); + + SpanData setTag(String key, float value); + + SpanData setTag(String key, double value); + + /** Sets a known tag by its {@code KnownTags.*_ID}; an id that names no known tag is ignored. */ + SpanData setTag(long tagId, Object value); + + SpanData setTag(long tagId, CharSequence value); + + SpanData setTag(long tagId, boolean value); + + SpanData setTag(long tagId, int value); + + SpanData setTag(long tagId, long value); + + SpanData setTag(long tagId, float value); + + SpanData setTag(long tagId, double value); + + /** A null entry leaves the tags unchanged. */ + SpanData setTag(TagMap.EntryReader entry); + + Object getTag(String key); + + SpanData setResourceName(CharSequence resourceName); + + SpanData setError(boolean error); + + SpanData setMeasured(boolean measured); +} diff --git a/internal-api/src/main/java/datadog/trace/bootstrap/instrumentation/api/TagContributor.java b/internal-api/src/main/java/datadog/trace/bootstrap/instrumentation/api/TagContributor.java index 637600b4203..1bc2c8b9c62 100644 --- a/internal-api/src/main/java/datadog/trace/bootstrap/instrumentation/api/TagContributor.java +++ b/internal-api/src/main/java/datadog/trace/bootstrap/instrumentation/api/TagContributor.java @@ -13,12 +13,13 @@ * List} sink — that re-megamorphizes and recreates the decorator problem. Prefer * stateless implementations. * - *

Targets {@link AgentSpan} for now (so it can also drive span-level state — resource name, - * error, measured — that is not yet expressed as tags). As those span fields migrate into the tag - * model, this surface narrows toward a future {@code addTo(TagMap)}. See {@link TagExtractor} for - * the extrinsic counterpart (foreign objects we do not own). Apply it with {@link + *

Targets {@link SpanData}: the span's tags with full tag semantics, plus the span-level state + * (resource name, error, measured) that is not yet expressed as tags. {@link + * AgentSpan#setTagsFrom(TagContributor)} may hand it a short-lived {@code SpanData} that holds the + * span's tag lock for the whole call; don't keep it past {@code addTo}. See {@link TagExtractor} + * for the extrinsic counterpart (foreign objects we do not own). Apply it with {@link * AgentSpan#setTagsFrom(TagContributor)}. */ public interface TagContributor { - void addTo(AgentSpan span); + void addTo(SpanData span); } diff --git a/internal-api/src/main/java/datadog/trace/bootstrap/instrumentation/api/TagExtractor.java b/internal-api/src/main/java/datadog/trace/bootstrap/instrumentation/api/TagExtractor.java index a62e10836b7..edc78c7e045 100644 --- a/internal-api/src/main/java/datadog/trace/bootstrap/instrumentation/api/TagExtractor.java +++ b/internal-api/src/main/java/datadog/trace/bootstrap/instrumentation/api/TagExtractor.java @@ -23,9 +23,10 @@ * a non-capturing lambda constant is accepted but is the weaker, speculative shape. The consumer, * {@link AgentSpan#setTagsFrom(Object, TagExtractor)}, is small so it inlines and each call site * sees the exact extractor type. Don't hold extractors in a field or collection of the abstract - * type and dispatch from there. Targets {@link AgentSpan} so it can also drive span-level state - * (resource name, error, status) during the transition; that surface narrows as those fields - * migrate into the tag model. + * type and dispatch from there. Targets {@link SpanData}: the span's tags with full tag semantics, + * plus the span-level state (resource name, error, measured) not yet expressed as tags. {@link + * AgentSpan#setTagsFrom(Object, TagExtractor)} may hand it a short-lived {@code SpanData} that + * holds the span's tag lock for the whole call; don't keep it past {@code extract}. * *

Apply it with {@link AgentSpan#setTagsFrom(Object, TagExtractor)}. * @@ -34,5 +35,5 @@ @Strategy @FunctionalInterface public interface TagExtractor { - void extract(T source, AgentSpan span); + void extract(T source, SpanData span); }