Skip to content

Cache span.kind as byte ordinal for fast isOutbound() #11116

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
dougqh wants to merge 9 commits into
base: master
Choose a base branch
from
Open

Cache span.kind as byte ordinal for fast isOutbound() #11116

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
f4d6a09
Cache span.kind as byte ordinal on DDSpanContext for fast isOutbound()
dougqh Apr 15, 2026
a8f31c4
Address review feedback on span.kind caching
dougqh Apr 15, 2026
97e0894
Simplify span.kind ordinal mapping per review feedback
dougqh Apr 15, 2026
a77da4c
Extract getSpanKindTag helper and simplify benchmark setup
dougqh Apr 15, 2026
bd848b5
Remove isClientSpanKind helper, rename getSpanKindTag to getSpanKindS…
dougqh Apr 15, 2026
632a5cd
Explicitly initialize spanKindOrdinal to SPAN_KIND_UNSET for clarity
dougqh Apr 15, 2026
0913173
Add clarifying comment in removeTag about unsafeTags update
dougqh Apr 15, 2026
e497ad0
Merge branch 'master' into dougqh/cache-span-kind-ordinal
dougqh Apr 15, 2026
0738b6a
Rename setSpanKind to setSpanKindOrdinal for clarity
dougqh Apr 16, 2026
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
69 changes: 69 additions & 0 deletions dd-trace-core/src/jmh/java/datadog/trace/core/IsOutboundBenchmark.java
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,69 @@
package datadog.trace.core;

import static java.util.concurrent.TimeUnit.NANOSECONDS;

import datadog.trace.bootstrap.instrumentation.api.Tags;
import org.openjdk.jmh.annotations.Benchmark;
import org.openjdk.jmh.annotations.BenchmarkMode;
import org.openjdk.jmh.annotations.Fork;
import org.openjdk.jmh.annotations.Measurement;
import org.openjdk.jmh.annotations.Mode;
import org.openjdk.jmh.annotations.OutputTimeUnit;
import org.openjdk.jmh.annotations.Scope;
import org.openjdk.jmh.annotations.Setup;
import org.openjdk.jmh.annotations.State;
import org.openjdk.jmh.annotations.Warmup;

