diff --git a/dd-java-agent/agent-profiling/build.gradle b/dd-java-agent/agent-profiling/build.gradle index a53ac40d8fe..e6585b3d510 100644 --- a/dd-java-agent/agent-profiling/build.gradle +++ b/dd-java-agent/agent-profiling/build.gradle @@ -13,7 +13,9 @@ 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 { @@ -23,6 +25,7 @@ dependencies { 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') @@ -42,6 +45,11 @@ configurations { tasks.named("shadowJar", ShadowJar) { dependencies deps.excludeShared + + // Exclude multi-release versioned classes from jafar-parser. + // These are duplicates of base classes for newer Java APIs and confuse + // the GraalVM native-image builder when the profiling jar is embedded in the agent. + exclude 'META-INF/versions/**' } tasks.named("jar", Jar) { 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..5c369d3762b --- /dev/null +++ b/dd-java-agent/agent-profiling/profiling-scrubber/build.gradle @@ -0,0 +1,17 @@ +apply from: "$rootDir/gradle/java.gradle" + +minimumInstructionCoverage = 0.0 +minimumBranchCoverage = 0.0 + +dependencies { + api libs.slf4j + + implementation(libs.jafar.tools) { + // Agent has its own slf4j binding + exclude group: 'org.slf4j', module: 'slf4j-simple' + } + + 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..a625041150f --- /dev/null +++ b/dd-java-agent/agent-profiling/profiling-scrubber/src/main/java/com/datadog/profiling/scrubber/DefaultScrubDefinition.java @@ -0,0 +1,53 @@ +package com.datadog.profiling.scrubber; + +import io.jafar.tools.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; + +/** 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<>(); + // ScrubField(keyField, valueField, predicate): null keyField = scrub all values unconditionally + // System properties may contain API keys, passwords + fields.put("jdk.InitialSystemProperty", new Scrubber.ScrubField(null, "value", (k, v) -> true)); + // JVM args may contain credentials in -D flags + fields.put("jdk.JVMInformation", new Scrubber.ScrubField(null, "jvmArguments", (k, v) -> true)); + // Env vars may contain secrets + fields.put( + "jdk.InitialEnvironmentVariable", new Scrubber.ScrubField(null, "value", (k, v) -> true)); + // Process command lines may reveal infrastructure + fields.put("jdk.SystemProcess", new Scrubber.ScrubField(null, "commandLine", (k, v) -> true)); + DEFAULT_SCRUB_FIELDS = Collections.unmodifiableMap(fields); + } + + /** + * Creates a scrubber with the default scrub definition. + * + * @param excludeEventTypes list of event type names to exclude from scrubbing, or null for none + * @return a configured {@link JfrScrubber} + */ + public static JfrScrubber create(List excludeEventTypes) { + Set excludeSet = + excludeEventTypes != null + ? new HashSet<>(excludeEventTypes) + : Collections.emptySet(); + + return new JfrScrubber( + 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..db6ea640ef4 --- /dev/null +++ b/dd-java-agent/agent-profiling/profiling-scrubber/src/main/java/com/datadog/profiling/scrubber/JfrScrubber.java @@ -0,0 +1,30 @@ +package com.datadog.profiling.scrubber; + +import io.jafar.tools.Scrubber; +import java.nio.file.Path; +import java.util.function.Function; + +/** + * Thin wrapper around {@link Scrubber} from jafar-tools, hiding jafar types from consumers outside + * the profiling-scrubber module. + */ +public final class JfrScrubber { + + private final Function scrubDefinition; + + /** Package-private: use {@link DefaultScrubDefinition#create} to obtain an instance. */ + JfrScrubber(Function scrubDefinition) { + this.scrubDefinition = scrubDefinition; + } + + /** + * Scrub the given file by replacing targeted field values with '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 { + Scrubber.scrubFile(input, output, scrubDefinition); + } +} 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..d1fbb2d499e --- /dev/null +++ b/dd-java-agent/agent-profiling/profiling-scrubber/src/test/java/com/datadog/profiling/scrubber/JfrScrubberTest.java @@ -0,0 +1,93 @@ +package com.datadog.profiling.scrubber; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertTrue; +import static org.openjdk.jmc.common.item.Attribute.attr; +import static org.openjdk.jmc.common.unit.UnitLookup.PLAIN_TEXT; + +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.Collections; +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.IAttribute; +import org.openjdk.jmc.common.item.IItem; +import org.openjdk.jmc.common.item.IItemCollection; +import org.openjdk.jmc.common.item.IItemIterable; +import org.openjdk.jmc.common.item.IMemberAccessor; +import org.openjdk.jmc.common.item.ItemFilters; +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 { + JfrScrubber scrubber = DefaultScrubDefinition.create(null); + Path outputFile = tempDir.resolve("output.jfr"); + scrubber.scrubFile(inputFile, outputFile); + + assertTrue(Files.exists(outputFile)); + assertTrue(Files.size(outputFile) > 0, "Scrubbed file should not be empty"); + + // Verify scrubbed values contain only 'x' characters + IItemCollection events = JfrLoaderToolkit.loadEvents(outputFile.toFile()); + IItemCollection systemPropertyEvents = + events.apply(ItemFilters.type("jdk.InitialSystemProperty")); + assertTrue(systemPropertyEvents.hasItems(), "Expected jdk.InitialSystemProperty events"); + + IAttribute valueAttr = attr("value", "value", "value", PLAIN_TEXT); + for (IItemIterable itemIterable : systemPropertyEvents) { + IMemberAccessor accessor = valueAttr.getAccessor(itemIterable.getType()); + for (IItem item : itemIterable) { + String value = accessor.getMember(item); + if (value != null && !value.isEmpty()) { + assertTrue( + value.chars().allMatch(c -> c == 'x'), + "System property value should be scrubbed: " + value); + } + } + } + } + + @Test + void scrubWithNoMatchingEvents() throws Exception { + // Scrubber with all default events excluded — nothing matches + JfrScrubber scrubber = new JfrScrubber(name -> null); + 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 { + // Exclude jdk.InitialSystemProperty from scrubbing + JfrScrubber scrubber = + DefaultScrubDefinition.create(Collections.singletonList("jdk.InitialSystemProperty")); + Path outputFile = tempDir.resolve("output.jfr"); + scrubber.scrubFile(inputFile, outputFile); + + assertTrue(Files.exists(outputFile)); + assertTrue(Files.size(outputFile) > 0); + } +} 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..d21b546a99a 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,6 +2,10 @@ 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.telemetry.LogCollector.SEND_TELEMETRY; @@ -24,6 +28,7 @@ import datadog.trace.api.profiling.RecordingDataListener; import datadog.trace.api.profiling.RecordingType; import datadog.trace.bootstrap.config.provider.ConfigProvider; +import de.thetaphi.forbiddenapis.SuppressForbidden; import java.io.IOException; import java.lang.instrument.Instrumentation; import java.lang.ref.WeakReference; @@ -32,6 +37,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 +143,25 @@ 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); + }; + } + // Scrubber wraps the combined dumper+uploader so debug dumps also contain scrubbed data + if (configProvider.getBoolean(PROFILING_SCRUB_ENABLED, PROFILING_SCRUB_ENABLED_DEFAULT)) { + List excludeEventTypes = + configProvider.getList(ProfilingConfig.PROFILING_SCRUB_EXCLUDE_EVENTS); + boolean failOpen = + configProvider.getBoolean( + PROFILING_SCRUB_FAIL_OPEN, PROFILING_SCRUB_FAIL_OPEN_DEFAULT); + listener = wrapWithScrubber(listener, excludeEventTypes, failOpen); + } + final Duration startupDelay = Duration.ofSeconds(config.getProfilingStartDelay()); final Duration uploadPeriod = Duration.ofSeconds(config.getProfilingUploadPeriod()); @@ -149,12 +174,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, @@ -181,6 +201,30 @@ public static synchronized boolean run(final boolean earlyStart, Instrumentation return false; } + /** + * Wraps a listener with the JFR scrubber using reflection to avoid a compile-time dependency on + * {@code ScrubRecordingDataListener} and its transitive jafar-parser classes. A direct reference + * would cause {@code NoClassDefFoundError} during GraalVM native-image builds when the + * VMRuntimeModule's helper injector walks transitive dependencies of this class. + */ + // Class.forName is safe here — the target class lives in the same classloader (agent-profiling + // shadow jar). We use reflection solely to avoid a compile-time type reference that would cause + // GraalVM native-image to walk jafar-parser's transitive dependencies. + @SuppressForbidden + private static RecordingDataListener wrapWithScrubber( + RecordingDataListener listener, List excludeEventTypes, boolean failOpen) { + try { + Class scrubClass = Class.forName("com.datadog.profiling.agent.ScrubRecordingDataListener"); + return (RecordingDataListener) + scrubClass + .getDeclaredMethod("wrap", RecordingDataListener.class, List.class, boolean.class) + .invoke(null, listener, excludeEventTypes, failOpen); + } catch (Exception e) { + log.warn(SEND_TELEMETRY, "Failed to initialize JFR scrubber", e); + return listener; + } + } + private static boolean isStartForceFirstSafe() { return isJavaVersionAtLeast(14) || (isJavaVersion(13) && isJavaVersionAtLeast(13, 0, 4)) 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..4692303054f --- /dev/null +++ b/dd-java-agent/agent-profiling/src/main/java/com/datadog/profiling/agent/ScrubRecordingDataListener.java @@ -0,0 +1,157 @@ +package com.datadog.profiling.agent; + +import static datadog.trace.api.telemetry.LogCollector.SEND_TELEMETRY; + +import com.datadog.profiling.scrubber.DefaultScrubDefinition; +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 datadog.trace.util.TempLocationManager; +import java.io.IOException; +import java.nio.file.Files; +import java.nio.file.Path; +import java.nio.file.Paths; +import java.nio.file.StandardCopyOption; +import java.util.List; +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 static final Path SCRUB_SUBDIR = Paths.get("scrub"); + + private final RecordingDataListener delegate; + private final JfrScrubber scrubber; + private final boolean failOpen; + private final Path tempDirOverride; + + /** + * Wraps {@code delegate} with a scrubbing listener. Invoked via reflection from ProfilingAgent. + */ + static RecordingDataListener wrap( + RecordingDataListener delegate, List excludeEventTypes, boolean failOpen) { + return new ScrubRecordingDataListener( + delegate, DefaultScrubDefinition.create(excludeEventTypes), failOpen); + } + + ScrubRecordingDataListener( + RecordingDataListener delegate, JfrScrubber scrubber, boolean failOpen) { + this(delegate, scrubber, failOpen, null); + } + + // visible for testing + ScrubRecordingDataListener( + RecordingDataListener delegate, JfrScrubber scrubber, boolean failOpen, Path tempDir) { + this.delegate = delegate; + this.scrubber = scrubber; + this.failOpen = failOpen; + this.tempDirOverride = tempDir; + } + + private Path getTempDir() { + if (tempDirOverride != null) { + return tempDirOverride; + } + return TempLocationManager.getInstance().getTempDir(SCRUB_SUBDIR); + } + + @Override + public void onNewData(RecordingType type, RecordingData data, boolean handleSynchronously) { + Path tempInput = null; + Path tempOutput = null; + try { + Path tempDir = getTempDir(); + // 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"); + try (RecordingInputStream in = data.getStream()) { + Files.copy(in, 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 + // Release the original recording eagerly — the scrubbed copy is now the source of truth. + // The delegate will call release() on ScrubbedRecordingData to clean up the output file. + 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..8c054dc217e --- /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.assertEquals; +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, false, tempDir); + + 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, false, tempDir); + + 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()); + assertEquals( + existingFile, inputCaptor.getValue(), "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, false, tempDir); + + 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, true, tempDir); + + 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, false, tempDir); + + 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(); + assertEquals(0L, jfrCount, "Temp input files should be cleaned up"); + } +} diff --git a/dd-java-agent/build.gradle b/dd-java-agent/build.gradle index 9885181de25..6d15674c6e9 100644 --- a/dd-java-agent/build.gradle +++ b/dd-java-agent/build.gradle @@ -436,7 +436,7 @@ tasks.register('checkAgentJarSize') { doLast { // Arbitrary limit to prevent unintentional increases to the agent jar size // Raise or lower as required - assert tasks.named("shadowJar", ShadowJar).get().archiveFile.get().getAsFile().length() <= 32 * 1024 * 1024 + assert tasks.named("shadowJar", ShadowJar).get().archiveFile.get().getAsFile().length() <= 33 * 1024 * 1024 } dependsOn "shadowJar" 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..28ed489ac5f 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,102 @@ 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.stream() + .filter(i -> "main.jfr".equals(i.getName())) + .findFirst() + .orElseThrow(() -> new AssertionError("main.jfr not found in upload")); + + 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)); + assertTrue( + systemPropertyEvents.hasItems(), + "Expected jdk.InitialSystemProperty events in recording"); + { + 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")); + assertTrue(jvmInfoEvents.hasItems(), "Expected jdk.JVMInformation events in recording"); + { + 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-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..4dba5b03169 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,13 @@ 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; + 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 9146f93c1af..aea2ea251fe 100644 --- a/gradle/libs.versions.toml +++ b/gradle/libs.versions.toml @@ -38,6 +38,7 @@ jmh = "1.37" # Profiling jmc = "8.1.0" +jafar = "0.13.2" # Web & Network jnr-unixsocket = "0.38.24" @@ -114,6 +115,8 @@ 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" } +jafar-tools = { module = "io.btrace:jafar-tools", version.ref = "jafar" } # Web & Network okio = { module = "com.datadoghq.okio:okio", version.ref = "okio" } 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",