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); } 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..c4273214179 --- /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(Thread.class, Runnable.class)); + } catch (NoSuchMethodException | IllegalAccessException e) { + throw new IllegalStateException(e); + } + + try { + return (Thread) h_startVThread.invoke(runnable); + } catch (Throwable e) { + throw new IllegalStateException(e); + } + } +} 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 c3f3d63ca8b..c7832b1a224 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 @@ -110,6 +110,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/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(); + } +} 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 5184a72d6da..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 @@ -24,6 +24,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; @@ -136,7 +137,7 @@ public class CoreTracer implements AgentTracer.TracerAPI, TracerFlare.Reporter { 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(); } @@ -242,6 +243,15 @@ public static 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 singleSpanBuilder + private static final boolean SPAN_BUILDER_REUSE_ENABLED = + Config.get().isSpanBuilderReuseEnabled(); + + // Cache used by buildSpan - instance so it can capture the CoreTracer + private final ReusableSingleSpanBuilderThreadLocalCache spanBuilderThreadLocalCache = + SPAN_BUILDER_REUSE_ENABLED ? new ReusableSingleSpanBuilderThreadLocalCache(this) : null; + @Override public ConfigSnapshot captureTraceConfig() { return dynamicConfig.captureTraceConfig(); @@ -975,24 +985,94 @@ long getTimeWithNanoTicks(long nanoTicks) { @Override public CoreSpanBuilder buildSpan( final String instrumentationName, final CharSequence operationName) { - return new CoreSpanBuilder(this, instrumentationName, operationName); + return createMultiSpanBuilder(instrumentationName, operationName); + } + + MultiSpanBuilder createMultiSpanBuilder( + final String instrumentationName, final CharSequence operationName) { + return new MultiSpanBuilder(this, instrumentationName, operationName); + } + + @Override + public CoreSpanBuilder singleSpanBuilder( + final String instrumentationName, final CharSequence operationName) { + return SPAN_BUILDER_REUSE_ENABLED + ? reuseSingleSpanBuilder(instrumentationName, operationName) + : createMultiSpanBuilder(instrumentationName, operationName); + } + + ReusableSingleSpanBuilder createSingleSpanBuilder( + final String instrumentationName, final CharSequence oprationName) { + ReusableSingleSpanBuilder singleSpanBuilder = new ReusableSingleSpanBuilder(this); + singleSpanBuilder.init(instrumentationName, oprationName); + return singleSpanBuilder; + } + + CoreSpanBuilder reuseSingleSpanBuilder( + final String instrumentationName, final CharSequence operationName) { + return reuseSingleSpanBuilder( + this, spanBuilderThreadLocalCache, instrumentationName, operationName); + } + + static final ReusableSingleSpanBuilder reuseSingleSpanBuilder( + final CoreTracer tracer, + 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 + return tracer.createSingleSpanBuilder(instrumentationName, operationName); + } + + // 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(); + boolean wasReset = tlSpanBuilder.reset(instrumentationName, operationName); + if (wasReset) return tlSpanBuilder; + + // TODO: counter for how often the fallback is used? + 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, + // that case could result in permanently burning the cache for a given thread. + + // 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. + + // 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); + + return newSpanBuilder; } @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 @@ -1408,42 +1488,40 @@ 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; + public abstract static class CoreSpanBuilder implements AgentTracer.SpanBuilder { + protected final CoreTracer tracer; + + 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; - - CoreSpanBuilder( - final CoreTracer tracer, - final String instrumentationName, - final CharSequence operationName) { - this.instrumentationName = instrumentationName; - this.operationName = operationName; + // Make sure any fields added here are also reset properly in ReusableSingleSpanBuilder.reset + 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; + // Make sure any fields added here are also reset properly in ReusableSingleSpanBuilder.reset + + CoreSpanBuilder(CoreTracer tracer) { this.tracer = tracer; } @Override - public CoreSpanBuilder ignoreActiveSpan() { + public final CoreSpanBuilder ignoreActiveSpan() { ignoreScope = true; return this; } - private DDSpan buildSpan() { + protected final DDSpan buildSpan() { DDSpan span = DDSpan.create(instrumentationName, timestampMicro, buildSpanContext(), links); if (span.isLocalRootSpan()) { EndpointTracker tracker = tracer.onRootSpanStarted(span); @@ -1454,7 +1532,7 @@ private DDSpan buildSpan() { 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(); @@ -1470,7 +1548,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(); @@ -1484,7 +1562,9 @@ private void addTerminatedContextAsLinks() { } @Override - public AgentSpan start() { + public abstract AgentSpan start(); + + protected AgentSpan startImpl() { AgentSpanContext pc = parent; if (pc == null && !ignoreScope) { final AgentSpan span = tracer.activeSpan(); @@ -1500,65 +1580,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; } @@ -1582,7 +1662,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; @@ -1598,7 +1679,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<>(); @@ -1609,7 +1690,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; } @@ -1620,7 +1701,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; @@ -1869,6 +1950,105 @@ private 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); + this.instrumentationName = instrumentationName; + this.operationName = operationName; + } + + @Override + public AgentSpan start() { + return this.startImpl(); + } + } + + 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); + } + } + + /** + * 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" + // until "start" is called + 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; + + this.inUse = true; + } + + /** + * 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; + + 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; + } + + /* + * Clears the inUse boolean, so this ReusableSpanBuilder can be reset + */ + @Override + public AgentSpan start() { + assert this.inUse + : "ReusableSingleSpanBuilder not reset properly -- multiple span construction?"; + + AgentSpan span = this.startImpl(); + 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 new file mode 100644 index 00000000000..120f3e812de --- /dev/null +++ b/dd-trace-core/src/test/java/datadog/trace/core/CoreTracerTest.java @@ -0,0 +1,180 @@ +package datadog.trace.core; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertFalse; +import static org.junit.jupiter.api.Assertions.assertNotSame; +import static org.junit.jupiter.api.Assertions.assertSame; +import static org.junit.jupiter.api.Assertions.assertThrows; +import static org.junit.jupiter.api.Assertions.assertTrue; + +import datadog.trace.api.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); + + @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.singleSpanBuilder("foo", "bar"); + builder1.start(); + + CoreSpanBuilder builder2 = TRACER.singleSpanBuilder("baz", "quux"); + builder2.start(); + + if (Config.get().isSpanBuilderReuseEnabled()) { + assertSame(builder1, builder2); + } else { + assertNotSame(builder1, builder2); + } + } + + @Test + 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"); + assertTrue(builder1.inUse); + + builder1.start(); + assertFalse(builder1.inUse); + + ReusableSingleSpanBuilder builder2 = + CoreTracer.reuseSingleSpanBuilder(TRACER, CACHE, "baz", "quux"); + assertTrue(builder2.inUse); + assertSame(builder1, builder2); + + builder2.start(); + assertFalse(builder2.inUse); + } + + @Test + 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"); + assertTrue(builder1.inUse); + + ReusableSingleSpanBuilder builder2 = + CoreTracer.reuseSingleSpanBuilder(TRACER, CACHE, "baz", "quux"); + assertTrue(builder2.inUse); + assertNotSame(builder1, builder2); + + builder2.start(); + assertFalse(builder2.inUse); + + 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); + } + + @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()); + } +} 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 ef0a02114da..d088ef603d2 100644 --- a/internal-api/src/main/java/datadog/trace/api/Config.java +++ b/internal-api/src/main/java/datadog/trace/api/Config.java @@ -1263,6 +1263,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; @@ -2806,6 +2807,8 @@ PROFILING_DATADOG_PROFILER_ENABLED, isDatadogProfilerSafeInCurrentEnvironment()) this.optimizedMapEnabled = configProvider.getBoolean(GeneralConfig.OPTIMIZED_MAP_ENABLED, false); + this.spanBuilderReuseEnabled = + configProvider.getBoolean(GeneralConfig.SPAN_BUILDER_REUSE_ENABLED, true); this.tagNameUtf8CacheSize = Math.max(configProvider.getInteger(GeneralConfig.TAG_NAME_UTF8_CACHE_SIZE, 128), 0); this.tagValueUtf8CacheSize = @@ -4568,6 +4571,10 @@ public boolean isOptimizedMapEnabled() { return optimizedMapEnabled; } + public boolean isSpanBuilderReuseEnabled() { + return spanBuilderReuseEnabled; + } + public int getTagNameUtf8CacheSize() { return tagNameUtf8CacheSize; } 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..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,8 +373,19 @@ 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); + /** + * 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 +554,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() {}