/**
* Measures the cost of DDSpan.isOutbound(), which is called on every root span start and finish.
*/
@State(Scope.Thread)
@Warmup(iterations = 3, time = 1)
@Measurement(iterations = 5, time = 1)
@BenchmarkMode(Mode.AverageTime)
@OutputTimeUnit(NANOSECONDS)
@Fork(value = 1)
public class IsOutboundBenchmark {

static final CoreTracer TRACER = CoreTracer.builder().build();

private DDSpan clientSpan;
private DDSpan serverSpan;
private DDSpan unsetSpan;

@Setup
public void setup() {
clientSpan = (DDSpan) TRACER.startSpan("benchmark", "client.op");
clientSpan.setTag(Tags.SPAN_KIND, Tags.SPAN_KIND_CLIENT);

serverSpan = (DDSpan) TRACER.startSpan("benchmark", "server.op");
serverSpan.setTag(Tags.SPAN_KIND, Tags.SPAN_KIND_SERVER);

unsetSpan = (DDSpan) TRACER.startSpan("benchmark", "unset.op");
}

@Benchmark
public boolean isOutbound_client() {
return clientSpan.isOutbound();
}

@Benchmark
public boolean isOutbound_server() {
return serverSpan.isOutbound();
}

@Benchmark
public boolean isOutbound_unset() {
return unsetSpan.isOutbound();
}

@Benchmark
public Object getTag_spanKind_client() {
return clientSpan.getTag(Tags.SPAN_KIND);
}

@Benchmark
public Object getTag_spanKind_unset() {
return unsetSpan.getTag(Tags.SPAN_KIND);
}
}
5 changes: 2 additions & 3 deletions dd-trace-core/src/main/java/datadog/trace/core/DDSpan.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,6 @@
import datadog.trace.bootstrap.instrumentation.api.ErrorPriorities;
import datadog.trace.bootstrap.instrumentation.api.ResourceNamePriorities;
import datadog.trace.bootstrap.instrumentation.api.SpanWrapper;
import datadog.trace.bootstrap.instrumentation.api.Tags;
import datadog.trace.core.util.StackTraces;
import edu.umd.cs.findbugs.annotations.NonNull;
import edu.umd.cs.findbugs.annotations.SuppressFBWarnings;
Expand Down Expand Up @@ -943,8 +942,8 @@ public DDSpan setMetaStruct(final String field, final Object value) {

@Override
public boolean isOutbound() {
Object spanKind = context.getTag(Tags.SPAN_KIND);
return Tags.SPAN_KIND_CLIENT.equals(spanKind) || Tags.SPAN_KIND_PRODUCER.equals(spanKind);
byte ordinal = context.getSpanKindOrdinal();
return ordinal == DDSpanContext.SPAN_KIND_CLIENT || ordinal == DDSpanContext.SPAN_KIND_PRODUCER;
}

@Override
Expand Down
85 changes: 79 additions & 6 deletions dd-trace-core/src/main/java/datadog/trace/core/DDSpanContext.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -97,6 +97,32 @@ public class DDSpanContext
private final UTF8BytesString threadName;

private volatile short httpStatusCode;
Comment thread
dougqh marked this conversation as resolved.

// Cached span.kind ordinal for fast isOutbound() checks.
Comment thread
dougqh marked this conversation as resolved.
// Ordinal constants -- keep in sync with SPAN_KIND_VALUES array.
Copy link
Copy Markdown
Contributor

@mcculls mcculls Apr 16, 2026

Choose a reason for hiding this comment

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

Suggestion: add a small unit test to enforce these are kept in sync

Copy link
Copy Markdown
Contributor Author

@dougqh dougqh Apr 16, 2026

Choose a reason for hiding this comment

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

Yes, I think that would be a good idea. I will have Claude add that.

static final byte SPAN_KIND_UNSET = 0;
static final byte SPAN_KIND_SERVER = 1;
static final byte SPAN_KIND_CLIENT = 2;
static final byte SPAN_KIND_PRODUCER = 3;
static final byte SPAN_KIND_CONSUMER = 4;
static final byte SPAN_KIND_INTERNAL = 5;
static final byte SPAN_KIND_BROKER = 6;
static final byte SPAN_KIND_CUSTOM = 7;

/** Maps ordinal to canonical string constant. Index 0 (UNSET) and 7 (CUSTOM) are null. */
static final String[] SPAN_KIND_VALUES = {
null, // UNSET
Tags.SPAN_KIND_SERVER,
Tags.SPAN_KIND_CLIENT,
Tags.SPAN_KIND_PRODUCER,
Tags.SPAN_KIND_CONSUMER,
Tags.SPAN_KIND_INTERNAL,
Tags.SPAN_KIND_BROKER,
null // CUSTOM
};

private volatile byte spanKindOrdinal = SPAN_KIND_UNSET;

private CharSequence integrationName;
private CharSequence serviceNameSource;

Expand Down Expand Up @@ -716,6 +742,51 @@ public short getHttpStatusCode() {
return httpStatusCode;
}

/** Identity-first string comparison: checks reference equality, then falls back to equals. */
static boolean tagEquals(String tagValue, String tagLiteral) {
return (tagValue == tagLiteral) || tagLiteral.equals(tagValue);
Copy link
Copy Markdown
Contributor

@mcculls mcculls Apr 16, 2026

Choose a reason for hiding this comment

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

Bit surprised this makes a difference, given String.equals has this == anObject as the first check :)

Copy link
Copy Markdown
Contributor Author

@dougqh dougqh Apr 16, 2026

Choose a reason for hiding this comment

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

Yes, the difference is usually pretty negligible especially when the the type is final and the call gets statically devirtualized.

I did see a small improvement from the same things for keys in TagMap, but don't recall how much.

}

/**
* Cache the span.kind ordinal for fast isOutbound() checks. Called from TagInterceptor when
* span.kind is set.
*/
public void setSpanKindOrdinal(String kind) {
if (kind == null) {
spanKindOrdinal = SPAN_KIND_UNSET;
} else if (tagEquals(kind, Tags.SPAN_KIND_SERVER)) {
Copy link
Copy Markdown
Contributor

@mcculls mcculls Apr 16, 2026

Choose a reason for hiding this comment

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

How does a string-switch perform compared to if...else ? I'd imagine the hashed jump would help here...

Copy link
Copy Markdown
Contributor Author

@dougqh dougqh Apr 16, 2026

Choose a reason for hiding this comment

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

Yes, I had Claude try that. It didn't make much difference.
Admittedly, that was a fast path of referential checks followed by a slow path of string switch.

In past benchmarks, I've found that the fastest option is usually...
int hash = str.hashCode();
if ( hash == LITERAL_HASH && LITERAL.equals(str) ) {
...
}

That is close to what string switch does, but string switch is slower.
I also expect that the benefit of hashCode comparison can be situational.

For tags, we're often in the situation where the tag parameter is a constant, so the JIT can take advantage of inlining and constant propagation to optimize a lot. And I suspect that the hash comparison would likely get in the way of that.

But when the parameter isn't a constant, the hash comparison can clearly be beneficial.

spanKindOrdinal = SPAN_KIND_SERVER;
} else if (tagEquals(kind, Tags.SPAN_KIND_CLIENT)) {
spanKindOrdinal = SPAN_KIND_CLIENT;
} else if (tagEquals(kind, Tags.SPAN_KIND_PRODUCER)) {
spanKindOrdinal = SPAN_KIND_PRODUCER;
} else if (tagEquals(kind, Tags.SPAN_KIND_CONSUMER)) {
spanKindOrdinal = SPAN_KIND_CONSUMER;
} else if (tagEquals(kind, Tags.SPAN_KIND_INTERNAL)) {
spanKindOrdinal = SPAN_KIND_INTERNAL;
} else if (tagEquals(kind, Tags.SPAN_KIND_BROKER)) {
spanKindOrdinal = SPAN_KIND_BROKER;
} else {
spanKindOrdinal = SPAN_KIND_CUSTOM;
}
}

