Skip to content

Commit 8930561

Browse files
rkennkedevflow.devflow-routing-intake
andauthored
profiling(ddprof): migrate context bridge to the all-native API (Phase 2) (#11899)
profiling(ddprof): migrate context bridge to the all-native API Switch the profiler context bridge off the deprecated DirectByteBuffer (DBB) context API onto java-profiler's all-native API (setTraceContext / clearTraceContext / setContextValue / clearContextValue). This eliminates the virtual-thread use-after-free (the DBB cached buffer that dangled on carrier migration) and folds the per-activation sequence (setContext + two setContextValue) into a single native call. - DatadogProfilingIntegration.activate: one setTraceContext(...) carrying trace/span context + operation and resource attributes (3 JNI calls -> 1); close/clearContext: clearTraceContext() (wipes op/resource slots too). - DatadogProfiler: setContextValue/clearContextValue/reapplyAppContext/ syncNativeAppContext are now all-native. reapplyAppContext uses a per-slot native setContextValue loop (native setContextValue publishes valid=1, so app context stays visible without an active span — preserves PR #11646). A native batch reapply is deferred to a measured follow-up (java-profiler PROF-15361). - AppContextSnapshot simplified to strings-only (the native path resolves each value's encoding via the process-wide cache; no cached id/utf8/snapshotTags). - snapshot() uses the new native copyContextTags read (no ThreadContext/DBB, so it observes native writes without resetting the record). - ContextSetter kept only for offsetOf + size (pure Java); no DBB usage remains. Requires java-profiler with the all-native API (ddprof >= the phase-1 release; DataDog/java-profiler#631). Build/test with -PddprofUseSnapshot=true against a local publishToMavenLocal 1.47.0-SNAPSHOT until that ships. ddprof suite is Linux-gated (assumeTrue(isLinux)) — verify on Linux/CI. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> profiling(ddprof): guard zero span in setTraceContext; document clearContextValue The native setTraceContext rejects spanId==0 with IllegalArgumentException (it is the activation path; clearing is clearTraceContext). The bridge's catch(Throwable) would swallow that throw and leave the previous span's context stale on the thread. Span ids are non-zero by construction (IdGenerationStrategy never yields 0, DDSpanId.ZERO means "no span", and DDSpanContext is the only ProfilerContext), so this is defensive: route a zero span to a clean clearTraceContext instead of a silently-swallowed throw over stale state. Also document clearContextValue(int)'s return contract (@param/@return) and add a testContextRegistration scenario asserting a zero-span activation does not throw and still reapplies app context. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> profiling(ddprof): describe current behavior in comments; drop historical references Rewrite or remove comments that documented the superseded DirectByteBuffer context API and the ddprof-version history of the bridge (e.g. "Replaces the previous setContext...", "no DBB read", the 1.41.0/1.45.0 no-op history in DatadogProfilingScope). Those explain how the code got here, not what it does now; the commit history and PRs carry that evolution. Comment-only, behavior unchanged. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> profiling(ddprof): fix JMH benchmark for strings-only AppContextSnapshot AppContextSnapshotBenchmark.setup() still called the old record(int, int, byte[], String) signature; AppContextSnapshot.record is now record(int, String) (strings-only, per the all-native context migration). compileJmhJava was failing in CI. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> profiling(ddprof): fix two reapply ordering bugs found by PR review bots - setContextValue: a rejected native write (e.g. >255-byte UTF-8) clears the native slot but left the Java snapshot untouched, so the attribute read as unset until the next span boundary silently resurrected the stale prior value. Reapply immediately on rejection so the prior value stays visible continuously, matching pre-migration DBB behavior. - setTraceContext: reapplyAppContext() ran unconditionally after the native call, so when profiling.context.attributes also names _dd.trace.operation/resource (with span-name/resource-name context enabled), the trailing reapply clobbered the span-derived value that setTraceContext just wrote to the same offset with a stale app-recorded one. reapplyAppContext now takes the operation/resource offsets to skip. Both were flagged independently by Codex and Datadog Autotest PR review bots on #11899, with a concrete repro for the first. Added regression coverage to DatadogProfilerTest#testContextRegistration for both (verified each new assertion fails without its corresponding fix). Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Merge branch 'master' into rkennke/profiler-all-native-context-phase2 Rename reapplyAppContext skip-offset params to operationOffset/resourceOffset Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Merge branch 'master' into rkennke/profiler-all-native-context-phase2 Co-authored-by: devflow.devflow-routing-intake <devflow.devflow-routing-intake@kubernetes.us1.ddbuild.io>
1 parent ca4153b commit 8930561

5 files changed

Lines changed: 211 additions & 111 deletions

File tree

dd-java-agent/agent-profiling/profiling-ddprof/src/jmh/java/com/datadog/profiling/ddprof/AppContextSnapshotBenchmark.java

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,5 @@
11
package com.datadog.profiling.ddprof;
22

3-
import java.nio.charset.StandardCharsets;
43
import java.util.concurrent.TimeUnit;
54
import org.openjdk.jmh.annotations.Benchmark;
65
import org.openjdk.jmh.annotations.BenchmarkMode;
@@ -52,8 +51,7 @@ public class AppContextSnapshotBenchmark {
5251
public void setup() {
5352
source = new DatadogProfiler.AppContextSnapshot(attrCount);
5453
for (int i = 0; i < attrCount; i++) {
55-
byte[] utf8 = ("value-" + i).getBytes(StandardCharsets.UTF_8);
56-
source.record(i, i + 1, utf8, "value-" + i);
54+
source.record(i, "value-" + i);
5755
}
5856
slot = new DatadogProfiler.AppContextSnapshot(attrCount);
5957
stack = new DatadogProfiler.ScopeStack(attrCount);

dd-java-agent/agent-profiling/profiling-ddprof/src/main/java/com/datadog/profiling/ddprof/DatadogProfiler.java

Lines changed: 127 additions & 91 deletions
Original file line numberDiff line numberDiff line change
@@ -44,7 +44,6 @@
4444
import datadog.trace.bootstrap.instrumentation.api.TaskWrapper;
4545
import datadog.trace.util.TempLocationManager;
4646
import java.io.IOException;
47-
import java.nio.charset.StandardCharsets;
4847
import java.nio.file.Files;
4948
import java.nio.file.Path;
5049
import java.nio.file.attribute.PosixFilePermissions;
@@ -111,30 +110,20 @@ public static DatadogProfiler newInstance(ConfigProvider configProvider) {
111110
private final List<String> orderedContextAttributes;
112111

113112
// True for each attribute slot that was configured by the application (e.g. foo, bar).
114-
// ddprof wipes all custom slots on setContext; these slots are re-applied via
115-
// reapplyAppContext() on span activation.
113+
// setTraceContext/clearTraceContext reset all custom slots; these app-owned slots are
114+
// re-applied afterwards via reapplyAppContext().
116115
private final boolean[] isAppOffset;
117116

118117
private final boolean hasAppContext;
119118

120119
/**
121120
* Per-thread snapshot of application attribute values. Lazily allocated; only threads that call
122-
* setContextValue for an app attribute ever allocate. Holds the ddprof constant ID and
123-
* pre-encoded UTF-8 bytes for each slot, ready for a zero-allocation reapply via
124-
* setContextValuesByIdAndBytes.
121+
* setContextValue for an app attribute ever allocate. Holds the String value for each slot;
122+
* reapply resolves each value's encoding through the process-wide value cache.
125123
*/
126124
private final ThreadLocal<AppContextSnapshot> appContextValues = new ThreadLocal<>();
127125

128126
private final ThreadLocal<ScopeStack> scopeStack = new ThreadLocal<>();
129-
// Scratch buffer for snapshotTags in recordAppContextValue; per-thread, sized to context slots.
130-
// Lives here rather than on AppContextSnapshot so save-slots in ScopeStack don't carry it.
131-
private final ThreadLocal<int[]> contextScratch =
132-
new ThreadLocal<int[]>() {
133-
@Override
134-
protected int[] initialValue() {
135-
return new int[isAppOffset.length];
136-
}
137-
};
138127

139128
/** Per-thread stack of pre-allocated save slots for {@link DatadogProfilingScope}. */
140129
static final class ScopeStack {
@@ -170,35 +159,28 @@ void release() {
170159

171160
// Package-private so DatadogProfilingScope can hold a typed reference for save/restore.
172161
static final class AppContextSnapshot {
173-
private final int[] ids;
174-
private final byte[][] utf8;
175-
// Cached string values for change detection — avoids re-encoding and re-snapshotting
176-
// the constant ID when the same value is set again on the same thread.
162+
// App-managed attribute values, indexed by slot. A value is written via
163+
// JavaProfiler.setContextValue, which resolves it through the process-wide value cache, so the
164+
// snapshot only needs the String value.
177165
private final String[] strings;
178-
// Count of slots with a non-zero constant ID; allows O(1) isEmpty().
166+
// Count of slots with a non-null value; allows O(1) isEmpty().
179167
private int nonZeroCount;
180168

181169
AppContextSnapshot(int size) {
182-
ids = new int[size];
183-
utf8 = new byte[size][];
184170
strings = new String[size];
185171
}
186172

187-
void record(int offset, int constantId, byte[] utf8Bytes, String value) {
188-
if (ids[offset] == 0 && constantId != 0) {
173+
void record(int offset, String value) {
174+
if (strings[offset] == null && value != null) {
189175
nonZeroCount++;
190-
} else if (ids[offset] != 0 && constantId == 0) {
176+
} else if (strings[offset] != null && value == null) {
191177
nonZeroCount--;
192178
}
193-
ids[offset] = constantId;
194-
utf8[offset] = utf8Bytes;
195179
strings[offset] = value;
196180
}
197181

198182
void clear(int offset) {
199-
if (ids[offset] != 0) nonZeroCount--;
200-
ids[offset] = 0;
201-
utf8[offset] = null;
183+
if (strings[offset] != null) nonZeroCount--;
202184
strings[offset] = null;
203185
}
204186

@@ -214,24 +196,12 @@ String stringAt(int offset) {
214196
return strings[offset];
215197
}
216198

217-
int[] ids() {
218-
return ids;
219-
}
220-
221-
byte[][] utf8() {
222-
return utf8;
223-
}
224-
225199
void copyFrom(AppContextSnapshot src) {
226-
System.arraycopy(src.ids, 0, ids, 0, ids.length);
227-
System.arraycopy(src.utf8, 0, utf8, 0, utf8.length);
228200
System.arraycopy(src.strings, 0, strings, 0, strings.length);
229201
nonZeroCount = src.nonZeroCount;
230202
}
231203

232204
void reset() {
233-
Arrays.fill(ids, 0);
234-
Arrays.fill(utf8, null);
235205
Arrays.fill(strings, null);
236206
nonZeroCount = 0;
237207
}
@@ -271,7 +241,8 @@ private DatadogProfiler(ConfigProvider configProvider) {
271241
this.orderedContextAttributes = getOrderedContextAttributes(contextAttributes, configProvider);
272242
this.contextSetter = new ContextSetter(profiler, orderedContextAttributes);
273243
// ContextSetter deduplicates and truncates to 10 internally; size arrays to its actual size.
274-
int contextSize = contextSetter.snapshotTags().length;
244+
// size() and offsetOf are pure-Java; they are the only ContextSetter methods this bridge uses.
245+
int contextSize = contextSetter.size();
275246
boolean[] appOffsets = new boolean[contextSize];
276247
boolean anyApp = false;
277248
for (String attribute : contextAttributes) {
@@ -514,35 +485,84 @@ public int offsetOf(String attribute) {
514485
return contextSetter.offsetOf(attribute);
515486
}
516487

517-
public void setSpanContext(long rootSpanId, long spanId, long traceIdHigh, long traceIdLow) {
488+
/**
489+
* Combined per-activation write: full trace/span context plus the span-derived operation and
490+
* resource attributes, in a single native call, followed by a reapply of app-managed attributes
491+
* (the native call resets all custom slots). A negative attribute offset (or null value) skips
492+
* that attribute.
493+
*/
494+
public void setTraceContext(
495+
long rootSpanId,
496+
long spanId,
497+
long traceIdHigh,
498+
long traceIdLow,
499+
int operationOffset,
500+
CharSequence operationName,
501+
int resourceOffset,
502+
CharSequence resourceName) {
503+
if (spanId == 0) {
504+
// The native setTraceContext is the activation path and rejects a zero span with
505+
// IllegalArgumentException — which the catch below would swallow, leaving the previous
506+
// span's context stale on this thread. Span ids are non-zero by construction
507+
// (IdGenerationStrategy never returns 0; DDSpanId.ZERO means "no span"), so a zero here is
508+
// not expected; degrade to a clean clear rather than a silently-swallowed throw over stale
509+
// state.
510+
clearTraceContext();
511+
return;
512+
}
518513
debugLogging(rootSpanId);
519514
try {
520-
profiler.setContext(rootSpanId, spanId, traceIdHigh, traceIdLow);
515+
profiler.setTraceContext(
516+
rootSpanId,
517+
spanId,
518+
traceIdHigh,
519+
traceIdLow,
520+
operationOffset,
521+
operationName,
522+
resourceOffset,
523+
resourceName);
521524
} catch (Throwable e) {
522-
log.debug("Failed to clear context", e);
525+
log.debug("Failed to set trace context", e);
523526
}
524-
reapplyAppContext();
527+
// Skip operationOffset/resourceOffset: setTraceContext just wrote the span-derived values
528+
// there natively. If profiling.context.attributes also names _dd.trace.operation/resource,
529+
// that offset is app-owned too (see isAppOffset); reapplying it here would immediately
530+
// overwrite the fresh span-derived value with a stale app-recorded one.
531+
reapplyAppContext(operationOffset, resourceOffset);
525532
}
526533

527-
public void clearSpanContext() {
534+
/** Per-deactivation clear; reapplies app-managed attributes afterwards (see setTraceContext). */
535+
public void clearTraceContext() {
528536
debugLogging(0L);
529537
try {
530-
profiler.setContext(0L, 0L, 0L, 0L);
538+
profiler.clearTraceContext();
531539
} catch (Throwable e) {
532-
log.debug("Failed to set context", e);
540+
log.debug("Failed to clear trace context", e);
533541
}
534542
reapplyAppContext();
535543
}
536544

545+
public void setSpanContext(long rootSpanId, long spanId, long traceIdHigh, long traceIdLow) {
546+
setTraceContext(rootSpanId, spanId, traceIdHigh, traceIdLow, -1, null, -1, null);
547+
}
548+
549+
public void clearSpanContext() {
550+
clearTraceContext();
551+
}
552+
537553
public boolean setContextValue(int offset, String value) {
538-
if (contextSetter != null && offset >= 0 && value != null) {
554+
if (offset >= 0 && value != null) {
539555
try {
540556
// Native call first; snapshot updated only on success so Java and ddprof state stay in
541557
// sync.
542-
if (contextSetter.setContextValue(offset, value)) {
558+
if (profiler.setContextValue(offset, value)) {
543559
recordAppContextValue(offset, value);
544560
return true;
545561
}
562+
// Rejected (e.g. >255-byte UTF-8, dictionary full): the native call already cleared this
563+
// slot. Restore it from the still-current Java snapshot so a rejected write doesn't blank
564+
// the attribute until the next span boundary.
565+
reapplyAppContext();
546566
} catch (Throwable e) {
547567
log.debug("Failed to set context value", e);
548568
}
@@ -564,14 +584,25 @@ public boolean clearContextValue(String attribute) {
564584
return false;
565585
}
566586

587+
/**
588+
* Clears the app-managed context attribute at {@code offset} on this thread (native slot plus the
589+
* per-thread snapshot). The underlying native {@code clearContextValue} is best-effort and
590+
* returns no status. No caller currently consumes the result; it is kept for symmetry with {@link
591+
* #setContextValue(int, String)} and to signal an unconfigured/failed clear.
592+
*
593+
* @param offset the app-managed context-attribute slot to clear; a negative value (an
594+
* unconfigured attribute) is a no-op
595+
* @return {@code true} if a valid, non-negative {@code offset} was cleared without error; {@code
596+
* false} if {@code offset} is negative or the native clear threw
597+
*/
567598
public boolean clearContextValue(int offset) {
568-
if (contextSetter != null && offset >= 0) {
599+
if (offset >= 0) {
569600
try {
570601
// Native call first; snapshot updated only after it returns so a throw leaves both sides
571602
// consistent.
572-
boolean cleared = contextSetter.clearContextValue(offset);
603+
profiler.clearContextValue(offset);
573604
recordAppContextValue(offset, null);
574-
return cleared;
605+
return true;
575606
} catch (Throwable t) {
576607
log.debug("Failed to clear context value", t);
577608
}
@@ -581,21 +612,27 @@ public boolean clearContextValue(int offset) {
581612

582613
/**
583614
* Re-applies this thread's application-managed context attributes after a span activation or
584-
* deactivation. ddprof's {@code setContext} clears all custom attribute slots; this restores only
585-
* the app-owned ones so they remain visible during the new span's lifetime (or after the last
586-
* span closes). No-op when no application attributes are configured or none have been set on this
587-
* thread.
588-
*
589-
* <p>Fast path (span activation): uses the pre-computed constant IDs and UTF-8 bytes from {@link
590-
* #recordAppContextValue} in a single {@code setContextValuesByIdAndBytes} call — no String
591-
* allocation, no hash lookup.
615+
* deactivation. The native {@code setTraceContext}/{@code clearTraceContext} clears all custom
616+
* attribute slots; this restores the app-owned ones so they remain visible during the new span's
617+
* lifetime — or after the last span closes, since native {@code setContextValue} publishes the
618+
* record (valid=1) even with no active span. No-op when no application attributes are configured
619+
* or none have been set on this thread.
592620
*
593-
* <p>Fallback (span deactivation via {@code setContext(0,0,0,0)}): {@code clearContextDirect}
594-
* calls {@code detach()} but not {@code attach()}, leaving the thread's {@code validOffset=0}.
595-
* {@code setContextValuesByIdAndBytes} returns {@code false} in that state, so we fall back to
596-
* individual {@code setContextValue} calls which go through the proper detach/attach cycle.
621+
* <p>Per-slot native {@code setContextValue} calls (each resolved through the process-wide value
622+
* cache, so no re-encoding on a hit). A single-JNI-call batch is a possible future optimization
623+
* (java-profiler PROF-15361); per-slot is adequate for the typical small app-attribute count.
597624
*/
598625
public void reapplyAppContext() {
626+
reapplyAppContext(-1, -1);
627+
}
628+
629+
/**
630+
* Same as {@link #reapplyAppContext()}, but leaves {@code operationOffset}/{@code resourceOffset}
631+
* alone. Used by {@link #setTraceContext} to avoid clobbering the operation/resource offsets it
632+
* just wrote natively, in the edge case where those offsets are also app-owned (see {@link
633+
* #isAppOffset}). Pass -1 for either argument to skip nothing.
634+
*/
635+
private void reapplyAppContext(int operationOffset, int resourceOffset) {
599636
if (!hasAppContext) {
600637
return;
601638
}
@@ -604,16 +641,15 @@ public void reapplyAppContext() {
604641
return;
605642
}
606643
try {
607-
if (!contextSetter.setContextValuesByIdAndBytes(snapshot.ids(), snapshot.utf8())) {
608-
// validOffset=0 after clearContextDirect (setContext(0,0,0,0) path) — fall back to
609-
// individual writes which go through the proper detach/attach cycle.
610-
int remaining = snapshot.nonZeroCount();
611-
for (int i = 0; i < isAppOffset.length && remaining > 0; i++) {
612-
String s = snapshot.stringAt(i);
613-
if (s != null) {
614-
contextSetter.setContextValue(i, s);
615-
remaining--;
616-
}
644+
int remaining = snapshot.nonZeroCount();
645+
for (int i = 0; i < isAppOffset.length && remaining > 0; i++) {
646+
if (i == operationOffset || i == resourceOffset) {
647+
continue;
648+
}
649+
String s = snapshot.stringAt(i);
650+
if (s != null) {
651+
profiler.setContextValue(i, s);
652+
remaining--;
617653
}
618654
}
619655
} catch (Throwable e) {
@@ -625,7 +661,6 @@ public void reapplyAppContext() {
625661
void clearAppContextSnapshot() {
626662
appContextValues.remove();
627663
scopeStack.remove();
628-
contextScratch.remove();
629664
}
630665

631666
/**
@@ -637,7 +672,7 @@ void clearAppContextSnapshot() {
637672
* the restored Java-side snapshot right away, without waiting for the next span activation.
638673
*/
639674
void syncNativeAppContext() {
640-
if (!hasAppContext || contextSetter == null) {
675+
if (!hasAppContext) {
641676
return;
642677
}
643678
AppContextSnapshot snapshot = appContextValues.get();
@@ -648,9 +683,9 @@ void syncNativeAppContext() {
648683
}
649684
String value = snapshot != null ? snapshot.stringAt(i) : null;
650685
if (value != null) {
651-
contextSetter.setContextValue(i, value);
686+
profiler.setContextValue(i, value);
652687
} else {
653-
contextSetter.clearContextValue(i);
688+
profiler.clearContextValue(i);
654689
}
655690
}
656691
} catch (Throwable e) {
@@ -719,13 +754,10 @@ private void recordAppContextValue(int offset, String value) {
719754
snapshot = new AppContextSnapshot(isAppOffset.length);
720755
appContextValues.set(snapshot);
721756
}
757+
// Reapply resolves the value → encoding via the process-wide cache, so the snapshot only needs
758+
// the String value.
722759
if (!value.equals(snapshot.stringAt(offset))) {
723-
byte[] utf8Bytes = value.getBytes(StandardCharsets.UTF_8);
724-
// ContextSetter has no single-slot readback API; snapshotTags fills all slots at once.
725-
// The scratch array is per-thread and reused across calls, so this is allocation-free.
726-
int[] scratch = contextScratch.get();
727-
contextSetter.snapshotTags(scratch);
728-
snapshot.record(offset, scratch[offset], utf8Bytes, value);
760+
snapshot.record(offset, value);
729761
}
730762
}
731763

@@ -736,10 +768,14 @@ private void debugLogging(long localRootSpanId) {
736768
}
737769

738770
public int[] snapshot() {
739-
if (contextSetter != null) {
740-
return contextSetter.snapshotTags();
741-
}
742-
return EMPTY;
771+
int n = isAppOffset.length;
772+
if (n == 0) {
773+
return EMPTY;
774+
}
775+
// Native read of the current thread's sidecar tag encodings; does not reset the record.
776+
int[] tags = new int[n];
777+
profiler.copyContextTags(tags);
778+
return tags;
743779
}
744780

745781
public void recordSetting(String name, String value) {

0 commit comments

Comments
 (0)