Skip to content
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
37 changes: 28 additions & 9 deletions dd-trace-core/src/main/java/datadog/trace/core/CoreTracer.java
Original file line number Diff line number Diff line change
Expand Up @@ -1219,7 +1219,7 @@ public AgentDataStreamsMonitoring getDataStreamsMonitoring() {
*
* @param trace a list of the spans related to the same trace
*/
void write(final List<DDSpan> trace) {
void write(final TraceList trace) {
if (trace.isEmpty() || !trace.get(0).traceConfig().isTraceEnabled()) {
return;
}
Expand Down Expand Up @@ -1256,9 +1256,22 @@ void write(final List<DDSpan> trace) {
}
}

private List<DDSpan> interceptCompleteTrace(List<DDSpan> trace) {
if (!interceptors.isEmpty() && !trace.isEmpty()) {
Collection<? extends MutableSpan> interceptedTrace = new ArrayList<>(trace);
private List<DDSpan> interceptCompleteTrace(TraceList originalTrace) {
if (!interceptors.isEmpty() && !originalTrace.isEmpty()) {
// Using TraceList to optimize the common case where the interceptors,
// don't alter the list. If the interceptors just return the provided
// List, then no need to copy to another List.

// As an extra precaution, also check the modCount before and after on
// the TraceList, since TraceInterceptor could put some other type of
// object into the List.

// There is still a risk that a TraceInterceptor holds onto the provided
// List and modifies it later on, but we cannot safeguard against
// every possible misuse.
Collection<? extends MutableSpan> interceptedTrace = originalTrace;
Copy link
Contributor Author

@dougqh dougqh Mar 12, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Previously, there was a defensive copy of the incoming trace; however, I couldn't see a reason why. The incoming List was already copied from the span buffer. And creating a new ArrayList didn't prevent mutation by the TraceInterceptor, since the TraceInterceptor could simply change the incoming list and return it anyway.

int originalModCount = originalTrace.modCount();

for (final TraceInterceptor interceptor : interceptors) {
try {
// If one TraceInterceptor throws an exception, then continue with the next one
Expand All @@ -1272,14 +1285,20 @@ private List<DDSpan> interceptCompleteTrace(List<DDSpan> trace) {
}
}

trace = new ArrayList<>(interceptedTrace.size());
for (final MutableSpan span : interceptedTrace) {
if (span instanceof DDSpan) {
trace.add((DDSpan) span);
if (interceptedTrace != originalTrace || originalTrace.modCount() != originalModCount) {
TraceList trace = new TraceList(interceptedTrace.size());
for (final MutableSpan span : interceptedTrace) {
if (span instanceof DDSpan) {
trace.add((DDSpan) span);
}
}
return trace;
} else {
return originalTrace;
}
} else {
return originalTrace;
}
return trace;
}

@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -327,7 +327,7 @@ private int write(boolean isPartial) {
if (!spans.isEmpty()) {
try (Recording recording = tracer.writeTimer()) {
// Only one writer at a time
final List<DDSpan> trace;
final TraceList trace;
int completedSpans = 0;
synchronized (this) {
if (!isPartial) {
Expand All @@ -346,10 +346,10 @@ private int write(boolean isPartial) {
// count(s) will be incremented, and any new spans added during the period that the count
// was negative will be written by someone even if we don't write them right now.
if (size > 0 && (!isPartial || size >= tracer.getPartialFlushMinSpans())) {
trace = new ArrayList<>(size);
trace = new TraceList(size);
completedSpans = enqueueSpansToWrite(trace, writeRunningSpans);
} else {
trace = EMPTY;
trace = TraceList.EMPTY;
}
}
if (!trace.isEmpty()) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@
import datadog.trace.api.time.TimeSource;
import datadog.trace.bootstrap.instrumentation.api.AgentScope;
import datadog.trace.core.monitor.HealthMetrics;
import java.util.Collections;
import java.util.concurrent.atomic.AtomicReferenceFieldUpdater;
import javax.annotation.Nonnull;

Expand Down Expand Up @@ -72,7 +71,7 @@ PublishState onPublish(DDSpan span) {
tracer.onRootSpanPublished(rootSpan);
}
healthMetrics.onFinishSpan();
tracer.write(Collections.singletonList(span));
tracer.write(TraceList.of(span));
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This part is slightly questionable, since TraceList (being an ArrayList) does incur an extra Object[] allocation that isn't needed for singletonList. I think this is offset by the efficiency gains elsewhere, but I should verify more carefully.

return PublishState.WRITTEN;
}

Expand Down
22 changes: 22 additions & 0 deletions dd-trace-core/src/main/java/datadog/trace/core/TraceList.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
package datadog.trace.core;

import java.util.ArrayList;

/** ArrayList that exposes modCount to allow for an optimization in TraceInterceptor handling */
final class TraceList extends ArrayList<DDSpan> {
static final TraceList EMPTY = new TraceList(0);

static final TraceList of(DDSpan span) {
TraceList list = new TraceList(1);
list.add(span);
return list;
}

TraceList(int capacity) {
super(capacity);
}

int modCount() {
return this.modCount;
}
}