From bc7f44ae9370ae1f214f63b1c17db55f91dfebd0 Mon Sep 17 00:00:00 2001 From: Roman Kennke Date: Thu, 9 Jul 2026 16:40:11 +0200 Subject: [PATCH] profiling(ddprof): migrate context bridge to the all-native API MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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) --- .../profiling/ddprof/DatadogProfiler.java | 174 +++++++++--------- .../ddprof/DatadogProfilingIntegration.java | 24 +-- 2 files changed, 98 insertions(+), 100 deletions(-) diff --git a/dd-java-agent/agent-profiling/profiling-ddprof/src/main/java/com/datadog/profiling/ddprof/DatadogProfiler.java b/dd-java-agent/agent-profiling/profiling-ddprof/src/main/java/com/datadog/profiling/ddprof/DatadogProfiler.java index df1b75789db..231dc455a58 100644 --- a/dd-java-agent/agent-profiling/profiling-ddprof/src/main/java/com/datadog/profiling/ddprof/DatadogProfiler.java +++ b/dd-java-agent/agent-profiling/profiling-ddprof/src/main/java/com/datadog/profiling/ddprof/DatadogProfiler.java @@ -44,7 +44,6 @@ import datadog.trace.bootstrap.instrumentation.api.TaskWrapper; import datadog.trace.util.TempLocationManager; import java.io.IOException; -import java.nio.charset.StandardCharsets; import java.nio.file.Files; import java.nio.file.Path; import java.nio.file.attribute.PosixFilePermissions; @@ -119,22 +118,12 @@ public static DatadogProfiler newInstance(ConfigProvider configProvider) { /** * Per-thread snapshot of application attribute values. Lazily allocated; only threads that call - * setContextValue for an app attribute ever allocate. Holds the ddprof constant ID and - * pre-encoded UTF-8 bytes for each slot, ready for a zero-allocation reapply via - * setContextValuesByIdAndBytes. + * setContextValue for an app attribute ever allocate. Holds the String value for each slot; the + * all-native reapply resolves each value's encoding through the process-wide value cache. */ private final ThreadLocal appContextValues = new ThreadLocal<>(); private final ThreadLocal scopeStack = new ThreadLocal<>(); - // Scratch buffer for snapshotTags in recordAppContextValue; per-thread, sized to context slots. - // Lives here rather than on AppContextSnapshot so save-slots in ScopeStack don't carry it. - private final ThreadLocal contextScratch = - new ThreadLocal() { - @Override - protected int[] initialValue() { - return new int[isAppOffset.length]; - } - }; /** Per-thread stack of pre-allocated save slots for {@link DatadogProfilingScope}. */ static final class ScopeStack { @@ -170,35 +159,28 @@ void release() { // Package-private so DatadogProfilingScope can hold a typed reference for save/restore. static final class AppContextSnapshot { - private final int[] ids; - private final byte[][] utf8; - // Cached string values for change detection — avoids re-encoding and re-snapshotting - // the constant ID when the same value is set again on the same thread. + // App-managed attribute values, indexed by slot. On the all-native path a value is written via + // JavaProfiler.setContextValue (which resolves it through the process-wide value cache), so the + // snapshot only needs the String — no cached constant ID / UTF-8 bytes. private final String[] strings; - // Count of slots with a non-zero constant ID; allows O(1) isEmpty(). + // Count of slots with a non-null value; allows O(1) isEmpty(). private int nonZeroCount; AppContextSnapshot(int size) { - ids = new int[size]; - utf8 = new byte[size][]; strings = new String[size]; } - void record(int offset, int constantId, byte[] utf8Bytes, String value) { - if (ids[offset] == 0 && constantId != 0) { + void record(int offset, String value) { + if (strings[offset] == null && value != null) { nonZeroCount++; - } else if (ids[offset] != 0 && constantId == 0) { + } else if (strings[offset] != null && value == null) { nonZeroCount--; } - ids[offset] = constantId; - utf8[offset] = utf8Bytes; strings[offset] = value; } void clear(int offset) { - if (ids[offset] != 0) nonZeroCount--; - ids[offset] = 0; - utf8[offset] = null; + if (strings[offset] != null) nonZeroCount--; strings[offset] = null; } @@ -214,24 +196,12 @@ String stringAt(int offset) { return strings[offset]; } - int[] ids() { - return ids; - } - - byte[][] utf8() { - return utf8; - } - void copyFrom(AppContextSnapshot src) { - System.arraycopy(src.ids, 0, ids, 0, ids.length); - System.arraycopy(src.utf8, 0, utf8, 0, utf8.length); System.arraycopy(src.strings, 0, strings, 0, strings.length); nonZeroCount = src.nonZeroCount; } void reset() { - Arrays.fill(ids, 0); - Arrays.fill(utf8, null); Arrays.fill(strings, null); nonZeroCount = 0; } @@ -271,7 +241,9 @@ private DatadogProfiler(ConfigProvider configProvider) { this.orderedContextAttributes = getOrderedContextAttributes(contextAttributes, configProvider); this.contextSetter = new ContextSetter(profiler, orderedContextAttributes); // ContextSetter deduplicates and truncates to 10 internally; size arrays to its actual size. - int contextSize = contextSetter.snapshotTags().length; + // size() is a pure-Java count (no DBB read) — kept alongside offsetOf as the only ContextSetter + // methods this bridge still uses; all context writes/reads are all-native. + int contextSize = contextSetter.size(); boolean[] appOffsets = new boolean[contextSize]; boolean anyApp = false; for (String attribute : contextAttributes) { @@ -514,32 +486,64 @@ public int offsetOf(String attribute) { return contextSetter.offsetOf(attribute); } - public void setSpanContext(long rootSpanId, long spanId, long traceIdHigh, long traceIdLow) { + /** + * Combined per-activation write: full trace/span context plus the span-derived operation and + * resource attributes, in a single native call, followed by a reapply of app-managed attributes + * (the native call resets all custom slots). Replaces the previous {@code setContext} + two + * {@code setContextValue} calls. A negative attribute offset (or null value) skips that + * attribute. + */ + public void setTraceContext( + long rootSpanId, + long spanId, + long traceIdHigh, + long traceIdLow, + int operationOffset, + CharSequence operationName, + int resourceOffset, + CharSequence resourceName) { debugLogging(rootSpanId); try { - profiler.setContext(rootSpanId, spanId, traceIdHigh, traceIdLow); + profiler.setTraceContext( + rootSpanId, + spanId, + traceIdHigh, + traceIdLow, + operationOffset, + operationName, + resourceOffset, + resourceName); } catch (Throwable e) { - log.debug("Failed to clear context", e); + log.debug("Failed to set trace context", e); } reapplyAppContext(); } - public void clearSpanContext() { + /** Per-deactivation clear; reapplies app-managed attributes afterwards (see setTraceContext). */ + public void clearTraceContext() { debugLogging(0L); try { - profiler.setContext(0L, 0L, 0L, 0L); + profiler.clearTraceContext(); } catch (Throwable e) { - log.debug("Failed to set context", e); + log.debug("Failed to clear trace context", e); } reapplyAppContext(); } + public void setSpanContext(long rootSpanId, long spanId, long traceIdHigh, long traceIdLow) { + setTraceContext(rootSpanId, spanId, traceIdHigh, traceIdLow, -1, null, -1, null); + } + + public void clearSpanContext() { + clearTraceContext(); + } + public boolean setContextValue(int offset, String value) { - if (contextSetter != null && offset >= 0 && value != null) { + if (offset >= 0 && value != null) { try { // Native call first; snapshot updated only on success so Java and ddprof state stay in // sync. - if (contextSetter.setContextValue(offset, value)) { + if (profiler.setContextValue(offset, value)) { recordAppContextValue(offset, value); return true; } @@ -565,13 +569,13 @@ public boolean clearContextValue(String attribute) { } public boolean clearContextValue(int offset) { - if (contextSetter != null && offset >= 0) { + if (offset >= 0) { try { // Native call first; snapshot updated only after it returns so a throw leaves both sides // consistent. - boolean cleared = contextSetter.clearContextValue(offset); + profiler.clearContextValue(offset); recordAppContextValue(offset, null); - return cleared; + return true; } catch (Throwable t) { log.debug("Failed to clear context value", t); } @@ -581,19 +585,16 @@ public boolean clearContextValue(int offset) { /** * Re-applies this thread's application-managed context attributes after a span activation or - * deactivation. ddprof's {@code setContext} clears all custom attribute slots; this restores only - * the app-owned ones so they remain visible during the new span's lifetime (or after the last - * span closes). No-op when no application attributes are configured or none have been set on this - * thread. + * deactivation. The native {@code setTraceContext}/{@code clearTraceContext} clears all custom + * attribute slots; this restores the app-owned ones so they remain visible during the new span's + * lifetime — or after the last span closes, since native {@code setContextValue} publishes the + * record (valid=1) even with no active span. No-op when no application attributes are configured + * or none have been set on this thread. * - *

Fast path (span activation): uses the pre-computed constant IDs and UTF-8 bytes from {@link - * #recordAppContextValue} in a single {@code setContextValuesByIdAndBytes} call — no String - * allocation, no hash lookup. - * - *

Fallback (span deactivation via {@code setContext(0,0,0,0)}): {@code clearContextDirect} - * calls {@code detach()} but not {@code attach()}, leaving the thread's {@code validOffset=0}. - * {@code setContextValuesByIdAndBytes} returns {@code false} in that state, so we fall back to - * individual {@code setContextValue} calls which go through the proper detach/attach cycle. + *

Per-slot native {@code setContextValue} calls (each resolved through the process-wide value + * cache, so no re-encoding on a hit). A single-JNI-call batch is a possible future optimization + * (see java-profiler PROF-15361); per-slot preserves the prior shape and is adequate for the + * typical small app-attribute count. */ public void reapplyAppContext() { if (!hasAppContext) { @@ -604,16 +605,12 @@ public void reapplyAppContext() { return; } try { - if (!contextSetter.setContextValuesByIdAndBytes(snapshot.ids(), snapshot.utf8())) { - // validOffset=0 after clearContextDirect (setContext(0,0,0,0) path) — fall back to - // individual writes which go through the proper detach/attach cycle. - int remaining = snapshot.nonZeroCount(); - for (int i = 0; i < isAppOffset.length && remaining > 0; i++) { - String s = snapshot.stringAt(i); - if (s != null) { - contextSetter.setContextValue(i, s); - remaining--; - } + int remaining = snapshot.nonZeroCount(); + for (int i = 0; i < isAppOffset.length && remaining > 0; i++) { + String s = snapshot.stringAt(i); + if (s != null) { + profiler.setContextValue(i, s); + remaining--; } } } catch (Throwable e) { @@ -625,7 +622,6 @@ public void reapplyAppContext() { void clearAppContextSnapshot() { appContextValues.remove(); scopeStack.remove(); - contextScratch.remove(); } /** @@ -637,7 +633,7 @@ void clearAppContextSnapshot() { * the restored Java-side snapshot right away, without waiting for the next span activation. */ void syncNativeAppContext() { - if (!hasAppContext || contextSetter == null) { + if (!hasAppContext) { return; } AppContextSnapshot snapshot = appContextValues.get(); @@ -648,9 +644,9 @@ void syncNativeAppContext() { } String value = snapshot != null ? snapshot.stringAt(i) : null; if (value != null) { - contextSetter.setContextValue(i, value); + profiler.setContextValue(i, value); } else { - contextSetter.clearContextValue(i); + profiler.clearContextValue(i); } } } catch (Throwable e) { @@ -719,13 +715,10 @@ private void recordAppContextValue(int offset, String value) { snapshot = new AppContextSnapshot(isAppOffset.length); appContextValues.set(snapshot); } + // The all-native path resolves the value → encoding via the process-wide cache on reapply, so + // the snapshot only needs the String (no per-thread snapshotTags read of the encoding). if (!value.equals(snapshot.stringAt(offset))) { - byte[] utf8Bytes = value.getBytes(StandardCharsets.UTF_8); - // ContextSetter has no single-slot readback API; snapshotTags fills all slots at once. - // The scratch array is per-thread and reused across calls, so this is allocation-free. - int[] scratch = contextScratch.get(); - contextSetter.snapshotTags(scratch); - snapshot.record(offset, scratch[offset], utf8Bytes, value); + snapshot.record(offset, value); } } @@ -736,10 +729,15 @@ private void debugLogging(long localRootSpanId) { } public int[] snapshot() { - if (contextSetter != null) { - return contextSetter.snapshotTags(); - } - return EMPTY; + int n = isAppOffset.length; + if (n == 0) { + return EMPTY; + } + // Native read of the current thread's sidecar tag encodings — no ThreadContext / DBB (which + // would reset the record). Observes encodings written through the all-native path. + int[] tags = new int[n]; + profiler.copyContextTags(tags); + return tags; } public void recordSetting(String name, String value) { diff --git a/dd-java-agent/agent-profiling/profiling-ddprof/src/main/java/com/datadog/profiling/ddprof/DatadogProfilingIntegration.java b/dd-java-agent/agent-profiling/profiling-ddprof/src/main/java/com/datadog/profiling/ddprof/DatadogProfilingIntegration.java index 65462a46bce..45f5f5fb015 100644 --- a/dd-java-agent/agent-profiling/profiling-ddprof/src/main/java/com/datadog/profiling/ddprof/DatadogProfilingIntegration.java +++ b/dd-java-agent/agent-profiling/profiling-ddprof/src/main/java/com/datadog/profiling/ddprof/DatadogProfilingIntegration.java @@ -32,24 +32,26 @@ public class DatadogProfilingIntegration implements ProfilingContextIntegration new Stateful() { @Override public void close() { - DDPROF.clearSpanContext(); - DDPROF.clearContextValue(SPAN_NAME_INDEX); - DDPROF.clearContextValue(RESOURCE_NAME_INDEX); + // clearTraceContext wipes all custom slots (incl. operation/resource) and reapplies + // app-managed context, so no separate clearContextValue calls are needed. + DDPROF.clearTraceContext(); } @Override public void activate(Object context) { if (context instanceof ProfilerContext) { ProfilerContext profilerContext = (ProfilerContext) context; - // setSpanContext() calls reapplyAppContext() internally, so no explicit call is needed. - DDPROF.setSpanContext( + // One native call: trace/span context + operation and resource attributes, then + // reapply of app-managed context (setTraceContext resets custom slots). + DDPROF.setTraceContext( profilerContext.getRootSpanId(), profilerContext.getSpanId(), profilerContext.getTraceIdHigh(), - profilerContext.getTraceIdLow()); - DDPROF.setContextValue(SPAN_NAME_INDEX, profilerContext.getOperationName().toString()); - DDPROF.setContextValue( - RESOURCE_NAME_INDEX, profilerContext.getResourceName().toString()); + profilerContext.getTraceIdLow(), + SPAN_NAME_INDEX, + profilerContext.getOperationName(), + RESOURCE_NAME_INDEX, + profilerContext.getResourceName()); } } }; @@ -79,9 +81,7 @@ public String name() { } public void clearContext() { - DDPROF.clearSpanContext(); - DDPROF.clearContextValue(SPAN_NAME_INDEX); - DDPROF.clearContextValue(RESOURCE_NAME_INDEX); + DDPROF.clearTraceContext(); } @Override