Skip to content
47 changes: 41 additions & 6 deletions dd-trace-core/src/main/java/datadog/trace/core/DDSpan.java
Original file line number Diff line number Diff line change
Expand Up @@ -116,7 +116,8 @@ static DDSpan create(
*/
private volatile int longRunningVersion = 0;

protected final List<AgentSpanLink> links;
private static final List<AgentSpanLink> EMPTY = Collections.emptyList();
Copy link
Copy Markdown
Contributor

@amarziali amarziali Mar 12, 2026

Choose a reason for hiding this comment

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

There is an issue with SpanPointersProcessor. In fact TagsPostProcessors manipulate links directly and adding something to an emptyList will cause an UnsupportedOperationException. What can be done, is to use an empty non threadsafe list when calling tag processors if the original is empty and eventually batch adding the list at the end if the processors added something

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Few ideas here:

  1. There is a TODO from Doug about the processor that mutates the span links:

// DQH - TODO - There's a lot room to optimize this using TagMap's capabilities

Does it mean it can skip span links and use tags directly?

  1. As processors only add span links, we can replace the span links collection by a method reference to the addSpanLink method as Consumer<SpanLink>. This will avoid exposing the span link implementation and we can keep it null until there is effectively span link stored.

Copy link
Copy Markdown
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.

That's a good catch.
I do think we need to improve the encapsulation; otherwise, these changes are going to be hard to make.

As for my prior TODO, TagMap has a bunch of methods for removeAndGetEntry, getString, stringValue, etc that could be used in the various *spanPointer methods. In theory, they could simplify the code significantly.

Copy link
Copy Markdown
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.

On a related note, I'm curious why spanLinks was put on DDSpan rather than DDSpanContext.
It seems like it would be more consistent to put on DDSpanContext and then we wouldn't have this issue.

I also think we might be pool DDSpanContext in the future, so I'd like to avoid adding more to DDSpan.

Copy link
Copy Markdown
Contributor

@PerfectSlayer PerfectSlayer Mar 13, 2026

Choose a reason for hiding this comment

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

I'm curious why spanLinks was put on DDSpan rather than DDSpanContext.

SpanContext was supposed to describe "a span context", not to hold the span data model.
We should be able to use SpanContext as reference descriptor for spans, but they end up holding way too much data.
So when I add to implement span links, I made sure to leave them in Span, not SpanContext.


I had a look at its Javadoc and it basically should only be IDs (trace + span), sampling, a bit of (W3C) trace state and baggage, nothing else :)

Copy link
Copy Markdown
Contributor Author

@dougqh dougqh Mar 24, 2026

Choose a reason for hiding this comment

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

Okay, that makes sense. I think that maybe we should repurpose SpanContext into SpanContents.

I have a bit of crazy idea to recycle SpanContents, but I want to do so while maintaining Span object identity. In other words, I'd always create a new Span, but the SpanContents inside the span would be reused.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I've addressed the TagPostProcessor interaction by introducing two new interfaces: WritableSpanLinks and SpanLinkAccessor.

WritableSpanLinks just provides the ability to addLink, it is passed to TagPostProcessor. SpanLinkAccessor extends WritableSpanLinks and also provides a way to get the links.

DDSpan now implements SpanLinkAccessor. I picked this design, so I could avoid a capturing lambda around the span.

protected volatile List<AgentSpanLink> links;

