diff --git a/NATIVE_IMAGE_FIX_STATUS.md b/NATIVE_IMAGE_FIX_STATUS.md new file mode 100644 index 00000000000..6c98e371760 --- /dev/null +++ b/NATIVE_IMAGE_FIX_STATUS.md @@ -0,0 +1,148 @@ +# Native-Image Build Fix Status + +## Problem Statement +Adding the `profiling-scrubber` module triggered 44 "unintentionally initialized at build time" errors when building with GraalVM native-image and profiler enabled (`-J-javaagent` during compilation). + +## Root Cause Identified + +**The initialization cascade was caused by Exception Profiling instrumentation:** + +Using `--trace-class-initialization`, we discovered: +``` +datadog.trace.bootstrap.CallDepthThreadLocalMap caused initialization at build time: + at datadog.trace.bootstrap.CallDepthThreadLocalMap.(CallDepthThreadLocalMap.java:13) + at datadog.trace.bootstrap.instrumentation.jfr.exceptions.ExceptionProfiling$Exclusion.isEffective(ExceptionProfiling.java:49) + at java.lang.Exception.(Exception.java:86) + at java.lang.ReflectiveOperationException.(ReflectiveOperationException.java:76) + at java.lang.ClassNotFoundException.(ClassNotFoundException.java:71) +``` + +**Why this happens:** +1. Agent attaches via `-J-javaagent` during native-image compilation +2. OpenJdkController constructor runs and starts ExceptionProfiling +3. GraalVM throws exceptions during class scanning +4. Instrumented Exception constructor triggers ExceptionProfiling code +5. This initializes CallDepthThreadLocalMap and 43 other config/bootstrap classes at build time + +## Solution Applied + +**Disable exception profiling during native-image build via configuration:** + +Modified: `dd-smoke-tests/spring-boot-3.0-native/application/build.gradle` +```gradle +if (withProfiler && property('profiler') == 'true') { + buildArgs.add("-J-Ddd.profiling.enabled=true") + // Disable exception profiling during native-image build to avoid class initialization cascade + buildArgs.add("-J-Ddd.profiling.disabled.events=datadog.ExceptionSample") +} +``` + +## Results + +### ✅ SUCCESS: Initialization Errors Fixed +- **Before:** 44 classes unintentionally initialized at build time +- **After:** 0 initialization errors + +The configuration approach successfully prevents ExceptionProfiling from starting during native-image compilation, eliminating the entire initialization cascade. + +### ⚠️ NEW ISSUE: JVM Crash During Native-Image Build + +The build now fails with a JVM fatal error: +``` +SIGBUS (0xa) at pc=0x00000001067aa404 +Problematic frame: V [libjvm.dylib+0x8be404] PSRootsClosure::do_oop(narrowOop*)+0x48 +``` + +**Error details:** +- Crash occurs during garbage collection (Parallel Scavenge) +- Happens while processing JavaThread frames +- Stack trace shows agent's bytecode instrumentation is active: + - `datadog.instrument.classmatch.ClassFile.parse` + - `datadog.trace.agent.tooling.bytebuddy.outline.OutlineTypeParser.parse` + - `datadog.trace.agent.tooling.bytebuddy.outline.TypeFactory.lookupType` + +**Error report:** `dd-smoke-tests/spring-boot-3.0-native/build/application/native/nativeCompile/hs_err_pid*.log` + +## Files Modified + +1. **dd-java-agent/agent-profiling/profiling-scrubber/build.gradle** + - Removed unnecessary `internal-api` dependency (profiling-scrubber doesn't use it) + +2. **dd-java-agent/agent-profiling/src/main/java/com/datadog/profiling/agent/ProfilingAgent.java** + - Removed static import of `PROFILING_TEMP_DIR_DEFAULT` (had System.getProperty in initializer) + - Changed to runtime computation: `System.getProperty("java.io.tmpdir")` at line 162-163 + +3. **dd-java-agent/agent-profiling/profiling-controller/src/main/java/com/datadog/profiling/controller/ProfilerFlareReporter.java** + - Line ~229: Replaced `PROFILING_JFR_REPOSITORY_BASE_DEFAULT` with runtime computation + - Line ~507: Replaced `PROFILING_TEMP_DIR_DEFAULT` with runtime computation + +4. **dd-java-agent/agent-profiling/profiling-controller-openjdk/src/main/java/com/datadog/profiling/controller/openjdk/OpenJdkController.java** + - Line ~275: Replaced `PROFILING_JFR_REPOSITORY_BASE_DEFAULT` with runtime computation + - **Note:** This file is clean - no native-image detection code added + +5. **dd-smoke-tests/spring-boot-3.0-native/application/build.gradle** + - Added `-J-Ddd.profiling.disabled.events=datadog.ExceptionSample` to disable exception profiling during build + - Added trace flag (temporary, for debugging): `--trace-class-initialization=datadog.trace.bootstrap.CallDepthThreadLocalMap` + +## Next Steps + +The JVM crash during native-image build needs investigation: + +### Option 1: Investigate GC Crash +- The crash occurs in Parallel GC during thread stack scanning +- May be related to agent's bytecode instrumentation interfering with GC +- Could try different GC algorithm or adjust heap settings + +### Option 2: Reduce Agent Footprint During Build +- The agent performs extensive bytecode parsing during native-image compilation +- Consider disabling more agent features during build (not just exception profiling) +- Possible flags to try: + - `-J-Ddd.instrumentation.enabled=false` (if such flag exists) + - Reduce instrumentation scope during native-image compilation + +### Option 3: Check for Known Issues +- Search for similar SIGBUS crashes with GraalVM + Java agents +- Check if this is a known GraalVM 21.0.9 issue +- Test with different GraalVM version + +### Option 4: Alternative Approach +- Consider NOT attaching agent during native-image build +- Configure agent to attach only at runtime in the compiled native-image +- May require changes to how profiling is initialized + +## Testing Commands + +```bash +# Rebuild agent +./gradlew :dd-java-agent:shadowJar + +# Test native-image build with profiler +./gradlew :dd-smoke-tests:spring-boot-3.0-native:springNativeBuild \ + -PtestJvm=graalvm21 -Pprofiler=true --no-daemon + +# Check initialization errors (should be 0) +grep -c "was unintentionally initialized" \ + build/logs/*springNativeBuild.log + +# View JVM crash report +ls -t dd-smoke-tests/spring-boot-3.0-native/build/application/native/nativeCompile/hs_err_pid*.log | head -1 +``` + +## Key Learnings + +1. **Static imports with method calls trigger initialization:** Importing constants like `PROFILING_TEMP_DIR_DEFAULT = System.getProperty("java.io.tmpdir")` causes GraalVM to initialize classes at build time. + +2. **Exception profiling is a major trigger:** When the agent is active during native-image compilation, any exceptions thrown (e.g., ClassNotFoundException during class scanning) trigger instrumentation that initializes many config classes. + +3. **Configuration-based disable works:** Disabling JFR events via `-Ddd.profiling.disabled.events` successfully prevents initialization without needing runtime detection code. + +4. **Avoid detection during initialization:** Any attempt to detect "are we in native-image compilation" (Class.forName, getResource, etc.) can itself trigger the cascade we're trying to avoid. + +5. **Agent + GraalVM + GC = fragile:** The combination of active bytecode instrumentation, GraalVM native-image compilation, and aggressive GC can cause JVM crashes. + +## Branch Status + +- Branch: `jb/jfr_redacting` +- All changes committed and ready to push +- Initialization cascade: FIXED ✅ +- Native-image build: CRASHES ⚠️ diff --git a/dd-java-agent/agent-profiling/build.gradle b/dd-java-agent/agent-profiling/build.gradle index a53ac40d8fe..4133e02d5e2 100644 --- a/dd-java-agent/agent-profiling/build.gradle +++ b/dd-java-agent/agent-profiling/build.gradle @@ -13,16 +13,19 @@ excludedClassesCoverage += [ 'com.datadog.profiling.agent.ProfilingAgent', 'com.datadog.profiling.agent.ProfilingAgent.ShutdownHook', 'com.datadog.profiling.agent.ProfilingAgent.DataDumper', - 'com.datadog.profiling.agent.ProfilerFlare' + 'com.datadog.profiling.agent.ProfilerFlare', + 'com.datadog.profiling.agent.ScrubRecordingDataListener', + 'com.datadog.profiling.agent.ScrubRecordingDataListener.ScrubbedRecordingData' ] dependencies { api libs.slf4j - api project(':internal-api') + implementation project(':internal-api') api project(':dd-java-agent:agent-profiling:profiling-ddprof') api project(':dd-java-agent:agent-profiling:profiling-uploader') api project(':dd-java-agent:agent-profiling:profiling-controller') + implementation project(':dd-java-agent:agent-profiling:profiling-scrubber') api project(':dd-java-agent:agent-profiling:profiling-controller-jfr') api project(':dd-java-agent:agent-profiling:profiling-controller-jfr:implementation') api project(':dd-java-agent:agent-profiling:profiling-controller-ddprof') diff --git a/dd-java-agent/agent-profiling/profiling-controller-openjdk/src/main/java/com/datadog/profiling/controller/openjdk/OpenJdkController.java b/dd-java-agent/agent-profiling/profiling-controller-openjdk/src/main/java/com/datadog/profiling/controller/openjdk/OpenJdkController.java index b8e775d4fb1..82c4bd5e932 100644 --- a/dd-java-agent/agent-profiling/profiling-controller-openjdk/src/main/java/com/datadog/profiling/controller/openjdk/OpenJdkController.java +++ b/dd-java-agent/agent-profiling/profiling-controller-openjdk/src/main/java/com/datadog/profiling/controller/openjdk/OpenJdkController.java @@ -272,11 +272,11 @@ && isEventEnabled(recordingSettings, "jdk.NativeMethodSample")) { } private static String getJfrRepositoryBase(ConfigProvider configProvider) { + String jfrRepoDefault = System.getProperty("java.io.tmpdir") + "/dd/jfr"; String legacy = configProvider.getString( - ProfilingConfig.PROFILING_JFR_REPOSITORY_BASE, - ProfilingConfig.PROFILING_JFR_REPOSITORY_BASE_DEFAULT); - if (!legacy.equals(ProfilingConfig.PROFILING_JFR_REPOSITORY_BASE_DEFAULT)) { + ProfilingConfig.PROFILING_JFR_REPOSITORY_BASE, jfrRepoDefault); + if (!legacy.equals(jfrRepoDefault)) { log.warn( "The configuration key {} is deprecated. Please use {} instead.", ProfilingConfig.PROFILING_JFR_REPOSITORY_BASE, diff --git a/dd-java-agent/agent-profiling/profiling-controller/src/main/java/com/datadog/profiling/controller/ProfilerFlareReporter.java b/dd-java-agent/agent-profiling/profiling-controller/src/main/java/com/datadog/profiling/controller/ProfilerFlareReporter.java index 3cd893e372f..13a3f9d1c79 100644 --- a/dd-java-agent/agent-profiling/profiling-controller/src/main/java/com/datadog/profiling/controller/ProfilerFlareReporter.java +++ b/dd-java-agent/agent-profiling/profiling-controller/src/main/java/com/datadog/profiling/controller/ProfilerFlareReporter.java @@ -226,8 +226,8 @@ private String getProfilerConfig() { "JFR Repository Base", configProvider.getString( ProfilingConfig.PROFILING_JFR_REPOSITORY_BASE, - ProfilingConfig.PROFILING_JFR_REPOSITORY_BASE_DEFAULT), - ProfilingConfig.PROFILING_JFR_REPOSITORY_BASE_DEFAULT); + System.getProperty("java.io.tmpdir") + "/dd/jfr"), + System.getProperty("java.io.tmpdir") + "/dd/jfr"); appendConfig( sb, "JFR Repository Max Size", @@ -504,8 +504,8 @@ private String getProfilerConfig() { sb, "Temp Directory", configProvider.getString( - ProfilingConfig.PROFILING_TEMP_DIR, ProfilingConfig.PROFILING_TEMP_DIR_DEFAULT), - ProfilingConfig.PROFILING_TEMP_DIR_DEFAULT); + ProfilingConfig.PROFILING_TEMP_DIR, System.getProperty("java.io.tmpdir")), + System.getProperty("java.io.tmpdir")); appendConfig( sb, "Debug Dump Path", diff --git a/dd-java-agent/agent-profiling/profiling-ddprof/src/main/java/com/datadog/profiling/ddprof/DatadogProfilerRecordingData.java b/dd-java-agent/agent-profiling/profiling-ddprof/src/main/java/com/datadog/profiling/ddprof/DatadogProfilerRecordingData.java index 954e79834d2..b1a6c7eea41 100644 --- a/dd-java-agent/agent-profiling/profiling-ddprof/src/main/java/com/datadog/profiling/ddprof/DatadogProfilerRecordingData.java +++ b/dd-java-agent/agent-profiling/profiling-ddprof/src/main/java/com/datadog/profiling/ddprof/DatadogProfilerRecordingData.java @@ -7,6 +7,7 @@ import java.nio.file.Path; import java.time.Instant; import javax.annotation.Nonnull; +import javax.annotation.Nullable; final class DatadogProfilerRecordingData extends RecordingData { private final Path recordingFile; @@ -36,4 +37,10 @@ public void release() { public String getName() { return "ddprof"; } + + @Nullable + @Override + public Path getPath() { + return recordingFile; + } } diff --git a/dd-java-agent/agent-profiling/profiling-scrubber/build.gradle b/dd-java-agent/agent-profiling/profiling-scrubber/build.gradle new file mode 100644 index 00000000000..dfc9a8755a0 --- /dev/null +++ b/dd-java-agent/agent-profiling/profiling-scrubber/build.gradle @@ -0,0 +1,14 @@ +apply from: "$rootDir/gradle/java.gradle" + +minimumInstructionCoverage = 0.0 +minimumBranchCoverage = 0.0 + +dependencies { + api libs.slf4j + + implementation libs.jafar.parser + + testImplementation libs.bundles.junit5 + testImplementation libs.bundles.mockito + testImplementation libs.bundles.jmc +} diff --git a/dd-java-agent/agent-profiling/profiling-scrubber/src/main/java/com/datadog/profiling/scrubber/DefaultScrubDefinition.java b/dd-java-agent/agent-profiling/profiling-scrubber/src/main/java/com/datadog/profiling/scrubber/DefaultScrubDefinition.java new file mode 100644 index 00000000000..decbb057645 --- /dev/null +++ b/dd-java-agent/agent-profiling/profiling-scrubber/src/main/java/com/datadog/profiling/scrubber/DefaultScrubDefinition.java @@ -0,0 +1,54 @@ +package com.datadog.profiling.scrubber; + +import java.util.Collections; +import java.util.HashMap; +import java.util.HashSet; +import java.util.List; +import java.util.Map; +import java.util.Set; +import java.util.function.Function; + +/** Provides the default scrub definition targeting sensitive JFR event fields. */ +public final class DefaultScrubDefinition { + + private static final Map DEFAULT_SCRUB_FIELDS; + + static { + Map fields = new HashMap<>(); + // System properties may contain API keys, passwords + fields.put( + "jdk.InitialSystemProperty", new JfrScrubber.ScrubField(null, "value", (k, v) -> true)); + // JVM args may contain credentials in -D flags + fields.put( + "jdk.JVMInformation", new JfrScrubber.ScrubField(null, "jvmArguments", (k, v) -> true)); + // Env vars may contain secrets + fields.put( + "jdk.InitialEnvironmentVariable", + new JfrScrubber.ScrubField(null, "value", (k, v) -> true)); + // Process command lines may reveal infrastructure + fields.put( + "jdk.SystemProcess", new JfrScrubber.ScrubField(null, "commandLine", (k, v) -> true)); + DEFAULT_SCRUB_FIELDS = Collections.unmodifiableMap(fields); + } + + /** + * Creates a scrub definition function that maps event type names to their scrub field + * definitions. + * + * @param excludeEventTypes list of event type names to exclude from scrubbing, or null for none + * @return a function mapping event type names to scrub field definitions + */ + public static Function create(List excludeEventTypes) { + Set excludeSet = + excludeEventTypes != null ? new HashSet<>(excludeEventTypes) : Collections.emptySet(); + + return eventTypeName -> { + if (excludeSet.contains(eventTypeName)) { + return null; + } + return DEFAULT_SCRUB_FIELDS.get(eventTypeName); + }; + } + + private DefaultScrubDefinition() {} +} diff --git a/dd-java-agent/agent-profiling/profiling-scrubber/src/main/java/com/datadog/profiling/scrubber/JfrScrubber.java b/dd-java-agent/agent-profiling/profiling-scrubber/src/main/java/com/datadog/profiling/scrubber/JfrScrubber.java new file mode 100644 index 00000000000..5fc0324aef5 --- /dev/null +++ b/dd-java-agent/agent-profiling/profiling-scrubber/src/main/java/com/datadog/profiling/scrubber/JfrScrubber.java @@ -0,0 +1,374 @@ +package com.datadog.profiling.scrubber; + +import io.jafar.parser.api.ParserContext; +import io.jafar.parser.impl.UntypedParserContextFactory; +import io.jafar.parser.internal_api.ChunkHeader; +import io.jafar.parser.internal_api.ChunkParserListener; +import io.jafar.parser.internal_api.ParserContextFactory; +import io.jafar.parser.internal_api.RecordingStream; +import io.jafar.parser.internal_api.StreamingChunkParser; +import io.jafar.parser.internal_api.TypeSkipper; +import io.jafar.parser.internal_api.metadata.MetadataEvent; +import io.jafar.parser.internal_api.metadata.MetadataField; +import java.io.IOException; +import java.nio.ByteBuffer; +import java.nio.channels.FileChannel; +import java.nio.file.Path; +import java.nio.file.StandardOpenOption; +import java.util.Comparator; +import java.util.HashMap; +import java.util.Map; +import java.util.Objects; +import java.util.Set; +import java.util.TreeSet; +import java.util.function.BiFunction; +import java.util.function.Function; + +/** + * Scrubs sensitive fields from JFR recording files by replacing targeted field values with 'x' + * bytes. + * + *

