From 79a60e138ce5d63aa5bf0e5b447fbfff23f4b4e7 Mon Sep 17 00:00:00 2001 From: Nikita Tkachenko <121111529+nikita-tkachenko-datadog@users.noreply.github.com> Date: Wed, 6 Aug 2025 14:50:14 +0200 Subject: [PATCH 01/11] =?UTF-8?q?=F0=9F=8D=92=209318=20-=20Do=20not=20foll?= =?UTF-8?q?ow=20symlinks=20by=20default=20when=20building=20repository=20i?= =?UTF-8?q?ndex=20(#9322)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- .../source/index/RepoIndexBuilder.java | 20 +++++++++++++------ .../trace/api/config/CiVisibilityConfig.java | 2 ++ .../main/java/datadog/trace/api/Config.java | 8 ++++++++ 3 files changed, 24 insertions(+), 6 deletions(-) diff --git a/dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/source/index/RepoIndexBuilder.java b/dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/source/index/RepoIndexBuilder.java index 805cb4e56c1..23b93ec5580 100644 --- a/dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/source/index/RepoIndexBuilder.java +++ b/dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/source/index/RepoIndexBuilder.java @@ -104,6 +104,7 @@ private static final class RepoIndexingFileVisitor implements FileVisitor private final RepoIndexingStats indexingStats; private final Path repoRoot; private final AtomicInteger sourceRootCounter; + private final boolean followSymlinks; private RepoIndexingFileVisitor( Config config, @@ -120,16 +121,23 @@ private RepoIndexingFileVisitor( packageTree = new PackageTree(config); indexingStats = new RepoIndexingStats(); sourceRootCounter = new AtomicInteger(); + followSymlinks = config.isCiVisibilityRepoIndexFollowSymlinks(); } @Override public FileVisitResult preVisitDirectory(Path dir, BasicFileAttributes attrs) { - if (Files.isSymbolicLink(dir) && readSymbolicLink(dir).startsWith(repoRoot)) { - // The path is a symlink that points inside the repo. - // We'll visit the folder that it points to anyway, - // moreover, we don't want two different results for one file - // (one containing the symlink, the other - the actual folder). - return FileVisitResult.SKIP_SUBTREE; + if (Files.isSymbolicLink(dir)) { + if (!followSymlinks) { + // Configured to skip symlinks + return FileVisitResult.SKIP_SUBTREE; + } + if (readSymbolicLink(dir).startsWith(repoRoot)) { + // The path is a symlink that points inside the repo. + // We'll visit the folder that it points to anyway, + // moreover, we don't want two different results for one file + // (one containing the symlink, the other - the actual folder). + return FileVisitResult.SKIP_SUBTREE; + } } return FileVisitResult.CONTINUE; } diff --git a/dd-trace-api/src/main/java/datadog/trace/api/config/CiVisibilityConfig.java b/dd-trace-api/src/main/java/datadog/trace/api/config/CiVisibilityConfig.java index ad93aa9dfc7..e39832f1432 100644 --- a/dd-trace-api/src/main/java/datadog/trace/api/config/CiVisibilityConfig.java +++ b/dd-trace-api/src/main/java/datadog/trace/api/config/CiVisibilityConfig.java @@ -44,6 +44,8 @@ public final class CiVisibilityConfig { "civisibility.ciprovider.integration.enabled"; public static final String CIVISIBILITY_REPO_INDEX_DUPLICATE_KEY_CHECK_ENABLED = "civisibility.repo.index.duplicate.key.check.enabled"; + public static final String CIVISIBILITY_REPO_INDEX_FOLLOW_SYMLINKS = + "civisibility.repo.index.follow.symlinks"; public static final String CIVISIBILITY_EXECUTION_SETTINGS_CACHE_SIZE = "civisibility.execution.settings.cache.size"; public static final String CIVISIBILITY_JVM_INFO_CACHE_SIZE = "civisibility.jvm.info.cache.size"; 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 5ff002d4144..1d257c56542 100644 --- a/internal-api/src/main/java/datadog/trace/api/Config.java +++ b/internal-api/src/main/java/datadog/trace/api/Config.java @@ -254,6 +254,7 @@ import static datadog.trace.api.config.CiVisibilityConfig.CIVISIBILITY_REMOTE_ENV_VARS_PROVIDER_KEY; import static datadog.trace.api.config.CiVisibilityConfig.CIVISIBILITY_REMOTE_ENV_VARS_PROVIDER_URL; import static datadog.trace.api.config.CiVisibilityConfig.CIVISIBILITY_REPO_INDEX_DUPLICATE_KEY_CHECK_ENABLED; +import static datadog.trace.api.config.CiVisibilityConfig.CIVISIBILITY_REPO_INDEX_FOLLOW_SYMLINKS; import static datadog.trace.api.config.CiVisibilityConfig.CIVISIBILITY_RESOURCE_FOLDER_NAMES; import static datadog.trace.api.config.CiVisibilityConfig.CIVISIBILITY_RUM_FLUSH_WAIT_MILLIS; import static datadog.trace.api.config.CiVisibilityConfig.CIVISIBILITY_SCALATEST_FORK_MONITOR_ENABLED; @@ -1001,6 +1002,7 @@ public static String getHostName() { private final boolean ciVisibilityTestSkippingEnabled; private final boolean ciVisibilityCiProviderIntegrationEnabled; private final boolean ciVisibilityRepoIndexDuplicateKeyCheckEnabled; + private final boolean ciVisibilityRepoIndexFollowSymlinks; private final int ciVisibilityExecutionSettingsCacheSize; private final int ciVisibilityJvmInfoCacheSize; private final int ciVisibilityCoverageRootPackagesLimit; @@ -2261,6 +2263,8 @@ PROFILING_DATADOG_PROFILER_ENABLED, isDatadogProfilerSafeInCurrentEnvironment()) configProvider.getBoolean(CIVISIBILITY_CIPROVIDER_INTEGRATION_ENABLED, true); ciVisibilityRepoIndexDuplicateKeyCheckEnabled = configProvider.getBoolean(CIVISIBILITY_REPO_INDEX_DUPLICATE_KEY_CHECK_ENABLED, true); + ciVisibilityRepoIndexFollowSymlinks = + configProvider.getBoolean(CIVISIBILITY_REPO_INDEX_FOLLOW_SYMLINKS, false); ciVisibilityExecutionSettingsCacheSize = configProvider.getInteger(CIVISIBILITY_EXECUTION_SETTINGS_CACHE_SIZE, 16); ciVisibilityJvmInfoCacheSize = configProvider.getInteger(CIVISIBILITY_JVM_INFO_CACHE_SIZE, 8); @@ -3822,6 +3826,10 @@ public boolean isCiVisibilityRepoIndexDuplicateKeyCheckEnabled() { return ciVisibilityRepoIndexDuplicateKeyCheckEnabled; } + public boolean isCiVisibilityRepoIndexFollowSymlinks() { + return ciVisibilityRepoIndexFollowSymlinks; + } + public int getCiVisibilityExecutionSettingsCacheSize() { return ciVisibilityExecutionSettingsCacheSize; } From 7beb0c310e6c02b599e70e4df40e1f678e913c8f Mon Sep 17 00:00:00 2001 From: Lucas Rogerio Caetano Ferreira Date: Thu, 7 Aug 2025 15:53:53 -0500 Subject: [PATCH 02/11] Update GraalVM config to reflect TempLocationManager's new package (#9338) --- .../nativeimage/NativeImageGeneratorRunnerInstrumentation.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/dd-java-agent/instrumentation/graal/native-image/src/main/java/datadog/trace/instrumentation/graal/nativeimage/NativeImageGeneratorRunnerInstrumentation.java b/dd-java-agent/instrumentation/graal/native-image/src/main/java/datadog/trace/instrumentation/graal/nativeimage/NativeImageGeneratorRunnerInstrumentation.java index fa3a0bc92c8..382a8266c78 100644 --- a/dd-java-agent/instrumentation/graal/native-image/src/main/java/datadog/trace/instrumentation/graal/nativeimage/NativeImageGeneratorRunnerInstrumentation.java +++ b/dd-java-agent/instrumentation/graal/native-image/src/main/java/datadog/trace/instrumentation/graal/nativeimage/NativeImageGeneratorRunnerInstrumentation.java @@ -70,7 +70,6 @@ public static void onEnter(@Advice.Argument(value = 0, readOnly = false) String[ + "com.datadog.profiling.controller.openjdk.events.TimelineEvent:build_time," + "com.datadog.profiling.controller.openjdk.events.SmapEntryEvent:build_time," + "com.datadog.profiling.controller.openjdk.events.SmapEntryFactory$SmapParseErrorEvent:build_time," - + "com.datadog.profiling.controller.TempLocationManager$SingletonHolder:run_time," + "com.datadog.profiling.ddprof.JavaProfilerLoader:run_time," + "datadog.environment.JavaVirtualMachine:rerun," + "datadog.trace.agent.tooling.WeakMaps$Adapter:build_time," @@ -154,6 +153,7 @@ public static void onEnter(@Advice.Argument(value = 0, readOnly = false) String[ + "datadog.trace.logging.LogReporter:build_time," + "datadog.trace.logging.PrintStreamWrapper:build_time," + "datadog.trace.util.CollectionUtils:build_time," + + "datadog.trace.util.TempLocationManager$SingletonHolder:run_time," + "datadog.slf4j.helpers.NOPLoggerFactory:build_time," + "datadog.slf4j.helpers.SubstituteLoggerFactory:build_time," + "datadog.slf4j.impl.StaticLoggerBinder:build_time," From 579165c0462c571e07651be204e2861b2a672d09 Mon Sep 17 00:00:00 2001 From: Andrea Marziali Date: Fri, 8 Aug 2025 15:29:33 +0200 Subject: [PATCH 03/11] =?UTF-8?q?=F0=9F=8D=92=209184=20-=20Make=20rum=20in?= =?UTF-8?q?jector=20stream/writer=20more=20resilient=20to=20errors=20(#934?= =?UTF-8?q?0)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * Make rum injector stream/writer more resilient to errors (cherry picked from commit a298c600bc8210c05f671e76ade2e7f356793320) * fix tests (cherry picked from commit 62acf7c61ea89e19c1cc42a7ccb06a5d0851bd86) * fix tests (cherry picked from commit 5a32d5116a442039465ac72fb5d64684247116df) * Update dd-java-agent/agent-bootstrap/src/main/java/datadog/trace/bootstrap/instrumentation/buffer/InjectingPipeOutputStream.java Co-authored-by: Stuart McCulloch (cherry picked from commit 87d15fbde3d22f829859cee530903ff94c132363) * apply suggestions (cherry picked from commit 21afeb2cac550b9fcfeca804b785ecd362c270b4) --------- Co-authored-by: Stuart McCulloch --- .../buffer/InjectingPipeOutputStream.java | 74 ++++++++++------ .../buffer/InjectingPipeWriter.java | 85 ++++++++++++------- .../InjectingPipeOutputStreamTest.groovy | 67 +++++++++++++++ .../buffer/InjectingPipeWriterTest.groovy | 67 +++++++++++++++ 4 files changed, 240 insertions(+), 53 deletions(-) diff --git a/dd-java-agent/agent-bootstrap/src/main/java/datadog/trace/bootstrap/instrumentation/buffer/InjectingPipeOutputStream.java b/dd-java-agent/agent-bootstrap/src/main/java/datadog/trace/bootstrap/instrumentation/buffer/InjectingPipeOutputStream.java index 633db0c8d4c..8fa6e115c4d 100644 --- a/dd-java-agent/agent-bootstrap/src/main/java/datadog/trace/bootstrap/instrumentation/buffer/InjectingPipeOutputStream.java +++ b/dd-java-agent/agent-bootstrap/src/main/java/datadog/trace/bootstrap/instrumentation/buffer/InjectingPipeOutputStream.java @@ -5,16 +5,19 @@ /** * An OutputStream containing a circular buffer with a lookbehind buffer of n bytes. The first time - * that the latest n bytes matches the marker, a content is injected before. + * that the latest n bytes matches the marker, a content is injected before. In case of IOException + * thrown by the downstream, the buffer will be lost unless the error occurred when draining it. In + * this case the draining will be resumed. */ public class InjectingPipeOutputStream extends OutputStream { private final byte[] lookbehind; private int pos; - private boolean bufferFilled; + private int count; private final byte[] marker; private final byte[] contentToInject; - private boolean found = false; - private int matchingPos = 0; + private boolean filter; + private boolean wasDraining; + private int matchingPos; private final Runnable onContentInjected; private final int bulkWriteThreshold; private final OutputStream downstream; @@ -34,6 +37,11 @@ public InjectingPipeOutputStream( this.marker = marker; this.lookbehind = new byte[marker.length]; this.pos = 0; + this.count = 0; + this.matchingPos = 0; + this.wasDraining = false; + // should filter the stream to potentially inject into it. + this.filter = true; this.contentToInject = contentToInject; this.onContentInjected = onContentInjected; this.bulkWriteThreshold = marker.length * 2 - 2; @@ -41,25 +49,27 @@ public InjectingPipeOutputStream( @Override public void write(int b) throws IOException { - if (found) { + if (!filter) { + if (wasDraining) { + // continue draining + drain(); + } downstream.write(b); return; } - if (bufferFilled) { + if (count == lookbehind.length) { downstream.write(lookbehind[pos]); + } else { + count++; } lookbehind[pos] = (byte) b; pos = (pos + 1) % lookbehind.length; - if (!bufferFilled) { - bufferFilled = pos == 0; - } - if (marker[matchingPos++] == b) { if (matchingPos == marker.length) { - found = true; + filter = false; downstream.write(contentToInject); if (onContentInjected != null) { onContentInjected.run(); @@ -73,10 +83,15 @@ public void write(int b) throws IOException { @Override public void write(byte[] array, int off, int len) throws IOException { - if (found) { + if (!filter) { + if (wasDraining) { + // needs drain + drain(); + } downstream.write(array, off, len); return; } + if (len > bulkWriteThreshold) { // if the content is large enough, we can bulk write everything but the N trail and tail. // This because the buffer can already contain some byte from a previous single write. @@ -84,7 +99,7 @@ public void write(byte[] array, int off, int len) throws IOException { int idx = arrayContains(array, off, len, marker); if (idx >= 0) { // we have a full match. just write everything - found = true; + filter = false; drain(); downstream.write(array, off, idx); downstream.write(contentToInject); @@ -99,7 +114,12 @@ public void write(byte[] array, int off, int len) throws IOException { write(array[i]); } drain(); + boolean wasFiltering = filter; + + // will be reset if no errors after the following write + filter = false; downstream.write(array, off + marker.length - 1, len - bulkWriteThreshold); + filter = wasFiltering; for (int i = len - marker.length + 1; i < len; i++) { write(array[i]); } @@ -133,16 +153,19 @@ private int arrayContains(byte[] array, int off, int len, byte[] search) { } private void drain() throws IOException { - if (bufferFilled) { - for (int i = 0; i < lookbehind.length; i++) { - downstream.write(lookbehind[(pos + i) % lookbehind.length]); + if (count > 0) { + boolean wasFiltering = filter; + filter = false; + wasDraining = true; + int start = (pos - count + lookbehind.length) % lookbehind.length; + int cnt = count; + for (int i = 0; i < cnt; i++) { + downstream.write(lookbehind[(start + i) % lookbehind.length]); + count--; } - } else { - downstream.write(this.lookbehind, 0, pos); + filter = wasFiltering; + wasDraining = false; } - pos = 0; - matchingPos = 0; - bufferFilled = false; } @Override @@ -152,9 +175,12 @@ public void flush() throws IOException { @Override public void close() throws IOException { - if (!found) { - drain(); + try { + if (filter || wasDraining) { + drain(); + } + } finally { + downstream.close(); } - downstream.close(); } } diff --git a/dd-java-agent/agent-bootstrap/src/main/java/datadog/trace/bootstrap/instrumentation/buffer/InjectingPipeWriter.java b/dd-java-agent/agent-bootstrap/src/main/java/datadog/trace/bootstrap/instrumentation/buffer/InjectingPipeWriter.java index f012c04cae4..d7128e9b385 100644 --- a/dd-java-agent/agent-bootstrap/src/main/java/datadog/trace/bootstrap/instrumentation/buffer/InjectingPipeWriter.java +++ b/dd-java-agent/agent-bootstrap/src/main/java/datadog/trace/bootstrap/instrumentation/buffer/InjectingPipeWriter.java @@ -5,16 +5,19 @@ /** * A Writer containing a circular buffer with a lookbehind buffer of n bytes. The first time that - * the latest n bytes matches the marker, a content is injected before. + * the latest n bytes matches the marker, a content is injected before. In case of IOException + * thrown by the downstream, the buffer will be lost unless the error occurred when draining it. In + * this case the draining will be resumed. */ public class InjectingPipeWriter extends Writer { private final char[] lookbehind; private int pos; - private boolean bufferFilled; + private int count; private final char[] marker; private final char[] contentToInject; - private boolean found = false; - private int matchingPos = 0; + private boolean filter; + private boolean wasDraining; + private int matchingPos; private final Runnable onContentInjected; private final int bulkWriteThreshold; private final Writer downstream; @@ -34,6 +37,11 @@ public InjectingPipeWriter( this.marker = marker; this.lookbehind = new char[marker.length]; this.pos = 0; + this.count = 0; + this.matchingPos = 0; + this.wasDraining = false; + // should filter the stream to potentially inject into it. + this.filter = true; this.contentToInject = contentToInject; this.onContentInjected = onContentInjected; this.bulkWriteThreshold = marker.length * 2 - 2; @@ -41,25 +49,27 @@ public InjectingPipeWriter( @Override public void write(int c) throws IOException { - if (found) { + if (!filter) { + if (wasDraining) { + // continue draining + drain(); + } downstream.write(c); return; } - if (bufferFilled) { + if (count == lookbehind.length) { downstream.write(lookbehind[pos]); + } else { + count++; } lookbehind[pos] = (char) c; pos = (pos + 1) % lookbehind.length; - if (!bufferFilled) { - bufferFilled = pos == 0; - } - if (marker[matchingPos++] == c) { if (matchingPos == marker.length) { - found = true; + filter = false; downstream.write(contentToInject); if (onContentInjected != null) { onContentInjected.run(); @@ -71,17 +81,17 @@ public void write(int c) throws IOException { } } - @Override - public void flush() throws IOException { - downstream.flush(); - } - @Override public void write(char[] array, int off, int len) throws IOException { - if (found) { + if (!filter) { + if (wasDraining) { + // needs drain + drain(); + } downstream.write(array, off, len); return; } + if (len > bulkWriteThreshold) { // if the content is large enough, we can bulk write everything but the N trail and tail. // This because the buffer can already contain some byte from a previous single write. @@ -89,7 +99,7 @@ public void write(char[] array, int off, int len) throws IOException { int idx = arrayContains(array, off, len, marker); if (idx >= 0) { // we have a full match. just write everything - found = true; + filter = false; drain(); downstream.write(array, off, idx); downstream.write(contentToInject); @@ -104,7 +114,13 @@ public void write(char[] array, int off, int len) throws IOException { write(array[i]); } drain(); + boolean wasFiltering = filter; + + // will be reset if no errors after the following write + filter = false; downstream.write(array, off + marker.length - 1, len - bulkWriteThreshold); + filter = wasFiltering; + for (int i = len - marker.length + 1; i < len; i++) { write(array[i]); } @@ -138,23 +154,34 @@ private int arrayContains(char[] array, int off, int len, char[] search) { } private void drain() throws IOException { - if (bufferFilled) { - for (int i = 0; i < lookbehind.length; i++) { - downstream.write(lookbehind[(pos + i) % lookbehind.length]); + if (count > 0) { + boolean wasFiltering = filter; + filter = false; + wasDraining = true; + int start = (pos - count + lookbehind.length) % lookbehind.length; + int cnt = count; + for (int i = 0; i < cnt; i++) { + downstream.write(lookbehind[(start + i) % lookbehind.length]); + count--; } - } else { - downstream.write(this.lookbehind, 0, pos); + filter = wasFiltering; + wasDraining = false; } - pos = 0; - matchingPos = 0; - bufferFilled = false; + } + + @Override + public void flush() throws IOException { + downstream.flush(); } @Override public void close() throws IOException { - if (!found) { - drain(); + try { + if (filter || wasDraining) { + drain(); + } + } finally { + downstream.close(); } - downstream.close(); } } diff --git a/dd-java-agent/agent-bootstrap/src/test/groovy/datadog/trace/bootstrap/instrumentation/buffer/InjectingPipeOutputStreamTest.groovy b/dd-java-agent/agent-bootstrap/src/test/groovy/datadog/trace/bootstrap/instrumentation/buffer/InjectingPipeOutputStreamTest.groovy index 457b26577ba..9b04234ad3d 100644 --- a/dd-java-agent/agent-bootstrap/src/test/groovy/datadog/trace/bootstrap/instrumentation/buffer/InjectingPipeOutputStreamTest.groovy +++ b/dd-java-agent/agent-bootstrap/src/test/groovy/datadog/trace/bootstrap/instrumentation/buffer/InjectingPipeOutputStreamTest.groovy @@ -3,6 +3,36 @@ package datadog.trace.bootstrap.instrumentation.buffer import datadog.trace.test.util.DDSpecification class InjectingPipeOutputStreamTest extends DDSpecification { + static class GlitchedOutputStream extends FilterOutputStream { + int glitchesPos + int count + final OutputStream out + + GlitchedOutputStream(OutputStream out, int glitchesPos) { + super(out) + this.out = out + this.glitchesPos = glitchesPos + } + + @Override + void write(byte[] b, int off, int len) throws IOException { + count += len + if (count >= glitchesPos) { + glitchesPos = Integer.MAX_VALUE + throw new IOException("Glitched after $count bytes") + } + out.write(b, off, len) + } + + @Override + void write(int b) throws IOException { + if (++count == glitchesPos) { + throw new IOException("Glitched after $glitchesPos bytes") + } + out.write(b) + } + } + def 'should filter a buffer and inject if found #found'() { setup: def downstream = new ByteArrayOutputStream() @@ -20,4 +50,41 @@ class InjectingPipeOutputStreamTest extends DDSpecification { "" | "" | "" | false | "" "" | "" | "" | false | "" } + + def 'should be resilient to exceptions when writing #body'() { + setup: + def baos = new ByteArrayOutputStream() + def downstream = new GlitchedOutputStream(baos, glichesAt) + def piped = new InjectingPipeOutputStream(downstream, marker.getBytes("UTF-8"), contentToInject.getBytes("UTF-8"), null) + when: + try { + for (String line : body) { + final bytes = line.getBytes("UTF-8") + try { + piped.write(bytes) + } catch (IOException ioe) { + ioe.printStackTrace() + piped.write(bytes) + } + } + } finally { + // it can throw when draining at close + try { + piped.close() + } catch (IOException ignored) { + } + } + then: + assert baos.toByteArray() == expected.getBytes("UTF-8") + where: + body | marker | contentToInject | glichesAt | expected + // write fails after the content has been injected + ["", "", "", "", "", ""] | "" | "" | 60 | "" + // write fails before the content has been injected + ["", "", "", "", "", ""] | "" | "" | 20 | "" + // write fails after having filled the buffer. The last line is written twice + ["", "", ""] | "" | "" | 10 | "" + // expected broken since the real write happens at close (drain) being the content smaller than the buffer. And retry on close is not a common practice. Hence, we suppose loosing content + [""] | "" | "" | 3 | "= glitchesPos) { + glitchesPos = Integer.MAX_VALUE + throw new IOException("Glitched after $count bytes") + } + out.write(c, off, len) + } + + @Override + void write(int c) throws IOException { + if (++count == glitchesPos) { + throw new IOException("Glitched after $glitchesPos bytes") + } + out.write(c) + } + } + def 'should filter a buffer and inject if found #found using write'() { setup: def downstream = new StringWriter() @@ -36,4 +66,41 @@ class InjectingPipeWriterTest extends DDSpecification { "" | "" | "" | false | "" "" | "" | "" | false | "" } + + def 'should be resilient to exceptions when writing #body'() { + setup: + def writer = new StringWriter() + def downstream = new GlitchedWriter(writer, glichesAt) + def piped = new InjectingPipeWriter(downstream, marker.toCharArray(), contentToInject.toCharArray(), null) + when: + try { + for (String line : body) { + final chars = line.toCharArray() + try { + piped.write(chars) + } catch (IOException ioe) { + ioe.printStackTrace() + piped.write(chars) + } + } + } finally { + // it can throw when draining at close + try { + piped.close() + } catch (IOException ignored) { + } + } + then: + assert writer.toString() == expected + where: + body | marker | contentToInject | glichesAt | expected + // write fails after the content has been injected + ["", "", "", "", "", ""] | "" | "" | 60 | "" + // write fails before the content has been injected + ["", "", "", "", "", ""] | "" | "" | 20 | "" + // write fails after having filled the buffer. The last line is written twice + ["", "", ""] | "" | "" | 10 | "" + // expected broken since the real write happens at close (drain) being the content smaller than the buffer. And retry on close is not a common practice. Hence, we suppose loosing content + [""] | "" | "" | 3 | " Date: Fri, 8 Aug 2025 17:27:45 +0200 Subject: [PATCH 04/11] =?UTF-8?q?=F0=9F=8D=92=209323=20-=20Improve=20RUM?= =?UTF-8?q?=20injection=20matching=20and=20avoid=20truncating=20responses?= =?UTF-8?q?=20(#9342)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * Make rum injector stream/writer more resilient to errors (cherry picked from commit 36246b44abd926a1eeca076561b142f095df3aff) * fix tests (cherry picked from commit 993bf57c0e76afe074722a04ae9a7be143ca4940) * Improve RUM injection matching and avoid truncating responses (cherry picked from commit f46c8f12b733e544b2bfe04f717db831b9bac86c) * Use MH for setContentLengthLong since available from 3.1 (cherry picked from commit 13753c11242f9761f267d29c1dd191ed4f2e436e) * Fix tomcat11 tests (cherry picked from commit 4fb41299ae676519cd147e032f1f9a6475370145) * Update dd-smoke-tests/rum/src/main/groovy/datadog/smoketest/rum/AbstractRumServerSmokeTest.groovy Co-authored-by: Bruce Bujon (cherry picked from commit 4b44a389d812cf32df3fb165ec3400b780ecab52) * Update dd-smoke-tests/rum/src/main/groovy/datadog/smoketest/rum/AbstractRumServerSmokeTest.groovy Co-authored-by: Bruce Bujon (cherry picked from commit d7e1fdfa77b51c43054123cc2c80ea9da252e2b0) * remove leftover (cherry picked from commit 22e53896216386c60886aefe24e2c2f2dd4f4dbb) --------- Co-authored-by: Bruce Bujon --- .../buffer/InjectingPipeOutputStream.java | 13 ++- .../buffer/InjectingPipeWriter.java | 13 ++- .../RumHttpServletResponseWrapper.java | 109 +++++++++++++----- .../servlet3/Servlet3Advice.java | 10 +- .../servlet3/WrappedServletOutputStream.java | 7 +- .../servlet3/RumServlet.groovy | 2 +- .../JakartaServletInstrumentation.java | 13 ++- .../RumHttpServletResponseWrapper.java | 86 +++++++++----- .../servlet5/WrappedServletOutputStream.java | 7 +- .../servlet5/RumServlet.groovy | 2 +- .../rum/AbstractRumServerSmokeTest.groovy | 4 +- .../main/java/com/example/HtmlServlet.java | 29 +++-- .../src/main/java/com/example/XmlServlet.java | 21 ++-- .../main/java/com/example/HtmlServlet.java | 29 +++-- .../src/main/java/com/example/XmlServlet.java | 21 ++-- .../main/java/com/example/HtmlServlet.java | 31 +++-- .../src/main/java/com/example/XmlServlet.java | 21 ++-- 17 files changed, 268 insertions(+), 150 deletions(-) diff --git a/dd-java-agent/agent-bootstrap/src/main/java/datadog/trace/bootstrap/instrumentation/buffer/InjectingPipeOutputStream.java b/dd-java-agent/agent-bootstrap/src/main/java/datadog/trace/bootstrap/instrumentation/buffer/InjectingPipeOutputStream.java index 8fa6e115c4d..93bb8f956fe 100644 --- a/dd-java-agent/agent-bootstrap/src/main/java/datadog/trace/bootstrap/instrumentation/buffer/InjectingPipeOutputStream.java +++ b/dd-java-agent/agent-bootstrap/src/main/java/datadog/trace/bootstrap/instrumentation/buffer/InjectingPipeOutputStream.java @@ -2,6 +2,7 @@ import java.io.IOException; import java.io.OutputStream; +import javax.annotation.concurrent.NotThreadSafe; /** * An OutputStream containing a circular buffer with a lookbehind buffer of n bytes. The first time @@ -9,6 +10,7 @@ * thrown by the downstream, the buffer will be lost unless the error occurred when draining it. In * this case the draining will be resumed. */ +@NotThreadSafe public class InjectingPipeOutputStream extends OutputStream { private final byte[] lookbehind; private int pos; @@ -168,6 +170,13 @@ private void drain() throws IOException { } } + public void commit() throws IOException { + if (filter || wasDraining) { + filter = false; + drain(); + } + } + @Override public void flush() throws IOException { downstream.flush(); @@ -176,9 +185,7 @@ public void flush() throws IOException { @Override public void close() throws IOException { try { - if (filter || wasDraining) { - drain(); - } + commit(); } finally { downstream.close(); } diff --git a/dd-java-agent/agent-bootstrap/src/main/java/datadog/trace/bootstrap/instrumentation/buffer/InjectingPipeWriter.java b/dd-java-agent/agent-bootstrap/src/main/java/datadog/trace/bootstrap/instrumentation/buffer/InjectingPipeWriter.java index d7128e9b385..e435b23d132 100644 --- a/dd-java-agent/agent-bootstrap/src/main/java/datadog/trace/bootstrap/instrumentation/buffer/InjectingPipeWriter.java +++ b/dd-java-agent/agent-bootstrap/src/main/java/datadog/trace/bootstrap/instrumentation/buffer/InjectingPipeWriter.java @@ -2,6 +2,7 @@ import java.io.IOException; import java.io.Writer; +import javax.annotation.concurrent.NotThreadSafe; /** * A Writer containing a circular buffer with a lookbehind buffer of n bytes. The first time that @@ -9,6 +10,7 @@ * thrown by the downstream, the buffer will be lost unless the error occurred when draining it. In * this case the draining will be resumed. */ +@NotThreadSafe public class InjectingPipeWriter extends Writer { private final char[] lookbehind; private int pos; @@ -169,6 +171,13 @@ private void drain() throws IOException { } } + public void commit() throws IOException { + if (filter || wasDraining) { + filter = false; + drain(); + } + } + @Override public void flush() throws IOException { downstream.flush(); @@ -177,9 +186,7 @@ public void flush() throws IOException { @Override public void close() throws IOException { try { - if (filter || wasDraining) { - drain(); - } + commit(); } finally { downstream.close(); } diff --git a/dd-java-agent/instrumentation/servlet/request-3/src/main/java/datadog/trace/instrumentation/servlet3/RumHttpServletResponseWrapper.java b/dd-java-agent/instrumentation/servlet/request-3/src/main/java/datadog/trace/instrumentation/servlet3/RumHttpServletResponseWrapper.java index d7b173c6827..9ba84665901 100644 --- a/dd-java-agent/instrumentation/servlet/request-3/src/main/java/datadog/trace/instrumentation/servlet3/RumHttpServletResponseWrapper.java +++ b/dd-java-agent/instrumentation/servlet/request-3/src/main/java/datadog/trace/instrumentation/servlet3/RumHttpServletResponseWrapper.java @@ -2,18 +2,37 @@ import datadog.trace.api.rum.RumInjector; import datadog.trace.bootstrap.instrumentation.buffer.InjectingPipeWriter; +import datadog.trace.util.MethodHandles; import java.io.IOException; import java.io.PrintWriter; +import java.lang.invoke.MethodHandle; import java.nio.charset.Charset; import javax.servlet.ServletOutputStream; +import javax.servlet.ServletResponse; import javax.servlet.http.HttpServletResponse; import javax.servlet.http.HttpServletResponseWrapper; public class RumHttpServletResponseWrapper extends HttpServletResponseWrapper { private final RumInjector rumInjector; - private ServletOutputStream outputStream; + private WrappedServletOutputStream outputStream; private PrintWriter printWriter; - private boolean shouldInject = false; + private InjectingPipeWriter wrappedPipeWriter; + private boolean shouldInject = true; + + private static final MethodHandle SET_CONTENT_LENGTH_LONG = getMh("setContentLengthLong"); + + private static MethodHandle getMh(final String name) { + try { + return new MethodHandles(ServletResponse.class.getClassLoader()) + .method(ServletResponse.class, name); + } catch (Throwable ignored) { + return null; + } + } + + private static void sneakyThrow(Throwable e) throws E { + throw (E) e; + } public RumHttpServletResponseWrapper(HttpServletResponse response) { super(response); @@ -22,50 +41,68 @@ public RumHttpServletResponseWrapper(HttpServletResponse response) { @Override public ServletOutputStream getOutputStream() throws IOException { + if (outputStream != null) { + return outputStream; + } if (!shouldInject) { return super.getOutputStream(); } - if (outputStream == null) { - String encoding = getCharacterEncoding(); - if (encoding == null) { - encoding = Charset.defaultCharset().name(); - } - outputStream = - new WrappedServletOutputStream( - super.getOutputStream(), - rumInjector.getMarkerBytes(encoding), - rumInjector.getSnippetBytes(encoding), - this::onInjected); + String encoding = getCharacterEncoding(); + if (encoding == null) { + encoding = Charset.defaultCharset().name(); } + outputStream = + new WrappedServletOutputStream( + super.getOutputStream(), + rumInjector.getMarkerBytes(encoding), + rumInjector.getSnippetBytes(encoding), + this::onInjected); + return outputStream; } @Override public PrintWriter getWriter() throws IOException { - final PrintWriter delegate = super.getWriter(); - if (!shouldInject) { - return delegate; + if (printWriter != null) { + return printWriter; } - if (printWriter == null) { - printWriter = - new PrintWriter( - new InjectingPipeWriter( - delegate, - rumInjector.getMarkerChars(), - rumInjector.getSnippetChars(), - this::onInjected)); + if (!shouldInject) { + return super.getWriter(); } + wrappedPipeWriter = + new InjectingPipeWriter( + super.getWriter(), + rumInjector.getMarkerChars(), + rumInjector.getSnippetChars(), + this::onInjected); + printWriter = new PrintWriter(wrappedPipeWriter); + return printWriter; } @Override public void setContentLength(int len) { // don't set it since we don't know if we will inject + if (!shouldInject) { + super.setContentLength(len); + } + } + + @Override + public void setContentLengthLong(long len) { + if (!shouldInject && SET_CONTENT_LENGTH_LONG != null) { + try { + SET_CONTENT_LENGTH_LONG.invoke(getResponse(), len); + } catch (Throwable t) { + sneakyThrow(t); + } + } } @Override public void reset() { this.outputStream = null; + this.wrappedPipeWriter = null; this.printWriter = null; this.shouldInject = false; super.reset(); @@ -74,8 +111,8 @@ public void reset() { @Override public void resetBuffer() { this.outputStream = null; + this.wrappedPipeWriter = null; this.printWriter = null; - this.shouldInject = false; super.resetBuffer(); } @@ -89,7 +126,27 @@ public void onInjected() { @Override public void setContentType(String type) { - shouldInject = type != null && type.contains("text/html"); + if (shouldInject) { + shouldInject = type != null && type.contains("text/html"); + } + if (!shouldInject) { + commit(); + } super.setContentType(type); } + + public void commit() { + if (wrappedPipeWriter != null) { + try { + wrappedPipeWriter.commit(); + } catch (Throwable ignored) { + } + } + if (outputStream != null) { + try { + outputStream.commit(); + } catch (Throwable ignored) { + } + } + } } diff --git a/dd-java-agent/instrumentation/servlet/request-3/src/main/java/datadog/trace/instrumentation/servlet3/Servlet3Advice.java b/dd-java-agent/instrumentation/servlet/request-3/src/main/java/datadog/trace/instrumentation/servlet3/Servlet3Advice.java index 134d1f28229..3b1adba0938 100644 --- a/dd-java-agent/instrumentation/servlet/request-3/src/main/java/datadog/trace/instrumentation/servlet3/Servlet3Advice.java +++ b/dd-java-agent/instrumentation/servlet/request-3/src/main/java/datadog/trace/instrumentation/servlet3/Servlet3Advice.java @@ -35,7 +35,8 @@ public static boolean onEnter( @Advice.Argument(value = 1, readOnly = false) ServletResponse response, @Advice.Local("isDispatch") boolean isDispatch, @Advice.Local("finishSpan") boolean finishSpan, - @Advice.Local("contextScope") ContextScope scope) { + @Advice.Local("contextScope") ContextScope scope, + @Advice.Local("rumServletWrapper") RumHttpServletResponseWrapper rumServletWrapper) { final boolean invalidRequest = !(request instanceof HttpServletRequest) || !(response instanceof HttpServletResponse); if (invalidRequest) { @@ -47,7 +48,8 @@ public static boolean onEnter( if (RumInjector.get().isEnabled() && httpServletRequest.getAttribute(DD_RUM_INJECTED) == null) { httpServletRequest.setAttribute(DD_RUM_INJECTED, Boolean.TRUE); - httpServletResponse = new RumHttpServletResponseWrapper(httpServletResponse); + rumServletWrapper = new RumHttpServletResponseWrapper(httpServletResponse); + httpServletResponse = rumServletWrapper; response = httpServletResponse; } @@ -108,7 +110,11 @@ public static void stopSpan( @Advice.Local("contextScope") final ContextScope scope, @Advice.Local("isDispatch") boolean isDispatch, @Advice.Local("finishSpan") boolean finishSpan, + @Advice.Local("rumServletWrapper") RumHttpServletResponseWrapper rumServletWrapper, @Advice.Thrown final Throwable throwable) { + if (rumServletWrapper != null) { + rumServletWrapper.commit(); + } // Set user.principal regardless of who created this span. final Object spanAttr = request.getAttribute(DD_SPAN_ATTRIBUTE); if (Config.get().isServletPrincipalEnabled() diff --git a/dd-java-agent/instrumentation/servlet/request-3/src/main/java/datadog/trace/instrumentation/servlet3/WrappedServletOutputStream.java b/dd-java-agent/instrumentation/servlet/request-3/src/main/java/datadog/trace/instrumentation/servlet3/WrappedServletOutputStream.java index d22d7899836..951eceb5db8 100644 --- a/dd-java-agent/instrumentation/servlet/request-3/src/main/java/datadog/trace/instrumentation/servlet3/WrappedServletOutputStream.java +++ b/dd-java-agent/instrumentation/servlet/request-3/src/main/java/datadog/trace/instrumentation/servlet3/WrappedServletOutputStream.java @@ -3,13 +3,12 @@ import datadog.trace.bootstrap.instrumentation.buffer.InjectingPipeOutputStream; import datadog.trace.util.MethodHandles; import java.io.IOException; -import java.io.OutputStream; import java.lang.invoke.MethodHandle; import javax.servlet.ServletOutputStream; import javax.servlet.WriteListener; public class WrappedServletOutputStream extends ServletOutputStream { - private final OutputStream filtered; + private final InjectingPipeOutputStream filtered; private final ServletOutputStream delegate; private static final MethodHandle IS_READY_MH = getMh("isReady"); @@ -83,4 +82,8 @@ public void setWriteListener(WriteListener writeListener) { sneakyThrow(e); } } + + public void commit() throws IOException { + filtered.commit(); + } } diff --git a/dd-java-agent/instrumentation/servlet/request-3/src/testFixtures/groovy/datadog/trace/instrumentation/servlet3/RumServlet.groovy b/dd-java-agent/instrumentation/servlet/request-3/src/testFixtures/groovy/datadog/trace/instrumentation/servlet3/RumServlet.groovy index 127950fc385..dd68e6223dc 100644 --- a/dd-java-agent/instrumentation/servlet/request-3/src/testFixtures/groovy/datadog/trace/instrumentation/servlet3/RumServlet.groovy +++ b/dd-java-agent/instrumentation/servlet/request-3/src/testFixtures/groovy/datadog/trace/instrumentation/servlet3/RumServlet.groovy @@ -14,8 +14,8 @@ class RumServlet extends HttpServlet { @Override protected void service(HttpServletRequest req, HttpServletResponse resp) throws ServletException, IOException { - resp.setContentType(mimeType) try (def writer = resp.getWriter()) { + resp.setContentType(mimeType) writer.println("\n" + "\n" + "\n" + diff --git a/dd-java-agent/instrumentation/servlet/request-5/src/main/java/datadog/trace/instrumentation/servlet5/JakartaServletInstrumentation.java b/dd-java-agent/instrumentation/servlet/request-5/src/main/java/datadog/trace/instrumentation/servlet5/JakartaServletInstrumentation.java index 6ef4940a41e..91e3c989019 100644 --- a/dd-java-agent/instrumentation/servlet/request-5/src/main/java/datadog/trace/instrumentation/servlet5/JakartaServletInstrumentation.java +++ b/dd-java-agent/instrumentation/servlet/request-5/src/main/java/datadog/trace/instrumentation/servlet5/JakartaServletInstrumentation.java @@ -68,7 +68,8 @@ public static class JakartaServletAdvice { @Advice.OnMethodEnter(suppress = Throwable.class) public static AgentSpan before( @Advice.Argument(0) final ServletRequest request, - @Advice.Argument(value = 1, readOnly = false) ServletResponse response) { + @Advice.Argument(value = 1, readOnly = false) ServletResponse response, + @Advice.Local("rumServletWrapper") RumHttpServletResponseWrapper rumServletWrapper) { if (!(request instanceof HttpServletRequest)) { return null; } @@ -79,7 +80,8 @@ public static AgentSpan before( if (RumInjector.get().isEnabled() && httpServletRequest.getAttribute(DD_RUM_INJECTED) == null) { httpServletRequest.setAttribute(DD_RUM_INJECTED, Boolean.TRUE); - response = new RumHttpServletResponseWrapper((HttpServletResponse) response); + rumServletWrapper = new RumHttpServletResponseWrapper((HttpServletResponse) response); + response = rumServletWrapper; } } @@ -95,10 +97,15 @@ public static AgentSpan before( @Advice.OnMethodExit(onThrowable = Throwable.class, suppress = Throwable.class) public static void after( - @Advice.Enter final AgentSpan span, @Advice.Argument(0) final ServletRequest request) { + @Advice.Enter final AgentSpan span, + @Advice.Argument(0) final ServletRequest request, + @Advice.Local("rumServletWrapper") RumHttpServletResponseWrapper rumServletWrapper) { if (span == null) { return; } + if (rumServletWrapper != null) { + rumServletWrapper.commit(); + } CallDepthThreadLocalMap.reset(HttpServletRequest.class); final HttpServletRequest httpServletRequest = diff --git a/dd-java-agent/instrumentation/servlet/request-5/src/main/java/datadog/trace/instrumentation/servlet5/RumHttpServletResponseWrapper.java b/dd-java-agent/instrumentation/servlet/request-5/src/main/java/datadog/trace/instrumentation/servlet5/RumHttpServletResponseWrapper.java index ce86c11e863..4b846b3c2db 100644 --- a/dd-java-agent/instrumentation/servlet/request-5/src/main/java/datadog/trace/instrumentation/servlet5/RumHttpServletResponseWrapper.java +++ b/dd-java-agent/instrumentation/servlet/request-5/src/main/java/datadog/trace/instrumentation/servlet5/RumHttpServletResponseWrapper.java @@ -11,9 +11,10 @@ public class RumHttpServletResponseWrapper extends HttpServletResponseWrapper { private final RumInjector rumInjector; - private ServletOutputStream outputStream; + private WrappedServletOutputStream outputStream; + private InjectingPipeWriter wrappedPipeWriter; private PrintWriter printWriter; - private boolean shouldInject = false; + private boolean shouldInject = true; public RumHttpServletResponseWrapper(HttpServletResponse response) { super(response); @@ -22,50 +23,63 @@ public RumHttpServletResponseWrapper(HttpServletResponse response) { @Override public ServletOutputStream getOutputStream() throws IOException { + if (outputStream != null) { + return outputStream; + } if (!shouldInject) { return super.getOutputStream(); } - if (outputStream == null) { - String encoding = getCharacterEncoding(); - if (encoding == null) { - encoding = Charset.defaultCharset().name(); - } - outputStream = - new WrappedServletOutputStream( - super.getOutputStream(), - rumInjector.getMarkerBytes(encoding), - rumInjector.getSnippetBytes(encoding), - this::onInjected); + String encoding = getCharacterEncoding(); + if (encoding == null) { + encoding = Charset.defaultCharset().name(); } + outputStream = + new WrappedServletOutputStream( + super.getOutputStream(), + rumInjector.getMarkerBytes(encoding), + rumInjector.getSnippetBytes(encoding), + this::onInjected); return outputStream; } @Override public PrintWriter getWriter() throws IOException { - final PrintWriter delegate = super.getWriter(); - if (!shouldInject) { - return delegate; + if (printWriter != null) { + return printWriter; } - if (printWriter == null) { - printWriter = - new PrintWriter( - new InjectingPipeWriter( - delegate, - rumInjector.getMarkerChars(), - rumInjector.getSnippetChars(), - this::onInjected)); + if (!shouldInject) { + return super.getWriter(); } + wrappedPipeWriter = + new InjectingPipeWriter( + super.getWriter(), + rumInjector.getMarkerChars(), + rumInjector.getSnippetChars(), + this::onInjected); + printWriter = new PrintWriter(wrappedPipeWriter); + return printWriter; } @Override public void setContentLength(int len) { // don't set it since we don't know if we will inject + if (!shouldInject) { + super.setContentLength(len); + } + } + + @Override + public void setContentLengthLong(long len) { + if (!shouldInject) { + super.setContentLengthLong(len); + } } @Override public void reset() { this.outputStream = null; + this.wrappedPipeWriter = null; this.printWriter = null; this.shouldInject = false; super.reset(); @@ -74,8 +88,8 @@ public void reset() { @Override public void resetBuffer() { this.outputStream = null; + this.wrappedPipeWriter = null; this.printWriter = null; - this.shouldInject = false; super.resetBuffer(); } @@ -89,7 +103,27 @@ public void onInjected() { @Override public void setContentType(String type) { - shouldInject = type != null && type.contains("text/html"); + if (shouldInject) { + shouldInject = type != null && type.contains("text/html"); + } + if (!shouldInject) { + commit(); + } super.setContentType(type); } + + public void commit() { + if (wrappedPipeWriter != null) { + try { + wrappedPipeWriter.commit(); + } catch (Throwable ignored) { + } + } + if (outputStream != null) { + try { + outputStream.commit(); + } catch (Throwable ignored) { + } + } + } } diff --git a/dd-java-agent/instrumentation/servlet/request-5/src/main/java/datadog/trace/instrumentation/servlet5/WrappedServletOutputStream.java b/dd-java-agent/instrumentation/servlet/request-5/src/main/java/datadog/trace/instrumentation/servlet5/WrappedServletOutputStream.java index 2c43af795f8..f2338ee1a71 100644 --- a/dd-java-agent/instrumentation/servlet/request-5/src/main/java/datadog/trace/instrumentation/servlet5/WrappedServletOutputStream.java +++ b/dd-java-agent/instrumentation/servlet/request-5/src/main/java/datadog/trace/instrumentation/servlet5/WrappedServletOutputStream.java @@ -4,10 +4,9 @@ import jakarta.servlet.ServletOutputStream; import jakarta.servlet.WriteListener; import java.io.IOException; -import java.io.OutputStream; public class WrappedServletOutputStream extends ServletOutputStream { - private final OutputStream filtered; + private final InjectingPipeOutputStream filtered; private final ServletOutputStream delegate; public WrappedServletOutputStream( @@ -31,6 +30,10 @@ public void write(byte[] b, int off, int len) throws IOException { filtered.write(b, off, len); } + public void commit() throws IOException { + filtered.commit(); + } + @Override public void flush() throws IOException { filtered.flush(); diff --git a/dd-java-agent/instrumentation/servlet/request-5/src/testFixtures/groovy/datadog/trace/instrumentation/servlet5/RumServlet.groovy b/dd-java-agent/instrumentation/servlet/request-5/src/testFixtures/groovy/datadog/trace/instrumentation/servlet5/RumServlet.groovy index 1fa57d02da8..af2851fda83 100644 --- a/dd-java-agent/instrumentation/servlet/request-5/src/testFixtures/groovy/datadog/trace/instrumentation/servlet5/RumServlet.groovy +++ b/dd-java-agent/instrumentation/servlet/request-5/src/testFixtures/groovy/datadog/trace/instrumentation/servlet5/RumServlet.groovy @@ -14,8 +14,8 @@ class RumServlet extends HttpServlet { @Override protected void service(HttpServletRequest req, HttpServletResponse resp) throws ServletException, IOException { - resp.setContentType(mimeType) try (def writer = resp.getWriter()) { + resp.setContentType(mimeType) writer.println("\n" + "\n" + "\n" + diff --git a/dd-smoke-tests/rum/src/main/groovy/datadog/smoketest/rum/AbstractRumServerSmokeTest.groovy b/dd-smoke-tests/rum/src/main/groovy/datadog/smoketest/rum/AbstractRumServerSmokeTest.groovy index d9ece96fde9..a4b79227e36 100644 --- a/dd-smoke-tests/rum/src/main/groovy/datadog/smoketest/rum/AbstractRumServerSmokeTest.groovy +++ b/dd-smoke-tests/rum/src/main/groovy/datadog/smoketest/rum/AbstractRumServerSmokeTest.groovy @@ -50,11 +50,13 @@ class AbstractRumServerSmokeTest extends AbstractServerSmokeTest { assert response.header('x-datadog-rum-injected') == '1': 'RUM injected header missing' def content = response.body().string() assert content.contains('https://www.datadoghq-browser-agent.com'): 'RUM script not injected' + assert content.endsWith(''): 'Response not fully flushed' } static void assertRumNotInjected(Response response) { assert response.header('x-datadog-rum-injected') == null: 'RUM header unexpectedly injected' - def content = response.body().toString() + def content = response.body().string() assert !content.contains('https://www.datadoghq-browser-agent.com'): 'RUM script unexpectedly injected' + assert content.endsWith(''): 'Response not fully flushed' } } diff --git a/dd-smoke-tests/rum/tomcat-10/src/main/java/com/example/HtmlServlet.java b/dd-smoke-tests/rum/tomcat-10/src/main/java/com/example/HtmlServlet.java index 31a3c0825d9..0940634b377 100644 --- a/dd-smoke-tests/rum/tomcat-10/src/main/java/com/example/HtmlServlet.java +++ b/dd-smoke-tests/rum/tomcat-10/src/main/java/com/example/HtmlServlet.java @@ -9,21 +9,20 @@ public class HtmlServlet extends HttpServlet { @Override protected void doGet(HttpServletRequest req, HttpServletResponse resp) throws IOException { + final PrintWriter writer = resp.getWriter(); resp.setContentType("text/html;charset=UTF-8"); - try (final PrintWriter writer = resp.getWriter()) { - writer.write( - "" - + "" - + "" - + " " - + " " - + " Hello Servlet" - + "" - + "" - + "

Hello from Tomcat 9 Servlet!

" - + "

This is a demo HTML page served by Java servlet.

" - + "" - + ""); - } + writer.write( + "" + + "" + + "" + + " " + + " " + + " Hello Servlet" + + "" + + "" + + "

Hello from Tomcat 9 Servlet!

" + + "

This is a demo HTML page served by Java servlet.

" + + "" + + ""); } } diff --git a/dd-smoke-tests/rum/tomcat-10/src/main/java/com/example/XmlServlet.java b/dd-smoke-tests/rum/tomcat-10/src/main/java/com/example/XmlServlet.java index 8f8399bb408..43cf679cc09 100644 --- a/dd-smoke-tests/rum/tomcat-10/src/main/java/com/example/XmlServlet.java +++ b/dd-smoke-tests/rum/tomcat-10/src/main/java/com/example/XmlServlet.java @@ -9,17 +9,16 @@ public class XmlServlet extends HttpServlet { @Override protected void doGet(HttpServletRequest req, HttpServletResponse resp) throws IOException { + final PrintWriter out = resp.getWriter(); resp.setContentType("application/xml;charset=UTF-8"); - try (PrintWriter out = resp.getWriter()) { - out.println(""); - out.println(""); - out.println(" success"); - out.println(" RUM injection test"); - out.println(" "); - out.println(" Test Item 1"); - out.println(" Test Item 2"); - out.println(" "); - out.println(""); - } + out.println(""); + out.println(""); + out.println(" success"); + out.println(" RUM injection test"); + out.println(" "); + out.println(" Test Item 1"); + out.println(" Test Item 2"); + out.println(" "); + out.print(""); } } diff --git a/dd-smoke-tests/rum/tomcat-11/src/main/java/com/example/HtmlServlet.java b/dd-smoke-tests/rum/tomcat-11/src/main/java/com/example/HtmlServlet.java index 31a3c0825d9..0940634b377 100644 --- a/dd-smoke-tests/rum/tomcat-11/src/main/java/com/example/HtmlServlet.java +++ b/dd-smoke-tests/rum/tomcat-11/src/main/java/com/example/HtmlServlet.java @@ -9,21 +9,20 @@ public class HtmlServlet extends HttpServlet { @Override protected void doGet(HttpServletRequest req, HttpServletResponse resp) throws IOException { + final PrintWriter writer = resp.getWriter(); resp.setContentType("text/html;charset=UTF-8"); - try (final PrintWriter writer = resp.getWriter()) { - writer.write( - "" - + "" - + "" - + " " - + " " - + " Hello Servlet" - + "" - + "" - + "

Hello from Tomcat 9 Servlet!

" - + "

This is a demo HTML page served by Java servlet.

" - + "" - + ""); - } + writer.write( + "" + + "" + + "" + + " " + + " " + + " Hello Servlet" + + "" + + "" + + "

Hello from Tomcat 9 Servlet!

" + + "

This is a demo HTML page served by Java servlet.

" + + "" + + ""); } } diff --git a/dd-smoke-tests/rum/tomcat-11/src/main/java/com/example/XmlServlet.java b/dd-smoke-tests/rum/tomcat-11/src/main/java/com/example/XmlServlet.java index 8f8399bb408..43cf679cc09 100644 --- a/dd-smoke-tests/rum/tomcat-11/src/main/java/com/example/XmlServlet.java +++ b/dd-smoke-tests/rum/tomcat-11/src/main/java/com/example/XmlServlet.java @@ -9,17 +9,16 @@ public class XmlServlet extends HttpServlet { @Override protected void doGet(HttpServletRequest req, HttpServletResponse resp) throws IOException { + final PrintWriter out = resp.getWriter(); resp.setContentType("application/xml;charset=UTF-8"); - try (PrintWriter out = resp.getWriter()) { - out.println(""); - out.println(""); - out.println(" success"); - out.println(" RUM injection test"); - out.println(" "); - out.println(" Test Item 1"); - out.println(" Test Item 2"); - out.println(" "); - out.println(""); - } + out.println(""); + out.println(""); + out.println(" success"); + out.println(" RUM injection test"); + out.println(" "); + out.println(" Test Item 1"); + out.println(" Test Item 2"); + out.println(" "); + out.print(""); } } diff --git a/dd-smoke-tests/rum/tomcat-9/src/main/java/com/example/HtmlServlet.java b/dd-smoke-tests/rum/tomcat-9/src/main/java/com/example/HtmlServlet.java index ed7283ec81e..90f13e7317f 100644 --- a/dd-smoke-tests/rum/tomcat-9/src/main/java/com/example/HtmlServlet.java +++ b/dd-smoke-tests/rum/tomcat-9/src/main/java/com/example/HtmlServlet.java @@ -1,7 +1,6 @@ package com.example; import java.io.IOException; -import java.io.PrintWriter; import javax.servlet.http.HttpServlet; import javax.servlet.http.HttpServletRequest; import javax.servlet.http.HttpServletResponse; @@ -9,21 +8,19 @@ public class HtmlServlet extends HttpServlet { @Override protected void doGet(HttpServletRequest req, HttpServletResponse resp) throws IOException { - resp.setContentType("text/html;charset=UTF-8"); - try (final PrintWriter writer = resp.getWriter()) { - writer.write( - "" - + "" - + "" - + " " - + " " - + " Hello Servlet" - + "" - + "" - + "

Hello from Tomcat 9 Servlet!

" - + "

This is a demo HTML page served by Java servlet.

" - + "" - + ""); - } + resp.getWriter() + .write( + "" + + "" + + "" + + " " + + " " + + " Hello Servlet" + + "" + + "" + + "

Hello from Tomcat 9 Servlet!

" + + "

This is a demo HTML page served by Java servlet.

" + + "" + + ""); } } diff --git a/dd-smoke-tests/rum/tomcat-9/src/main/java/com/example/XmlServlet.java b/dd-smoke-tests/rum/tomcat-9/src/main/java/com/example/XmlServlet.java index 15bb275acac..8f7a02b7a6f 100644 --- a/dd-smoke-tests/rum/tomcat-9/src/main/java/com/example/XmlServlet.java +++ b/dd-smoke-tests/rum/tomcat-9/src/main/java/com/example/XmlServlet.java @@ -10,16 +10,15 @@ public class XmlServlet extends HttpServlet { @Override protected void doGet(HttpServletRequest req, HttpServletResponse resp) throws IOException { resp.setContentType("application/xml;charset=UTF-8"); - try (PrintWriter out = resp.getWriter()) { - out.println(""); - out.println(""); - out.println(" success"); - out.println(" RUM injection test"); - out.println(" "); - out.println(" Test Item 1"); - out.println(" Test Item 2"); - out.println(" "); - out.println(""); - } + final PrintWriter out = resp.getWriter(); + out.println(""); + out.println(""); + out.println(" success"); + out.println(" RUM injection test"); + out.println(" "); + out.println(" Test Item 1"); + out.println(" Test Item 2"); + out.println(" "); + out.print(""); } } From 04771b68f9f04806cdcb5c60d82e8c4159c117ac Mon Sep 17 00:00:00 2001 From: Andrea Marziali Date: Fri, 8 Aug 2025 18:16:26 +0200 Subject: [PATCH 05/11] =?UTF-8?q?=F0=9F=8D=92=209333=20-=20Support=20async?= =?UTF-8?q?=20servlet=20for=20RUM=20injection=20(#9343)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * Support async servlet for RUM injection (cherry picked from commit 736260f87cb24170f73f6c211f7f67cf43d45d56) * Move isRumEnabled to instrumenter config (cherry picked from commit 54d6a7ec250ffb83d09336bf4fd9853e4b310b0c) --- .../buffer/InjectingPipeOutputStream.java | 5 +- .../buffer/InjectingPipeWriter.java | 5 +- .../RumAsyncContextInstrumentation.java | 64 +++++++++++++++++++ .../RumHttpServletRequestWrapper.java | 45 +++++++++++++ .../RumHttpServletResponseWrapper.java | 11 ++++ .../servlet3/Servlet3Advice.java | 15 +++-- .../servlet3/Servlet3Instrumentation.java | 1 + .../servlet3/WrappedServletOutputStream.java | 4 ++ .../JakartaServletInstrumentation.java | 21 ++++-- .../RumAsyncContextInstrumentation.java | 64 +++++++++++++++++++ .../RumHttpServletRequestWrapper.java | 45 +++++++++++++ .../RumHttpServletResponseWrapper.java | 11 ++++ .../servlet5/WrappedServletOutputStream.java | 4 ++ .../servlet5/AsyncRumServlet.groovy | 58 +++++++++++++++++ .../groovy/TomcatServletTest.groovy | 11 ++++ .../rum/AbstractRumServerSmokeTest.groovy | 6 +- .../java/com/example/HtmlAsyncServlet.java | 46 +++++++++++++ .../src/main/java/com/example/Main.java | 5 ++ .../java/com/example/HtmlAsyncServlet.java | 46 +++++++++++++ .../src/main/java/com/example/Main.java | 5 ++ .../java/com/example/HtmlAsyncServlet.java | 46 +++++++++++++ .../src/main/java/com/example/Main.java | 5 ++ .../main/java/datadog/trace/api/Config.java | 12 +--- .../datadog/trace/api/InstrumenterConfig.java | 12 +++- .../datadog/trace/api/rum/RumInjector.java | 8 ++- .../trace/api/rum/RumInjectorTest.groovy | 16 +++-- 26 files changed, 534 insertions(+), 37 deletions(-) create mode 100644 dd-java-agent/instrumentation/servlet/request-3/src/main/java/datadog/trace/instrumentation/servlet3/RumAsyncContextInstrumentation.java create mode 100644 dd-java-agent/instrumentation/servlet/request-3/src/main/java/datadog/trace/instrumentation/servlet3/RumHttpServletRequestWrapper.java create mode 100644 dd-java-agent/instrumentation/servlet/request-5/src/main/java/datadog/trace/instrumentation/servlet5/RumAsyncContextInstrumentation.java create mode 100644 dd-java-agent/instrumentation/servlet/request-5/src/main/java/datadog/trace/instrumentation/servlet5/RumHttpServletRequestWrapper.java create mode 100644 dd-java-agent/instrumentation/servlet/request-5/src/testFixtures/groovy/datadog/trace/instrumentation/servlet5/AsyncRumServlet.groovy create mode 100644 dd-smoke-tests/rum/tomcat-10/src/main/java/com/example/HtmlAsyncServlet.java create mode 100644 dd-smoke-tests/rum/tomcat-11/src/main/java/com/example/HtmlAsyncServlet.java create mode 100644 dd-smoke-tests/rum/tomcat-9/src/main/java/com/example/HtmlAsyncServlet.java diff --git a/dd-java-agent/agent-bootstrap/src/main/java/datadog/trace/bootstrap/instrumentation/buffer/InjectingPipeOutputStream.java b/dd-java-agent/agent-bootstrap/src/main/java/datadog/trace/bootstrap/instrumentation/buffer/InjectingPipeOutputStream.java index 93bb8f956fe..8f91a8e38cd 100644 --- a/dd-java-agent/agent-bootstrap/src/main/java/datadog/trace/bootstrap/instrumentation/buffer/InjectingPipeOutputStream.java +++ b/dd-java-agent/agent-bootstrap/src/main/java/datadog/trace/bootstrap/instrumentation/buffer/InjectingPipeOutputStream.java @@ -172,7 +172,6 @@ private void drain() throws IOException { public void commit() throws IOException { if (filter || wasDraining) { - filter = false; drain(); } } @@ -190,4 +189,8 @@ public void close() throws IOException { downstream.close(); } } + + public void setFilter(boolean filter) { + this.filter = filter; + } } diff --git a/dd-java-agent/agent-bootstrap/src/main/java/datadog/trace/bootstrap/instrumentation/buffer/InjectingPipeWriter.java b/dd-java-agent/agent-bootstrap/src/main/java/datadog/trace/bootstrap/instrumentation/buffer/InjectingPipeWriter.java index e435b23d132..7a3d4a75f19 100644 --- a/dd-java-agent/agent-bootstrap/src/main/java/datadog/trace/bootstrap/instrumentation/buffer/InjectingPipeWriter.java +++ b/dd-java-agent/agent-bootstrap/src/main/java/datadog/trace/bootstrap/instrumentation/buffer/InjectingPipeWriter.java @@ -173,7 +173,6 @@ private void drain() throws IOException { public void commit() throws IOException { if (filter || wasDraining) { - filter = false; drain(); } } @@ -191,4 +190,8 @@ public void close() throws IOException { downstream.close(); } } + + public void setFilter(boolean filter) { + this.filter = filter; + } } diff --git a/dd-java-agent/instrumentation/servlet/request-3/src/main/java/datadog/trace/instrumentation/servlet3/RumAsyncContextInstrumentation.java b/dd-java-agent/instrumentation/servlet/request-3/src/main/java/datadog/trace/instrumentation/servlet3/RumAsyncContextInstrumentation.java new file mode 100644 index 00000000000..c6b420fdc65 --- /dev/null +++ b/dd-java-agent/instrumentation/servlet/request-3/src/main/java/datadog/trace/instrumentation/servlet3/RumAsyncContextInstrumentation.java @@ -0,0 +1,64 @@ +package datadog.trace.instrumentation.servlet3; + +import static datadog.trace.agent.tooling.bytebuddy.matcher.HierarchyMatchers.implementsInterface; +import static datadog.trace.agent.tooling.bytebuddy.matcher.NameMatchers.named; +import static datadog.trace.agent.tooling.bytebuddy.matcher.NameMatchers.namedOneOf; +import static datadog.trace.bootstrap.instrumentation.decorator.HttpServerDecorator.DD_RUM_INJECTED; +import static net.bytebuddy.matcher.ElementMatchers.isMethod; + +import com.google.auto.service.AutoService; +import datadog.trace.agent.tooling.Instrumenter; +import datadog.trace.agent.tooling.InstrumenterModule; +import datadog.trace.api.InstrumenterConfig; +import javax.servlet.AsyncContext; +import net.bytebuddy.asm.Advice; +import net.bytebuddy.description.type.TypeDescription; +import net.bytebuddy.matcher.ElementMatcher; + +@AutoService(InstrumenterModule.class) +public class RumAsyncContextInstrumentation extends InstrumenterModule.Tracing + implements Instrumenter.ForTypeHierarchy, Instrumenter.HasMethodAdvice { + + public RumAsyncContextInstrumentation() { + super("servlet", "servlet-3", "servlet-3-async-context"); + } + + @Override + public String hierarchyMarkerType() { + return "javax.servlet.AsyncContext"; + } + + @Override + public String[] helperClassNames() { + return new String[] { + packageName + ".RumHttpServletResponseWrapper", packageName + ".WrappedServletOutputStream", + }; + } + + @Override + public ElementMatcher hierarchyMatcher() { + return implementsInterface(named(hierarchyMarkerType())); + } + + @Override + public boolean isEnabled() { + return super.isEnabled() && InstrumenterConfig.get().isRumEnabled(); + } + + @Override + public void methodAdvice(MethodTransformer transformer) { + transformer.applyAdvice( + isMethod().and(namedOneOf("complete", "dispatch")), getClass().getName() + "$CommitAdvice"); + } + + public static class CommitAdvice { + @Advice.OnMethodEnter(suppress = Throwable.class) + public static void commitRumBuffer(@Advice.This final AsyncContext asyncContext) { + final Object maybeRumWrappedResponse = + asyncContext.getRequest().getAttribute(DD_RUM_INJECTED); + if (maybeRumWrappedResponse instanceof RumHttpServletResponseWrapper) { + ((RumHttpServletResponseWrapper) maybeRumWrappedResponse).commit(); + } + } + } +} diff --git a/dd-java-agent/instrumentation/servlet/request-3/src/main/java/datadog/trace/instrumentation/servlet3/RumHttpServletRequestWrapper.java b/dd-java-agent/instrumentation/servlet/request-3/src/main/java/datadog/trace/instrumentation/servlet3/RumHttpServletRequestWrapper.java new file mode 100644 index 00000000000..e0b4240097e --- /dev/null +++ b/dd-java-agent/instrumentation/servlet/request-3/src/main/java/datadog/trace/instrumentation/servlet3/RumHttpServletRequestWrapper.java @@ -0,0 +1,45 @@ +package datadog.trace.instrumentation.servlet3; + +import static datadog.trace.bootstrap.instrumentation.decorator.HttpServerDecorator.DD_RUM_INJECTED; + +import javax.servlet.AsyncContext; +import javax.servlet.ServletRequest; +import javax.servlet.ServletResponse; +import javax.servlet.http.HttpServletRequest; +import javax.servlet.http.HttpServletRequestWrapper; +import javax.servlet.http.HttpServletResponse; + +public class RumHttpServletRequestWrapper extends HttpServletRequestWrapper { + + private final HttpServletResponse response; + + public RumHttpServletRequestWrapper( + final HttpServletRequest request, final HttpServletResponse response) { + super(request); + this.response = response; + } + + @Override + public AsyncContext startAsync() throws IllegalStateException { + // need to hide this method otherwise we cannot control the wrapped response used asynchronously + return startAsync(getRequest(), response); + } + + @Override + public AsyncContext startAsync(ServletRequest servletRequest, ServletResponse servletResponse) + throws IllegalStateException { + // deactivate the previous wrapper + final Object maybeRumWrappedResponse = (servletRequest.getAttribute(DD_RUM_INJECTED)); + if (maybeRumWrappedResponse instanceof RumHttpServletResponseWrapper) { + ((RumHttpServletResponseWrapper) maybeRumWrappedResponse).commit(); + ((RumHttpServletResponseWrapper) maybeRumWrappedResponse).stopFiltering(); + } + ServletResponse actualResponse = servletResponse; + // rewrap it + if (servletResponse instanceof HttpServletResponse) { + actualResponse = new RumHttpServletResponseWrapper((HttpServletResponse) servletResponse); + servletRequest.setAttribute(DD_RUM_INJECTED, actualResponse); + } + return super.startAsync(servletRequest, actualResponse); + } +} diff --git a/dd-java-agent/instrumentation/servlet/request-3/src/main/java/datadog/trace/instrumentation/servlet3/RumHttpServletResponseWrapper.java b/dd-java-agent/instrumentation/servlet/request-3/src/main/java/datadog/trace/instrumentation/servlet3/RumHttpServletResponseWrapper.java index 9ba84665901..a5defc56dfe 100644 --- a/dd-java-agent/instrumentation/servlet/request-3/src/main/java/datadog/trace/instrumentation/servlet3/RumHttpServletResponseWrapper.java +++ b/dd-java-agent/instrumentation/servlet/request-3/src/main/java/datadog/trace/instrumentation/servlet3/RumHttpServletResponseWrapper.java @@ -131,6 +131,7 @@ public void setContentType(String type) { } if (!shouldInject) { commit(); + stopFiltering(); } super.setContentType(type); } @@ -149,4 +150,14 @@ public void commit() { } } } + + public void stopFiltering() { + shouldInject = false; + if (wrappedPipeWriter != null) { + wrappedPipeWriter.setFilter(false); + } + if (outputStream != null) { + outputStream.setFilter(false); + } + } } diff --git a/dd-java-agent/instrumentation/servlet/request-3/src/main/java/datadog/trace/instrumentation/servlet3/Servlet3Advice.java b/dd-java-agent/instrumentation/servlet/request-3/src/main/java/datadog/trace/instrumentation/servlet3/Servlet3Advice.java index 3b1adba0938..96755749047 100644 --- a/dd-java-agent/instrumentation/servlet/request-3/src/main/java/datadog/trace/instrumentation/servlet3/Servlet3Advice.java +++ b/dd-java-agent/instrumentation/servlet/request-3/src/main/java/datadog/trace/instrumentation/servlet3/Servlet3Advice.java @@ -46,11 +46,16 @@ public static boolean onEnter( final HttpServletRequest httpServletRequest = (HttpServletRequest) request; HttpServletResponse httpServletResponse = (HttpServletResponse) response; - if (RumInjector.get().isEnabled() && httpServletRequest.getAttribute(DD_RUM_INJECTED) == null) { - httpServletRequest.setAttribute(DD_RUM_INJECTED, Boolean.TRUE); - rumServletWrapper = new RumHttpServletResponseWrapper(httpServletResponse); - httpServletResponse = rumServletWrapper; - response = httpServletResponse; + if (RumInjector.get().isEnabled()) { + final Object maybeRumWrapper = httpServletRequest.getAttribute(DD_RUM_INJECTED); + if (maybeRumWrapper instanceof RumHttpServletResponseWrapper) { + rumServletWrapper = (RumHttpServletResponseWrapper) maybeRumWrapper; + } else { + rumServletWrapper = new RumHttpServletResponseWrapper((HttpServletResponse) response); + httpServletRequest.setAttribute(DD_RUM_INJECTED, rumServletWrapper); + response = rumServletWrapper; + request = new RumHttpServletRequestWrapper(httpServletRequest, rumServletWrapper); + } } Object dispatchSpan = request.getAttribute(DD_DISPATCH_SPAN_ATTRIBUTE); diff --git a/dd-java-agent/instrumentation/servlet/request-3/src/main/java/datadog/trace/instrumentation/servlet3/Servlet3Instrumentation.java b/dd-java-agent/instrumentation/servlet/request-3/src/main/java/datadog/trace/instrumentation/servlet3/Servlet3Instrumentation.java index d7732045329..4835d9d2b0b 100644 --- a/dd-java-agent/instrumentation/servlet/request-3/src/main/java/datadog/trace/instrumentation/servlet3/Servlet3Instrumentation.java +++ b/dd-java-agent/instrumentation/servlet/request-3/src/main/java/datadog/trace/instrumentation/servlet3/Servlet3Instrumentation.java @@ -52,6 +52,7 @@ public String[] helperClassNames() { packageName + ".Servlet3Decorator", packageName + ".ServletRequestURIAdapter", packageName + ".FinishAsyncDispatchListener", + packageName + ".RumHttpServletRequestWrapper", packageName + ".RumHttpServletResponseWrapper", packageName + ".WrappedServletOutputStream", "datadog.trace.instrumentation.servlet.ServletBlockingHelper", diff --git a/dd-java-agent/instrumentation/servlet/request-3/src/main/java/datadog/trace/instrumentation/servlet3/WrappedServletOutputStream.java b/dd-java-agent/instrumentation/servlet/request-3/src/main/java/datadog/trace/instrumentation/servlet3/WrappedServletOutputStream.java index 951eceb5db8..109a55491d8 100644 --- a/dd-java-agent/instrumentation/servlet/request-3/src/main/java/datadog/trace/instrumentation/servlet3/WrappedServletOutputStream.java +++ b/dd-java-agent/instrumentation/servlet/request-3/src/main/java/datadog/trace/instrumentation/servlet3/WrappedServletOutputStream.java @@ -86,4 +86,8 @@ public void setWriteListener(WriteListener writeListener) { public void commit() throws IOException { filtered.commit(); } + + public void setFilter(boolean filter) { + filtered.setFilter(filter); + } } diff --git a/dd-java-agent/instrumentation/servlet/request-5/src/main/java/datadog/trace/instrumentation/servlet5/JakartaServletInstrumentation.java b/dd-java-agent/instrumentation/servlet/request-5/src/main/java/datadog/trace/instrumentation/servlet5/JakartaServletInstrumentation.java index 91e3c989019..d3630cc1fd8 100644 --- a/dd-java-agent/instrumentation/servlet/request-5/src/main/java/datadog/trace/instrumentation/servlet5/JakartaServletInstrumentation.java +++ b/dd-java-agent/instrumentation/servlet/request-5/src/main/java/datadog/trace/instrumentation/servlet5/JakartaServletInstrumentation.java @@ -42,7 +42,9 @@ public String hierarchyMarkerType() { @Override public String[] helperClassNames() { return new String[] { - packageName + ".RumHttpServletResponseWrapper", packageName + ".WrappedServletOutputStream", + packageName + ".RumHttpServletRequestWrapper", + packageName + ".RumHttpServletResponseWrapper", + packageName + ".WrappedServletOutputStream", }; } @@ -67,7 +69,7 @@ public void methodAdvice(MethodTransformer transformer) { public static class JakartaServletAdvice { @Advice.OnMethodEnter(suppress = Throwable.class) public static AgentSpan before( - @Advice.Argument(0) final ServletRequest request, + @Advice.Argument(value = 0, readOnly = false) ServletRequest request, @Advice.Argument(value = 1, readOnly = false) ServletResponse response, @Advice.Local("rumServletWrapper") RumHttpServletResponseWrapper rumServletWrapper) { if (!(request instanceof HttpServletRequest)) { @@ -77,11 +79,16 @@ public static AgentSpan before( if (response instanceof HttpServletResponse) { final HttpServletRequest httpServletRequest = (HttpServletRequest) request; - if (RumInjector.get().isEnabled() - && httpServletRequest.getAttribute(DD_RUM_INJECTED) == null) { - httpServletRequest.setAttribute(DD_RUM_INJECTED, Boolean.TRUE); - rumServletWrapper = new RumHttpServletResponseWrapper((HttpServletResponse) response); - response = rumServletWrapper; + if (RumInjector.get().isEnabled()) { + final Object maybeRumWrapper = httpServletRequest.getAttribute(DD_RUM_INJECTED); + if (maybeRumWrapper instanceof RumHttpServletResponseWrapper) { + rumServletWrapper = (RumHttpServletResponseWrapper) maybeRumWrapper; + } else { + rumServletWrapper = new RumHttpServletResponseWrapper((HttpServletResponse) response); + httpServletRequest.setAttribute(DD_RUM_INJECTED, rumServletWrapper); + response = rumServletWrapper; + request = new RumHttpServletRequestWrapper(httpServletRequest, rumServletWrapper); + } } } diff --git a/dd-java-agent/instrumentation/servlet/request-5/src/main/java/datadog/trace/instrumentation/servlet5/RumAsyncContextInstrumentation.java b/dd-java-agent/instrumentation/servlet/request-5/src/main/java/datadog/trace/instrumentation/servlet5/RumAsyncContextInstrumentation.java new file mode 100644 index 00000000000..a4150f17ad1 --- /dev/null +++ b/dd-java-agent/instrumentation/servlet/request-5/src/main/java/datadog/trace/instrumentation/servlet5/RumAsyncContextInstrumentation.java @@ -0,0 +1,64 @@ +package datadog.trace.instrumentation.servlet5; + +import static datadog.trace.agent.tooling.bytebuddy.matcher.HierarchyMatchers.implementsInterface; +import static datadog.trace.agent.tooling.bytebuddy.matcher.NameMatchers.named; +import static datadog.trace.agent.tooling.bytebuddy.matcher.NameMatchers.namedOneOf; +import static datadog.trace.bootstrap.instrumentation.decorator.HttpServerDecorator.DD_RUM_INJECTED; +import static net.bytebuddy.matcher.ElementMatchers.isMethod; + +import com.google.auto.service.AutoService; +import datadog.trace.agent.tooling.Instrumenter; +import datadog.trace.agent.tooling.InstrumenterModule; +import datadog.trace.api.InstrumenterConfig; +import jakarta.servlet.AsyncContext; +import net.bytebuddy.asm.Advice; +import net.bytebuddy.description.type.TypeDescription; +import net.bytebuddy.matcher.ElementMatcher; + +@AutoService(InstrumenterModule.class) +public class RumAsyncContextInstrumentation extends InstrumenterModule.Tracing + implements Instrumenter.ForTypeHierarchy, Instrumenter.HasMethodAdvice { + + public RumAsyncContextInstrumentation() { + super("servlet", "servlet-5", "servlet-5-async-context"); + } + + @Override + public String hierarchyMarkerType() { + return "jakarta.servlet.AsyncContext"; + } + + @Override + public String[] helperClassNames() { + return new String[] { + packageName + ".RumHttpServletResponseWrapper", packageName + ".WrappedServletOutputStream", + }; + } + + @Override + public ElementMatcher hierarchyMatcher() { + return implementsInterface(named(hierarchyMarkerType())); + } + + @Override + public boolean isEnabled() { + return super.isEnabled() && InstrumenterConfig.get().isRumEnabled(); + } + + @Override + public void methodAdvice(MethodTransformer transformer) { + transformer.applyAdvice( + isMethod().and(namedOneOf("complete", "dispatch")), getClass().getName() + "$CommitAdvice"); + } + + public static class CommitAdvice { + @Advice.OnMethodEnter(suppress = Throwable.class) + public static void commitRumBuffer(@Advice.This final AsyncContext asyncContext) { + final Object maybeRumWrappedResponse = + asyncContext.getRequest().getAttribute(DD_RUM_INJECTED); + if (maybeRumWrappedResponse instanceof RumHttpServletResponseWrapper) { + ((RumHttpServletResponseWrapper) maybeRumWrappedResponse).commit(); + } + } + } +} diff --git a/dd-java-agent/instrumentation/servlet/request-5/src/main/java/datadog/trace/instrumentation/servlet5/RumHttpServletRequestWrapper.java b/dd-java-agent/instrumentation/servlet/request-5/src/main/java/datadog/trace/instrumentation/servlet5/RumHttpServletRequestWrapper.java new file mode 100644 index 00000000000..c2a05680488 --- /dev/null +++ b/dd-java-agent/instrumentation/servlet/request-5/src/main/java/datadog/trace/instrumentation/servlet5/RumHttpServletRequestWrapper.java @@ -0,0 +1,45 @@ +package datadog.trace.instrumentation.servlet5; + +import static datadog.trace.bootstrap.instrumentation.decorator.HttpServerDecorator.DD_RUM_INJECTED; + +import jakarta.servlet.AsyncContext; +import jakarta.servlet.ServletRequest; +import jakarta.servlet.ServletResponse; +import jakarta.servlet.http.HttpServletRequest; +import jakarta.servlet.http.HttpServletRequestWrapper; +import jakarta.servlet.http.HttpServletResponse; + +public class RumHttpServletRequestWrapper extends HttpServletRequestWrapper { + + private final HttpServletResponse response; + + public RumHttpServletRequestWrapper( + final HttpServletRequest request, final HttpServletResponse response) { + super(request); + this.response = response; + } + + @Override + public AsyncContext startAsync() throws IllegalStateException { + // need to hide this method otherwise we cannot control the wrapped response used asynchronously + return startAsync(getRequest(), response); + } + + @Override + public AsyncContext startAsync(ServletRequest servletRequest, ServletResponse servletResponse) + throws IllegalStateException { + // deactivate the previous wrapper + final Object maybeRumWrappedResponse = (servletRequest.getAttribute(DD_RUM_INJECTED)); + if (maybeRumWrappedResponse instanceof RumHttpServletResponseWrapper) { + ((RumHttpServletResponseWrapper) maybeRumWrappedResponse).commit(); + ((RumHttpServletResponseWrapper) maybeRumWrappedResponse).stopFiltering(); + } + ServletResponse actualResponse = servletResponse; + // rewrap it + if (servletResponse instanceof HttpServletResponse) { + actualResponse = new RumHttpServletResponseWrapper((HttpServletResponse) servletResponse); + servletRequest.setAttribute(DD_RUM_INJECTED, actualResponse); + } + return super.startAsync(servletRequest, actualResponse); + } +} diff --git a/dd-java-agent/instrumentation/servlet/request-5/src/main/java/datadog/trace/instrumentation/servlet5/RumHttpServletResponseWrapper.java b/dd-java-agent/instrumentation/servlet/request-5/src/main/java/datadog/trace/instrumentation/servlet5/RumHttpServletResponseWrapper.java index 4b846b3c2db..4b91afd3890 100644 --- a/dd-java-agent/instrumentation/servlet/request-5/src/main/java/datadog/trace/instrumentation/servlet5/RumHttpServletResponseWrapper.java +++ b/dd-java-agent/instrumentation/servlet/request-5/src/main/java/datadog/trace/instrumentation/servlet5/RumHttpServletResponseWrapper.java @@ -108,6 +108,7 @@ public void setContentType(String type) { } if (!shouldInject) { commit(); + stopFiltering(); } super.setContentType(type); } @@ -126,4 +127,14 @@ public void commit() { } } } + + public void stopFiltering() { + shouldInject = false; + if (wrappedPipeWriter != null) { + wrappedPipeWriter.setFilter(false); + } + if (outputStream != null) { + outputStream.setFilter(false); + } + } } diff --git a/dd-java-agent/instrumentation/servlet/request-5/src/main/java/datadog/trace/instrumentation/servlet5/WrappedServletOutputStream.java b/dd-java-agent/instrumentation/servlet/request-5/src/main/java/datadog/trace/instrumentation/servlet5/WrappedServletOutputStream.java index f2338ee1a71..db956377708 100644 --- a/dd-java-agent/instrumentation/servlet/request-5/src/main/java/datadog/trace/instrumentation/servlet5/WrappedServletOutputStream.java +++ b/dd-java-agent/instrumentation/servlet/request-5/src/main/java/datadog/trace/instrumentation/servlet5/WrappedServletOutputStream.java @@ -53,4 +53,8 @@ public boolean isReady() { public void setWriteListener(WriteListener writeListener) { delegate.setWriteListener(writeListener); } + + public void setFilter(boolean filter) { + filtered.setFilter(filter); + } } diff --git a/dd-java-agent/instrumentation/servlet/request-5/src/testFixtures/groovy/datadog/trace/instrumentation/servlet5/AsyncRumServlet.groovy b/dd-java-agent/instrumentation/servlet/request-5/src/testFixtures/groovy/datadog/trace/instrumentation/servlet5/AsyncRumServlet.groovy new file mode 100644 index 00000000000..7a1f736ceca --- /dev/null +++ b/dd-java-agent/instrumentation/servlet/request-5/src/testFixtures/groovy/datadog/trace/instrumentation/servlet5/AsyncRumServlet.groovy @@ -0,0 +1,58 @@ +package datadog.trace.instrumentation.servlet5 + +import jakarta.servlet.AsyncContext +import jakarta.servlet.ServletException +import jakarta.servlet.http.HttpServlet +import jakarta.servlet.http.HttpServletRequest +import jakarta.servlet.http.HttpServletResponse + +class AsyncRumServlet extends HttpServlet { + private final String mimeType + + AsyncRumServlet(String mime) { + this.mimeType = mime + } + + @Override + protected void service(HttpServletRequest req, HttpServletResponse resp) throws ServletException, IOException { + // write a partial content + resp.getWriter().println("\n" + + "") + // finish it later + final AsyncContext asyncContext = req.startAsync() + final String mime = mimeType + new Timer().schedule(new TimerTask() { + @Override + void run() { + def writer = asyncContext.getResponse().getWriter() + try { + asyncContext.getResponse().setContentType(mime) + writer.println( + "\n" + + " \n" + + " This is the title of the webpage!\n" + + " \n" + + " \n" + + "

This is an example paragraph. Anything in the body tag will appear on the page, just like this p tag and its contents.

\n" + + " \n" + + "") + } finally { + asyncContext.complete() + } + } + + }, 2000) + } +} + +class HtmlAsyncRumServlet extends AsyncRumServlet { + HtmlAsyncRumServlet() { + super("text/html") + } +} + +class XmlAsyncRumServlet extends AsyncRumServlet { + XmlAsyncRumServlet() { + super("text/xml") + } +} diff --git a/dd-java-agent/instrumentation/tomcat-5.5/src/latestDepTest/groovy/TomcatServletTest.groovy b/dd-java-agent/instrumentation/tomcat-5.5/src/latestDepTest/groovy/TomcatServletTest.groovy index 999d9d62c8a..ca541ea817f 100644 --- a/dd-java-agent/instrumentation/tomcat-5.5/src/latestDepTest/groovy/TomcatServletTest.groovy +++ b/dd-java-agent/instrumentation/tomcat-5.5/src/latestDepTest/groovy/TomcatServletTest.groovy @@ -1,7 +1,9 @@ import datadog.trace.agent.test.base.HttpServer import datadog.trace.api.ProcessTags +import datadog.trace.instrumentation.servlet5.HtmlAsyncRumServlet import datadog.trace.instrumentation.servlet5.HtmlRumServlet import datadog.trace.instrumentation.servlet5.TestServlet5 +import datadog.trace.instrumentation.servlet5.XmlAsyncRumServlet import datadog.trace.instrumentation.servlet5.XmlRumServlet import jakarta.servlet.Filter import jakarta.servlet.Servlet @@ -307,6 +309,15 @@ class TomcatRumInjectionForkedTest extends TomcatServletTest { } } +class TomcatAsyncRumInjectionForkedTest extends TomcatRumInjectionForkedTest { + @Override + protected void setupServlets(Context context) { + super.setupServlets(context) + addServlet(context, "/gimme-html", HtmlAsyncRumServlet) + addServlet(context, "/gimme-xml", XmlAsyncRumServlet) + } +} + diff --git a/dd-smoke-tests/rum/src/main/groovy/datadog/smoketest/rum/AbstractRumServerSmokeTest.groovy b/dd-smoke-tests/rum/src/main/groovy/datadog/smoketest/rum/AbstractRumServerSmokeTest.groovy index a4b79227e36..d26cffe3d44 100644 --- a/dd-smoke-tests/rum/src/main/groovy/datadog/smoketest/rum/AbstractRumServerSmokeTest.groovy +++ b/dd-smoke-tests/rum/src/main/groovy/datadog/smoketest/rum/AbstractRumServerSmokeTest.groovy @@ -14,9 +14,9 @@ class AbstractRumServerSmokeTest extends AbstractServerSmokeTest { "-Ddd.rum.remote.configuration.id=12345", ] - void 'test RUM SDK injection on html'() { + void 'test RUM SDK injection on html for path #servletPath'() { given: - def url = "http://localhost:${httpPort}/html" + def url = "http://localhost:${httpPort}/${servletPath}" def request = new Request.Builder() .url(url) .get() @@ -28,6 +28,8 @@ class AbstractRumServerSmokeTest extends AbstractServerSmokeTest { then: response.code() == 200 assertRumInjected(response) + where: + servletPath << ["html", "html_async"] } void 'test RUM SDK injection skip on unsupported mime type'() { diff --git a/dd-smoke-tests/rum/tomcat-10/src/main/java/com/example/HtmlAsyncServlet.java b/dd-smoke-tests/rum/tomcat-10/src/main/java/com/example/HtmlAsyncServlet.java new file mode 100644 index 00000000000..dc568cfc5b9 --- /dev/null +++ b/dd-smoke-tests/rum/tomcat-10/src/main/java/com/example/HtmlAsyncServlet.java @@ -0,0 +1,46 @@ +package com.example; + +import jakarta.servlet.AsyncContext; +import jakarta.servlet.http.HttpServlet; +import jakarta.servlet.http.HttpServletRequest; +import jakarta.servlet.http.HttpServletResponse; +import java.io.IOException; +import java.io.UncheckedIOException; +import java.util.Timer; +import java.util.TimerTask; + +public class HtmlAsyncServlet extends HttpServlet { + @Override + protected void doGet(HttpServletRequest req, HttpServletResponse resp) throws IOException { + final AsyncContext asyncContext = req.startAsync(); + new Timer(false) + .schedule( + new TimerTask() { + @Override + public void run() { + try { + asyncContext + .getResponse() + .getWriter() + .write( + "" + + "" + + "" + + " " + + " " + + " Hello Servlet" + + "" + + "" + + "

Hello from Tomcat 9 Servlet!

" + + "

This is a demo HTML page served by Java servlet.

" + + "" + + ""); + asyncContext.complete(); + } catch (IOException e) { + throw new UncheckedIOException(e); + } + } + }, + 2000); + } +} diff --git a/dd-smoke-tests/rum/tomcat-10/src/main/java/com/example/Main.java b/dd-smoke-tests/rum/tomcat-10/src/main/java/com/example/Main.java index d9992f37c92..524e62907c0 100644 --- a/dd-smoke-tests/rum/tomcat-10/src/main/java/com/example/Main.java +++ b/dd-smoke-tests/rum/tomcat-10/src/main/java/com/example/Main.java @@ -1,5 +1,6 @@ package com.example; +import jakarta.servlet.ServletRegistration; import java.io.File; import org.apache.catalina.Context; import org.apache.catalina.LifecycleException; @@ -26,6 +27,10 @@ public static void main(String[] args) throws LifecycleException { context.addServletContainerInitializer( (c, ctx) -> { ctx.addServlet("htmlServlet", new HtmlServlet()).addMapping("/html"); + final ServletRegistration.Dynamic registration = + ctx.addServlet("htmlAsyncServlet", new HtmlAsyncServlet()); + registration.addMapping("/html_async"); + registration.setAsyncSupported(true); ctx.addServlet("xmlServlet", new XmlServlet()).addMapping("/xml"); }, null); diff --git a/dd-smoke-tests/rum/tomcat-11/src/main/java/com/example/HtmlAsyncServlet.java b/dd-smoke-tests/rum/tomcat-11/src/main/java/com/example/HtmlAsyncServlet.java new file mode 100644 index 00000000000..dc568cfc5b9 --- /dev/null +++ b/dd-smoke-tests/rum/tomcat-11/src/main/java/com/example/HtmlAsyncServlet.java @@ -0,0 +1,46 @@ +package com.example; + +import jakarta.servlet.AsyncContext; +import jakarta.servlet.http.HttpServlet; +import jakarta.servlet.http.HttpServletRequest; +import jakarta.servlet.http.HttpServletResponse; +import java.io.IOException; +import java.io.UncheckedIOException; +import java.util.Timer; +import java.util.TimerTask; + +public class HtmlAsyncServlet extends HttpServlet { + @Override + protected void doGet(HttpServletRequest req, HttpServletResponse resp) throws IOException { + final AsyncContext asyncContext = req.startAsync(); + new Timer(false) + .schedule( + new TimerTask() { + @Override + public void run() { + try { + asyncContext + .getResponse() + .getWriter() + .write( + "" + + "" + + "" + + " " + + " " + + " Hello Servlet" + + "" + + "" + + "

Hello from Tomcat 9 Servlet!

" + + "

This is a demo HTML page served by Java servlet.

" + + "" + + ""); + asyncContext.complete(); + } catch (IOException e) { + throw new UncheckedIOException(e); + } + } + }, + 2000); + } +} diff --git a/dd-smoke-tests/rum/tomcat-11/src/main/java/com/example/Main.java b/dd-smoke-tests/rum/tomcat-11/src/main/java/com/example/Main.java index d9992f37c92..524e62907c0 100644 --- a/dd-smoke-tests/rum/tomcat-11/src/main/java/com/example/Main.java +++ b/dd-smoke-tests/rum/tomcat-11/src/main/java/com/example/Main.java @@ -1,5 +1,6 @@ package com.example; +import jakarta.servlet.ServletRegistration; import java.io.File; import org.apache.catalina.Context; import org.apache.catalina.LifecycleException; @@ -26,6 +27,10 @@ public static void main(String[] args) throws LifecycleException { context.addServletContainerInitializer( (c, ctx) -> { ctx.addServlet("htmlServlet", new HtmlServlet()).addMapping("/html"); + final ServletRegistration.Dynamic registration = + ctx.addServlet("htmlAsyncServlet", new HtmlAsyncServlet()); + registration.addMapping("/html_async"); + registration.setAsyncSupported(true); ctx.addServlet("xmlServlet", new XmlServlet()).addMapping("/xml"); }, null); diff --git a/dd-smoke-tests/rum/tomcat-9/src/main/java/com/example/HtmlAsyncServlet.java b/dd-smoke-tests/rum/tomcat-9/src/main/java/com/example/HtmlAsyncServlet.java new file mode 100644 index 00000000000..81406c68ba4 --- /dev/null +++ b/dd-smoke-tests/rum/tomcat-9/src/main/java/com/example/HtmlAsyncServlet.java @@ -0,0 +1,46 @@ +package com.example; + +import java.io.IOException; +import java.io.UncheckedIOException; +import java.util.Timer; +import java.util.TimerTask; +import javax.servlet.AsyncContext; +import javax.servlet.http.HttpServlet; +import javax.servlet.http.HttpServletRequest; +import javax.servlet.http.HttpServletResponse; + +public class HtmlAsyncServlet extends HttpServlet { + @Override + protected void doGet(HttpServletRequest req, HttpServletResponse resp) throws IOException { + final AsyncContext asyncContext = req.startAsync(); + new Timer(false) + .schedule( + new TimerTask() { + @Override + public void run() { + try { + asyncContext + .getResponse() + .getWriter() + .write( + "" + + "" + + "" + + " " + + " " + + " Hello Servlet" + + "" + + "" + + "

Hello from Tomcat 9 Servlet!

" + + "

This is a demo HTML page served by Java servlet.

" + + "" + + ""); + asyncContext.complete(); + } catch (IOException e) { + throw new UncheckedIOException(e); + } + } + }, + 2000); + } +} diff --git a/dd-smoke-tests/rum/tomcat-9/src/main/java/com/example/Main.java b/dd-smoke-tests/rum/tomcat-9/src/main/java/com/example/Main.java index d9992f37c92..ad56bd12c28 100644 --- a/dd-smoke-tests/rum/tomcat-9/src/main/java/com/example/Main.java +++ b/dd-smoke-tests/rum/tomcat-9/src/main/java/com/example/Main.java @@ -1,6 +1,7 @@ package com.example; import java.io.File; +import javax.servlet.ServletRegistration; import org.apache.catalina.Context; import org.apache.catalina.LifecycleException; import org.apache.catalina.startup.Tomcat; @@ -26,6 +27,10 @@ public static void main(String[] args) throws LifecycleException { context.addServletContainerInitializer( (c, ctx) -> { ctx.addServlet("htmlServlet", new HtmlServlet()).addMapping("/html"); + final ServletRegistration.Dynamic registration = + ctx.addServlet("htmlAsyncServlet", new HtmlAsyncServlet()); + registration.addMapping("/html_async"); + registration.setAsyncSupported(true); ctx.addServlet("xmlServlet", new XmlServlet()).addMapping("/xml"); }, null); 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 1d257c56542..2e1072d83d3 100644 --- a/internal-api/src/main/java/datadog/trace/api/Config.java +++ b/internal-api/src/main/java/datadog/trace/api/Config.java @@ -121,7 +121,6 @@ import static datadog.trace.api.ConfigDefaults.DEFAULT_REMOTE_CONFIG_POLL_INTERVAL_SECONDS; import static datadog.trace.api.ConfigDefaults.DEFAULT_REMOTE_CONFIG_TARGETS_KEY; import static datadog.trace.api.ConfigDefaults.DEFAULT_REMOTE_CONFIG_TARGETS_KEY_ID; -import static datadog.trace.api.ConfigDefaults.DEFAULT_RUM_ENABLED; import static datadog.trace.api.ConfigDefaults.DEFAULT_RUM_MAJOR_VERSION; import static datadog.trace.api.ConfigDefaults.DEFAULT_SCOPE_DEPTH_LIMIT; import static datadog.trace.api.ConfigDefaults.DEFAULT_SCOPE_ITERATION_KEEP_ALIVE; @@ -472,7 +471,6 @@ import static datadog.trace.api.config.RumConfig.RUM_APPLICATION_ID; import static datadog.trace.api.config.RumConfig.RUM_CLIENT_TOKEN; import static datadog.trace.api.config.RumConfig.RUM_DEFAULT_PRIVACY_LEVEL; -import static datadog.trace.api.config.RumConfig.RUM_ENABLED; import static datadog.trace.api.config.RumConfig.RUM_ENVIRONMENT; import static datadog.trace.api.config.RumConfig.RUM_MAJOR_VERSION; import static datadog.trace.api.config.RumConfig.RUM_REMOTE_CONFIGURATION_ID; @@ -1226,7 +1224,6 @@ public static String getHostName() { private final boolean optimizedMapEnabled; private final int stackTraceLengthLimit; - private final boolean rumEnabled; private final RumInjectorConfig rumInjectorConfig; // Read order: System Properties -> Env Variables, [-> properties file], [-> default value] @@ -2745,14 +2742,13 @@ PROFILING_DATADOG_PROFILER_ENABLED, isDatadogProfilerSafeInCurrentEnvironment()) this.stackTraceLengthLimit = configProvider.getInteger(STACK_TRACE_LENGTH_LIMIT, defaultStackTraceLengthLimit); - this.rumEnabled = configProvider.getBoolean(RUM_ENABLED, DEFAULT_RUM_ENABLED); this.rumInjectorConfig = parseRumConfig(configProvider); log.debug("New instance: {}", this); } private RumInjectorConfig parseRumConfig(ConfigProvider configProvider) { - if (!this.rumEnabled) { + if (!instrumenterConfig.isRumEnabled()) { return null; } try { @@ -5036,10 +5032,6 @@ public int getCloudPayloadTaggingMaxTags() { return cloudPayloadTaggingMaxTags; } - public boolean isRumEnabled() { - return this.rumEnabled; - } - public RumInjectorConfig getRumInjectorConfig() { return this.rumInjectorConfig; } @@ -5724,8 +5716,6 @@ public String toString() { + cloudResponsePayloadTagging + ", experimentalPropagateProcessTagsEnabled=" + experimentalPropagateProcessTagsEnabled - + ", rumEnabled=" - + rumEnabled + ", rumInjectorConfig=" + (rumInjectorConfig == null ? "null" : rumInjectorConfig.jsonPayload()) + '}'; diff --git a/internal-api/src/main/java/datadog/trace/api/InstrumenterConfig.java b/internal-api/src/main/java/datadog/trace/api/InstrumenterConfig.java index 851a84d5b17..532d1472c6e 100644 --- a/internal-api/src/main/java/datadog/trace/api/InstrumenterConfig.java +++ b/internal-api/src/main/java/datadog/trace/api/InstrumenterConfig.java @@ -8,6 +8,7 @@ import static datadog.trace.api.ConfigDefaults.DEFAULT_LLM_OBS_ENABLED; import static datadog.trace.api.ConfigDefaults.DEFAULT_MEASURE_METHODS; import static datadog.trace.api.ConfigDefaults.DEFAULT_RESOLVER_RESET_INTERVAL; +import static datadog.trace.api.ConfigDefaults.DEFAULT_RUM_ENABLED; import static datadog.trace.api.ConfigDefaults.DEFAULT_RUNTIME_CONTEXT_FIELD_INJECTION; import static datadog.trace.api.ConfigDefaults.DEFAULT_SERIALVERSIONUID_FIELD_INJECTION; import static datadog.trace.api.ConfigDefaults.DEFAULT_TELEMETRY_ENABLED; @@ -32,6 +33,7 @@ import static datadog.trace.api.config.ProfilingConfig.PROFILING_DIRECT_ALLOCATION_ENABLED_DEFAULT; import static datadog.trace.api.config.ProfilingConfig.PROFILING_ENABLED; import static datadog.trace.api.config.ProfilingConfig.PROFILING_ENABLED_DEFAULT; +import static datadog.trace.api.config.RumConfig.RUM_ENABLED; import static datadog.trace.api.config.TraceInstrumentationConfig.AKKA_FORK_JOIN_EXECUTOR_TASK_NAME; import static datadog.trace.api.config.TraceInstrumentationConfig.AKKA_FORK_JOIN_POOL_NAME; import static datadog.trace.api.config.TraceInstrumentationConfig.AKKA_FORK_JOIN_TASK_NAME; @@ -168,6 +170,8 @@ public class InstrumenterConfig { private final Collection additionalJaxRsAnnotations; + private final boolean rumEnabled; + private InstrumenterConfig() { this(ConfigProvider.createDefault()); } @@ -195,7 +199,7 @@ private InstrumenterConfig() { profilingEnabled = ProfilingEnablement.of( configProvider.getString(PROFILING_ENABLED, String.valueOf(PROFILING_ENABLED_DEFAULT))); - + rumEnabled = configProvider.getBoolean(RUM_ENABLED, DEFAULT_RUM_ENABLED); if (!Platform.isNativeImageBuilder()) { ciVisibilityEnabled = configProvider.getBoolean(CIVISIBILITY_ENABLED, DEFAULT_CIVISIBILITY_ENABLED); @@ -567,6 +571,10 @@ public boolean isLegacyInstrumentationEnabled( Arrays.asList(integrationNames), "", ".legacy.tracing.enabled", defaultEnabled); } + public boolean isRumEnabled() { + return rumEnabled; + } + // This has to be placed after all other static fields to give them a chance to initialize @SuppressFBWarnings("SI_INSTANCE_BEFORE_FINALS_ASSIGNED") private static final InstrumenterConfig INSTANCE = @@ -669,6 +677,8 @@ public String toString() { + websocketTracingEnabled + ", pekkoSchedulerEnabled=" + pekkoSchedulerEnabled + + ", rumEnabled=" + + rumEnabled + '}'; } } diff --git a/internal-api/src/main/java/datadog/trace/api/rum/RumInjector.java b/internal-api/src/main/java/datadog/trace/api/rum/RumInjector.java index 03917bc0cfc..5501c80e62c 100644 --- a/internal-api/src/main/java/datadog/trace/api/rum/RumInjector.java +++ b/internal-api/src/main/java/datadog/trace/api/rum/RumInjector.java @@ -1,13 +1,15 @@ package datadog.trace.api.rum; import datadog.trace.api.Config; +import datadog.trace.api.InstrumenterConfig; import datadog.trace.api.cache.DDCache; import datadog.trace.api.cache.DDCaches; import java.util.function.Function; import javax.annotation.Nullable; public final class RumInjector { - private static final RumInjector INSTANCE = new RumInjector(Config.get()); + private static final RumInjector INSTANCE = + new RumInjector(Config.get(), InstrumenterConfig.get()); private static final String MARKER = ""; private static final char[] MARKER_CHARS = MARKER.toCharArray(); private static final Function MARKER_BYTES = @@ -27,8 +29,8 @@ public final class RumInjector { private final DDCache markerCache; private final Function snippetBytes; - RumInjector(Config config) { - boolean rumEnabled = config.isRumEnabled(); + RumInjector(Config config, InstrumenterConfig instrumenterConfig) { + boolean rumEnabled = instrumenterConfig.isRumEnabled(); RumInjectorConfig injectorConfig = config.getRumInjectorConfig(); // If both RUM is enabled and injector config is valid if (rumEnabled && injectorConfig != null) { diff --git a/internal-api/src/test/groovy/datadog/trace/api/rum/RumInjectorTest.groovy b/internal-api/src/test/groovy/datadog/trace/api/rum/RumInjectorTest.groovy index c64f756cac0..98988e40242 100644 --- a/internal-api/src/test/groovy/datadog/trace/api/rum/RumInjectorTest.groovy +++ b/internal-api/src/test/groovy/datadog/trace/api/rum/RumInjectorTest.groovy @@ -1,6 +1,7 @@ package datadog.trace.api.rum import datadog.trace.api.Config +import datadog.trace.api.InstrumenterConfig import datadog.trace.test.util.DDSpecification import static org.mockito.Mockito.mock @@ -12,11 +13,12 @@ class RumInjectorTest extends DDSpecification { void 'disabled injector'(){ setup: Config config = mock(Config) + InstrumenterConfig instrumenterConfig = mock(InstrumenterConfig) RumInjector injector when: - when(config.isRumEnabled()).thenReturn(false) - injector = new RumInjector(config) + when(instrumenterConfig.isRumEnabled()).thenReturn(false) + injector = new RumInjector(config, instrumenterConfig) then: !injector.isEnabled() @@ -27,12 +29,13 @@ class RumInjectorTest extends DDSpecification { void 'invalid config injector'() { setup: Config config = mock(Config) + InstrumenterConfig instrumenterConfig = mock(InstrumenterConfig) RumInjector injector when: - when(config.isRumEnabled()).thenReturn(true) + when(instrumenterConfig.isRumEnabled()).thenReturn(true) when(config.rumInjectorConfig).thenReturn(null) - injector = new RumInjector(config) + injector = new RumInjector(config, instrumenterConfig) then: !injector.isEnabled() @@ -45,14 +48,15 @@ class RumInjectorTest extends DDSpecification { void 'enabled injector'() { setup: Config config = mock(Config) + InstrumenterConfig instrumenterConfig = mock(InstrumenterConfig) def injectorConfig = mock(RumInjectorConfig) RumInjector injector when: - when(config.isRumEnabled()).thenReturn(true) + when(instrumenterConfig.isRumEnabled()).thenReturn(true) when(config.rumInjectorConfig).thenReturn(injectorConfig) when(injectorConfig.snippet).thenReturn("") - injector = new RumInjector(config) + injector = new RumInjector(config, instrumenterConfig) then: injector.isEnabled() From 0ed0f18005d4ba85774eb83d548cda70dcf1030c Mon Sep 17 00:00:00 2001 From: Andrea Marziali Date: Tue, 12 Aug 2025 16:17:40 +0200 Subject: [PATCH 06/11] =?UTF-8?q?=F0=9F=8D=92=209353=20-=20Avoid=20NPE=20o?= =?UTF-8?q?n=20featureDiscovery=20creation=20(#9354)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * Avoid NPE on featureDiscovery creation (cherry picked from commit cf7d5bbc11c14c4c100edb0b8c89f89520723403) * Review (cherry picked from commit 67ccb2e820991748672dcea2316e4c8d4389fe77) --- .../communication/ddagent/SharedCommunicationObjects.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/communication/src/main/java/datadog/communication/ddagent/SharedCommunicationObjects.java b/communication/src/main/java/datadog/communication/ddagent/SharedCommunicationObjects.java index b855731bc08..68c0f287f7e 100644 --- a/communication/src/main/java/datadog/communication/ddagent/SharedCommunicationObjects.java +++ b/communication/src/main/java/datadog/communication/ddagent/SharedCommunicationObjects.java @@ -142,7 +142,7 @@ public DDAgentFeaturesDiscovery featuresDiscovery(Config config) { DDAgentFeaturesDiscovery ret = featuresDiscovery; if (ret == null) { synchronized (this) { - if (featuresDiscovery == null) { + if ((ret = featuresDiscovery) == null) { createRemaining(config); ret = new DDAgentFeaturesDiscovery( From 8777b20480f2a1c23cde1b1f27b3f899b031a029 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Alejandro=20Gonz=C3=A1lez=20Garc=C3=ADa?= Date: Tue, 12 Aug 2025 16:49:27 +0200 Subject: [PATCH 07/11] 9355 - Fix NullPointerException log in AppSec (#9356) Backport #9355 to release/v1.52.x --- .../java/com/datadog/appsec/ddwaf/WAFModule.java | 5 +++++ .../appsec/ddwaf/WAFModuleSpecification.groovy | 13 +++++++++++++ 2 files changed, 18 insertions(+) diff --git a/dd-java-agent/appsec/src/main/java/com/datadog/appsec/ddwaf/WAFModule.java b/dd-java-agent/appsec/src/main/java/com/datadog/appsec/ddwaf/WAFModule.java index 325c1313263..1aef1c5f964 100644 --- a/dd-java-agent/appsec/src/main/java/com/datadog/appsec/ddwaf/WAFModule.java +++ b/dd-java-agent/appsec/src/main/java/com/datadog/appsec/ddwaf/WAFModule.java @@ -1,5 +1,6 @@ package com.datadog.appsec.ddwaf; +import static datadog.trace.api.telemetry.LogCollector.SEND_TELEMETRY; import static datadog.trace.util.stacktrace.StackTraceEvent.DEFAULT_LANGUAGE; import static java.util.Collections.emptyList; import static java.util.Collections.singletonList; @@ -557,6 +558,10 @@ private Waf.ResultWithData runWafTransient( } private Collection buildEvents(Waf.ResultWithData actionWithData) { + if (actionWithData.data == null) { + log.debug(SEND_TELEMETRY, "WAF result data is null"); + return Collections.emptyList(); + } Collection listResults; try { listResults = RES_JSON_ADAPTER.fromJson(actionWithData.data); diff --git a/dd-java-agent/appsec/src/test/groovy/com/datadog/appsec/ddwaf/WAFModuleSpecification.groovy b/dd-java-agent/appsec/src/test/groovy/com/datadog/appsec/ddwaf/WAFModuleSpecification.groovy index aeed7c23d56..be191e9a749 100644 --- a/dd-java-agent/appsec/src/test/groovy/com/datadog/appsec/ddwaf/WAFModuleSpecification.groovy +++ b/dd-java-agent/appsec/src/test/groovy/com/datadog/appsec/ddwaf/WAFModuleSpecification.groovy @@ -1682,6 +1682,19 @@ class WAFModuleSpecification extends DDSpecification { internal == libddwaf } + void 'ResultWithData - null data'() { + def waf = new WAFModule() + Waf.ResultWithData rwd = new Waf.ResultWithData(null, null, null, null) + Collection ret + + when: + ret = waf.buildEvents(rwd) + + then: + noExceptionThrown() + ret.isEmpty() + } + /** * Helper to return a concrete Waf exception for each WafErrorCode */ From 6b6db17410943612d2705162f2a1477b873157bc Mon Sep 17 00:00:00 2001 From: Jaroslav Bachorik Date: Wed, 13 Aug 2025 16:41:49 +0200 Subject: [PATCH 08/11] Properly handle trace agent IPv6 URL in profiling (#9339) (cherry picked from commit ffa80b2a5b356cfef9bbca9e182532b144bcf091) --- .../src/main/java/datadog/trace/api/Config.java | 2 +- .../test/groovy/datadog/trace/api/ConfigTest.groovy | 13 +++++++++++++ 2 files changed, 14 insertions(+), 1 deletion(-) 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 2e1072d83d3..aae53ec7e07 100644 --- a/internal-api/src/main/java/datadog/trace/api/Config.java +++ b/internal-api/src/main/java/datadog/trace/api/Config.java @@ -4742,7 +4742,7 @@ public String getFinalProfilingUrl() { return "https://intake.profile." + site + "/api/v2/profile"; } else { // when profilingUrl and agentless are not set we send to the dd trace agent running locally - return "http://" + agentHost + ":" + agentPort + "/profiling/v1/input"; + return getAgentUrl() + "/profiling/v1/input"; } } diff --git a/internal-api/src/test/groovy/datadog/trace/api/ConfigTest.groovy b/internal-api/src/test/groovy/datadog/trace/api/ConfigTest.groovy index 0149e2e636e..2905470f5c5 100644 --- a/internal-api/src/test/groovy/datadog/trace/api/ConfigTest.groovy +++ b/internal-api/src/test/groovy/datadog/trace/api/ConfigTest.groovy @@ -1652,6 +1652,19 @@ class ConfigTest extends DDSpecification { config.getFinalProfilingUrl() == "https://some.new.url/goes/here" } + def "ipv6 profiling url"() { + setup: + def configuredUrl = "http://[2600:1f14:1cfc:5f07::38d4]:8126" + def props = new Properties() + props.setProperty(TRACE_AGENT_URL, configuredUrl) + + when: + Config config = Config.get(props) + + then: + config.getFinalProfilingUrl() == configuredUrl + "/profiling/v1/input" + } + def "fallback to DD_TAGS"() { setup: environmentVariables.set(DD_TAGS_ENV, "a:1,b:2,c:3") From 61dc0f200177ec1fe3ced373b14f424718e52687 Mon Sep 17 00:00:00 2001 From: Daniel Mohedano Date: Fri, 22 Aug 2025 15:17:13 +0200 Subject: [PATCH 09/11] add logging --- .../domain/AbstractTestModule.java | 7 +++++ .../domain/AbstractTestSession.java | 7 +++++ .../trace/civisibility/domain/TestImpl.java | 14 ++++++++++ .../civisibility/domain/TestSuiteImpl.java | 7 +++++ .../writer/ddagent/TraceMapperV0_4.java | 26 +++++++++++++++++++ .../writer/ddagent/TraceMapperV0_5.java | 26 +++++++++++++++++++ 6 files changed, 87 insertions(+) diff --git a/dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/domain/AbstractTestModule.java b/dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/domain/AbstractTestModule.java index e87807d0ed0..b6dc725b2ed 100644 --- a/dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/domain/AbstractTestModule.java +++ b/dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/domain/AbstractTestModule.java @@ -18,9 +18,13 @@ import datadog.trace.civisibility.source.SourcePathResolver; import java.util.function.Consumer; import javax.annotation.Nullable; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; public abstract class AbstractTestModule { + private static final Logger log = LoggerFactory.getLogger(AbstractTestModule.class); + protected final AgentSpan span; protected final String moduleName; protected final Config config; @@ -73,6 +77,9 @@ public AbstractTestModule( span.setTag(Tags.TEST_MODULE_ID, span.getSpanId()); span.setTag(Tags.TEST_SESSION_ID, span.getTraceId()); + log.debug("Setting module hierarchy IDs: moduleId={}, sessionId={}, moduleName={}, instrumentationType={}", + span.getSpanId(), span.getTraceId(), moduleName, instrumentationType); + // setting status to skip initially, // as we do not know in advance whether the module will have any children span.setTag(Tags.TEST_STATUS, TestStatus.skip); diff --git a/dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/domain/AbstractTestSession.java b/dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/domain/AbstractTestSession.java index 4338a9a7ba8..0f7d8ba8e4b 100644 --- a/dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/domain/AbstractTestSession.java +++ b/dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/domain/AbstractTestSession.java @@ -34,9 +34,13 @@ import java.util.Collection; import java.util.Collections; import javax.annotation.Nullable; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; public abstract class AbstractTestSession { + private static final Logger log = LoggerFactory.getLogger(AbstractTestSession.class); + protected final Provider ciProvider; protected final InstrumentationType instrumentationType; protected final AgentSpan span; @@ -98,6 +102,9 @@ public AbstractTestSession( span.setTag(Tags.SPAN_KIND, Tags.SPAN_KIND_TEST_SESSION); span.setTag(Tags.TEST_SESSION_ID, span.getTraceId()); + log.debug("Setting TEST_SESSION_ID: traceId={}, spanId={}, project={}, instrumentationType={}", + span.getTraceId(), span.getSpanId(), projectName, instrumentationType); + // setting status to skip initially, // as we do not know in advance whether the session will have any children span.setTag(Tags.TEST_STATUS, TestStatus.skip); diff --git a/dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/domain/TestImpl.java b/dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/domain/TestImpl.java index a2d8d155ea6..62ee23c53c0 100644 --- a/dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/domain/TestImpl.java +++ b/dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/domain/TestImpl.java @@ -135,6 +135,9 @@ public TestImpl( span.setTag(Tags.TEST_MODULE_ID, moduleSpanContext.getSpanId()); span.setTag(Tags.TEST_SESSION_ID, moduleSpanContext.getTraceId()); + log.debug("Setting test hierarchy IDs: sessionId={}, moduleId={}, suiteId={}, test={}.{}, instrumentationType={}", + moduleSpanContext.getTraceId(), moduleSpanContext.getSpanId(), suiteId, testSuiteName, testName, instrumentationType); + span.setTag(Tags.TEST_STATUS, TestStatus.pass); if (testClass != null && !testClass.getName().equals(testSuiteName)) { @@ -156,6 +159,17 @@ public TestImpl( testDecorator.afterStart(span); + // Validation logging for missing IDs + if (moduleSpanContext.getTraceId() == null) { + log.error("Missing session trace ID for test {}.{}, moduleSpanContext={}", testSuiteName, testName, moduleSpanContext); + } + if (moduleSpanContext.getSpanId() == 0) { + log.error("Missing module span ID for test {}.{}, moduleSpanContext={}", testSuiteName, testName, moduleSpanContext); + } + if (suiteId == 0) { + log.error("Missing suite ID for test {}.{}, suiteId={}", testSuiteName, testName, suiteId); + } + metricCollector.add(CiVisibilityCountMetric.EVENT_CREATED, 1, instrumentation, EventType.TEST); if (instrumentationType == InstrumentationType.MANUAL_API) { diff --git a/dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/domain/TestSuiteImpl.java b/dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/domain/TestSuiteImpl.java index 24319318968..a94b1dcab6e 100644 --- a/dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/domain/TestSuiteImpl.java +++ b/dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/domain/TestSuiteImpl.java @@ -118,6 +118,10 @@ public TestSuiteImpl( span.setTag(Tags.TEST_MODULE_ID, moduleSpanContext.getSpanId()); span.setTag(Tags.TEST_SESSION_ID, moduleSpanContext.getTraceId()); + log.debug("Setting suite hierarchy IDs: suiteId={}, moduleId={}, sessionId={}, suiteName={}, moduleName={}, instrumentationType={}", + span.getSpanId(), moduleSpanContext.getSpanId(), moduleSpanContext.getTraceId(), + testSuiteName, moduleName, instrumentationType); + // setting status to skip initially, // as we do not know in advance whether the suite will have any children span.setTag(Tags.TEST_STATUS, TestStatus.skip); @@ -242,6 +246,9 @@ public TestImpl testStart( @Nullable String testParameters, @Nullable Method testMethod, @Nullable Long startTime) { + log.debug("Creating test with context propagation: moduleSpanContext.traceId={}, moduleSpanContext.spanId={}, suiteSpanId={}, testName={}", + moduleSpanContext.getTraceId(), moduleSpanContext.getSpanId(), span.getSpanId(), testName); + return new TestImpl( moduleSpanContext, span.getSpanId(), diff --git a/dd-trace-core/src/main/java/datadog/trace/common/writer/ddagent/TraceMapperV0_4.java b/dd-trace-core/src/main/java/datadog/trace/common/writer/ddagent/TraceMapperV0_4.java index a1d60164b82..379f7714a8c 100644 --- a/dd-trace-core/src/main/java/datadog/trace/common/writer/ddagent/TraceMapperV0_4.java +++ b/dd-trace-core/src/main/java/datadog/trace/common/writer/ddagent/TraceMapperV0_4.java @@ -21,9 +21,15 @@ import java.util.List; import java.util.Map; import okhttp3.RequestBody; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; +import datadog.trace.bootstrap.instrumentation.api.Tags; +import datadog.trace.bootstrap.instrumentation.api.InternalSpanTypes; public final class TraceMapperV0_4 implements TraceMapper { + private static final Logger log = LoggerFactory.getLogger(TraceMapperV0_4.class); + private final int size; public TraceMapperV0_4(int size) { @@ -285,6 +291,26 @@ public void map(List> trace, final Writable writable) { writable.writeUTF8(ERROR); writable.writeInt(span.getError()); /* 11, 12 */ + // Validation logging for missing test hierarchy IDs before serialization + if (InternalSpanTypes.TEST.equals(span.getType())) { + String sessionId = (String) span.getTag(Tags.TEST_SESSION_ID); + String moduleId = (String) span.getTag(Tags.TEST_MODULE_ID); + String suiteId = (String) span.getTag(Tags.TEST_SUITE_ID); + + if (sessionId == null) { + log.warn("Test span missing TEST_SESSION_ID before serialization: spanId={}, resource={}, traceId={}", + span.getSpanId(), span.getResourceName(), span.getTraceId()); + } + if (moduleId == null) { + log.warn("Test span missing TEST_MODULE_ID before serialization: spanId={}, resource={}, traceId={}", + span.getSpanId(), span.getResourceName(), span.getTraceId()); + } + if (suiteId == null) { + log.warn("Test span missing TEST_SUITE_ID before serialization: spanId={}, resource={}, traceId={}", + span.getSpanId(), span.getResourceName(), span.getTraceId()); + } + } + span.processTagsAndBaggage( metaWriter .withWritable(writable) diff --git a/dd-trace-core/src/main/java/datadog/trace/common/writer/ddagent/TraceMapperV0_5.java b/dd-trace-core/src/main/java/datadog/trace/common/writer/ddagent/TraceMapperV0_5.java index c89582e71ec..42057ca5990 100644 --- a/dd-trace-core/src/main/java/datadog/trace/common/writer/ddagent/TraceMapperV0_5.java +++ b/dd-trace-core/src/main/java/datadog/trace/common/writer/ddagent/TraceMapperV0_5.java @@ -23,9 +23,15 @@ import java.util.List; import java.util.Map; import okhttp3.RequestBody; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; +import datadog.trace.bootstrap.instrumentation.api.Tags; +import datadog.trace.bootstrap.instrumentation.api.InternalSpanTypes; public final class TraceMapperV0_5 implements TraceMapper { + private static final Logger log = LoggerFactory.getLogger(TraceMapperV0_5.class); + private final WritableFormatter dictionaryWriter; private final DictionaryMapper dictionaryMapper = new DictionaryMapper(); private final Map encoding = new HashMap<>(); @@ -76,6 +82,26 @@ public void map(final List> trace, final Writable writable /* 9 */ writable.writeInt(span.getError()); /* 10, 11 */ + // Validation logging for missing test hierarchy IDs before serialization + if (InternalSpanTypes.TEST.equals(span.getType())) { + String sessionId = (String) span.getTag(Tags.TEST_SESSION_ID); + String moduleId = (String) span.getTag(Tags.TEST_MODULE_ID); + String suiteId = (String) span.getTag(Tags.TEST_SUITE_ID); + + if (sessionId == null) { + log.warn("Test span missing TEST_SESSION_ID before serialization: spanId={}, resource={}, traceId={}", + span.getSpanId(), span.getResourceName(), span.getTraceId()); + } + if (moduleId == null) { + log.warn("Test span missing TEST_MODULE_ID before serialization: spanId={}, resource={}, traceId={}", + span.getSpanId(), span.getResourceName(), span.getTraceId()); + } + if (suiteId == null) { + log.warn("Test span missing TEST_SUITE_ID before serialization: spanId={}, resource={}, traceId={}", + span.getSpanId(), span.getResourceName(), span.getTraceId()); + } + } + span.processTagsAndBaggage( metaWriter .withWritable(writable) From 907cdfadaa9a42d4b5da3eab9164ed5a65031e69 Mon Sep 17 00:00:00 2001 From: Daniel Mohedano Date: Wed, 27 Aug 2025 15:49:49 +0200 Subject: [PATCH 10/11] add mapper logging --- .../domain/AbstractTestModule.java | 7 ----- .../domain/AbstractTestSession.java | 5 --- .../trace/civisibility/domain/TestImpl.java | 14 --------- .../civisibility/domain/TestSuiteImpl.java | 6 ---- .../writer/ddintake/CiTestCycleMapperV1.java | 13 ++++++++ .../writer/ddagent/TraceMapperV0_4.java | 31 +++++-------------- .../writer/ddagent/TraceMapperV0_5.java | 31 +++++-------------- 7 files changed, 29 insertions(+), 78 deletions(-) diff --git a/dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/domain/AbstractTestModule.java b/dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/domain/AbstractTestModule.java index b6dc725b2ed..e87807d0ed0 100644 --- a/dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/domain/AbstractTestModule.java +++ b/dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/domain/AbstractTestModule.java @@ -18,13 +18,9 @@ import datadog.trace.civisibility.source.SourcePathResolver; import java.util.function.Consumer; import javax.annotation.Nullable; -import org.slf4j.Logger; -import org.slf4j.LoggerFactory; public abstract class AbstractTestModule { - private static final Logger log = LoggerFactory.getLogger(AbstractTestModule.class); - protected final AgentSpan span; protected final String moduleName; protected final Config config; @@ -77,9 +73,6 @@ public AbstractTestModule( span.setTag(Tags.TEST_MODULE_ID, span.getSpanId()); span.setTag(Tags.TEST_SESSION_ID, span.getTraceId()); - log.debug("Setting module hierarchy IDs: moduleId={}, sessionId={}, moduleName={}, instrumentationType={}", - span.getSpanId(), span.getTraceId(), moduleName, instrumentationType); - // setting status to skip initially, // as we do not know in advance whether the module will have any children span.setTag(Tags.TEST_STATUS, TestStatus.skip); diff --git a/dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/domain/AbstractTestSession.java b/dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/domain/AbstractTestSession.java index 0f7d8ba8e4b..7b4b53874a9 100644 --- a/dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/domain/AbstractTestSession.java +++ b/dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/domain/AbstractTestSession.java @@ -39,8 +39,6 @@ public abstract class AbstractTestSession { - private static final Logger log = LoggerFactory.getLogger(AbstractTestSession.class); - protected final Provider ciProvider; protected final InstrumentationType instrumentationType; protected final AgentSpan span; @@ -102,9 +100,6 @@ public AbstractTestSession( span.setTag(Tags.SPAN_KIND, Tags.SPAN_KIND_TEST_SESSION); span.setTag(Tags.TEST_SESSION_ID, span.getTraceId()); - log.debug("Setting TEST_SESSION_ID: traceId={}, spanId={}, project={}, instrumentationType={}", - span.getTraceId(), span.getSpanId(), projectName, instrumentationType); - // setting status to skip initially, // as we do not know in advance whether the session will have any children span.setTag(Tags.TEST_STATUS, TestStatus.skip); diff --git a/dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/domain/TestImpl.java b/dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/domain/TestImpl.java index 62ee23c53c0..a2d8d155ea6 100644 --- a/dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/domain/TestImpl.java +++ b/dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/domain/TestImpl.java @@ -135,9 +135,6 @@ public TestImpl( span.setTag(Tags.TEST_MODULE_ID, moduleSpanContext.getSpanId()); span.setTag(Tags.TEST_SESSION_ID, moduleSpanContext.getTraceId()); - log.debug("Setting test hierarchy IDs: sessionId={}, moduleId={}, suiteId={}, test={}.{}, instrumentationType={}", - moduleSpanContext.getTraceId(), moduleSpanContext.getSpanId(), suiteId, testSuiteName, testName, instrumentationType); - span.setTag(Tags.TEST_STATUS, TestStatus.pass); if (testClass != null && !testClass.getName().equals(testSuiteName)) { @@ -159,17 +156,6 @@ public TestImpl( testDecorator.afterStart(span); - // Validation logging for missing IDs - if (moduleSpanContext.getTraceId() == null) { - log.error("Missing session trace ID for test {}.{}, moduleSpanContext={}", testSuiteName, testName, moduleSpanContext); - } - if (moduleSpanContext.getSpanId() == 0) { - log.error("Missing module span ID for test {}.{}, moduleSpanContext={}", testSuiteName, testName, moduleSpanContext); - } - if (suiteId == 0) { - log.error("Missing suite ID for test {}.{}, suiteId={}", testSuiteName, testName, suiteId); - } - metricCollector.add(CiVisibilityCountMetric.EVENT_CREATED, 1, instrumentation, EventType.TEST); if (instrumentationType == InstrumentationType.MANUAL_API) { diff --git a/dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/domain/TestSuiteImpl.java b/dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/domain/TestSuiteImpl.java index a94b1dcab6e..28d5df031f4 100644 --- a/dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/domain/TestSuiteImpl.java +++ b/dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/domain/TestSuiteImpl.java @@ -118,10 +118,6 @@ public TestSuiteImpl( span.setTag(Tags.TEST_MODULE_ID, moduleSpanContext.getSpanId()); span.setTag(Tags.TEST_SESSION_ID, moduleSpanContext.getTraceId()); - log.debug("Setting suite hierarchy IDs: suiteId={}, moduleId={}, sessionId={}, suiteName={}, moduleName={}, instrumentationType={}", - span.getSpanId(), moduleSpanContext.getSpanId(), moduleSpanContext.getTraceId(), - testSuiteName, moduleName, instrumentationType); - // setting status to skip initially, // as we do not know in advance whether the suite will have any children span.setTag(Tags.TEST_STATUS, TestStatus.skip); @@ -246,8 +242,6 @@ public TestImpl testStart( @Nullable String testParameters, @Nullable Method testMethod, @Nullable Long startTime) { - log.debug("Creating test with context propagation: moduleSpanContext.traceId={}, moduleSpanContext.spanId={}, suiteSpanId={}, testName={}", - moduleSpanContext.getTraceId(), moduleSpanContext.getSpanId(), span.getSpanId(), testName); return new TestImpl( moduleSpanContext, diff --git a/dd-trace-core/src/main/java/datadog/trace/civisibility/writer/ddintake/CiTestCycleMapperV1.java b/dd-trace-core/src/main/java/datadog/trace/civisibility/writer/ddintake/CiTestCycleMapperV1.java index 534282701b7..31771651afe 100644 --- a/dd-trace-core/src/main/java/datadog/trace/civisibility/writer/ddintake/CiTestCycleMapperV1.java +++ b/dd-trace-core/src/main/java/datadog/trace/civisibility/writer/ddintake/CiTestCycleMapperV1.java @@ -28,8 +28,11 @@ import java.nio.charset.StandardCharsets; import java.util.*; import okhttp3.RequestBody; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; public class CiTestCycleMapperV1 implements RemoteMapper { + private static final Logger LOGGER = LoggerFactory.getLogger(CiTestCycleMapperV1.class); private static final byte[] VERSION = "version".getBytes(StandardCharsets.UTF_8); private static final byte[] METADATA = "metadata".getBytes(StandardCharsets.UTF_8); @@ -154,6 +157,16 @@ public void map(List> trace, Writable writable) { version = 1; } + LOGGER.debug( + "Test span serialization - span: {}, sessionID: {}, moduleID: {}, suiteID: {}, traceID: {}, spanID: {}, parentID: {}", + span.getOperationName(), + testSessionId, + testModuleId, + testSuiteId, + traceId, + spanId, + parentId); + int contentChildrenCount = 8 + (traceId != null ? 1 : 0) diff --git a/dd-trace-core/src/main/java/datadog/trace/common/writer/ddagent/TraceMapperV0_4.java b/dd-trace-core/src/main/java/datadog/trace/common/writer/ddagent/TraceMapperV0_4.java index 379f7714a8c..2b9f4ff9a98 100644 --- a/dd-trace-core/src/main/java/datadog/trace/common/writer/ddagent/TraceMapperV0_4.java +++ b/dd-trace-core/src/main/java/datadog/trace/common/writer/ddagent/TraceMapperV0_4.java @@ -23,12 +23,10 @@ import okhttp3.RequestBody; import org.slf4j.Logger; import org.slf4j.LoggerFactory; -import datadog.trace.bootstrap.instrumentation.api.Tags; -import datadog.trace.bootstrap.instrumentation.api.InternalSpanTypes; public final class TraceMapperV0_4 implements TraceMapper { - private static final Logger log = LoggerFactory.getLogger(TraceMapperV0_4.class); + private static final Logger LOGGER = LoggerFactory.getLogger(TraceMapperV0_4.class); private final int size; @@ -258,6 +256,12 @@ public void map(List> trace, final Writable writable) { writable.startArray(trace.size()); for (int i = 0; i < trace.size(); i++) { final CoreSpan span = trace.get(i); + LOGGER.debug( + "Span serialization - span: {}, traceID: {}, spanID: {}, parentID: {}", + span.getOperationName(), + span.getTraceId().toLong(), + span.getSpanId(), + span.getParentId()); final Map metaStruct = span.getMetaStruct(); writable.startMap(metaStruct.isEmpty() ? 12 : 13); /* 1 */ @@ -291,26 +295,7 @@ public void map(List> trace, final Writable writable) { writable.writeUTF8(ERROR); writable.writeInt(span.getError()); /* 11, 12 */ - // Validation logging for missing test hierarchy IDs before serialization - if (InternalSpanTypes.TEST.equals(span.getType())) { - String sessionId = (String) span.getTag(Tags.TEST_SESSION_ID); - String moduleId = (String) span.getTag(Tags.TEST_MODULE_ID); - String suiteId = (String) span.getTag(Tags.TEST_SUITE_ID); - - if (sessionId == null) { - log.warn("Test span missing TEST_SESSION_ID before serialization: spanId={}, resource={}, traceId={}", - span.getSpanId(), span.getResourceName(), span.getTraceId()); - } - if (moduleId == null) { - log.warn("Test span missing TEST_MODULE_ID before serialization: spanId={}, resource={}, traceId={}", - span.getSpanId(), span.getResourceName(), span.getTraceId()); - } - if (suiteId == null) { - log.warn("Test span missing TEST_SUITE_ID before serialization: spanId={}, resource={}, traceId={}", - span.getSpanId(), span.getResourceName(), span.getTraceId()); - } - } - + span.processTagsAndBaggage( metaWriter .withWritable(writable) diff --git a/dd-trace-core/src/main/java/datadog/trace/common/writer/ddagent/TraceMapperV0_5.java b/dd-trace-core/src/main/java/datadog/trace/common/writer/ddagent/TraceMapperV0_5.java index 42057ca5990..761a8d1b861 100644 --- a/dd-trace-core/src/main/java/datadog/trace/common/writer/ddagent/TraceMapperV0_5.java +++ b/dd-trace-core/src/main/java/datadog/trace/common/writer/ddagent/TraceMapperV0_5.java @@ -25,12 +25,10 @@ import okhttp3.RequestBody; import org.slf4j.Logger; import org.slf4j.LoggerFactory; -import datadog.trace.bootstrap.instrumentation.api.Tags; -import datadog.trace.bootstrap.instrumentation.api.InternalSpanTypes; public final class TraceMapperV0_5 implements TraceMapper { - private static final Logger log = LoggerFactory.getLogger(TraceMapperV0_5.class); + private static final Logger LOGGER = LoggerFactory.getLogger(TraceMapperV0_5.class); private final WritableFormatter dictionaryWriter; private final DictionaryMapper dictionaryMapper = new DictionaryMapper(); @@ -62,6 +60,13 @@ public void map(final List> trace, final Writable writable writable.startArray(trace.size()); for (int i = 0; i < trace.size(); i++) { final CoreSpan span = trace.get(i); + LOGGER.debug( + "Span serialization - span: {}, traceID: {}, spanID: {}, parentID: {}", + span.getOperationName(), + span.getTraceId().toLong(), + span.getSpanId(), + span.getParentId()); + writable.startArray(12); /* 1 */ writeDictionaryEncoded(writable, span.getServiceName()); @@ -82,26 +87,6 @@ public void map(final List> trace, final Writable writable /* 9 */ writable.writeInt(span.getError()); /* 10, 11 */ - // Validation logging for missing test hierarchy IDs before serialization - if (InternalSpanTypes.TEST.equals(span.getType())) { - String sessionId = (String) span.getTag(Tags.TEST_SESSION_ID); - String moduleId = (String) span.getTag(Tags.TEST_MODULE_ID); - String suiteId = (String) span.getTag(Tags.TEST_SUITE_ID); - - if (sessionId == null) { - log.warn("Test span missing TEST_SESSION_ID before serialization: spanId={}, resource={}, traceId={}", - span.getSpanId(), span.getResourceName(), span.getTraceId()); - } - if (moduleId == null) { - log.warn("Test span missing TEST_MODULE_ID before serialization: spanId={}, resource={}, traceId={}", - span.getSpanId(), span.getResourceName(), span.getTraceId()); - } - if (suiteId == null) { - log.warn("Test span missing TEST_SUITE_ID before serialization: spanId={}, resource={}, traceId={}", - span.getSpanId(), span.getResourceName(), span.getTraceId()); - } - } - span.processTagsAndBaggage( metaWriter .withWritable(writable) From 9916a8adf4f9b09936c853c9aebd4c84fb0dfc48 Mon Sep 17 00:00:00 2001 From: Daniel Mohedano Date: Mon, 1 Sep 2025 13:00:39 +0200 Subject: [PATCH 11/11] additional debug --- .../serialization/msgpack/MsgPackWriter.java | 2 ++ .../writer/ddintake/CiTestCycleMapperV1.java | 13 ++++++++++++- .../trace/common/writer/TraceProcessingWorker.java | 8 ++++++++ 3 files changed, 22 insertions(+), 1 deletion(-) diff --git a/communication/src/main/java/datadog/communication/serialization/msgpack/MsgPackWriter.java b/communication/src/main/java/datadog/communication/serialization/msgpack/MsgPackWriter.java index 9d3d5154e74..4b888c0d269 100644 --- a/communication/src/main/java/datadog/communication/serialization/msgpack/MsgPackWriter.java +++ b/communication/src/main/java/datadog/communication/serialization/msgpack/MsgPackWriter.java @@ -81,6 +81,7 @@ public void flush() { @Override public boolean format(T message, Mapper mapper) { try { + log.debug("[ISSUE DEBUG] Beginning serialization"); mapper.map(message, this); buffer.mark(); return true; @@ -90,6 +91,7 @@ public boolean format(T message, Mapper mapper) { // max capacity, then reject the message if (buffer.flush()) { try { + log.debug("[ISSUE DEBUG] Beginning serialization after overflow and flush"); mapper.map(message, this); buffer.mark(); return true; diff --git a/dd-trace-core/src/main/java/datadog/trace/civisibility/writer/ddintake/CiTestCycleMapperV1.java b/dd-trace-core/src/main/java/datadog/trace/civisibility/writer/ddintake/CiTestCycleMapperV1.java index 31771651afe..72b8dcc21c4 100644 --- a/dd-trace-core/src/main/java/datadog/trace/civisibility/writer/ddintake/CiTestCycleMapperV1.java +++ b/dd-trace-core/src/main/java/datadog/trace/civisibility/writer/ddintake/CiTestCycleMapperV1.java @@ -27,6 +27,7 @@ import java.nio.channels.WritableByteChannel; import java.nio.charset.StandardCharsets; import java.util.*; +import java.util.stream.Collectors; import okhttp3.RequestBody; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -85,6 +86,11 @@ public CiTestCycleMapperV1(CiVisibilityWellKnownTags wellKnownTags, boolean comp public void map(List> trace, Writable writable) { long serializationStartTimestamp = System.currentTimeMillis(); + LOGGER.debug( + "[ISSUE DEBUG] Trace serialization - {}", + trace.stream() + .map(span -> String.format("(t_id=%s,s_id=%s)", span.getTraceId(), span.getSpanId())) + .collect(Collectors.joining(", ", "[", "]"))); for (final CoreSpan span : trace) { DDTraceId testSessionId = span.getTag(Tags.TEST_SESSION_ID); span.removeTag(Tags.TEST_SESSION_ID); @@ -158,7 +164,7 @@ public void map(List> trace, Writable writable) { } LOGGER.debug( - "Test span serialization - span: {}, sessionID: {}, moduleID: {}, suiteID: {}, traceID: {}, spanID: {}, parentID: {}", + "[ISSUE_DEBUG] Test span serialization - span: {}, sessionID: {}, moduleID: {}, suiteID: {}, traceID: {}, spanID: {}, parentID: {}", span.getOperationName(), testSessionId, testModuleId, @@ -234,6 +240,11 @@ public void map(List> trace, Writable writable) { writable.writeInt(span.getError()); /* 7 (meta), 8 (metrics) */ span.processTagsAndBaggage(metaWriter.withWritable(writable)); + + LOGGER.debug( + "[ISSUE_DEBUG] Test span serialization finished for - traceID: {}, spanID: {}", + traceId, + spanId); } eventCount += trace.size(); serializationTimeMillis += (int) (System.currentTimeMillis() - serializationStartTimestamp); diff --git a/dd-trace-core/src/main/java/datadog/trace/common/writer/TraceProcessingWorker.java b/dd-trace-core/src/main/java/datadog/trace/common/writer/TraceProcessingWorker.java index e5bddd5c48d..169394b9858 100644 --- a/dd-trace-core/src/main/java/datadog/trace/common/writer/TraceProcessingWorker.java +++ b/dd-trace-core/src/main/java/datadog/trace/common/writer/TraceProcessingWorker.java @@ -19,6 +19,7 @@ import java.util.concurrent.CountDownLatch; import java.util.concurrent.TimeUnit; import java.util.function.BooleanSupplier; +import java.util.stream.Collectors; import org.jctools.queues.MessagePassingQueue; import org.jctools.queues.MpscBlockingConsumerArrayQueue; import org.slf4j.Logger; @@ -184,6 +185,13 @@ public void onEvent(Object event) { try { if (event instanceof List) { List trace = (List) event; + log.debug( + "[ISSUE DEBUG] Processing event and adding to dispatcher - {}", + trace.stream() + .map( + span -> + String.format("(t_id=%s,s_id=%s)", span.getTraceId(), span.getSpanId())) + .collect(Collectors.joining(", ", "[", "]"))); maybeTracePostProcessing(trace); // TODO populate `_sample_rate` metric in a way that accounts for lost/dropped traces payloadDispatcher.addTrace(trace);