byte getSpanKindOrdinal() {
return spanKindOrdinal;
}

/** Returns the span.kind string from the cached ordinal, or falls back to the tag map. */
public String getSpanKindString() {
byte ordinal = spanKindOrdinal;
if (ordinal > SPAN_KIND_UNSET && ordinal < SPAN_KIND_CUSTOM) {
return SPAN_KIND_VALUES[ordinal];
}
// UNSET or CUSTOM -- fall through to tag map
synchronized (unsafeTags) {
return unsafeTags.getString(Tags.SPAN_KIND);
}
}

public void setOrigin(final CharSequence origin) {
DDSpanContext context = getRootSpanContextOrThis();
context.origin = origin;
Expand Down Expand Up @@ -763,6 +834,10 @@ public void setMetric(final TagMap.EntryReader entry) {
}

public void removeTag(String tag) {
if (tagEquals(tag, Tags.SPAN_KIND)) {
// Clear the cached ordinal; unsafeTags still needs to be updated below.
spanKindOrdinal = SPAN_KIND_UNSET;
Comment thread
dougqh marked this conversation as resolved.
}
synchronized (unsafeTags) {
unsafeTags.remove(tag);
}
Expand All @@ -782,9 +857,7 @@ public void setTag(final String tag, final Object value) {
return;
}
if (null == value) {
synchronized (unsafeTags) {
unsafeTags.remove(tag);
}
removeTag(tag);
} else if (!tagInterceptor.interceptTag(this, tag, value)) {
synchronized (unsafeTags) {
unsafeTags.set(tag, value);
Expand All @@ -797,9 +870,7 @@ public void setTag(final String tag, final String value) {
return;
}
if (null == value) {
synchronized (unsafeTags) {
unsafeTags.remove(tag);
}
removeTag(tag);
} else if (!tagInterceptor.interceptTag(this, tag, value)) {
synchronized (unsafeTags) {
unsafeTags.set(tag, value);
Expand Down Expand Up @@ -1015,6 +1086,8 @@ Object getTag(final String key) {
return threadName.toString();
case Tags.HTTP_STATUS:
return 0 == httpStatusCode ? null : (int) httpStatusCode;
case Tags.SPAN_KIND:
return getSpanKindString();
default:
Object value;
synchronized (unsafeTags) {
Expand Down
8 changes: 7 additions & 1 deletion dd-trace-core/src/main/java/datadog/trace/core/taginterceptor/TagInterceptor.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -123,6 +123,7 @@ public boolean needsIntercept(String tag) {
case HTTP_URL:
case ORIGIN_KEY:
case MEASURED:
case Tags.SPAN_KIND:
return true;

default:
Expand Down Expand Up @@ -193,6 +194,11 @@ public boolean interceptTag(DDSpanContext span, String tag, Object value) {
return interceptOrigin(span, value);
case MEASURED:
return interceptMeasured(span, value);
case Tags.SPAN_KIND:
// Cache the ordinal for fast isOutbound() checks.
// Return false so the value is still stored in unsafeTags for serialization.
span.setSpanKindOrdinal(String.valueOf(value));
return false;
default:
return intercept(span, tag, value);
}
Expand Down Expand Up @@ -223,7 +229,7 @@ private static void setResourceFromUrl(
path = uri == null ? null : uri.getPath();
}
if (path != null) {
final boolean isClient = Tags.SPAN_KIND_CLIENT.equals(span.unsafeGetTag(Tags.SPAN_KIND));
final boolean isClient = Tags.SPAN_KIND_CLIENT.equals(span.getSpanKindString());
Copy link
Copy Markdown
Contributor Author

@dougqh dougqh Apr 15, 2026 •
edited
Loading

Choose a reason for hiding this comment

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

Because I decided to keep the "enum" package visible for now, we cannot do a faster ordinal comparison here. I think that's okay.

Pair<CharSequence, Byte> normalized =
isClient
? HttpResourceNames.computeForClient(method, path, false)
Expand Down
Loading