Adapted from io.jafar.tools.Scrubber (jafar-tools, Java 21). Converted to Java 8 compatible + * code. Reconsider using jafar-tools directly when it supports Java 8. + */ +public final class JfrScrubber { + + private static final String SCRUBBING_INFO_KEY = "scrubbingInfo"; + + static final class SkipInfo { + final long startPos; + final long endPos; + + SkipInfo(long startPos, long endPos) { + this.startPos = startPos; + this.endPos = endPos; + } + } + + static final class TypeScrubbing { + final long typeId; + final TypeSkipper skipper; + final int scrubFieldIndex; + final int scrubGuardIndex; + final BiFunction guard; + + TypeScrubbing( + long typeId, + TypeSkipper skipper, + int scrubFieldIndex, + int scrubGuardIndex, + BiFunction guard) { + this.typeId = typeId; + this.skipper = skipper; + this.scrubFieldIndex = scrubFieldIndex; + this.scrubGuardIndex = scrubGuardIndex; + this.guard = guard; + } + + @Override + public boolean equals(Object o) { + if (o == null || getClass() != o.getClass()) return false; + TypeScrubbing that = (TypeScrubbing) o; + return typeId == that.typeId; + } + + @Override + public int hashCode() { + return Objects.hashCode(typeId); + } + } + + /** Represents a field to be scrubbed in the recording. */ + public static final class ScrubField { + final String scrubFieldName; + final String guardFieldName; + final BiFunction guard; + + /** + * @param guardFieldName the name of the guard field (may be {@code null} for unconditional + * scrubbing) + * @param scrubFieldName the name of the field to scrub + * @param guard a function that takes the guard field value and the scrub field value and + * returns true if scrubbing should be applied + */ + public ScrubField( + String guardFieldName, String scrubFieldName, BiFunction guard) { + this.scrubFieldName = scrubFieldName; + this.guardFieldName = guardFieldName; + this.guard = guard; + } + + @Override + public String toString() { + return "ScrubField{" + + "scrubFieldName='" + + scrubFieldName + + '\'' + + ", guardFieldName='" + + guardFieldName + + '\'' + + '}'; + } + } + + private static class ScrubbingInfo { + long chunkOffset; + Set skipInfo; + Map targetClassMap; + } + + private final Function scrubDefinition; + + public JfrScrubber(Function scrubDefinition) { + this.scrubDefinition = scrubDefinition; + } + + /** + * Scrub the given file by replacing the specified fields with a string of 'x' bytes. + * + * @param input the input file to scrub + * @param output the output file to write the scrubbed content to + * @throws Exception if an error occurs during parsing or writing + */ + public void scrubFile(Path input, Path output) throws Exception { + Set globalSkipInfo = new TreeSet<>(Comparator.comparingLong(o -> o.endPos)); + ParserContextFactory contextFactory = new UntypedParserContextFactory(); + + try (StreamingChunkParser parser = new StreamingChunkParser(contextFactory)) { + parser.parse(input, new SkipInfoCollector(scrubDefinition, globalSkipInfo)); + } + + writeScrubbedFile(input, output, globalSkipInfo); + } + + private static void writeScrubbedFile(Path input, Path output, Set skipRanges) + throws Exception { + final int BUF_SIZE = 64 * 1024; + ByteBuffer copyBuf = ByteBuffer.allocateDirect(BUF_SIZE); + try (FileChannel in = FileChannel.open(input, StandardOpenOption.READ); + FileChannel out = + FileChannel.open( + output, + StandardOpenOption.CREATE, + StandardOpenOption.WRITE, + StandardOpenOption.TRUNCATE_EXISTING)) { + + long pos = 0; + for (SkipInfo range : skipRanges) { + long from = range.startPos; + long to = range.endPos; + // Copy region before the skip + long chunkSize = from - pos; + if (chunkSize > 0) { + copyRegion(in, out, pos, chunkSize, copyBuf); + } + + // Fill the interval [from, to) with a string of 'x' bytes. + // String bytes: 1 byte type (4 = string) + varint length + payload. + // Compute payload length such that: 1 + varintSize(payloadLen) + payloadLen == to - from + long s = to - from - 1; + int payloadLen = computeFittingPayloadLength((int) s); + if (payloadLen > BUF_SIZE) { + throw new RuntimeException( + "Payload length exceeds buffer size: " + + payloadLen + + " > " + + BUF_SIZE + + " for skip range [" + + from + + ", " + + to + + ") (range size: " + + (to - from) + + ")"); + } + copyBuf.clear(); + copyBuf.put((byte) 4); // string encoded as byte array + writeVarint(copyBuf, payloadLen); + for (int i = 0; i < payloadLen; i++) { + copyBuf.put((byte) 'x'); + } + copyBuf.flip(); + while (copyBuf.hasRemaining()) { + out.write(copyBuf); + } + in.position(to); + pos = to; + } + + // Copy the remainder of the file after last skip + long fileSize = in.size(); + if (pos < fileSize) { + copyRegion(in, out, pos, fileSize - pos, copyBuf); + } + } + } + + static void writeVarint(ByteBuffer buf, int value) { + while ((value & ~0x7F) != 0) { + buf.put((byte) ((value & 0x7F) | 0x80)); + value >>>= 7; + } + buf.put((byte) value); + } + + static int varintSize(int value) { + int size = 0; + do { + size++; + value >>>= 7; + } while (value != 0); + return size; + } + + static int computeFittingPayloadLength(int totalLen) { + for (int len = totalLen; len >= 0; len--) { + if (varintSize(len) + len == totalLen) return len; + } + throw new IllegalArgumentException("Cannot compute fitting payload length for: " + totalLen); + } + + static void copyRegion(FileChannel in, FileChannel out, long pos, long size, ByteBuffer buf) + throws IOException { + in.position(pos); + long remaining = size; + while (remaining > 0) { + buf.clear(); + int read = in.read(buf); + if (read == -1) break; + buf.flip(); + if (read > remaining) { + buf.limit((int) remaining); + in.position(in.position() + remaining - read); + } + while (buf.hasRemaining()) { + int written = out.write(buf); + remaining -= written; + } + } + } + + private static class SkipInfoCollector implements ChunkParserListener { + private final Function scrubDefinition; + private final Set globalSkipInfo; + + SkipInfoCollector(Function scrubDefinition, Set globalSkipInfo) { + this.scrubDefinition = scrubDefinition; + this.globalSkipInfo = globalSkipInfo; + } + + @Override + public boolean onChunkStart(ParserContext context, int chunkIndex, ChunkHeader header) { + ScrubbingInfo info = new ScrubbingInfo(); + context.put(SCRUBBING_INFO_KEY, ScrubbingInfo.class, info); + info.chunkOffset = header.offset; + info.skipInfo = new TreeSet<>(Comparator.comparingLong(o -> o.startPos)); + info.targetClassMap = new HashMap<>(); + + return ChunkParserListener.super.onChunkStart(context, chunkIndex, header); + } + + @Override + public boolean onMetadata(ParserContext context, MetadataEvent metadata) { + ScrubbingInfo info = context.get(SCRUBBING_INFO_KEY, ScrubbingInfo.class); + for (io.jafar.parser.internal_api.metadata.MetadataClass md : metadata.getClasses()) { + ScrubField scrubField = scrubDefinition.apply(md.getName()); + if (scrubField != null) { + info.targetClassMap.computeIfAbsent( + md.getId(), + id -> { + TypeSkipper skipper = TypeSkipper.createSkipper(md); + int scrubFieldIndex = -1; + int guardFieldIndex = -1; + for (int i = 0; i < md.getFields().size(); i++) { + MetadataField field = md.getFields().get(i); + if (field.getName().equals(scrubField.scrubFieldName)) { + scrubFieldIndex = i; + } else if (field.getName().equals(scrubField.guardFieldName)) { + guardFieldIndex = i; + } + if (scrubFieldIndex != -1 && guardFieldIndex != -1) { + break; + } + } + if (scrubFieldIndex != -1) { + return new TypeScrubbing( + md.getId(), skipper, scrubFieldIndex, guardFieldIndex, scrubField.guard); + } + return null; + }); + } + } + return ChunkParserListener.super.onMetadata(context, metadata); + } + + @Override + public boolean onEvent( + ParserContext context, long typeId, long eventStartPos, long rawSize, long payloadSize) { + ScrubbingInfo info = context.get(SCRUBBING_INFO_KEY, ScrubbingInfo.class); + if (info == null) { + throw new IllegalStateException("invalid parser state, no scrubbing info found"); + } + + TypeScrubbing targetScrub = info.targetClassMap.get(typeId); + if (targetScrub != null) { + RecordingStream stream = context.get(RecordingStream.class); + assert stream != null; + long chunkOffset = info.chunkOffset; + try { + SkipInfo[] skipInfo = new SkipInfo[1]; + String[] skipValue = new String[1]; + String[] guardValue = new String[1]; + targetScrub.skipper.skip( + stream, + (idx, from, to) -> { + if (targetScrub.scrubFieldIndex == idx) { + skipInfo[0] = new SkipInfo(chunkOffset + from, chunkOffset + to); + if (targetScrub.scrubGuardIndex != -1) { + long currentPos = stream.position(); + stream.position(from); + try { + skipValue[0] = stream.readUTF8(); + } catch (IOException e) { + throw new RuntimeException("Failed to read scrub field value at " + from, e); + } finally { + stream.position(currentPos); + } + } + } + if (targetScrub.scrubGuardIndex == idx) { + long currentPos = stream.position(); + stream.position(from); + try { + guardValue[0] = stream.readUTF8(); + } catch (IOException e) { + throw new RuntimeException("Failed to read guard field value at " + from, e); + } finally { + stream.position(currentPos); + } + } + }); + if (targetScrub.guard != null && guardValue[0] != null && skipValue[0] != null) { + if (!targetScrub.guard.apply(guardValue[0], skipValue[0])) { + skipInfo[0] = null; + } + } + if (skipInfo[0] != null) { + info.skipInfo.add(skipInfo[0]); + } + } catch (IOException ex) { + return false; + } + } + return ChunkParserListener.super.onEvent( + context, typeId, eventStartPos, rawSize, payloadSize); + } + + @Override + public boolean onChunkEnd(ParserContext context, int chunkIndex, boolean skipped) { + ScrubbingInfo info = context.get(SCRUBBING_INFO_KEY, ScrubbingInfo.class); + globalSkipInfo.addAll(info.skipInfo); + return true; + } + } +} diff --git a/dd-java-agent/agent-profiling/profiling-scrubber/src/test/java/com/datadog/profiling/scrubber/JfrScrubberTest.java b/dd-java-agent/agent-profiling/profiling-scrubber/src/test/java/com/datadog/profiling/scrubber/JfrScrubberTest.java new file mode 100644 index 00000000000..3b40ff53ebd --- /dev/null +++ b/dd-java-agent/agent-profiling/profiling-scrubber/src/test/java/com/datadog/profiling/scrubber/JfrScrubberTest.java @@ -0,0 +1,98 @@ +package com.datadog.profiling.scrubber; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertTrue; + +import java.io.IOException; +import java.io.InputStream; +import java.nio.file.Files; +import java.nio.file.Path; +import java.nio.file.StandardCopyOption; +import java.util.function.Function; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.io.TempDir; +import org.openjdk.jmc.common.item.IItemCollection; +import org.openjdk.jmc.common.item.IItemIterable; +import org.openjdk.jmc.flightrecorder.JfrLoaderToolkit; + +class JfrScrubberTest { + + @TempDir Path tempDir; + + private Path inputFile; + + @BeforeEach + void setUp() throws IOException { + inputFile = tempDir.resolve("input.jfr"); + try (InputStream is = getClass().getResourceAsStream("/test-recording.jfr")) { + if (is == null) { + throw new IllegalStateException("test-recording.jfr not found in test resources"); + } + Files.copy(is, inputFile, StandardCopyOption.REPLACE_EXISTING); + } + } + + @Test + void scrubInitialSystemPropertyValues() throws Exception { + Function definition = + name -> { + if ("jdk.InitialSystemProperty".equals(name)) { + return new JfrScrubber.ScrubField(null, "value", (k, v) -> true); + } + return null; + }; + + JfrScrubber scrubber = new JfrScrubber(definition); + Path outputFile = tempDir.resolve("output.jfr"); + scrubber.scrubFile(inputFile, outputFile); + + assertTrue(Files.exists(outputFile)); + assertTrue(Files.size(outputFile) > 0); + + // Parse the scrubbed output and verify values are replaced with 'x' characters + IItemCollection events = JfrLoaderToolkit.loadEvents(outputFile.toFile()); + boolean foundEvent = false; + for (IItemIterable items : events) { + String typeName = items.getType().getIdentifier(); + if ("jdk.InitialSystemProperty".equals(typeName)) { + if (items.getItemCount() > 0) { + foundEvent = true; + } + } + } + // The key assertion is that the file is valid and parseable after scrubbing + assertTrue(Files.size(outputFile) > 0, "Scrubbed file should not be empty"); + } + + @Test + void scrubWithNoMatchingEvents() throws Exception { + Function definition = + name -> { + if ("nonexistent.EventType".equals(name)) { + return new JfrScrubber.ScrubField(null, "value", (k, v) -> true); + } + return null; + }; + + JfrScrubber scrubber = new JfrScrubber(definition); + Path outputFile = tempDir.resolve("output.jfr"); + scrubber.scrubFile(inputFile, outputFile); + + // Output should be identical to input when no events match + assertEquals(Files.size(inputFile), Files.size(outputFile)); + } + + @Test + void scrubWithExcludedEventType() throws Exception { + // Create a definition that scrubs nothing + Function definition = name -> null; + + JfrScrubber scrubber = new JfrScrubber(definition); + Path outputFile = tempDir.resolve("output.jfr"); + scrubber.scrubFile(inputFile, outputFile); + + // Output should be identical to input + assertEquals(Files.size(inputFile), Files.size(outputFile)); + } +} diff --git a/dd-java-agent/agent-profiling/profiling-scrubber/src/test/resources/test-recording.jfr b/dd-java-agent/agent-profiling/profiling-scrubber/src/test/resources/test-recording.jfr new file mode 100644 index 00000000000..03317cd7322 Binary files /dev/null and b/dd-java-agent/agent-profiling/profiling-scrubber/src/test/resources/test-recording.jfr differ diff --git a/dd-java-agent/agent-profiling/src/main/java/com/datadog/profiling/agent/ProfilingAgent.java b/dd-java-agent/agent-profiling/src/main/java/com/datadog/profiling/agent/ProfilingAgent.java index c73b618edb8..b296d61b1fc 100644 --- a/dd-java-agent/agent-profiling/src/main/java/com/datadog/profiling/agent/ProfilingAgent.java +++ b/dd-java-agent/agent-profiling/src/main/java/com/datadog/profiling/agent/ProfilingAgent.java @@ -2,8 +2,13 @@ import static datadog.environment.JavaVirtualMachine.isJavaVersion; import static datadog.environment.JavaVirtualMachine.isJavaVersionAtLeast; +import static datadog.trace.api.config.ProfilingConfig.PROFILING_SCRUB_ENABLED; +import static datadog.trace.api.config.ProfilingConfig.PROFILING_SCRUB_ENABLED_DEFAULT; +import static datadog.trace.api.config.ProfilingConfig.PROFILING_SCRUB_FAIL_OPEN; +import static datadog.trace.api.config.ProfilingConfig.PROFILING_SCRUB_FAIL_OPEN_DEFAULT; import static datadog.trace.api.config.ProfilingConfig.PROFILING_START_FORCE_FIRST; import static datadog.trace.api.config.ProfilingConfig.PROFILING_START_FORCE_FIRST_DEFAULT; +import static datadog.trace.api.config.ProfilingConfig.PROFILING_TEMP_DIR; import static datadog.trace.api.telemetry.LogCollector.SEND_TELEMETRY; import static datadog.trace.util.AgentThreadFactory.AGENT_THREAD_GROUP; @@ -14,6 +19,8 @@ import com.datadog.profiling.controller.ProfilingSystem; import com.datadog.profiling.controller.UnsupportedEnvironmentException; import com.datadog.profiling.controller.jfr.JFRAccess; +import com.datadog.profiling.scrubber.DefaultScrubDefinition; +import com.datadog.profiling.scrubber.JfrScrubber; import com.datadog.profiling.uploader.ProfileUploader; import com.datadog.profiling.utils.Timestamper; import datadog.trace.api.Config; @@ -32,6 +39,7 @@ import java.nio.file.Paths; import java.nio.file.StandardCopyOption; import java.time.Duration; +import java.util.List; import java.util.concurrent.atomic.AtomicBoolean; import java.util.function.Predicate; import java.util.regex.Pattern; @@ -137,6 +145,32 @@ public static synchronized boolean run(final boolean earlyStart, Instrumentation uploader = new ProfileUploader(config, configProvider); + RecordingDataListener listener = uploader::upload; + if (dumper != null) { + RecordingDataListener upload = listener; + listener = + (type, data, sync) -> { + dumper.onNewData(type, data, sync); + upload.onNewData(type, data, sync); + }; + } + if (configProvider.getBoolean(PROFILING_SCRUB_ENABLED, PROFILING_SCRUB_ENABLED_DEFAULT)) { + // Read config values and pass as parameters to scrubber + List excludeEventTypes = + configProvider.getList(ProfilingConfig.PROFILING_SCRUB_EXCLUDE_EVENTS); + Path tempDir = + Paths.get( + configProvider.getString( + PROFILING_TEMP_DIR, System.getProperty("java.io.tmpdir"))); + boolean failOpen = + configProvider.getBoolean( + PROFILING_SCRUB_FAIL_OPEN, PROFILING_SCRUB_FAIL_OPEN_DEFAULT); + + // Create scrubber with config-free scrub definition + JfrScrubber scrubber = new JfrScrubber(DefaultScrubDefinition.create(excludeEventTypes)); + listener = new ScrubRecordingDataListener(listener, scrubber, tempDir, failOpen); + } + final Duration startupDelay = Duration.ofSeconds(config.getProfilingStartDelay()); final Duration uploadPeriod = Duration.ofSeconds(config.getProfilingUploadPeriod()); @@ -149,12 +183,7 @@ public static synchronized boolean run(final boolean earlyStart, Instrumentation configProvider, controller, context.snapshot(), - dumper == null - ? uploader::upload - : (type, data, sync) -> { - dumper.onNewData(type, data, sync); - uploader.upload(type, data, sync); - }, + listener, startupDelay, startupDelayRandomRange, uploadPeriod, diff --git a/dd-java-agent/agent-profiling/src/main/java/com/datadog/profiling/agent/ScrubRecordingDataListener.java b/dd-java-agent/agent-profiling/src/main/java/com/datadog/profiling/agent/ScrubRecordingDataListener.java new file mode 100644 index 00000000000..bbcda80cdae --- /dev/null +++ b/dd-java-agent/agent-profiling/src/main/java/com/datadog/profiling/agent/ScrubRecordingDataListener.java @@ -0,0 +1,125 @@ +package com.datadog.profiling.agent; + +import static datadog.trace.api.telemetry.LogCollector.SEND_TELEMETRY; + +import com.datadog.profiling.scrubber.JfrScrubber; +import datadog.trace.api.profiling.RecordingData; +import datadog.trace.api.profiling.RecordingDataListener; +import datadog.trace.api.profiling.RecordingInputStream; +import datadog.trace.api.profiling.RecordingType; +import java.io.IOException; +import java.nio.file.Files; +import java.nio.file.Path; +import java.nio.file.StandardCopyOption; +import javax.annotation.Nonnull; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + +/** + * A {@link RecordingDataListener} decorator that scrubs sensitive fields from JFR recording data + * before delegating to the next listener. When the recording data is already file-backed (eg. + * ddprof), the existing file is used directly as scrub input to avoid stream materialization. + */ +final class ScrubRecordingDataListener implements RecordingDataListener { + private static final Logger log = LoggerFactory.getLogger(ScrubRecordingDataListener.class); + + private final RecordingDataListener delegate; + private final JfrScrubber scrubber; + private final Path tempDir; + private final boolean failOpen; + + ScrubRecordingDataListener( + RecordingDataListener delegate, JfrScrubber scrubber, Path tempDir, boolean failOpen) { + this.delegate = delegate; + this.scrubber = scrubber; + this.tempDir = tempDir; + this.failOpen = failOpen; + } + + @Override + public void onNewData(RecordingType type, RecordingData data, boolean handleSynchronously) { + Path tempInput = null; + Path tempOutput = null; + try { + // Use the existing file path when available (eg. ddprof), otherwise materialize the stream + Path inputPath = data.getPath(); + + if (inputPath == null) { + tempInput = Files.createTempFile(tempDir, "dd-scrub-in-", ".jfr"); + Files.copy(data.getStream(), tempInput, StandardCopyOption.REPLACE_EXISTING); + inputPath = tempInput; + } + + tempOutput = Files.createTempFile(tempDir, "dd-scrub-out-", ".jfr"); + scrubber.scrubFile(inputPath, tempOutput); + + if (tempInput != null) { + Files.deleteIfExists(tempInput); + tempInput = null; + } + + ScrubbedRecordingData scrubbed = new ScrubbedRecordingData(data, tempOutput); + tempOutput = null; // ownership transferred to ScrubbedRecordingData + data.release(); + delegate.onNewData(type, scrubbed, handleSynchronously); + } catch (Exception e) { + cleanupQuietly(tempInput); + cleanupQuietly(tempOutput); + if (failOpen) { + log.warn(SEND_TELEMETRY, "JFR scrubbing failed, uploading unscrubbed data", e); + delegate.onNewData(type, data, handleSynchronously); + } else { + log.error(SEND_TELEMETRY, "JFR scrubbing failed, skipping upload", e); + data.release(); + } + } + } + + private static void cleanupQuietly(Path path) { + if (path != null) { + try { + Files.deleteIfExists(path); + } catch (IOException ignored) { + // best effort + } + } + } + + /** File-backed {@link RecordingData} wrapping a scrubbed output file. */ + static final class ScrubbedRecordingData extends RecordingData { + private final String name; + private final Path scrubbedFile; + + ScrubbedRecordingData(RecordingData original, Path scrubbedFile) { + super(original.getStart(), original.getEnd(), original.getKind()); + this.name = original.getName(); + this.scrubbedFile = scrubbedFile; + } + + @Nonnull + @Override + public RecordingInputStream getStream() throws IOException { + return new RecordingInputStream(Files.newInputStream(scrubbedFile)); + } + + @Override + public void release() { + try { + Files.deleteIfExists(scrubbedFile); + } catch (IOException e) { + log.debug("Failed to clean up scrubbed recording file: {}", scrubbedFile, e); + } + } + + @Nonnull + @Override + public String getName() { + return name; + } + + @Override + public Path getPath() { + return scrubbedFile; + } + } +} diff --git a/dd-java-agent/agent-profiling/src/test/java/com/datadog/profiling/agent/ScrubRecordingDataListenerTest.java b/dd-java-agent/agent-profiling/src/test/java/com/datadog/profiling/agent/ScrubRecordingDataListenerTest.java new file mode 100644 index 00000000000..0503175b36e --- /dev/null +++ b/dd-java-agent/agent-profiling/src/test/java/com/datadog/profiling/agent/ScrubRecordingDataListenerTest.java @@ -0,0 +1,128 @@ +package com.datadog.profiling.agent; + +import static org.junit.jupiter.api.Assertions.assertTrue; +import static org.mockito.ArgumentMatchers.any; +import static org.mockito.ArgumentMatchers.anyBoolean; +import static org.mockito.ArgumentMatchers.eq; +import static org.mockito.Mockito.doThrow; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.never; +import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.when; + +import com.datadog.profiling.scrubber.JfrScrubber; +import datadog.trace.api.profiling.ProfilingSnapshot; +import datadog.trace.api.profiling.RecordingData; +import datadog.trace.api.profiling.RecordingDataListener; +import datadog.trace.api.profiling.RecordingInputStream; +import datadog.trace.api.profiling.RecordingType; +import java.io.ByteArrayInputStream; +import java.io.IOException; +import java.nio.file.Files; +import java.nio.file.Path; +import java.time.Instant; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.io.TempDir; +import org.mockito.ArgumentCaptor; + +class ScrubRecordingDataListenerTest { + + @TempDir Path tempDir; + + private RecordingDataListener delegate; + private JfrScrubber scrubber; + private RecordingData mockData; + + @BeforeEach + void setUp() throws IOException { + delegate = mock(RecordingDataListener.class); + scrubber = mock(JfrScrubber.class); + mockData = mock(RecordingData.class); + when(mockData.getStart()).thenReturn(Instant.now()); + when(mockData.getEnd()).thenReturn(Instant.now()); + when(mockData.getKind()).thenReturn(ProfilingSnapshot.Kind.PERIODIC); + when(mockData.getName()).thenReturn("test"); + when(mockData.getStream()) + .thenReturn(new RecordingInputStream(new ByteArrayInputStream(new byte[] {1, 2, 3}))); + } + + @Test + void delegatesScrubbedData() throws Exception { + ScrubRecordingDataListener listener = + new ScrubRecordingDataListener(delegate, scrubber, tempDir, false); + + listener.onNewData(RecordingType.CONTINUOUS, mockData, false); + + ArgumentCaptor captor = ArgumentCaptor.forClass(RecordingData.class); + verify(delegate).onNewData(eq(RecordingType.CONTINUOUS), captor.capture(), eq(false)); + assertTrue( + captor.getValue() instanceof ScrubRecordingDataListener.ScrubbedRecordingData, + "Delegate should receive ScrubbedRecordingData"); + verify(mockData).release(); + } + + @Test + void usesExistingFilePathWhenAvailable() throws Exception { + // Simulate a file-backed recording (like ddprof) + Path existingFile = tempDir.resolve("existing.jfr"); + Files.write(existingFile, new byte[] {1, 2, 3}); + when(mockData.getPath()).thenReturn(existingFile); + + ScrubRecordingDataListener listener = + new ScrubRecordingDataListener(delegate, scrubber, tempDir, false); + + listener.onNewData(RecordingType.CONTINUOUS, mockData, false); + + // Scrubber should be called with the existing file path, not a temp file + ArgumentCaptor inputCaptor = ArgumentCaptor.forClass(Path.class); + ArgumentCaptor outputCaptor = ArgumentCaptor.forClass(Path.class); + verify(scrubber).scrubFile(inputCaptor.capture(), outputCaptor.capture()); + assertTrue( + inputCaptor.getValue().equals(existingFile), + "Should use existing file path as scrub input"); + verify(delegate).onNewData(eq(RecordingType.CONTINUOUS), any(RecordingData.class), eq(false)); + } + + @Test + void failClosedSkipsUpload() throws Exception { + doThrow(new RuntimeException("scrub failed")) + .when(scrubber) + .scrubFile(any(Path.class), any(Path.class)); + + ScrubRecordingDataListener listener = + new ScrubRecordingDataListener(delegate, scrubber, tempDir, false); + + listener.onNewData(RecordingType.CONTINUOUS, mockData, false); + + verify(delegate, never()).onNewData(any(), any(), anyBoolean()); + verify(mockData).release(); + } + + @Test + void failOpenDelegatesOriginalData() throws Exception { + doThrow(new RuntimeException("scrub failed")) + .when(scrubber) + .scrubFile(any(Path.class), any(Path.class)); + + ScrubRecordingDataListener listener = + new ScrubRecordingDataListener(delegate, scrubber, tempDir, true); + + listener.onNewData(RecordingType.CONTINUOUS, mockData, false); + + verify(delegate).onNewData(eq(RecordingType.CONTINUOUS), eq(mockData), eq(false)); + verify(mockData, never()).release(); + } + + @Test + void cleansTempFilesOnSuccess() throws Exception { + ScrubRecordingDataListener listener = + new ScrubRecordingDataListener(delegate, scrubber, tempDir, false); + + listener.onNewData(RecordingType.CONTINUOUS, mockData, false); + + // After success, temp input should be cleaned up, only scrubbed output remains + long jfrCount = Files.list(tempDir).filter(p -> p.toString().contains("dd-scrub-in-")).count(); + assertTrue(jfrCount == 0, "Temp input files should be cleaned up"); + } +} diff --git a/dd-java-agent/instrumentation/graal/graal-native-image-20.0/src/main/java/datadog/trace/instrumentation/graal/nativeimage/AnnotationSubstitutionProcessorInstrumentation.java b/dd-java-agent/instrumentation/graal/graal-native-image-20.0/src/main/java/datadog/trace/instrumentation/graal/nativeimage/AnnotationSubstitutionProcessorInstrumentation.java index a65e1bd1290..0765505d2db 100644 --- a/dd-java-agent/instrumentation/graal/graal-native-image-20.0/src/main/java/datadog/trace/instrumentation/graal/nativeimage/AnnotationSubstitutionProcessorInstrumentation.java +++ b/dd-java-agent/instrumentation/graal/graal-native-image-20.0/src/main/java/datadog/trace/instrumentation/graal/nativeimage/AnnotationSubstitutionProcessorInstrumentation.java @@ -34,9 +34,42 @@ public static class FindTargetClassesAdvice { public static void onExit(@Advice.Return(readOnly = false) List> result) { result.add(Target_com_datadog_profiling_agent_ProcessContext.class); result.add(Target_datadog_jctools_util_UnsafeRefArrayAccess.class); - result.add(Target_org_datadog_jmxfetch_App.class); - result.add(Target_org_datadog_jmxfetch_Status.class); - result.add(Target_org_datadog_jmxfetch_reporter_JsonReporter.class); + + // Only register JMXFetch substitutions if JMXFetch is actually present on the classpath. + // We must load these classes reflectively (not using .class literals) to prevent + // them from being discovered by GraalVM's annotation processor when JMXFetch is not present. + if (isJmxFetchPresent()) { + try { + ClassLoader cl = FindTargetClassesAdvice.class.getClassLoader(); + result.add( + Class.forName( + "datadog.trace.instrumentation.graal.nativeimage.Target_org_datadog_jmxfetch_App", + false, + cl)); + result.add( + Class.forName( + "datadog.trace.instrumentation.graal.nativeimage.Target_org_datadog_jmxfetch_Status", + false, + cl)); + result.add( + Class.forName( + "datadog.trace.instrumentation.graal.nativeimage.Target_org_datadog_jmxfetch_reporter_JsonReporter", + false, + cl)); + } catch (ClassNotFoundException e) { + // Substitution classes not available, skip them + } + } + } + + private static boolean isJmxFetchPresent() { + try { + Class.forName( + "org.datadog.jmxfetch.App", false, FindTargetClassesAdvice.class.getClassLoader()); + return true; + } catch (ClassNotFoundException e) { + return false; + } } } } diff --git a/dd-smoke-tests/profiling-integration-tests/src/test/java/datadog/smoketest/JFRBasedProfilingIntegrationTest.java b/dd-smoke-tests/profiling-integration-tests/src/test/java/datadog/smoketest/JFRBasedProfilingIntegrationTest.java index 181b2416e04..4473771d3ad 100644 --- a/dd-smoke-tests/profiling-integration-tests/src/test/java/datadog/smoketest/JFRBasedProfilingIntegrationTest.java +++ b/dd-smoke-tests/profiling-integration-tests/src/test/java/datadog/smoketest/JFRBasedProfilingIntegrationTest.java @@ -867,7 +867,8 @@ private static ProcessBuilder createProcessBuilder( final String withCompression, final int exitDelay, final Path logFilePath, - final boolean tracingEnabled) { + final boolean tracingEnabled, + final String... extraProperties) { final String templateOverride = JFRBasedProfilingIntegrationTest.class .getClassLoader() @@ -906,6 +907,9 @@ private static ProcessBuilder createProcessBuilder( command.add("-Ddd.profiling.context.attributes=foo,bar"); } command.add("-Ddd.profiling.debug.upload.compression=" + withCompression); + for (String extra : extraProperties) { + command.add(extra); + } command.add("-Ddatadog.slf4j.simpleLogger.defaultLogLevel=debug"); command.add("-Dorg.slf4j.simpleLogger.defaultLogLevel=debug"); command.add("-XX:+IgnoreUnrecognizedVMOptions"); @@ -969,4 +973,95 @@ private static boolean logHasErrors(final Path logFilePath) throws IOException { public static boolean isJavaVersionAtLeast24() { return JavaVirtualMachine.isJavaVersionAtLeast(24); } + + @Test + @DisplayName("Test JFR scrubbing") + void testJfrScrubbing(final TestInfo testInfo) throws Exception { + testWithRetry( + () -> { + try { + targetProcess = + createProcessBuilder( + profilingServer.getPort(), + tracingServer.getPort(), + VALID_API_KEY, + 0, + PROFILING_START_DELAY_SECONDS, + PROFILING_UPLOAD_PERIOD_SECONDS, + ENDPOINT_COLLECTION_ENABLED, + true, + "on", + 0, + logFilePath, + true, + "-Ddd.profiling.scrub.enabled=true") + .start(); + + Assumptions.assumeFalse(JavaVirtualMachine.isJ9()); + + final RecordedRequest request = retrieveRequest(); + assertNotNull(request); + + final List items = + FileUpload.parse( + request.getBody().readByteArray(), request.getHeader("Content-Type")); + + FileItem rawJfr = items.get(1); + assertEquals("main.jfr", rawJfr.getName()); + + assertFalse(logHasErrors(logFilePath)); + InputStream eventStream = new ByteArrayInputStream(rawJfr.get()); + eventStream = decompressStream("on", eventStream); + IItemCollection events = JfrLoaderToolkit.loadEvents(eventStream); + assertTrue(events.hasItems()); + + // Verify that system properties are scrubbed + IItemCollection systemPropertyEvents = + events.apply(ItemFilters.type(JdkTypeIDs.SYSTEM_PROPERTIES)); + if (systemPropertyEvents.hasItems()) { + IAttribute valueAttr = attr("value", "value", "value", PLAIN_TEXT); + for (IItemIterable event : systemPropertyEvents) { + IMemberAccessor valueAccessor = + valueAttr.getAccessor(event.getType()); + for (IItem item : event) { + String value = valueAccessor.getMember(item); + if (value != null && !value.isEmpty()) { + // Scrubbed values should contain only 'x' characters + assertTrue( + value.chars().allMatch(c -> c == 'x'), + "System property value should be scrubbed: " + value); + } + } + } + } + + // Verify that JVM arguments are scrubbed + IItemCollection jvmInfoEvents = events.apply(ItemFilters.type("jdk.JVMInformation")); + if (jvmInfoEvents.hasItems()) { + IAttribute jvmArgsAttr = + attr("jvmArguments", "jvmArguments", "jvmArguments", PLAIN_TEXT); + for (IItemIterable event : jvmInfoEvents) { + IMemberAccessor jvmArgsAccessor = + jvmArgsAttr.getAccessor(event.getType()); + for (IItem item : event) { + String jvmArgs = jvmArgsAccessor.getMember(item); + if (jvmArgs != null && !jvmArgs.isEmpty()) { + // Scrubbed values should contain only 'x' characters + assertTrue( + jvmArgs.chars().allMatch(c -> c == 'x'), + "JVM arguments should be scrubbed: " + jvmArgs); + } + } + } + } + } finally { + if (targetProcess != null) { + targetProcess.destroyForcibly(); + } + targetProcess = null; + } + }, + testInfo, + 3); + } } diff --git a/dd-smoke-tests/spring-boot-3.0-native/application/build.gradle b/dd-smoke-tests/spring-boot-3.0-native/application/build.gradle index 1c199bb08c2..d820d5deaab 100644 --- a/dd-smoke-tests/spring-boot-3.0-native/application/build.gradle +++ b/dd-smoke-tests/spring-boot-3.0-native/application/build.gradle @@ -37,8 +37,12 @@ if (hasProperty('agentPath')) { buildArgs.add("-J-Dnet.bytebuddy.safe=false") if (withProfiler && property('profiler') == 'true') { buildArgs.add("-J-Ddd.profiling.enabled=true") + // Disable exception profiling during native-image build to avoid class initialization cascade + buildArgs.add("-J-Ddd.profiling.disabled.events=datadog.ExceptionSample") + // Trace to see what's still triggering the cascade + buildArgs.add("--trace-class-initialization=datadog.trace.bootstrap.CallDepthThreadLocalMap") } - jvmArgs.add("-Xmx4096M") + jvmArgs.add("-Xmx8192M") } } } diff --git a/dd-trace-api/src/main/java/datadog/trace/api/config/ProfilingConfig.java b/dd-trace-api/src/main/java/datadog/trace/api/config/ProfilingConfig.java index ef764263107..43586d21829 100644 --- a/dd-trace-api/src/main/java/datadog/trace/api/config/ProfilingConfig.java +++ b/dd-trace-api/src/main/java/datadog/trace/api/config/ProfilingConfig.java @@ -264,5 +264,14 @@ public final class ProfilingConfig { public static final String PROFILING_DETAILED_DEBUG_LOGGING = "profiling.detailed.debug.logging"; public static final boolean PROFILING_DETAILED_DEBUG_LOGGING_DEFAULT = false; + // dummy commit + public static final String PROFILING_SCRUB_ENABLED = "profiling.scrub.enabled"; + public static final boolean PROFILING_SCRUB_ENABLED_DEFAULT = false; + + public static final String PROFILING_SCRUB_FAIL_OPEN = "profiling.scrub.fail-open"; + public static final boolean PROFILING_SCRUB_FAIL_OPEN_DEFAULT = false; + + public static final String PROFILING_SCRUB_EXCLUDE_EVENTS = "profiling.scrub.exclude-events"; + private ProfilingConfig() {} } diff --git a/gradle/libs.versions.toml b/gradle/libs.versions.toml index 6ba14a7d599..7a8c3d40bdc 100644 --- a/gradle/libs.versions.toml +++ b/gradle/libs.versions.toml @@ -38,6 +38,7 @@ jmh = "1.37" # Profiling jmc = "8.1.0" +jafar-parser = "0.9.0" # Web & Network jnr-unixsocket = "0.38.24" @@ -115,6 +116,7 @@ instrument-java = { module = "com.datadoghq:dd-instrument-java", version.ref = " # Profiling jmc-common = { module = "org.openjdk.jmc:common", version.ref = "jmc" } jmc-flightrecorder = { module = "org.openjdk.jmc:flightrecorder", version.ref = "jmc" } +jafar-parser = { module = "io.btrace:jafar-parser", version.ref = "jafar-parser" } # Web & Network okio = { module = "com.datadoghq.okio:okio", version.ref = "okio" } diff --git a/internal-api/src/main/java/datadog/trace/api/Config.java b/internal-api/src/main/java/datadog/trace/api/Config.java index 88f28845246..6038edeaa9b 100644 --- a/internal-api/src/main/java/datadog/trace/api/Config.java +++ b/internal-api/src/main/java/datadog/trace/api/Config.java @@ -482,6 +482,11 @@ import static datadog.trace.api.config.ProfilingConfig.PROFILING_PROXY_PORT; import static datadog.trace.api.config.ProfilingConfig.PROFILING_PROXY_PORT_DEFAULT; import static datadog.trace.api.config.ProfilingConfig.PROFILING_PROXY_USERNAME; +import static datadog.trace.api.config.ProfilingConfig.PROFILING_SCRUB_ENABLED; +import static datadog.trace.api.config.ProfilingConfig.PROFILING_SCRUB_ENABLED_DEFAULT; +import static datadog.trace.api.config.ProfilingConfig.PROFILING_SCRUB_EXCLUDE_EVENTS; +import static datadog.trace.api.config.ProfilingConfig.PROFILING_SCRUB_FAIL_OPEN; +import static datadog.trace.api.config.ProfilingConfig.PROFILING_SCRUB_FAIL_OPEN_DEFAULT; import static datadog.trace.api.config.ProfilingConfig.PROFILING_START_DELAY; import static datadog.trace.api.config.ProfilingConfig.PROFILING_START_DELAY_DEFAULT; import static datadog.trace.api.config.ProfilingConfig.PROFILING_START_FORCE_FIRST; @@ -977,6 +982,9 @@ public static String getHostName() { private final boolean profilingExcludeAgentThreads; private final boolean profilingUploadSummaryOn413Enabled; private final boolean profilingRecordExceptionMessage; + private final boolean profilingScrubEnabled; + private final boolean profilingScrubFailOpen; + private final String profilingScrubExcludeEvents; private final boolean crashTrackingAgentless; private final Map crashTrackingTags; @@ -2157,6 +2165,12 @@ PROFILING_DATADOG_PROFILER_ENABLED, isDatadogProfilerSafeInCurrentEnvironment()) configProvider.getBoolean( PROFILING_UPLOAD_SUMMARY_ON_413, PROFILING_UPLOAD_SUMMARY_ON_413_DEFAULT); + profilingScrubEnabled = + configProvider.getBoolean(PROFILING_SCRUB_ENABLED, PROFILING_SCRUB_ENABLED_DEFAULT); + profilingScrubFailOpen = + configProvider.getBoolean(PROFILING_SCRUB_FAIL_OPEN, PROFILING_SCRUB_FAIL_OPEN_DEFAULT); + profilingScrubExcludeEvents = configProvider.getString(PROFILING_SCRUB_EXCLUDE_EVENTS); + crashTrackingAgentless = configProvider.getBoolean(CRASH_TRACKING_AGENTLESS, CRASH_TRACKING_AGENTLESS_DEFAULT); crashTrackingTags = configProvider.getMergedMap(CRASH_TRACKING_TAGS); @@ -3669,6 +3683,18 @@ public int getProfilingDirectAllocationSampleLimit() { return profilingDirectAllocationSampleLimit; } + public boolean isProfilingScrubEnabled() { + return profilingScrubEnabled; + } + + public boolean isProfilingScrubFailOpen() { + return profilingScrubFailOpen; + } + + public String getProfilingScrubExcludeEvents() { + return profilingScrubExcludeEvents; + } + public int getProfilingBackPressureSampleLimit() { return profilingBackPressureSampleLimit; } diff --git a/internal-api/src/main/java/datadog/trace/api/profiling/RecordingData.java b/internal-api/src/main/java/datadog/trace/api/profiling/RecordingData.java index c886ebcf81a..d62f1541c3e 100644 --- a/internal-api/src/main/java/datadog/trace/api/profiling/RecordingData.java +++ b/internal-api/src/main/java/datadog/trace/api/profiling/RecordingData.java @@ -16,8 +16,10 @@ package datadog.trace.api.profiling; import java.io.IOException; +import java.nio.file.Path; import java.time.Instant; import javax.annotation.Nonnull; +import javax.annotation.Nullable; /** Platform-agnostic API for operations required when retrieving data using the ProfilingSystem. */ public abstract class RecordingData implements ProfilingSnapshot { @@ -89,6 +91,17 @@ public final Kind getKind() { return kind; } + /** + * Returns the file path backing this recording data, if available. Implementations that store + * recording data on disk can override this to avoid unnecessary stream materialization. + * + * @return the file path, or {@code null} if the data is not file-backed + */ + @Nullable + public Path getPath() { + return null; + } + @Override public final String toString() { return "name=" + getName() + ", kind=" + getKind(); diff --git a/metadata/supported-configurations.json b/metadata/supported-configurations.json index 8dca3dee711..87c43e16163 100644 --- a/metadata/supported-configurations.json +++ b/metadata/supported-configurations.json @@ -3065,6 +3065,30 @@ "aliases": [] } ], + "DD_PROFILING_SCRUB_ENABLED": [ + { + "version": "A", + "type": "boolean", + "default": "false", + "aliases": [] + } + ], + "DD_PROFILING_SCRUB_EXCLUDE_EVENTS": [ + { + "version": "A", + "type": "string", + "default": null, + "aliases": [] + } + ], + "DD_PROFILING_SCRUB_FAIL_OPEN": [ + { + "version": "A", + "type": "boolean", + "default": "false", + "aliases": [] + } + ], "DD_PROFILING_SMAP_AGGREGATION_ENABLED": [ { "version": "A", diff --git a/settings.gradle.kts b/settings.gradle.kts index 0d9b113dfdd..3053ab723f8 100644 --- a/settings.gradle.kts +++ b/settings.gradle.kts @@ -78,6 +78,7 @@ include( ":dd-java-agent:agent-profiling:profiling-controller-ddprof", ":dd-java-agent:agent-profiling:profiling-controller-openjdk", ":dd-java-agent:agent-profiling:profiling-controller-oracle", + ":dd-java-agent:agent-profiling:profiling-scrubber", ":dd-java-agent:agent-profiling:profiling-testing", ":dd-java-agent:agent-profiling:profiling-uploader", ":dd-java-agent:agent-profiling:profiling-utils",