/**
* Spans should be constructed using the builder, not by calling the constructor directly.
Expand Down Expand Up @@ -144,7 +145,7 @@ private DDSpan(
context.getTraceCollector().touch(); // external clock: explicitly update lastReferenced
}

this.links = links == null ? new CopyOnWriteArrayList<>() : new CopyOnWriteArrayList<>(links);
this.links = links == null || links.isEmpty() ? EMPTY : new CopyOnWriteArrayList<>(links);
}

public boolean isFinished() {
Expand Down Expand Up @@ -764,12 +765,12 @@ public CharSequence getType() {

@Override
public void processServiceTags() {
context.earlyProcessTags(links);
context.earlyProcessTags(this);
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

this DDSpan is now a SpanLinkAccessor / WritableSpanLinks

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Updated - now just an AppendableSpanLinks (previously WritableSpanLinks)
SpanLinkAccessor has been removed

}

@Override
public void processTagsAndBaggage(final MetadataConsumer consumer) {
context.processTagsAndBaggage(consumer, longRunningVersion, links);
context.processTagsAndBaggage(consumer, longRunningVersion, this);
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

this DDSpan is now a SpanLinkAccessor / WritableSpanLinks

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Updated - now just an AppendableSpanLinks (previously WritableSpanLinks)
SpanLinkAccessor has been removed

}

@Override
Expand Down Expand Up @@ -883,10 +884,44 @@ public TraceConfig traceConfig() {
return context.getTraceCollector().getTraceConfig();
}

List<? extends AgentSpanLink> getLinks() {
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I've kept this accessor, but it is now only package visible.
It was previously public because of needing to implement the SpanLinksAccessor interface which has now been removed.

return this.links;
}

@Override
public void addLink(AgentSpanLink link) {
if (link != null) {
this.links.add(link);
if (link == null) {
return;
}

// If links are initially null / empty, then the shared placeholder List EMPTY is used.
// Bacause EMPTY is shared, EMPTY is safe for reading, but not for writing.
// On write - if links is the EMPTY placeholder, then need to create a CopyOnWriteArrayList
// owned by this DDSpan

// Creation of the CopyOnWriteArrayList is done via double checking locking using volatile &
// synchronized

// If before or inside the synchronized block, links no longer points to EMPTY,
// then this thread or another thread has already handled the list construction,
// so just add to the list

// If links still points to EMPTY inside the synchronized block, then construct a new
// CopyOnWriteArrayList containing the newly added link

List<AgentSpanLink> links = this.links;
if (links != EMPTY) {
links.add(link);
return;
}

synchronized (this) {
links = this.links;
if (links != EMPTY) {
links.add(link);
} else {
this.links = new CopyOnWriteArrayList<>(Collections.singletonList(link));
}
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@
import datadog.trace.api.sampling.PrioritySampling;
import datadog.trace.api.sampling.SamplingMechanism;
import datadog.trace.bootstrap.instrumentation.api.AgentSpanContext;
import datadog.trace.bootstrap.instrumentation.api.AgentSpanLink;
import datadog.trace.bootstrap.instrumentation.api.AppendableSpanLinks;
import datadog.trace.bootstrap.instrumentation.api.Baggage;
import datadog.trace.bootstrap.instrumentation.api.ProfilerContext;
import datadog.trace.bootstrap.instrumentation.api.ProfilingContextIntegration;
Expand Down Expand Up @@ -1087,19 +1087,23 @@ public <T> void setMetaStruct(final String field, final T value) {
}
}

public void earlyProcessTags(List<AgentSpanLink> links) {
void earlyProcessTags(AppendableSpanLinks links) {
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Reduced the visibility of earlyProcessTags and processTagsAndBaggage
As far as I can tell, these are really just helper methods of DDSpan

synchronized (unsafeTags) {
TagsPostProcessorFactory.eagerProcessor().processTags(unsafeTags, this, links);
}
}

public void processTagsAndBaggage(
final MetadataConsumer consumer, int longRunningVersion, List<AgentSpanLink> links) {
void processTagsAndBaggage(
final MetadataConsumer consumer, int longRunningVersion, DDSpan restrictedSpan) {
// NOTE: The span is passed for the sole purpose of allowing updating & reading of the span links
Copy link
Copy Markdown
Contributor Author

@dougqh dougqh Mar 27, 2026

Choose a reason for hiding this comment

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

The name restrictedSpan and the comment are to try and dissuade others from doing unwanted things with span. That's also why I originally created SpanLinksAccessor, but that required making getLinks public which was arguably worse.

// This is a compromise to avoid...
// - creating an extra wrapper object that would create significant allocation
// - implementing an interface to read the spans that require making the read method public
synchronized (unsafeTags) {
// Tags
TagsPostProcessorFactory.lazyProcessor().processTags(unsafeTags, this, links);
TagsPostProcessorFactory.lazyProcessor().processTags(unsafeTags, this, restrictedSpan);

String linksTag = DDSpanLink.toTag(links);
String linksTag = DDSpanLink.toTag(restrictedSpan.getLinks());
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🎯 suggestion: ‏We don't even need to introduce getLinks() if you want to keep the changes minimal as the links field is already accessible from this place:

Suggested change
String linksTag = DDSpanLink.toTag(restrictedSpan.getLinks());
String linksTag = DDSpanLink.toTag(restrictedSpan.links);

if (linksTag != null) {
unsafeTags.put(SPAN_LINKS, linksTag);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,7 @@ public static SpanLink from(ExtractedContext context, SpanAttributes attributes)
* @param links The span link collection to encode.
* @return The encoded tag value, {@code null} if no links.
*/
public static String toTag(List<AgentSpanLink> links) {
public static String toTag(List<? extends AgentSpanLink> links) {
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Updated to accommodate return from SpanLinkAccessor.getLinks

if (links == null || links.isEmpty()) {
return null;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,9 +6,8 @@

import datadog.trace.api.TagMap;
import datadog.trace.api.endpoint.EndpointResolver;
import datadog.trace.bootstrap.instrumentation.api.AgentSpanLink;
import datadog.trace.bootstrap.instrumentation.api.AppendableSpanLinks;
import datadog.trace.core.DDSpanContext;
import java.util.List;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;

Expand Down Expand Up @@ -56,7 +55,7 @@ public HttpEndpointPostProcessor() {

@Override
public void processTags(
TagMap unsafeTags, DDSpanContext spanContext, List<AgentSpanLink> spanLinks) {
TagMap unsafeTags, DDSpanContext spanContext, AppendableSpanLinks spanLinks) {
if (!endpointResolver.isEnabled()) {
log.debug("EndpointResolver is not enabled, skipping HTTP endpoint post processing");
return;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,14 +3,13 @@
import static datadog.trace.api.DDTags.DD_INTEGRATION;

import datadog.trace.api.TagMap;
import datadog.trace.bootstrap.instrumentation.api.AgentSpanLink;
import datadog.trace.bootstrap.instrumentation.api.AppendableSpanLinks;
import datadog.trace.core.DDSpanContext;
import java.util.List;

public class IntegrationAdder extends TagsPostProcessor {
@Override
public void processTags(
TagMap unsafeTags, DDSpanContext spanContext, List<AgentSpanLink> spanLinks) {
TagMap unsafeTags, DDSpanContext spanContext, AppendableSpanLinks spanLinks) {
final CharSequence instrumentationName = spanContext.getIntegrationName();
if (instrumentationName != null) {
unsafeTags.set(DD_INTEGRATION, instrumentationName);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,10 +4,9 @@

import datadog.trace.api.DDTags;
import datadog.trace.api.TagMap;
import datadog.trace.bootstrap.instrumentation.api.AgentSpanLink;
import datadog.trace.bootstrap.instrumentation.api.AppendableSpanLinks;
import datadog.trace.bootstrap.instrumentation.api.UTF8BytesString;
import datadog.trace.core.DDSpanContext;
import java.util.List;
import javax.annotation.Nullable;

public final class InternalTagsAdder extends TagsPostProcessor {
Expand All @@ -21,7 +20,7 @@ public InternalTagsAdder(@Nullable final String ddService, @Nullable final Strin

@Override
public void processTags(
TagMap unsafeTags, DDSpanContext spanContext, List<AgentSpanLink> spanLinks) {
TagMap unsafeTags, DDSpanContext spanContext, AppendableSpanLinks spanLinks) {
if (spanContext == null || ddService == null) {
return;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@
import datadog.trace.api.ConfigDefaults;
import datadog.trace.api.TagMap;
import datadog.trace.api.telemetry.LogCollector;
import datadog.trace.bootstrap.instrumentation.api.AgentSpanLink;
import datadog.trace.bootstrap.instrumentation.api.AppendableSpanLinks;
import datadog.trace.core.DDSpanContext;
import datadog.trace.core.util.JsonStreamParser;
import datadog.trace.payloadtags.PayloadTagsData;
Expand Down Expand Up @@ -72,7 +72,7 @@ public static PayloadTagsProcessor create(Config config) {

@Override
public void processTags(
TagMap unsafeTags, DDSpanContext spanContext, List<AgentSpanLink> spanLinks) {
TagMap unsafeTags, DDSpanContext spanContext, AppendableSpanLinks spanLinks) {
int spanMaxTags = maxTags + unsafeTags.size();
for (Map.Entry<String, RedactionRules> tagPrefixRedactionRules :
redactionRulesByTagPrefix.entrySet()) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,10 +5,9 @@
import datadog.trace.api.TagMap;
import datadog.trace.api.naming.NamingSchema;
import datadog.trace.api.naming.SpanNaming;
import datadog.trace.bootstrap.instrumentation.api.AgentSpanLink;
import datadog.trace.bootstrap.instrumentation.api.AppendableSpanLinks;
import datadog.trace.bootstrap.instrumentation.api.Tags;
import datadog.trace.core.DDSpanContext;
import java.util.List;
import java.util.Map;
import javax.annotation.Nonnull;

Expand All @@ -34,7 +33,7 @@ public PeerServiceCalculator() {

@Override
public void processTags(
TagMap unsafeTags, DDSpanContext spanContext, List<AgentSpanLink> spanLinks) {
TagMap unsafeTags, DDSpanContext spanContext, AppendableSpanLinks spanLinks) {
Object peerService = unsafeTags.getObject(Tags.PEER_SERVICE);
// the user set it
if (peerService != null) {
Expand Down
Original file line number Diff line number Diff line change
@@ -1,9 +1,8 @@
package datadog.trace.core.tagprocessor;

import datadog.trace.api.TagMap;
import datadog.trace.bootstrap.instrumentation.api.AgentSpanLink;
import datadog.trace.bootstrap.instrumentation.api.AppendableSpanLinks;
import datadog.trace.core.DDSpanContext;
import java.util.List;
import java.util.Objects;
import javax.annotation.Nonnull;

Expand All @@ -16,7 +15,7 @@ public PostProcessorChain(@Nonnull final TagsPostProcessor... processors) {

@Override
public void processTags(
TagMap unsafeTags, DDSpanContext spanContext, List<AgentSpanLink> spanLinks) {
TagMap unsafeTags, DDSpanContext spanContext, AppendableSpanLinks spanLinks) {
for (final TagsPostProcessor tagsPostProcessor : chain) {
tagsPostProcessor.processTags(unsafeTags, spanContext, spanLinks);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,11 +5,10 @@
import com.google.re2j.PatternSyntaxException;
import datadog.trace.api.DDTags;
import datadog.trace.api.TagMap;
import datadog.trace.bootstrap.instrumentation.api.AgentSpanLink;
import datadog.trace.bootstrap.instrumentation.api.AppendableSpanLinks;
import datadog.trace.bootstrap.instrumentation.api.Tags;
import datadog.trace.core.DDSpanContext;
import datadog.trace.util.Strings;
import java.util.List;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;

Expand Down Expand Up @@ -59,7 +58,7 @@ private String obfuscate(String query) {

@Override
public void processTags(
TagMap unsafeTags, DDSpanContext spanContext, List<AgentSpanLink> spanLinks) {
TagMap unsafeTags, DDSpanContext spanContext, AppendableSpanLinks spanLinks) {
Object query = unsafeTags.getObject(DDTags.HTTP_QUERY);
if (query instanceof CharSequence) {
query = obfuscate(query.toString());
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,9 +2,8 @@

import datadog.trace.api.DDTags;
import datadog.trace.api.TagMap;
import datadog.trace.bootstrap.instrumentation.api.AgentSpanLink;
import datadog.trace.bootstrap.instrumentation.api.AppendableSpanLinks;
import datadog.trace.core.DDSpanContext;
import java.util.List;
import java.util.function.Supplier;

public final class RemoteHostnameAdder extends TagsPostProcessor {
Expand All @@ -16,7 +15,7 @@ public RemoteHostnameAdder(Supplier<String> hostnameSupplier) {

@Override
public void processTags(
TagMap unsafeTags, DDSpanContext spanContext, List<AgentSpanLink> spanLinks) {
TagMap unsafeTags, DDSpanContext spanContext, AppendableSpanLinks spanLinks) {
if (spanContext.getSpanId() == spanContext.getRootSpanId()) {
unsafeTags.put(DDTags.TRACER_HOST, hostnameSupplier.get());
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,14 +3,13 @@
import static datadog.trace.api.DDTags.DD_SVC_SRC;

import datadog.trace.api.TagMap;
import datadog.trace.bootstrap.instrumentation.api.AgentSpanLink;
import datadog.trace.bootstrap.instrumentation.api.AppendableSpanLinks;
import datadog.trace.core.DDSpanContext;
import java.util.List;

public class ServiceNameSourceAdder extends TagsPostProcessor {
@Override
public void processTags(
TagMap unsafeTags, DDSpanContext spanContext, List<AgentSpanLink> spanLinks) {
TagMap unsafeTags, DDSpanContext spanContext, AppendableSpanLinks spanLinks) {
final CharSequence serviceNameSource = spanContext.getServiceNameSource();
if (serviceNameSource != null) {
unsafeTags.set(DD_SVC_SRC, serviceNameSource);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@

import datadog.trace.api.TagMap;
import datadog.trace.bootstrap.instrumentation.api.AgentSpanLink;
import datadog.trace.bootstrap.instrumentation.api.AppendableSpanLinks;
import datadog.trace.bootstrap.instrumentation.api.SpanAttributes;
import datadog.trace.bootstrap.instrumentation.api.SpanLink;
import datadog.trace.core.DDSpanContext;
Expand All @@ -21,7 +22,6 @@
import java.security.MessageDigest;
import java.security.NoSuchAlgorithmException;
import java.util.Arrays;
import java.util.List;
import java.util.Map;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
Expand All @@ -38,16 +38,16 @@ public class SpanPointersProcessor extends TagsPostProcessor {

@Override
public void processTags(
TagMap unsafeTags, DDSpanContext spanContext, List<AgentSpanLink> spanLinks) {
TagMap unsafeTags, DDSpanContext spanContext, AppendableSpanLinks spanLinks) {
// DQH - TODO - There's a lot room to optimize this using TagMap's capabilities
AgentSpanLink s3Link = handleS3SpanPointer(unsafeTags);
if (s3Link != null) {
spanLinks.add(s3Link);
spanLinks.addLink(s3Link);
}

AgentSpanLink dynamoDbLink = handleDynamoDbSpanPointer(unsafeTags);
if (dynamoDbLink != null) {
spanLinks.add(dynamoDbLink);
spanLinks.addLink(dynamoDbLink);
}
}

Expand Down
Original file line number Diff line number Diff line change
@@ -1,23 +1,10 @@
package datadog.trace.core.tagprocessor;

import datadog.trace.api.TagMap;
import datadog.trace.bootstrap.instrumentation.api.AgentSpanLink;
import datadog.trace.bootstrap.instrumentation.api.AppendableSpanLinks;
import datadog.trace.core.DDSpanContext;
import java.util.List;
import java.util.Map;

public abstract class TagsPostProcessor {
/*
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Got rid of the old processTags which I had kept around for test migration.
That does mean that I had to update a lot of tests.

* DQH - For testing purposes only
*/
@Deprecated
final Map<String, Object> processTags(
Map<String, Object> unsafeTags, DDSpanContext context, List<AgentSpanLink> links) {
TagMap map = TagMap.fromMap(unsafeTags);
this.processTags(map, context, links);
return map;
}

public abstract void processTags(
TagMap unsafeTags, DDSpanContext spanContext, List<AgentSpanLink> spanLinks);
TagMap unsafeTags, DDSpanContext spanContext, AppendableSpanLinks spanLinks);
}
Loading
Loading