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..a3ae5853a62 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,10 @@ && 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)) { + configProvider.getString(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..9d683308394 --- /dev/null +++ b/dd-java-agent/agent-profiling/profiling-scrubber/build.gradle @@ -0,0 +1,19 @@ +apply from: "$rootDir/gradle/java.gradle" + +minimumInstructionCoverage = 0.0 +minimumBranchCoverage = 0.0 + +dependencies { + api libs.slf4j + + implementation(libs.jafar.tools) { + // ASM is only used by jafar's CodeGenerator/Deserializer path which the scrubber does not use + exclude group: 'org.ow2.asm', module: 'asm' + // 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..d7707575213 --- /dev/null +++ b/dd-java-agent/agent-profiling/profiling-scrubber/src/main/java/com/datadog/profiling/scrubber/DefaultScrubDefinition.java @@ -0,0 +1,52 @@ +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<>(); + // 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..1dacfdf8173 --- /dev/null +++ b/dd-java-agent/agent-profiling/profiling-scrubber/src/main/java/com/datadog/profiling/scrubber/JfrScrubber.java @@ -0,0 +1,29 @@ +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; + + 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..e804ed4cf31 --- /dev/null +++ b/dd-java-agent/agent-profiling/profiling-scrubber/src/test/java/com/datadog/profiling/scrubber/JfrScrubberTest.java @@ -0,0 +1,69 @@ +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.Collections; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.io.TempDir; +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 the scrubbed file is valid and parseable + JfrLoaderToolkit.loadEvents(outputFile.toFile()); + } + + @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..27ad28dce59 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; @@ -14,6 +18,7 @@ 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.uploader.ProfileUploader; import com.datadog.profiling.utils.Timestamper; import datadog.trace.api.Config; @@ -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,27 @@ 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)) { + List excludeEventTypes = + configProvider.getList(ProfilingConfig.PROFILING_SCRUB_EXCLUDE_EVENTS); + boolean failOpen = + configProvider.getBoolean( + PROFILING_SCRUB_FAIL_OPEN, PROFILING_SCRUB_FAIL_OPEN_DEFAULT); + + listener = + new ScrubRecordingDataListener( + listener, DefaultScrubDefinition.create(excludeEventTypes), failOpen); + } + final Duration startupDelay = Duration.ofSeconds(config.getProfilingStartDelay()); final Duration uploadPeriod = Duration.ofSeconds(config.getProfilingUploadPeriod()); @@ -149,12 +176,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..4b9ae04381b --- /dev/null +++ b/dd-java-agent/agent-profiling/src/main/java/com/datadog/profiling/agent/ScrubRecordingDataListener.java @@ -0,0 +1,144 @@ +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 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 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; + + 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 + 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..bc060ba0f43 --- /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, 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()); + 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, 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(); + assertTrue(jfrCount == 0, "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-java-agent/instrumentation/datadog/profiling/exception-profiling/src/main/java/datadog/exceptions/instrumentation/ThrowableInstrumentation.java b/dd-java-agent/instrumentation/datadog/profiling/exception-profiling/src/main/java/datadog/exceptions/instrumentation/ThrowableInstrumentation.java index eb91b4e1088..daf54555cab 100644 --- a/dd-java-agent/instrumentation/datadog/profiling/exception-profiling/src/main/java/datadog/exceptions/instrumentation/ThrowableInstrumentation.java +++ b/dd-java-agent/instrumentation/datadog/profiling/exception-profiling/src/main/java/datadog/exceptions/instrumentation/ThrowableInstrumentation.java @@ -18,7 +18,7 @@ public ThrowableInstrumentation() { @Override public boolean isEnabled() { - return Platform.hasJfr() && super.isEnabled(); + return Platform.hasJfr() && !Platform.isNativeImageBuilder() && super.isEnabled(); } @Override 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-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..05a6a79e4c9 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.0" # 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/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",