From d2cc4ec022b161c9e2dc2f407f5f6404fdfe6620 Mon Sep 17 00:00:00 2001 From: Douglas Q Hawkins Date: Fri, 12 Sep 2025 16:02:42 -0400 Subject: [PATCH 01/43] Experimenting with introducing CoreSpanBuilder reuse Initial performance experiment The idea is to store a CoreSpanBuilder per thread, since usually only SpanBuilder is in use at a given time per thread -- and CoreSpanBuilder isn't thread safe This simple change provides a giant boost in small heaps Improving Spring petclinic throughput from -39% to -19% with 80m heap --- .../java/datadog/trace/core/CoreTracer.java | 42 +++++++++++++++---- 1 file changed, 34 insertions(+), 8 deletions(-) diff --git a/dd-trace-core/src/main/java/datadog/trace/core/CoreTracer.java b/dd-trace-core/src/main/java/datadog/trace/core/CoreTracer.java index d77dd7f8b68..5239949bcef 100644 --- a/dd-trace-core/src/main/java/datadog/trace/core/CoreTracer.java +++ b/dd-trace-core/src/main/java/datadog/trace/core/CoreTracer.java @@ -243,6 +243,15 @@ public static CoreTracerBuilder builder() { private final PropagationTags.Factory propagationTagsFactory; + // Cache used by buildSpan + private final ThreadLocal tlSpanBuilder = + new ThreadLocal() { + @Override + protected CoreSpanBuilder initialValue() { + return new CoreSpanBuilder(CoreTracer.this); + } + }; + @Override public ConfigSnapshot captureTraceConfig() { return dynamicConfig.captureTraceConfig(); @@ -968,7 +977,9 @@ long getTimeWithNanoTicks(long nanoTicks) { @Override public CoreSpanBuilder buildSpan( final String instrumentationName, final CharSequence operationName) { - return new CoreSpanBuilder(this, instrumentationName, operationName); + CoreSpanBuilder tlSpanBuilder = this.tlSpanBuilder.get(); + tlSpanBuilder.reset(instrumentationName, operationName); + return tlSpanBuilder; } @Override @@ -1401,10 +1412,11 @@ private static Map invertMap(Map map) { /** Spans are built using this builder */ public static class CoreSpanBuilder implements AgentTracer.SpanBuilder { - private final String instrumentationName; - private final CharSequence operationName; private final CoreTracer tracer; + private String instrumentationName; + private CharSequence operationName; + // Builder attributes private TagMap.Ledger tagLedger; private long timestampMicro; @@ -1420,13 +1432,27 @@ public static class CoreSpanBuilder implements AgentTracer.SpanBuilder { private List links; private long spanId; - CoreSpanBuilder( - final CoreTracer tracer, - final String instrumentationName, - final CharSequence operationName) { + CoreSpanBuilder(CoreTracer tracer) { + this.tracer = tracer; + } + + void reset(String instrumentationName, CharSequence operationName) { this.instrumentationName = instrumentationName; this.operationName = operationName; - this.tracer = tracer; + + if (this.tagLedger != null) this.tagLedger.reset(); + this.timestampMicro = 0L; + this.parent = null; + this.serviceName = null; + this.resourceName = null; + this.errorFlag = false; + this.spanType = null; + this.ignoreScope = false; + this.builderCiVisibilityContextData = null; + this.builderRequestContextDataIast = null; + this.builderCiVisibilityContextData = null; + this.links = null; + this.spanId = 0L; } @Override From d83ff1609a286e276c67926babf662cf173b3a4a Mon Sep 17 00:00:00 2001 From: Douglas Q Hawkins Date: Mon, 15 Sep 2025 15:30:51 -0400 Subject: [PATCH 02/43] Adding protection against reusing a CoreSpanBuilder that's still in-use --- .../java/datadog/trace/core/CoreTracer.java | 46 ++++++++++++++++--- 1 file changed, 39 insertions(+), 7 deletions(-) diff --git a/dd-trace-core/src/main/java/datadog/trace/core/CoreTracer.java b/dd-trace-core/src/main/java/datadog/trace/core/CoreTracer.java index 5239949bcef..30e65fbbdea 100644 --- a/dd-trace-core/src/main/java/datadog/trace/core/CoreTracer.java +++ b/dd-trace-core/src/main/java/datadog/trace/core/CoreTracer.java @@ -977,9 +977,28 @@ long getTimeWithNanoTicks(long nanoTicks) { @Override public CoreSpanBuilder buildSpan( final String instrumentationName, final CharSequence operationName) { - CoreSpanBuilder tlSpanBuilder = this.tlSpanBuilder.get(); - tlSpanBuilder.reset(instrumentationName, operationName); - return tlSpanBuilder; + // retrieve the thread's typical SpanBuilder and check if it is currently in use + CoreSpanBuilder tlSpanBuilder = this.tlSpanBuilder.get(); + boolean wasReset = tlSpanBuilder.reset(instrumentationName, operationName); + if ( wasReset ) return tlSpanBuilder; + + // TODO: counter for how often the fallback is used? + CoreSpanBuilder newSpanBuilder = new CoreSpanBuilder(this); + newSpanBuilder.reset(instrumentationName, operationName); + + // DQH - Debated how best to handle the case of someone requesting a CoreSpanBuilder + // and then not using it. Without an ability to replace the cached CoreSpanBuilder, + // that case could result in permanently burning the cache for a given thread. + + // That could be solved with additional logic during CoreSpanBuilder#build + // that checks to see if the cached CoreSpanBuilder is in use and then replaces it + // with the freed CoreSpanBuilder, but that would put extra logic in the common path. + + // Instead of making the release process more complicating, I'm chosing to just + // override here when we know we're already in an uncommon path. + this.tlSpanBuilder.set(newSpanBuilder); + + return newSpanBuilder; } @Override @@ -1409,10 +1428,16 @@ private static Map invertMap(Map map) { } return Collections.unmodifiableMap(inverted); } - + /** Spans are built using this builder */ - public static class CoreSpanBuilder implements AgentTracer.SpanBuilder { + /* + * To reduce overhead, CoreSpanBuilders are reused. + * For simplicity, CoreSpanBuilder isn't thread safe, so CoreSpanBuilders can only be reused within one thread. + */ + public static final class CoreSpanBuilder implements AgentTracer.SpanBuilder { private final CoreTracer tracer; + + private boolean inUse = false; private String instrumentationName; private CharSequence operationName; @@ -1436,7 +1461,10 @@ public static class CoreSpanBuilder implements AgentTracer.SpanBuilder { this.tracer = tracer; } - void reset(String instrumentationName, CharSequence operationName) { + boolean reset(String instrumentationName, CharSequence operationName) { + if ( this.inUse ) return false; + this.inUse = true; + this.instrumentationName = instrumentationName; this.operationName = operationName; @@ -1448,11 +1476,13 @@ void reset(String instrumentationName, CharSequence operationName) { this.errorFlag = false; this.spanType = null; this.ignoreScope = false; - this.builderCiVisibilityContextData = null; + this.builderRequestContextDataAppSec = null; this.builderRequestContextDataIast = null; this.builderCiVisibilityContextData = null; this.links = null; this.spanId = 0L; + + return true; } @Override @@ -1469,6 +1499,8 @@ private DDSpan buildSpan() { span.setEndpointTracker(tracker); } } + + this.inUse = false; return span; } From 045e4c9dcbe2c32780ff0d82db8e05db99fa4ccb Mon Sep 17 00:00:00 2001 From: Douglas Q Hawkins Date: Mon, 15 Sep 2025 15:49:19 -0400 Subject: [PATCH 03/43] spotless --- .../java/datadog/trace/core/CoreTracer.java | 40 +++++++++---------- 1 file changed, 20 insertions(+), 20 deletions(-) diff --git a/dd-trace-core/src/main/java/datadog/trace/core/CoreTracer.java b/dd-trace-core/src/main/java/datadog/trace/core/CoreTracer.java index 30e65fbbdea..2dfb0aa95a3 100644 --- a/dd-trace-core/src/main/java/datadog/trace/core/CoreTracer.java +++ b/dd-trace-core/src/main/java/datadog/trace/core/CoreTracer.java @@ -977,27 +977,27 @@ long getTimeWithNanoTicks(long nanoTicks) { @Override public CoreSpanBuilder buildSpan( final String instrumentationName, final CharSequence operationName) { - // retrieve the thread's typical SpanBuilder and check if it is currently in use - CoreSpanBuilder tlSpanBuilder = this.tlSpanBuilder.get(); - boolean wasReset = tlSpanBuilder.reset(instrumentationName, operationName); - if ( wasReset ) return tlSpanBuilder; - + // retrieve the thread's typical SpanBuilder and check if it is currently in use + CoreSpanBuilder tlSpanBuilder = this.tlSpanBuilder.get(); + boolean wasReset = tlSpanBuilder.reset(instrumentationName, operationName); + if (wasReset) return tlSpanBuilder; + // TODO: counter for how often the fallback is used? CoreSpanBuilder newSpanBuilder = new CoreSpanBuilder(this); newSpanBuilder.reset(instrumentationName, operationName); - - // DQH - Debated how best to handle the case of someone requesting a CoreSpanBuilder - // and then not using it. Without an ability to replace the cached CoreSpanBuilder, + + // DQH - Debated how best to handle the case of someone requesting a CoreSpanBuilder + // and then not using it. Without an ability to replace the cached CoreSpanBuilder, // that case could result in permanently burning the cache for a given thread. - - // That could be solved with additional logic during CoreSpanBuilder#build - // that checks to see if the cached CoreSpanBuilder is in use and then replaces it + + // That could be solved with additional logic during CoreSpanBuilder#build + // that checks to see if the cached CoreSpanBuilder is in use and then replaces it // with the freed CoreSpanBuilder, but that would put extra logic in the common path. - - // Instead of making the release process more complicating, I'm chosing to just + + // Instead of making the release process more complicating, I'm chosing to just // override here when we know we're already in an uncommon path. this.tlSpanBuilder.set(newSpanBuilder); - + return newSpanBuilder; } @@ -1428,7 +1428,7 @@ private static Map invertMap(Map map) { } return Collections.unmodifiableMap(inverted); } - + /** Spans are built using this builder */ /* * To reduce overhead, CoreSpanBuilders are reused. @@ -1436,7 +1436,7 @@ private static Map invertMap(Map map) { */ public static final class CoreSpanBuilder implements AgentTracer.SpanBuilder { private final CoreTracer tracer; - + private boolean inUse = false; private String instrumentationName; @@ -1462,9 +1462,9 @@ public static final class CoreSpanBuilder implements AgentTracer.SpanBuilder { } boolean reset(String instrumentationName, CharSequence operationName) { - if ( this.inUse ) return false; + if (this.inUse) return false; this.inUse = true; - + this.instrumentationName = instrumentationName; this.operationName = operationName; @@ -1481,7 +1481,7 @@ boolean reset(String instrumentationName, CharSequence operationName) { this.builderCiVisibilityContextData = null; this.links = null; this.spanId = 0L; - + return true; } @@ -1499,7 +1499,7 @@ private DDSpan buildSpan() { span.setEndpointTracker(tracker); } } - + this.inUse = false; return span; } From 95e6f177eae39010b4e78b67570a72fef2301d36 Mon Sep 17 00:00:00 2001 From: Douglas Q Hawkins Date: Mon, 15 Sep 2025 16:32:15 -0400 Subject: [PATCH 04/43] Adding configuration option to control reuse of SpanBuilders --- .../trace/api/config/GeneralConfig.java | 1 + .../java/datadog/trace/core/CoreTracer.java | 35 +++++++++++++++---- .../main/java/datadog/trace/api/Config.java | 7 ++++ 3 files changed, 36 insertions(+), 7 deletions(-) diff --git a/dd-trace-api/src/main/java/datadog/trace/api/config/GeneralConfig.java b/dd-trace-api/src/main/java/datadog/trace/api/config/GeneralConfig.java index 43c72885ef8..1c97fb5df31 100644 --- a/dd-trace-api/src/main/java/datadog/trace/api/config/GeneralConfig.java +++ b/dd-trace-api/src/main/java/datadog/trace/api/config/GeneralConfig.java @@ -107,6 +107,7 @@ public final class GeneralConfig { public static final String OPTIMIZED_MAP_ENABLED = "optimized.map.enabled"; public static final String TAG_NAME_UTF8_CACHE_SIZE = "tag.name.utf8.cache.size"; public static final String TAG_VALUE_UTF8_CACHE_SIZE = "tag.value.utf8.cache.size"; + public static final String SPAN_BUILDER_REUSE_ENABLED = "span.builder.reuse.enabled"; public static final String STACK_TRACE_LENGTH_LIMIT = "stack.trace.length.limit"; public static final String SSI_INJECTION_ENABLED = "injection.enabled"; diff --git a/dd-trace-core/src/main/java/datadog/trace/core/CoreTracer.java b/dd-trace-core/src/main/java/datadog/trace/core/CoreTracer.java index 2dfb0aa95a3..6fd781a151c 100644 --- a/dd-trace-core/src/main/java/datadog/trace/core/CoreTracer.java +++ b/dd-trace-core/src/main/java/datadog/trace/core/CoreTracer.java @@ -243,14 +243,21 @@ public static CoreTracerBuilder builder() { private final PropagationTags.Factory propagationTagsFactory; - // Cache used by buildSpan + // DQH - storing into a static constant, so value will constant propagate and dead code eliminate + // the other branch in buildSpan + private static final boolean SPAN_BUILDER_REUSE_ENABLED = + Config.get().isSpanBuilderReuseEnabled(); + + // Cache used by buildSpan - instance so it can capture the CoreTracer private final ThreadLocal tlSpanBuilder = - new ThreadLocal() { - @Override - protected CoreSpanBuilder initialValue() { - return new CoreSpanBuilder(CoreTracer.this); - } - }; + SPAN_BUILDER_REUSE_ENABLED + ? new ThreadLocal() { + @Override + protected CoreSpanBuilder initialValue() { + return new CoreSpanBuilder(CoreTracer.this); + } + } + : null; @Override public ConfigSnapshot captureTraceConfig() { @@ -977,6 +984,20 @@ long getTimeWithNanoTicks(long nanoTicks) { @Override public CoreSpanBuilder buildSpan( final String instrumentationName, final CharSequence operationName) { + return SPAN_BUILDER_REUSE_ENABLED + ? this.reuseSpanBuilder(instrumentationName, operationName) + : this.createSpanBuilder(instrumentationName, operationName); + } + + private CoreSpanBuilder createSpanBuilder( + final String instrumentationName, final CharSequence operationName) { + CoreSpanBuilder newBuilder = new CoreSpanBuilder(this); + newBuilder.reset(instrumentationName, operationName); + return newBuilder; + } + + private CoreSpanBuilder reuseSpanBuilder( + final String instrumentationName, final CharSequence operationName) { // retrieve the thread's typical SpanBuilder and check if it is currently in use CoreSpanBuilder tlSpanBuilder = this.tlSpanBuilder.get(); boolean wasReset = tlSpanBuilder.reset(instrumentationName, operationName); diff --git a/internal-api/src/main/java/datadog/trace/api/Config.java b/internal-api/src/main/java/datadog/trace/api/Config.java index a705725c31c..3478d5ceb5b 100644 --- a/internal-api/src/main/java/datadog/trace/api/Config.java +++ b/internal-api/src/main/java/datadog/trace/api/Config.java @@ -1233,6 +1233,7 @@ public static String getHostName() { private final boolean jdkSocketEnabled; private final boolean optimizedMapEnabled; + private final boolean spanBuilderReuseEnabled; private final int tagNameUtf8CacheSize; private final int tagValueUtf8CacheSize; private final int stackTraceLengthLimit; @@ -2749,6 +2750,8 @@ PROFILING_DATADOG_PROFILER_ENABLED, isDatadogProfilerSafeInCurrentEnvironment()) this.optimizedMapEnabled = configProvider.getBoolean(GeneralConfig.OPTIMIZED_MAP_ENABLED, false); + this.spanBuilderReuseEnabled = + configProvider.getBoolean(GeneralConfig.SPAN_BUILDER_REUSE_ENABLED, false); this.tagNameUtf8CacheSize = Math.max(configProvider.getInteger(GeneralConfig.TAG_NAME_UTF8_CACHE_SIZE, 128), 0); this.tagValueUtf8CacheSize = @@ -4474,6 +4477,10 @@ public boolean isOptimizedMapEnabled() { return optimizedMapEnabled; } + public boolean isSpanBuilderReuseEnabled() { + return spanBuilderReuseEnabled; + } + public int getTagNameUtf8CacheSize() { return tagNameUtf8CacheSize; } From 55b56e796069156273e4001da3801c98171316fe Mon Sep 17 00:00:00 2001 From: Douglas Q Hawkins Date: Mon, 15 Sep 2025 16:37:06 -0400 Subject: [PATCH 05/43] Adding explanatory comments --- dd-trace-core/src/main/java/datadog/trace/core/CoreTracer.java | 3 +++ 1 file changed, 3 insertions(+) diff --git a/dd-trace-core/src/main/java/datadog/trace/core/CoreTracer.java b/dd-trace-core/src/main/java/datadog/trace/core/CoreTracer.java index 6fd781a151c..15c044ef341 100644 --- a/dd-trace-core/src/main/java/datadog/trace/core/CoreTracer.java +++ b/dd-trace-core/src/main/java/datadog/trace/core/CoreTracer.java @@ -1458,6 +1458,9 @@ private static Map invertMap(Map map) { public static final class CoreSpanBuilder implements AgentTracer.SpanBuilder { private final CoreTracer tracer; + // Used to track whether the CoreSpanBuilder is actively being used + // CoreSpanBuilder becomes "inUse" after a succesful reset and remains "inUse" until "build" is + // called private boolean inUse = false; private String instrumentationName; From 8de4b30ef17588e4c5e824e7e4db68c539753361 Mon Sep 17 00:00:00 2001 From: Douglas Q Hawkins Date: Tue, 16 Sep 2025 11:32:56 -0400 Subject: [PATCH 06/43] Enabling by default --- internal-api/src/main/java/datadog/trace/api/Config.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/internal-api/src/main/java/datadog/trace/api/Config.java b/internal-api/src/main/java/datadog/trace/api/Config.java index 3478d5ceb5b..0f1173884d0 100644 --- a/internal-api/src/main/java/datadog/trace/api/Config.java +++ b/internal-api/src/main/java/datadog/trace/api/Config.java @@ -2751,7 +2751,7 @@ PROFILING_DATADOG_PROFILER_ENABLED, isDatadogProfilerSafeInCurrentEnvironment()) this.optimizedMapEnabled = configProvider.getBoolean(GeneralConfig.OPTIMIZED_MAP_ENABLED, false); this.spanBuilderReuseEnabled = - configProvider.getBoolean(GeneralConfig.SPAN_BUILDER_REUSE_ENABLED, false); + configProvider.getBoolean(GeneralConfig.SPAN_BUILDER_REUSE_ENABLED, true); this.tagNameUtf8CacheSize = Math.max(configProvider.getInteger(GeneralConfig.TAG_NAME_UTF8_CACHE_SIZE, 128), 0); this.tagValueUtf8CacheSize = From 5763a31cf91a2faae42a315392a08656cbd095d0 Mon Sep 17 00:00:00 2001 From: Douglas Q Hawkins Date: Tue, 16 Sep 2025 13:19:06 -0400 Subject: [PATCH 07/43] Adding reuse tests Refactored code, so tests work regardless of Config --- .../java/datadog/trace/core/CoreTracer.java | 53 ++++++++++++------ .../datadog/trace/core/CoreTracerTest.java | 55 +++++++++++++++++++ 2 files changed, 90 insertions(+), 18 deletions(-) create mode 100644 dd-trace-core/src/test/java/datadog/trace/core/CoreTracerTest.java diff --git a/dd-trace-core/src/main/java/datadog/trace/core/CoreTracer.java b/dd-trace-core/src/main/java/datadog/trace/core/CoreTracer.java index 15c044ef341..f1febb47c16 100644 --- a/dd-trace-core/src/main/java/datadog/trace/core/CoreTracer.java +++ b/dd-trace-core/src/main/java/datadog/trace/core/CoreTracer.java @@ -135,7 +135,7 @@ public class CoreTracer implements AgentTracer.TracerAPI { public static final BigInteger TRACE_ID_MAX = BigInteger.valueOf(2).pow(64).subtract(BigInteger.ONE); - public static CoreTracerBuilder builder() { + public static final CoreTracerBuilder builder() { return new CoreTracerBuilder(); } @@ -249,15 +249,8 @@ public static CoreTracerBuilder builder() { Config.get().isSpanBuilderReuseEnabled(); // Cache used by buildSpan - instance so it can capture the CoreTracer - private final ThreadLocal tlSpanBuilder = - SPAN_BUILDER_REUSE_ENABLED - ? new ThreadLocal() { - @Override - protected CoreSpanBuilder initialValue() { - return new CoreSpanBuilder(CoreTracer.this); - } - } - : null; + private final CoreSpanBuilderThreadLocalCache spanBuilderThreadLocalCache = + SPAN_BUILDER_REUSE_ENABLED ? new CoreSpanBuilderThreadLocalCache(this) : null; @Override public ConfigSnapshot captureTraceConfig() { @@ -989,22 +982,33 @@ public CoreSpanBuilder buildSpan( : this.createSpanBuilder(instrumentationName, operationName); } - private CoreSpanBuilder createSpanBuilder( + CoreSpanBuilder createSpanBuilder( final String instrumentationName, final CharSequence operationName) { CoreSpanBuilder newBuilder = new CoreSpanBuilder(this); newBuilder.reset(instrumentationName, operationName); return newBuilder; } - private CoreSpanBuilder reuseSpanBuilder( - final String instrumentationName, final CharSequence operationName) { - // retrieve the thread's typical SpanBuilder and check if it is currently in use - CoreSpanBuilder tlSpanBuilder = this.tlSpanBuilder.get(); + CoreSpanBuilder reuseSpanBuilder( + final String instrumentationName, final CharSequence operationName) + { + return reuseSpanBuilder(this, this.spanBuilderThreadLocalCache, instrumentationName, operationName); + } + + static final CoreSpanBuilder reuseSpanBuilder( + final CoreTracer tracer, + final CoreSpanBuilderThreadLocalCache tlCache, + final String instrumentationName, + final CharSequence operationName) + { + // retrieve the thread's typical SpanBuilder and try to reset it + // reset will fail if the CoreSpanBuilder is still "in-use" + CoreSpanBuilder tlSpanBuilder = tlCache.get(); boolean wasReset = tlSpanBuilder.reset(instrumentationName, operationName); if (wasReset) return tlSpanBuilder; // TODO: counter for how often the fallback is used? - CoreSpanBuilder newSpanBuilder = new CoreSpanBuilder(this); + CoreSpanBuilder newSpanBuilder = new CoreSpanBuilder(tracer); newSpanBuilder.reset(instrumentationName, operationName); // DQH - Debated how best to handle the case of someone requesting a CoreSpanBuilder @@ -1017,7 +1021,7 @@ private CoreSpanBuilder reuseSpanBuilder( // Instead of making the release process more complicating, I'm chosing to just // override here when we know we're already in an uncommon path. - this.tlSpanBuilder.set(newSpanBuilder); + tlCache.set(newSpanBuilder); return newSpanBuilder; } @@ -1448,6 +1452,19 @@ private static Map invertMap(Map map) { inverted.put(entry.getValue(), entry.getKey()); } return Collections.unmodifiableMap(inverted); + } + + static final class CoreSpanBuilderThreadLocalCache extends ThreadLocal { + private final CoreTracer tracer; + + public CoreSpanBuilderThreadLocalCache(CoreTracer tracer) { + this.tracer = tracer; + } + + @Override + protected CoreSpanBuilder initialValue() { + return new CoreSpanBuilder(this.tracer); + } } /** Spans are built using this builder */ @@ -1461,7 +1478,7 @@ public static final class CoreSpanBuilder implements AgentTracer.SpanBuilder { // Used to track whether the CoreSpanBuilder is actively being used // CoreSpanBuilder becomes "inUse" after a succesful reset and remains "inUse" until "build" is // called - private boolean inUse = false; + boolean inUse = false; private String instrumentationName; private CharSequence operationName; diff --git a/dd-trace-core/src/test/java/datadog/trace/core/CoreTracerTest.java b/dd-trace-core/src/test/java/datadog/trace/core/CoreTracerTest.java new file mode 100644 index 00000000000..6345d206778 --- /dev/null +++ b/dd-trace-core/src/test/java/datadog/trace/core/CoreTracerTest.java @@ -0,0 +1,55 @@ +package datadog.trace.core; + +import org.junit.Test; + +import static org.junit.Assert.assertFalse; +import static org.junit.Assert.assertTrue; +import static org.junit.Assert.assertNotSame; +import static org.junit.Assert.assertSame; + +import datadog.trace.core.CoreTracer.CoreSpanBuilder; +import datadog.trace.core.CoreTracer.CoreSpanBuilderThreadLocalCache; + +public class CoreTracerTest { + static final CoreTracer createTracer() { + return CoreTracer.builder().build(); + } + + @Test + public void spanBuilderReuse() { + CoreTracer tracer = createTracer(); + CoreSpanBuilderThreadLocalCache cache = new CoreSpanBuilderThreadLocalCache(tracer); + + CoreSpanBuilder builder1 = CoreTracer.reuseSpanBuilder(tracer, cache, "foo", "bar"); + assertTrue(builder1.inUse); + + builder1.start(); + assertFalse(builder1.inUse); + + CoreSpanBuilder builder2 = CoreTracer.reuseSpanBuilder(tracer, cache, "baz", "quux"); + assertTrue(builder2.inUse); + assertSame(builder1, builder2); + + builder2.start(); + assertFalse(builder2.inUse); + } + + @Test + public void spanBuilderReuse_stillInUse() { + CoreTracer tracer = createTracer(); + CoreSpanBuilderThreadLocalCache cache = new CoreSpanBuilderThreadLocalCache(tracer); + + CoreSpanBuilder builder1 = CoreTracer.reuseSpanBuilder(tracer, cache, "foo", "bar"); + assertTrue(builder1.inUse); + + CoreSpanBuilder builder2 = CoreTracer.reuseSpanBuilder(tracer, cache, "baz", "quux"); + assertTrue(builder2.inUse); + assertNotSame(builder1, builder2); + + builder2.start(); + assertFalse(builder2.inUse); + + builder1.start(); + assertFalse(builder1.inUse); + } +} From 23b03b87ca5f23a9dc217fbbf8fa680b2d13beb7 Mon Sep 17 00:00:00 2001 From: Douglas Q Hawkins Date: Tue, 16 Sep 2025 13:22:07 -0400 Subject: [PATCH 08/43] spotless --- .../java/datadog/trace/core/CoreTracer.java | 43 ++++++----- .../datadog/trace/core/CoreTracerTest.java | 71 +++++++++---------- 2 files changed, 56 insertions(+), 58 deletions(-) diff --git a/dd-trace-core/src/main/java/datadog/trace/core/CoreTracer.java b/dd-trace-core/src/main/java/datadog/trace/core/CoreTracer.java index 51e1b777937..04ddd7bd1df 100644 --- a/dd-trace-core/src/main/java/datadog/trace/core/CoreTracer.java +++ b/dd-trace-core/src/main/java/datadog/trace/core/CoreTracer.java @@ -984,19 +984,18 @@ CoreSpanBuilder createSpanBuilder( } CoreSpanBuilder reuseSpanBuilder( - final String instrumentationName, final CharSequence operationName) - { - return reuseSpanBuilder(this, this.spanBuilderThreadLocalCache, instrumentationName, operationName); + final String instrumentationName, final CharSequence operationName) { + return reuseSpanBuilder( + this, this.spanBuilderThreadLocalCache, instrumentationName, operationName); } - + static final CoreSpanBuilder reuseSpanBuilder( - final CoreTracer tracer, - final CoreSpanBuilderThreadLocalCache tlCache, - final String instrumentationName, - final CharSequence operationName) - { + final CoreTracer tracer, + final CoreSpanBuilderThreadLocalCache tlCache, + final String instrumentationName, + final CharSequence operationName) { // retrieve the thread's typical SpanBuilder and try to reset it - // reset will fail if the CoreSpanBuilder is still "in-use" + // reset will fail if the CoreSpanBuilder is still "in-use" CoreSpanBuilder tlSpanBuilder = tlCache.get(); boolean wasReset = tlSpanBuilder.reset(instrumentationName, operationName); if (wasReset) return tlSpanBuilder; @@ -1446,19 +1445,19 @@ private static Map invertMap(Map map) { inverted.put(entry.getValue(), entry.getKey()); } return Collections.unmodifiableMap(inverted); - } - + } + static final class CoreSpanBuilderThreadLocalCache extends ThreadLocal { - private final CoreTracer tracer; - - public CoreSpanBuilderThreadLocalCache(CoreTracer tracer) { - this.tracer = tracer; - } - - @Override - protected CoreSpanBuilder initialValue() { - return new CoreSpanBuilder(this.tracer); - } + private final CoreTracer tracer; + + public CoreSpanBuilderThreadLocalCache(CoreTracer tracer) { + this.tracer = tracer; + } + + @Override + protected CoreSpanBuilder initialValue() { + return new CoreSpanBuilder(this.tracer); + } } /** Spans are built using this builder */ diff --git a/dd-trace-core/src/test/java/datadog/trace/core/CoreTracerTest.java b/dd-trace-core/src/test/java/datadog/trace/core/CoreTracerTest.java index 6345d206778..cd1b01da046 100644 --- a/dd-trace-core/src/test/java/datadog/trace/core/CoreTracerTest.java +++ b/dd-trace-core/src/test/java/datadog/trace/core/CoreTracerTest.java @@ -1,55 +1,54 @@ package datadog.trace.core; -import org.junit.Test; - import static org.junit.Assert.assertFalse; -import static org.junit.Assert.assertTrue; import static org.junit.Assert.assertNotSame; import static org.junit.Assert.assertSame; +import static org.junit.Assert.assertTrue; import datadog.trace.core.CoreTracer.CoreSpanBuilder; import datadog.trace.core.CoreTracer.CoreSpanBuilderThreadLocalCache; +import org.junit.Test; public class CoreTracerTest { static final CoreTracer createTracer() { - return CoreTracer.builder().build(); + return CoreTracer.builder().build(); } - + @Test public void spanBuilderReuse() { - CoreTracer tracer = createTracer(); - CoreSpanBuilderThreadLocalCache cache = new CoreSpanBuilderThreadLocalCache(tracer); - - CoreSpanBuilder builder1 = CoreTracer.reuseSpanBuilder(tracer, cache, "foo", "bar"); - assertTrue(builder1.inUse); - - builder1.start(); - assertFalse(builder1.inUse); - - CoreSpanBuilder builder2 = CoreTracer.reuseSpanBuilder(tracer, cache, "baz", "quux"); - assertTrue(builder2.inUse); - assertSame(builder1, builder2); - - builder2.start(); - assertFalse(builder2.inUse); + CoreTracer tracer = createTracer(); + CoreSpanBuilderThreadLocalCache cache = new CoreSpanBuilderThreadLocalCache(tracer); + + CoreSpanBuilder builder1 = CoreTracer.reuseSpanBuilder(tracer, cache, "foo", "bar"); + assertTrue(builder1.inUse); + + builder1.start(); + assertFalse(builder1.inUse); + + CoreSpanBuilder builder2 = CoreTracer.reuseSpanBuilder(tracer, cache, "baz", "quux"); + assertTrue(builder2.inUse); + assertSame(builder1, builder2); + + builder2.start(); + assertFalse(builder2.inUse); } - + @Test public void spanBuilderReuse_stillInUse() { - CoreTracer tracer = createTracer(); - CoreSpanBuilderThreadLocalCache cache = new CoreSpanBuilderThreadLocalCache(tracer); - - CoreSpanBuilder builder1 = CoreTracer.reuseSpanBuilder(tracer, cache, "foo", "bar"); - assertTrue(builder1.inUse); - - CoreSpanBuilder builder2 = CoreTracer.reuseSpanBuilder(tracer, cache, "baz", "quux"); - assertTrue(builder2.inUse); - assertNotSame(builder1, builder2); - - builder2.start(); - assertFalse(builder2.inUse); - - builder1.start(); - assertFalse(builder1.inUse); + CoreTracer tracer = createTracer(); + CoreSpanBuilderThreadLocalCache cache = new CoreSpanBuilderThreadLocalCache(tracer); + + CoreSpanBuilder builder1 = CoreTracer.reuseSpanBuilder(tracer, cache, "foo", "bar"); + assertTrue(builder1.inUse); + + CoreSpanBuilder builder2 = CoreTracer.reuseSpanBuilder(tracer, cache, "baz", "quux"); + assertTrue(builder2.inUse); + assertNotSame(builder1, builder2); + + builder2.start(); + assertFalse(builder2.inUse); + + builder1.start(); + assertFalse(builder1.inUse); } } From 68a6f0360a4131bf22396e7627d44d3dd247f52a Mon Sep 17 00:00:00 2001 From: Douglas Q Hawkins Date: Tue, 16 Sep 2025 16:10:46 -0400 Subject: [PATCH 09/43] Changed the API to be safe for atypical usage To avoid breaking any potential code that builds multiple spans from the same SpanBuilder, updated the SpanBuilder pooling approach Introduced a new method singleSpanBuilder which can build one and only one span, this method can be used by automatic instrumentation as an optimization. singleSpanBuilder is now used inside the startSpan convenience methods, since we know they only build and return one span. Any automatic instrumentation using startSpan gets the optimization for free. buildSpan maintains its original semantics, so all existing continues to work as is. --- .../java/datadog/trace/core/CoreTracer.java | 24 ++++++++--- .../datadog/trace/core/CoreTracerTest.java | 43 +++++++++++++------ .../instrumentation/api/AgentTracer.java | 12 ++++++ 3 files changed, 59 insertions(+), 20 deletions(-) diff --git a/dd-trace-core/src/main/java/datadog/trace/core/CoreTracer.java b/dd-trace-core/src/main/java/datadog/trace/core/CoreTracer.java index 04ddd7bd1df..f10de1127e7 100644 --- a/dd-trace-core/src/main/java/datadog/trace/core/CoreTracer.java +++ b/dd-trace-core/src/main/java/datadog/trace/core/CoreTracer.java @@ -971,9 +971,15 @@ long getTimeWithNanoTicks(long nanoTicks) { @Override public CoreSpanBuilder buildSpan( final String instrumentationName, final CharSequence operationName) { + return createSpanBuilder(instrumentationName, operationName); + } + + @Override + public CoreSpanBuilder singleSpanBuilder( + final String instrumentationName, final CharSequence operationName) { return SPAN_BUILDER_REUSE_ENABLED - ? this.reuseSpanBuilder(instrumentationName, operationName) - : this.createSpanBuilder(instrumentationName, operationName); + ? reuseSpanBuilder(instrumentationName, operationName) + : createSpanBuilder(instrumentationName, operationName); } CoreSpanBuilder createSpanBuilder( @@ -985,8 +991,7 @@ CoreSpanBuilder createSpanBuilder( CoreSpanBuilder reuseSpanBuilder( final String instrumentationName, final CharSequence operationName) { - return reuseSpanBuilder( - this, this.spanBuilderThreadLocalCache, instrumentationName, operationName); + return reuseSpanBuilder(this, spanBuilderThreadLocalCache, instrumentationName, operationName); } static final CoreSpanBuilder reuseSpanBuilder( @@ -1021,19 +1026,24 @@ static final CoreSpanBuilder reuseSpanBuilder( @Override public AgentSpan startSpan(final String instrumentationName, final CharSequence spanName) { - return buildSpan(instrumentationName, spanName).start(); + return singleSpanBuilder(instrumentationName, spanName).start(); } @Override public AgentSpan startSpan( final String instrumentationName, final CharSequence spanName, final long startTimeMicros) { - return buildSpan(instrumentationName, spanName).withStartTimestamp(startTimeMicros).start(); + return singleSpanBuilder(instrumentationName, spanName) + .withStartTimestamp(startTimeMicros) + .start(); } @Override public AgentSpan startSpan( String instrumentationName, final CharSequence spanName, final AgentSpanContext parent) { - return buildSpan(instrumentationName, spanName).ignoreActiveSpan().asChildOf(parent).start(); + return singleSpanBuilder(instrumentationName, spanName) + .ignoreActiveSpan() + .asChildOf(parent) + .start(); } @Override diff --git a/dd-trace-core/src/test/java/datadog/trace/core/CoreTracerTest.java b/dd-trace-core/src/test/java/datadog/trace/core/CoreTracerTest.java index cd1b01da046..ba66c6c8c87 100644 --- a/dd-trace-core/src/test/java/datadog/trace/core/CoreTracerTest.java +++ b/dd-trace-core/src/test/java/datadog/trace/core/CoreTracerTest.java @@ -5,27 +5,47 @@ import static org.junit.Assert.assertSame; import static org.junit.Assert.assertTrue; +import datadog.trace.api.Config; import datadog.trace.core.CoreTracer.CoreSpanBuilder; import datadog.trace.core.CoreTracer.CoreSpanBuilderThreadLocalCache; import org.junit.Test; -public class CoreTracerTest { - static final CoreTracer createTracer() { - return CoreTracer.builder().build(); +public final class CoreTracerTest { + static final CoreTracer TRACER = CoreTracer.builder().build(); + static final CoreSpanBuilderThreadLocalCache CACHE = new CoreSpanBuilderThreadLocalCache(TRACER); + + @Test + public void buildSpan() { + CoreSpanBuilder builder1 = TRACER.buildSpan("foo", "bar"); + CoreSpanBuilder builder2 = TRACER.buildSpan("foo", "bar"); + + assertNotSame(builder1, builder2); } @Test - public void spanBuilderReuse() { - CoreTracer tracer = createTracer(); - CoreSpanBuilderThreadLocalCache cache = new CoreSpanBuilderThreadLocalCache(tracer); + public void singleUseSpanBuilder() { + CoreSpanBuilder builder1 = TRACER.buildSpan("foo", "bar"); + builder1.start(); - CoreSpanBuilder builder1 = CoreTracer.reuseSpanBuilder(tracer, cache, "foo", "bar"); + CoreSpanBuilder builder2 = TRACER.buildSpan("baz", "quux"); + builder2.start(); + + if (Config.get().isSpanBuilderReuseEnabled()) { + assertSame(builder1, builder2); + } else { + assertNotSame(builder1, builder2); + } + } + + @Test + public void spanBuilderReuse() { + CoreSpanBuilder builder1 = CoreTracer.reuseSpanBuilder(TRACER, CACHE, "foo", "bar"); assertTrue(builder1.inUse); builder1.start(); assertFalse(builder1.inUse); - CoreSpanBuilder builder2 = CoreTracer.reuseSpanBuilder(tracer, cache, "baz", "quux"); + CoreSpanBuilder builder2 = CoreTracer.reuseSpanBuilder(TRACER, CACHE, "baz", "quux"); assertTrue(builder2.inUse); assertSame(builder1, builder2); @@ -35,13 +55,10 @@ public void spanBuilderReuse() { @Test public void spanBuilderReuse_stillInUse() { - CoreTracer tracer = createTracer(); - CoreSpanBuilderThreadLocalCache cache = new CoreSpanBuilderThreadLocalCache(tracer); - - CoreSpanBuilder builder1 = CoreTracer.reuseSpanBuilder(tracer, cache, "foo", "bar"); + CoreSpanBuilder builder1 = CoreTracer.reuseSpanBuilder(TRACER, CACHE, "foo", "bar"); assertTrue(builder1.inUse); - CoreSpanBuilder builder2 = CoreTracer.reuseSpanBuilder(tracer, cache, "baz", "quux"); + CoreSpanBuilder builder2 = CoreTracer.reuseSpanBuilder(TRACER, CACHE, "baz", "quux"); assertTrue(builder2.inUse); assertNotSame(builder1, builder2); diff --git a/internal-api/src/main/java/datadog/trace/bootstrap/instrumentation/api/AgentTracer.java b/internal-api/src/main/java/datadog/trace/bootstrap/instrumentation/api/AgentTracer.java index e795f6c98dc..9c6c7681ad2 100644 --- a/internal-api/src/main/java/datadog/trace/bootstrap/instrumentation/api/AgentTracer.java +++ b/internal-api/src/main/java/datadog/trace/bootstrap/instrumentation/api/AgentTracer.java @@ -375,6 +375,12 @@ default SpanBuilder buildSpan(CharSequence spanName) { SpanBuilder buildSpan(String instrumentationName, CharSequence spanName); + /** + * Returns a SpanBuilder that can be used to produce one and only one span. By imposing the + * single span creation limitation, this method is more efficient than {@link buildSpan} + */ + SpanBuilder singleSpanBuilder(String instrumentationName, CharSequence spanName); + void close(); /** @@ -543,6 +549,12 @@ public SpanBuilder buildSpan(final String instrumentationName, final CharSequenc return null; } + @Override + public SpanBuilder singleSpanBuilder( + final String instrumentationName, final CharSequence spanName) { + return null; + } + @Override public void close() {} From 2d22b41f0d427dc1360fae69f4c1adcfdd54211f Mon Sep 17 00:00:00 2001 From: Douglas Q Hawkins Date: Wed, 17 Sep 2025 08:24:58 -0400 Subject: [PATCH 10/43] Fleshing out tests --- .../java/datadog/trace/core/CoreTracerTest.java | 13 +++++++++++-- 1 file changed, 11 insertions(+), 2 deletions(-) diff --git a/dd-trace-core/src/test/java/datadog/trace/core/CoreTracerTest.java b/dd-trace-core/src/test/java/datadog/trace/core/CoreTracerTest.java index ba66c6c8c87..c3f4abd7e89 100644 --- a/dd-trace-core/src/test/java/datadog/trace/core/CoreTracerTest.java +++ b/dd-trace-core/src/test/java/datadog/trace/core/CoreTracerTest.java @@ -16,18 +16,25 @@ public final class CoreTracerTest { @Test public void buildSpan() { + // buildSpan allows for constructing multiple spans from each returned CoreSpanBuilder + // so buildSpan cannot recycle objects - even when SpanBuilder reuse is enabled CoreSpanBuilder builder1 = TRACER.buildSpan("foo", "bar"); + + // need to build/start a span to prove that builder isn't being recycled + builder1.start(); + CoreSpanBuilder builder2 = TRACER.buildSpan("foo", "bar"); + builder2.start(); assertNotSame(builder1, builder2); } @Test public void singleUseSpanBuilder() { - CoreSpanBuilder builder1 = TRACER.buildSpan("foo", "bar"); + CoreSpanBuilder builder1 = TRACER.singleSpanBuilder("foo", "bar"); builder1.start(); - CoreSpanBuilder builder2 = TRACER.buildSpan("baz", "quux"); + CoreSpanBuilder builder2 = TRACER.singleSpanBuilder("baz", "quux"); builder2.start(); if (Config.get().isSpanBuilderReuseEnabled()) { @@ -39,6 +46,7 @@ public void singleUseSpanBuilder() { @Test public void spanBuilderReuse() { + // Doesn't call reuseSpanBuilder(String, CharSeq) directly, since that will fail when the Config is disabled CoreSpanBuilder builder1 = CoreTracer.reuseSpanBuilder(TRACER, CACHE, "foo", "bar"); assertTrue(builder1.inUse); @@ -55,6 +63,7 @@ public void spanBuilderReuse() { @Test public void spanBuilderReuse_stillInUse() { + // Doesn't call reuseSpanBuilder(String, CharSeq) directly, since that will fail when the Config is disabled CoreSpanBuilder builder1 = CoreTracer.reuseSpanBuilder(TRACER, CACHE, "foo", "bar"); assertTrue(builder1.inUse); From 55bd5006fcbfbf530c548384b9b09c7f5b0f9a61 Mon Sep 17 00:00:00 2001 From: Douglas Q Hawkins Date: Wed, 17 Sep 2025 09:06:15 -0400 Subject: [PATCH 11/43] spotless --- .../java/datadog/trace/core/CoreTracerTest.java | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-) diff --git a/dd-trace-core/src/test/java/datadog/trace/core/CoreTracerTest.java b/dd-trace-core/src/test/java/datadog/trace/core/CoreTracerTest.java index c3f4abd7e89..0c641c65d69 100644 --- a/dd-trace-core/src/test/java/datadog/trace/core/CoreTracerTest.java +++ b/dd-trace-core/src/test/java/datadog/trace/core/CoreTracerTest.java @@ -16,13 +16,13 @@ public final class CoreTracerTest { @Test public void buildSpan() { - // buildSpan allows for constructing multiple spans from each returned CoreSpanBuilder - // so buildSpan cannot recycle objects - even when SpanBuilder reuse is enabled + // buildSpan allows for constructing multiple spans from each returned CoreSpanBuilder + // so buildSpan cannot recycle objects - even when SpanBuilder reuse is enabled CoreSpanBuilder builder1 = TRACER.buildSpan("foo", "bar"); - + // need to build/start a span to prove that builder isn't being recycled builder1.start(); - + CoreSpanBuilder builder2 = TRACER.buildSpan("foo", "bar"); builder2.start(); @@ -46,7 +46,8 @@ public void singleUseSpanBuilder() { @Test public void spanBuilderReuse() { - // Doesn't call reuseSpanBuilder(String, CharSeq) directly, since that will fail when the Config is disabled + // Doesn't call reuseSpanBuilder(String, CharSeq) directly, since that will fail when the Config + // is disabled CoreSpanBuilder builder1 = CoreTracer.reuseSpanBuilder(TRACER, CACHE, "foo", "bar"); assertTrue(builder1.inUse); @@ -63,7 +64,8 @@ public void spanBuilderReuse() { @Test public void spanBuilderReuse_stillInUse() { - // Doesn't call reuseSpanBuilder(String, CharSeq) directly, since that will fail when the Config is disabled + // Doesn't call reuseSpanBuilder(String, CharSeq) directly, since that will fail when the Config + // is disabled CoreSpanBuilder builder1 = CoreTracer.reuseSpanBuilder(TRACER, CACHE, "foo", "bar"); assertTrue(builder1.inUse); From 079d1f5d6ec7eafa65407aad61c12e8c6e76e114 Mon Sep 17 00:00:00 2001 From: Douglas Q Hawkins Date: Wed, 17 Sep 2025 19:45:21 -0400 Subject: [PATCH 12/43] Improving single threaded performance In a microbenchmark, buildSpan was performing worse than previously. To address, that shortcoming and to clean-up the code... Made CoreSpanBuilder abstract and introduced two child classes: MultiSpanBuilder and ReusableSingleSpanBuilder MultiSpanBuilder is used by buildSpan ReusableSingleSpanBuilder is used by singleSpanBuilder / startSpan (indirectly) --- .../java/datadog/trace/core/CoreTracer.java | 216 ++++++++++-------- .../datadog/trace/core/CoreTracerTest.java | 10 +- 2 files changed, 124 insertions(+), 102 deletions(-) diff --git a/dd-trace-core/src/main/java/datadog/trace/core/CoreTracer.java b/dd-trace-core/src/main/java/datadog/trace/core/CoreTracer.java index f10de1127e7..b4144c4d887 100644 --- a/dd-trace-core/src/main/java/datadog/trace/core/CoreTracer.java +++ b/dd-trace-core/src/main/java/datadog/trace/core/CoreTracer.java @@ -248,8 +248,8 @@ public static final CoreTracerBuilder builder() { Config.get().isSpanBuilderReuseEnabled(); // Cache used by buildSpan - instance so it can capture the CoreTracer - private final CoreSpanBuilderThreadLocalCache spanBuilderThreadLocalCache = - SPAN_BUILDER_REUSE_ENABLED ? new CoreSpanBuilderThreadLocalCache(this) : null; + private final ReusableSingleSpanBuilderThreadLocalCache spanBuilderThreadLocalCache = + SPAN_BUILDER_REUSE_ENABLED ? new ReusableSingleSpanBuilderThreadLocalCache(this) : null; @Override public ConfigSnapshot captureTraceConfig() { @@ -978,35 +978,33 @@ public CoreSpanBuilder buildSpan( public CoreSpanBuilder singleSpanBuilder( final String instrumentationName, final CharSequence operationName) { return SPAN_BUILDER_REUSE_ENABLED - ? reuseSpanBuilder(instrumentationName, operationName) + ? reuseSingleSpanBuilder(instrumentationName, operationName) : createSpanBuilder(instrumentationName, operationName); } CoreSpanBuilder createSpanBuilder( final String instrumentationName, final CharSequence operationName) { - CoreSpanBuilder newBuilder = new CoreSpanBuilder(this); - newBuilder.reset(instrumentationName, operationName); - return newBuilder; + return new MultiSpanBuilder(this, instrumentationName, operationName); } - CoreSpanBuilder reuseSpanBuilder( + CoreSpanBuilder reuseSingleSpanBuilder( final String instrumentationName, final CharSequence operationName) { - return reuseSpanBuilder(this, spanBuilderThreadLocalCache, instrumentationName, operationName); + return reuseSingleSpanBuilder(this, spanBuilderThreadLocalCache, instrumentationName, operationName); } - static final CoreSpanBuilder reuseSpanBuilder( + static final ReusableSingleSpanBuilder reuseSingleSpanBuilder( final CoreTracer tracer, - final CoreSpanBuilderThreadLocalCache tlCache, + final ReusableSingleSpanBuilderThreadLocalCache tlCache, final String instrumentationName, final CharSequence operationName) { // retrieve the thread's typical SpanBuilder and try to reset it // reset will fail if the CoreSpanBuilder is still "in-use" - CoreSpanBuilder tlSpanBuilder = tlCache.get(); + ReusableSingleSpanBuilder tlSpanBuilder = tlCache.get(); boolean wasReset = tlSpanBuilder.reset(instrumentationName, operationName); if (wasReset) return tlSpanBuilder; // TODO: counter for how often the fallback is used? - CoreSpanBuilder newSpanBuilder = new CoreSpanBuilder(tracer); + ReusableSingleSpanBuilder newSpanBuilder = new ReusableSingleSpanBuilder(tracer); newSpanBuilder.reset(instrumentationName, operationName); // DQH - Debated how best to handle the case of someone requesting a CoreSpanBuilder @@ -1457,85 +1455,41 @@ private static Map invertMap(Map map) { return Collections.unmodifiableMap(inverted); } - static final class CoreSpanBuilderThreadLocalCache extends ThreadLocal { - private final CoreTracer tracer; - - public CoreSpanBuilderThreadLocalCache(CoreTracer tracer) { - this.tracer = tracer; - } - - @Override - protected CoreSpanBuilder initialValue() { - return new CoreSpanBuilder(this.tracer); - } - } - /** Spans are built using this builder */ - /* - * To reduce overhead, CoreSpanBuilders are reused. - * For simplicity, CoreSpanBuilder isn't thread safe, so CoreSpanBuilders can only be reused within one thread. - */ - public static final class CoreSpanBuilder implements AgentTracer.SpanBuilder { - private final CoreTracer tracer; - - // Used to track whether the CoreSpanBuilder is actively being used - // CoreSpanBuilder becomes "inUse" after a succesful reset and remains "inUse" until "build" is - // called - boolean inUse = false; + public abstract static class CoreSpanBuilder implements AgentTracer.SpanBuilder { + protected final CoreTracer tracer; - private String instrumentationName; - private CharSequence operationName; + protected String instrumentationName; + protected CharSequence operationName; // Builder attributes - private TagMap.Ledger tagLedger; - private long timestampMicro; - private AgentSpanContext parent; - private String serviceName; - private String resourceName; - private boolean errorFlag; - private CharSequence spanType; - private boolean ignoreScope = false; - private Object builderRequestContextDataAppSec; - private Object builderRequestContextDataIast; - private Object builderCiVisibilityContextData; - private List links; - private long spanId; + protected TagMap.Ledger tagLedger; + protected long timestampMicro; + protected AgentSpanContext parent; + protected String serviceName; + protected String resourceName; + protected boolean errorFlag; + protected CharSequence spanType; + protected boolean ignoreScope = false; + protected Object builderRequestContextDataAppSec; + protected Object builderRequestContextDataIast; + protected Object builderCiVisibilityContextData; + protected List links; + protected long spanId; CoreSpanBuilder(CoreTracer tracer) { this.tracer = tracer; } - boolean reset(String instrumentationName, CharSequence operationName) { - if (this.inUse) return false; - this.inUse = true; - - this.instrumentationName = instrumentationName; - this.operationName = operationName; - - if (this.tagLedger != null) this.tagLedger.reset(); - this.timestampMicro = 0L; - this.parent = null; - this.serviceName = null; - this.resourceName = null; - this.errorFlag = false; - this.spanType = null; - this.ignoreScope = false; - this.builderRequestContextDataAppSec = null; - this.builderRequestContextDataIast = null; - this.builderCiVisibilityContextData = null; - this.links = null; - this.spanId = 0L; - - return true; - } - @Override - public CoreSpanBuilder ignoreActiveSpan() { + public final CoreSpanBuilder ignoreActiveSpan() { ignoreScope = true; return this; } - private DDSpan buildSpan() { + protected abstract DDSpan buildSpan(); + + protected final DDSpan buildSpanImpl() { DDSpan span = DDSpan.create(instrumentationName, timestampMicro, buildSpanContext(), links); if (span.isLocalRootSpan()) { EndpointTracker tracker = tracer.onRootSpanStarted(span); @@ -1543,12 +1497,10 @@ private DDSpan buildSpan() { span.setEndpointTracker(tracker); } } - - this.inUse = false; return span; } - private void addParentContextAsLinks(AgentSpanContext parentContext) { + private final void addParentContextAsLinks(AgentSpanContext parentContext) { SpanLink link; if (parentContext instanceof ExtractedContext) { String headers = ((ExtractedContext) parentContext).getPropagationStyle().toString(); @@ -1564,7 +1516,7 @@ private void addParentContextAsLinks(AgentSpanContext parentContext) { withLink(link); } - private void addTerminatedContextAsLinks() { + private final void addTerminatedContextAsLinks() { if (this.parent instanceof TagContext) { List terminatedContextLinks = ((TagContext) this.parent).getTerminatedContextLinks(); @@ -1578,7 +1530,7 @@ private void addTerminatedContextAsLinks() { } @Override - public AgentSpan start() { + public final AgentSpan start() { AgentSpanContext pc = parent; if (pc == null && !ignoreScope) { final AgentSpan span = tracer.activeSpan(); @@ -1594,65 +1546,65 @@ public AgentSpan start() { } @Override - public CoreSpanBuilder withTag(final String tag, final Number number) { + public final CoreSpanBuilder withTag(final String tag, final Number number) { return withTag(tag, (Object) number); } @Override - public CoreSpanBuilder withTag(final String tag, final String string) { + public final CoreSpanBuilder withTag(final String tag, final String string) { return withTag(tag, (Object) (string == null || string.isEmpty() ? null : string)); } @Override - public CoreSpanBuilder withTag(final String tag, final boolean bool) { + public final CoreSpanBuilder withTag(final String tag, final boolean bool) { return withTag(tag, (Object) bool); } @Override - public CoreSpanBuilder withStartTimestamp(final long timestampMicroseconds) { + public final CoreSpanBuilder withStartTimestamp(final long timestampMicroseconds) { timestampMicro = timestampMicroseconds; return this; } @Override - public CoreSpanBuilder withServiceName(final String serviceName) { + public final CoreSpanBuilder withServiceName(final String serviceName) { this.serviceName = serviceName; return this; } @Override - public CoreSpanBuilder withResourceName(final String resourceName) { + public final CoreSpanBuilder withResourceName(final String resourceName) { this.resourceName = resourceName; return this; } @Override - public CoreSpanBuilder withErrorFlag() { + public final CoreSpanBuilder withErrorFlag() { errorFlag = true; return this; } @Override - public CoreSpanBuilder withSpanType(final CharSequence spanType) { + public final CoreSpanBuilder withSpanType(final CharSequence spanType) { this.spanType = spanType; return this; } @Override - public CoreSpanBuilder asChildOf(final AgentSpanContext spanContext) { + public final CoreSpanBuilder asChildOf(final AgentSpanContext spanContext) { // TODO we will start propagating stack trace hash and it will need to // be extracted here if available parent = spanContext; return this; } - public CoreSpanBuilder asChildOf(final AgentSpan agentSpan) { + public final CoreSpanBuilder asChildOf(final AgentSpan agentSpan) { parent = agentSpan.context(); return this; } @Override - public CoreSpanBuilder withTag(final String tag, final Object value) { + public final CoreSpanBuilder withTag(final String tag, final Object value) { if (tag == null) { return this; } @@ -1676,7 +1628,8 @@ public CoreSpanBuilder withTag(final String tag, final Object value) { } @Override - public AgentTracer.SpanBuilder withRequestContextData(RequestContextSlot slot, T data) { + public final AgentTracer.SpanBuilder withRequestContextData( + RequestContextSlot slot, T data) { switch (slot) { case APPSEC: builderRequestContextDataAppSec = data; @@ -1692,7 +1645,7 @@ public AgentTracer.SpanBuilder withRequestContextData(RequestContextSlot slo } @Override - public AgentTracer.SpanBuilder withLink(AgentSpanLink link) { + public final AgentTracer.SpanBuilder withLink(AgentSpanLink link) { if (link != null) { if (this.links == null) { this.links = new ArrayList<>(); @@ -1703,7 +1656,7 @@ public AgentTracer.SpanBuilder withLink(AgentSpanLink link) { } @Override - public CoreSpanBuilder withSpanId(final long spanId) { + public final CoreSpanBuilder withSpanId(final long spanId) { this.spanId = spanId; return this; } @@ -1714,7 +1667,7 @@ public CoreSpanBuilder withSpanId(final long spanId) { * * @return the context */ - private DDSpanContext buildSpanContext() { + private final DDSpanContext buildSpanContext() { final DDTraceId traceId; final long spanId; final long parentSpanId; @@ -1963,6 +1916,75 @@ private DDSpanContext buildSpanContext() { } } + static final class MultiSpanBuilder extends CoreSpanBuilder { + MultiSpanBuilder(CoreTracer tracer, String instrumentationName, CharSequence operationName) { + super(tracer); + this.instrumentationName = instrumentationName; + this.operationName = operationName; + } + + @Override + protected DDSpan buildSpan() { + return this.buildSpanImpl(); + } + } + + static final class ReusableSingleSpanBuilderThreadLocalCache extends ThreadLocal { + private final CoreTracer tracer; + + public ReusableSingleSpanBuilderThreadLocalCache(CoreTracer tracer) { + this.tracer = tracer; + } + + @Override + protected ReusableSingleSpanBuilder initialValue() { + return new ReusableSingleSpanBuilder(this.tracer); + } + } + + static final class ReusableSingleSpanBuilder extends CoreSpanBuilder { + // Used to track whether the CoreSpanBuilder is actively being used + // CoreSpanBuilder becomes "inUse" after a succesful reset and remains "inUse" until "build" is + // called + protected boolean inUse = false; + + ReusableSingleSpanBuilder(CoreTracer tracer) { + super(tracer); + this.inUse = false; + } + + final boolean reset(String instrumentationName, CharSequence operationName) { + if (this.inUse) return false; + this.inUse = true; + + this.instrumentationName = instrumentationName; + this.operationName = operationName; + + if (this.tagLedger != null) this.tagLedger.reset(); + this.timestampMicro = 0L; + this.parent = null; + this.serviceName = null; + this.resourceName = null; + this.errorFlag = false; + this.spanType = null; + this.ignoreScope = false; + this.builderRequestContextDataAppSec = null; + this.builderRequestContextDataIast = null; + this.builderCiVisibilityContextData = null; + this.links = null; + this.spanId = 0L; + + return true; + } + + @Override + protected DDSpan buildSpan() { + DDSpan span = this.buildSpanImpl(); + this.inUse = false; + return span; + } + } + private static class ShutdownHook extends Thread { private final WeakReference reference; diff --git a/dd-trace-core/src/test/java/datadog/trace/core/CoreTracerTest.java b/dd-trace-core/src/test/java/datadog/trace/core/CoreTracerTest.java index 0c641c65d69..92f4e44c8c5 100644 --- a/dd-trace-core/src/test/java/datadog/trace/core/CoreTracerTest.java +++ b/dd-trace-core/src/test/java/datadog/trace/core/CoreTracerTest.java @@ -7,7 +7,7 @@ import datadog.trace.api.Config; import datadog.trace.core.CoreTracer.CoreSpanBuilder; -import datadog.trace.core.CoreTracer.CoreSpanBuilderThreadLocalCache; +import datadog.trace.core.CoreTracer.ReusableSingleSpanBuilder; import org.junit.Test; public final class CoreTracerTest { @@ -48,13 +48,13 @@ public void singleUseSpanBuilder() { public void spanBuilderReuse() { // Doesn't call reuseSpanBuilder(String, CharSeq) directly, since that will fail when the Config // is disabled - CoreSpanBuilder builder1 = CoreTracer.reuseSpanBuilder(TRACER, CACHE, "foo", "bar"); + ReusableSingleSpanBuilder builder1 = CoreTracer.reuseSingleSpanBuilder(TRACER, CACHE, "foo", "bar"); assertTrue(builder1.inUse); builder1.start(); assertFalse(builder1.inUse); - CoreSpanBuilder builder2 = CoreTracer.reuseSpanBuilder(TRACER, CACHE, "baz", "quux"); + ReusableSingleSpanBuilder builder2 = CoreTracer.reuseSingleSpanBuilder(TRACER, CACHE, "baz", "quux"); assertTrue(builder2.inUse); assertSame(builder1, builder2); @@ -66,10 +66,10 @@ public void spanBuilderReuse() { public void spanBuilderReuse_stillInUse() { // Doesn't call reuseSpanBuilder(String, CharSeq) directly, since that will fail when the Config // is disabled - CoreSpanBuilder builder1 = CoreTracer.reuseSpanBuilder(TRACER, CACHE, "foo", "bar"); + ReusableSingleSpanBuilder builder1 = CoreTracer.reuseSingleSpanBuilder(TRACER, CACHE, "foo", "bar"); assertTrue(builder1.inUse); - CoreSpanBuilder builder2 = CoreTracer.reuseSpanBuilder(TRACER, CACHE, "baz", "quux"); + ReusableSingleSpanBuilder builder2 = CoreTracer.reuseSingleSpanBuilder(TRACER, CACHE, "baz", "quux"); assertTrue(builder2.inUse); assertNotSame(builder1, builder2); From a55095ed57b346194749e413889024475ebb8c5d Mon Sep 17 00:00:00 2001 From: Douglas Q Hawkins Date: Wed, 17 Sep 2025 19:46:04 -0400 Subject: [PATCH 13/43] spotless --- .../src/main/java/datadog/trace/core/CoreTracer.java | 6 ++++-- .../test/java/datadog/trace/core/CoreTracerTest.java | 12 ++++++++---- 2 files changed, 12 insertions(+), 6 deletions(-) diff --git a/dd-trace-core/src/main/java/datadog/trace/core/CoreTracer.java b/dd-trace-core/src/main/java/datadog/trace/core/CoreTracer.java index b4144c4d887..66c0656d481 100644 --- a/dd-trace-core/src/main/java/datadog/trace/core/CoreTracer.java +++ b/dd-trace-core/src/main/java/datadog/trace/core/CoreTracer.java @@ -989,7 +989,8 @@ CoreSpanBuilder createSpanBuilder( CoreSpanBuilder reuseSingleSpanBuilder( final String instrumentationName, final CharSequence operationName) { - return reuseSingleSpanBuilder(this, spanBuilderThreadLocalCache, instrumentationName, operationName); + return reuseSingleSpanBuilder( + this, spanBuilderThreadLocalCache, instrumentationName, operationName); } static final ReusableSingleSpanBuilder reuseSingleSpanBuilder( @@ -1929,7 +1930,8 @@ protected DDSpan buildSpan() { } } - static final class ReusableSingleSpanBuilderThreadLocalCache extends ThreadLocal { + static final class ReusableSingleSpanBuilderThreadLocalCache + extends ThreadLocal { private final CoreTracer tracer; public ReusableSingleSpanBuilderThreadLocalCache(CoreTracer tracer) { diff --git a/dd-trace-core/src/test/java/datadog/trace/core/CoreTracerTest.java b/dd-trace-core/src/test/java/datadog/trace/core/CoreTracerTest.java index 92f4e44c8c5..f90313fb6dc 100644 --- a/dd-trace-core/src/test/java/datadog/trace/core/CoreTracerTest.java +++ b/dd-trace-core/src/test/java/datadog/trace/core/CoreTracerTest.java @@ -48,13 +48,15 @@ public void singleUseSpanBuilder() { public void spanBuilderReuse() { // Doesn't call reuseSpanBuilder(String, CharSeq) directly, since that will fail when the Config // is disabled - ReusableSingleSpanBuilder builder1 = CoreTracer.reuseSingleSpanBuilder(TRACER, CACHE, "foo", "bar"); + ReusableSingleSpanBuilder builder1 = + CoreTracer.reuseSingleSpanBuilder(TRACER, CACHE, "foo", "bar"); assertTrue(builder1.inUse); builder1.start(); assertFalse(builder1.inUse); - ReusableSingleSpanBuilder builder2 = CoreTracer.reuseSingleSpanBuilder(TRACER, CACHE, "baz", "quux"); + ReusableSingleSpanBuilder builder2 = + CoreTracer.reuseSingleSpanBuilder(TRACER, CACHE, "baz", "quux"); assertTrue(builder2.inUse); assertSame(builder1, builder2); @@ -66,10 +68,12 @@ public void spanBuilderReuse() { public void spanBuilderReuse_stillInUse() { // Doesn't call reuseSpanBuilder(String, CharSeq) directly, since that will fail when the Config // is disabled - ReusableSingleSpanBuilder builder1 = CoreTracer.reuseSingleSpanBuilder(TRACER, CACHE, "foo", "bar"); + ReusableSingleSpanBuilder builder1 = + CoreTracer.reuseSingleSpanBuilder(TRACER, CACHE, "foo", "bar"); assertTrue(builder1.inUse); - ReusableSingleSpanBuilder builder2 = CoreTracer.reuseSingleSpanBuilder(TRACER, CACHE, "baz", "quux"); + ReusableSingleSpanBuilder builder2 = + CoreTracer.reuseSingleSpanBuilder(TRACER, CACHE, "baz", "quux"); assertTrue(builder2.inUse); assertNotSame(builder1, builder2); From e5161ef798c990e60a6a06abe416953feac5a13a Mon Sep 17 00:00:00 2001 From: Douglas Q Hawkins Date: Wed, 17 Sep 2025 21:29:19 -0400 Subject: [PATCH 14/43] Fixing test that renaming didn't update properly --- .../src/main/java/datadog/trace/core/CoreTracer.java | 1 + .../src/test/java/datadog/trace/core/CoreTracerTest.java | 4 +++- 2 files changed, 4 insertions(+), 1 deletion(-) diff --git a/dd-trace-core/src/main/java/datadog/trace/core/CoreTracer.java b/dd-trace-core/src/main/java/datadog/trace/core/CoreTracer.java index 66c0656d481..2001d908d4f 100644 --- a/dd-trace-core/src/main/java/datadog/trace/core/CoreTracer.java +++ b/dd-trace-core/src/main/java/datadog/trace/core/CoreTracer.java @@ -1463,6 +1463,7 @@ public abstract static class CoreSpanBuilder implements AgentTracer.SpanBuilder protected String instrumentationName; protected CharSequence operationName; + // Make sure any fields added here are also reset properly in ReusableSingleSpanBuilder.reset (below) // Builder attributes protected TagMap.Ledger tagLedger; protected long timestampMicro; diff --git a/dd-trace-core/src/test/java/datadog/trace/core/CoreTracerTest.java b/dd-trace-core/src/test/java/datadog/trace/core/CoreTracerTest.java index f90313fb6dc..0d76d19c36b 100644 --- a/dd-trace-core/src/test/java/datadog/trace/core/CoreTracerTest.java +++ b/dd-trace-core/src/test/java/datadog/trace/core/CoreTracerTest.java @@ -8,11 +8,13 @@ import datadog.trace.api.Config; import datadog.trace.core.CoreTracer.CoreSpanBuilder; import datadog.trace.core.CoreTracer.ReusableSingleSpanBuilder; +import datadog.trace.core.CoreTracer.ReusableSingleSpanBuilderThreadLocalCache; import org.junit.Test; public final class CoreTracerTest { static final CoreTracer TRACER = CoreTracer.builder().build(); - static final CoreSpanBuilderThreadLocalCache CACHE = new CoreSpanBuilderThreadLocalCache(TRACER); + static final ReusableSingleSpanBuilderThreadLocalCache CACHE = + new ReusableSingleSpanBuilderThreadLocalCache(TRACER); @Test public void buildSpan() { From 40ba13d4d61d3077322f2a15dba5224cf8b53c23 Mon Sep 17 00:00:00 2001 From: Douglas Q Hawkins Date: Wed, 17 Sep 2025 21:32:06 -0400 Subject: [PATCH 15/43] Adding benchmarks for span creation --- .../trace/core/CoreTracerBenchmark.java | 49 +++++++++++++++++++ 1 file changed, 49 insertions(+) create mode 100644 dd-trace-core/src/jmh/java/datadog/trace/core/CoreTracerBenchmark.java diff --git a/dd-trace-core/src/jmh/java/datadog/trace/core/CoreTracerBenchmark.java b/dd-trace-core/src/jmh/java/datadog/trace/core/CoreTracerBenchmark.java new file mode 100644 index 00000000000..d719264e2c3 --- /dev/null +++ b/dd-trace-core/src/jmh/java/datadog/trace/core/CoreTracerBenchmark.java @@ -0,0 +1,49 @@ +package datadog.trace.core; + +import static java.util.concurrent.TimeUnit.MICROSECONDS; + +import datadog.trace.bootstrap.instrumentation.api.AgentSpan; +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.State; +import org.openjdk.jmh.annotations.Threads; +import org.openjdk.jmh.annotations.Warmup; + +/** + * Benchmark of key operations of the CoreTracer + * + *

NOTE: This is a multi-threaded benchmark; single threaded benchmarks don't accurately reflect + * some of the optimizations. + * + *

Use -t 1, if you'd like to do a single threaded run + */ +@State(Scope.Benchmark) +@Warmup(iterations = 3) +@Measurement(iterations = 5) +@BenchmarkMode(Mode.Throughput) +@Threads(8) +@OutputTimeUnit(MICROSECONDS) +@Fork(value = 1) +public class CoreTracerBenchmark { + static final CoreTracer TRACER = CoreTracer.builder().build(); + + @Benchmark + public AgentSpan startSpan() { + return TRACER.startSpan("foo", "bar"); + } + + @Benchmark + public AgentSpan buildSpan() { + return TRACER.buildSpan("foo", "bar").start(); + } + + @Benchmark + public AgentSpan singleSpanBuilder() { + return TRACER.singleSpanBuilder("foo", "bar").start(); + } +} From 70eccff674419732fc9b483fa96b54c596fb4863 Mon Sep 17 00:00:00 2001 From: Douglas Q Hawkins Date: Thu, 18 Sep 2025 08:58:14 -0400 Subject: [PATCH 16/43] spotless --- dd-trace-core/src/main/java/datadog/trace/core/CoreTracer.java | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/dd-trace-core/src/main/java/datadog/trace/core/CoreTracer.java b/dd-trace-core/src/main/java/datadog/trace/core/CoreTracer.java index 2001d908d4f..a99c89388cf 100644 --- a/dd-trace-core/src/main/java/datadog/trace/core/CoreTracer.java +++ b/dd-trace-core/src/main/java/datadog/trace/core/CoreTracer.java @@ -1463,7 +1463,8 @@ public abstract static class CoreSpanBuilder implements AgentTracer.SpanBuilder protected String instrumentationName; protected CharSequence operationName; - // Make sure any fields added here are also reset properly in ReusableSingleSpanBuilder.reset (below) + // Make sure any fields added here are also reset properly in ReusableSingleSpanBuilder.reset + // (below) // Builder attributes protected TagMap.Ledger tagLedger; protected long timestampMicro; From 5b594a20ab65f528ef9cf4610e17ac17510ff13e Mon Sep 17 00:00:00 2001 From: Douglas Q Hawkins Date: Thu, 18 Sep 2025 09:17:04 -0400 Subject: [PATCH 17/43] Adding clarifying comments & more tests --- .../java/datadog/trace/core/CoreTracer.java | 8 +++--- .../datadog/trace/core/CoreTracerTest.java | 28 +++++++++++++++++++ 2 files changed, 32 insertions(+), 4 deletions(-) diff --git a/dd-trace-core/src/main/java/datadog/trace/core/CoreTracer.java b/dd-trace-core/src/main/java/datadog/trace/core/CoreTracer.java index a99c89388cf..a4e803d99f4 100644 --- a/dd-trace-core/src/main/java/datadog/trace/core/CoreTracer.java +++ b/dd-trace-core/src/main/java/datadog/trace/core/CoreTracer.java @@ -243,7 +243,7 @@ public static final CoreTracerBuilder builder() { private final PropagationTags.Factory propagationTagsFactory; // DQH - storing into a static constant, so value will constant propagate and dead code eliminate - // the other branch in buildSpan + // the other branch in singleSpanBuilder private static final boolean SPAN_BUILDER_REUSE_ENABLED = Config.get().isSpanBuilderReuseEnabled(); @@ -999,7 +999,7 @@ static final ReusableSingleSpanBuilder reuseSingleSpanBuilder( final String instrumentationName, final CharSequence operationName) { // retrieve the thread's typical SpanBuilder and try to reset it - // reset will fail if the CoreSpanBuilder is still "in-use" + // reset will fail if the ReusableSingleSpanBuilder is still "in-use" ReusableSingleSpanBuilder tlSpanBuilder = tlCache.get(); boolean wasReset = tlSpanBuilder.reset(instrumentationName, operationName); if (wasReset) return tlSpanBuilder; @@ -1008,8 +1008,8 @@ static final ReusableSingleSpanBuilder reuseSingleSpanBuilder( ReusableSingleSpanBuilder newSpanBuilder = new ReusableSingleSpanBuilder(tracer); newSpanBuilder.reset(instrumentationName, operationName); - // DQH - Debated how best to handle the case of someone requesting a CoreSpanBuilder - // and then not using it. Without an ability to replace the cached CoreSpanBuilder, + // DQH - Debated how best to handle the case of someone requesting a SpanBuilder + // and then not using it. Without an ability to replace the cached SpanBuilder, // that case could result in permanently burning the cache for a given thread. // That could be solved with additional logic during CoreSpanBuilder#build diff --git a/dd-trace-core/src/test/java/datadog/trace/core/CoreTracerTest.java b/dd-trace-core/src/test/java/datadog/trace/core/CoreTracerTest.java index 0d76d19c36b..8746bb2098f 100644 --- a/dd-trace-core/src/test/java/datadog/trace/core/CoreTracerTest.java +++ b/dd-trace-core/src/test/java/datadog/trace/core/CoreTracerTest.java @@ -85,4 +85,32 @@ public void spanBuilderReuse_stillInUse() { builder1.start(); assertFalse(builder1.inUse); } + + @Test + public void spanBuilderReuse_abandoned() { + // Doesn't call reuseSpanBuilder(String, CharSeq) directly, since that will fail when the Config + // is disabled + + ReusableSingleSpanBuilder abandonedBuilder = + CoreTracer.reuseSingleSpanBuilder(TRACER, CACHE, "foo", "bar"); + assertTrue(abandonedBuilder.inUse); + + // Requesting the next builder will replace the previous one in the thread local cache + // This is done, so that an abandoned builder doesn't permanently burn the cache for a thread + ReusableSingleSpanBuilder builder1 = + CoreTracer.reuseSingleSpanBuilder(TRACER, CACHE, "baz", "quux"); + assertTrue(builder1.inUse); + assertNotSame(abandonedBuilder, builder1); + + builder1.start(); + assertFalse(builder1.inUse); + + ReusableSingleSpanBuilder builder2 = + CoreTracer.reuseSingleSpanBuilder(TRACER, CACHE, "baz", "quux"); + assertTrue(builder2.inUse); + assertSame(builder1, builder2); + + builder2.start(); + assertFalse(builder2.inUse); + } } From 65fa1d7e29dd581716a9f92bd4c97ff1ff57cafa Mon Sep 17 00:00:00 2001 From: Douglas Q Hawkins Date: Thu, 18 Sep 2025 09:23:24 -0400 Subject: [PATCH 18/43] spotless --- .../datadog/trace/core/CoreTracerTest.java | 30 +++++++++---------- 1 file changed, 15 insertions(+), 15 deletions(-) diff --git a/dd-trace-core/src/test/java/datadog/trace/core/CoreTracerTest.java b/dd-trace-core/src/test/java/datadog/trace/core/CoreTracerTest.java index 8746bb2098f..c17582f31d5 100644 --- a/dd-trace-core/src/test/java/datadog/trace/core/CoreTracerTest.java +++ b/dd-trace-core/src/test/java/datadog/trace/core/CoreTracerTest.java @@ -85,31 +85,31 @@ public void spanBuilderReuse_stillInUse() { builder1.start(); assertFalse(builder1.inUse); } - + @Test public void spanBuilderReuse_abandoned() { - // Doesn't call reuseSpanBuilder(String, CharSeq) directly, since that will fail when the Config - // is disabled - - ReusableSingleSpanBuilder abandonedBuilder = - CoreTracer.reuseSingleSpanBuilder(TRACER, CACHE, "foo", "bar"); - assertTrue(abandonedBuilder.inUse); - - // Requesting the next builder will replace the previous one in the thread local cache - // This is done, so that an abandoned builder doesn't permanently burn the cache for a thread + // Doesn't call reuseSpanBuilder(String, CharSeq) directly, since that will fail when the Config + // is disabled + + ReusableSingleSpanBuilder abandonedBuilder = + CoreTracer.reuseSingleSpanBuilder(TRACER, CACHE, "foo", "bar"); + assertTrue(abandonedBuilder.inUse); + + // Requesting the next builder will replace the previous one in the thread local cache + // This is done, so that an abandoned builder doesn't permanently burn the cache for a thread ReusableSingleSpanBuilder builder1 = - CoreTracer.reuseSingleSpanBuilder(TRACER, CACHE, "baz", "quux"); + CoreTracer.reuseSingleSpanBuilder(TRACER, CACHE, "baz", "quux"); assertTrue(builder1.inUse); assertNotSame(abandonedBuilder, builder1); - + builder1.start(); assertFalse(builder1.inUse); - + ReusableSingleSpanBuilder builder2 = - CoreTracer.reuseSingleSpanBuilder(TRACER, CACHE, "baz", "quux"); + CoreTracer.reuseSingleSpanBuilder(TRACER, CACHE, "baz", "quux"); assertTrue(builder2.inUse); assertSame(builder1, builder2); - + builder2.start(); assertFalse(builder2.inUse); } From 136329e8e7e6be6a8b68fcdac5d2d4b7ac9b2524 Mon Sep 17 00:00:00 2001 From: Douglas Q Hawkins Date: Thu, 18 Sep 2025 09:23:55 -0400 Subject: [PATCH 19/43] tweaking comments --- .../src/test/java/datadog/trace/core/CoreTracerTest.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/dd-trace-core/src/test/java/datadog/trace/core/CoreTracerTest.java b/dd-trace-core/src/test/java/datadog/trace/core/CoreTracerTest.java index c17582f31d5..1003639c84c 100644 --- a/dd-trace-core/src/test/java/datadog/trace/core/CoreTracerTest.java +++ b/dd-trace-core/src/test/java/datadog/trace/core/CoreTracerTest.java @@ -96,7 +96,7 @@ public void spanBuilderReuse_abandoned() { assertTrue(abandonedBuilder.inUse); // Requesting the next builder will replace the previous one in the thread local cache - // This is done, so that an abandoned builder doesn't permanently burn the cache for a thread + // This is done so that an abandoned builder doesn't permanently burn the cache for a thread ReusableSingleSpanBuilder builder1 = CoreTracer.reuseSingleSpanBuilder(TRACER, CACHE, "baz", "quux"); assertTrue(builder1.inUse); From 2e7da543bf54b41edb6d386152bc7eba6fa2db35 Mon Sep 17 00:00:00 2001 From: Douglas Q Hawkins Date: Thu, 18 Sep 2025 09:35:55 -0400 Subject: [PATCH 20/43] More comments --- .../src/main/java/datadog/trace/core/CoreTracer.java | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/dd-trace-core/src/main/java/datadog/trace/core/CoreTracer.java b/dd-trace-core/src/main/java/datadog/trace/core/CoreTracer.java index a4e803d99f4..192fcf3baac 100644 --- a/dd-trace-core/src/main/java/datadog/trace/core/CoreTracer.java +++ b/dd-trace-core/src/main/java/datadog/trace/core/CoreTracer.java @@ -1957,6 +1957,11 @@ static final class ReusableSingleSpanBuilder extends CoreSpanBuilder { this.inUse = false; } + /** + * Resets the ReusableSingleSpanBuilder, so it may be used to build another single span Returns + * true if the reset was successful Returns false if this ReusableSingleSpanBuilder is still + * "in-use" + */ final boolean reset(String instrumentationName, CharSequence operationName) { if (this.inUse) return false; this.inUse = true; @@ -1981,6 +1986,9 @@ final boolean reset(String instrumentationName, CharSequence operationName) { return true; } + /* + * Clears the inUse boolean, so this ReusableSpanBuilder can be reset + */ @Override protected DDSpan buildSpan() { DDSpan span = this.buildSpanImpl(); From 98d56035f819a5e54941d1ea391a434de0c9e69a Mon Sep 17 00:00:00 2001 From: Douglas Q Hawkins Date: Thu, 18 Sep 2025 09:41:06 -0400 Subject: [PATCH 21/43] More comment clean-up --- .../src/main/java/datadog/trace/core/CoreTracer.java | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/dd-trace-core/src/main/java/datadog/trace/core/CoreTracer.java b/dd-trace-core/src/main/java/datadog/trace/core/CoreTracer.java index 192fcf3baac..97ccae5d1b1 100644 --- a/dd-trace-core/src/main/java/datadog/trace/core/CoreTracer.java +++ b/dd-trace-core/src/main/java/datadog/trace/core/CoreTracer.java @@ -1012,11 +1012,11 @@ static final ReusableSingleSpanBuilder reuseSingleSpanBuilder( // and then not using it. Without an ability to replace the cached SpanBuilder, // that case could result in permanently burning the cache for a given thread. - // That could be solved with additional logic during CoreSpanBuilder#build - // that checks to see if the cached CoreSpanBuilder is in use and then replaces it - // with the freed CoreSpanBuilder, but that would put extra logic in the common path. + // That could be solved with additional logic during ReusableSingleSpanBuilder#buildSpan + // that checks to see if the cached the Builder is in use and then replaces it + // with the freed Builder, but that would put extra logic in the common path. - // Instead of making the release process more complicating, I'm chosing to just + // Instead of making the release process more complicated, I'm chosing to just // override here when we know we're already in an uncommon path. tlCache.set(newSpanBuilder); From ff9acbe38fdc8bbbc9154e86dfac92618d61754a Mon Sep 17 00:00:00 2001 From: Douglas Q Hawkins Date: Thu, 18 Sep 2025 21:57:11 -0400 Subject: [PATCH 22/43] Addressing review feedback Added an init instead of calling reset when creating a new ReusableSingleSpanBuilder Added some asserts to catch misuse of the API --- .../main/java/datadog/trace/core/CoreTracer.java | 15 +++++++++++++-- 1 file changed, 13 insertions(+), 2 deletions(-) diff --git a/dd-trace-core/src/main/java/datadog/trace/core/CoreTracer.java b/dd-trace-core/src/main/java/datadog/trace/core/CoreTracer.java index 97ccae5d1b1..ad1e6e3900a 100644 --- a/dd-trace-core/src/main/java/datadog/trace/core/CoreTracer.java +++ b/dd-trace-core/src/main/java/datadog/trace/core/CoreTracer.java @@ -1006,7 +1006,7 @@ static final ReusableSingleSpanBuilder reuseSingleSpanBuilder( // TODO: counter for how often the fallback is used? ReusableSingleSpanBuilder newSpanBuilder = new ReusableSingleSpanBuilder(tracer); - newSpanBuilder.reset(instrumentationName, operationName); + newSpanBuilder.init(instrumentationName, operationName); // DQH - Debated how best to handle the case of someone requesting a SpanBuilder // and then not using it. Without an ability to replace the cached SpanBuilder, @@ -1950,13 +1950,21 @@ static final class ReusableSingleSpanBuilder extends CoreSpanBuilder { // Used to track whether the CoreSpanBuilder is actively being used // CoreSpanBuilder becomes "inUse" after a succesful reset and remains "inUse" until "build" is // called - protected boolean inUse = false; + protected boolean inUse; ReusableSingleSpanBuilder(CoreTracer tracer) { super(tracer); this.inUse = false; } + /** Similar to reset, but only valid on first use */ + void init(String instrumentationName, CharSequence operationName) { + assert !this.inUse; + + this.instrumentationName = instrumentationName; + this.operationName = operationName; + } + /** * Resets the ReusableSingleSpanBuilder, so it may be used to build another single span Returns * true if the reset was successful Returns false if this ReusableSingleSpanBuilder is still @@ -1991,6 +1999,9 @@ final boolean reset(String instrumentationName, CharSequence operationName) { */ @Override protected DDSpan buildSpan() { + assert this.inUse + : "ReusableSingleSpanBuilder not reset properly -- multiple span construction?"; + DDSpan span = this.buildSpanImpl(); this.inUse = false; return span; From 4475e424993bcb7075c0979a2fa42a5b8fdb55ae Mon Sep 17 00:00:00 2001 From: Douglas Q Hawkins Date: Tue, 7 Oct 2025 11:22:10 -0400 Subject: [PATCH 23/43] Adding overload to just check major version --- .../src/main/java/datadog/environment/JavaVersion.java | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/components/environment/src/main/java/datadog/environment/JavaVersion.java b/components/environment/src/main/java/datadog/environment/JavaVersion.java index 48aaed0c66e..e880adfa158 100644 --- a/components/environment/src/main/java/datadog/environment/JavaVersion.java +++ b/components/environment/src/main/java/datadog/environment/JavaVersion.java @@ -87,6 +87,10 @@ public boolean is(int major, int minor, int update) { return this.major == major && this.minor == minor && this.update == update; } + public boolean isAtLeast(int major) { + return isAtLeast(major, 0, 0); + } + public boolean isAtLeast(int major, int minor, int update) { return isAtLeast(this.major, this.minor, this.update, major, minor, update); } From 4d2ec09a6eaaf2f59057ce05b8abbe7df18185e0 Mon Sep 17 00:00:00 2001 From: Douglas Q Hawkins Date: Tue, 7 Oct 2025 11:23:01 -0400 Subject: [PATCH 24/43] Adding ThreadUtils Adding ThreadUtils class that enables checking if Threads are virtual threads --- .../java/datadog/environment/ThreadUtils.java | 85 ++++++++++++++ .../datadog/environment/ThreadUtilsTest.java | 106 ++++++++++++++++++ 2 files changed, 191 insertions(+) create mode 100644 components/environment/src/main/java/datadog/environment/ThreadUtils.java create mode 100644 components/environment/src/test/java/datadog/environment/ThreadUtilsTest.java diff --git a/components/environment/src/main/java/datadog/environment/ThreadUtils.java b/components/environment/src/main/java/datadog/environment/ThreadUtils.java new file mode 100644 index 00000000000..e07ccb44dce --- /dev/null +++ b/components/environment/src/main/java/datadog/environment/ThreadUtils.java @@ -0,0 +1,85 @@ +package datadog.environment; + +import java.lang.invoke.MethodHandle; +import java.lang.invoke.MethodHandles; +import java.lang.invoke.MethodType; + +/** + * Helper class for working with Threads + * + *

Uses feature detection and provides static helpers to work with different versions of Java + * + *

This class is designed to use MethodHandles that constant propagate to minimize the overhead + */ +public final class ThreadUtils { + static final MethodHandle H_IS_VIRTUAL = lookupIsVirtual(); + static final MethodHandle H_ID = lookupId(); + + private ThreadUtils() {} + + /** Provides the best id available for the Thread Uses threadId on 19+; getId on older JVMs */ + public static final long threadId(Thread thread) { + try { + return (long) H_ID.invoke(thread); + } catch (Throwable t) { + return 0L; + } + } + + /** Indicates whether virtual threads are supported on this JVM */ + public static final boolean supportsVirtualThreads() { + return (H_IS_VIRTUAL != null); + } + + /** Indicates if the current thread is a virtual thread */ + public static final boolean isCurrentThreadVirtual() { + // H_IS_VIRTUAL will constant propagate -- then dead code eliminate -- and inline as needed + try { + return (H_IS_VIRTUAL != null) && (boolean) H_IS_VIRTUAL.invoke(Thread.currentThread()); + } catch (Throwable t) { + return false; + } + } + + /** Indicates if the provided thread is a virtual thread */ + public static final boolean isVirtual(Thread thread) { + // H_IS_VIRTUAL will constant propagate -- then dead code eliminate -- and inline as needed + try { + return (H_IS_VIRTUAL != null) && (boolean) H_IS_VIRTUAL.invoke(thread); + } catch (Throwable t) { + return false; + } + } + + private static final MethodHandle lookupIsVirtual() { + try { + return MethodHandles.lookup() + .findVirtual(Thread.class, "isVirtual", MethodType.methodType(boolean.class)); + } catch (NoSuchMethodException | IllegalAccessException e) { + return null; + } + } + + private static final MethodHandle lookupId() { + MethodHandle threadIdHandle = lookupThreadId(); + return threadIdHandle != null ? threadIdHandle : lookupGetId(); + } + + private static final MethodHandle lookupThreadId() { + try { + return MethodHandles.lookup() + .findVirtual(Thread.class, "threadId", MethodType.methodType(long.class)); + } catch (NoSuchMethodException | IllegalAccessException e) { + return null; + } + } + + private static final MethodHandle lookupGetId() { + try { + return MethodHandles.lookup() + .findVirtual(Thread.class, "getId", MethodType.methodType(long.class)); + } catch (NoSuchMethodException | IllegalAccessException e) { + return null; + } + } +} diff --git a/components/environment/src/test/java/datadog/environment/ThreadUtilsTest.java b/components/environment/src/test/java/datadog/environment/ThreadUtilsTest.java new file mode 100644 index 00000000000..6f8dc43170e --- /dev/null +++ b/components/environment/src/test/java/datadog/environment/ThreadUtilsTest.java @@ -0,0 +1,106 @@ +package datadog.environment; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertFalse; +import static org.junit.jupiter.api.Assertions.assertTrue; + +import java.lang.invoke.MethodHandle; +import java.lang.invoke.MethodHandles; +import java.lang.invoke.MethodType; +import java.util.concurrent.ExecutionException; +import java.util.concurrent.ExecutorService; +import java.util.concurrent.Executors; +import java.util.concurrent.atomic.AtomicBoolean; +import org.junit.jupiter.api.Test; + +public class ThreadUtilsTest { + @Test + public void threadId() throws InterruptedException { + Thread thread = new Thread("foo"); + thread.start(); + try { + // always works on Thread's where getId isn't overridden by child class + assertEquals(thread.getId(), ThreadUtils.threadId(thread)); + } finally { + thread.join(); + } + } + + @Test + public void supportsVirtualThreads() { + assertEquals( + JavaVersion.getRuntimeVersion().isAtLeast(21), ThreadUtils.supportsVirtualThreads()); + } + + @Test + public void isVirtualThread_false() throws InterruptedException { + Thread thread = new Thread("foo"); + thread.start(); + try { + assertFalse(ThreadUtils.isVirtual(thread)); + } finally { + thread.join(); + } + } + + @Test + public void isCurrentThreadVirtual_false() throws InterruptedException, ExecutionException { + ExecutorService executor = Executors.newSingleThreadExecutor(); + try { + assertFalse(executor.submit(() -> ThreadUtils.isCurrentThreadVirtual()).get()); + } finally { + executor.shutdown(); + } + } + + @Test + public void isVirtualThread_true() throws InterruptedException { + if (!ThreadUtils.supportsVirtualThreads()) return; + + Thread vThread = startVirtualThread(() -> {}); + try { + assertTrue(ThreadUtils.isVirtual(vThread)); + } finally { + vThread.join(); + } + } + + @Test + public void isCurrentThreadVirtual_true() throws InterruptedException { + if (!ThreadUtils.supportsVirtualThreads()) return; + + AtomicBoolean result = new AtomicBoolean(); + + Thread vThread = + startVirtualThread( + () -> { + result.set(ThreadUtils.isCurrentThreadVirtual()); + }); + + vThread.join(); + assertTrue(result.get()); + } + + /* + * Should only be called on JVMs that support virtual threads + */ + static final Thread startVirtualThread(Runnable runnable) { + MethodHandle h_startVThread; + try { + h_startVThread = + MethodHandles.lookup() + .findStatic( + Thread.class, + "startVirtualThread", + MethodType.methodType(Runnable.class, Thread.class)); + } catch (NoSuchMethodException | IllegalAccessException e) { + throw new IllegalStateException(e); + } + + try { + return (Thread) h_startVThread.invoke(runnable); + } catch (Throwable e) { + throw new IllegalStateException(e); + } + } +} From aea9d3b69f969fb78f75b1664953b7b14ba4b849 Mon Sep 17 00:00:00 2001 From: Douglas Q Hawkins Date: Tue, 7 Oct 2025 11:23:41 -0400 Subject: [PATCH 25/43] Adding isVirtualThread check to reuseSingleSpanBuilder --- .../java/datadog/trace/core/CoreTracer.java | 18 +++++++++++++++--- 1 file changed, 15 insertions(+), 3 deletions(-) diff --git a/dd-trace-core/src/main/java/datadog/trace/core/CoreTracer.java b/dd-trace-core/src/main/java/datadog/trace/core/CoreTracer.java index ad1e6e3900a..a367288e35b 100644 --- a/dd-trace-core/src/main/java/datadog/trace/core/CoreTracer.java +++ b/dd-trace-core/src/main/java/datadog/trace/core/CoreTracer.java @@ -23,6 +23,7 @@ import datadog.communication.monitor.Monitoring; import datadog.communication.monitor.Recording; import datadog.context.propagation.Propagators; +import datadog.environment.ThreadUtils; import datadog.trace.api.ClassloaderConfigurationOverrides; import datadog.trace.api.Config; import datadog.trace.api.DDSpanId; @@ -998,6 +999,17 @@ static final ReusableSingleSpanBuilder reuseSingleSpanBuilder( final ReusableSingleSpanBuilderThreadLocalCache tlCache, final String instrumentationName, final CharSequence operationName) { + if (ThreadUtils.isCurrentThreadVirtual()) { + // Since virtual threads are created and destroyed often, + // cautiously decided not to create a thread local for the virtual threads. + + // TODO: This could probably be improved by having a single thread local that + // holds the core things that we need for tracing. e.g. context, etc + ReusableSingleSpanBuilder newSpanBuilder = new ReusableSingleSpanBuilder(tracer); + newSpanBuilder.init(instrumentationName, operationName); + return newSpanBuilder; + } + // retrieve the thread's typical SpanBuilder and try to reset it // reset will fail if the ReusableSingleSpanBuilder is still "in-use" ReusableSingleSpanBuilder tlSpanBuilder = tlCache.get(); @@ -1947,9 +1959,9 @@ protected ReusableSingleSpanBuilder initialValue() { } static final class ReusableSingleSpanBuilder extends CoreSpanBuilder { - // Used to track whether the CoreSpanBuilder is actively being used - // CoreSpanBuilder becomes "inUse" after a succesful reset and remains "inUse" until "build" is - // called + // Used to track whether the ReusableSingleSpanBuilder is actively being used + // ReusableSingleSpanBuilder becomes "inUse" after a succesful reset and remains "inUse" + // until "buildSpan" is called protected boolean inUse; ReusableSingleSpanBuilder(CoreTracer tracer) { From 6ed469b63f088dc94c8ac5bc94f81582187f7cb9 Mon Sep 17 00:00:00 2001 From: Douglas Q Hawkins Date: Tue, 7 Oct 2025 17:21:18 -0400 Subject: [PATCH 26/43] Addressing review comments - reduced visibility --- .../src/main/java/datadog/trace/core/CoreTracer.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/dd-trace-core/src/main/java/datadog/trace/core/CoreTracer.java b/dd-trace-core/src/main/java/datadog/trace/core/CoreTracer.java index 15ab9237abf..32eaceac0aa 100644 --- a/dd-trace-core/src/main/java/datadog/trace/core/CoreTracer.java +++ b/dd-trace-core/src/main/java/datadog/trace/core/CoreTracer.java @@ -1974,9 +1974,9 @@ protected ReusableSingleSpanBuilder initialValue() { static final class ReusableSingleSpanBuilder extends CoreSpanBuilder { // Used to track whether the ReusableSingleSpanBuilder is actively being used - // ReusableSingleSpanBuilder becomes "inUse" after a succesful reset and remains "inUse" + // ReusableSingleSpanBuilder becomes "inUse" after a succesful init/reset and remains "inUse" // until "buildSpan" is called - protected boolean inUse; + boolean inUse; ReusableSingleSpanBuilder(CoreTracer tracer) { super(tracer); From 66f62fbb2dc443a756cd1bf87b1a31650055a747 Mon Sep 17 00:00:00 2001 From: Douglas Q Hawkins Date: Tue, 14 Oct 2025 13:27:22 -0400 Subject: [PATCH 27/43] Update dd-trace-core/src/main/java/datadog/trace/core/CoreTracer.java Co-authored-by: Brice Dutheil --- dd-trace-core/src/main/java/datadog/trace/core/CoreTracer.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/dd-trace-core/src/main/java/datadog/trace/core/CoreTracer.java b/dd-trace-core/src/main/java/datadog/trace/core/CoreTracer.java index 32eaceac0aa..7d4b5158c7e 100644 --- a/dd-trace-core/src/main/java/datadog/trace/core/CoreTracer.java +++ b/dd-trace-core/src/main/java/datadog/trace/core/CoreTracer.java @@ -1038,7 +1038,7 @@ static final ReusableSingleSpanBuilder reuseSingleSpanBuilder( // that case could result in permanently burning the cache for a given thread. // That could be solved with additional logic during ReusableSingleSpanBuilder#buildSpan - // that checks to see if the cached the Builder is in use and then replaces it + // that checks to see if the cached Builder is in use and then replaces it // with the freed Builder, but that would put extra logic in the common path. // Instead of making the release process more complicated, I'm chosing to just From 94970a4bad66834789a68927b0f5154704e6cdbf Mon Sep 17 00:00:00 2001 From: Douglas Q Hawkins Date: Tue, 14 Oct 2025 13:28:21 -0400 Subject: [PATCH 28/43] Update internal-api/src/main/java/datadog/trace/bootstrap/instrumentation/api/AgentTracer.java Co-authored-by: Brice Dutheil --- .../trace/bootstrap/instrumentation/api/AgentTracer.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/internal-api/src/main/java/datadog/trace/bootstrap/instrumentation/api/AgentTracer.java b/internal-api/src/main/java/datadog/trace/bootstrap/instrumentation/api/AgentTracer.java index 9c6c7681ad2..847a308052d 100644 --- a/internal-api/src/main/java/datadog/trace/bootstrap/instrumentation/api/AgentTracer.java +++ b/internal-api/src/main/java/datadog/trace/bootstrap/instrumentation/api/AgentTracer.java @@ -377,7 +377,7 @@ default SpanBuilder buildSpan(CharSequence spanName) { /** * Returns a SpanBuilder that can be used to produce one and only one span. By imposing the - * single span creation limitation, this method is more efficient than {@link buildSpan} + * single span creation limitation, this method is more efficient than {@link #buildSpan} */ SpanBuilder singleSpanBuilder(String instrumentationName, CharSequence spanName); From 86c03d9de029858a27b48480c2152ecf61390c14 Mon Sep 17 00:00:00 2001 From: Douglas Q Hawkins Date: Tue, 14 Oct 2025 14:09:44 -0400 Subject: [PATCH 29/43] Update dd-trace-core/src/main/java/datadog/trace/core/CoreTracer.java Co-authored-by: Brice Dutheil --- .../src/main/java/datadog/trace/core/CoreTracer.java | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/dd-trace-core/src/main/java/datadog/trace/core/CoreTracer.java b/dd-trace-core/src/main/java/datadog/trace/core/CoreTracer.java index 7d4b5158c7e..a34a46a486c 100644 --- a/dd-trace-core/src/main/java/datadog/trace/core/CoreTracer.java +++ b/dd-trace-core/src/main/java/datadog/trace/core/CoreTracer.java @@ -1992,11 +1992,10 @@ void init(String instrumentationName, CharSequence operationName) { } /** - * Resets the ReusableSingleSpanBuilder, so it may be used to build another single span Returns - * true if the reset was successful Returns false if this ReusableSingleSpanBuilder is still - * "in-use" + * Resets the {@link ReusableSingleSpanBuilder}, so it may be used to build another single span + * @returns true if the reset was successful, otherwise false + * if this ReusableSingleSpanBuilder is still "in-use". */ - final boolean reset(String instrumentationName, CharSequence operationName) { if (this.inUse) return false; this.inUse = true; From 8e3ee47ee091b746193059b4af63707e9772f9a7 Mon Sep 17 00:00:00 2001 From: Douglas Q Hawkins Date: Wed, 15 Oct 2025 10:31:31 -0400 Subject: [PATCH 30/43] Fixing bad update from GitHub suggestion merge --- dd-trace-core/src/main/java/datadog/trace/core/CoreTracer.java | 1 + 1 file changed, 1 insertion(+) diff --git a/dd-trace-core/src/main/java/datadog/trace/core/CoreTracer.java b/dd-trace-core/src/main/java/datadog/trace/core/CoreTracer.java index a34a46a486c..d794ef0feba 100644 --- a/dd-trace-core/src/main/java/datadog/trace/core/CoreTracer.java +++ b/dd-trace-core/src/main/java/datadog/trace/core/CoreTracer.java @@ -1996,6 +1996,7 @@ void init(String instrumentationName, CharSequence operationName) { * @returns true if the reset was successful, otherwise false * if this ReusableSingleSpanBuilder is still "in-use". */ + final boolean reset(String instrumentationName, CharSequence operationName) { if (this.inUse) return false; this.inUse = true; From 0b968aed550d20bb41ce9815b8e0ac7b282218b1 Mon Sep 17 00:00:00 2001 From: Douglas Q Hawkins Date: Wed, 15 Oct 2025 10:31:42 -0400 Subject: [PATCH 31/43] Adding Javadoc --- .../trace/bootstrap/instrumentation/api/AgentTracer.java | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/internal-api/src/main/java/datadog/trace/bootstrap/instrumentation/api/AgentTracer.java b/internal-api/src/main/java/datadog/trace/bootstrap/instrumentation/api/AgentTracer.java index 847a308052d..c78e3fffdd1 100644 --- a/internal-api/src/main/java/datadog/trace/bootstrap/instrumentation/api/AgentTracer.java +++ b/internal-api/src/main/java/datadog/trace/bootstrap/instrumentation/api/AgentTracer.java @@ -373,6 +373,11 @@ default SpanBuilder buildSpan(CharSequence spanName) { return buildSpan(DEFAULT_INSTRUMENTATION_NAME, spanName); } + /** + * Returns a SpanBuilder that can be used to produce multiple spans. To minimize overhead, use + * of {@link #singleSpanBuilder(String, CharSequence)} is preferred when only a single span is + * being built. + */ SpanBuilder buildSpan(String instrumentationName, CharSequence spanName); /** From 097e7424e6043ea3bef44fe8d4942ab1a68b6628 Mon Sep 17 00:00:00 2001 From: Douglas Q Hawkins Date: Wed, 15 Oct 2025 10:32:15 -0400 Subject: [PATCH 32/43] spotless --- .../src/main/java/datadog/trace/core/CoreTracer.java | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/dd-trace-core/src/main/java/datadog/trace/core/CoreTracer.java b/dd-trace-core/src/main/java/datadog/trace/core/CoreTracer.java index d794ef0feba..4838732ba0e 100644 --- a/dd-trace-core/src/main/java/datadog/trace/core/CoreTracer.java +++ b/dd-trace-core/src/main/java/datadog/trace/core/CoreTracer.java @@ -1992,9 +1992,10 @@ void init(String instrumentationName, CharSequence operationName) { } /** - * Resets the {@link ReusableSingleSpanBuilder}, so it may be used to build another single span - * @returns true if the reset was successful, otherwise false - * if this ReusableSingleSpanBuilder is still "in-use". + * Resets the {@link ReusableSingleSpanBuilder}, so it may be used to build another single span + * + * @returns true if the reset was successful, otherwise false if this + * ReusableSingleSpanBuilder is still "in-use". */ final boolean reset(String instrumentationName, CharSequence operationName) { if (this.inUse) return false; From 826f528c244c39c93b101c4a7715acd7b587f8ce Mon Sep 17 00:00:00 2001 From: Douglas Q Hawkins Date: Wed, 15 Oct 2025 13:28:35 -0400 Subject: [PATCH 33/43] Update CoreTracer.java Fixing assertion with blackhole test Moved the inUse tracking to start (rather than buildSpan) --- .../java/datadog/trace/core/CoreTracer.java | 23 ++++++++++++------- 1 file changed, 15 insertions(+), 8 deletions(-) diff --git a/dd-trace-core/src/main/java/datadog/trace/core/CoreTracer.java b/dd-trace-core/src/main/java/datadog/trace/core/CoreTracer.java index 4838732ba0e..ff0ecb85c3f 100644 --- a/dd-trace-core/src/main/java/datadog/trace/core/CoreTracer.java +++ b/dd-trace-core/src/main/java/datadog/trace/core/CoreTracer.java @@ -1516,9 +1516,7 @@ public final CoreSpanBuilder ignoreActiveSpan() { return this; } - protected abstract DDSpan buildSpan(); - - protected final DDSpan buildSpanImpl() { + protected final DDSpan buildSpan() { DDSpan span = DDSpan.create(instrumentationName, timestampMicro, buildSpanContext(), links); if (span.isLocalRootSpan()) { EndpointTracker tracker = tracer.onRootSpanStarted(span); @@ -1559,7 +1557,9 @@ private final void addTerminatedContextAsLinks() { } @Override - public final AgentSpan start() { + public abstract AgentSpan start(); + + protected AgentSpan startImpl() { AgentSpanContext pc = parent; if (pc == null && !ignoreScope) { final AgentSpan span = tracer.activeSpan(); @@ -1945,6 +1945,7 @@ private final DDSpanContext buildSpanContext() { } } + /** CoreSpanBuilder that can be used to produce multiple spans */ static final class MultiSpanBuilder extends CoreSpanBuilder { MultiSpanBuilder(CoreTracer tracer, String instrumentationName, CharSequence operationName) { super(tracer); @@ -1953,8 +1954,8 @@ static final class MultiSpanBuilder extends CoreSpanBuilder { } @Override - protected DDSpan buildSpan() { - return this.buildSpanImpl(); + public AgentSpan start() { + return this.startImpl(); } } @@ -1972,6 +1973,12 @@ protected ReusableSingleSpanBuilder initialValue() { } } + /** + * Reusable CoreSpanBuilder that can be used to build one and only one span before being reset + * + *

{@link CoreTracer#singleSpanBuilder(String, CharSequence)} reuses instances of this object + * to reduce the overhead of span construction + */ static final class ReusableSingleSpanBuilder extends CoreSpanBuilder { // Used to track whether the ReusableSingleSpanBuilder is actively being used // ReusableSingleSpanBuilder becomes "inUse" after a succesful init/reset and remains "inUse" @@ -2025,11 +2032,11 @@ final boolean reset(String instrumentationName, CharSequence operationName) { * Clears the inUse boolean, so this ReusableSpanBuilder can be reset */ @Override - protected DDSpan buildSpan() { + public AgentSpan start() { assert this.inUse : "ReusableSingleSpanBuilder not reset properly -- multiple span construction?"; - DDSpan span = this.buildSpanImpl(); + AgentSpan span = this.startImpl(); this.inUse = false; return span; } From bd4b95742d1d1476ceefb49de1e987651df97680 Mon Sep 17 00:00:00 2001 From: Douglas Q Hawkins Date: Wed, 15 Oct 2025 14:19:46 -0400 Subject: [PATCH 34/43] Updating comments to reflect the usage logic was moved into start --- .../src/main/java/datadog/trace/core/CoreTracer.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/dd-trace-core/src/main/java/datadog/trace/core/CoreTracer.java b/dd-trace-core/src/main/java/datadog/trace/core/CoreTracer.java index ff0ecb85c3f..9ad3f2002e7 100644 --- a/dd-trace-core/src/main/java/datadog/trace/core/CoreTracer.java +++ b/dd-trace-core/src/main/java/datadog/trace/core/CoreTracer.java @@ -1037,7 +1037,7 @@ static final ReusableSingleSpanBuilder reuseSingleSpanBuilder( // and then not using it. Without an ability to replace the cached SpanBuilder, // that case could result in permanently burning the cache for a given thread. - // That could be solved with additional logic during ReusableSingleSpanBuilder#buildSpan + // That could be solved with additional logic during ReusableSingleSpanBuilder#start // that checks to see if the cached Builder is in use and then replaces it // with the freed Builder, but that would put extra logic in the common path. @@ -1982,7 +1982,7 @@ protected ReusableSingleSpanBuilder initialValue() { static final class ReusableSingleSpanBuilder extends CoreSpanBuilder { // Used to track whether the ReusableSingleSpanBuilder is actively being used // ReusableSingleSpanBuilder becomes "inUse" after a succesful init/reset and remains "inUse" - // until "buildSpan" is called + // until "start" is called boolean inUse; ReusableSingleSpanBuilder(CoreTracer tracer) { From e9cf952e85c6144452bda8ae8f1532b01c3db8b0 Mon Sep 17 00:00:00 2001 From: Douglas Q Hawkins Date: Wed, 15 Oct 2025 14:34:42 -0400 Subject: [PATCH 35/43] Tweaking comments --- dd-trace-core/src/main/java/datadog/trace/core/CoreTracer.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/dd-trace-core/src/main/java/datadog/trace/core/CoreTracer.java b/dd-trace-core/src/main/java/datadog/trace/core/CoreTracer.java index 9ad3f2002e7..58bd38d7eb2 100644 --- a/dd-trace-core/src/main/java/datadog/trace/core/CoreTracer.java +++ b/dd-trace-core/src/main/java/datadog/trace/core/CoreTracer.java @@ -1034,7 +1034,7 @@ static final ReusableSingleSpanBuilder reuseSingleSpanBuilder( newSpanBuilder.init(instrumentationName, operationName); // DQH - Debated how best to handle the case of someone requesting a SpanBuilder - // and then not using it. Without an ability to replace the cached SpanBuilder, + // and then not using it. Without the ability to replace the cached SpanBuilder, // that case could result in permanently burning the cache for a given thread. // That could be solved with additional logic during ReusableSingleSpanBuilder#start From eb2996dadd4a498b66d67c6b9bb06158658c82da Mon Sep 17 00:00:00 2001 From: Douglas Q Hawkins Date: Thu, 16 Oct 2025 10:19:26 -0400 Subject: [PATCH 36/43] Improving test coverage - exposed bug with not setting inUse in init --- .../java/datadog/trace/core/CoreTracer.java | 6 +- .../datadog/trace/core/CoreTracerTest.java | 76 +++++++++++++++++-- 2 files changed, 75 insertions(+), 7 deletions(-) diff --git a/dd-trace-core/src/main/java/datadog/trace/core/CoreTracer.java b/dd-trace-core/src/main/java/datadog/trace/core/CoreTracer.java index 58bd38d7eb2..326e459ba5d 100644 --- a/dd-trace-core/src/main/java/datadog/trace/core/CoreTracer.java +++ b/dd-trace-core/src/main/java/datadog/trace/core/CoreTracer.java @@ -1489,9 +1489,8 @@ public abstract static class CoreSpanBuilder implements AgentTracer.SpanBuilder protected String instrumentationName; protected CharSequence operationName; - // Make sure any fields added here are also reset properly in ReusableSingleSpanBuilder.reset - // (below) // Builder attributes + // Make sure any fields added here are also reset properly in ReusableSingleSpanBuilder.reset protected TagMap.Ledger tagLedger; protected long timestampMicro; protected AgentSpanContext parent; @@ -1505,6 +1504,7 @@ public abstract static class CoreSpanBuilder implements AgentTracer.SpanBuilder protected Object builderCiVisibilityContextData; protected List links; protected long spanId; + // Make sure any fields added here are also reset properly in ReusableSingleSpanBuilder.reset CoreSpanBuilder(CoreTracer tracer) { this.tracer = tracer; @@ -1996,6 +1996,8 @@ void init(String instrumentationName, CharSequence operationName) { this.instrumentationName = instrumentationName; this.operationName = operationName; + + this.inUse = true; } /** diff --git a/dd-trace-core/src/test/java/datadog/trace/core/CoreTracerTest.java b/dd-trace-core/src/test/java/datadog/trace/core/CoreTracerTest.java index 1003639c84c..d7ee1390b39 100644 --- a/dd-trace-core/src/test/java/datadog/trace/core/CoreTracerTest.java +++ b/dd-trace-core/src/test/java/datadog/trace/core/CoreTracerTest.java @@ -1,18 +1,23 @@ package datadog.trace.core; -import static org.junit.Assert.assertFalse; -import static org.junit.Assert.assertNotSame; -import static org.junit.Assert.assertSame; -import static org.junit.Assert.assertTrue; +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertFalse; +import static org.junit.jupiter.api.Assertions.assertNotSame; +import static org.junit.jupiter.api.Assertions.assertSame; +import static org.junit.jupiter.api.Assertions.assertTrue; +import static org.junit.jupiter.api.Assertions.assertThrows; + +import org.junit.jupiter.api.Test; import datadog.trace.api.Config; +import datadog.trace.bootstrap.instrumentation.api.AgentSpan; import datadog.trace.core.CoreTracer.CoreSpanBuilder; import datadog.trace.core.CoreTracer.ReusableSingleSpanBuilder; import datadog.trace.core.CoreTracer.ReusableSingleSpanBuilderThreadLocalCache; -import org.junit.Test; public final class CoreTracerTest { static final CoreTracer TRACER = CoreTracer.builder().build(); + static final ReusableSingleSpanBuilderThreadLocalCache CACHE = new ReusableSingleSpanBuilderThreadLocalCache(TRACER); @@ -113,4 +118,65 @@ public void spanBuilderReuse_abandoned() { builder2.start(); assertFalse(builder2.inUse); } + + @Test + public void init_twice() { + ReusableSingleSpanBuilder builder = new ReusableSingleSpanBuilder(TRACER); + builder.init("foo", "bar"); + assertTrue(builder.inUse); + assertEquals("foo", builder.instrumentationName); + assertEquals("bar", builder.operationName); + + assertThrows(AssertionError.class, () -> builder.init("baz", "quux")); + } + + @Test + public void reset_twice() { + ReusableSingleSpanBuilder builder = new ReusableSingleSpanBuilder(TRACER); + builder.reset("foo", "bar"); + assertTrue(builder.inUse); + assertEquals("foo", builder.instrumentationName); + assertEquals("bar", builder.operationName); + + assertFalse(builder.reset("baz", "quux")); + assertEquals("foo", builder.instrumentationName); + assertEquals("bar", builder.operationName); + } + + @Test + public void reset_and_start() { + ReusableSingleSpanBuilder builder = new ReusableSingleSpanBuilder(TRACER); + builder.reset("foo", "bar"); + assertTrue(builder.inUse); + assertEquals("foo", builder.instrumentationName); + assertEquals("bar", builder.operationName); + + AgentSpan span = builder.start(); + assertEquals(span.getOperationName(), "bar"); + } + + @Test + public void init_and_start() { + ReusableSingleSpanBuilder builder = new ReusableSingleSpanBuilder(TRACER); + builder.reset("foo", "bar"); + assertTrue(builder.inUse); + assertEquals("foo", builder.instrumentationName); + assertEquals("bar", builder.operationName); + + AgentSpan span = builder.start(); + assertFalse(builder.inUse); + assertEquals(span.getOperationName(), "bar"); + + + builder.reset("baz", "quux"); + assertTrue(builder.inUse); + assertEquals("baz", builder.instrumentationName); + assertEquals("quux", builder.operationName); + } + + @Test + public void start_not_inUse() { + ReusableSingleSpanBuilder builder = new ReusableSingleSpanBuilder(TRACER); + assertThrows(AssertionError.class, () -> builder.start()); + } } From 5fd6e1ded918f57b7d05da40bb4d158be07c2e1f Mon Sep 17 00:00:00 2001 From: Douglas Q Hawkins Date: Thu, 16 Oct 2025 10:55:56 -0400 Subject: [PATCH 37/43] spotless --- .../java/datadog/trace/core/CoreTracer.java | 2 +- .../datadog/trace/core/CoreTracerTest.java | 98 +++++++++---------- 2 files changed, 49 insertions(+), 51 deletions(-) diff --git a/dd-trace-core/src/main/java/datadog/trace/core/CoreTracer.java b/dd-trace-core/src/main/java/datadog/trace/core/CoreTracer.java index 326e459ba5d..b903ba6cec8 100644 --- a/dd-trace-core/src/main/java/datadog/trace/core/CoreTracer.java +++ b/dd-trace-core/src/main/java/datadog/trace/core/CoreTracer.java @@ -1996,7 +1996,7 @@ void init(String instrumentationName, CharSequence operationName) { this.instrumentationName = instrumentationName; this.operationName = operationName; - + this.inUse = true; } diff --git a/dd-trace-core/src/test/java/datadog/trace/core/CoreTracerTest.java b/dd-trace-core/src/test/java/datadog/trace/core/CoreTracerTest.java index d7ee1390b39..120f3e812de 100644 --- a/dd-trace-core/src/test/java/datadog/trace/core/CoreTracerTest.java +++ b/dd-trace-core/src/test/java/datadog/trace/core/CoreTracerTest.java @@ -4,20 +4,19 @@ import static org.junit.jupiter.api.Assertions.assertFalse; import static org.junit.jupiter.api.Assertions.assertNotSame; import static org.junit.jupiter.api.Assertions.assertSame; -import static org.junit.jupiter.api.Assertions.assertTrue; import static org.junit.jupiter.api.Assertions.assertThrows; - -import org.junit.jupiter.api.Test; +import static org.junit.jupiter.api.Assertions.assertTrue; import datadog.trace.api.Config; import datadog.trace.bootstrap.instrumentation.api.AgentSpan; import datadog.trace.core.CoreTracer.CoreSpanBuilder; import datadog.trace.core.CoreTracer.ReusableSingleSpanBuilder; import datadog.trace.core.CoreTracer.ReusableSingleSpanBuilderThreadLocalCache; +import org.junit.jupiter.api.Test; public final class CoreTracerTest { static final CoreTracer TRACER = CoreTracer.builder().build(); - + static final ReusableSingleSpanBuilderThreadLocalCache CACHE = new ReusableSingleSpanBuilderThreadLocalCache(TRACER); @@ -118,65 +117,64 @@ public void spanBuilderReuse_abandoned() { builder2.start(); assertFalse(builder2.inUse); } - + @Test public void init_twice() { - ReusableSingleSpanBuilder builder = new ReusableSingleSpanBuilder(TRACER); - builder.init("foo", "bar"); - assertTrue(builder.inUse); - assertEquals("foo", builder.instrumentationName); - assertEquals("bar", builder.operationName); - - assertThrows(AssertionError.class, () -> builder.init("baz", "quux")); + ReusableSingleSpanBuilder builder = new ReusableSingleSpanBuilder(TRACER); + builder.init("foo", "bar"); + assertTrue(builder.inUse); + assertEquals("foo", builder.instrumentationName); + assertEquals("bar", builder.operationName); + + assertThrows(AssertionError.class, () -> builder.init("baz", "quux")); } - + @Test public void reset_twice() { - ReusableSingleSpanBuilder builder = new ReusableSingleSpanBuilder(TRACER); - builder.reset("foo", "bar"); - assertTrue(builder.inUse); - assertEquals("foo", builder.instrumentationName); - assertEquals("bar", builder.operationName); - - assertFalse(builder.reset("baz", "quux")); - assertEquals("foo", builder.instrumentationName); - assertEquals("bar", builder.operationName); + ReusableSingleSpanBuilder builder = new ReusableSingleSpanBuilder(TRACER); + builder.reset("foo", "bar"); + assertTrue(builder.inUse); + assertEquals("foo", builder.instrumentationName); + assertEquals("bar", builder.operationName); + + assertFalse(builder.reset("baz", "quux")); + assertEquals("foo", builder.instrumentationName); + assertEquals("bar", builder.operationName); } - + @Test public void reset_and_start() { - ReusableSingleSpanBuilder builder = new ReusableSingleSpanBuilder(TRACER); - builder.reset("foo", "bar"); - assertTrue(builder.inUse); - assertEquals("foo", builder.instrumentationName); - assertEquals("bar", builder.operationName); - - AgentSpan span = builder.start(); - assertEquals(span.getOperationName(), "bar"); + ReusableSingleSpanBuilder builder = new ReusableSingleSpanBuilder(TRACER); + builder.reset("foo", "bar"); + assertTrue(builder.inUse); + assertEquals("foo", builder.instrumentationName); + assertEquals("bar", builder.operationName); + + AgentSpan span = builder.start(); + assertEquals(span.getOperationName(), "bar"); } - + @Test public void init_and_start() { - ReusableSingleSpanBuilder builder = new ReusableSingleSpanBuilder(TRACER); - builder.reset("foo", "bar"); - assertTrue(builder.inUse); - assertEquals("foo", builder.instrumentationName); - assertEquals("bar", builder.operationName); - - AgentSpan span = builder.start(); - assertFalse(builder.inUse); - assertEquals(span.getOperationName(), "bar"); - - - builder.reset("baz", "quux"); - assertTrue(builder.inUse); - assertEquals("baz", builder.instrumentationName); - assertEquals("quux", builder.operationName); + ReusableSingleSpanBuilder builder = new ReusableSingleSpanBuilder(TRACER); + builder.reset("foo", "bar"); + assertTrue(builder.inUse); + assertEquals("foo", builder.instrumentationName); + assertEquals("bar", builder.operationName); + + AgentSpan span = builder.start(); + assertFalse(builder.inUse); + assertEquals(span.getOperationName(), "bar"); + + builder.reset("baz", "quux"); + assertTrue(builder.inUse); + assertEquals("baz", builder.instrumentationName); + assertEquals("quux", builder.operationName); } - + @Test public void start_not_inUse() { - ReusableSingleSpanBuilder builder = new ReusableSingleSpanBuilder(TRACER); - assertThrows(AssertionError.class, () -> builder.start()); + ReusableSingleSpanBuilder builder = new ReusableSingleSpanBuilder(TRACER); + assertThrows(AssertionError.class, () -> builder.start()); } } From e46f8f58148a6b8aa760546f0e33dc367a9d6ca1 Mon Sep 17 00:00:00 2001 From: Douglas Q Hawkins Date: Thu, 16 Oct 2025 11:16:10 -0400 Subject: [PATCH 38/43] A bit of clean-up - introducing some helper methods --- .../java/datadog/trace/core/CoreTracer.java | 25 +++++++++++-------- 1 file changed, 15 insertions(+), 10 deletions(-) diff --git a/dd-trace-core/src/main/java/datadog/trace/core/CoreTracer.java b/dd-trace-core/src/main/java/datadog/trace/core/CoreTracer.java index b903ba6cec8..1763a916dbb 100644 --- a/dd-trace-core/src/main/java/datadog/trace/core/CoreTracer.java +++ b/dd-trace-core/src/main/java/datadog/trace/core/CoreTracer.java @@ -985,7 +985,12 @@ long getTimeWithNanoTicks(long nanoTicks) { @Override public CoreSpanBuilder buildSpan( final String instrumentationName, final CharSequence operationName) { - return createSpanBuilder(instrumentationName, operationName); + return createMultiSpanBuilder(instrumentationName, operationName); + } + + MultiSpanBuilder createMultiSpanBuilder( + final String instrumentationName, final CharSequence operationName) { + return new MultiSpanBuilder(this, instrumentationName, operationName); } @Override @@ -993,12 +998,14 @@ public CoreSpanBuilder singleSpanBuilder( final String instrumentationName, final CharSequence operationName) { return SPAN_BUILDER_REUSE_ENABLED ? reuseSingleSpanBuilder(instrumentationName, operationName) - : createSpanBuilder(instrumentationName, operationName); + : createMultiSpanBuilder(instrumentationName, operationName); } - CoreSpanBuilder createSpanBuilder( - final String instrumentationName, final CharSequence operationName) { - return new MultiSpanBuilder(this, instrumentationName, operationName); + ReusableSingleSpanBuilder createSingleSpanBuilder( + final String instrumentationName, final CharSequence oprationName) { + ReusableSingleSpanBuilder singleSpanBuilder = new ReusableSingleSpanBuilder(this); + singleSpanBuilder.init(instrumentationName, oprationName); + return singleSpanBuilder; } CoreSpanBuilder reuseSingleSpanBuilder( @@ -1018,9 +1025,7 @@ static final ReusableSingleSpanBuilder reuseSingleSpanBuilder( // TODO: This could probably be improved by having a single thread local that // holds the core things that we need for tracing. e.g. context, etc - ReusableSingleSpanBuilder newSpanBuilder = new ReusableSingleSpanBuilder(tracer); - newSpanBuilder.init(instrumentationName, operationName); - return newSpanBuilder; + return tracer.createSingleSpanBuilder(instrumentationName, operationName); } // retrieve the thread's typical SpanBuilder and try to reset it @@ -1030,8 +1035,8 @@ static final ReusableSingleSpanBuilder reuseSingleSpanBuilder( if (wasReset) return tlSpanBuilder; // TODO: counter for how often the fallback is used? - ReusableSingleSpanBuilder newSpanBuilder = new ReusableSingleSpanBuilder(tracer); - newSpanBuilder.init(instrumentationName, operationName); + ReusableSingleSpanBuilder newSpanBuilder = + tracer.createSingleSpanBuilder(instrumentationName, operationName); // DQH - Debated how best to handle the case of someone requesting a SpanBuilder // and then not using it. Without the ability to replace the cached SpanBuilder, From 054485ac6630b0ba0e1c54a9a5a6d5579911b44a Mon Sep 17 00:00:00 2001 From: Douglas Q Hawkins Date: Thu, 16 Oct 2025 12:31:58 -0400 Subject: [PATCH 39/43] Fixing v-thread tests --- .../src/test/java/datadog/environment/ThreadUtilsTest.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/components/environment/src/test/java/datadog/environment/ThreadUtilsTest.java b/components/environment/src/test/java/datadog/environment/ThreadUtilsTest.java index 6f8dc43170e..c4273214179 100644 --- a/components/environment/src/test/java/datadog/environment/ThreadUtilsTest.java +++ b/components/environment/src/test/java/datadog/environment/ThreadUtilsTest.java @@ -92,7 +92,7 @@ static final Thread startVirtualThread(Runnable runnable) { .findStatic( Thread.class, "startVirtualThread", - MethodType.methodType(Runnable.class, Thread.class)); + MethodType.methodType(Thread.class, Runnable.class)); } catch (NoSuchMethodException | IllegalAccessException e) { throw new IllegalStateException(e); } From cf34eaf17d8afd9a56f82b3f360db94204ff02e4 Mon Sep 17 00:00:00 2001 From: Douglas Q Hawkins Date: Wed, 22 Oct 2025 12:58:42 -0400 Subject: [PATCH 40/43] CoreTracerTest -> CoreTracerTest2 Seeing if removing naming conflict with Groovy tests, fix Jacoco coverage calculation --- .../trace/core/{CoreTracerTest.java => CoreTracerTest2.java} | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) rename dd-trace-core/src/test/java/datadog/trace/core/{CoreTracerTest.java => CoreTracerTest2.java} (99%) diff --git a/dd-trace-core/src/test/java/datadog/trace/core/CoreTracerTest.java b/dd-trace-core/src/test/java/datadog/trace/core/CoreTracerTest2.java similarity index 99% rename from dd-trace-core/src/test/java/datadog/trace/core/CoreTracerTest.java rename to dd-trace-core/src/test/java/datadog/trace/core/CoreTracerTest2.java index 120f3e812de..64346828358 100644 --- a/dd-trace-core/src/test/java/datadog/trace/core/CoreTracerTest.java +++ b/dd-trace-core/src/test/java/datadog/trace/core/CoreTracerTest2.java @@ -14,7 +14,7 @@ import datadog.trace.core.CoreTracer.ReusableSingleSpanBuilderThreadLocalCache; import org.junit.jupiter.api.Test; -public final class CoreTracerTest { +public final class CoreTracerTest2 { static final CoreTracer TRACER = CoreTracer.builder().build(); static final ReusableSingleSpanBuilderThreadLocalCache CACHE = From 4d1770117d7e48e7f51bb5e0ad22773abca8bc99 Mon Sep 17 00:00:00 2001 From: Douglas Q Hawkins Date: Wed, 22 Oct 2025 13:53:16 -0400 Subject: [PATCH 41/43] Excluded ThreadUtils from coverage - JVM version specific --- components/environment/build.gradle.kts | 1 + 1 file changed, 1 insertion(+) diff --git a/components/environment/build.gradle.kts b/components/environment/build.gradle.kts index 0b620c76090..8d9febfe679 100644 --- a/components/environment/build.gradle.kts +++ b/components/environment/build.gradle.kts @@ -24,6 +24,7 @@ val excludedClassesCoverage by extra { "datadog.environment.JavaVirtualMachine.JvmOptionsHolder", // depends on OS and JVM vendor "datadog.environment.JvmOptions", // depends on OS and JVM vendor "datadog.environment.OperatingSystem**", // depends on OS + "datadog.environment.ThreadUtils", // depends on JVM version ) } val excludedClassesBranchCoverage by extra { From 43979a50011abb080d28f66c7c9e4d7fddb7c27d Mon Sep 17 00:00:00 2001 From: Douglas Q Hawkins Date: Wed, 22 Oct 2025 14:28:31 -0400 Subject: [PATCH 42/43] Addressing review comments - switching to JUnit assume --- .../src/test/java/datadog/environment/ThreadUtilsTest.java | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/components/environment/src/test/java/datadog/environment/ThreadUtilsTest.java b/components/environment/src/test/java/datadog/environment/ThreadUtilsTest.java index c4273214179..008897f6199 100644 --- a/components/environment/src/test/java/datadog/environment/ThreadUtilsTest.java +++ b/components/environment/src/test/java/datadog/environment/ThreadUtilsTest.java @@ -3,6 +3,7 @@ import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertFalse; import static org.junit.jupiter.api.Assertions.assertTrue; +import static org.junit.jupiter.api.Assumptions.assumeTrue; import java.lang.invoke.MethodHandle; import java.lang.invoke.MethodHandles; @@ -55,7 +56,7 @@ public void isCurrentThreadVirtual_false() throws InterruptedException, Executio @Test public void isVirtualThread_true() throws InterruptedException { - if (!ThreadUtils.supportsVirtualThreads()) return; + assumeTrue(ThreadUtils.supportsVirtualThreads()); Thread vThread = startVirtualThread(() -> {}); try { @@ -67,7 +68,7 @@ public void isVirtualThread_true() throws InterruptedException { @Test public void isCurrentThreadVirtual_true() throws InterruptedException { - if (!ThreadUtils.supportsVirtualThreads()) return; + assumeTrue(ThreadUtils.supportsVirtualThreads()); AtomicBoolean result = new AtomicBoolean(); From 3d9c52713fd7e747b74791604c2ecb38161e6e93 Mon Sep 17 00:00:00 2001 From: Douglas Q Hawkins Date: Wed, 22 Oct 2025 14:28:51 -0400 Subject: [PATCH 43/43] Adding comment to explain the name --- .../src/test/java/datadog/trace/core/CoreTracerTest2.java | 2 ++ 1 file changed, 2 insertions(+) diff --git a/dd-trace-core/src/test/java/datadog/trace/core/CoreTracerTest2.java b/dd-trace-core/src/test/java/datadog/trace/core/CoreTracerTest2.java index 64346828358..b613c92a21e 100644 --- a/dd-trace-core/src/test/java/datadog/trace/core/CoreTracerTest2.java +++ b/dd-trace-core/src/test/java/datadog/trace/core/CoreTracerTest2.java @@ -14,6 +14,8 @@ import datadog.trace.core.CoreTracer.ReusableSingleSpanBuilderThreadLocalCache; import org.junit.jupiter.api.Test; +// named CoreTracerTest2 to avoid collision with Groovy which appears to have messed up test +// coverage public final class CoreTracerTest2 { static final CoreTracer TRACER = CoreTracer.builder().build();